From mboxrd@z Thu Jan 1 00:00:00 1970 Received: by 2002:a5d:6089:0:0:0:0:0 with SMTP id w9csp810177wrt; Thu, 22 Nov 2018 07:14:31 -0800 (PST) X-Google-Smtp-Source: AJdET5dWnrK8N/hzP4civyVTLJ/jf2Y9BlZgRDBI84FyOM8IqqITgplcFD3xCj6RDTBudrlpQVRV X-Received: by 2002:aed:2cc4:: with SMTP id g62mr10369648qtd.192.1542899671745; Thu, 22 Nov 2018 07:14:31 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1542899671; cv=none; d=google.com; s=arc-20160816; b=SwpswXUYUl/V99UtBkvpgazLB7po+S6uC3BdTx2mxF37qaMBAAOQ15kbo6Ua6k/4P7 QSibyhzsKd3RR/4QvF6NCimzGiv7Hn/rKSGBd3HWhsUTooDHF8w9i+fdcI4scXJYdcgG bgsCZUohI7P8+OEJGZzCVkWBm3sxI0892BY2KT8sfGKtCTWQexpYIh3qxy87hKk2UzSx kvgE4/AV4lLfdzt259irPHR4L/oosIFWEo4I3VA2MqD+uoPqslsLtsNcsgcEXAg2y8hX G5pKSOeKvPjp/iJsC1V5fpVIfcCPeHnaVYSaraVJkfuSWBstUajIICNz1EMeIBi4Vbfb VAlg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:cc:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:subject :content-transfer-encoding:mime-version:references:in-reply-to :message-id:to:from:date; bh=lEzjc6h6bH3sURDhTmd0FvegHlMWo75z/xbqgbxoK8w=; b=y6A5K2Z3U9sNLZudjX70Xl0THoUIMz+DoJ48dUgttUqUqSkDY8TmzZlecXzAjEFx8b CQ0N83Sr7wVebLh9uyfpR1YaNwLSKkB4fxka1BSRPXkYx7rvovx0CMwUyiGr+huk58IV NzgrVk708Tfyz9DFhk6Khz9sWvU6o5Pb6TbLMfsIhAfOpmvsXict3Q3T6XtzX1pb5drR aE+42nWPLDJmg7OakjVkRZHWKFwVUaC4BjJIzJgLIaRRVpSwRmy8cXQETFk1GfnQEY5z SOclbUQjRk4/GdLNmcx7ITlZM7MLKz0IvbVTPDeorvoUUo0v7KMszk0OPCBINB/BXibN LD6A== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) smtp.mailfrom="qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org. [2001:4830:134:3::11]) by mx.google.com with ESMTPS id 98si5351557qky.119.2018.11.22.07.14.31 for (version=TLS1 cipher=AES128-SHA bits=128/128); Thu, 22 Nov 2018 07:14:31 -0800 (PST) Received-SPF: pass (google.com: domain of qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) client-ip=2001:4830:134:3::11; Authentication-Results: mx.google.com; spf=pass (google.com: domain of qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) smtp.mailfrom="qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from localhost ([::1]:47308 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gPqgY-0005LG-UO for alex.bennee@linaro.org; Thu, 22 Nov 2018 10:14:30 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33375) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gPqgJ-0005JP-2B for qemu-arm@nongnu.org; Thu, 22 Nov 2018 10:14:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gPqgD-00031p-Mb for qemu-arm@nongnu.org; Thu, 22 Nov 2018 10:14:14 -0500 Received: from mx1.redhat.com ([209.132.183.28]:53596) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gPqgD-0002yv-8q; Thu, 22 Nov 2018 10:14:09 -0500 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id DF4D73086265; Thu, 22 Nov 2018 15:14:07 +0000 (UTC) Received: from localhost (unknown [10.43.2.182]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5E1D27EFCF; Thu, 22 Nov 2018 15:14:01 +0000 (UTC) Date: Thu, 22 Nov 2018 16:13:59 +0100 From: Igor Mammedov To: Samuel Ortiz Message-ID: <20181122161359.246b5978@redhat.com> In-Reply-To: <20181121144237.GF4426@caravaggio> References: <20181105014047.26447-1-sameo@linux.intel.com> <20181105014047.26447-2-sameo@linux.intel.com> <20181109152316.43b79217@redhat.com> <20181121144237.GF4426@caravaggio> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.49]); Thu, 22 Nov 2018 15:14:08 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: Re: [Qemu-arm] [Qemu-devel] [PATCH v5 01/24] hw: i386: Decouple the ACPI build from the PC machine type X-BeenThere: qemu-arm@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Peter Maydell , Stefano Stabellini , Eduardo Habkost , "Michael S. Tsirkin" , qemu-devel@nongnu.org, Shannon Zhao , qemu-arm@nongnu.org, xen-devel@lists.xenproject.org, Anthony Perard , Paolo Bonzini , Richard Henderson Errors-To: qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org Sender: "Qemu-arm" X-TUID: B49YXp7BJQ62 On Wed, 21 Nov 2018 15:42:37 +0100 Samuel Ortiz wrote: > Hi Igor, > > On Fri, Nov 09, 2018 at 03:23:16PM +0100, Igor Mammedov wrote: > > On Mon, 5 Nov 2018 02:40:24 +0100 > > Samuel Ortiz wrote: > > > > > ACPI tables are platform and machine type and even architecture > > > agnostic, and as such we want to provide an internal ACPI API that > > > only depends on platform agnostic information. > > > > > > For the x86 architecture, in order to build ACPI tables independently > > > from the PC or Q35 machine types, we are moving a few MachineState > > > structure fields into a machine type agnostic structure called > > > AcpiConfiguration. The structure fields we move are: > > > > It's not obvious why new structure is needed, especially at > > the beginning of series. We probably should place this patch > > much later in the series (if we need it at all) and try > > generalize a much as possible without using it. > Patches order set aside, this new structure is needed to make the > existing API not completely bound to the pc machine type anymore and > "Decouple the ACPI build from the PC machine type". > > It was either creating a structure to build ACPI tables in a machine > type independent fashion, or pass custom structures (or potentially long > list of arguments) to the existing APIs. See below. > > > > And try to come up with an API that doesn't need centralized collection > > of data somehow related to ACPI (most of the fields here are not generic > > and applicable to a specific board/target). > > > > For generic API, I'd prefer a separate building blocks > > like build_fadt()/... that take as an input only parameters > > necessary to compose a table/aml part with occasional board > > interface hooks instead of all encompassing AcpiConfiguration > > and board specific 'acpi_build' that would use them when/if needed. > Let's take build_madt as an example. With my patch we define: > > void build_madt(GArray *table_data, BIOSLinker *linker, > MachineState *ms, AcpiConfiguration *conf); > > And you're suggesting we'd define: > > void build_madt(GArray *table_data, BIOSLinker *linker, > MachineState *ms, HotplugHandler *acpi_dev, > bool apic_xrupt_override); > > instead. Is that correct? in general, yes and let acpi_build() to fish out that info from board somehow. In case of build_madt(), I doubt we can generalize it across targets. What we can do with it though, is to generalize entries that are placed into it. i.e. create helpers to create them instead of open-codding them like now, something like: void build_append_madt_apic(*table, uid, apic_id, flags); void build_append_madt_x2apic(*table, uid, apic_id, flags); void build_append_madt_ioapic(*table, io_apic_id, io_apic_addr, interrupt); ... converting to build_append_int_noprefix() API ioapic entry example: diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h index af8e023..5911b94 100644 --- a/include/hw/acpi/acpi-defs.h +++ b/include/hw/acpi/acpi-defs.h @@ -242,16 +242,6 @@ struct AcpiMadtProcessorApic { } QEMU_PACKED; typedef struct AcpiMadtProcessorApic AcpiMadtProcessorApic; -struct AcpiMadtIoApic { - ACPI_SUB_HEADER_DEF - uint8_t io_apic_id; /* I/O APIC ID */ - uint8_t reserved; /* Reserved - must be zero */ - uint32_t address; /* APIC physical address */ - uint32_t interrupt; /* Global system interrupt where INTI - * lines start */ -} QEMU_PACKED; -typedef struct AcpiMadtIoApic AcpiMadtIoApic; - struct AcpiMadtIntsrcovr { ACPI_SUB_HEADER_DEF uint8_t bus; diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h index 6c36903..b28c2ce 100644 --- a/include/hw/acpi/aml-build.h +++ b/include/hw/acpi/aml-build.h @@ -409,6 +409,9 @@ build_append_gas_from_struct(GArray *table, const struct AcpiGenericAddress *s) s->access_width, s->address); } +void build_append_madt_ioapic(GArray *tbl, uint8_t id, uint32_t addr, + uint32_t intr); + void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base, uint64_t len, int node, MemoryAffinityFlags flags); diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c index 1e43cd7..f0445df 100644 --- a/hw/acpi/aml-build.c +++ b/hw/acpi/aml-build.c @@ -1655,6 +1655,23 @@ void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base, } /* + * ACPI 1.0b: 5.2.8.2 IO APIC + <--- earliest spec where structure was introduced, that's the way + we avoid writing extra docs for things that are described in spec --> + */ +void build_append_madt_ioapic(GArray *tbl, uint8_t id, uint32_t addr, + uint32_t intr) +{ + // comments are verbatim copy of the field name from spec + build_append_int_noprefix(tbl, 1 /* I/O APIC structure */, 1); /* Type */ + build_append_int_noprefix(tbl, 12, 1); /* Length */ + build_append_int_noprefix(tbl, id, 1); /* I/O APIC ID */ + build_append_int_noprefix(tbl, 0, 1); /* Reserved */ + build_append_int_noprefix(tbl, addr, 4); /* I/O APIC Address */ + build_append_int_noprefix(tbl, intr, 4); /* Global System Interrupt Base */ +} + +/* * ACPI spec 5.2.17 System Locality Distance Information Table * (Revision 2.0 or later) */ diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 131c565..2f154c9 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -348,7 +348,6 @@ build_madt(GArray *table_data, BIOSLinker *linker, PCMachineState *pcms) bool x2apic_mode = false; AcpiMultipleApicTable *madt; - AcpiMadtIoApic *io_apic; AcpiMadtIntsrcovr *intsrcovr; int i; @@ -363,12 +362,8 @@ build_madt(GArray *table_data, BIOSLinker *linker, PCMachineState *pcms) } } - io_apic = acpi_data_push(table_data, sizeof *io_apic); - io_apic->type = ACPI_APIC_IO; - io_apic->length = sizeof(*io_apic); - io_apic->io_apic_id = ACPI_BUILD_IOAPIC_ID; - io_apic->address = cpu_to_le32(IO_APIC_DEFAULT_ADDRESS); - io_apic->interrupt = cpu_to_le32(0); + build_append_madt_ioapic(table_data, + ACPI_BUILD_IOAPIC_ID, IO_APIC_DEFAULT_ADDRESS, 0); if (pcms->apic_xrupt_override) { intsrcovr = acpi_data_push(table_data, sizeof *intsrcovr); .... Then build_madt() is reduced to quite readable several lines only and if it makes sense having a custom board specific one, it is not an issue as there aren't much duplication. In case of i386/virt, one probably can throw away all legacy stuff we have over there and have custom build_madt_virt(). > Pros for the latter is the fact that, as you said, we would not need to > define a centralized structure holding all possibly needed ACPI related > fields. > > Pros for the former is about defining a pointer to all needed ACPI > fields once and for all and hiding the details of the API in the AML > building implementation. it's hard to create an maintain centralized structure for generic use, for example this series ended up with a bunch of x86 specific fields there and misplaced other fields that do not belong acpi but are used by it somehow. If one would thing about possible future refactorings, then it gets more difficult as changes to centralized structure affect whole codebase instead of being localized. I suggest to keep board and acpi code as separate as possible and acpi_setup/acpi_build() being a glue that gets and feeds board/target specific data to an ACPI primitives/table builders. That way we would have a machine and in-depended acpi code, and custom glue that ties all together. One would have to give up on generic acpi_build and AcpiBuilder callbacks, but acpi_build() is not a lot of code so it is not an issue to have several different ones (we can generalize parts of it is necessary). Essentially this series would change from ---------- pc.c: AcpiBuilder->build_a = pc_build_a; AcpiBuilder->build_b = pc_build_b; ... virt_pc.c: AcpiBuilder->build_a = virt_pc_build_a; AcpiBuilder->build_b = virt_pc_build_b; ... acpi...c: pc_acpi_build() AcpiBuilder->build_a() AcpiBuilder->build_b() // some legacy hacks here ... virt_pc...c: virt_acpi_build() AcpiBuilder->build_a() AcpiBuilder->build_b() ... ... same for arm since set of tables is different and board/target depended ... --------------- acpi...c: pc_acpi_build() pc_build_a() pc_build_b() ... virt_pc_..c: virt_acpi_build() virt_pc_build_a() virt_pc_build_b() ... ... i.e. about the same as the former but without indirection and without headache how to generalize AcpiBuilder->build_foo() interface to work for every target/board. Once we have hw/i386/acpi_reduced.c, working and used by i386/virt it would be easier to assess if we can generalize virt_pc_acpi_build() to be usable by arm, but right now it looks like premature action. > > We probably should split series into several smaller > > (if possible independent) ones, so people won't be scared of > > its sheer size and run away from reviewing it. > I will try to split it in smaller chunks if that helps. > > Cheers, > Samuel. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Igor Mammedov Subject: Re: [Qemu-devel] [PATCH v5 01/24] hw: i386: Decouple the ACPI build from the PC machine type Date: Thu, 22 Nov 2018 16:13:59 +0100 Message-ID: <20181122161359.246b5978@redhat.com> References: <20181105014047.26447-1-sameo@linux.intel.com> <20181105014047.26447-2-sameo@linux.intel.com> <20181109152316.43b79217@redhat.com> <20181121144237.GF4426@caravaggio> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1gPqgE-0004Lc-0a for xen-devel@lists.xenproject.org; Thu, 22 Nov 2018 15:14:10 +0000 In-Reply-To: <20181121144237.GF4426@caravaggio> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" To: Samuel Ortiz Cc: Peter Maydell , Stefano Stabellini , Eduardo Habkost , "Michael S. Tsirkin" , qemu-devel@nongnu.org, Shannon Zhao , qemu-arm@nongnu.org, xen-devel@lists.xenproject.org, Anthony Perard , Paolo Bonzini , Richard Henderson List-Id: xen-devel@lists.xenproject.org T24gV2VkLCAyMSBOb3YgMjAxOCAxNTo0MjozNyArMDEwMApTYW11ZWwgT3J0aXogPHNhbWVvQGxp bnV4LmludGVsLmNvbT4gd3JvdGU6Cgo+IEhpIElnb3IsCj4gCj4gT24gRnJpLCBOb3YgMDksIDIw MTggYXQgMDM6MjM6MTZQTSArMDEwMCwgSWdvciBNYW1tZWRvdiB3cm90ZToKPiA+IE9uIE1vbiwg IDUgTm92IDIwMTggMDI6NDA6MjQgKzAxMDAKPiA+IFNhbXVlbCBPcnRpeiA8c2FtZW9AbGludXgu aW50ZWwuY29tPiB3cm90ZToKPiA+ICAgCj4gPiA+IEFDUEkgdGFibGVzIGFyZSBwbGF0Zm9ybSBh bmQgbWFjaGluZSB0eXBlIGFuZCBldmVuIGFyY2hpdGVjdHVyZQo+ID4gPiBhZ25vc3RpYywgYW5k IGFzIHN1Y2ggd2Ugd2FudCB0byBwcm92aWRlIGFuIGludGVybmFsIEFDUEkgQVBJIHRoYXQKPiA+ ID4gb25seSBkZXBlbmRzIG9uIHBsYXRmb3JtIGFnbm9zdGljIGluZm9ybWF0aW9uLgo+ID4gPiAK PiA+ID4gRm9yIHRoZSB4ODYgYXJjaGl0ZWN0dXJlLCBpbiBvcmRlciB0byBidWlsZCBBQ1BJIHRh YmxlcyBpbmRlcGVuZGVudGx5Cj4gPiA+IGZyb20gdGhlIFBDIG9yIFEzNSBtYWNoaW5lIHR5cGVz LCB3ZSBhcmUgbW92aW5nIGEgZmV3IE1hY2hpbmVTdGF0ZQo+ID4gPiBzdHJ1Y3R1cmUgZmllbGRz IGludG8gYSBtYWNoaW5lIHR5cGUgYWdub3N0aWMgc3RydWN0dXJlIGNhbGxlZAo+ID4gPiBBY3Bp Q29uZmlndXJhdGlvbi4gVGhlIHN0cnVjdHVyZSBmaWVsZHMgd2UgbW92ZSBhcmU6ICAKPiA+IAo+ ID4gSXQncyBub3Qgb2J2aW91cyB3aHkgbmV3IHN0cnVjdHVyZSBpcyBuZWVkZWQsIGVzcGVjaWFs bHkgYXQKPiA+IHRoZSBiZWdpbm5pbmcgb2Ygc2VyaWVzLiBXZSBwcm9iYWJseSBzaG91bGQgcGxh Y2UgdGhpcyBwYXRjaAo+ID4gbXVjaCBsYXRlciBpbiB0aGUgc2VyaWVzIChpZiB3ZSBuZWVkIGl0 IGF0IGFsbCkgYW5kIHRyeQo+ID4gZ2VuZXJhbGl6ZSBhIG11Y2ggYXMgcG9zc2libGUgd2l0aG91 dCB1c2luZyBpdC4gIAo+IFBhdGNoZXMgb3JkZXIgc2V0IGFzaWRlLCB0aGlzIG5ldyBzdHJ1Y3R1 cmUgaXMgbmVlZGVkIHRvIG1ha2UgdGhlCj4gZXhpc3RpbmcgQVBJIG5vdCBjb21wbGV0ZWx5IGJv dW5kIHRvIHRoZSBwYyBtYWNoaW5lIHR5cGUgYW55bW9yZSBhbmQKPiAiRGVjb3VwbGUgdGhlIEFD UEkgYnVpbGQgZnJvbSB0aGUgUEMgbWFjaGluZSB0eXBlIi4KPiAKPiBJdCB3YXMgZWl0aGVyIGNy ZWF0aW5nIGEgc3RydWN0dXJlIHRvIGJ1aWxkIEFDUEkgdGFibGVzIGluIGEgbWFjaGluZQo+IHR5 cGUgaW5kZXBlbmRlbnQgZmFzaGlvbiwgb3IgcGFzcyBjdXN0b20gc3RydWN0dXJlcyAob3IgcG90 ZW50aWFsbHkgbG9uZwo+IGxpc3Qgb2YgYXJndW1lbnRzKSB0byB0aGUgZXhpc3RpbmcgQVBJcy4g U2VlIGJlbG93Lgo+IAo+IAo+ID4gQW5kIHRyeSB0byBjb21lIHVwIHdpdGggYW4gQVBJIHRoYXQg ZG9lc24ndCBuZWVkIGNlbnRyYWxpemVkIGNvbGxlY3Rpb24KPiA+IG9mIGRhdGEgc29tZWhvdyBy ZWxhdGVkIHRvIEFDUEkgKG1vc3Qgb2YgdGhlIGZpZWxkcyBoZXJlIGFyZSBub3QgZ2VuZXJpYwo+ ID4gYW5kIGFwcGxpY2FibGUgdG8gYSBzcGVjaWZpYyBib2FyZC90YXJnZXQpLgo+ID4gCj4gPiBG b3IgZ2VuZXJpYyBBUEksIEknZCBwcmVmZXIgYSBzZXBhcmF0ZSBidWlsZGluZyBibG9ja3MKPiA+ IGxpa2UgYnVpbGRfZmFkdCgpLy4uLiB0aGF0IHRha2UgYXMgYW4gaW5wdXQgb25seSBwYXJhbWV0 ZXJzCj4gPiBuZWNlc3NhcnkgdG8gY29tcG9zZSBhIHRhYmxlL2FtbCBwYXJ0IHdpdGggb2NjYXNp b25hbCBib2FyZAo+ID4gaW50ZXJmYWNlIGhvb2tzIGluc3RlYWQgb2YgYWxsIGVuY29tcGFzc2lu ZyBBY3BpQ29uZmlndXJhdGlvbgo+ID4gYW5kIGJvYXJkIHNwZWNpZmljICdhY3BpX2J1aWxkJyB0 aGF0IHdvdWxkIHVzZSB0aGVtIHdoZW4vaWYgbmVlZGVkLiAgCj4gTGV0J3MgdGFrZSBidWlsZF9t YWR0IGFzIGFuIGV4YW1wbGUuIFdpdGggbXkgcGF0Y2ggd2UgZGVmaW5lOgo+IAo+IHZvaWQgYnVp bGRfbWFkdChHQXJyYXkgKnRhYmxlX2RhdGEsIEJJT1NMaW5rZXIgKmxpbmtlciwKPiAgICAgICAg ICAgICAgICAgTWFjaGluZVN0YXRlICptcywgQWNwaUNvbmZpZ3VyYXRpb24gKmNvbmYpOwo+IAo+ IEFuZCB5b3UncmUgc3VnZ2VzdGluZyB3ZSdkIGRlZmluZToKPiAKPiB2b2lkIGJ1aWxkX21hZHQo R0FycmF5ICp0YWJsZV9kYXRhLCBCSU9TTGlua2VyICpsaW5rZXIsCj4gICAgICAgICAgICAgICAg IE1hY2hpbmVTdGF0ZSAqbXMsIEhvdHBsdWdIYW5kbGVyICphY3BpX2RldiwKPiAgICAgICAgICAg ICAgICAgYm9vbCBhcGljX3hydXB0X292ZXJyaWRlKTsKPiAKPiBpbnN0ZWFkLiBJcyB0aGF0IGNv cnJlY3Q/CmluIGdlbmVyYWwsIHllcwphbmQgbGV0IGFjcGlfYnVpbGQoKSB0byBmaXNoIG91dCB0 aGF0IGluZm8gZnJvbSBib2FyZCBzb21laG93LgoKSW4gY2FzZSBvZiBidWlsZF9tYWR0KCksIEkg ZG91YnQgd2UgY2FuIGdlbmVyYWxpemUgaXQgYWNyb3NzIHRhcmdldHMuCldoYXQgd2UgY2FuIGRv IHdpdGggaXQgdGhvdWdoLCBpcyB0byBnZW5lcmFsaXplIGVudHJpZXMgdGhhdCBhcmUKcGxhY2Vk IGludG8gaXQuIGkuZS4gY3JlYXRlIGhlbHBlcnMgdG8gY3JlYXRlIHRoZW0gaW5zdGVhZCBvZgpv cGVuLWNvZGRpbmcgdGhlbSBsaWtlIG5vdywgc29tZXRoaW5nIGxpa2U6Cgp2b2lkIGJ1aWxkX2Fw cGVuZF9tYWR0X2FwaWMoKnRhYmxlLCB1aWQsIGFwaWNfaWQsIGZsYWdzKTsKdm9pZCBidWlsZF9h cHBlbmRfbWFkdF94MmFwaWMoKnRhYmxlLCB1aWQsIGFwaWNfaWQsIGZsYWdzKTsKdm9pZCBidWls ZF9hcHBlbmRfbWFkdF9pb2FwaWMoKnRhYmxlLCBpb19hcGljX2lkLCBpb19hcGljX2FkZHIsIGlu dGVycnVwdCk7Ci4uLgpjb252ZXJ0aW5nIHRvIGJ1aWxkX2FwcGVuZF9pbnRfbm9wcmVmaXgoKSBB UEkgaW9hcGljIGVudHJ5IGV4YW1wbGU6CgpkaWZmIC0tZ2l0IGEvaW5jbHVkZS9ody9hY3BpL2Fj cGktZGVmcy5oIGIvaW5jbHVkZS9ody9hY3BpL2FjcGktZGVmcy5oCmluZGV4IGFmOGUwMjMuLjU5 MTFiOTQgMTAwNjQ0Ci0tLSBhL2luY2x1ZGUvaHcvYWNwaS9hY3BpLWRlZnMuaAorKysgYi9pbmNs dWRlL2h3L2FjcGkvYWNwaS1kZWZzLmgKQEAgLTI0MiwxNiArMjQyLDYgQEAgc3RydWN0IEFjcGlN YWR0UHJvY2Vzc29yQXBpYyB7CiB9IFFFTVVfUEFDS0VEOwogdHlwZWRlZiBzdHJ1Y3QgQWNwaU1h ZHRQcm9jZXNzb3JBcGljIEFjcGlNYWR0UHJvY2Vzc29yQXBpYzsKIAotc3RydWN0IEFjcGlNYWR0 SW9BcGljIHsKLSAgICBBQ1BJX1NVQl9IRUFERVJfREVGCi0gICAgdWludDhfdCAgaW9fYXBpY19p ZDsgICAgICAgICAgICAgLyogSS9PIEFQSUMgSUQgKi8KLSAgICB1aW50OF90ICByZXNlcnZlZDsg ICAgICAgICAgICAgICAvKiBSZXNlcnZlZCAtIG11c3QgYmUgemVybyAqLwotICAgIHVpbnQzMl90 IGFkZHJlc3M7ICAgICAgICAgICAgICAgIC8qIEFQSUMgcGh5c2ljYWwgYWRkcmVzcyAqLwotICAg IHVpbnQzMl90IGludGVycnVwdDsgICAgICAgICAgICAgIC8qIEdsb2JhbCBzeXN0ZW0gaW50ZXJy dXB0IHdoZXJlIElOVEkKLSAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICogbGluZXMg c3RhcnQgKi8KLX0gUUVNVV9QQUNLRUQ7Ci10eXBlZGVmIHN0cnVjdCBBY3BpTWFkdElvQXBpYyBB Y3BpTWFkdElvQXBpYzsKLQogc3RydWN0IEFjcGlNYWR0SW50c3Jjb3ZyIHsKICAgICBBQ1BJX1NV Ql9IRUFERVJfREVGCiAgICAgdWludDhfdCAgYnVzOwpkaWZmIC0tZ2l0IGEvaW5jbHVkZS9ody9h Y3BpL2FtbC1idWlsZC5oIGIvaW5jbHVkZS9ody9hY3BpL2FtbC1idWlsZC5oCmluZGV4IDZjMzY5 MDMuLmIyOGMyY2UgMTAwNjQ0Ci0tLSBhL2luY2x1ZGUvaHcvYWNwaS9hbWwtYnVpbGQuaAorKysg Yi9pbmNsdWRlL2h3L2FjcGkvYW1sLWJ1aWxkLmgKQEAgLTQwOSw2ICs0MDksOSBAQCBidWlsZF9h cHBlbmRfZ2FzX2Zyb21fc3RydWN0KEdBcnJheSAqdGFibGUsIGNvbnN0IHN0cnVjdCBBY3BpR2Vu ZXJpY0FkZHJlc3MgKnMpCiAgICAgICAgICAgICAgICAgICAgICBzLT5hY2Nlc3Nfd2lkdGgsIHMt PmFkZHJlc3MpOwogfQogCit2b2lkIGJ1aWxkX2FwcGVuZF9tYWR0X2lvYXBpYyhHQXJyYXkgKnRi bCwgdWludDhfdCBpZCwgdWludDMyX3QgYWRkciwKKyAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgIHVpbnQzMl90IGludHIpOworCiB2b2lkIGJ1aWxkX3NyYXRfbWVtb3J5KEFjcGlTcmF0TWVt b3J5QWZmaW5pdHkgKm51bWFtZW0sIHVpbnQ2NF90IGJhc2UsCiAgICAgICAgICAgICAgICAgICAg ICAgIHVpbnQ2NF90IGxlbiwgaW50IG5vZGUsIE1lbW9yeUFmZmluaXR5RmxhZ3MgZmxhZ3MpOwog CmRpZmYgLS1naXQgYS9ody9hY3BpL2FtbC1idWlsZC5jIGIvaHcvYWNwaS9hbWwtYnVpbGQuYwpp bmRleCAxZTQzY2Q3Li5mMDQ0NWRmIDEwMDY0NAotLS0gYS9ody9hY3BpL2FtbC1idWlsZC5jCisr KyBiL2h3L2FjcGkvYW1sLWJ1aWxkLmMKQEAgLTE2NTUsNiArMTY1NSwyMyBAQCB2b2lkIGJ1aWxk X3NyYXRfbWVtb3J5KEFjcGlTcmF0TWVtb3J5QWZmaW5pdHkgKm51bWFtZW0sIHVpbnQ2NF90IGJh c2UsCiB9CiAKIC8qCisgKiBBQ1BJIDEuMGI6IDUuMi44LjIgSU8gQVBJQworICAgPC0tLSBlYXJs aWVzdCBzcGVjIHdoZXJlIHN0cnVjdHVyZSB3YXMgaW50cm9kdWNlZCwgdGhhdCdzIHRoZSB3YXkK KyAgICAgICAgd2UgYXZvaWQgd3JpdGluZyBleHRyYSBkb2NzIGZvciB0aGluZ3MgdGhhdCBhcmUg ZGVzY3JpYmVkIGluIHNwZWMgLS0+CisgKi8KK3ZvaWQgYnVpbGRfYXBwZW5kX21hZHRfaW9hcGlj KEdBcnJheSAqdGJsLCB1aW50OF90IGlkLCB1aW50MzJfdCBhZGRyLAorICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgdWludDMyX3QgaW50cikKK3sKKyAgICAvLyBjb21tZW50cyBhcmUgdmVy YmF0aW0gY29weSBvZiB0aGUgZmllbGQgbmFtZSBmcm9tIHNwZWMKKyAgICBidWlsZF9hcHBlbmRf aW50X25vcHJlZml4KHRibCwgMSAvKiBJL08gQVBJQyBzdHJ1Y3R1cmUgKi8sIDEpOyAvKiBUeXBl ICovCisgICAgYnVpbGRfYXBwZW5kX2ludF9ub3ByZWZpeCh0YmwsIDEyLCAxKTsgICAgLyogTGVu Z3RoICovCisgICAgYnVpbGRfYXBwZW5kX2ludF9ub3ByZWZpeCh0YmwsIGlkLCAxKTsgICAgLyog SS9PIEFQSUMgSUQgKi8KKyAgICBidWlsZF9hcHBlbmRfaW50X25vcHJlZml4KHRibCwgMCwgMSk7 ICAgICAvKiBSZXNlcnZlZCAqLworICAgIGJ1aWxkX2FwcGVuZF9pbnRfbm9wcmVmaXgodGJsLCBh ZGRyLCA0KTsgIC8qIEkvTyBBUElDIEFkZHJlc3MgKi8KKyAgICBidWlsZF9hcHBlbmRfaW50X25v cHJlZml4KHRibCwgaW50ciwgNCk7IC8qIEdsb2JhbCBTeXN0ZW0gSW50ZXJydXB0IEJhc2UgKi8K K30KKworLyoKICAqIEFDUEkgc3BlYyA1LjIuMTcgU3lzdGVtIExvY2FsaXR5IERpc3RhbmNlIElu Zm9ybWF0aW9uIFRhYmxlCiAgKiAoUmV2aXNpb24gMi4wIG9yIGxhdGVyKQogICovCmRpZmYgLS1n aXQgYS9ody9pMzg2L2FjcGktYnVpbGQuYyBiL2h3L2kzODYvYWNwaS1idWlsZC5jCmluZGV4IDEz MWM1NjUuLjJmMTU0YzkgMTAwNjQ0Ci0tLSBhL2h3L2kzODYvYWNwaS1idWlsZC5jCisrKyBiL2h3 L2kzODYvYWNwaS1idWlsZC5jCkBAIC0zNDgsNyArMzQ4LDYgQEAgYnVpbGRfbWFkdChHQXJyYXkg KnRhYmxlX2RhdGEsIEJJT1NMaW5rZXIgKmxpbmtlciwgUENNYWNoaW5lU3RhdGUgKnBjbXMpCiAg ICAgYm9vbCB4MmFwaWNfbW9kZSA9IGZhbHNlOwogCiAgICAgQWNwaU11bHRpcGxlQXBpY1RhYmxl ICptYWR0OwotICAgIEFjcGlNYWR0SW9BcGljICppb19hcGljOwogICAgIEFjcGlNYWR0SW50c3Jj b3ZyICppbnRzcmNvdnI7CiAgICAgaW50IGk7CiAKQEAgLTM2MywxMiArMzYyLDggQEAgYnVpbGRf bWFkdChHQXJyYXkgKnRhYmxlX2RhdGEsIEJJT1NMaW5rZXIgKmxpbmtlciwgUENNYWNoaW5lU3Rh dGUgKnBjbXMpCiAgICAgICAgIH0KICAgICB9CiAKLSAgICBpb19hcGljID0gYWNwaV9kYXRhX3B1 c2godGFibGVfZGF0YSwgc2l6ZW9mICppb19hcGljKTsKLSAgICBpb19hcGljLT50eXBlID0gQUNQ SV9BUElDX0lPOwotICAgIGlvX2FwaWMtPmxlbmd0aCA9IHNpemVvZigqaW9fYXBpYyk7Ci0gICAg aW9fYXBpYy0+aW9fYXBpY19pZCA9IEFDUElfQlVJTERfSU9BUElDX0lEOwotICAgIGlvX2FwaWMt PmFkZHJlc3MgPSBjcHVfdG9fbGUzMihJT19BUElDX0RFRkFVTFRfQUREUkVTUyk7Ci0gICAgaW9f YXBpYy0+aW50ZXJydXB0ID0gY3B1X3RvX2xlMzIoMCk7CisgICAgYnVpbGRfYXBwZW5kX21hZHRf aW9hcGljKHRhYmxlX2RhdGEsCisgICAgICAgIEFDUElfQlVJTERfSU9BUElDX0lELCBJT19BUElD X0RFRkFVTFRfQUREUkVTUywgMCk7CiAKICAgICBpZiAocGNtcy0+YXBpY194cnVwdF9vdmVycmlk ZSkgewogICAgICAgICBpbnRzcmNvdnIgPSBhY3BpX2RhdGFfcHVzaCh0YWJsZV9kYXRhLCBzaXpl b2YgKmludHNyY292cik7CgoKLi4uLgoKVGhlbiBidWlsZF9tYWR0KCkgaXMgcmVkdWNlZCB0byBx dWl0ZSByZWFkYWJsZSBzZXZlcmFsIGxpbmVzIG9ubHkKYW5kIGlmIGl0IG1ha2VzIHNlbnNlIGhh dmluZyBhIGN1c3RvbSBib2FyZCBzcGVjaWZpYyBvbmUsIGl0IGlzIG5vdAphbiBpc3N1ZSBhcyB0 aGVyZSBhcmVuJ3QgbXVjaCBkdXBsaWNhdGlvbi4KSW4gY2FzZSBvZiBpMzg2L3ZpcnQsIG9uZSBw cm9iYWJseSBjYW4gdGhyb3cgYXdheSBhbGwgbGVnYWN5CnN0dWZmIHdlIGhhdmUgb3ZlciB0aGVy ZSBhbmQgaGF2ZSBjdXN0b20gYnVpbGRfbWFkdF92aXJ0KCkuCgo+IFByb3MgZm9yIHRoZSBsYXR0 ZXIgaXMgdGhlIGZhY3QgdGhhdCwgYXMgeW91IHNhaWQsIHdlIHdvdWxkIG5vdCBuZWVkIHRvCj4g ZGVmaW5lIGEgY2VudHJhbGl6ZWQgc3RydWN0dXJlIGhvbGRpbmcgYWxsIHBvc3NpYmx5IG5lZWRl ZCBBQ1BJIHJlbGF0ZWQKPiBmaWVsZHMuCj4KPiBQcm9zIGZvciB0aGUgZm9ybWVyIGlzIGFib3V0 IGRlZmluaW5nIGEgcG9pbnRlciB0byBhbGwgbmVlZGVkIEFDUEkKPiBmaWVsZHMgb25jZSBhbmQg Zm9yIGFsbCBhbmQgaGlkaW5nIHRoZSBkZXRhaWxzIG9mIHRoZSBBUEkgaW4gdGhlIEFNTAo+IGJ1 aWxkaW5nIGltcGxlbWVudGF0aW9uLgoKaXQncyBoYXJkIHRvIGNyZWF0ZSBhbiBtYWludGFpbiBj ZW50cmFsaXplZCBzdHJ1Y3R1cmUgZm9yIGdlbmVyaWMgdXNlLApmb3IgZXhhbXBsZSB0aGlzIHNl cmllcyBlbmRlZCB1cCB3aXRoIGEgYnVuY2ggb2YgeDg2IHNwZWNpZmljIGZpZWxkcyB0aGVyZQph bmQgbWlzcGxhY2VkIG90aGVyIGZpZWxkcyB0aGF0IGRvIG5vdCBiZWxvbmcgYWNwaSBidXQgYXJl IHVzZWQgYnkgaXQgc29tZWhvdy4KSWYgb25lIHdvdWxkIHRoaW5nIGFib3V0IHBvc3NpYmxlIGZ1 dHVyZSByZWZhY3RvcmluZ3MsIHRoZW4gaXQgZ2V0cwptb3JlIGRpZmZpY3VsdCBhcyBjaGFuZ2Vz IHRvIGNlbnRyYWxpemVkIHN0cnVjdHVyZSBhZmZlY3Qgd2hvbGUgY29kZWJhc2UKaW5zdGVhZCBv ZiBiZWluZyBsb2NhbGl6ZWQuCgpJIHN1Z2dlc3QgdG8ga2VlcCBib2FyZCBhbmQgYWNwaSBjb2Rl IGFzIHNlcGFyYXRlIGFzIHBvc3NpYmxlCmFuZCBhY3BpX3NldHVwL2FjcGlfYnVpbGQoKSBiZWlu ZyBhIGdsdWUgdGhhdCBnZXRzIGFuZCBmZWVkcwpib2FyZC90YXJnZXQgc3BlY2lmaWMgZGF0YSB0 byBhbiBBQ1BJIHByaW1pdGl2ZXMvdGFibGUgYnVpbGRlcnMuClRoYXQgd2F5IHdlIHdvdWxkIGhh dmUgYSBtYWNoaW5lIGFuZCBpbi1kZXBlbmRlZCBhY3BpIGNvZGUsCmFuZCBjdXN0b20gZ2x1ZSB0 aGF0IHRpZXMgYWxsIHRvZ2V0aGVyLgoKT25lIHdvdWxkIGhhdmUgdG8gZ2l2ZSB1cCBvbiBnZW5l cmljIGFjcGlfYnVpbGQgYW5kIEFjcGlCdWlsZGVyCmNhbGxiYWNrcywgYnV0IGFjcGlfYnVpbGQo KSBpcyBub3QgYSBsb3Qgb2YgY29kZSBzbyBpdCBpcyBub3QKYW4gaXNzdWUgdG8gaGF2ZSBzZXZl cmFsIGRpZmZlcmVudCBvbmVzICh3ZSBjYW4gZ2VuZXJhbGl6ZSBwYXJ0cwpvZiBpdCBpcyBuZWNl c3NhcnkpLgpFc3NlbnRpYWxseSB0aGlzIHNlcmllcyB3b3VsZCBjaGFuZ2UgZnJvbQotLS0tLS0t LS0tCnBjLmM6CkFjcGlCdWlsZGVyLT5idWlsZF9hID0gcGNfYnVpbGRfYTsKQWNwaUJ1aWxkZXIt PmJ1aWxkX2IgPSBwY19idWlsZF9iOwouLi4KCnZpcnRfcGMuYzoKQWNwaUJ1aWxkZXItPmJ1aWxk X2EgPSB2aXJ0X3BjX2J1aWxkX2E7CkFjcGlCdWlsZGVyLT5idWlsZF9iID0gdmlydF9wY19idWls ZF9iOwouLi4KCmFjcGkuLi5jOgpwY19hY3BpX2J1aWxkKCkKICAgQWNwaUJ1aWxkZXItPmJ1aWxk X2EoKQogICBBY3BpQnVpbGRlci0+YnVpbGRfYigpCiAgIC8vIHNvbWUgbGVnYWN5IGhhY2tzIGhl cmUKICAgLi4uCnZpcnRfcGMuLi5jOgp2aXJ0X2FjcGlfYnVpbGQoKQogICBBY3BpQnVpbGRlci0+ YnVpbGRfYSgpCiAgIEFjcGlCdWlsZGVyLT5idWlsZF9iKCkKICAgLi4uCgouLi4gc2FtZSBmb3Ig YXJtIHNpbmNlIHNldCBvZiB0YWJsZXMgaXMgZGlmZmVyZW50IGFuZCBib2FyZC90YXJnZXQgZGVw ZW5kZWQgLi4uCi0tLS0tLS0tLS0tLS0tLQphY3BpLi4uYzoKcGNfYWNwaV9idWlsZCgpCiAgcGNf YnVpbGRfYSgpCiAgcGNfYnVpbGRfYigpCiAgLi4uCgp2aXJ0X3BjXy4uYzoKdmlydF9hY3BpX2J1 aWxkKCkKICB2aXJ0X3BjX2J1aWxkX2EoKQogIHZpcnRfcGNfYnVpbGRfYigpCiAgLi4uCi4uLgoK aS5lLiBhYm91dCB0aGUgc2FtZSAgYXMgdGhlIGZvcm1lciBidXQgd2l0aG91dCBpbmRpcmVjdGlv biBhbmQKd2l0aG91dCBoZWFkYWNoZSBob3cgdG8gZ2VuZXJhbGl6ZSBBY3BpQnVpbGRlci0+YnVp bGRfZm9vKCkKaW50ZXJmYWNlIHRvIHdvcmsgZm9yIGV2ZXJ5IHRhcmdldC9ib2FyZC4KCk9uY2Ug d2UgaGF2ZSBody9pMzg2L2FjcGlfcmVkdWNlZC5jLCB3b3JraW5nIGFuZCB1c2VkIGJ5IGkzODYv dmlydAppdCB3b3VsZCBiZSBlYXNpZXIgdG8gYXNzZXNzIGlmIHdlIGNhbiBnZW5lcmFsaXplIHZp cnRfcGNfYWNwaV9idWlsZCgpCnRvIGJlIHVzYWJsZSBieSBhcm0sIGJ1dCByaWdodCBub3cgaXQg bG9va3MgbGlrZSBwcmVtYXR1cmUgYWN0aW9uLgoKPiA+IFdlIHByb2JhYmx5IHNob3VsZCBzcGxp dCBzZXJpZXMgaW50byBzZXZlcmFsIHNtYWxsZXIKPiA+IChpZiBwb3NzaWJsZSBpbmRlcGVuZGVu dCkgb25lcywgc28gcGVvcGxlIHdvbid0IGJlIHNjYXJlZCBvZgo+ID4gaXRzIHNoZWVyIHNpemUg YW5kIHJ1biBhd2F5IGZyb20gcmV2aWV3aW5nIGl0LiAgCj4gSSB3aWxsIHRyeSB0byBzcGxpdCBp dCBpbiBzbWFsbGVyIGNodW5rcyBpZiB0aGF0IGhlbHBzLgo+IAo+IENoZWVycywKPiBTYW11ZWwu CgoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRl dmVsIG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVucHJvamVjdC5vcmcKaHR0cHM6Ly9s aXN0cy54ZW5wcm9qZWN0Lm9yZy9tYWlsbWFuL2xpc3RpbmZvL3hlbi1kZXZlbA== From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33407) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gPqgN-0005Lr-Gj for qemu-devel@nongnu.org; Thu, 22 Nov 2018 10:14:21 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gPqgL-00039M-CY for qemu-devel@nongnu.org; Thu, 22 Nov 2018 10:14:19 -0500 Date: Thu, 22 Nov 2018 16:13:59 +0100 From: Igor Mammedov Message-ID: <20181122161359.246b5978@redhat.com> In-Reply-To: <20181121144237.GF4426@caravaggio> References: <20181105014047.26447-1-sameo@linux.intel.com> <20181105014047.26447-2-sameo@linux.intel.com> <20181109152316.43b79217@redhat.com> <20181121144237.GF4426@caravaggio> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 01/24] hw: i386: Decouple the ACPI build from the PC machine type List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Samuel Ortiz Cc: Peter Maydell , Stefano Stabellini , Eduardo Habkost , "Michael S. Tsirkin" , qemu-devel@nongnu.org, Shannon Zhao , qemu-arm@nongnu.org, Paolo Bonzini , Anthony Perard , xen-devel@lists.xenproject.org, Richard Henderson On Wed, 21 Nov 2018 15:42:37 +0100 Samuel Ortiz wrote: > Hi Igor, > > On Fri, Nov 09, 2018 at 03:23:16PM +0100, Igor Mammedov wrote: > > On Mon, 5 Nov 2018 02:40:24 +0100 > > Samuel Ortiz wrote: > > > > > ACPI tables are platform and machine type and even architecture > > > agnostic, and as such we want to provide an internal ACPI API that > > > only depends on platform agnostic information. > > > > > > For the x86 architecture, in order to build ACPI tables independently > > > from the PC or Q35 machine types, we are moving a few MachineState > > > structure fields into a machine type agnostic structure called > > > AcpiConfiguration. The structure fields we move are: > > > > It's not obvious why new structure is needed, especially at > > the beginning of series. We probably should place this patch > > much later in the series (if we need it at all) and try > > generalize a much as possible without using it. > Patches order set aside, this new structure is needed to make the > existing API not completely bound to the pc machine type anymore and > "Decouple the ACPI build from the PC machine type". > > It was either creating a structure to build ACPI tables in a machine > type independent fashion, or pass custom structures (or potentially long > list of arguments) to the existing APIs. See below. > > > > And try to come up with an API that doesn't need centralized collection > > of data somehow related to ACPI (most of the fields here are not generic > > and applicable to a specific board/target). > > > > For generic API, I'd prefer a separate building blocks > > like build_fadt()/... that take as an input only parameters > > necessary to compose a table/aml part with occasional board > > interface hooks instead of all encompassing AcpiConfiguration > > and board specific 'acpi_build' that would use them when/if needed. > Let's take build_madt as an example. With my patch we define: > > void build_madt(GArray *table_data, BIOSLinker *linker, > MachineState *ms, AcpiConfiguration *conf); > > And you're suggesting we'd define: > > void build_madt(GArray *table_data, BIOSLinker *linker, > MachineState *ms, HotplugHandler *acpi_dev, > bool apic_xrupt_override); > > instead. Is that correct? in general, yes and let acpi_build() to fish out that info from board somehow. In case of build_madt(), I doubt we can generalize it across targets. What we can do with it though, is to generalize entries that are placed into it. i.e. create helpers to create them instead of open-codding them like now, something like: void build_append_madt_apic(*table, uid, apic_id, flags); void build_append_madt_x2apic(*table, uid, apic_id, flags); void build_append_madt_ioapic(*table, io_apic_id, io_apic_addr, interrupt); ... converting to build_append_int_noprefix() API ioapic entry example: diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h index af8e023..5911b94 100644 --- a/include/hw/acpi/acpi-defs.h +++ b/include/hw/acpi/acpi-defs.h @@ -242,16 +242,6 @@ struct AcpiMadtProcessorApic { } QEMU_PACKED; typedef struct AcpiMadtProcessorApic AcpiMadtProcessorApic; -struct AcpiMadtIoApic { - ACPI_SUB_HEADER_DEF - uint8_t io_apic_id; /* I/O APIC ID */ - uint8_t reserved; /* Reserved - must be zero */ - uint32_t address; /* APIC physical address */ - uint32_t interrupt; /* Global system interrupt where INTI - * lines start */ -} QEMU_PACKED; -typedef struct AcpiMadtIoApic AcpiMadtIoApic; - struct AcpiMadtIntsrcovr { ACPI_SUB_HEADER_DEF uint8_t bus; diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h index 6c36903..b28c2ce 100644 --- a/include/hw/acpi/aml-build.h +++ b/include/hw/acpi/aml-build.h @@ -409,6 +409,9 @@ build_append_gas_from_struct(GArray *table, const struct AcpiGenericAddress *s) s->access_width, s->address); } +void build_append_madt_ioapic(GArray *tbl, uint8_t id, uint32_t addr, + uint32_t intr); + void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base, uint64_t len, int node, MemoryAffinityFlags flags); diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c index 1e43cd7..f0445df 100644 --- a/hw/acpi/aml-build.c +++ b/hw/acpi/aml-build.c @@ -1655,6 +1655,23 @@ void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base, } /* + * ACPI 1.0b: 5.2.8.2 IO APIC + <--- earliest spec where structure was introduced, that's the way + we avoid writing extra docs for things that are described in spec --> + */ +void build_append_madt_ioapic(GArray *tbl, uint8_t id, uint32_t addr, + uint32_t intr) +{ + // comments are verbatim copy of the field name from spec + build_append_int_noprefix(tbl, 1 /* I/O APIC structure */, 1); /* Type */ + build_append_int_noprefix(tbl, 12, 1); /* Length */ + build_append_int_noprefix(tbl, id, 1); /* I/O APIC ID */ + build_append_int_noprefix(tbl, 0, 1); /* Reserved */ + build_append_int_noprefix(tbl, addr, 4); /* I/O APIC Address */ + build_append_int_noprefix(tbl, intr, 4); /* Global System Interrupt Base */ +} + +/* * ACPI spec 5.2.17 System Locality Distance Information Table * (Revision 2.0 or later) */ diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 131c565..2f154c9 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -348,7 +348,6 @@ build_madt(GArray *table_data, BIOSLinker *linker, PCMachineState *pcms) bool x2apic_mode = false; AcpiMultipleApicTable *madt; - AcpiMadtIoApic *io_apic; AcpiMadtIntsrcovr *intsrcovr; int i; @@ -363,12 +362,8 @@ build_madt(GArray *table_data, BIOSLinker *linker, PCMachineState *pcms) } } - io_apic = acpi_data_push(table_data, sizeof *io_apic); - io_apic->type = ACPI_APIC_IO; - io_apic->length = sizeof(*io_apic); - io_apic->io_apic_id = ACPI_BUILD_IOAPIC_ID; - io_apic->address = cpu_to_le32(IO_APIC_DEFAULT_ADDRESS); - io_apic->interrupt = cpu_to_le32(0); + build_append_madt_ioapic(table_data, + ACPI_BUILD_IOAPIC_ID, IO_APIC_DEFAULT_ADDRESS, 0); if (pcms->apic_xrupt_override) { intsrcovr = acpi_data_push(table_data, sizeof *intsrcovr); .... Then build_madt() is reduced to quite readable several lines only and if it makes sense having a custom board specific one, it is not an issue as there aren't much duplication. In case of i386/virt, one probably can throw away all legacy stuff we have over there and have custom build_madt_virt(). > Pros for the latter is the fact that, as you said, we would not need to > define a centralized structure holding all possibly needed ACPI related > fields. > > Pros for the former is about defining a pointer to all needed ACPI > fields once and for all and hiding the details of the API in the AML > building implementation. it's hard to create an maintain centralized structure for generic use, for example this series ended up with a bunch of x86 specific fields there and misplaced other fields that do not belong acpi but are used by it somehow. If one would thing about possible future refactorings, then it gets more difficult as changes to centralized structure affect whole codebase instead of being localized. I suggest to keep board and acpi code as separate as possible and acpi_setup/acpi_build() being a glue that gets and feeds board/target specific data to an ACPI primitives/table builders. That way we would have a machine and in-depended acpi code, and custom glue that ties all together. One would have to give up on generic acpi_build and AcpiBuilder callbacks, but acpi_build() is not a lot of code so it is not an issue to have several different ones (we can generalize parts of it is necessary). Essentially this series would change from ---------- pc.c: AcpiBuilder->build_a = pc_build_a; AcpiBuilder->build_b = pc_build_b; ... virt_pc.c: AcpiBuilder->build_a = virt_pc_build_a; AcpiBuilder->build_b = virt_pc_build_b; ... acpi...c: pc_acpi_build() AcpiBuilder->build_a() AcpiBuilder->build_b() // some legacy hacks here ... virt_pc...c: virt_acpi_build() AcpiBuilder->build_a() AcpiBuilder->build_b() ... ... same for arm since set of tables is different and board/target depended ... --------------- acpi...c: pc_acpi_build() pc_build_a() pc_build_b() ... virt_pc_..c: virt_acpi_build() virt_pc_build_a() virt_pc_build_b() ... ... i.e. about the same as the former but without indirection and without headache how to generalize AcpiBuilder->build_foo() interface to work for every target/board. Once we have hw/i386/acpi_reduced.c, working and used by i386/virt it would be easier to assess if we can generalize virt_pc_acpi_build() to be usable by arm, but right now it looks like premature action. > > We probably should split series into several smaller > > (if possible independent) ones, so people won't be scared of > > its sheer size and run away from reviewing it. > I will try to split it in smaller chunks if that helps. > > Cheers, > Samuel.