Date: Mon, 17 Nov 2003 22:18:11 +0100 From: Harald Schmalzbauer <h@schmalzbauer.de> To: Nate Lawson <nate@root.org> Cc: current@freebsd.org Subject: Re: acpi_cpu_idle panic (Was: Re: kernel panic with todays source) Message-ID: <200311172218.16314@harrymail> In-Reply-To: <20031117094346.E60600@root.org> References: <20031116120622.O57495@root.org> <200311170420.22770@harrymail> <20031117094346.E60600@root.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--Boundary-02=_YsTu/Kdv9OyLS53
Content-Type: text/plain;
charset="iso-8859-15"
Content-Transfer-Encoding: quoted-printable
Content-Description: signed data
Content-Disposition: inline
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.=
=20
> > > 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=3D"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=20
loaded.
But the debug.acpi.disable=3D"cpu" tweak is still working. I'm using the ke=
rnel=20
with your patch(es) at the moment.
cale:/home/harry# sysctl hw.acpi.cpu
sysctl: unknown oid 'hw.acpi.cpu'
Thanks a lot,
=2DHarry
>
> Thanks,
> Nate
>
>
> Index: sys/dev/acpica/acpi_cpu.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> 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 =3D=3D 0 || AcpiGbl_FADT->Pm2CntLen =
=3D=3D 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 =3D 0;
> + sc->cpu_cx_count =3D 0;
> error =3D acpi_cpu_cx_cst(sc);
> if (error !=3D 0) {
> - if (cpu_p_blk_len !=3D 6)
> - return (ENXIO);
> cx_ptr =3D sc->cpu_cx_states;
>
> /* C1 has been required since just after ACPI 1.0 */
> @@ -432,7 +430,10 @@
> cx_ptr->trans_lat =3D 0;
> cpu_non_c3 =3D 0;
> cx_ptr++;
> - cpu_cx_count++;
> + sc->cpu_cx_count++;
> +
> + if (cpu_p_blk_len !=3D 6)
> + goto done;
>
> /* Validate and allocate resources for C2 (P_LVL2). */
> gas.AddressSpaceId =3D ACPI_ADR_SPACE_SYSTEM_IO;
> @@ -446,7 +447,7 @@
> cx_ptr->trans_lat =3D AcpiGbl_FADT->Plvl2Lat;
> cpu_non_c3 =3D 1;
> cx_ptr++;
> - cpu_cx_count++;
> + sc->cpu_cx_count++;
> }
> }
>
> @@ -461,40 +462,16 @@
> cx_ptr->type =3D ACPI_STATE_C3;
> cx_ptr->trans_lat =3D 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 =3D=3D 0)
> + if (sc->cpu_cx_count =3D=3D 0)
> return (ENXIO);
>
> - sbuf_new(&sb, cpu_cx_supported, sizeof(cpu_cx_supported),
> SBUF_FIXEDLEN); - for (i =3D 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 =3D cpu_cx_lowest;
> - cpu_idle_hook =3D acpi_cpu_idle;
> -
> return (0);
> }
>
> @@ -534,13 +511,12 @@
> count =3D 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 =3D MAX_CX_STATES;
> }
>
> /* Set up all valid states. */
> - cpu_cx_count =3D 0;
> + sc->cpu_cx_count =3D 0;
> cx_ptr =3D sc->cpu_cx_states;
> for (i =3D 0; i < count; i++) {
> pkg =3D &top->Package.Elements[i + 1];
> @@ -559,7 +535,7 @@
> case ACPI_STATE_C1:
> cpu_non_c3 =3D 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 =3D (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 =3D MAX_CX_STATES;
> + for (i =3D 0; i < cpu_ndevices; i++)
> + cpu_cx_count =3D min(sc->cpu_cx_count, count);
> +
> + /* Perform throttling and Cx final initialization. */
> if (sc->cpu_p_cnt !=3D 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 =3D CPU_MAX_SPEED;
> @@ -654,10 +640,11 @@
> }
>
> /* If ACPI 2.0+, signal platform that we are taking over throttling.
> */ - if (cpu_pstate_cnt !=3D 0)
> + if (cpu_pstate_cnt !=3D 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 =3D device_get_softc(cpu_devices[0]);
> + sbuf_new(&sb, cpu_cx_supported, sizeof(cpu_cx_supported),
> SBUF_FIXEDLEN); + for (i =3D 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 =3D cpu_cx_lowest;
> +
> + /* Take over idling from cpu_idle_default(). */
> + cpu_idle_hook =3D acpi_cpu_idle;
> +}
> +
> /*
> * Set CPUs to the new state.
> *
> @@ -727,7 +750,7 @@
> KASSERT(sc !=3D NULL, ("NULL softc for %d", PCPU_GET(acpi_id)));
>
> /* If disabled, return immediately. */
> - if (cpu_cx_lowest < 0 || cpu_cx_count =3D=3D 0) {
> + if (cpu_cx_count =3D=3D 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 =3D device_get_softc(cpu_devices[0]);
> val =3D cpu_cx_lowest;
> error =3D sysctl_handle_int(oidp, &val, 0, req);
> if (error !=3D 0 || req->newptr =3D=3D 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
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> 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.
--Boundary-02=_YsTu/Kdv9OyLS53
Content-Type: application/pgp-signature
Content-Description: signature
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3 (FreeBSD)
iD8DBQA/uTsYBylq0S4AzzwRAsK9AJ4kMQQNx7PkIsx5zKYUkINq8vmwtQCgh/4N
E157oFXgC6WfbF6EAjrnpzw=
=qrfJ
-----END PGP SIGNATURE-----
--Boundary-02=_YsTu/Kdv9OyLS53--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200311172218.16314>