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 <jonat...@d14n.org>
> > 
> > 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 <jonat...@d14n.org>
> > > > 
> > > > 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 <mark.kette...@xs4all.nl>
> > > > > > 
> > > > > > 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(&res) where res is stack junk.  memset(&res, 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 -0000      1.252
> > > +++ dev/acpi/dsdt.c       28 Jul 2020 09:04:15 -0000
> > > @@ -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;
> > 
> > 

Reply via email to