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

From: Harald Schmalzbauer <h_at_schmalzbauer.de>
Date: Mon, 17 Nov 2003 22:18:11 +0100
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
> _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 - 12:18:25 UTC

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