Re: acpicpu(4) and ACPI0007

2020-08-01 Thread Mark Kettenis
> Date: Sat, 1 Aug 2020 18:23:08 +1000
> From: Jonathan Matthew 
> Cc: tech@openbsd.org
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
> 
> On Wed, Jul 29, 2020 at 08:29:31PM +1000, Jonathan Matthew wrote:
> > On Wed, Jul 29, 2020 at 10:06:14AM +0200, Mark Kettenis wrote:
> > > > Date: Wed, 29 Jul 2020 10:38:55 +1000
> > > > From: Jonathan Matthew 
> > > > 
> > > > On Tue, Jul 28, 2020 at 07:30:36PM +0200, Mark Kettenis wrote:
> > > > > > Date: Tue, 28 Jul 2020 21:42:46 +1000
> > > > > > From: Jonathan Matthew 
> > > > > > 
> > > > > > On Tue, Jul 28, 2020 at 11:12:21AM +0200, Mark Kettenis wrote:
> > > > > > > > Date: Tue, 28 Jul 2020 13:46:34 +1000
> > > > > > > > From: Jonathan Matthew 
> > > > > > > > 
> > > > > > > > On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > > > > > > > > > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > > > > > > > > > From: Mark Kettenis 
> > > > > > > > > > 
> > > > > > > > > > Recent ACPI versions have deprecated "Processor()" nodes in 
> > > > > > > > > > favout of
> > > > > > > > > > "Device()" nodes with a _HID() method that returns 
> > > > > > > > > > "ACPI0007".  This
> > > > > > > > > > diff tries to support machines with firmware that 
> > > > > > > > > > implements this.  If
> > > > > > > > > > you see something like:
> > > > > > > > > > 
> > > > > > > > > >   "ACPI0007" at acpi0 not configured
> > > > > > > > > > 
> > > > > > > > > > please try the following diff and report back with an 
> > > > > > > > > > updated dmesg.
> > > > > > > > > > 
> > > > > > > > > > Cheers,
> > > > > > > > > > 
> > > > > > > > > > Mark
> > > > > > > > > 
> > > > > > > > > And now with the right diff...
> > > > > > > > 
> > > > > > > > On a dell r6415, it looks like this:
> > > > > > > > 
> > > > > > > > acpicpu0 at acpi0copyvalue: 6: C1(@1 halt!)
> > > > > > > > all the way up to
> > > > > > > > acpicpu127 at acpi0copyvalue: 6: no cpu matching ACPI ID 127
> > > > > > > > 
> > > > > > > > which I guess means aml_copyvalue() needs to learn how to copy 
> > > > > > > > AML_OBJTYPE_DEVICE.
> > > > > > > 
> > > > > > > Yes.  It is not immediately obvious how this should work.  Do we 
> > > > > > > need
> > > > > > > to copy the aml_node pointer or not?  We don't do that for
> > > > > > > AML_OBJTYPE_PROCESSOR and AML_OBJTYPE_POWERRSRC types which are
> > > > > > > similar to AML_OBJTYPE_DEVICE.  But AML_OBJTYPE_DEVICE object 
> > > > > > > don't
> > > > > > > carry any additional information.  So we end up with just an empty
> > > > > > > case to avoid the warning.
> > > > > > > 
> > > > > > > Does this work on the Dell machines?
> > > > > > 
> > > > > > We've seen crashes in pool_cache_get() in various places after all 
> > > > > > the acpicpus
> > > > > > attach, which we haven't seen before on these machines, so I think 
> > > > > > it's
> > > > > > corrupting memory somehow.
> > > > > 
> > > > > Does that happen with only the acpicpu(4) diff?
> > > > 
> > > > Yes.  Looking at this a bit more, in the case where aml_evalnode() can't
> > > > copy the result value, it leaves it uninitialised, which means we'll 
> > > > call
> > > > aml_freevalue() where res is stack junk.  memset(, 0, 
> > > > sizeof(res))
> > > > seems to fix it.
> > > 
> > > Eh, where exactly?
> > 
> > I had it just before the call to aml_evalnode(), but that can't be it,
> > since aml_evalnode() does the same thing.
> 
> Much better theory: the acpicpu_sc array has MAXCPUS elements, but on this
> system (and all R6415s, as far as I can tell) we have more acpicpu devices
> than that.  I suppose we should just make acpicpu_match fail if cf->cf_unit
> is >= MAXCPUS as we do with the actual cpu devices.

Yes, that makes more sense.

There are a couple of problems with this approach:

1. The set of "active" CPUs might not actually be covered by the first
   MAXCPUS ACPI0007 devices that appear in the ACPI device tree.

2. There are machines that have both Processor() nodes and ACPI0007
   Device() nodes.  See for example the machine from the "no output on
   glass console after switching to serial" post from sthen@ on tech@.

To address #1 we probably should check for a matching CPU early in
acpicpu_attach() and only add it to the acpicpu_sc[] array if there is
a match.  The current code is actually broken since the array is
indexed by acpicpu(4) dv_unit in places but cpu_number() in other
places.

To address #2 we could move the check for Processor() nodes later and
skip it if any acpicpu(4) devices already attached.

Cheers,

Mark

P.S. I don't have amd64 hardware with ACPI0007 devices, so if you want
 to take this diff, that is fine with me.  If not, let me know and
 I'll try to come up with a suitable diff.


> 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 

Re: acpicpu(4) and ACPI0007

2020-08-01 Thread Jan Stary
On Aug 01 18:23:08, jonat...@d14n.org wrote:
> Much better theory: the acpicpu_sc array has MAXCPUS elements, but on this
> system (and all R6415s, as far as I can tell) we have more acpicpu devices
> than that.  I suppose we should just make acpicpu_match fail if cf->cf_unit
> is >= MAXCPUS as we do with the actual cpu devices.
> 
> 
> 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 1 Aug 2020 08:18:49 -
> @@ -186,6 +186,11 @@ struct cfdriver acpicpu_cd = {
>   NULL, "acpicpu", DV_DULL
>  };
>  
> +const char *acpicpu_hids[] = {
> + "ACPI0007",
> + NULL
> +};
> +
>  extern int setperf_prio;
>  
>  struct acpicpu_softc *acpicpu_sc[MAXCPUS];
> @@ -650,6 +655,12 @@ acpicpu_match(struct device *parent, voi
>   struct acpi_attach_args *aa = aux;
>   struct cfdata   *cf = match;
>  
> + if (cf->cf_unit >= MAXCPUS)
> + return (0);
> +
> + if (acpi_matchhids(aa, acpicpu_hids, cf->cf_driver->cd_name))
> + return (1);
> +
>   /* sanity */
>   if (aa->aaa_name == NULL ||
>   strcmp(aa->aaa_name, cf->cf_driver->cd_name) != 0 ||
> @@ -665,6 +676,7 @@ acpicpu_attach(struct device *parent, st
>   struct acpicpu_softc*sc = (struct acpicpu_softc *)self;
>   struct acpi_attach_args *aa = aux;
>   struct aml_valueres;
> + int64_t uid;
>   int i;
>   uint32_tstatus = 0;
>   CPU_INFO_ITERATOR   cii;
> @@ -675,6 +687,10 @@ acpicpu_attach(struct device *parent, st
>   acpicpu_sc[sc->sc_dev.dv_unit] = sc;
>  
>   SLIST_INIT(>sc_cstates);
> +
> + if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode,
> + "_UID", 0, NULL, ) == 0)
> + sc->sc_cpu = uid;
>  
>   if (aml_evalnode(sc->sc_acpi, sc->sc_devnode, 0, NULL, ) == 0) {
>   if (res.type == AML_OBJTYPE_PROCESSOR) {
> 

See below for the old and the new dmesg of current/amd64 at an APU2.

Edited highlights of the diff:

--- apu2e.20200610  Wed Jun 10 22:24:25 2020
+++ apu2e.20200801  Sat Aug  1 19:17:00 2020
-"ACPI0007" at acpi0 not configured
-"ACPI0007" at acpi0 not configured
-"ACPI0007" at acpi0 not configured
-"ACPI0007" at acpi0 not configured
-"ACPI0007" at acpi0 not configured
-"ACPI0007" at acpi0 not configured
-"ACPI0007" at acpi0 not configured
-"ACPI0007" at acpi0 not configured
+acpicpu0 at acpi0copyvalue: 6: C2(0@400 io@0x1771), C1(@1 halt!), PSS
+acpicpu1 at acpi0copyvalue: 6: C2(0@400 io@0x1771), C1(@1 halt!), PSS
+acpicpu2 at acpi0copyvalue: 6: C2(0@400 io@0x1771), C1(@1 halt!), PSS
+acpicpu3 at acpi0copyvalue: 6: C2(0@400 io@0x1771), C1(@1 halt!), PSS
+acpicpu4 at acpi0copyvalue: 6: no cpu matching ACPI ID 4
+acpicpu5 at acpi0copyvalue: 6: no cpu matching ACPI ID 5
+acpicpu6 at acpi0copyvalue: 6: no cpu matching ACPI ID 6
+acpicpu7 at acpi0copyvalue: 6: no cpu matching ACPI ID 7
+cpu0: 998 MHz: speeds: 1000 800 600 MHz

Jan


OpenBSD 6.7-current (GENERIC.MP) #0: Wed Jun 10 22:14:40 CEST 2020
h...@uvt.stare.cz:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 1996484608 (1903MB)
avail mem = 1921105920 (1832MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.8 @ 0x7ee8d020 (13 entries)
bios0: vendor coreboot version "v4.11.0.5" date 03/29/2020
bios0: PC Engines apu2
acpi0 at bios0: ACPI 4.0
acpi0: sleep states S0 S1 S4 S5
acpi0: tables DSDT FACP SSDT MCFG TPM2 APIC HEST SSDT SSDT HPET
acpi0: wakeup devices PWRB(S4) PBR4(S4) PBR5(S4) PBR6(S4) PBR7(S4) PBR8(S4) 
UOH1(S3) UOH2(S3) UOH3(S3) UOH4(S3) UOH5(S3) UOH6(S3) XHC0(S4)
acpitimer0 at acpi0: 3579545 Hz, 32 bits
acpimcfg0 at acpi0
acpimcfg0: addr 0xf800, bus 0-64
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: AMD GX-412TC SOC, 998.26 MHz, 16-30-01
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,SKINIT,TOPEXT,DBKP,PERFTSC,PCTRL3,ITSC,BMI1,XSAVEOPT
cpu0: 32KB 64b/line 2-way I-cache, 32KB 64b/line 8-way D-cache, 2MB 64b/line 
16-way L2 cache
cpu0: ITLB 32 4KB entries fully associative, 8 4MB entries fully associative
cpu0: DTLB 40 4KB entries fully associative, 8 4MB entries fully associative
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
cpu0: apic clock running at 99MHz
cpu0: mwait min=64, max=64, IBE
cpu1 at mainbus0: apid 1 (application processor)
cpu1: AMD GX-412TC SOC, 998.14 MHz, 16-30-01
cpu1: 

Re: acpicpu(4) and ACPI0007

2020-08-01 Thread Jonathan Matthew
On Wed, Jul 29, 2020 at 08:29:31PM +1000, Jonathan Matthew wrote:
> On Wed, Jul 29, 2020 at 10:06:14AM +0200, Mark Kettenis wrote:
> > > Date: Wed, 29 Jul 2020 10:38:55 +1000
> > > From: Jonathan Matthew 
> > > 
> > > On Tue, Jul 28, 2020 at 07:30:36PM +0200, Mark Kettenis wrote:
> > > > > Date: Tue, 28 Jul 2020 21:42:46 +1000
> > > > > From: Jonathan Matthew 
> > > > > 
> > > > > On Tue, Jul 28, 2020 at 11:12:21AM +0200, Mark Kettenis wrote:
> > > > > > > Date: Tue, 28 Jul 2020 13:46:34 +1000
> > > > > > > From: Jonathan Matthew 
> > > > > > > 
> > > > > > > On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > > > > > > > > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > > > > > > > > From: Mark Kettenis 
> > > > > > > > > 
> > > > > > > > > Recent ACPI versions have deprecated "Processor()" nodes in 
> > > > > > > > > favout of
> > > > > > > > > "Device()" nodes with a _HID() method that returns 
> > > > > > > > > "ACPI0007".  This
> > > > > > > > > diff tries to support machines with firmware that implements 
> > > > > > > > > this.  If
> > > > > > > > > you see something like:
> > > > > > > > > 
> > > > > > > > >   "ACPI0007" at acpi0 not configured
> > > > > > > > > 
> > > > > > > > > please try the following diff and report back with an updated 
> > > > > > > > > dmesg.
> > > > > > > > > 
> > > > > > > > > Cheers,
> > > > > > > > > 
> > > > > > > > > Mark
> > > > > > > > 
> > > > > > > > And now with the right diff...
> > > > > > > 
> > > > > > > On a dell r6415, it looks like this:
> > > > > > > 
> > > > > > > acpicpu0 at acpi0copyvalue: 6: C1(@1 halt!)
> > > > > > > all the way up to
> > > > > > > acpicpu127 at acpi0copyvalue: 6: no cpu matching ACPI ID 127
> > > > > > > 
> > > > > > > which I guess means aml_copyvalue() needs to learn how to copy 
> > > > > > > AML_OBJTYPE_DEVICE.
> > > > > > 
> > > > > > Yes.  It is not immediately obvious how this should work.  Do we 
> > > > > > need
> > > > > > to copy the aml_node pointer or not?  We don't do that for
> > > > > > AML_OBJTYPE_PROCESSOR and AML_OBJTYPE_POWERRSRC types which are
> > > > > > similar to AML_OBJTYPE_DEVICE.  But AML_OBJTYPE_DEVICE object don't
> > > > > > carry any additional information.  So we end up with just an empty
> > > > > > case to avoid the warning.
> > > > > > 
> > > > > > Does this work on the Dell machines?
> > > > > 
> > > > > We've seen crashes in pool_cache_get() in various places after all 
> > > > > the acpicpus
> > > > > attach, which we haven't seen before on these machines, so I think 
> > > > > it's
> > > > > corrupting memory somehow.
> > > > 
> > > > Does that happen with only the acpicpu(4) diff?
> > > 
> > > Yes.  Looking at this a bit more, in the case where aml_evalnode() can't
> > > copy the result value, it leaves it uninitialised, which means we'll call
> > > aml_freevalue() where res is stack junk.  memset(, 0, sizeof(res))
> > > seems to fix it.
> > 
> > Eh, where exactly?
> 
> I had it just before the call to aml_evalnode(), but that can't be it,
> since aml_evalnode() does the same thing.

Much better theory: the acpicpu_sc array has MAXCPUS elements, but on this
system (and all R6415s, as far as I can tell) we have more acpicpu devices
than that.  I suppose we should just make acpicpu_match fail if cf->cf_unit
is >= MAXCPUS as we do with the actual cpu devices.


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   1 Aug 2020 08:18:49 -
@@ -186,6 +186,11 @@ struct cfdriver acpicpu_cd = {
NULL, "acpicpu", DV_DULL
 };
 
+const char *acpicpu_hids[] = {
+   "ACPI0007",
+   NULL
+};
+
 extern int setperf_prio;
 
 struct acpicpu_softc *acpicpu_sc[MAXCPUS];
@@ -650,6 +655,12 @@ acpicpu_match(struct device *parent, voi
struct acpi_attach_args *aa = aux;
struct cfdata   *cf = match;
 
+   if (cf->cf_unit >= MAXCPUS)
+   return (0);
+
+   if (acpi_matchhids(aa, acpicpu_hids, cf->cf_driver->cd_name))
+   return (1);
+
/* sanity */
if (aa->aaa_name == NULL ||
strcmp(aa->aaa_name, cf->cf_driver->cd_name) != 0 ||
@@ -665,6 +676,7 @@ acpicpu_attach(struct device *parent, st
struct acpicpu_softc*sc = (struct acpicpu_softc *)self;
struct acpi_attach_args *aa = aux;
struct aml_valueres;
+   int64_t uid;
int i;
uint32_tstatus = 0;
CPU_INFO_ITERATOR   cii;
@@ -675,6 +687,10 @@ acpicpu_attach(struct device *parent, st
acpicpu_sc[sc->sc_dev.dv_unit] = sc;
 
SLIST_INIT(>sc_cstates);
+
+   if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode,
+   "_UID", 0, NULL, ) == 0)
+   sc->sc_cpu = uid;
 
if (aml_evalnode(sc->sc_acpi, sc->sc_devnode, 0, NULL, 

Re: acpicpu(4) and ACPI0007

2020-07-29 Thread Jonathan Matthew
On Wed, Jul 29, 2020 at 10:06:14AM +0200, Mark Kettenis wrote:
> > Date: Wed, 29 Jul 2020 10:38:55 +1000
> > From: Jonathan Matthew 
> > 
> > On Tue, Jul 28, 2020 at 07:30:36PM +0200, Mark Kettenis wrote:
> > > > Date: Tue, 28 Jul 2020 21:42:46 +1000
> > > > From: Jonathan Matthew 
> > > > 
> > > > On Tue, Jul 28, 2020 at 11:12:21AM +0200, Mark Kettenis wrote:
> > > > > > Date: Tue, 28 Jul 2020 13:46:34 +1000
> > > > > > From: Jonathan Matthew 
> > > > > > 
> > > > > > On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > > > > > > > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > > > > > > > From: Mark Kettenis 
> > > > > > > > 
> > > > > > > > Recent ACPI versions have deprecated "Processor()" nodes in 
> > > > > > > > favout of
> > > > > > > > "Device()" nodes with a _HID() method that returns "ACPI0007".  
> > > > > > > > This
> > > > > > > > diff tries to support machines with firmware that implements 
> > > > > > > > this.  If
> > > > > > > > you see something like:
> > > > > > > > 
> > > > > > > >   "ACPI0007" at acpi0 not configured
> > > > > > > > 
> > > > > > > > please try the following diff and report back with an updated 
> > > > > > > > dmesg.
> > > > > > > > 
> > > > > > > > Cheers,
> > > > > > > > 
> > > > > > > > Mark
> > > > > > > 
> > > > > > > And now with the right diff...
> > > > > > 
> > > > > > On a dell r6415, it looks like this:
> > > > > > 
> > > > > > acpicpu0 at acpi0copyvalue: 6: C1(@1 halt!)
> > > > > > all the way up to
> > > > > > acpicpu127 at acpi0copyvalue: 6: no cpu matching ACPI ID 127
> > > > > > 
> > > > > > which I guess means aml_copyvalue() needs to learn how to copy 
> > > > > > AML_OBJTYPE_DEVICE.
> > > > > 
> > > > > Yes.  It is not immediately obvious how this should work.  Do we need
> > > > > to copy the aml_node pointer or not?  We don't do that for
> > > > > AML_OBJTYPE_PROCESSOR and AML_OBJTYPE_POWERRSRC types which are
> > > > > similar to AML_OBJTYPE_DEVICE.  But AML_OBJTYPE_DEVICE object don't
> > > > > carry any additional information.  So we end up with just an empty
> > > > > case to avoid the warning.
> > > > > 
> > > > > Does this work on the Dell machines?
> > > > 
> > > > We've seen crashes in pool_cache_get() in various places after all the 
> > > > acpicpus
> > > > attach, which we haven't seen before on these machines, so I think it's
> > > > corrupting memory somehow.
> > > 
> > > Does that happen with only the acpicpu(4) diff?
> > 
> > Yes.  Looking at this a bit more, in the case where aml_evalnode() can't
> > copy the result value, it leaves it uninitialised, which means we'll call
> > aml_freevalue() where res is stack junk.  memset(, 0, sizeof(res))
> > seems to fix it.
> 
> Eh, where exactly?

I had it just before the call to aml_evalnode(), but that can't be it,
since aml_evalnode() does the same thing.

> 
> > > > With this addition, we get this for each cpu:
> > > > acpicpu0 at acpi0: C1(@1 halt!)
> > > 
> > > The exclamation mark indicates that this is the "fallback" C-state.
> > > Is there a _CST method at all?
> > > 
> > > Anyway, given that this is a server system, it isn't really surprising
> > > that there isn't any fancy power saving stuff.
> > 
> > Right, there doesn't seem to be any.  The processor devices look like this
> > in the aml:
> > 
> > Scope (_SB)
> > {
> > Device (C000)
> > {
> > Name (_HID, "ACPI0007" /* Processor Device */)  // _HID: 
> > Hardware ID
> > Name (_UID, 0x00)  // _UID: Unique ID
> > }
> > 
> > Device (C001)
> > {
> > Name (_HID, "ACPI0007" /* Processor Device */)  // _HID: 
> > Hardware ID
> > Name (_UID, 0x01)  // _UID: Unique ID
> > }
> > 
> >  .. and so on.
> 
> Usually there is an SSDT that fills in the details.  The acpidump
> output I have for the r6415 does have one. but it doesn't add
> anything.

Same here.

> 
> > > > > Index: dev/acpi/dsdt.c
> > > > > ===
> > > > > RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
> > > > > retrieving revision 1.252
> > > > > diff -u -p -r1.252 dsdt.c
> > > > > --- dev/acpi/dsdt.c   21 Jul 2020 03:48:06 -  1.252
> > > > > +++ dev/acpi/dsdt.c   28 Jul 2020 09:04:15 -
> > > > > @@ -996,6 +996,8 @@ aml_copyvalue(struct aml_value *lhs, str
> > > > >   lhs->v_objref = rhs->v_objref;
> > > > >   aml_addref(lhs->v_objref.ref, "");
> > > > >   break;
> > > > > + case AML_OBJTYPE_DEVICE:
> > > > > + break;
> > > > >   default:
> > > > >   printf("copyvalue: %x", rhs->type);
> > > > >   break;
> > > > 
> > > > 
> > 



Re: acpicpu(4) and ACPI0007

2020-07-29 Thread Mark Kettenis
> Date: Wed, 29 Jul 2020 10:38:55 +1000
> From: Jonathan Matthew 
> 
> On Tue, Jul 28, 2020 at 07:30:36PM +0200, Mark Kettenis wrote:
> > > Date: Tue, 28 Jul 2020 21:42:46 +1000
> > > From: Jonathan Matthew 
> > > 
> > > On Tue, Jul 28, 2020 at 11:12:21AM +0200, Mark Kettenis wrote:
> > > > > Date: Tue, 28 Jul 2020 13:46:34 +1000
> > > > > From: Jonathan Matthew 
> > > > > 
> > > > > On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > > > > > > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > > > > > > From: Mark Kettenis 
> > > > > > > 
> > > > > > > Recent ACPI versions have deprecated "Processor()" nodes in 
> > > > > > > favout of
> > > > > > > "Device()" nodes with a _HID() method that returns "ACPI0007".  
> > > > > > > This
> > > > > > > diff tries to support machines with firmware that implements 
> > > > > > > this.  If
> > > > > > > you see something like:
> > > > > > > 
> > > > > > >   "ACPI0007" at acpi0 not configured
> > > > > > > 
> > > > > > > please try the following diff and report back with an updated 
> > > > > > > dmesg.
> > > > > > > 
> > > > > > > Cheers,
> > > > > > > 
> > > > > > > Mark
> > > > > > 
> > > > > > And now with the right diff...
> > > > > 
> > > > > On a dell r6415, it looks like this:
> > > > > 
> > > > > acpicpu0 at acpi0copyvalue: 6: C1(@1 halt!)
> > > > > all the way up to
> > > > > acpicpu127 at acpi0copyvalue: 6: no cpu matching ACPI ID 127
> > > > > 
> > > > > which I guess means aml_copyvalue() needs to learn how to copy 
> > > > > AML_OBJTYPE_DEVICE.
> > > > 
> > > > Yes.  It is not immediately obvious how this should work.  Do we need
> > > > to copy the aml_node pointer or not?  We don't do that for
> > > > AML_OBJTYPE_PROCESSOR and AML_OBJTYPE_POWERRSRC types which are
> > > > similar to AML_OBJTYPE_DEVICE.  But AML_OBJTYPE_DEVICE object don't
> > > > carry any additional information.  So we end up with just an empty
> > > > case to avoid the warning.
> > > > 
> > > > Does this work on the Dell machines?
> > > 
> > > We've seen crashes in pool_cache_get() in various places after all the 
> > > acpicpus
> > > attach, which we haven't seen before on these machines, so I think it's
> > > corrupting memory somehow.
> > 
> > Does that happen with only the acpicpu(4) diff?
> 
> Yes.  Looking at this a bit more, in the case where aml_evalnode() can't
> copy the result value, it leaves it uninitialised, which means we'll call
> aml_freevalue() where res is stack junk.  memset(, 0, sizeof(res))
> seems to fix it.

Eh, where exactly?

> > > With this addition, we get this for each cpu:
> > > acpicpu0 at acpi0: C1(@1 halt!)
> > 
> > The exclamation mark indicates that this is the "fallback" C-state.
> > Is there a _CST method at all?
> > 
> > Anyway, given that this is a server system, it isn't really surprising
> > that there isn't any fancy power saving stuff.
> 
> Right, there doesn't seem to be any.  The processor devices look like this
> in the aml:
> 
> Scope (_SB)
> {
> Device (C000)
> {
> Name (_HID, "ACPI0007" /* Processor Device */)  // _HID: Hardware 
> ID
> Name (_UID, 0x00)  // _UID: Unique ID
> }
> 
> Device (C001)
> {
> Name (_HID, "ACPI0007" /* Processor Device */)  // _HID: Hardware 
> ID
> Name (_UID, 0x01)  // _UID: Unique ID
> }
> 
>  .. and so on.

Usually there is an SSDT that fills in the details.  The acpidump
output I have for the r6415 does have one. but it doesn't add
anything.

> > > > Index: dev/acpi/dsdt.c
> > > > ===
> > > > RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
> > > > retrieving revision 1.252
> > > > diff -u -p -r1.252 dsdt.c
> > > > --- dev/acpi/dsdt.c 21 Jul 2020 03:48:06 -  1.252
> > > > +++ dev/acpi/dsdt.c 28 Jul 2020 09:04:15 -
> > > > @@ -996,6 +996,8 @@ aml_copyvalue(struct aml_value *lhs, str
> > > > lhs->v_objref = rhs->v_objref;
> > > > aml_addref(lhs->v_objref.ref, "");
> > > > break;
> > > > +   case AML_OBJTYPE_DEVICE:
> > > > +   break;
> > > > default:
> > > > printf("copyvalue: %x", rhs->type);
> > > > break;
> > > 
> > > 
> 



Re: acpicpu(4) and ACPI0007

2020-07-28 Thread Jonathan Matthew
On Tue, Jul 28, 2020 at 07:30:36PM +0200, Mark Kettenis wrote:
> > Date: Tue, 28 Jul 2020 21:42:46 +1000
> > From: Jonathan Matthew 
> > 
> > On Tue, Jul 28, 2020 at 11:12:21AM +0200, Mark Kettenis wrote:
> > > > Date: Tue, 28 Jul 2020 13:46:34 +1000
> > > > From: Jonathan Matthew 
> > > > 
> > > > On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > > > > > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > > > > > From: Mark Kettenis 
> > > > > > 
> > > > > > Recent ACPI versions have deprecated "Processor()" nodes in favout 
> > > > > > of
> > > > > > "Device()" nodes with a _HID() method that returns "ACPI0007".  This
> > > > > > diff tries to support machines with firmware that implements this.  
> > > > > > If
> > > > > > you see something like:
> > > > > > 
> > > > > >   "ACPI0007" at acpi0 not configured
> > > > > > 
> > > > > > please try the following diff and report back with an updated dmesg.
> > > > > > 
> > > > > > Cheers,
> > > > > > 
> > > > > > Mark
> > > > > 
> > > > > And now with the right diff...
> > > > 
> > > > On a dell r6415, it looks like this:
> > > > 
> > > > acpicpu0 at acpi0copyvalue: 6: C1(@1 halt!)
> > > > all the way up to
> > > > acpicpu127 at acpi0copyvalue: 6: no cpu matching ACPI ID 127
> > > > 
> > > > which I guess means aml_copyvalue() needs to learn how to copy 
> > > > AML_OBJTYPE_DEVICE.
> > > 
> > > Yes.  It is not immediately obvious how this should work.  Do we need
> > > to copy the aml_node pointer or not?  We don't do that for
> > > AML_OBJTYPE_PROCESSOR and AML_OBJTYPE_POWERRSRC types which are
> > > similar to AML_OBJTYPE_DEVICE.  But AML_OBJTYPE_DEVICE object don't
> > > carry any additional information.  So we end up with just an empty
> > > case to avoid the warning.
> > > 
> > > Does this work on the Dell machines?
> > 
> > We've seen crashes in pool_cache_get() in various places after all the 
> > acpicpus
> > attach, which we haven't seen before on these machines, so I think it's
> > corrupting memory somehow.
> 
> Does that happen with only the acpicpu(4) diff?

Yes.  Looking at this a bit more, in the case where aml_evalnode() can't
copy the result value, it leaves it uninitialised, which means we'll call
aml_freevalue() where res is stack junk.  memset(, 0, sizeof(res))
seems to fix it.

> 
> > With this addition, we get this for each cpu:
> > acpicpu0 at acpi0: C1(@1 halt!)
> 
> The exclamation mark indicates that this is the "fallback" C-state.
> Is there a _CST method at all?
> 
> Anyway, given that this is a server system, it isn't really surprising
> that there isn't any fancy power saving stuff.

Right, there doesn't seem to be any.  The processor devices look like this
in the aml:

Scope (_SB)
{
Device (C000)
{
Name (_HID, "ACPI0007" /* Processor Device */)  // _HID: Hardware ID
Name (_UID, 0x00)  // _UID: Unique ID
}

Device (C001)
{
Name (_HID, "ACPI0007" /* Processor Device */)  // _HID: Hardware ID
Name (_UID, 0x01)  // _UID: Unique ID
}

 .. and so on.

> 
> > > Index: dev/acpi/dsdt.c
> > > ===
> > > RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
> > > retrieving revision 1.252
> > > diff -u -p -r1.252 dsdt.c
> > > --- dev/acpi/dsdt.c   21 Jul 2020 03:48:06 -  1.252
> > > +++ dev/acpi/dsdt.c   28 Jul 2020 09:04:15 -
> > > @@ -996,6 +996,8 @@ aml_copyvalue(struct aml_value *lhs, str
> > >   lhs->v_objref = rhs->v_objref;
> > >   aml_addref(lhs->v_objref.ref, "");
> > >   break;
> > > + case AML_OBJTYPE_DEVICE:
> > > + break;
> > >   default:
> > >   printf("copyvalue: %x", rhs->type);
> > >   break;
> > 
> > 



Re: acpicpu(4) and ACPI0007

2020-07-28 Thread Bryan Steele
On Tue, Jul 28, 2020 at 01:44:33PM -0400, Johan Huldtgren wrote:
> hello,
> 
> On 2020-07-28 11:12, Mark Kettenis wrote:
> > > Date: Tue, 28 Jul 2020 13:46:34 +1000
> > > From: Jonathan Matthew 
> > > 
> > > On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > > > > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > > > > From: Mark Kettenis 
> > > > > 
> > > > > Recent ACPI versions have deprecated "Processor()" nodes in favout of
> > > > > "Device()" nodes with a _HID() method that returns "ACPI0007".  This
> > > > > diff tries to support machines with firmware that implements this.  If
> > > > > you see something like:
> > > > > 
> > > > >   "ACPI0007" at acpi0 not configured
> > > > > 
> > > > > please try the following diff and report back with an updated dmesg.
> > > > > 
> > > > > Cheers,
> > > > > 
> > > > > Mark
> > > > 
> > > > And now with the right diff...
> > > 
> > > On a dell r6415, it looks like this:
> > > 
> > > acpicpu0 at acpi0copyvalue: 6: C1(@1 halt!)
> > > all the way up to
> > > acpicpu127 at acpi0copyvalue: 6: no cpu matching ACPI ID 127
> > > 
> > > which I guess means aml_copyvalue() needs to learn how to copy 
> > > AML_OBJTYPE_DEVICE.
> > 
> > Yes.  It is not immediately obvious how this should work.  Do we need
> > to copy the aml_node pointer or not?  We don't do that for
> > AML_OBJTYPE_PROCESSOR and AML_OBJTYPE_POWERRSRC types which are
> > similar to AML_OBJTYPE_DEVICE.  But AML_OBJTYPE_DEVICE object don't
> > carry any additional information.  So we end up with just an empty
> > case to avoid the warning.
> > 
> > Does this work on the Dell machines?
> > 
> > 
> > Index: dev/acpi/dsdt.c
> > ===
> > RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
> > retrieving revision 1.252
> > diff -u -p -r1.252 dsdt.c
> > --- dev/acpi/dsdt.c 21 Jul 2020 03:48:06 -  1.252
> > +++ dev/acpi/dsdt.c 28 Jul 2020 09:04:15 -
> > @@ -996,6 +996,8 @@ aml_copyvalue(struct aml_value *lhs, str
> > lhs->v_objref = rhs->v_objref;
> > aml_addref(lhs->v_objref.ref, "");
> > break;
> > +   case AML_OBJTYPE_DEVICE:
> > +   break;
> > default:
> > printf("copyvalue: %x", rhs->type);
> > break;
> > 


Mark, This one below looks like it's attaching on both Processor() nodes
and "ACPI0007", I'm assuming we don't want to do that?

> before diffs I see:
> 
> "ACPI0004" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0004" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> 
> after applying both diffs I see that as
> 
> "ACPI0004" at acpi0 not configured
> acpicpu24 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu25 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu26 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu27 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu28 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu29 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu30 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu31 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu32 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu33 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu34 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu35 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> "ACPI0004" at acpi0 not configured
> acpicpu36 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu37 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu38 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu39 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu40 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu41 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu42 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu43 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu44 at acpi0: C2(350@41 mwait.3@0x20), 

Re: acpicpu(4) and ACPI0007

2020-07-28 Thread Johan Huldtgren
hello,

On 2020-07-28 11:12, Mark Kettenis wrote:
> > Date: Tue, 28 Jul 2020 13:46:34 +1000
> > From: Jonathan Matthew 
> > 
> > On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > > > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > > > From: Mark Kettenis 
> > > > 
> > > > Recent ACPI versions have deprecated "Processor()" nodes in favout of
> > > > "Device()" nodes with a _HID() method that returns "ACPI0007".  This
> > > > diff tries to support machines with firmware that implements this.  If
> > > > you see something like:
> > > > 
> > > >   "ACPI0007" at acpi0 not configured
> > > > 
> > > > please try the following diff and report back with an updated dmesg.
> > > > 
> > > > Cheers,
> > > > 
> > > > Mark
> > > 
> > > And now with the right diff...
> > 
> > On a dell r6415, it looks like this:
> > 
> > acpicpu0 at acpi0copyvalue: 6: C1(@1 halt!)
> > all the way up to
> > acpicpu127 at acpi0copyvalue: 6: no cpu matching ACPI ID 127
> > 
> > which I guess means aml_copyvalue() needs to learn how to copy 
> > AML_OBJTYPE_DEVICE.
> 
> Yes.  It is not immediately obvious how this should work.  Do we need
> to copy the aml_node pointer or not?  We don't do that for
> AML_OBJTYPE_PROCESSOR and AML_OBJTYPE_POWERRSRC types which are
> similar to AML_OBJTYPE_DEVICE.  But AML_OBJTYPE_DEVICE object don't
> carry any additional information.  So we end up with just an empty
> case to avoid the warning.
> 
> Does this work on the Dell machines?
> 
> 
> Index: dev/acpi/dsdt.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
> retrieving revision 1.252
> diff -u -p -r1.252 dsdt.c
> --- dev/acpi/dsdt.c   21 Jul 2020 03:48:06 -  1.252
> +++ dev/acpi/dsdt.c   28 Jul 2020 09:04:15 -
> @@ -996,6 +996,8 @@ aml_copyvalue(struct aml_value *lhs, str
>   lhs->v_objref = rhs->v_objref;
>   aml_addref(lhs->v_objref.ref, "");
>   break;
> + case AML_OBJTYPE_DEVICE:
> + break;
>   default:
>   printf("copyvalue: %x", rhs->type);
>   break;
> 

before diffs I see:

"ACPI0004" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0004" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured

after applying both diffs I see that as

"ACPI0004" at acpi0 not configured
acpicpu24 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu25 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu26 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu27 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu28 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu29 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu30 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu31 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu32 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu33 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu34 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu35 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
"ACPI0004" at acpi0 not configured
acpicpu36 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu37 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu38 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu39 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu40 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu41 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu42 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu43 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu44 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu45 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu46 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu47 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS

Full dmesg attached.

thanks,

.jh
OpenBSD 6.7-current (GENERIC.MP) #1: Tue Jul 28 11:35:11 EDT 2020


Re: acpicpu(4) and ACPI0007

2020-07-28 Thread Mark Kettenis
> Date: Tue, 28 Jul 2020 21:42:46 +1000
> From: Jonathan Matthew 
> 
> On Tue, Jul 28, 2020 at 11:12:21AM +0200, Mark Kettenis wrote:
> > > Date: Tue, 28 Jul 2020 13:46:34 +1000
> > > From: Jonathan Matthew 
> > > 
> > > On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > > > > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > > > > From: Mark Kettenis 
> > > > > 
> > > > > Recent ACPI versions have deprecated "Processor()" nodes in favout of
> > > > > "Device()" nodes with a _HID() method that returns "ACPI0007".  This
> > > > > diff tries to support machines with firmware that implements this.  If
> > > > > you see something like:
> > > > > 
> > > > >   "ACPI0007" at acpi0 not configured
> > > > > 
> > > > > please try the following diff and report back with an updated dmesg.
> > > > > 
> > > > > Cheers,
> > > > > 
> > > > > Mark
> > > > 
> > > > And now with the right diff...
> > > 
> > > On a dell r6415, it looks like this:
> > > 
> > > acpicpu0 at acpi0copyvalue: 6: C1(@1 halt!)
> > > all the way up to
> > > acpicpu127 at acpi0copyvalue: 6: no cpu matching ACPI ID 127
> > > 
> > > which I guess means aml_copyvalue() needs to learn how to copy 
> > > AML_OBJTYPE_DEVICE.
> > 
> > Yes.  It is not immediately obvious how this should work.  Do we need
> > to copy the aml_node pointer or not?  We don't do that for
> > AML_OBJTYPE_PROCESSOR and AML_OBJTYPE_POWERRSRC types which are
> > similar to AML_OBJTYPE_DEVICE.  But AML_OBJTYPE_DEVICE object don't
> > carry any additional information.  So we end up with just an empty
> > case to avoid the warning.
> > 
> > Does this work on the Dell machines?
> 
> We've seen crashes in pool_cache_get() in various places after all the 
> acpicpus
> attach, which we haven't seen before on these machines, so I think it's
> corrupting memory somehow.

Does that happen with only the acpicpu(4) diff?

> With this addition, we get this for each cpu:
> acpicpu0 at acpi0: C1(@1 halt!)

The exclamation mark indicates that this is the "fallback" C-state.
Is there a _CST method at all?

Anyway, given that this is a server system, it isn't really surprising
that there isn't any fancy power saving stuff.

> > Index: dev/acpi/dsdt.c
> > ===
> > RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
> > retrieving revision 1.252
> > diff -u -p -r1.252 dsdt.c
> > --- dev/acpi/dsdt.c 21 Jul 2020 03:48:06 -  1.252
> > +++ dev/acpi/dsdt.c 28 Jul 2020 09:04:15 -
> > @@ -996,6 +996,8 @@ aml_copyvalue(struct aml_value *lhs, str
> > lhs->v_objref = rhs->v_objref;
> > aml_addref(lhs->v_objref.ref, "");
> > break;
> > +   case AML_OBJTYPE_DEVICE:
> > +   break;
> > default:
> > printf("copyvalue: %x", rhs->type);
> > break;
> 
> 



Re: acpicpu(4) and ACPI0007

2020-07-28 Thread Bryan Steele
On Tue, Jul 28, 2020 at 01:09:51PM +0200, Mark Kettenis wrote:
> > Date: Tue, 28 Jul 2020 11:16:56 +0100
> > From: Jason McIntyre 
> > 
> > On Tue, Jul 28, 2020 at 11:12:21AM +0200, Mark Kettenis wrote:
> > > > Date: Tue, 28 Jul 2020 13:46:34 +1000
> > > > From: Jonathan Matthew 
> > > > 
> > > > On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > > > > > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > > > > > From: Mark Kettenis 
> > > > > > 
> > > > > > Recent ACPI versions have deprecated "Processor()" nodes in favout 
> > > > > > of
> > > > > > "Device()" nodes with a _HID() method that returns "ACPI0007".  This
> > > > > > diff tries to support machines with firmware that implements this.  
> > > > > > If
> > > > > > you see something like:
> > > > > > 
> > > > > >   "ACPI0007" at acpi0 not configured
> > > > > > 
> > > > > > please try the following diff and report back with an updated dmesg.
> > > > > > 
> > > > > > Cheers,
> > > > > > 
> > > > > > Mark
> > > > > 
> > > > > And now with the right diff...
> > > > 
> > > > On a dell r6415, it looks like this:
> > > > 
> > > > acpicpu0 at acpi0copyvalue: 6: C1(@1 halt!)
> > > > all the way up to
> > > > acpicpu127 at acpi0copyvalue: 6: no cpu matching ACPI ID 127
> > > > 
> > > > which I guess means aml_copyvalue() needs to learn how to copy 
> > > > AML_OBJTYPE_DEVICE.
> > > 
> > > Yes.  It is not immediately obvious how this should work.  Do we need
> > > to copy the aml_node pointer or not?  We don't do that for
> > > AML_OBJTYPE_PROCESSOR and AML_OBJTYPE_POWERRSRC types which are
> > > similar to AML_OBJTYPE_DEVICE.  But AML_OBJTYPE_DEVICE object don't
> > > carry any additional information.  So we end up with just an empty
> > > case to avoid the warning.
> > > 
> > > Does this work on the Dell machines?
> > > 
> > > 
> > > Index: dev/acpi/dsdt.c
> > > ===
> > > RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
> > > retrieving revision 1.252
> > > diff -u -p -r1.252 dsdt.c
> > > --- dev/acpi/dsdt.c   21 Jul 2020 03:48:06 -  1.252
> > > +++ dev/acpi/dsdt.c   28 Jul 2020 09:04:15 -
> > > @@ -996,6 +996,8 @@ aml_copyvalue(struct aml_value *lhs, str
> > >   lhs->v_objref = rhs->v_objref;
> > >   aml_addref(lhs->v_objref.ref, "");
> > >   break;
> > > + case AML_OBJTYPE_DEVICE:
> > > + break;
> > >   default:
> > >   printf("copyvalue: %x", rhs->type);
> > >   break;
> > > 
> > 
> > morning. it displays this here:
> > 
> > acpicpu0 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
> > mwait), PSS
> > acpicpu1 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
> > mwait), PSS
> > acpicpu2 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
> > mwait), PSS
> > acpicpu3 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
> > mwait), PSS
> > acpicpu4 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
> > mwait), PSS
> > acpicpu5 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
> > mwait), PSS
> > acpicpu6 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
> > mwait), PSS
> > acpicpu7 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
> > mwait), PSS
> > acpicpu8 at acpi0: no cpu matching ACPI ID 8
> > acpicpu9 at acpi0: no cpu matching ACPI ID 9
> > acpicpu10 at acpi0: no cpu matching ACPI ID 10
> > acpicpu11 at acpi0: no cpu matching ACPI ID 11
> > acpicpu12 at acpi0: no cpu matching ACPI ID 12
> > acpicpu13 at acpi0: no cpu matching ACPI ID 13
> > acpicpu14 at acpi0: no cpu matching ACPI ID 14
> > acpicpu15 at acpi0: no cpu matching ACPI ID 15
> 
> Excellent!
> 
> We may want to do something about those "no cpu matching ACPU ID XX"
> messages at some point.  But that's a diff for another day.
> 
> So ok's for both diffs are welcome.

reads fine and clearly improves things for limited function ACPI
systems.

ok brynet@

> Cheers,
> 
> Mark
> 
> P.S. I've also established that that the EC-related messages are
> indeed harmless and a result of sloppy BIOS writers.
> 
> > OpenBSD 6.7-current (GENERIC.MP) #3: Tue Jul 28 10:59:50 BST 2020
> > jmc@kansas:/usr/src/sys/arch/amd64/compile/GENERIC.MP
> > real mem = 7895654400 (7529MB)
> > avail mem = 7641284608 (7287MB)
> > random: good seed from bootblocks
> > mpath0 at root
> > scsibus0 at mpath0: 256 targets
> > mainbus0 at root
> > bios0 at mainbus0: SMBIOS rev. 3.2 @ 0xca707000 (77 entries)
> > bios0: vendor Dell Inc. version "1.1.0" date 05/27/2020
> > bios0: Dell Inc. Inspiron 5505
> > acpi0 at bios0: ACPI 5.0Undefined scope: \\_SB_.PCI0.LPC0.EC0_
> > 
> > acpi0: sleep states S0 S4 S5
> > acpi0: tables DSDT FACP UEFI SSDT IVRS SSDT TPM2 MSDM ASF! BOOT HPET APIC 
> > MCFG SLIC SSDT WSMT VFCT SSDT SSDT CRAT CDIT SSDT SSDT SSDT SSDT FPDT SSDT 
> > SSDT SSDT SSDT SSDT SSDT SSDT BGRT
> > acpi0: wakeup devices GP17(S4)
> > acpitimer0 at acpi0: 3579545 

Re: acpicpu(4) and ACPI0007

2020-07-28 Thread Jonathan Matthew
On Tue, Jul 28, 2020 at 11:12:21AM +0200, Mark Kettenis wrote:
> > Date: Tue, 28 Jul 2020 13:46:34 +1000
> > From: Jonathan Matthew 
> > 
> > On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > > > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > > > From: Mark Kettenis 
> > > > 
> > > > Recent ACPI versions have deprecated "Processor()" nodes in favout of
> > > > "Device()" nodes with a _HID() method that returns "ACPI0007".  This
> > > > diff tries to support machines with firmware that implements this.  If
> > > > you see something like:
> > > > 
> > > >   "ACPI0007" at acpi0 not configured
> > > > 
> > > > please try the following diff and report back with an updated dmesg.
> > > > 
> > > > Cheers,
> > > > 
> > > > Mark
> > > 
> > > And now with the right diff...
> > 
> > On a dell r6415, it looks like this:
> > 
> > acpicpu0 at acpi0copyvalue: 6: C1(@1 halt!)
> > all the way up to
> > acpicpu127 at acpi0copyvalue: 6: no cpu matching ACPI ID 127
> > 
> > which I guess means aml_copyvalue() needs to learn how to copy 
> > AML_OBJTYPE_DEVICE.
> 
> Yes.  It is not immediately obvious how this should work.  Do we need
> to copy the aml_node pointer or not?  We don't do that for
> AML_OBJTYPE_PROCESSOR and AML_OBJTYPE_POWERRSRC types which are
> similar to AML_OBJTYPE_DEVICE.  But AML_OBJTYPE_DEVICE object don't
> carry any additional information.  So we end up with just an empty
> case to avoid the warning.
> 
> Does this work on the Dell machines?

We've seen crashes in pool_cache_get() in various places after all the acpicpus
attach, which we haven't seen before on these machines, so I think it's
corrupting memory somehow.

With this addition, we get this for each cpu:
acpicpu0 at acpi0: C1(@1 halt!)

> 
> 
> Index: dev/acpi/dsdt.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
> retrieving revision 1.252
> diff -u -p -r1.252 dsdt.c
> --- dev/acpi/dsdt.c   21 Jul 2020 03:48:06 -  1.252
> +++ dev/acpi/dsdt.c   28 Jul 2020 09:04:15 -
> @@ -996,6 +996,8 @@ aml_copyvalue(struct aml_value *lhs, str
>   lhs->v_objref = rhs->v_objref;
>   aml_addref(lhs->v_objref.ref, "");
>   break;
> + case AML_OBJTYPE_DEVICE:
> + break;
>   default:
>   printf("copyvalue: %x", rhs->type);
>   break;



Re: acpicpu(4) and ACPI0007

2020-07-28 Thread Mark Kettenis
> Date: Tue, 28 Jul 2020 11:16:56 +0100
> From: Jason McIntyre 
> 
> On Tue, Jul 28, 2020 at 11:12:21AM +0200, Mark Kettenis wrote:
> > > Date: Tue, 28 Jul 2020 13:46:34 +1000
> > > From: Jonathan Matthew 
> > > 
> > > On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > > > > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > > > > From: Mark Kettenis 
> > > > > 
> > > > > Recent ACPI versions have deprecated "Processor()" nodes in favout of
> > > > > "Device()" nodes with a _HID() method that returns "ACPI0007".  This
> > > > > diff tries to support machines with firmware that implements this.  If
> > > > > you see something like:
> > > > > 
> > > > >   "ACPI0007" at acpi0 not configured
> > > > > 
> > > > > please try the following diff and report back with an updated dmesg.
> > > > > 
> > > > > Cheers,
> > > > > 
> > > > > Mark
> > > > 
> > > > And now with the right diff...
> > > 
> > > On a dell r6415, it looks like this:
> > > 
> > > acpicpu0 at acpi0copyvalue: 6: C1(@1 halt!)
> > > all the way up to
> > > acpicpu127 at acpi0copyvalue: 6: no cpu matching ACPI ID 127
> > > 
> > > which I guess means aml_copyvalue() needs to learn how to copy 
> > > AML_OBJTYPE_DEVICE.
> > 
> > Yes.  It is not immediately obvious how this should work.  Do we need
> > to copy the aml_node pointer or not?  We don't do that for
> > AML_OBJTYPE_PROCESSOR and AML_OBJTYPE_POWERRSRC types which are
> > similar to AML_OBJTYPE_DEVICE.  But AML_OBJTYPE_DEVICE object don't
> > carry any additional information.  So we end up with just an empty
> > case to avoid the warning.
> > 
> > Does this work on the Dell machines?
> > 
> > 
> > Index: dev/acpi/dsdt.c
> > ===
> > RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
> > retrieving revision 1.252
> > diff -u -p -r1.252 dsdt.c
> > --- dev/acpi/dsdt.c 21 Jul 2020 03:48:06 -  1.252
> > +++ dev/acpi/dsdt.c 28 Jul 2020 09:04:15 -
> > @@ -996,6 +996,8 @@ aml_copyvalue(struct aml_value *lhs, str
> > lhs->v_objref = rhs->v_objref;
> > aml_addref(lhs->v_objref.ref, "");
> > break;
> > +   case AML_OBJTYPE_DEVICE:
> > +   break;
> > default:
> > printf("copyvalue: %x", rhs->type);
> > break;
> > 
> 
> morning. it displays this here:
> 
>   acpicpu0 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
> mwait), PSS
>   acpicpu1 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
> mwait), PSS
>   acpicpu2 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
> mwait), PSS
>   acpicpu3 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
> mwait), PSS
>   acpicpu4 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
> mwait), PSS
>   acpicpu5 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
> mwait), PSS
>   acpicpu6 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
> mwait), PSS
>   acpicpu7 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
> mwait), PSS
>   acpicpu8 at acpi0: no cpu matching ACPI ID 8
>   acpicpu9 at acpi0: no cpu matching ACPI ID 9
>   acpicpu10 at acpi0: no cpu matching ACPI ID 10
>   acpicpu11 at acpi0: no cpu matching ACPI ID 11
>   acpicpu12 at acpi0: no cpu matching ACPI ID 12
>   acpicpu13 at acpi0: no cpu matching ACPI ID 13
>   acpicpu14 at acpi0: no cpu matching ACPI ID 14
>   acpicpu15 at acpi0: no cpu matching ACPI ID 15

Excellent!

We may want to do something about those "no cpu matching ACPU ID XX"
messages at some point.  But that's a diff for another day.

So ok's for both diffs are welcome.

Cheers,

Mark

P.S. I've also established that that the EC-related messages are
indeed harmless and a result of sloppy BIOS writers.

> OpenBSD 6.7-current (GENERIC.MP) #3: Tue Jul 28 10:59:50 BST 2020
> jmc@kansas:/usr/src/sys/arch/amd64/compile/GENERIC.MP
> real mem = 7895654400 (7529MB)
> avail mem = 7641284608 (7287MB)
> random: good seed from bootblocks
> mpath0 at root
> scsibus0 at mpath0: 256 targets
> mainbus0 at root
> bios0 at mainbus0: SMBIOS rev. 3.2 @ 0xca707000 (77 entries)
> bios0: vendor Dell Inc. version "1.1.0" date 05/27/2020
> bios0: Dell Inc. Inspiron 5505
> acpi0 at bios0: ACPI 5.0Undefined scope: \\_SB_.PCI0.LPC0.EC0_
> 
> acpi0: sleep states S0 S4 S5
> acpi0: tables DSDT FACP UEFI SSDT IVRS SSDT TPM2 MSDM ASF! BOOT HPET APIC 
> MCFG SLIC SSDT WSMT VFCT SSDT SSDT CRAT CDIT SSDT SSDT SSDT SSDT FPDT SSDT 
> SSDT SSDT SSDT SSDT SSDT SSDT BGRT
> acpi0: wakeup devices GP17(S4)
> acpitimer0 at acpi0: 3579545 Hz, 32 bits
> acpihpet0 at acpi0: 14318180 Hz
> acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
> cpu0 at mainbus0: apid 0 (boot processor)
> cpu0: AMD Ryzen 7 4700U with Radeon Graphics, 1996.48 MHz, 17-60-01
> cpu0: 
> 

Re: acpicpu(4) and ACPI0007

2020-07-28 Thread Jason McIntyre
On Tue, Jul 28, 2020 at 11:12:21AM +0200, Mark Kettenis wrote:
> > Date: Tue, 28 Jul 2020 13:46:34 +1000
> > From: Jonathan Matthew 
> > 
> > On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > > > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > > > From: Mark Kettenis 
> > > > 
> > > > Recent ACPI versions have deprecated "Processor()" nodes in favout of
> > > > "Device()" nodes with a _HID() method that returns "ACPI0007".  This
> > > > diff tries to support machines with firmware that implements this.  If
> > > > you see something like:
> > > > 
> > > >   "ACPI0007" at acpi0 not configured
> > > > 
> > > > please try the following diff and report back with an updated dmesg.
> > > > 
> > > > Cheers,
> > > > 
> > > > Mark
> > > 
> > > And now with the right diff...
> > 
> > On a dell r6415, it looks like this:
> > 
> > acpicpu0 at acpi0copyvalue: 6: C1(@1 halt!)
> > all the way up to
> > acpicpu127 at acpi0copyvalue: 6: no cpu matching ACPI ID 127
> > 
> > which I guess means aml_copyvalue() needs to learn how to copy 
> > AML_OBJTYPE_DEVICE.
> 
> Yes.  It is not immediately obvious how this should work.  Do we need
> to copy the aml_node pointer or not?  We don't do that for
> AML_OBJTYPE_PROCESSOR and AML_OBJTYPE_POWERRSRC types which are
> similar to AML_OBJTYPE_DEVICE.  But AML_OBJTYPE_DEVICE object don't
> carry any additional information.  So we end up with just an empty
> case to avoid the warning.
> 
> Does this work on the Dell machines?
> 
> 
> Index: dev/acpi/dsdt.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
> retrieving revision 1.252
> diff -u -p -r1.252 dsdt.c
> --- dev/acpi/dsdt.c   21 Jul 2020 03:48:06 -  1.252
> +++ dev/acpi/dsdt.c   28 Jul 2020 09:04:15 -
> @@ -996,6 +996,8 @@ aml_copyvalue(struct aml_value *lhs, str
>   lhs->v_objref = rhs->v_objref;
>   aml_addref(lhs->v_objref.ref, "");
>   break;
> + case AML_OBJTYPE_DEVICE:
> + break;
>   default:
>   printf("copyvalue: %x", rhs->type);
>   break;
> 

morning. it displays this here:

acpicpu0 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
mwait), PSS
acpicpu1 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
mwait), PSS
acpicpu2 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
mwait), PSS
acpicpu3 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
mwait), PSS
acpicpu4 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
mwait), PSS
acpicpu5 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
mwait), PSS
acpicpu6 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
mwait), PSS
acpicpu7 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
mwait), PSS
acpicpu8 at acpi0: no cpu matching ACPI ID 8
acpicpu9 at acpi0: no cpu matching ACPI ID 9
acpicpu10 at acpi0: no cpu matching ACPI ID 10
acpicpu11 at acpi0: no cpu matching ACPI ID 11
acpicpu12 at acpi0: no cpu matching ACPI ID 12
acpicpu13 at acpi0: no cpu matching ACPI ID 13
acpicpu14 at acpi0: no cpu matching ACPI ID 14
acpicpu15 at acpi0: no cpu matching ACPI ID 15

