From: Laszlo Ersek <lersek@redhat.com>
To: SeaBIOS@seabios.org, qemu-devel@nongnu.org, edk2-devel@lists.01.org
Cc: "Michael S. Tsirkin" <mst@redhat.com>,
Ard Biesheuvel <ard.biesheuvel@linaro.org>,
Ben Warren <ben@skyportsystems.com>,
Dongjiu Geng <gengdongjiu@huawei.com>,
Igor Mammedov <imammedo@redhat.com>,
Shannon Zhao <zhaoshenglong@huawei.com>,
Stefan Berger <stefanb@linux.vnet.ibm.com>,
Xiao Guangrong <guangrong.xiao@linux.intel.com>
Subject: [qemu PATCH 5/7] hw/acpi/vmgenid: ask the fw to alloc VMGENID_GUID_FW_CFG_FILE as NOACPI
Date: Fri, 2 Jun 2017 18:00:04 +0200 [thread overview]
Message-ID: <20170602160006.1748-6-lersek@redhat.com> (raw)
In-Reply-To: <20170602160006.1748-1-lersek@redhat.com>
The "etc/vmgenid_guid" fw_cfg blob is guaranteed not to contain ACPI
tables, so turning off the ACPI SDT header probe in OVMF is the right
thing to do.
SeaBIOS needs a patch for recognizing (and masking out) the
BIOS_LINKER_LOADER_ALLOC_CONTENT_NOACPI bit, but its behavior will not
change.
By setting the BIOS_LINKER_LOADER_ALLOC_CONTENT_NOACPI bit, we can
eliminate the "OVMF SDT Header probe suppressor" -- we can shift the GUID
to offset zero from offset 40 decimal (VMGENID_GUID_OFFSET).
Regarding the allocation zone, we cannot relax that to 64-bit, because the
"VGIA" object, into which the address of "etc/vmgenid_guid" is patched, is
only a DWORD.
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Ben Warren <ben@skyportsystems.com>
Cc: Dongjiu Geng <gengdongjiu@huawei.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Shannon Zhao <zhaoshenglong@huawei.com>
Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
I tested this change extensively,
- by repeating the steps written up in
<http://mid.mail-archive.com/c052d05e-71a5-1a6a-f34f-17d14167c2f6@redhat.com>,
- by doing the same after S3 suspend/resume,
- by verifying firmware logs,
- by directly checking ACPI content and dmesg in a Linux guest.
I know Ben has a pending patch titled "[PATCH] tests: Add unit tests for
the VM Generation ID feature", that one should be adapted as well
(replace VMGENID_GUID_OFFSET with plain 0). I'd be happy to do that.
docs/specs/vmgenid.txt | 49 ++++++++++++++++++++---------------------------
include/hw/acpi/vmgenid.h | 3 ---
hw/acpi/vmgenid.c | 21 ++++----------------
3 files changed, 25 insertions(+), 48 deletions(-)
diff --git a/docs/specs/vmgenid.txt b/docs/specs/vmgenid.txt
index aa9f5186767c..fb9f372edbc8 100644
--- a/docs/specs/vmgenid.txt
+++ b/docs/specs/vmgenid.txt
@@ -63,76 +63,74 @@ Xen) put it in the main descriptor table (Differentiated System Description
Table or DSDT). For ease of debugging and implementation, we have decided to
put it in its own Secondary System Description Table, or SSDT.
The following is a dump of the contents from a running system:
-# iasl -p ./SSDT -d /sys/firmware/acpi/tables/SSDT
+# acpidump -n SSDT -b
+# iasl -d ssdt.dat
Intel ACPI Component Architecture
-ASL+ Optimizing Compiler version 20150717-64
-Copyright (c) 2000 - 2015 Intel Corporation
+ASL+ Optimizing Compiler version 20160527-64
+Copyright (c) 2000 - 2016 Intel Corporation
-Reading ACPI table from file /sys/firmware/acpi/tables/SSDT - Length
-00000198 (0x0000C6)
+Input file ssdt.dat, Length 0xC6 (198) bytes
ACPI: SSDT 0x0000000000000000 0000C6 (v01 BOCHS VMGENID 00000001 BXPC
00000001)
-Acpi table [SSDT] successfully installed and loaded
Pass 1 parse of [SSDT]
Pass 2 parse of [SSDT]
Parsing Deferred Opcodes (Methods/Buffers/Packages/Regions)
Parsing completed
Disassembly completed
-ASL Output: ./SSDT.dsl - 1631 bytes
-# cat SSDT.dsl
+ASL Output: ssdt.dsl - 1559 bytes
+# cat ssdt.dsl
/*
* Intel ACPI Component Architecture
- * AML/ASL+ Disassembler version 20150717-64
- * Copyright (c) 2000 - 2015 Intel Corporation
+ * AML/ASL+ Disassembler version 20160527-64
+ * Copyright (c) 2000 - 2016 Intel Corporation
*
* Disassembling to symbolic ASL+ operators
*
- * Disassembly of /sys/firmware/acpi/tables/SSDT, Sun Feb 5 00:19:37 2017
+ * Disassembly of ssdt.dat, Fri Jun 2 15:29:10 2017
*
* Original Table Header:
* Signature "SSDT"
- * Length 0x000000CA (202)
+ * Length 0x000000C6 (198)
* Revision 0x01
- * Checksum 0x4B
+ * Checksum 0x38
* OEM ID "BOCHS "
* OEM Table ID "VMGENID"
* OEM Revision 0x00000001 (1)
* Compiler ID "BXPC"
* Compiler Version 0x00000001 (1)
*/
-DefinitionBlock ("/sys/firmware/acpi/tables/SSDT.aml", "SSDT", 1, "BOCHS ",
-"VMGENID", 0x00000001)
+DefinitionBlock ("", "SSDT", 1, "BOCHS ", "VMGENID", 0x00000001)
{
- Name (VGIA, 0x07FFF000)
+ Name (VGIA, 0xBFFFF000)
Scope (\_SB)
{
Device (VGEN)
{
Name (_HID, "QEMUVGID") // _HID: Hardware ID
Name (_CID, "VM_Gen_Counter") // _CID: Compatible ID
Name (_DDN, "VM_Gen_Counter") // _DDN: DOS Device Name
Method (_STA, 0, NotSerialized) // _STA: Status
{
Local0 = 0x0F
- If ((VGIA == Zero))
+ If (VGIA == Zero)
{
Local0 = Zero
}
Return (Local0)
}
Method (ADDR, 0, NotSerialized)
{
Local0 = Package (0x02) {}
- Index (Local0, Zero) = (VGIA + 0x28)
- Index (Local0, One) = Zero
+ Local0 [Zero] = VGIA /* \VGIA */
+ Local0 [One] = Zero
Return (Local0)
}
}
}
@@ -197,12 +195,11 @@ GUID values displayed via monitor are shown in big-endian format.
GUID Storage Format:
--------------------
-In order to implement an OVMF "SDT Header Probe Suppressor", the contents of
-the vmgenid_guid fw_cfg blob are not simply a 128-bit GUID. There is also
+The contents of the vmgenid_guid fw_cfg blob are a 128-bit GUID, followed by
significant padding in order to align and fill a memory page, as shown in the
following diagram:
+----------------------------------+
| SSDT with OEM Table ID = VMGENID |
@@ -210,17 +207,13 @@ following diagram:
| ... | TOP OF PAGE
| VGIA dword object ---------------|-----> +---------------------------+
| ... | | fw-allocated array for |
| _STA method referring to VGIA | | "etc/vmgenid_guid" |
| ... | +---------------------------+
-| ADDR method referring to VGIA | | 0: OVMF SDT Header probe |
-| ... | | suppressor |
-+----------------------------------+ | 36: padding for 8-byte |
- | alignment |
- | 40: GUID |
- | 56: padding to page size |
- +---------------------------+
+| ADDR method referring to VGIA | | 0: GUID |
+| ... | | 16: padding to page size |
++----------------------------------+ +---------------------------+
END OF PAGE
Device Usage:
-------------
diff --git a/include/hw/acpi/vmgenid.h b/include/hw/acpi/vmgenid.h
index 7beb9592fb01..b7cde45c0dea 100644
--- a/include/hw/acpi/vmgenid.h
+++ b/include/hw/acpi/vmgenid.h
@@ -9,13 +9,10 @@
#define VMGENID_GUID "guid"
#define VMGENID_GUID_FW_CFG_FILE "etc/vmgenid_guid"
#define VMGENID_ADDR_FW_CFG_FILE "etc/vmgenid_addr"
#define VMGENID_FW_CFG_SIZE 4096 /* Occupy a page of memory */
-#define VMGENID_GUID_OFFSET 40 /* allow space for
- * OVMF SDT Header Probe Supressor
- */
#define VMGENID(obj) OBJECT_CHECK(VmGenIdState, (obj), VMGENID_DEVICE)
typedef struct VmGenIdState {
DeviceClass parent_obj;
diff --git a/hw/acpi/vmgenid.c b/hw/acpi/vmgenid.c
index dc97771de5f7..92067bc52421 100644
--- a/hw/acpi/vmgenid.c
+++ b/hw/acpi/vmgenid.c
@@ -29,16 +29,11 @@ void vmgenid_build_acpi(VmGenIdState *vms, GArray *table_data, GArray *guid,
* first, since that's what the guest expects
*/
g_array_set_size(guid, VMGENID_FW_CFG_SIZE - ARRAY_SIZE(guid_le.data));
guid_le = vms->guid;
qemu_uuid_bswap(&guid_le);
- /* The GUID is written at a fixed offset into the fw_cfg file
- * in order to implement the "OVMF SDT Header probe suppressor"
- * see docs/specs/vmgenid.txt for more details
- */
- g_array_insert_vals(guid, VMGENID_GUID_OFFSET, guid_le.data,
- ARRAY_SIZE(guid_le.data));
+ g_array_insert_vals(guid, 0, guid_le.data, ARRAY_SIZE(guid_le.data));
/* Put this in a separate SSDT table */
ssdt = init_aml_allocator();
/* Reserve space for header */
@@ -70,12 +65,11 @@ void vmgenid_build_acpi(VmGenIdState *vms, GArray *table_data, GArray *guid,
method = aml_method("ADDR", 0, AML_NOTSERIALIZED);
addr = aml_local(0);
aml_append(method, aml_store(aml_package(2), addr));
- aml_append(method, aml_store(aml_add(aml_name("VGIA"),
- aml_int(VMGENID_GUID_OFFSET), NULL),
+ aml_append(method, aml_store(aml_name("VGIA"),
aml_index(addr, aml_int(0))));
aml_append(method, aml_store(aml_int(0), aml_index(addr, aml_int(1))));
aml_append(method, aml_return(addr));
aml_append(dev, method);
@@ -91,22 +85,19 @@ void vmgenid_build_acpi(VmGenIdState *vms, GArray *table_data, GArray *guid,
/* Allocate guest memory for the Data fw_cfg blob */
bios_linker_loader_alloc(linker, VMGENID_GUID_FW_CFG_FILE, guid,
4096 /* page boundary */,
BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH,
- BIOS_LINKER_LOADER_ALLOC_CONTENT_MIXED);
+ BIOS_LINKER_LOADER_ALLOC_CONTENT_NOACPI);
/* Patch address of GUID fw_cfg blob into the ADDR fw_cfg blob
* so QEMU can write the GUID there. The address is expected to be
* < 4GB, but write 64 bits anyway.
- * The address that is patched in is offset in order to implement
- * the "OVMF SDT Header probe suppressor"
- * see docs/specs/vmgenid.txt for more details.
*/
bios_linker_loader_write_pointer(linker,
VMGENID_ADDR_FW_CFG_FILE, 0, sizeof(uint64_t),
- VMGENID_GUID_FW_CFG_FILE, VMGENID_GUID_OFFSET);
+ VMGENID_GUID_FW_CFG_FILE, 0);
/* Patch address of GUID fw_cfg blob into the AML so OSPM can retrieve
* and read it. Note that while we provide storage for 64 bits, only
* the least-signficant 32 get patched into AML.
*/
@@ -150,14 +141,10 @@ static void vmgenid_update_guest(VmGenIdState *vms)
* however, will expect the fields to be little-endian.
* Perform a byte swap immediately before writing.
*/
guid_le = vms->guid;
qemu_uuid_bswap(&guid_le);
- /* The GUID is written at a fixed offset into the fw_cfg file
- * in order to implement the "OVMF SDT Header probe suppressor"
- * see docs/specs/vmgenid.txt for more details.
- */
cpu_physical_memory_write(vmgenid_addr, guid_le.data,
sizeof(guid_le.data));
/* Send _GPE.E05 event */
acpi_send_event(DEVICE(obj), ACPI_VMGENID_CHANGE_STATUS);
}
--
2.9.3
next prev parent reply other threads:[~2017-06-02 15:59 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-02 15:45 allocation zone extensions for the firmware linker/loader Laszlo Ersek
2017-06-02 15:59 ` [qemu PATCH 0/7] bios-linker-loader: introduce the NOACPI hint and the 64-bit zone for ALLOCATE Laszlo Ersek
2017-06-02 16:00 ` [qemu PATCH 1/7] hw/acpi/bios-linker-loader: expose allocation zone as an enum Laszlo Ersek
2017-06-02 16:00 ` [qemu PATCH 2/7] hw/acpi/bios-linker-loader: introduce "no ACPI tables" content hint for ALLOC Laszlo Ersek
2017-06-02 16:00 ` [qemu PATCH 3/7] hw/acpi/bios-linker-loader: introduce BIOS_LINKER_LOADER_ALLOC_ZONE_64BIT Laszlo Ersek
2017-06-02 16:00 ` [qemu PATCH 4/7] hw/acpi/nvdimm: ask the firmware to allocate NVDIMM_DSM_MEM_FILE as NOACPI Laszlo Ersek
2017-06-02 16:00 ` Laszlo Ersek [this message]
2017-06-02 16:00 ` [qemu PATCH 6/7] hw/i386/acpi-build: ask the fw to alloc ACPI_BUILD_TPMLOG_FILE with 64bit/NOACPI Laszlo Ersek
2017-06-02 16:00 ` [qemu PATCH 7/7] hw/arm/virt-acpi-build: make the fw alloc blobs with ACPI tables as 64bit Laszlo Ersek
2017-06-02 16:02 ` [seabios PATCH 0/2] romfile_loader: cope with the UEFI-oriented allocation extensions Laszlo Ersek
2017-06-02 16:02 ` [seabios PATCH 1/2] romfile_loader: alloc: cope with the UEFI-oriented NOACPI content hint Laszlo Ersek
2017-06-02 16:02 ` [seabios PATCH 2/2] romfile_loader: alloc: cope with the UEFI-oriented 64BIT zone hint Laszlo Ersek
2017-06-02 16:03 ` [edk2 PATCH 0/3] OvmfPkg/AcpiPlatformDxe: NOACPI hint and 64-bit zone in fw_cfg blob alloc Laszlo Ersek
2017-06-02 16:03 ` [edk2 PATCH 1/3] OvmfPkg/AcpiPlatformDxe: rename BLOB.HostsOnlyTableData to BLOB.Releasable Laszlo Ersek
2017-06-02 16:03 ` [edk2 PATCH 2/3] OvmfPkg/AcpiPlatformDxe: support NOACPI content hint in ALLOCATE command Laszlo Ersek
2017-06-02 16:03 ` [edk2 PATCH 3/3] OvmfPkg/AcpiPlatformDxe: support 64-bit zone " Laszlo Ersek
[not found] ` <20170602191230-mutt-send-email-mst@kernel.org>
2017-06-02 23:20 ` allocation zone extensions for the firmware linker/loader Laszlo Ersek
2017-06-03 7:36 ` Laszlo Ersek
2017-06-05 9:54 ` [Qemu-devel] " Igor Mammedov
2017-06-06 17:52 ` Laszlo Ersek
[not found] ` <20170605185815-mutt-send-email-mst@kernel.org>
2017-06-06 18:10 ` Laszlo Ersek
[not found] ` <20170608204207-mutt-send-email-mst@kernel.org>
2017-06-12 16:05 ` Paolo Bonzini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170602160006.1748-6-lersek@redhat.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox