From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54174) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZpG9k-0006Nv-Em for qemu-devel@nongnu.org; Thu, 22 Oct 2015 09:43:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZpG9h-0001Bw-7R for qemu-devel@nongnu.org; Thu, 22 Oct 2015 09:43:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53350) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZpG9g-0001BW-SX for qemu-devel@nongnu.org; Thu, 22 Oct 2015 09:43:45 -0400 Date: Thu, 22 Oct 2015 16:43:40 +0300 From: "Michael S. Tsirkin" Message-ID: <20151022163713-mutt-send-email-mst@redhat.com> References: <1445510291-8764-1-git-send-email-matthias.lange@kernkonzept.com> <20151022144537-mutt-send-email-mst@redhat.com> <5628D99C.4080902@kernkonzept.com> <20151022155947-mutt-send-email-mst@redhat.com> <5628E5F6.1030406@kernkonzept.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <5628E5F6.1030406@kernkonzept.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] i386/acpi: add _HID to processor nodes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Matthias Lange Cc: imammedo@redhat.com, rth@twiddle.net, qemu-devel@nongnu.org, ehabkost@redhat.com, pbonzini@redhat.com On Thu, Oct 22, 2015 at 03:34:46PM +0200, Matthias Lange wrote: > 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 > >>> > >>> Indeed, it does that. But why? > >>> > >>> ACPI0007 Processor Device. This device provides an alternative to > >>> declaring processors using the > >>> Processor ASL statement. > >>> =20 > >>> And =E2=80=9CDeclaring Processors=E2=80=9D says > >>> > >>> > >>> Each processor in the system must be declared in the ACPI namespac= e 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 point= s 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 provid= ed > >>> exclusively by objects in the > >>> processor device=E2=80=99s 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? > >=20 > > Where possible - it's handy for old guests. >=20 > I understand. But then, why are processors declared under the \_SB scop= e > and not under \_PR? Because even very old guests don't seem to care. We can consider moving it there. > >> And e.g. > >> seabios also defines processors using the ALS processor statement > >> including the HID. > >=20 > > I don't see it there: > > [mst@robin seabios]$ git grep ACI0007 > > [mst@robin seabios]$ >=20 > It's a typo in your command. Have a look into >=20 > seabios/src/fw/ssdt-proc.dsl >=20 > Matthias. Right. qemu used to have it too. This probably means it's safe to add. Was dropped in commit 20843d16632c4da3d3d35ddc8d5eb047167693ce without an explanation. Doesn't mean we'll add it back without an explanation too :) > >>> 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. > >=20 > >>>> --- > >>>> 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 *linke= r, > >>>> for (i =3D 0; i < acpi_cpus; i++) { > >>>> dev =3D aml_processor(i, 0, 0, "CP%.02X", i); > >>>> =20 > >>>> + aml_append(dev, aml_name_decl("_HID", aml_string("ACP= I0007"))); > >>>> method =3D aml_method("_MAT", 0); > >>>> aml_append(method, aml_return(aml_call1("CPMA", aml_i= nt(i)))); > >>>> aml_append(dev, method); > >>>> --=20 > >>>> 1.9.1 > >> > >> > >> --=20 > >> Matthias Lange, matthias.lange@kernkonzept.com, +49 - 351 - 41 88 86= 14 > >> > >> Kernkonzept GmbH. Sitz: Dresden. Amtsgericht Dresden, HRB 31129. > >> Gesch=C3=A4ftsf=C3=BChrer: Dr.-Ing. Michael Hohmuth >=20 >=20 > --=20 > Matthias Lange, matthias.lange@kernkonzept.com, +49 - 351 - 41 88 86 14 >=20 > Kernkonzept GmbH. Sitz: Dresden. Amtsgericht Dresden, HRB 31129. > Gesch=C3=A4ftsf=C3=BChrer: Dr.-Ing. Michael Hohmuth