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 2/7] hw/acpi/bios-linker-loader: introduce "no ACPI tables" content hint for ALLOC
Date: Fri, 2 Jun 2017 18:00:01 +0200 [thread overview]
Message-ID: <20170602160006.1748-3-lersek@redhat.com> (raw)
In-Reply-To: <20170602160006.1748-1-lersek@redhat.com>
OvmfPkg/AcpiPlatformDxe, which implements the client for QEMU's
linker/loader in the OVMF and ArmVirtQemu virtual UEFI firmwares,
currently relies on a 2nd pass processing of the ADD_POINTER commands, to
identify potential ACPI tables in the pointed-to blobs. The reason for
this is that ACPI tables must be individually passed to
EFI_ACPI_TABLE_PROTOCOL.InstallAcpiTable() for installation.
In order to tell apart ACPI tables from other operation region-like areas
within pointed-to blobs, OvmfPkg/AcpiPlatformDxe employs a heuristic
called "ACPI SDT header probe" at the target locations of the ADD_POINTER
commands. While all ACPI tables generated by QEMU satisfy this check
(i.e., there are no false negatives), blob content that is *not* an ACPI
table has a very slight chance to pass the test as well (i.e., there is a
small chance for false positives).
In order to suppress this small chance, we've historically formatted
opregion-like areas in blobs with a fixed size zero prefix (see e.g.
"docs/specs/vmgenid.txt"), which guarantees that the probe in
OvmfPkg/AcpiPlatformDxe will fail. However, this "suppressor prefix" has
had to be taken into account explicitly in generated AML code -- the
prefix size has had to be added to the patched integer object in AML, at
runtime --, leading to awkwardness.
Introduce a new hint for the ALLOC command, as the most significant bit of
the uint8_t "zone" field, for disabling the ACPI SDT header probe in
OvmfPkg/AcpiPlatformDxe, for all the pointers that point into the blob
downloaded with the ALLOC command. When the bit is set, the blob is
guaranteed to contain no ACPI tables. When the bit is clear, the behavior
is left unchanged.
In this initial patch, all bios_linker_loader_alloc() invocations are left
with intact behavior.
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>
---
include/hw/acpi/bios-linker-loader.h | 11 ++++++++++-
hw/acpi/bios-linker-loader.c | 8 ++++++--
hw/acpi/nvdimm.c | 3 ++-
hw/acpi/vmgenid.c | 3 ++-
hw/arm/virt-acpi-build.c | 6 ++++--
hw/i386/acpi-build.c | 9 ++++++---
6 files changed, 30 insertions(+), 10 deletions(-)
diff --git a/include/hw/acpi/bios-linker-loader.h b/include/hw/acpi/bios-linker-loader.h
index 8d55f1fab32b..5202fd14977d 100644
--- a/include/hw/acpi/bios-linker-loader.h
+++ b/include/hw/acpi/bios-linker-loader.h
@@ -13,17 +13,26 @@ typedef enum BIOSLinkerLoaderAllocZone {
/* request blob allocation in FSEG zone (useful for the RSDP ACPI table) */
BIOS_LINKER_LOADER_ALLOC_ZONE_FSEG = 0x2,
} BIOSLinkerLoaderAllocZone;
+typedef enum BIOSLinkerLoaderAllocContent {
+ /* the blob may or may not contain ACPI tables */
+ BIOS_LINKER_LOADER_ALLOC_CONTENT_MIXED = 0x00,
+
+ /* the blob is guaranteed not to contain ACPI tables */
+ BIOS_LINKER_LOADER_ALLOC_CONTENT_NOACPI = 0x80,
+} BIOSLinkerLoaderAllocContent;
+
BIOSLinker *bios_linker_loader_init(void);
void bios_linker_loader_alloc(BIOSLinker *linker,
const char *file_name,
GArray *file_blob,
uint32_t alloc_align,
- BIOSLinkerLoaderAllocZone zone);
+ BIOSLinkerLoaderAllocZone zone,
+ BIOSLinkerLoaderAllocContent content);
void bios_linker_loader_add_checksum(BIOSLinker *linker, const char *file,
unsigned start_offset, unsigned size,
unsigned checksum_offset);
diff --git a/hw/acpi/bios-linker-loader.c b/hw/acpi/bios-linker-loader.c
index 9754d98e7345..4ad9260fe72d 100644
--- a/hw/acpi/bios-linker-loader.c
+++ b/hw/acpi/bios-linker-loader.c
@@ -39,10 +39,12 @@ struct BiosLinkerLoaderEntry {
union {
/*
* COMMAND_ALLOCATE - allocate a table from @alloc.file
* subject to @alloc.align alignment (must be power of 2)
* and @alloc.zone (see BIOSLinkerLoaderAllocZone) requirements.
+ * The most significant bit (bit 7) of @alloc.zone is used as a content
+ * hint for UEFI guest firmware, see BIOSLinkerLoaderAllocContent.
*
* Must appear exactly once for each file, and before
* this file is referenced by any other command.
*/
struct {
@@ -169,18 +171,20 @@ bios_linker_find_file(const BIOSLinker *linker, const char *name)
* @linker: linker object instance
* @file_name: name of the file blob to be loaded
* @file_blob: pointer to blob corresponding to @file_name
* @alloc_align: required minimal alignment in bytes. Must be a power of 2.
* @zone: request allocation in this zone
+ * @content: information about the blob content for the firmware
*
* Note: this command must precede any other linker command using this file.
*/
void bios_linker_loader_alloc(BIOSLinker *linker,
const char *file_name,
GArray *file_blob,
uint32_t alloc_align,
- BIOSLinkerLoaderAllocZone zone)
+ BIOSLinkerLoaderAllocZone zone,
+ BIOSLinkerLoaderAllocContent content)
{
BiosLinkerLoaderEntry entry;
BiosLinkerFileEntry file = { g_strdup(file_name), file_blob};
assert(!(alloc_align & (alloc_align - 1)));
@@ -190,11 +194,11 @@ void bios_linker_loader_alloc(BIOSLinker *linker,
memset(&entry, 0, sizeof entry);
strncpy(entry.alloc.file, file_name, sizeof entry.alloc.file - 1);
entry.command = cpu_to_le32(BIOS_LINKER_LOADER_COMMAND_ALLOCATE);
entry.alloc.align = cpu_to_le32(alloc_align);
- entry.alloc.zone = zone;
+ entry.alloc.zone = zone | content;
/* Alloc entries must come first, so prepend them */
g_array_prepend_vals(linker->cmd_blob, &entry, sizeof entry);
}
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 91dd0df4b128..81bd0214fb3e 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -1262,11 +1262,12 @@ static void nvdimm_build_ssdt(GArray *table_offsets, GArray *table_data,
NVDIMM_ACPI_MEM_ADDR);
bios_linker_loader_alloc(linker,
NVDIMM_DSM_MEM_FILE, dsm_dma_arrea,
sizeof(NvdimmDsmIn),
- BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH);
+ BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH,
+ BIOS_LINKER_LOADER_ALLOC_CONTENT_MIXED);
bios_linker_loader_add_pointer(linker,
ACPI_BUILD_TABLE_FILE, mem_addr_offset, sizeof(uint32_t),
NVDIMM_DSM_MEM_FILE, 0);
build_header(linker, table_data,
(void *)(table_data->data + nvdimm_ssdt),
diff --git a/hw/acpi/vmgenid.c b/hw/acpi/vmgenid.c
index 315d3b3327ed..dc97771de5f7 100644
--- a/hw/acpi/vmgenid.c
+++ b/hw/acpi/vmgenid.c
@@ -90,11 +90,12 @@ void vmgenid_build_acpi(VmGenIdState *vms, GArray *table_data, GArray *guid,
g_array_append_vals(table_data, ssdt->buf->data, ssdt->buf->len);
/* 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_ZONE_HIGH,
+ BIOS_LINKER_LOADER_ALLOC_CONTENT_MIXED);
/* 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
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index a378e18b0d97..1c20b851a611 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -370,11 +370,12 @@ build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned xsdt_tbl_offset)
unsigned xsdt_pa_size = sizeof(rsdp->xsdt_physical_address);
unsigned xsdt_pa_offset =
(char *)&rsdp->xsdt_physical_address - rsdp_table->data;
bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, rsdp_table, 16,
- BIOS_LINKER_LOADER_ALLOC_ZONE_FSEG);
+ BIOS_LINKER_LOADER_ALLOC_ZONE_FSEG,
+ BIOS_LINKER_LOADER_ALLOC_CONTENT_MIXED);
memcpy(&rsdp->signature, "RSD PTR ", sizeof(rsdp->signature));
memcpy(rsdp->oem_id, ACPI_BUILD_APPNAME6, sizeof(rsdp->oem_id));
rsdp->length = cpu_to_le32(sizeof(*rsdp));
rsdp->revision = 0x02;
@@ -749,11 +750,12 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
table_offsets = g_array_new(false, true /* clear */,
sizeof(uint32_t));
bios_linker_loader_alloc(tables->linker,
ACPI_BUILD_TABLE_FILE, tables_blob,
- 64, BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH);
+ 64, BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH,
+ BIOS_LINKER_LOADER_ALLOC_CONTENT_MIXED);
/* DSDT is pointed to by FADT */
dsdt = tables_blob->len;
build_dsdt(tables_blob, tables->linker, vms);
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 4e7b30b44d5a..3c4c28c6c2ca 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2285,11 +2285,12 @@ build_tpm_tcpa(GArray *table_data, BIOSLinker *linker, GArray *tcpalog)
tcpa->platform_class = cpu_to_le16(TPM_TCPA_ACPI_CLASS_CLIENT);
tcpa->log_area_minimum_length = cpu_to_le32(TPM_LOG_AREA_MINIMUM_SIZE);
acpi_data_push(tcpalog, le32_to_cpu(tcpa->log_area_minimum_length));
bios_linker_loader_alloc(linker, ACPI_BUILD_TPMLOG_FILE, tcpalog, 1,
- BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH);
+ BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH,
+ BIOS_LINKER_LOADER_ALLOC_CONTENT_MIXED);
/* log area start address to be filled by Guest linker */
bios_linker_loader_add_pointer(linker,
ACPI_BUILD_TABLE_FILE, log_addr_offset, log_addr_size,
ACPI_BUILD_TPMLOG_FILE, 0);
@@ -2570,11 +2571,12 @@ build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt_tbl_offset)
unsigned rsdt_pa_size = sizeof(rsdp->rsdt_physical_address);
unsigned rsdt_pa_offset =
(char *)&rsdp->rsdt_physical_address - rsdp_table->data;
bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, rsdp_table, 16,
- BIOS_LINKER_LOADER_ALLOC_ZONE_FSEG);
+ BIOS_LINKER_LOADER_ALLOC_ZONE_FSEG,
+ BIOS_LINKER_LOADER_ALLOC_CONTENT_MIXED);
memcpy(&rsdp->signature, "RSD PTR ", 8);
memcpy(rsdp->oem_id, ACPI_BUILD_APPNAME6, 6);
/* Address to be filled by Guest linker */
bios_linker_loader_add_pointer(linker,
@@ -2649,11 +2651,12 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
ACPI_BUILD_DPRINTF("init ACPI tables\n");
bios_linker_loader_alloc(tables->linker,
ACPI_BUILD_TABLE_FILE, tables_blob,
64 /* Ensure FACS is aligned */,
- BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH);
+ BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH,
+ BIOS_LINKER_LOADER_ALLOC_CONTENT_MIXED);
/*
* FACS is pointed to by FADT.
* We place it first since it's the only table that has alignment
* requirements.
--
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 ` Laszlo Ersek [this message]
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 ` [qemu PATCH 5/7] hw/acpi/vmgenid: ask the fw to alloc VMGENID_GUID_FW_CFG_FILE " Laszlo Ersek
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-3-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