From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44188) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZqfcC-00014f-18 for qemu-devel@nongnu.org; Mon, 26 Oct 2015 07:07:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zqfc7-0005dl-1i for qemu-devel@nongnu.org; Mon, 26 Oct 2015 07:06:59 -0400 Received: from mx1.redhat.com ([209.132.183.28]:43149) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zqfc6-0005df-QY for qemu-devel@nongnu.org; Mon, 26 Oct 2015 07:06:54 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id 370A4DA0 for ; Mon, 26 Oct 2015 11:06:54 +0000 (UTC) Date: Mon, 26 Oct 2015 12:06:51 +0100 From: Igor Mammedov Message-ID: <20151026120651.59505ebb@nial.brq.redhat.com> In-Reply-To: <20151026100750.GB12077@redhat.com> References: <1445612242-79172-1-git-send-email-imammedo@redhat.com> <1445612242-79172-16-git-send-email-imammedo@redhat.com> <20151024223944-mutt-send-email-mst@redhat.com> <20151026110301.1c5037fd@nial.brq.redhat.com> <20151026100750.GB12077@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 15/19] pc: acpi: bump DSDT revision compliance to v2 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: pbonzini@redhat.com, qemu-devel@nongnu.org On Mon, 26 Oct 2015 12:07:50 +0200 "Michael S. Tsirkin" wrote: > On Mon, Oct 26, 2015 at 11:03:01AM +0100, Igor Mammedov wrote: > > On Sat, 24 Oct 2015 22:40:59 +0300 > > "Michael S. Tsirkin" wrote: > > > > > On Fri, Oct 23, 2015 at 04:57:18PM +0200, Igor Mammedov wrote: > > > > it turns on 64-bit integer handling in OSPM, which we could use > > > > for writing simpler/smaller AML code. > > > > Tested with Windows XP and Windows Server 2008, Linux: > > > > * XP doesn't care about revision and continues to use 32 integers > > > > and boots just fine with this change. > > > > * WS 2008 and Linux - support rev2 and use 64-bit integers > > > > > > > > Signed-off-by: Igor Mammedov > > > > > > This is still planned, right? IIUC you didn't post any code > > > that needs the 64 bit math. > > nope, the next patch 16/19 uses 64-bit math, > > > > it greatly simplifies _CRS as we don't have to do > > 64-bit math manually using 32-bit integers. > > > > And even if we put new MHPD.MCRS() that uses 64-bit math in SSDT, > > it won't crash XP unless someone would try to do memory hotplug > > > > and even it could be 'fixed' if we check _REV on every > > hotplug event, it's a bit ugly but should work. > > Aha. That's exactly what I said. All that patch commit log > says is > pc: acpi: memhp: move MHPD.MCRS method into MHPT table > when in fact you also rework it all to use 64 bit math. yep, it's my fault. I'll fix it. > > So pls don't do this. Pls start by rewriting ASL in C > while keeping AML code identical. Cleanups - next. That's what I'm trying to avoid in this case as reworked code is much simpler than the original ASL. Rewriting original complex MHPD.MCRS() ASL into C and immediately replacing it with simplified version is rather counterproductive especially taking in account that 'make check' tests will fail anyway since code is moved between tables and C-produced AML doesn't exactly match blobs produced by a particular IASL version anyway. But I can certainly do/split it other way around, simplify ASL first and then rewrite result in C. That should make reviewing easier even if tests are broken. > > > > > > > > --- > > > > hw/i386/acpi-build.c | 2 +- > > > > hw/i386/acpi-dsdt.dsl | 2 +- > > > > hw/i386/q35-acpi-dsdt.dsl | 2 +- > > > > 3 files changed, 3 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > > > index 8add4d9..c929540 100644 > > > > --- a/hw/i386/acpi-build.c > > > > +++ b/hw/i386/acpi-build.c > > > > @@ -1484,7 +1484,7 @@ build_dsdt(GArray *table_data, GArray *linker, AcpiMiscInfo *misc) > > > > > > > > memset(dsdt, 0, sizeof *dsdt); > > > > build_header(linker, table_data, dsdt, "DSDT", > > > > - misc->dsdt_size, 1); > > > > + misc->dsdt_size, 2); > > > > } > > > > > > > > static GArray * > > > > diff --git a/hw/i386/acpi-dsdt.dsl b/hw/i386/acpi-dsdt.dsl > > > > index 8dba096..6d46b36 100644 > > > > --- a/hw/i386/acpi-dsdt.dsl > > > > +++ b/hw/i386/acpi-dsdt.dsl > > > > @@ -22,7 +22,7 @@ ACPI_EXTRACT_ALL_CODE AcpiDsdtAmlCode > > > > DefinitionBlock ( > > > > "acpi-dsdt.aml", // Output Filename > > > > "DSDT", // Signature > > > > - 0x01, // DSDT Compliance Revision > > > > + 0x02, // DSDT Compliance Revision > > > > "BXPC", // OEMID > > > > "BXDSDT", // TABLE ID > > > > 0x1 // OEM Revision > > > > diff --git a/hw/i386/q35-acpi-dsdt.dsl b/hw/i386/q35-acpi-dsdt.dsl > > > > index 7be7b37..ecefdec 100644 > > > > --- a/hw/i386/q35-acpi-dsdt.dsl > > > > +++ b/hw/i386/q35-acpi-dsdt.dsl > > > > @@ -28,7 +28,7 @@ ACPI_EXTRACT_ALL_CODE Q35AcpiDsdtAmlCode > > > > DefinitionBlock ( > > > > "q35-acpi-dsdt.aml",// Output Filename > > > > "DSDT", // Signature > > > > - 0x01, // DSDT Compliance Revision > > > > + 0x02, // DSDT Compliance Revision > > > > "BXPC", // OEMID > > > > "BXDSDT", // TABLE ID > > > > 0x2 // OEM Revision > > > > -- > > > > 1.8.3.1