jmc

OpenBSD 6.7-current (GENERIC.MP) #3: Tue Jul 28 10:59:50 BST 2020
jmc@kansas:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 7895654400 (7529MB)
avail mem = 7641284608 (7287MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 3.2 @ 0xca707000 (77 entries)
bios0: vendor Dell Inc. version "1.1.0" date 05/27/2020
bios0: Dell Inc. Inspiron 5505
acpi0 at bios0: ACPI 5.0Undefined scope: \\_SB_.PCI0.LPC0.EC0_

acpi0: sleep states S0 S4 S5
acpi0: tables DSDT FACP UEFI SSDT IVRS SSDT TPM2 MSDM ASF! BOOT HPET APIC MCFG 
SLIC SSDT WSMT VFCT SSDT SSDT CRAT CDIT SSDT SSDT SSDT SSDT FPDT SSDT SSDT SSDT 
SSDT SSDT SSDT SSDT BGRT
acpi0: wakeup devices GP17(S4)
acpitimer0 at acpi0: 3579545 Hz, 32 bits
acpihpet0 at acpi0: 14318180 Hz
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: AMD Ryzen 7 4700U with Radeon Graphics, 1996.48 MHz, 17-60-01
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,SKINIT,TCE,TOPEXT,CPCTR,DBKP,PCTRL3,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,PQM,RDSEED,ADX,SMAP,CLFLUSHOPT,CLWB,SHA,UMIP,IBPB,IBRS,STIBP,SSBD,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
cpu0: 32KB 64b/line 8-way I-cache, 32KB 64b/line 8-way D-cache, 512KB 64b/line 
8-way L2 cache, 8MB 64b/line disabled L3 cache
cpu0: ITLB 64 4KB entries fully associative, 64 4MB entries fully associative
cpu0: DTLB 64 4KB entries fully associative, 

Re: acpicpu(4) and ACPI0007

2020-07-28 Thread Mark Kettenis
> Date: Tue, 28 Jul 2020 13:46:34 +1000
> From: Jonathan Matthew 
> 
> On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > > From: Mark Kettenis 
> > > 
> > > Recent ACPI versions have deprecated "Processor()" nodes in favout of
> > > "Device()" nodes with a _HID() method that returns "ACPI0007".  This
> > > diff tries to support machines with firmware that implements this.  If
> > > you see something like:
> > > 
> > >   "ACPI0007" at acpi0 not configured
> > > 
> > > please try the following diff and report back with an updated dmesg.
> > > 
> > > Cheers,
> > > 
> > > Mark
> > 
> > And now with the right diff...
> 
> On a dell r6415, it looks like this:
> 
> acpicpu0 at acpi0copyvalue: 6: C1(@1 halt!)
> all the way up to
> acpicpu127 at acpi0copyvalue: 6: no cpu matching ACPI ID 127
> 
> which I guess means aml_copyvalue() needs to learn how to copy 
> AML_OBJTYPE_DEVICE.

Yes.  It is not immediately obvious how this should work.  Do we need
to copy the aml_node pointer or not?  We don't do that for
AML_OBJTYPE_PROCESSOR and AML_OBJTYPE_POWERRSRC types which are
similar to AML_OBJTYPE_DEVICE.  But AML_OBJTYPE_DEVICE object don't
carry any additional information.  So we end up with just an empty
case to avoid the warning.

Does this work on the Dell machines?


Index: dev/acpi/dsdt.c
===
RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
retrieving revision 1.252
diff -u -p -r1.252 dsdt.c
--- dev/acpi/dsdt.c 21 Jul 2020 03:48:06 -  1.252
+++ dev/acpi/dsdt.c 28 Jul 2020 09:04:15 -
@@ -996,6 +996,8 @@ aml_copyvalue(struct aml_value *lhs, str
lhs->v_objref = rhs->v_objref;
aml_addref(lhs->v_objref.ref, "");
break;
+   case AML_OBJTYPE_DEVICE:
+   break;
default:
printf("copyvalue: %x", rhs->type);
break;



Re: acpicpu(4) and ACPI0007

2020-07-27 Thread Jonathan Matthew
On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > From: Mark Kettenis 
> > 
> > Recent ACPI versions have deprecated "Processor()" nodes in favout of
> > "Device()" nodes with a _HID() method that returns "ACPI0007".  This
> > diff tries to support machines with firmware that implements this.  If
> > you see something like:
> > 
> >   "ACPI0007" at acpi0 not configured
> > 
> > please try the following diff and report back with an updated dmesg.
> > 
> > Cheers,
> > 
> > Mark
> 
> And now with the right diff...

On a dell r6415, it looks like this:

acpicpu0 at acpi0copyvalue: 6: C1(@1 halt!)
all the way up to
acpicpu127 at acpi0copyvalue: 6: no cpu matching ACPI ID 127

which I guess means aml_copyvalue() needs to learn how to copy 
AML_OBJTYPE_DEVICE.

> 
> 
> Index: dev/acpi/acpicpu.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpicpu.c,v
> retrieving revision 1.85
> diff -u -p -r1.85 acpicpu.c
> --- dev/acpi/acpicpu.c27 May 2020 05:02:21 -  1.85
> +++ dev/acpi/acpicpu.c27 Jul 2020 14:58:38 -
> @@ -186,6 +186,11 @@ struct cfdriver acpicpu_cd = {
>   NULL, "acpicpu", DV_DULL
>  };
>  
> +const char *acpicpu_hids[] = {
> + "ACPI0007",
> + NULL
> +};
> +
>  extern int setperf_prio;
>  
>  struct acpicpu_softc *acpicpu_sc[MAXCPUS];
> @@ -650,6 +655,9 @@ acpicpu_match(struct device *parent, voi
>   struct acpi_attach_args *aa = aux;
>   struct cfdata   *cf = match;
>  
> + if (acpi_matchhids(aa, acpicpu_hids, cf->cf_driver->cd_name))
> + return (1);
> +
>   /* sanity */
>   if (aa->aaa_name == NULL ||
>   strcmp(aa->aaa_name, cf->cf_driver->cd_name) != 0 ||
> @@ -665,6 +673,7 @@ acpicpu_attach(struct device *parent, st
>   struct acpicpu_softc*sc = (struct acpicpu_softc *)self;
>   struct acpi_attach_args *aa = aux;
>   struct aml_valueres;
> + int64_t uid;
>   int i;
>   uint32_tstatus = 0;
>   CPU_INFO_ITERATOR   cii;
> @@ -675,6 +684,10 @@ acpicpu_attach(struct device *parent, st
>   acpicpu_sc[sc->sc_dev.dv_unit] = sc;
>  
>   SLIST_INIT(>sc_cstates);
> +
> + if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode,
> + "_UID", 0, NULL, ) == 0)
> + sc->sc_cpu = uid;
>  
>   if (aml_evalnode(sc->sc_acpi, sc->sc_devnode, 0, NULL, ) == 0) {
>   if (res.type == AML_OBJTYPE_PROCESSOR) {
> 



Re: acpicpu(4) and ACPI0007

2020-07-27 Thread Jason McIntyre
On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > From: Mark Kettenis 
> > 
> > Recent ACPI versions have deprecated "Processor()" nodes in favout of
> > "Device()" nodes with a _HID() method that returns "ACPI0007".  This
> > diff tries to support machines with firmware that implements this.  If
> > you see something like:
> > 
> >   "ACPI0007" at acpi0 not configured
> > 
> > please try the following diff and report back with an updated dmesg.
> > 
> > Cheers,
> > 
> > Mark
> 
> And now with the right diff...
> 
> 

hi. i'm not sure if the output is correct, but it does look like the cpu
is adjustable with this:

cpu0: 1996 MHz: speeds: 2000 1700 1400 MHz

this part looks weird:

acpicpu0 at acpi0copyvalue: 6: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
mwait), PSS
acpicpu1 at acpi0copyvalue: 6: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
mwait), PSS
acpicpu2 at acpi0copyvalue: 6: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
mwait), PSS
acpicpu3 at acpi0copyvalue: 6: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
mwait), PSS
acpicpu4 at acpi0copyvalue: 6: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
mwait), PSS
acpicpu5 at acpi0copyvalue: 6: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
mwait), PSS
acpicpu6 at acpi0copyvalue: 6: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
mwait), PSS
acpicpu7 at acpi0copyvalue: 6: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1 
mwait), PSS
acpicpu8 at acpi0copyvalue: 6: no cpu matching ACPI ID 8
acpicpu9 at acpi0copyvalue: 6: no cpu matching ACPI ID 9
acpicpu10 at acpi0copyvalue: 6: no cpu matching ACPI ID 10
acpicpu11 at acpi0copyvalue: 6: no cpu matching ACPI ID 11
acpicpu12 at acpi0copyvalue: 6: no cpu matching ACPI ID 12
acpicpu13 at acpi0copyvalue: 6: no cpu matching ACPI ID 13
acpicpu14 at acpi0copyvalue: 6: no cpu matching ACPI ID 14
acpicpu15 at acpi0copyvalue: 6: no cpu matching ACPI ID 15

