Re: [Qemu-devel] [PATCH] i386/acpi: add _HID to processor nodes

[Matthias Lange <matthias.lange@kernkonzept.com>]

On 10/22/2015 03:02 PM, Michael S. Tsirkin wrote:
> On Thu, Oct 22, 2015 at 02:42:05PM +0200, Matthias Lange wrote:
>> On 10/22/2015 01:48 PM, Michael S. Tsirkin wrote:
>>> On Thu, Oct 22, 2015 at 12:38:11PM +0200, Matthias Lange wrote:
>>>> Processor nodes created via the acpi/aml framework currently don't
>>>> feature a _HID string. This patch appends "ACPI0007" as the _HID
>>>> string to each processor node.
>>>>
>>>> Signed-off-by: Matthias Lange <matthias.lange@kernkonzept.com>
>>>
>>> Indeed,  it does that.  But why?
>>>
>>> ACPI0007 Processor Device. This device provides an alternative to
>>> declaring processors using the
>>> Processor ASL statement.
>>>  
>>> And “Declaring Processors” says
>>>
>>>
>>> 	Each processor in the system must be declared in the ACPI namespace in
>>> 	either the \_SB or \_PR
>>> 	scope but not both. Declaration of processors in the \_PR scope is
>>> 	required for platforms desiring
>>> 	compatibility with ACPI 1.0-based OSPM implementations. Processors are
>>> 	declared either via the
>>> 	ASL Processor statement or the ASL Device statement. A Processor
>>> 	definition declares a processor
>>> 	object that provides processor configuration information and points to
>>> 	the processor register block
>>> 	(P_BLK). A Device definition for a processor is declared using the
>>> 	ACPI0007 hardware identifier
>>> 	(HID). In this case, processor configuration information is provided
>>> 	exclusively by objects in the
>>> 	processor device’s object list.
>>
>> Thanks for pointing me to the right section.
>>
>>> In other words, Processor directive does not need a HID.
>>> In fact, ACPI 1.0 didn't include the ACPI0007 HID at all.
>>
>> Right. But, does Qemu strive for ACPI 1.0 compatibility?
> 
> Where possible - it's handy for old guests.

I understand. But then, why are processors declared under the \_SB scope
and not under \_PR?

>> And e.g.
>> seabios also defines processors using the ALS processor statement
>> including the HID.
> 
> I don't see it there:
> [mst@robin seabios]$ git grep ACI0007
> [mst@robin seabios]$

It's a typo in your command. Have a look into

seabios/src/fw/ssdt-proc.dsl

Matthias.

>>> Please include this info in both the commit log and a code comment.
>>
>> Will do.
>>
> Not the ACPI spec info. I have that. The answer to my question - the
> actual motivation for the patch.
> 
>>>> ---
>>>>  hw/i386/acpi-build.c | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>>>> index 95e0c65..95f7bf9 100644
>>>> --- a/hw/i386/acpi-build.c
>>>> +++ b/hw/i386/acpi-build.c
>>>> @@ -1153,6 +1153,7 @@ build_ssdt(GArray *table_data, GArray *linker,
>>>>          for (i = 0; i < acpi_cpus; i++) {
>>>>              dev = aml_processor(i, 0, 0, "CP%.02X", i);
>>>>  
>>>> +            aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0007")));
>>>>              method = aml_method("_MAT", 0);
>>>>              aml_append(method, aml_return(aml_call1("CPMA", aml_int(i))));
>>>>              aml_append(dev, method);
>>>> -- 
>>>> 1.9.1
>>
>>
>> -- 
>> Matthias Lange, matthias.lange@kernkonzept.com, +49 - 351 - 41 88 86 14
>>
>> Kernkonzept GmbH.  Sitz: Dresden.  Amtsgericht Dresden, HRB 31129.
>> Geschäftsführer: Dr.-Ing. Michael Hohmuth


-- 
Matthias Lange, matthias.lange@kernkonzept.com, +49 - 351 - 41 88 86 14

Kernkonzept GmbH.  Sitz: Dresden.  Amtsgericht Dresden, HRB 31129.
Geschäftsführer: Dr.-Ing. Michael Hohmuth