On Monday 17 November 2003 18:45, Nate Lawson wrote:
> On Mon, 17 Nov 2003, Harald Schmalzbauer wrote:
> > On Sunday 16 November 2003 21:11, Nate Lawson wrote:
> > > The panic you see is a result of the new acpi_cpu driver, not ULE.  In
> > > any case, it appears that acpi_cpu_idle is being called and trying to
> > > read one of the processor control registers before they are present. 
> > > Please send me privately the output of:
> > >    acpidump -t -d > harald-MachineType.asl
> > >
> > > As a workaround, please set this in loader.conf:
> > >    debug.acpi.disable="cpu"
> > >
> > > That will allow you to get running and give me some time to look into
> > > this.
> >
> > Now I followed your advise and found out the following (source from some
> > hours ago, 4BSD scheduler, and the acpidump went away to you by private
> > mail) The panic only occurs if the nvidia.ko module is was loaded.
> > I use it straight from the ports.
> > But your sysctl tweak keeps the whole thing working (I'm writing with
> > kmail)!
>
> Please try the patch below.  It does more than fix your problem but the
> other changes will be needed eventually anyways.  If your system boots ok,
> please send me the output of sysctl hw.acpi.cpu
>
> Also, be sure to comment out the disable CPU line in loader.conf while
> testing the patch.
Sorry, things got worse. Now it panics even if the nvidia module is not 
loaded.
But the debug.acpi.disable="cpu" tweak is still working. I'm using the kernel 
with your patch(es) at the moment.

cale:/home/harry# sysctl hw.acpi.cpu
sysctl: unknown oid 'hw.acpi.cpu'

Thanks a lot,

-Harry

