From mboxrd@z Thu Jan 1 00:00:00 1970 Received: by 2002:a5d:6089:0:0:0:0:0 with SMTP id w9csp1190881wrt; Mon, 19 Nov 2018 10:27:29 -0800 (PST) X-Google-Smtp-Source: AJdET5cSPwY6KGe4/c+crsiUY/Sjr8X33jg7NMvoBGMUZwM+zdSFrkeSu5qoCH3cvgTAbkaKQOnI X-Received: by 2002:a81:7307:: with SMTP id o7-v6mr21949716ywc.183.1542652049440; Mon, 19 Nov 2018 10:27:29 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1542652049; cv=none; d=google.com; s=arc-20160816; b=dcC+gL1kt+H8/WfZ/6uIDZ86TckxQ83aer6bSRqjkerD7VDgiR9lonjmpR0AZsTBKf 7kcwisM0JZxzSHYH1j0shWD3xH3z7rXU0R3m+CA58yBc+kvdS3VqaHKupxZVrmKfIRSR DmGrgCpBjDis3qPRLAsHgaYIAdmsZPeQ4iNuUQAGydo/NkCxbmd9WgFjrXr+a1t8gX3L aBq9ndi9VV/O5pGwLv4y4v9j5xLM+hKZXNmXc+ULlevqVpzX3KmuMsjaLkAf3cMLB6gx AlLynV72pFNDuyEFGWkoulSRoHvbqiCq3rJZcb/OOvFIZNus/09AkxIPx5SVwvLSJA2y qMGg== 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:in-reply-to :content-disposition:mime-version:references:message-id:to:from:date; bh=cKCnRlrupFI/j5QcjMae0iF73H1P0f6tJva5O3tO/u4=; b=c/kzOZsJXAXCb3W9dshnC1Ftp2Xlwsjiqhu9sj+z487cc54xuJdDiVL0wo+YA7Lelc bS15nkDKQcsXkKdl0gSUK2T0uSJWywfJNktprizLkCXaq0E82o4vYuF46GIPV1/iiR7P Itj2SCMKEwxRHNndzvyvti0Jzoc5OGJQcvrToxWiOBKubJIpqVjB2OaWjHuJG4FqOGg9 rL9y5l6BTHF/Jv9NFnhxdYaUFJS2CGthhyzqEYmkOAQ0QiSScPzOrUUYXxEcsdwzY4qF sBVNjKO/cvglhX+X2N7gxTNtlppfRjd29oya9LuUHqMFSakNZM60MmklIiHLuLXMn9gw fg5Q== 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 s203si8027165yws.274.2018.11.19.10.27.29 for (version=TLS1 cipher=AES128-SHA bits=128/128); Mon, 19 Nov 2018 10:27:29 -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]:58535 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gOoGe-0004C2-UT for alex.bennee@linaro.org; Mon, 19 Nov 2018 13:27:28 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34520) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gOoGX-0004Bx-DC for qemu-arm@nongnu.org; Mon, 19 Nov 2018 13:27:22 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gOoGU-0006dF-5t for qemu-arm@nongnu.org; Mon, 19 Nov 2018 13:27:21 -0500 Received: from mx1.redhat.com ([209.132.183.28]:56458) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gOoGT-0006b3-QW; Mon, 19 Nov 2018 13:27:18 -0500 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D5F0730832D3; Mon, 19 Nov 2018 18:27:16 +0000 (UTC) Received: from redhat.com (ovpn-122-92.rdu2.redhat.com [10.10.122.92]) by smtp.corp.redhat.com (Postfix) with SMTP id 29DA9608E7; Mon, 19 Nov 2018 18:27:15 +0000 (UTC) Date: Mon, 19 Nov 2018 13:27:14 -0500 From: "Michael S. Tsirkin" To: Igor Mammedov Message-ID: <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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20181108151623.4de26ecb@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.44]); Mon, 19 Nov 2018 18:27:17 +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: C9jTNG+po0Ir 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. > > 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: "Michael S. Tsirkin" Subject: Re: [PATCH v5 05/24] hw: acpi: Implement XSDT support for RSDP Date: Mon, 19 Nov 2018 13:27:14 -0500 Message-ID: <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> 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 1gOoGV-0000iX-Fi for xen-devel@lists.xenproject.org; Mon, 19 Nov 2018 18:27:19 +0000 Content-Disposition: inline In-Reply-To: <20181108151623.4de26ecb@redhat.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" To: Igor Mammedov 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 T24gVGh1LCBOb3YgMDgsIDIwMTggYXQgMDM6MTY6MjNQTSArMDEwMCwgSWdvciBNYW1tZWRvdiB3 cm90ZToKPiBPbiBNb24sICA1IE5vdiAyMDE4IDAyOjQwOjI4ICswMTAwCj4gU2FtdWVsIE9ydGl6 IDxzYW1lb0BsaW51eC5pbnRlbC5jb20+IHdyb3RlOgo+IAo+ID4gWFNEVCBpcyB0aGUgNjQtYml0 IHZlcnNpb24gb2YgdGhlIGxlZ2FjeSBBQ1BJIFJTRFQgKFJvb3QgU3lzdGVtCj4gPiBEZXNjcmlw dGlvbiBUYWJsZSkuIFJTRFQgb25seSBhbGxvdyBmb3IgMzItYml0IGFkZHJlc3NzZXMgYW5kIGhh dmUgdGh1cwo+ID4gYmVlbiBkZXByZWNhdGVkLiBTaW5jZSBBQ1BJIHZlcnNpb24gMi4wLCBSU0RQ cyBzaG91bGQgcG9pbnQgYXQgWFNEVHMgYW5kCj4gPiBubyBsb25nZXIgUlNEVHMsIGFsdGhvdWdo IFJTRFRzIGFyZSBzdGlsbCBzdXBwb3J0ZWQgZm9yIGJhY2t3YXJkCj4gPiBjb21wYXRpYmlsaXR5 Lgo+ID4gCj4gPiBTaW5jZSB2ZXJzaW9uIDIuMCwgUlNEUHMgc2hvdWxkIGFkZCBhbiBleHRlbmRl ZCBjaGVja3N1bSwgYSBjb21wbGV0ZSB0YWJsZQo+ID4gbGVuZ3RoIGFuZCBhIHZlcnNpb24gZmll bGQgdG8gdGhlIHRhYmxlLgo+IAo+IFRoaXMgcGF0Y2ggcmUtaW1wbGVtZW50cyB3aGF0IGFybS92 aXJ0IGJvYXJkIGFscmVhZHkgZG9lcwo+IGFuZCBmaXhlcyBjaGVja3N1bSBidWcgaW4gdGhlIGxh dGVyIGFuZCBhdCB0aGUgc2FtZSB0aW1lCj4gd2l0aG91dCBhIHVzZXIgKHdpdGhpbiB0aGUgcGF0 Y2gpLgo+IAo+IEknZCBzdWdnZXN0IHJlZG8gaXQgYSB3YXkgc2ltaWxhciB0byBGQURUIHJlZmFj dG9yaW5nCj4gICBwYXRjaCAxOiBmaXggY2hlY2tzdW0gYnVnIGluIHZpcnQvYXJtCj4gICBwYXRj aCAyOiB1cGRhdGUgcmVmZXJlbmNlIHRhYmxlcyBpbiB0ZXN0Cj4gICBwYXRjaCAzOiBpbnRyb2R1 Y2UgQWNwaVJzZHBEYXRhIHNpbWlsYXIgdG8gY29tbWl0IDkzN2QxYjU4Nwo+ICAgICAgICAgICAg ICAoYm90aCBhcm0gYW5kIHg4Nikgd2ljaCBzdG9yZXMgYWxsIGRhdGEgaW4gaG9zIGJ5dGUgb3Jk ZXIKPiAgIHBhdGNoIDQ6IGNvbnZlcnQgYXJtJ3MgaW1wbC4gdG8gYnVpbGRfYXBwZW5kX2ludF9u b3ByZWZpeCgpIEFQSSAoY29tbWl0IDVkN2EzMzRmNykKPiAgICAgICAgICAgIC4uLiBtb3ZlIG91 dCB0byBhbWwtYnVpbGQuYwo+ICAgcGF0Y2ggNTogcmV1c2UgZ2VuZXJhbGl6ZWQgYXJtJ3MgYnVp bGRfcnNkcCgpIGZvciB4ODYsIGRyb3BwaW5nIHg4NiBzcGVjaWZpYyBvbmUKPiAgICAgICBhbWVu ZGluZyBpdCB0byBnZW5lcmF0ZSByZXYxIHZhcmlhbnQgZGVmaW5lZCBieSByZXZpc2lvbiBpbiBB Y3BpUnNkcERhdGEKPiAgICAgICAoY29tbWl0IGRkMWIyMDM3YSkKPiAKPiAgICdtYWtlIGNoZWNr IFY9MScgc2hvdWxkbid0IG9ic2VydmUgYW55IEFDUEkgdGFibGVzIGNoYW5nZXMgYWZ0ZXIgcGF0 Y2ggMi4KCkFuZCB5b3VyIG5leHQgc3VnZ2VzdGlvbiBpcyB0byBhZGQgcGF0Y2ggNi4gIEkgZ3Vl c3MgaXQncyBkb2FibGUgYnV0CnRoaXMgd2lsbCBtYWtlIGEgc2luZ2xlIHBhdGNoIGEgNiBwYXRj aCBzZXJpZXMuIEF0IHRoaXMgcmF0ZSB0aGlzIHNlcmllcwp3aWxsIGJlIGF0IDIwMCBwYXRjaGVz IGVhc2lseS4KCkF1dG9tYXRlZCBjaGVja3MgYXJlIGNvb2wgYnV0IGhleSBpdCdzIGVhc3kgdG8g c2VlIHdoYXQgY2hhbmdlZCBpbiBhCmRpc2Fzc2VtYmxlZCB0YWJsZSwgYW5kIHdlIGRvIG5vdCB1 cGRhdGUgdGhlbSBibGluZGx5LiBTbyBqdXN0IG5vdGUgaW4KdGhlIGNvbW1lbnQgdGhhdCB0aGVy ZSdzIGEgdGFibGUgY2hhbmdlIGZvciBBUk0gYW5kIGV4cGVjdGVkIGZpbGVzIG5lZWQKdG8gYmUg dXBkYXRlZCBhbmQgd2Ugc2hvdWxkIGJlIGZpbmUgSU1ITy4KCj4gPiBTaWduZWQtb2ZmLWJ5OiBT YW11ZWwgT3J0aXogPHNhbWVvQGxpbnV4LmludGVsLmNvbT4KPiA+IC0tLQo+ID4gIGluY2x1ZGUv aHcvYWNwaS9hbWwtYnVpbGQuaCB8ICAzICsrKwo+ID4gIGh3L2FjcGkvYW1sLWJ1aWxkLmMgICAg ICAgICB8IDM3ICsrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysKPiA+ICAyIGZp bGVzIGNoYW5nZWQsIDQwIGluc2VydGlvbnMoKykKPiA+IAo+ID4gZGlmZiAtLWdpdCBhL2luY2x1 ZGUvaHcvYWNwaS9hbWwtYnVpbGQuaCBiL2luY2x1ZGUvaHcvYWNwaS9hbWwtYnVpbGQuaAo+ID4g aW5kZXggYzliY2IzMmQ4MS4uMzU4MGQwY2U5MCAxMDA2NDQKPiA+IC0tLSBhL2luY2x1ZGUvaHcv YWNwaS9hbWwtYnVpbGQuaAo+ID4gKysrIGIvaW5jbHVkZS9ody9hY3BpL2FtbC1idWlsZC5oCj4g PiBAQCAtMzkzLDYgKzM5Myw5IEBAIHZvaWQKPiA+ICBidWlsZF9yc2RwKEdBcnJheSAqdGFibGVf ZGF0YSwKPiA+ICAgICAgICAgICAgIEJJT1NMaW5rZXIgKmxpbmtlciwgdW5zaWduZWQgcnNkdF90 Ymxfb2Zmc2V0KTsKPiA+ICB2b2lkCj4gPiArYnVpbGRfcnNkcF94c2R0KEdBcnJheSAqdGFibGVf ZGF0YSwKPiA+ICsgICAgICAgICAgICAgICAgQklPU0xpbmtlciAqbGlua2VyLCB1bnNpZ25lZCB4 c2R0X3RibF9vZmZzZXQpOwo+ID4gK3ZvaWQKPiA+ICBidWlsZF9yc2R0KEdBcnJheSAqdGFibGVf ZGF0YSwgQklPU0xpbmtlciAqbGlua2VyLCBHQXJyYXkgKnRhYmxlX29mZnNldHMsCj4gPiAgICAg ICAgICAgICBjb25zdCBjaGFyICpvZW1faWQsIGNvbnN0IGNoYXIgKm9lbV90YWJsZV9pZCk7Cj4g PiAgdm9pZAo+ID4gZGlmZiAtLWdpdCBhL2h3L2FjcGkvYW1sLWJ1aWxkLmMgYi9ody9hY3BpL2Ft bC1idWlsZC5jCj4gPiBpbmRleCA1MWI2MDg0MzJmLi5hMDMwZDQwNjc0IDEwMDY0NAo+ID4gLS0t IGEvaHcvYWNwaS9hbWwtYnVpbGQuYwo+ID4gKysrIGIvaHcvYWNwaS9hbWwtYnVpbGQuYwo+ID4g QEAgLTE2NTEsNiArMTY1MSw0MyBAQCBidWlsZF94c2R0KEdBcnJheSAqdGFibGVfZGF0YSwgQklP U0xpbmtlciAqbGlua2VyLCBHQXJyYXkgKnRhYmxlX29mZnNldHMsCj4gPiAgICAgICAgICAgICAg ICAgICAodm9pZCAqKXhzZHQsICJYU0RUIiwgeHNkdF9sZW4sIDEsIG9lbV9pZCwgb2VtX3RhYmxl X2lkKTsKPiA+ICB9Cj4gPiAgCj4gPiArLyogUlNEUCBwb2ludGluZyBhdCBhbiBYU0RUICovCj4g PiArdm9pZAo+ID4gK2J1aWxkX3JzZHBfeHNkdChHQXJyYXkgKnJzZHBfdGFibGUsCj4gPiArICAg ICAgICAgICAgICAgIEJJT1NMaW5rZXIgKmxpbmtlciwgdW5zaWduZWQgeHNkdF90Ymxfb2Zmc2V0 KQo+ID4gK3sKPiA+ICsgICAgQWNwaVJzZHBEZXNjcmlwdG9yICpyc2RwID0gYWNwaV9kYXRhX3B1 c2gocnNkcF90YWJsZSwgc2l6ZW9mICpyc2RwKTsKPiA+ICsgICAgdW5zaWduZWQgeHNkdF9wYV9z aXplID0gc2l6ZW9mKHJzZHAtPnhzZHRfcGh5c2ljYWxfYWRkcmVzcyk7Cj4gPiArICAgIHVuc2ln bmVkIHhzZHRfcGFfb2Zmc2V0ID0KPiA+ICsgICAgICAgIChjaGFyICopJnJzZHAtPnhzZHRfcGh5 c2ljYWxfYWRkcmVzcyAtIHJzZHBfdGFibGUtPmRhdGE7Cj4gPiArICAgIHVuc2lnbmVkIHhzZHRf b2Zmc2V0ID0KPiA+ICsgICAgICAgIChjaGFyICopJnJzZHAtPmxlbmd0aCAtIHJzZHBfdGFibGUt PmRhdGE7CgpUaGVyZSdzIGEgY2xlYW5lciB3YXkgdG8gZ2V0IGF0IHRoZSBvZmZzZXRzIHRoYW4g cG9pbnRlciBtYXRoOgoxLiBzYXZlIHJzZHBfdGFibGUgbGVuZ3RoIGJlZm9yZSB5b3UgcHVzaAoy LiBhZGQgb2Zmc2V0X29mIGZvciBmaWVsZHMKCklmIHN3aXRjaGluZyB0byBidWlsZF9hcHBlbmRf aW50X25vcHJlZml4IHRoZW4gaXQncyBldmVuCmVhc2llciAtIGp1c3Qgc2F2ZSBsZW5ndGggYmVm b3JlIHlvdSBhcHBlbmQgdGhlIGludAp5b3UgaW50ZW5kIHRvIHBhdGNoLgoKCj4gPiArCj4gPiAr ICAgIGJpb3NfbGlua2VyX2xvYWRlcl9hbGxvYyhsaW5rZXIsIEFDUElfQlVJTERfUlNEUF9GSUxF LCByc2RwX3RhYmxlLCAxNiwKPiA+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgIHRydWUg LyogZnNlZyBtZW1vcnkgKi8pOwo+ID4gKwo+ID4gKyAgICBtZW1jcHkoJnJzZHAtPnNpZ25hdHVy ZSwgIlJTRCBQVFIgIiwgOCk7Cj4gPiArICAgIG1lbWNweShyc2RwLT5vZW1faWQsIEFDUElfQlVJ TERfQVBQTkFNRTYsIDYpOwo+ID4gKyAgICByc2RwLT5sZW5ndGggPSBjcHVfdG9fbGUzMihzaXpl b2YoKnJzZHApKTsKPiA+ICsgICAgLyogdmVyc2lvbiAyLCB3ZSB3aWxsIHVzZSB0aGUgWFNEVCBw b2ludGVyICovCj4gPiArICAgIHJzZHAtPnJldmlzaW9uID0gMHgwMjsKPiA+ICsKPiA+ICsgICAg LyogQWRkcmVzcyB0byBiZSBmaWxsZWQgYnkgR3Vlc3QgbGlua2VyICovCj4gPiArICAgIGJpb3Nf bGlua2VyX2xvYWRlcl9hZGRfcG9pbnRlcihsaW5rZXIsCj4gPiArICAgICAgICBBQ1BJX0JVSUxE X1JTRFBfRklMRSwgeHNkdF9wYV9vZmZzZXQsIHhzZHRfcGFfc2l6ZSwKPiA+ICsgICAgICAgIEFD UElfQlVJTERfVEFCTEVfRklMRSwgeHNkdF90Ymxfb2Zmc2V0KTsKPiA+ICsKPiA+ICsgICAgLyog TGVnYWN5IGNoZWNrc3VtIHRvIGJlIGZpbGxlZCBieSBHdWVzdCBsaW5rZXIgKi8KPiA+ICsgICAg Ymlvc19saW5rZXJfbG9hZGVyX2FkZF9jaGVja3N1bShsaW5rZXIsIEFDUElfQlVJTERfUlNEUF9G SUxFLAo+ID4gKyAgICAgICAgKGNoYXIgKilyc2RwIC0gcnNkcF90YWJsZS0+ZGF0YSwgeHNkdF9v ZmZzZXQsCj4gPiArICAgICAgICAoY2hhciAqKSZyc2RwLT5jaGVja3N1bSAtIHJzZHBfdGFibGUt PmRhdGEpOwo+ID4gKwo+ID4gKyAgICAvKiBFeHRlbmRlZCBjaGVja3N1bSB0byBiZSBmaWxsZWQg YnkgR3Vlc3QgbGlua2VyICovCj4gPiArICAgIGJpb3NfbGlua2VyX2xvYWRlcl9hZGRfY2hlY2tz dW0obGlua2VyLCBBQ1BJX0JVSUxEX1JTRFBfRklMRSwKPiA+ICsgICAgICAgIChjaGFyICopcnNk cCAtIHJzZHBfdGFibGUtPmRhdGEsIHNpemVvZiAqcnNkcCwKPiA+ICsgICAgICAgIChjaGFyICop JnJzZHAtPmV4dGVuZGVkX2NoZWNrc3VtIC0gcnNkcF90YWJsZS0+ZGF0YSk7Cj4gPiArfQo+ID4g Kwo+ID4gIHZvaWQgYnVpbGRfc3JhdF9tZW1vcnkoQWNwaVNyYXRNZW1vcnlBZmZpbml0eSAqbnVt YW1lbSwgdWludDY0X3QgYmFzZSwKPiA+ICAgICAgICAgICAgICAgICAgICAgICAgIHVpbnQ2NF90 IGxlbiwgaW50IG5vZGUsIE1lbW9yeUFmZmluaXR5RmxhZ3MgZmxhZ3MpCj4gPiAgewoKX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVsIG1haWxp bmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVucHJvamVjdC5vcmcKaHR0cHM6Ly9saXN0cy54ZW5w cm9qZWN0Lm9yZy9tYWlsbWFuL2xpc3RpbmZvL3hlbi1kZXZlbA== From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34539) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gOoGZ-0004CM-G9 for qemu-devel@nongnu.org; Mon, 19 Nov 2018 13:27:24 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gOoGY-0006j1-HH for qemu-devel@nongnu.org; Mon, 19 Nov 2018 13:27:23 -0500 Date: Mon, 19 Nov 2018 13:27:14 -0500 From: "Michael S. Tsirkin" Message-ID: <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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20181108151623.4de26ecb@redhat.com> 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: Igor Mammedov 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 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. > > 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) > > {