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

["Michael S. Tsirkin" <mst@redhat.com>]

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.

> 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]$

> > Please include this info in both the commit log and a code comment.
> 
> Will do.
> 
> Matthias.

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