Re: acpicpu: remove acpicpu_sc array

2020-08-05 Thread Mark Kettenis
> Date: Wed, 5 Aug 2020 17:10:37 +1000
> From: Jonathan Matthew 
> 
> This came out of the work on supporting ACPI0007 devices in acpicpu(4), but
> it's independent of that and I'd like to get it in the tree separately.
> 
> Since it was first added, acpicpu stores instances of itself in an array, 
> which it uses to find the acpicpu device for a cpu.  This runs into problems
> when there are more than MAXCPUS acpicpu devices.  Currently it overwrites
> whatever's after the array, leading to varying crashes and hangs depending
> on kernel link order.
> 
> More recently, we've added a pointer to struct cpu_info that does this more
> directly, and also has the advantage that it actually matches up the cpu ids
> rather than assuming cpu3 maps to acpicpu3.
> 
> This diff removes the acpicpu_sc array and uses the pointer from struct
> cpu_info instead.  Most of the accesses are just looking for the first 
> acpicpu,
> so we can use cpu_info_primary to find that.
> 
> I've tested this on a few different machines (including one with 128 acpicpu
> devices) and everything still works.
> 
> ok?

ok kettenis@

> Index: acpicpu.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpicpu.c,v
> retrieving revision 1.85
> diff -u -p -r1.85 acpicpu.c
> --- acpicpu.c 27 May 2020 05:02:21 -  1.85
> +++ acpicpu.c 3 Aug 2020 05:10:45 -
> @@ -188,8 +188,6 @@ struct cfdriver acpicpu_cd = {
>  
>  extern int setperf_prio;
>  
> -struct acpicpu_softc *acpicpu_sc[MAXCPUS];
> -
>  #if 0
>  void
>  acpicpu_set_throttle(struct acpicpu_softc *sc, int level)
> @@ -672,7 +670,6 @@ acpicpu_attach(struct device *parent, st
>  
>   sc->sc_acpi = (struct acpi_softc *)parent;
>   sc->sc_devnode = aa->aaa_node;
> - acpicpu_sc[sc->sc_dev.dv_unit] = sc;
>  
>   SLIST_INIT(>sc_cstates);
>  
> @@ -979,7 +976,7 @@ acpicpu_fetch_pss(struct acpicpu_pss **p
>* the bios ensures this...
>*/
>  
> - sc = acpicpu_sc[0];
> + sc = (struct acpicpu_softc *)cpu_info_primary.ci_acpicpudev;
>   if (!sc)
>   return 0;
>   *pss = sc->sc_pss;
> @@ -1024,7 +1021,7 @@ acpicpu_set_notify(void (*func)(struct a
>  {
>   struct acpicpu_softc*sc;
>  
> - sc = acpicpu_sc[0];
> + sc = (struct acpicpu_softc *)cpu_info_primary.ci_acpicpudev;
>   if (sc != NULL)
>   sc->sc_notify = func;
>  }
> @@ -1034,7 +1031,7 @@ acpicpu_setperf_ppc_change(struct acpicp
>  {
>   struct acpicpu_softc*sc;
>  
> - sc = acpicpu_sc[0];
> + sc = (struct acpicpu_softc *)cpu_info_primary.ci_acpicpudev;
>  
>   if (sc != NULL)
>   cpu_setperf(sc->sc_level);
> @@ -1048,7 +1045,7 @@ acpicpu_setperf(int level)
>   int idx, len;
>   uint32_tstatus = 0;
>  
> - sc = acpicpu_sc[cpu_number()];
> + sc = (struct acpicpu_softc *)curcpu()->ci_acpicpudev;
>  
>   dnprintf(10, "%s: acpicpu setperf level %d\n",
>   sc->sc_devnode->name, level);
> 
> 



acpicpu: remove acpicpu_sc array

2020-08-05 Thread Jonathan Matthew
This came out of the work on supporting ACPI0007 devices in acpicpu(4), but
it's independent of that and I'd like to get it in the tree separately.

Since it was first added, acpicpu stores instances of itself in an array, 
which it uses to find the acpicpu device for a cpu.  This runs into problems
when there are more than MAXCPUS acpicpu devices.  Currently it overwrites
whatever's after the array, leading to varying crashes and hangs depending
on kernel link order.

More recently, we've added a pointer to struct cpu_info that does this more
directly, and also has the advantage that it actually matches up the cpu ids
rather than assuming cpu3 maps to acpicpu3.

This diff removes the acpicpu_sc array and uses the pointer from struct
cpu_info instead.  Most of the accesses are just looking for the first acpicpu,
so we can use cpu_info_primary to find that.

I've tested this on a few different machines (including one with 128 acpicpu
devices) and everything still works.

ok?


Index: acpicpu.c
===
RCS file: /cvs/src/sys/dev/acpi/acpicpu.c,v
retrieving revision 1.85
diff -u -p -r1.85 acpicpu.c
--- acpicpu.c   27 May 2020 05:02:21 -  1.85
+++ acpicpu.c   3 Aug 2020 05:10:45 -
@@ -188,8 +188,6 @@ struct cfdriver acpicpu_cd = {
 
 extern int setperf_prio;
 
-struct acpicpu_softc *acpicpu_sc[MAXCPUS];
-
 #if 0
 void
 acpicpu_set_throttle(struct acpicpu_softc *sc, int level)
@@ -672,7 +670,6 @@ acpicpu_attach(struct device *parent, st
 
sc->sc_acpi = (struct acpi_softc *)parent;
sc->sc_devnode = aa->aaa_node;
-   acpicpu_sc[sc->sc_dev.dv_unit] = sc;
 
SLIST_INIT(>sc_cstates);
 
@@ -979,7 +976,7 @@ acpicpu_fetch_pss(struct acpicpu_pss **p
 * the bios ensures this...
 */
 
-   sc = acpicpu_sc[0];
+   sc = (struct acpicpu_softc *)cpu_info_primary.ci_acpicpudev;
if (!sc)
return 0;
*pss = sc->sc_pss;
@@ -1024,7 +1021,7 @@ acpicpu_set_notify(void (*func)(struct a
 {
struct acpicpu_softc*sc;
 
-   sc = acpicpu_sc[0];
+   sc = (struct acpicpu_softc *)cpu_info_primary.ci_acpicpudev;
if (sc != NULL)
sc->sc_notify = func;
 }
@@ -1034,7 +1031,7 @@ acpicpu_setperf_ppc_change(struct acpicp
 {
struct acpicpu_softc*sc;
 
-   sc = acpicpu_sc[0];
+   sc = (struct acpicpu_softc *)cpu_info_primary.ci_acpicpudev;
 
if (sc != NULL)
cpu_setperf(sc->sc_level);
@@ -1048,7 +1045,7 @@ acpicpu_setperf(int level)
int idx, len;
uint32_tstatus = 0;
 
-   sc = acpicpu_sc[cpu_number()];
+   sc = (struct acpicpu_softc *)curcpu()->ci_acpicpudev;
 
dnprintf(10, "%s: acpicpu setperf level %d\n",
sc->sc_devnode->name, level);