Re: acpi_cpu_idle panic (Was: Re: kernel panic with todays source)

From: Nate Lawson <nate_at_root.org>
Date: Mon, 17 Nov 2003 09:45:58 -0800 (PST)
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.

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
_at__at_ -78,8 +78,8 _at__at_
     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) 					\
_at__at_ -151,6 +151,7 _at__at_
 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);
_at__at_ -406,8 +407,7 _at__at_
 {
     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) {
_at__at_ -420,11 +420,9 _at__at_
      * 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 */
_at__at_ -432,7 +430,10 _at__at_
 	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;
_at__at_ -446,7 +447,7 _at__at_
 		cx_ptr->trans_lat = AcpiGbl_FADT->Plvl2Lat;
 		cpu_non_c3 = 1;
 		cx_ptr++;
-		cpu_cx_count++;
+		sc->cpu_cx_count++;
 	    }
 	}

_at__at_ -461,40 +462,16 _at__at_
 		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);
 }

_at__at_ -534,13 +511,12 _at__at_
 	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];
_at__at_ -559,7 +535,7 _at__at_
 	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) {
_at__at_ -594,7 +570,7 _at__at_
 	    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);
_at__at_ -604,17 +580,12 _at__at_

 /*
  * 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);
_at__at_ -623,16 +594,31 _at__at_
     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;
_at__at_ -654,10 +640,11 _at__at_
     }

     /* 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);
_at__at_ -667,6 +654,42 _at__at_
 	   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.
  *
_at__at_ -727,7 +750,7 _at__at_
     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;
     }
_at__at_ -1020,14 +1043,15 _at__at_
 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
_at__at_ -342,7 +342,6 _at__at_
 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.
Received on Mon Nov 17 2003 - 08:45:58 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:37:29 UTC