From mboxrd@z Thu Jan 1 00:00:00 1970 Received: by 2002:a5d:6089:0:0:0:0:0 with SMTP id w9csp284738wrt; Tue, 20 Nov 2018 00:23:45 -0800 (PST) X-Google-Smtp-Source: AFSGD/WQEHEHQhBvNvj858ji8khGsYs0eVJZCBxQR8UaXNcobRqgV5Ycdfz1QzS9w147eUnNBTW3 X-Received: by 2002:a25:a141:: with SMTP id z59-v6mr835558ybh.340.1542702225550; Tue, 20 Nov 2018 00:23:45 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1542702225; cv=none; d=google.com; s=arc-20160816; b=wethpxh22b7Bn6w9u1MAWQEklF1QmIXTPGoxXckME1zgUl3JvE31puo2gpnjNeMWMx g6V3pBaZQj5lK7OgW4MdDZHIWZJd4OWuK3Tg1FyiJEwedk/bxRL6jiLjoX9GEfXQofXc bwErTUcm4ksT+AvkHrSwiGS/sn32jdkGpTgM4KyNwTvPv6x7kD+gygJY5kv4Zc481zGK ZFtuPDUJhbfkeWIgYGk3D8U4j3PSB5O+lZ5bsLUfiEZekWY71jbOGVLeZTAcivVAoiv7 gwQNk6dR9BsyfZ9fI2YOoLPQtlJM0aGdufnhUDPbQvj20kdP0KMEexIcdhJ+gz8p9Fmb SoDg== 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=UblW4Vj9rD0JLAvCj2LYz0tTS/R5jkotdfTmWj0jRaw=; b=wuWTbitlT7LVoVNBTOJXVbZteCoHhz6vqGt59+nCu3YTX19ZrzgtfQdzuUIx+f0KI1 7SlDJ6Q55sV8NKQ/kdHjeOaDKo5X9RtDv3qgUGN1r4WNvk7pbStguU8fzGOIdPD8vR+s hVJTW3MCjkNa9O47bGxy0k68cjyW/ZdB5WRFhWmBOo56oVJDshEU0lLzREd3WSpMrkcJ 00iShuS6aFOC5vhn9zb0f2mqddnpw/n2h7QvKmiCChdasw+Ox55vRKccWTz8O9Ta9XDQ jjDhOoeUxKuOAGjyx8zvtG31w7CJeZ/YreD3hFB4RRGp461qCyAuA/t/HFCz76yR3PKo ZCiw== 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 w3-v6si26213772ywj.336.2018.11.20.00.23.45 for (version=TLS1 cipher=AES128-SHA bits=128/128); Tue, 20 Nov 2018 00:23:45 -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]:60731 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gP1Jw-0007iv-Rk for alex.bennee@linaro.org; Tue, 20 Nov 2018 03:23:44 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55561) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gP1Jl-0007io-Lj for qemu-arm@nongnu.org; Tue, 20 Nov 2018 03:23:34 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gP1Jh-0000NJ-Md for qemu-arm@nongnu.org; Tue, 20 Nov 2018 03:23:33 -0500 Received: from mx1.redhat.com ([209.132.183.28]:41786) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gP1Jh-0000MY-Eh; Tue, 20 Nov 2018 03:23:29 -0500 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 193343001921; Tue, 20 Nov 2018 08:23:27 +0000 (UTC) Received: from localhost (unknown [10.43.2.182]) by smtp.corp.redhat.com (Postfix) with ESMTP id 88A5868739; Tue, 20 Nov 2018 08:23:19 +0000 (UTC) Date: Tue, 20 Nov 2018 09:23:18 +0100 From: Igor Mammedov To: "Michael S. Tsirkin" Message-ID: <20181120092318.48f69fa4@redhat.com> In-Reply-To: <20181119131625-mutt-send-email-mst@kernel.org> References: <20181105014047.26447-1-sameo@linux.intel.com> <20181105014047.26447-6-sameo@linux.intel.com> <20181108151623.4de26ecb@redhat.com> <20181119131625-mutt-send-email-mst@kernel.org> 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.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.47]); Tue, 20 Nov 2018 08:23:27 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: Re: [Qemu-arm] [PATCH v5 05/24] hw: acpi: Implement XSDT support for RSDP 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 , Samuel Ortiz , qemu-devel@nongnu.org, Eduardo Habkost , Shannon Zhao , qemu-arm@nongnu.org, Marcel Apfelbaum , Paolo Bonzini , Anthony Perard , xen-devel@lists.xenproject.org, Richard Henderson Errors-To: qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org Sender: "Qemu-arm" X-TUID: ApbVHboTXV1f On Mon, 19 Nov 2018 13:27:14 -0500 "Michael S. Tsirkin" wrote: > On Thu, Nov 08, 2018 at 03:16:23PM +0100, Igor Mammedov wrote: > > On Mon, 5 Nov 2018 02:40:28 +0100 > > Samuel Ortiz wrote: > > > > > XSDT is the 64-bit version of the legacy ACPI RSDT (Root System > > > Description Table). RSDT only allow for 32-bit addressses and have thus > > > been deprecated. Since ACPI version 2.0, RSDPs should point at XSDTs and > > > no longer RSDTs, although RSDTs are still supported for backward > > > compatibility. > > > > > > Since version 2.0, RSDPs should add an extended checksum, a complete table > > > length and a version field to the table. > > > > This patch re-implements what arm/virt board already does > > and fixes checksum bug in the later and at the same time > > without a user (within the patch). > > > > I'd suggest redo it a way similar to FADT refactoring > > patch 1: fix checksum bug in virt/arm > > patch 2: update reference tables in test > > patch 3: introduce AcpiRsdpData similar to commit 937d1b587 > > (both arm and x86) wich stores all data in hos byte order > > patch 4: convert arm's impl. to build_append_int_noprefix() API (commit 5d7a334f7) > > ... move out to aml-build.c > > patch 5: reuse generalized arm's build_rsdp() for x86, dropping x86 specific one > > amending it to generate rev1 variant defined by revision in AcpiRsdpData > > (commit dd1b2037a) > > > > 'make check V=1' shouldn't observe any ACPI tables changes after patch 2. > > And your next suggestion is to add patch 6. I guess it's doable but > this will make a single patch a 6 patch series. At this rate this series > will be at 200 patches easily. > > Automated checks are cool but hey it's easy to see what changed in a > disassembled table, and we do not update them blindly. So just note in > the comment that there's a table change for ARM and expected files need > to be updated and we should be fine IMHO. Point was to move patches that change tables content first, where we would pay extra attentions to changes in tables and then refactoring which shouldn't cause any changes will be mostly automatic (at least at that point we won't have to worry about tables correctness) > > > Signed-off-by: Samuel Ortiz > > > --- > > > include/hw/acpi/aml-build.h | 3 +++ > > > hw/acpi/aml-build.c | 37 +++++++++++++++++++++++++++++++++++++ > > > 2 files changed, 40 insertions(+) > > > > > > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h > > > index c9bcb32d81..3580d0ce90 100644 > > > --- a/include/hw/acpi/aml-build.h > > > +++ b/include/hw/acpi/aml-build.h > > > @@ -393,6 +393,9 @@ void > > > build_rsdp(GArray *table_data, > > > BIOSLinker *linker, unsigned rsdt_tbl_offset); > > > void > > > +build_rsdp_xsdt(GArray *table_data, > > > + BIOSLinker *linker, unsigned xsdt_tbl_offset); > > > +void > > > build_rsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets, > > > const char *oem_id, const char *oem_table_id); > > > void > > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > > > index 51b608432f..a030d40674 100644 > > > --- a/hw/acpi/aml-build.c > > > +++ b/hw/acpi/aml-build.c > > > @@ -1651,6 +1651,43 @@ build_xsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets, > > > (void *)xsdt, "XSDT", xsdt_len, 1, oem_id, oem_table_id); > > > } > > > > > > +/* RSDP pointing at an XSDT */ > > > +void > > > +build_rsdp_xsdt(GArray *rsdp_table, > > > + BIOSLinker *linker, unsigned xsdt_tbl_offset) > > > +{ > > > + AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp); > > > + unsigned xsdt_pa_size = sizeof(rsdp->xsdt_physical_address); > > > + unsigned xsdt_pa_offset = > > > + (char *)&rsdp->xsdt_physical_address - rsdp_table->data; > > > + unsigned xsdt_offset = > > > + (char *)&rsdp->length - rsdp_table->data; > > There's a cleaner way to get at the offsets than pointer math: > 1. save rsdp_table length before you push > 2. add offset_of for fields > > If switching to build_append_int_noprefix then it's even > easier - just save length before you append the int > you intend to patch. > > > > > + > > > + bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, rsdp_table, 16, > > > + true /* fseg memory */); > > > + > > > + memcpy(&rsdp->signature, "RSD PTR ", 8); > > > + memcpy(rsdp->oem_id, ACPI_BUILD_APPNAME6, 6); > > > + rsdp->length = cpu_to_le32(sizeof(*rsdp)); > > > + /* version 2, we will use the XSDT pointer */ > > > + rsdp->revision = 0x02; > > > + > > > + /* Address to be filled by Guest linker */ > > > + bios_linker_loader_add_pointer(linker, > > > + ACPI_BUILD_RSDP_FILE, xsdt_pa_offset, xsdt_pa_size, > > > + ACPI_BUILD_TABLE_FILE, xsdt_tbl_offset); > > > + > > > + /* Legacy checksum to be filled by Guest linker */ > > > + bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE, > > > + (char *)rsdp - rsdp_table->data, xsdt_offset, > > > + (char *)&rsdp->checksum - rsdp_table->data); > > > + > > > + /* Extended checksum to be filled by Guest linker */ > > > + bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE, > > > + (char *)rsdp - rsdp_table->data, sizeof *rsdp, > > > + (char *)&rsdp->extended_checksum - rsdp_table->data); > > > +} > > > + > > > void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base, > > > uint64_t len, int node, MemoryAffinityFlags flags) > > > { From mboxrd@z Thu Jan 1 00:00:00 1970 From: Igor Mammedov Subject: Re: [PATCH v5 05/24] hw: acpi: Implement XSDT support for RSDP Date: Tue, 20 Nov 2018 09:23:18 +0100 Message-ID: <20181120092318.48f69fa4@redhat.com> References: <20181105014047.26447-1-sameo@linux.intel.com> <20181105014047.26447-6-sameo@linux.intel.com> <20181108151623.4de26ecb@redhat.com> <20181119131625-mutt-send-email-mst@kernel.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from us1-rack-dfw2.inumbo.com ([104.130.134.6]) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1gP1Jh-0004Z8-0a for xen-devel@lists.xenproject.org; Tue, 20 Nov 2018 08:23:29 +0000 In-Reply-To: <20181119131625-mutt-send-email-mst@kernel.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" To: "Michael S. Tsirkin" Cc: Peter Maydell , Stefano Stabellini , Samuel Ortiz , qemu-devel@nongnu.org, Eduardo Habkost , Shannon Zhao , qemu-arm@nongnu.org, Marcel Apfelbaum , Paolo Bonzini , Anthony Perard , xen-devel@lists.xenproject.org, Richard Henderson List-Id: xen-devel@lists.xenproject.org T24gTW9uLCAxOSBOb3YgMjAxOCAxMzoyNzoxNCAtMDUwMAoiTWljaGFlbCBTLiBUc2lya2luIiA8 bXN0QHJlZGhhdC5jb20+IHdyb3RlOgoKPiBPbiBUaHUsIE5vdiAwOCwgMjAxOCBhdCAwMzoxNjoy M1BNICswMTAwLCBJZ29yIE1hbW1lZG92IHdyb3RlOgo+ID4gT24gTW9uLCAgNSBOb3YgMjAxOCAw Mjo0MDoyOCArMDEwMAo+ID4gU2FtdWVsIE9ydGl6IDxzYW1lb0BsaW51eC5pbnRlbC5jb20+IHdy b3RlOgo+ID4gICAKPiA+ID4gWFNEVCBpcyB0aGUgNjQtYml0IHZlcnNpb24gb2YgdGhlIGxlZ2Fj eSBBQ1BJIFJTRFQgKFJvb3QgU3lzdGVtCj4gPiA+IERlc2NyaXB0aW9uIFRhYmxlKS4gUlNEVCBv bmx5IGFsbG93IGZvciAzMi1iaXQgYWRkcmVzc3NlcyBhbmQgaGF2ZSB0aHVzCj4gPiA+IGJlZW4g ZGVwcmVjYXRlZC4gU2luY2UgQUNQSSB2ZXJzaW9uIDIuMCwgUlNEUHMgc2hvdWxkIHBvaW50IGF0 IFhTRFRzIGFuZAo+ID4gPiBubyBsb25nZXIgUlNEVHMsIGFsdGhvdWdoIFJTRFRzIGFyZSBzdGls bCBzdXBwb3J0ZWQgZm9yIGJhY2t3YXJkCj4gPiA+IGNvbXBhdGliaWxpdHkuCj4gPiA+IAo+ID4g PiBTaW5jZSB2ZXJzaW9uIDIuMCwgUlNEUHMgc2hvdWxkIGFkZCBhbiBleHRlbmRlZCBjaGVja3N1 bSwgYSBjb21wbGV0ZSB0YWJsZQo+ID4gPiBsZW5ndGggYW5kIGEgdmVyc2lvbiBmaWVsZCB0byB0 aGUgdGFibGUuICAKPiA+IAo+ID4gVGhpcyBwYXRjaCByZS1pbXBsZW1lbnRzIHdoYXQgYXJtL3Zp cnQgYm9hcmQgYWxyZWFkeSBkb2VzCj4gPiBhbmQgZml4ZXMgY2hlY2tzdW0gYnVnIGluIHRoZSBs YXRlciBhbmQgYXQgdGhlIHNhbWUgdGltZQo+ID4gd2l0aG91dCBhIHVzZXIgKHdpdGhpbiB0aGUg cGF0Y2gpLgo+ID4gCj4gPiBJJ2Qgc3VnZ2VzdCByZWRvIGl0IGEgd2F5IHNpbWlsYXIgdG8gRkFE VCByZWZhY3RvcmluZwo+ID4gICBwYXRjaCAxOiBmaXggY2hlY2tzdW0gYnVnIGluIHZpcnQvYXJt Cj4gPiAgIHBhdGNoIDI6IHVwZGF0ZSByZWZlcmVuY2UgdGFibGVzIGluIHRlc3QKPiA+ICAgcGF0 Y2ggMzogaW50cm9kdWNlIEFjcGlSc2RwRGF0YSBzaW1pbGFyIHRvIGNvbW1pdCA5MzdkMWI1ODcK PiA+ICAgICAgICAgICAgICAoYm90aCBhcm0gYW5kIHg4Nikgd2ljaCBzdG9yZXMgYWxsIGRhdGEg aW4gaG9zIGJ5dGUgb3JkZXIKPiA+ICAgcGF0Y2ggNDogY29udmVydCBhcm0ncyBpbXBsLiB0byBi dWlsZF9hcHBlbmRfaW50X25vcHJlZml4KCkgQVBJIChjb21taXQgNWQ3YTMzNGY3KQo+ID4gICAg ICAgICAgICAuLi4gbW92ZSBvdXQgdG8gYW1sLWJ1aWxkLmMKPiA+ICAgcGF0Y2ggNTogcmV1c2Ug Z2VuZXJhbGl6ZWQgYXJtJ3MgYnVpbGRfcnNkcCgpIGZvciB4ODYsIGRyb3BwaW5nIHg4NiBzcGVj aWZpYyBvbmUKPiA+ICAgICAgIGFtZW5kaW5nIGl0IHRvIGdlbmVyYXRlIHJldjEgdmFyaWFudCBk ZWZpbmVkIGJ5IHJldmlzaW9uIGluIEFjcGlSc2RwRGF0YQo+ID4gICAgICAgKGNvbW1pdCBkZDFi MjAzN2EpCj4gPiAKPiA+ICAgJ21ha2UgY2hlY2sgVj0xJyBzaG91bGRuJ3Qgb2JzZXJ2ZSBhbnkg QUNQSSB0YWJsZXMgY2hhbmdlcyBhZnRlciBwYXRjaCAyLiAgCj4gCj4gQW5kIHlvdXIgbmV4dCBz dWdnZXN0aW9uIGlzIHRvIGFkZCBwYXRjaCA2LiAgSSBndWVzcyBpdCdzIGRvYWJsZSBidXQKPiB0 aGlzIHdpbGwgbWFrZSBhIHNpbmdsZSBwYXRjaCBhIDYgcGF0Y2ggc2VyaWVzLiBBdCB0aGlzIHJh dGUgdGhpcyBzZXJpZXMKPiB3aWxsIGJlIGF0IDIwMCBwYXRjaGVzIGVhc2lseS4KPiAKPiBBdXRv bWF0ZWQgY2hlY2tzIGFyZSBjb29sIGJ1dCBoZXkgaXQncyBlYXN5IHRvIHNlZSB3aGF0IGNoYW5n ZWQgaW4gYQo+IGRpc2Fzc2VtYmxlZCB0YWJsZSwgYW5kIHdlIGRvIG5vdCB1cGRhdGUgdGhlbSBi bGluZGx5LiBTbyBqdXN0IG5vdGUgaW4KPiB0aGUgY29tbWVudCB0aGF0IHRoZXJlJ3MgYSB0YWJs ZSBjaGFuZ2UgZm9yIEFSTSBhbmQgZXhwZWN0ZWQgZmlsZXMgbmVlZAo+IHRvIGJlIHVwZGF0ZWQg YW5kIHdlIHNob3VsZCBiZSBmaW5lIElNSE8uClBvaW50IHdhcyB0byBtb3ZlIHBhdGNoZXMgdGhh dCBjaGFuZ2UgdGFibGVzIGNvbnRlbnQgZmlyc3QsCndoZXJlIHdlIHdvdWxkIHBheSBleHRyYSBh dHRlbnRpb25zIHRvIGNoYW5nZXMgaW4gdGFibGVzIGFuZAp0aGVuIHJlZmFjdG9yaW5nIHdoaWNo IHNob3VsZG4ndCBjYXVzZSBhbnkgY2hhbmdlcyB3aWxsIGJlIG1vc3RseQphdXRvbWF0aWMgIChh dCBsZWFzdCBhdCB0aGF0IHBvaW50IHdlIHdvbid0IGhhdmUgdG8gd29ycnkgYWJvdXQgCnRhYmxl cyBjb3JyZWN0bmVzcykKCgo+ID4gPiBTaWduZWQtb2ZmLWJ5OiBTYW11ZWwgT3J0aXogPHNhbWVv QGxpbnV4LmludGVsLmNvbT4KPiA+ID4gLS0tCj4gPiA+ICBpbmNsdWRlL2h3L2FjcGkvYW1sLWJ1 aWxkLmggfCAgMyArKysKPiA+ID4gIGh3L2FjcGkvYW1sLWJ1aWxkLmMgICAgICAgICB8IDM3ICsr KysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysKPiA+ID4gIDIgZmlsZXMgY2hhbmdl ZCwgNDAgaW5zZXJ0aW9ucygrKQo+ID4gPiAKPiA+ID4gZGlmZiAtLWdpdCBhL2luY2x1ZGUvaHcv YWNwaS9hbWwtYnVpbGQuaCBiL2luY2x1ZGUvaHcvYWNwaS9hbWwtYnVpbGQuaAo+ID4gPiBpbmRl eCBjOWJjYjMyZDgxLi4zNTgwZDBjZTkwIDEwMDY0NAo+ID4gPiAtLS0gYS9pbmNsdWRlL2h3L2Fj cGkvYW1sLWJ1aWxkLmgKPiA+ID4gKysrIGIvaW5jbHVkZS9ody9hY3BpL2FtbC1idWlsZC5oCj4g PiA+IEBAIC0zOTMsNiArMzkzLDkgQEAgdm9pZAo+ID4gPiAgYnVpbGRfcnNkcChHQXJyYXkgKnRh YmxlX2RhdGEsCj4gPiA+ICAgICAgICAgICAgIEJJT1NMaW5rZXIgKmxpbmtlciwgdW5zaWduZWQg cnNkdF90Ymxfb2Zmc2V0KTsKPiA+ID4gIHZvaWQKPiA+ID4gK2J1aWxkX3JzZHBfeHNkdChHQXJy YXkgKnRhYmxlX2RhdGEsCj4gPiA+ICsgICAgICAgICAgICAgICAgQklPU0xpbmtlciAqbGlua2Vy LCB1bnNpZ25lZCB4c2R0X3RibF9vZmZzZXQpOwo+ID4gPiArdm9pZAo+ID4gPiAgYnVpbGRfcnNk dChHQXJyYXkgKnRhYmxlX2RhdGEsIEJJT1NMaW5rZXIgKmxpbmtlciwgR0FycmF5ICp0YWJsZV9v ZmZzZXRzLAo+ID4gPiAgICAgICAgICAgICBjb25zdCBjaGFyICpvZW1faWQsIGNvbnN0IGNoYXIg Km9lbV90YWJsZV9pZCk7Cj4gPiA+ICB2b2lkCj4gPiA+IGRpZmYgLS1naXQgYS9ody9hY3BpL2Ft bC1idWlsZC5jIGIvaHcvYWNwaS9hbWwtYnVpbGQuYwo+ID4gPiBpbmRleCA1MWI2MDg0MzJmLi5h MDMwZDQwNjc0IDEwMDY0NAo+ID4gPiAtLS0gYS9ody9hY3BpL2FtbC1idWlsZC5jCj4gPiA+ICsr KyBiL2h3L2FjcGkvYW1sLWJ1aWxkLmMKPiA+ID4gQEAgLTE2NTEsNiArMTY1MSw0MyBAQCBidWls ZF94c2R0KEdBcnJheSAqdGFibGVfZGF0YSwgQklPU0xpbmtlciAqbGlua2VyLCBHQXJyYXkgKnRh YmxlX29mZnNldHMsCj4gPiA+ICAgICAgICAgICAgICAgICAgICh2b2lkICopeHNkdCwgIlhTRFQi LCB4c2R0X2xlbiwgMSwgb2VtX2lkLCBvZW1fdGFibGVfaWQpOwo+ID4gPiAgfQo+ID4gPiAgCj4g PiA+ICsvKiBSU0RQIHBvaW50aW5nIGF0IGFuIFhTRFQgKi8KPiA+ID4gK3ZvaWQKPiA+ID4gK2J1 aWxkX3JzZHBfeHNkdChHQXJyYXkgKnJzZHBfdGFibGUsCj4gPiA+ICsgICAgICAgICAgICAgICAg QklPU0xpbmtlciAqbGlua2VyLCB1bnNpZ25lZCB4c2R0X3RibF9vZmZzZXQpCj4gPiA+ICt7Cj4g PiA+ICsgICAgQWNwaVJzZHBEZXNjcmlwdG9yICpyc2RwID0gYWNwaV9kYXRhX3B1c2gocnNkcF90 YWJsZSwgc2l6ZW9mICpyc2RwKTsKPiA+ID4gKyAgICB1bnNpZ25lZCB4c2R0X3BhX3NpemUgPSBz aXplb2YocnNkcC0+eHNkdF9waHlzaWNhbF9hZGRyZXNzKTsKPiA+ID4gKyAgICB1bnNpZ25lZCB4 c2R0X3BhX29mZnNldCA9Cj4gPiA+ICsgICAgICAgIChjaGFyICopJnJzZHAtPnhzZHRfcGh5c2lj YWxfYWRkcmVzcyAtIHJzZHBfdGFibGUtPmRhdGE7Cj4gPiA+ICsgICAgdW5zaWduZWQgeHNkdF9v ZmZzZXQgPQo+ID4gPiArICAgICAgICAoY2hhciAqKSZyc2RwLT5sZW5ndGggLSByc2RwX3RhYmxl LT5kYXRhOyAgCj4gCj4gVGhlcmUncyBhIGNsZWFuZXIgd2F5IHRvIGdldCBhdCB0aGUgb2Zmc2V0 cyB0aGFuIHBvaW50ZXIgbWF0aDoKPiAxLiBzYXZlIHJzZHBfdGFibGUgbGVuZ3RoIGJlZm9yZSB5 b3UgcHVzaAo+IDIuIGFkZCBvZmZzZXRfb2YgZm9yIGZpZWxkcwo+IAo+IElmIHN3aXRjaGluZyB0 byBidWlsZF9hcHBlbmRfaW50X25vcHJlZml4IHRoZW4gaXQncyBldmVuCj4gZWFzaWVyIC0ganVz dCBzYXZlIGxlbmd0aCBiZWZvcmUgeW91IGFwcGVuZCB0aGUgaW50Cj4geW91IGludGVuZCB0byBw YXRjaC4KPiAKPiAKPiA+ID4gKwo+ID4gPiArICAgIGJpb3NfbGlua2VyX2xvYWRlcl9hbGxvYyhs aW5rZXIsIEFDUElfQlVJTERfUlNEUF9GSUxFLCByc2RwX3RhYmxlLCAxNiwKPiA+ID4gKyAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgdHJ1ZSAvKiBmc2VnIG1lbW9yeSAqLyk7Cj4gPiA+ICsK PiA+ID4gKyAgICBtZW1jcHkoJnJzZHAtPnNpZ25hdHVyZSwgIlJTRCBQVFIgIiwgOCk7Cj4gPiA+ ICsgICAgbWVtY3B5KHJzZHAtPm9lbV9pZCwgQUNQSV9CVUlMRF9BUFBOQU1FNiwgNik7Cj4gPiA+ ICsgICAgcnNkcC0+bGVuZ3RoID0gY3B1X3RvX2xlMzIoc2l6ZW9mKCpyc2RwKSk7Cj4gPiA+ICsg ICAgLyogdmVyc2lvbiAyLCB3ZSB3aWxsIHVzZSB0aGUgWFNEVCBwb2ludGVyICovCj4gPiA+ICsg ICAgcnNkcC0+cmV2aXNpb24gPSAweDAyOwo+ID4gPiArCj4gPiA+ICsgICAgLyogQWRkcmVzcyB0 byBiZSBmaWxsZWQgYnkgR3Vlc3QgbGlua2VyICovCj4gPiA+ICsgICAgYmlvc19saW5rZXJfbG9h ZGVyX2FkZF9wb2ludGVyKGxpbmtlciwKPiA+ID4gKyAgICAgICAgQUNQSV9CVUlMRF9SU0RQX0ZJ TEUsIHhzZHRfcGFfb2Zmc2V0LCB4c2R0X3BhX3NpemUsCj4gPiA+ICsgICAgICAgIEFDUElfQlVJ TERfVEFCTEVfRklMRSwgeHNkdF90Ymxfb2Zmc2V0KTsKPiA+ID4gKwo+ID4gPiArICAgIC8qIExl Z2FjeSBjaGVja3N1bSB0byBiZSBmaWxsZWQgYnkgR3Vlc3QgbGlua2VyICovCj4gPiA+ICsgICAg Ymlvc19saW5rZXJfbG9hZGVyX2FkZF9jaGVja3N1bShsaW5rZXIsIEFDUElfQlVJTERfUlNEUF9G SUxFLAo+ID4gPiArICAgICAgICAoY2hhciAqKXJzZHAgLSByc2RwX3RhYmxlLT5kYXRhLCB4c2R0 X29mZnNldCwKPiA+ID4gKyAgICAgICAgKGNoYXIgKikmcnNkcC0+Y2hlY2tzdW0gLSByc2RwX3Rh YmxlLT5kYXRhKTsKPiA+ID4gKwo+ID4gPiArICAgIC8qIEV4dGVuZGVkIGNoZWNrc3VtIHRvIGJl IGZpbGxlZCBieSBHdWVzdCBsaW5rZXIgKi8KPiA+ID4gKyAgICBiaW9zX2xpbmtlcl9sb2FkZXJf YWRkX2NoZWNrc3VtKGxpbmtlciwgQUNQSV9CVUlMRF9SU0RQX0ZJTEUsCj4gPiA+ICsgICAgICAg IChjaGFyICopcnNkcCAtIHJzZHBfdGFibGUtPmRhdGEsIHNpemVvZiAqcnNkcCwKPiA+ID4gKyAg ICAgICAgKGNoYXIgKikmcnNkcC0+ZXh0ZW5kZWRfY2hlY2tzdW0gLSByc2RwX3RhYmxlLT5kYXRh KTsKPiA+ID4gK30KPiA+ID4gKwo+ID4gPiAgdm9pZCBidWlsZF9zcmF0X21lbW9yeShBY3BpU3Jh dE1lbW9yeUFmZmluaXR5ICpudW1hbWVtLCB1aW50NjRfdCBiYXNlLAo+ID4gPiAgICAgICAgICAg ICAgICAgICAgICAgICB1aW50NjRfdCBsZW4sIGludCBub2RlLCBNZW1vcnlBZmZpbml0eUZsYWdz IGZsYWdzKQo+ID4gPiAgeyAgCgoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX18KWGVuLWRldmVsIG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVucHJv amVjdC5vcmcKaHR0cHM6Ly9saXN0cy54ZW5wcm9qZWN0Lm9yZy9tYWlsbWFuL2xpc3RpbmZvL3hl bi1kZXZlbA== From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55582) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gP1Jn-0007jH-NS for qemu-devel@nongnu.org; Tue, 20 Nov 2018 03:23:36 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gP1Jm-0000PX-Pa for qemu-devel@nongnu.org; Tue, 20 Nov 2018 03:23:35 -0500 Date: Tue, 20 Nov 2018 09:23:18 +0100 From: Igor Mammedov Message-ID: <20181120092318.48f69fa4@redhat.com> In-Reply-To: <20181119131625-mutt-send-email-mst@kernel.org> References: <20181105014047.26447-1-sameo@linux.intel.com> <20181105014047.26447-6-sameo@linux.intel.com> <20181108151623.4de26ecb@redhat.com> <20181119131625-mutt-send-email-mst@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 05/24] hw: acpi: Implement XSDT support for RSDP List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Samuel Ortiz , qemu-devel@nongnu.org, Shannon Zhao , Stefano Stabellini , Anthony Perard , Richard Henderson , Marcel Apfelbaum , xen-devel@lists.xenproject.org, Paolo Bonzini , qemu-arm@nongnu.org, Peter Maydell , Eduardo Habkost On Mon, 19 Nov 2018 13:27:14 -0500 "Michael S. Tsirkin" wrote: > On Thu, Nov 08, 2018 at 03:16:23PM +0100, Igor Mammedov wrote: > > On Mon, 5 Nov 2018 02:40:28 +0100 > > Samuel Ortiz wrote: > > > > > XSDT is the 64-bit version of the legacy ACPI RSDT (Root System > > > Description Table). RSDT only allow for 32-bit addressses and have thus > > > been deprecated. Since ACPI version 2.0, RSDPs should point at XSDTs and > > > no longer RSDTs, although RSDTs are still supported for backward > > > compatibility. > > > > > > Since version 2.0, RSDPs should add an extended checksum, a complete table > > > length and a version field to the table. > > > > This patch re-implements what arm/virt board already does > > and fixes checksum bug in the later and at the same time > > without a user (within the patch). > > > > I'd suggest redo it a way similar to FADT refactoring > > patch 1: fix checksum bug in virt/arm > > patch 2: update reference tables in test > > patch 3: introduce AcpiRsdpData similar to commit 937d1b587 > > (both arm and x86) wich stores all data in hos byte order > > patch 4: convert arm's impl. to build_append_int_noprefix() API (commit 5d7a334f7) > > ... move out to aml-build.c > > patch 5: reuse generalized arm's build_rsdp() for x86, dropping x86 specific one > > amending it to generate rev1 variant defined by revision in AcpiRsdpData > > (commit dd1b2037a) > > > > 'make check V=1' shouldn't observe any ACPI tables changes after patch 2. > > And your next suggestion is to add patch 6. I guess it's doable but > this will make a single patch a 6 patch series. At this rate this series > will be at 200 patches easily. > > Automated checks are cool but hey it's easy to see what changed in a > disassembled table, and we do not update them blindly. So just note in > the comment that there's a table change for ARM and expected files need > to be updated and we should be fine IMHO. Point was to move patches that change tables content first, where we would pay extra attentions to changes in tables and then refactoring which shouldn't cause any changes will be mostly automatic (at least at that point we won't have to worry about tables correctness) > > > Signed-off-by: Samuel Ortiz > > > --- > > > include/hw/acpi/aml-build.h | 3 +++ > > > hw/acpi/aml-build.c | 37 +++++++++++++++++++++++++++++++++++++ > > > 2 files changed, 40 insertions(+) > > > > > > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h > > > index c9bcb32d81..3580d0ce90 100644 > > > --- a/include/hw/acpi/aml-build.h > > > +++ b/include/hw/acpi/aml-build.h > > > @@ -393,6 +393,9 @@ void > > > build_rsdp(GArray *table_data, > > > BIOSLinker *linker, unsigned rsdt_tbl_offset); > > > void > > > +build_rsdp_xsdt(GArray *table_data, > > > + BIOSLinker *linker, unsigned xsdt_tbl_offset); > > > +void > > > build_rsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets, > > > const char *oem_id, const char *oem_table_id); > > > void > > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > > > index 51b608432f..a030d40674 100644 > > > --- a/hw/acpi/aml-build.c > > > +++ b/hw/acpi/aml-build.c > > > @@ -1651,6 +1651,43 @@ build_xsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets, > > > (void *)xsdt, "XSDT", xsdt_len, 1, oem_id, oem_table_id); > > > } > > > > > > +/* RSDP pointing at an XSDT */ > > > +void > > > +build_rsdp_xsdt(GArray *rsdp_table, > > > + BIOSLinker *linker, unsigned xsdt_tbl_offset) > > > +{ > > > + AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp); > > > + unsigned xsdt_pa_size = sizeof(rsdp->xsdt_physical_address); > > > + unsigned xsdt_pa_offset = > > > + (char *)&rsdp->xsdt_physical_address - rsdp_table->data; > > > + unsigned xsdt_offset = > > > + (char *)&rsdp->length - rsdp_table->data; > > There's a cleaner way to get at the offsets than pointer math: > 1. save rsdp_table length before you push > 2. add offset_of for fields > > If switching to build_append_int_noprefix then it's even > easier - just save length before you append the int > you intend to patch. > > > > > + > > > + bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, rsdp_table, 16, > > > + true /* fseg memory */); > > > + > > > + memcpy(&rsdp->signature, "RSD PTR ", 8); > > > + memcpy(rsdp->oem_id, ACPI_BUILD_APPNAME6, 6); > > > + rsdp->length = cpu_to_le32(sizeof(*rsdp)); > > > + /* version 2, we will use the XSDT pointer */ > > > + rsdp->revision = 0x02; > > > + > > > + /* Address to be filled by Guest linker */ > > > + bios_linker_loader_add_pointer(linker, > > > + ACPI_BUILD_RSDP_FILE, xsdt_pa_offset, xsdt_pa_size, > > > + ACPI_BUILD_TABLE_FILE, xsdt_tbl_offset); > > > + > > > + /* Legacy checksum to be filled by Guest linker */ > > > + bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE, > > > + (char *)rsdp - rsdp_table->data, xsdt_offset, > > > + (char *)&rsdp->checksum - rsdp_table->data); > > > + > > > + /* Extended checksum to be filled by Guest linker */ > > > + bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE, > > > + (char *)rsdp - rsdp_table->data, sizeof *rsdp, > > > + (char *)&rsdp->extended_checksum - rsdp_table->data); > > > +} > > > + > > > void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base, > > > uint64_t len, int node, MemoryAffinityFlags flags) > > > {