full dmesg below.
jmc

OpenBSD 6.7-current (GENERIC.MP) #2: Mon Jul 27 16:18:30 BST 2020
jmc@kansas:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 7895654400 (7529MB)
avail mem = 7641284608 (7287MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 3.2 @ 0xca707000 (77 entries)
bios0: vendor Dell Inc. version "1.1.0" date 05/27/2020
bios0: Dell Inc. Inspiron 5505
acpi0 at bios0: ACPI 5.0Undefined scope: \\_SB_.PCI0.LPC0.EC0_

acpi0: sleep states S0 S4 S5
acpi0: tables DSDT FACP UEFI SSDT IVRS SSDT TPM2 MSDM ASF! BOOT HPET APIC MCFG 
SLIC SSDT WSMT VFCT SSDT SSDT CRAT CDIT SSDT SSDT SSDT SSDT FPDT SSDT SSDT SSDT 
SSDT SSDT SSDT SSDT BGRT
acpi0: wakeup devices GP17(S4)
acpitimer0 at acpi0: 3579545 Hz, 32 bits
acpihpet0 at acpi0: 14318180 Hz
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: AMD Ryzen 7 4700U with Radeon Graphics, 1996.53 MHz, 17-60-01
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,SKINIT,TCE,TOPEXT,CPCTR,DBKP,PCTRL3,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,PQM,RDSEED,ADX,SMAP,CLFLUSHOPT,CLWB,SHA,UMIP,IBPB,IBRS,STIBP,SSBD,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
cpu0: 32KB 64b/line 8-way I-cache, 32KB 64b/line 8-way D-cache, 512KB 64b/line 
8-way L2 cache, 8MB 64b/line disabled L3 cache
cpu0: ITLB 64 4KB entries fully associative, 64 4MB entries fully associative
cpu0: DTLB 64 4KB entries fully associative, 64 4MB entries fully associative
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
cpu0: apic clock running at 99MHz
cpu0: mwait min=64, max=64, C-substates=1.1, IBE
cpu1 at mainbus0: apid 1 (application processor)
cpu1: AMD Ryzen 7 4700U with Radeon Graphics, 1996.25 MHz, 17-60-01
cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,SKINIT,TCE,TOPEXT,CPCTR,DBKP,PCTRL3,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,PQM,RDSEED,ADX,SMAP,CLFLUSHOPT,CLWB,SHA,UMIP,IBPB,IBRS,STIBP,SSBD,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
cpu1: 32KB 64b/line 8-way I-cache, 32KB 64b/line 8-way D-cache, 512KB 64b/line 
8-way L2 cache, 8MB 64b/line disabled L3 cache
cpu1: ITLB 64 4KB entries fully associative, 64 4MB entries fully associative
cpu1: DTLB 64 4KB entries fully associative, 64 4MB entries fully associative
cpu1: smt 0, core 1, package 0
cpu2 at mainbus0: apid 2 (application processor)
cpu2: AMD Ryzen 7 4700U with Radeon Graphics, 1996.25 MHz, 17-60-01
cpu2: 

Re: acpicpu(4) and ACPI0007

2020-07-27 Thread Mark Kettenis
> Date: Mon, 27 Jul 2020 11:10:42 -0400
> From: Bryan Steele 
> 
> On Mon, Jul 27, 2020 at 05:02:41PM +0200, Mark Kettenis wrote:
> > Recent ACPI versions have deprecated "Processor()" nodes in favout of
> > "Device()" nodes with a _HID() method that returns "ACPI0007".  This
> > diff tries to support machines with firmware that implements this.  If
> > you see something like:
> > 
> >   "ACPI0007" at acpi0 not configured
> > 
> > please try the following diff and report back with an updated dmesg.
> > 
> > Cheers,
> > 
> > Mark
> > 
> 
> Wrong diff?

Yes, too many diffs that start with acpi...

Thanks,

Mark

> > Index: dev/acpi/acpi.c
> > ===
> > RCS file: /cvs/src/sys/dev/acpi/acpi.c,v
> > retrieving revision 1.384
> > diff -u -p -r1.384 acpi.c
> > --- dev/acpi/acpi.c 11 May 2020 17:57:17 -  1.384
> > +++ dev/acpi/acpi.c 13 May 2020 18:44:32 -
> > @@ -72,6 +72,7 @@ int   acpi_debug = 16;
> >  
> >  intacpi_poll_enabled;
> >  intacpi_hasprocfvs;
> > +intacpi_haspci;
> >  
> >  #define ACPIEN_RETRIES 15
> >  
> > Index: dev/acpi/acpivar.h
> > ===
> > RCS file: /cvs/src/sys/dev/acpi/acpivar.h,v
> > retrieving revision 1.108
> > diff -u -p -r1.108 acpivar.h
> > --- dev/acpi/acpivar.h  8 May 2020 11:18:01 -   1.108
> > +++ dev/acpi/acpivar.h  13 May 2020 18:44:32 -
> > @@ -43,6 +43,7 @@ extern int acpi_debug;
> >  #endif
> >  
> >  extern int acpi_hasprocfvs;
> > +extern int acpi_haspci;
> >  
> >  struct klist;
> >  struct acpiec_softc;
> > Index: arch/amd64/amd64/mainbus.c
> > ===
> > RCS file: /cvs/src/sys/arch/amd64/amd64/mainbus.c,v
> > retrieving revision 1.49
> > diff -u -p -r1.49 mainbus.c
> > --- arch/amd64/amd64/mainbus.c  7 Sep 2019 13:46:19 -   1.49
> > +++ arch/amd64/amd64/mainbus.c  13 May 2020 18:44:32 -
> > @@ -231,6 +231,13 @@ mainbus_attach(struct device *parent, st
> >  #endif
> >  
> >  #if NPCI > 0
> > +#if NACPI > 0
> > +   if (acpi_haspci) {
> > +   extern void acpipci_attach_busses(struct device *);
> > +
> > +   acpipci_attach_busses(self);
> > +   } else
> > +#endif
> > {
> > pci_init_extents();
> >  
> > @@ -245,9 +252,6 @@ mainbus_attach(struct device *parent, st
> > mba.mba_pba.pba_domain = pci_ndomains++;
> > mba.mba_pba.pba_bus = 0;
> > config_found(self, _pba, mainbus_print);
> > -#if NACPI > 0
> > -   acpi_pciroots_attach(self, _pba, mainbus_print);
> > -#endif
> > }
> >  #endif
> >  
> > Index: arch/amd64/conf/RAMDISK
> > ===
> > RCS file: /cvs/src/sys/arch/amd64/conf/RAMDISK,v
> > retrieving revision 1.77
> > diff -u -p -r1.77 RAMDISK
> > --- arch/amd64/conf/RAMDISK 5 Mar 2020 16:36:30 -   1.77
> > +++ arch/amd64/conf/RAMDISK 13 May 2020 18:44:32 -
> > @@ -30,6 +30,7 @@ acpi0 at bios?
> >  #acpicpu*  at acpi?
> >  acpicmos*  at acpi?
> >  acpiec*at acpi?
> > +acpipci*   at acpi?
> >  acpiprt*   at acpi?
> >  acpimadt0  at acpi?
> >  #acpitz*   at acpi?
> > Index: arch/amd64/conf/RAMDISK_CD
> > ===
> > RCS file: /cvs/src/sys/arch/amd64/conf/RAMDISK_CD,v
> > retrieving revision 1.188
> > diff -u -p -r1.188 RAMDISK_CD
> > --- arch/amd64/conf/RAMDISK_CD  15 Feb 2020 08:49:11 -  1.188
> > +++ arch/amd64/conf/RAMDISK_CD  13 May 2020 18:44:32 -
> > @@ -37,6 +37,7 @@ acpi0 at bios?
> >  #acpicpu*  at acpi?
> >  acpicmos*  at acpi?
> >  acpiec*at acpi?
> > +acpipci*   at acpi?
> >  acpiprt*   at acpi?
> >  acpimadt0  at acpi?
> >  #acpitz*   at acpi?
> > Index: arch/amd64/pci/acpipci.c
> > ===
> > RCS file: /cvs/src/sys/arch/amd64/pci/acpipci.c,v
> > retrieving revision 1.3
> > diff -u -p -r1.3 acpipci.c
> > --- arch/amd64/pci/acpipci.c7 Sep 2019 13:46:19 -   1.3
> > +++ arch/amd64/pci/acpipci.c13 May 2020 18:44:32 -
> > @@ -53,6 +53,19 @@ struct acpipci_softc {
> > struct device   sc_dev;
> > struct acpi_softc *sc_acpi;
> > struct aml_node *sc_node;
> > +
> > +   bus_space_tag_t sc_iot;
> > +   bus_space_tag_t sc_memt;
> > +   bus_dma_tag_t   sc_dmat;
> > +
> > +   struct extent   *sc_busex;
> > +   struct extent   *sc_memex;
> > +   struct extent   *sc_ioex;
> > +   charsc_busex_name[32];
> > +   charsc_ioex_name[32];
> > +   charsc_memex_name[32];
> > +   int sc_bus;
> > +   uint32_tsc_seg;
> >  };
> >  
> >  intacpipci_match(struct device *, void *, void *);
> > @@ -72,6 +85,11 @@ const char *acpipci_hids[] = {
> > NULL
> >  };
> >  
> > +void   

Re: acpicpu(4) and ACPI0007

2020-07-27 Thread Mark Kettenis
> Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> From: Mark Kettenis 
> 
> Recent ACPI versions have deprecated "Processor()" nodes in favout of
> "Device()" nodes with a _HID() method that returns "ACPI0007".  This
> diff tries to support machines with firmware that implements this.  If
> you see something like:
> 
>   "ACPI0007" at acpi0 not configured
> 
> please try the following diff and report back with an updated dmesg.
> 
> Cheers,
> 
> Mark

And now with the right diff...


Index: dev/acpi/acpicpu.c
===
RCS file: /cvs/src/sys/dev/acpi/acpicpu.c,v
retrieving revision 1.85
diff -u -p -r1.85 acpicpu.c
--- dev/acpi/acpicpu.c  27 May 2020 05:02:21 -  1.85
+++ dev/acpi/acpicpu.c  27 Jul 2020 14:58:38 -
@@ -186,6 +186,11 @@ struct cfdriver acpicpu_cd = {
NULL, "acpicpu", DV_DULL
 };
 
+const char *acpicpu_hids[] = {
+   "ACPI0007",
+   NULL
+};
+
 extern int setperf_prio;
 
 struct acpicpu_softc *acpicpu_sc[MAXCPUS];
@@ -650,6 +655,9 @@ acpicpu_match(struct device *parent, voi
struct acpi_attach_args *aa = aux;
struct cfdata   *cf = match;
 
+   if (acpi_matchhids(aa, acpicpu_hids, cf->cf_driver->cd_name))
+   return (1);
+
/* sanity */
if (aa->aaa_name == NULL ||
strcmp(aa->aaa_name, cf->cf_driver->cd_name) != 0 ||
@@ -665,6 +673,7 @@ acpicpu_attach(struct device *parent, st
struct acpicpu_softc*sc = (struct acpicpu_softc *)self;
struct acpi_attach_args *aa = aux;
struct aml_valueres;
+   int64_t uid;
int i;
uint32_tstatus = 0;
CPU_INFO_ITERATOR   cii;
@@ -675,6 +684,10 @@ acpicpu_attach(struct device *parent, st
acpicpu_sc[sc->sc_dev.dv_unit] = sc;
 
SLIST_INIT(>sc_cstates);
+
+   if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode,
+   "_UID", 0, NULL, ) == 0)
+   sc->sc_cpu = uid;
 
if (aml_evalnode(sc->sc_acpi, sc->sc_devnode, 0, NULL, ) == 0) {
if (res.type == AML_OBJTYPE_PROCESSOR) {



Re: acpicpu(4) and ACPI0007

2020-07-27 Thread Bryan Steele
On Mon, Jul 27, 2020 at 05:02:41PM +0200, Mark Kettenis wrote:
> Recent ACPI versions have deprecated "Processor()" nodes in favout of
> "Device()" nodes with a _HID() method that returns "ACPI0007".  This
> diff tries to support machines with firmware that implements this.  If
> you see something like:
> 
>   "ACPI0007" at acpi0 not configured
> 
> please try the following diff and report back with an updated dmesg.
> 
> Cheers,
> 
> Mark
> 

Wrong diff?

> 
> Index: dev/acpi/acpi.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpi.c,v
> retrieving revision 1.384
> diff -u -p -r1.384 acpi.c
> --- dev/acpi/acpi.c   11 May 2020 17:57:17 -  1.384
> +++ dev/acpi/acpi.c   13 May 2020 18:44:32 -
> @@ -72,6 +72,7 @@ int acpi_debug = 16;
>  
>  int  acpi_poll_enabled;
>  int  acpi_hasprocfvs;
> +int  acpi_haspci;
>  
>  #define ACPIEN_RETRIES 15
>  
> Index: dev/acpi/acpivar.h
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpivar.h,v
> retrieving revision 1.108
> diff -u -p -r1.108 acpivar.h
> --- dev/acpi/acpivar.h8 May 2020 11:18:01 -   1.108
> +++ dev/acpi/acpivar.h13 May 2020 18:44:32 -
> @@ -43,6 +43,7 @@ extern int acpi_debug;
>  #endif
>  
>  extern int acpi_hasprocfvs;
> +extern int acpi_haspci;
>  
>  struct klist;
>  struct acpiec_softc;
> Index: arch/amd64/amd64/mainbus.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/mainbus.c,v
> retrieving revision 1.49
> diff -u -p -r1.49 mainbus.c
> --- arch/amd64/amd64/mainbus.c7 Sep 2019 13:46:19 -   1.49
> +++ arch/amd64/amd64/mainbus.c13 May 2020 18:44:32 -
> @@ -231,6 +231,13 @@ mainbus_attach(struct device *parent, st
>  #endif
>  
>  #if NPCI > 0
> +#if NACPI > 0
> + if (acpi_haspci) {
> + extern void acpipci_attach_busses(struct device *);
> +
> + acpipci_attach_busses(self);
> + } else
> +#endif
>   {
>   pci_init_extents();
>  
> @@ -245,9 +252,6 @@ mainbus_attach(struct device *parent, st
>   mba.mba_pba.pba_domain = pci_ndomains++;
>   mba.mba_pba.pba_bus = 0;
>   config_found(self, _pba, mainbus_print);
> -#if NACPI > 0
> - acpi_pciroots_attach(self, _pba, mainbus_print);
> -#endif
>   }
>  #endif
>  
> Index: arch/amd64/conf/RAMDISK
> ===
> RCS file: /cvs/src/sys/arch/amd64/conf/RAMDISK,v
> retrieving revision 1.77
> diff -u -p -r1.77 RAMDISK
> --- arch/amd64/conf/RAMDISK   5 Mar 2020 16:36:30 -   1.77
> +++ arch/amd64/conf/RAMDISK   13 May 2020 18:44:32 -
> @@ -30,6 +30,7 @@ acpi0   at bios?
>  #acpicpu*at acpi?
>  acpicmos*at acpi?
>  acpiec*  at acpi?
> +acpipci* at acpi?
>  acpiprt* at acpi?
>  acpimadt0at acpi?
>  #acpitz* at acpi?
> Index: arch/amd64/conf/RAMDISK_CD
> ===
> RCS file: /cvs/src/sys/arch/amd64/conf/RAMDISK_CD,v
> retrieving revision 1.188
> diff -u -p -r1.188 RAMDISK_CD
> --- arch/amd64/conf/RAMDISK_CD15 Feb 2020 08:49:11 -  1.188
> +++ arch/amd64/conf/RAMDISK_CD13 May 2020 18:44:32 -
> @@ -37,6 +37,7 @@ acpi0   at bios?
>  #acpicpu*at acpi?
>  acpicmos*at acpi?
>  acpiec*  at acpi?
> +acpipci* at acpi?
>  acpiprt* at acpi?
>  acpimadt0at acpi?
>  #acpitz* at acpi?
> Index: arch/amd64/pci/acpipci.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/pci/acpipci.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 acpipci.c
> --- arch/amd64/pci/acpipci.c  7 Sep 2019 13:46:19 -   1.3
> +++ arch/amd64/pci/acpipci.c  13 May 2020 18:44:32 -
> @@ -53,6 +53,19 @@ struct acpipci_softc {
>   struct device   sc_dev;
>   struct acpi_softc *sc_acpi;
>   struct aml_node *sc_node;
> +
> + bus_space_tag_t sc_iot;
> + bus_space_tag_t sc_memt;
> + bus_dma_tag_t   sc_dmat;
> +
> + struct extent   *sc_busex;
> + struct extent   *sc_memex;
> + struct extent   *sc_ioex;
> + charsc_busex_name[32];
> + charsc_ioex_name[32];
> + charsc_memex_name[32];
> + int sc_bus;
> + uint32_tsc_seg;
>  };
>  
>  int  acpipci_match(struct device *, void *, void *);
> @@ -72,6 +85,11 @@ const char *acpipci_hids[] = {
>   NULL
>  };
>  
> +void acpipci_attach_deferred(struct device *);
> +int  acpipci_print(void *, const char *);
> +int  acpipci_parse_resources(int, union acpi_resource *, void *);
> +void acpipci_osc(struct acpipci_softc *);
> +
>  int
>  acpipci_match(struct device *parent, void *match, void *aux)
>  {
> @@ -86,15 +104,225 @@ acpipci_attach(struct device *parent, st
>  {
>   

acpicpu(4) and ACPI0007

2020-07-27 Thread Mark Kettenis
Recent ACPI versions have deprecated "Processor()" nodes in favout of
"Device()" nodes with a _HID() method that returns "ACPI0007".  This
diff tries to support machines with firmware that implements this.  If
you see something like:

  "ACPI0007" at acpi0 not configured

please try the following diff and report back with an updated dmesg.

Cheers,

Mark



Index: dev/acpi/acpi.c
===
RCS file: /cvs/src/sys/dev/acpi/acpi.c,v
retrieving revision 1.384
diff -u -p -r1.384 acpi.c
--- dev/acpi/acpi.c 11 May 2020 17:57:17 -  1.384
+++ dev/acpi/acpi.c 13 May 2020 18:44:32 -
@@ -72,6 +72,7 @@ int   acpi_debug = 16;
 
 intacpi_poll_enabled;
 intacpi_hasprocfvs;
+intacpi_haspci;
 
 #define ACPIEN_RETRIES 15
 
Index: dev/acpi/acpivar.h
===
RCS file: /cvs/src/sys/dev/acpi/acpivar.h,v
retrieving revision 1.108
diff -u -p -r1.108 acpivar.h
--- dev/acpi/acpivar.h  8 May 2020 11:18:01 -   1.108
+++ dev/acpi/acpivar.h  13 May 2020 18:44:32 -
@@ -43,6 +43,7 @@ extern int acpi_debug;
 #endif
 
 extern int acpi_hasprocfvs;
+extern int acpi_haspci;
 
 struct klist;
 struct acpiec_softc;
Index: arch/amd64/amd64/mainbus.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/mainbus.c,v
retrieving revision 1.49
diff -u -p -r1.49 mainbus.c
--- arch/amd64/amd64/mainbus.c  7 Sep 2019 13:46:19 -   1.49
+++ arch/amd64/amd64/mainbus.c  13 May 2020 18:44:32 -
@@ -231,6 +231,13 @@ mainbus_attach(struct device *parent, st
 #endif
 
 #if NPCI > 0
+#if NACPI > 0
+   if (acpi_haspci) {
+   extern void acpipci_attach_busses(struct device *);
+
+   acpipci_attach_busses(self);
+   } else
+#endif
{
pci_init_extents();
 
@@ -245,9 +252,6 @@ mainbus_attach(struct device *parent, st
mba.mba_pba.pba_domain = pci_ndomains++;
mba.mba_pba.pba_bus = 0;
config_found(self, _pba, mainbus_print);
-#if NACPI > 0
-   acpi_pciroots_attach(self, _pba, mainbus_print);
-#endif
}
 #endif
 
Index: arch/amd64/conf/RAMDISK
===
RCS file: /cvs/src/sys/arch/amd64/conf/RAMDISK,v
retrieving revision 1.77
diff -u -p -r1.77 RAMDISK
--- arch/amd64/conf/RAMDISK 5 Mar 2020 16:36:30 -   1.77
+++ arch/amd64/conf/RAMDISK 13 May 2020 18:44:32 -
@@ -30,6 +30,7 @@ acpi0 at bios?
 #acpicpu*  at acpi?
 acpicmos*  at acpi?
 acpiec*at acpi?
+acpipci*   at acpi?
 acpiprt*   at acpi?
 acpimadt0  at acpi?
 #acpitz*   at acpi?
Index: arch/amd64/conf/RAMDISK_CD
===
RCS file: /cvs/src/sys/arch/amd64/conf/RAMDISK_CD,v
retrieving revision 1.188
diff -u -p -r1.188 RAMDISK_CD
--- arch/amd64/conf/RAMDISK_CD  15 Feb 2020 08:49:11 -  1.188
+++ arch/amd64/conf/RAMDISK_CD  13 May 2020 18:44:32 -
@@ -37,6 +37,7 @@ acpi0 at bios?
 #acpicpu*  at acpi?
 acpicmos*  at acpi?
 acpiec*at acpi?
+acpipci*   at acpi?
 acpiprt*   at acpi?
 acpimadt0  at acpi?
 #acpitz*   at acpi?
Index: arch/amd64/pci/acpipci.c
===
RCS file: /cvs/src/sys/arch/amd64/pci/acpipci.c,v
retrieving revision 1.3
diff -u -p -r1.3 acpipci.c
--- arch/amd64/pci/acpipci.c7 Sep 2019 13:46:19 -   1.3
+++ arch/amd64/pci/acpipci.c13 May 2020 18:44:32 -
@@ -53,6 +53,19 @@ struct acpipci_softc {
struct device   sc_dev;
struct acpi_softc *sc_acpi;
struct aml_node *sc_node;
+
+   bus_space_tag_t sc_iot;
+   bus_space_tag_t sc_memt;
+   bus_dma_tag_t   sc_dmat;
+
+   struct extent   *sc_busex;
+   struct extent   *sc_memex;
+   struct extent   *sc_ioex;
+   charsc_busex_name[32];
+   charsc_ioex_name[32];
+   charsc_memex_name[32];
+   int sc_bus;
+   uint32_tsc_seg;
 };
 
 intacpipci_match(struct device *, void *, void *);
@@ -72,6 +85,11 @@ const char *acpipci_hids[] = {
NULL
 };
 
+void   acpipci_attach_deferred(struct device *);
+intacpipci_print(void *, const char *);
+intacpipci_parse_resources(int, union acpi_resource *, void *);
+void   acpipci_osc(struct acpipci_softc *);
+
 int
 acpipci_match(struct device *parent, void *match, void *aux)
 {
@@ -86,15 +104,225 @@ acpipci_attach(struct device *parent, st
 {
struct acpi_attach_args *aaa = aux;
struct acpipci_softc *sc = (struct acpipci_softc *)self;
-   struct aml_value args[4];
struct aml_value res;
-   static uint8_t uuid[16] = ACPI_PCI_UUID;
-   uint32_t buf[3];
+   uint64_t bbn = 0;
+   uint64_t seg = 0;
+
+