>
> Thanks,
> Nate
>
>
> Index: sys/dev/acpica/acpi_cpu.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/dev/acpica/acpi_cpu.c,v
> retrieving revision 1.19
> diff -u -r1.19 acpi_cpu.c
> --- sys/dev/acpica/acpi_cpu.c 15 Nov 2003 19:26:05 -0000      1.19
> +++ sys/dev/acpica/acpi_cpu.c 17 Nov 2003 17:39:54 -0000
> @@ -78,8 +78,8 @@
>      ACPI_HANDLE               cpu_handle;
>      uint32_t          cpu_id;        /* ACPI processor id */
>      struct resource  *cpu_p_cnt;     /* Throttling control register */
> +    int                       cpu_cx_count;  /* Number of valid Cx states. */
>      struct acpi_cx    cpu_cx_states[MAX_CX_STATES];
> -    int                       cpu_bm_ok;     /* Bus mastering control available. */
>  };
>
>  #define CPU_GET_REG(reg, width)                                      \
> @@ -151,6 +151,7 @@
>  static int   acpi_cpu_cx_cst(struct acpi_cpu_softc *sc);
>  static void  acpi_cpu_startup(void *arg);
>  static void  acpi_cpu_startup_throttling(void);
> +static void  acpi_cpu_startup_cx(void);
>  static void  acpi_cpu_throttle_set(uint32_t speed);
>  static void  acpi_cpu_idle(void);
>  static void  acpi_cpu_c1(void);
> @@ -406,8 +407,7 @@
>  {
>      ACPI_GENERIC_ADDRESS gas;
>      struct acpi_cx   *cx_ptr;
> -    struct sbuf               sb;
> -    int                       i, error;
> +    int                       error;
>
>      /* Bus mastering arbitration control is needed for C3. */
>      if (AcpiGbl_FADT->V1_Pm2CntBlk == 0 || AcpiGbl_FADT->Pm2CntLen == 0) {
> @@ -420,11 +420,9 @@
>       * First, check for the ACPI 2.0 _CST sleep states object.
>       * If not usable, fall back to the P_BLK's P_LVL2 and P_LVL3.
>       */
> -    cpu_cx_count = 0;
> +    sc->cpu_cx_count = 0;
>      error = acpi_cpu_cx_cst(sc);
>      if (error != 0) {
> -     if (cpu_p_blk_len != 6)
> -         return (ENXIO);
>       cx_ptr = sc->cpu_cx_states;
>
>       /* C1 has been required since just after ACPI 1.0 */
> @@ -432,7 +430,10 @@
>       cx_ptr->trans_lat = 0;
>       cpu_non_c3 = 0;
>       cx_ptr++;
> -     cpu_cx_count++;
> +     sc->cpu_cx_count++;
> +
> +     if (cpu_p_blk_len != 6)
> +         goto done;
>
>       /* Validate and allocate resources for C2 (P_LVL2). */
>       gas.AddressSpaceId = ACPI_ADR_SPACE_SYSTEM_IO;
> @@ -446,7 +447,7 @@
>               cx_ptr->trans_lat = AcpiGbl_FADT->Plvl2Lat;
>               cpu_non_c3 = 1;
>               cx_ptr++;
> -             cpu_cx_count++;
> +             sc->cpu_cx_count++;
>           }
>       }
>
> @@ -461,40 +462,16 @@
>               cx_ptr->type = ACPI_STATE_C3;
>               cx_ptr->trans_lat = AcpiGbl_FADT->Plvl3Lat;
>               cx_ptr++;
> -             cpu_cx_count++;
> +             sc->cpu_cx_count++;
>           }
>       }
>      }
>
> +done:
>      /* If no valid registers were found, don't attach. */
> -    if (cpu_cx_count == 0)
> +    if (sc->cpu_cx_count == 0)
>       return (ENXIO);
>
> -    sbuf_new(&sb, cpu_cx_supported, sizeof(cpu_cx_supported),
> SBUF_FIXEDLEN); -    for (i = 0; i < cpu_cx_count; i++) {
> -     sbuf_printf(&sb, "C%d/%d ", sc->cpu_cx_states[i].type,
> -                 sc->cpu_cx_states[i].trans_lat);
> -    }
> -    sbuf_trim(&sb);
> -    sbuf_finish(&sb);
> -    SYSCTL_ADD_STRING(&acpi_cpu_sysctl_ctx,
> -                   SYSCTL_CHILDREN(acpi_cpu_sysctl_tree),
> -                   OID_AUTO, "cx_supported", CTLFLAG_RD, cpu_cx_supported,
> -                   0, "Cx/microsecond values for supported Cx states");
> -    SYSCTL_ADD_PROC(&acpi_cpu_sysctl_ctx,
> -                 SYSCTL_CHILDREN(acpi_cpu_sysctl_tree),
> -                 OID_AUTO, "cx_lowest", CTLTYPE_INT | CTLFLAG_RW,
> -                 NULL, 0, acpi_cpu_cx_lowest_sysctl, "I",
> -                 "lowest Cx sleep state to use");
> -    SYSCTL_ADD_PROC(&acpi_cpu_sysctl_ctx,
> -                 SYSCTL_CHILDREN(acpi_cpu_sysctl_tree),
> -                 OID_AUTO, "cx_history", CTLTYPE_STRING | CTLFLAG_RD,
> -                 NULL, 0, acpi_cpu_history_sysctl, "A", "");
> -
> -    /* Set next sleep state and hook the idle function. */
> -    cpu_cx_next = cpu_cx_lowest;
> -    cpu_idle_hook = acpi_cpu_idle;
> -
>      return (0);
>  }
>
> @@ -534,13 +511,12 @@
>       count = top->Package.Count - 1;
>      }
>      if (count > MAX_CX_STATES) {
> -     device_printf(sc->cpu_dev, "_CST has too many states (%d)\n",
> -                   count);
> +     device_printf(sc->cpu_dev, "_CST has too many states (%d)\n", count);
>       count = MAX_CX_STATES;
>      }
>
>      /* Set up all valid states. */
> -    cpu_cx_count = 0;
> +    sc->cpu_cx_count = 0;
>      cx_ptr = sc->cpu_cx_states;
>      for (i = 0; i < count; i++) {
>       pkg = &top->Package.Elements[i + 1];
> @@ -559,7 +535,7 @@
>       case ACPI_STATE_C1:
>           cpu_non_c3 = i;
>           cx_ptr++;
> -         cpu_cx_count++;
> +         sc->cpu_cx_count++;
>           continue;
>       case ACPI_STATE_C2:
>           if (cx_ptr->trans_lat > 100) {
> @@ -594,7 +570,7 @@
>           device_printf(sc->cpu_dev, "C%d state %d lat\n", cx_ptr->type,
>                         cx_ptr->trans_lat);
>           cx_ptr++;
> -         cpu_cx_count++;
> +         sc->cpu_cx_count++;
>       }
>      }
>      AcpiOsFree(buf.Pointer);
> @@ -604,17 +580,12 @@
>
>  /*
>   * Call this *after* all CPUs have been attached.
> - *
> - * Takes the ACPI lock to avoid fighting anyone over the SMI command
> - * port.  Could probably lock less code.
>   */
>  static void
>  acpi_cpu_startup(void *arg)
>  {
>      struct acpi_cpu_softc *sc = (struct acpi_cpu_softc *)arg;
> -    ACPI_LOCK_DECL;
> -
> -    ACPI_LOCK;
> +    int count, i;
>
>      /* Get set of CPU devices */
>      devclass_get_devices(acpi_cpu_devclass, &cpu_devices, &cpu_ndevices);
> @@ -623,16 +594,31 @@
>      EVENTHANDLER_REGISTER(power_profile_change, acpi_cpu_power_profile,
>                         NULL, 0);
>
> +    /*
> +     * Make sure all the processors' Cx counts match.  We should probably
> +     * also check the contents of each.  However, no known systems have
> +     * non-matching Cx counts so we'll deal with this later.
> +     */
> +    count = MAX_CX_STATES;
> +    for (i = 0; i < cpu_ndevices; i++)
> +     cpu_cx_count = min(sc->cpu_cx_count, count);
> +
> +    /* Perform throttling and Cx final initialization. */
>      if (sc->cpu_p_cnt != NULL)
>       acpi_cpu_startup_throttling();
> -    else
> -     ACPI_UNLOCK;
> +    if (cpu_cx_count > 0)
> +     acpi_cpu_startup_cx();
>  }
>
> +/*
> + * Takes the ACPI lock to avoid fighting anyone over the SMI command
> + * port.
> + */
>  static void
>  acpi_cpu_startup_throttling()
>  {
>      int cpu_temp_speed;
> +    ACPI_LOCK_DECL;
>
>      /* Initialise throttling states */
>      cpu_max_state = CPU_MAX_SPEED;
> @@ -654,10 +640,11 @@
>      }
>
>      /* If ACPI 2.0+, signal platform that we are taking over throttling.
> */ -    if (cpu_pstate_cnt != 0)
> +    if (cpu_pstate_cnt != 0) {
> +     ACPI_LOCK;
>       AcpiOsWritePort(cpu_smi_cmd, cpu_pstate_cnt, 8);
> -
> -    ACPI_UNLOCK;
> +     ACPI_UNLOCK;
> +    }
>
>      /* Set initial speed */
>      acpi_cpu_power_profile(NULL);
> @@ -667,6 +654,42 @@
>          CPU_SPEED_PRINTABLE(cpu_current_state));
>  }
>
> +static void
> +acpi_cpu_startup_cx()
> +{
> +    struct acpi_cpu_softc *sc;
> +    struct sbuf               sb;
> +    int i;
> +
> +    sc = device_get_softc(cpu_devices[0]);
> +    sbuf_new(&sb, cpu_cx_supported, sizeof(cpu_cx_supported),
> SBUF_FIXEDLEN); +    for (i = 0; i < cpu_cx_count; i++) {
> +     sbuf_printf(&sb, "C%d/%d ", sc->cpu_cx_states[i].type,
> +                 sc->cpu_cx_states[i].trans_lat);
> +    }
> +    sbuf_trim(&sb);
> +    sbuf_finish(&sb);
> +    SYSCTL_ADD_STRING(&acpi_cpu_sysctl_ctx,
> +                   SYSCTL_CHILDREN(acpi_cpu_sysctl_tree),
> +                   OID_AUTO, "cx_supported", CTLFLAG_RD, cpu_cx_supported,
> +                   0, "Cx/microsecond values for supported Cx states");
> +    SYSCTL_ADD_PROC(&acpi_cpu_sysctl_ctx,
> +                 SYSCTL_CHILDREN(acpi_cpu_sysctl_tree),
> +                 OID_AUTO, "cx_lowest", CTLTYPE_INT | CTLFLAG_RW,
> +                 NULL, 0, acpi_cpu_cx_lowest_sysctl, "I",
> +                 "lowest Cx sleep state to use");
> +    SYSCTL_ADD_PROC(&acpi_cpu_sysctl_ctx,
> +                 SYSCTL_CHILDREN(acpi_cpu_sysctl_tree),
> +                 OID_AUTO, "cx_history", CTLTYPE_STRING | CTLFLAG_RD,
> +                 NULL, 0, acpi_cpu_history_sysctl, "A", "");
> +
> +    /* Set next sleep state and hook the idle function. */
> +    cpu_cx_next = cpu_cx_lowest;
> +
> +    /* Take over idling from cpu_idle_default(). */
> +    cpu_idle_hook = acpi_cpu_idle;
> +}
> +
>  /*
>   * Set CPUs to the new state.
>   *
> @@ -727,7 +750,7 @@
>      KASSERT(sc != NULL, ("NULL softc for %d", PCPU_GET(acpi_id)));
>
>      /* If disabled, return immediately. */
> -    if (cpu_cx_lowest < 0 || cpu_cx_count == 0) {
> +    if (cpu_cx_count == 0) {
>       ACPI_ENABLE_IRQS();
>       return;
>      }
> @@ -1020,14 +1043,15 @@
>  acpi_cpu_cx_lowest_sysctl(SYSCTL_HANDLER_ARGS)
>  {
>      struct    acpi_cpu_softc *sc;
> -    int               val, error, i;
> +    int               error, i;
> +    uint32_t  val;
>
>      sc = device_get_softc(cpu_devices[0]);
>      val = cpu_cx_lowest;
>      error = sysctl_handle_int(oidp, &val, 0, req);
>      if (error != 0 || req->newptr == NULL)
>       return (error);
> -    if (val < -1 || val > cpu_cx_count - 1)
> +    if (val > cpu_cx_count - 1)
>       return (EINVAL);
>
>      /* Use the new value for the next idle slice. */
> Index: share/man/man4/acpi.4
> ===================================================================
> RCS file: /home/ncvs/src/share/man/man4/acpi.4,v
> retrieving revision 1.17
> diff -u -r1.17 acpi.4
> --- share/man/man4/acpi.4     15 Nov 2003 19:26:05 -0000      1.17
> +++ share/man/man4/acpi.4     17 Nov 2003 17:18:09 -0000
> @@ -342,7 +342,6 @@
>  is modified.
>  .It Va hw.acpi.cpu.cx_lowest
>  Zero-based index of the lowest CPU idle state to use.
> -A value of -1 disables ACPI CPU idle states.
>  To enable ACPI CPU idling control,
>  .Va machdep.cpu_idle_hlt
>  must be set to 1.

Attachment: pgp00000.pgp
Description: signature

Reply via email to