qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] acpi: Remove duplicated code handling OEM ID and OEM table ID fields
@ 2021-02-13 10:22 Marian Postevca
  2021-02-15 17:44 ` Igor Mammedov
  0 siblings, 1 reply; 4+ messages in thread
From: Marian Postevca @ 2021-02-13 10:22 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Xiao Guangrong, Sergio Lopez, Michael S. Tsirkin,
	Richard Henderson, Dongjiu Geng, Shannon Zhao, Xiang Zheng,
	open list:ACPI/HEST/GHES, Marian Postevca, Paolo Bonzini,
	Igor Mammedov, Eduardo Habkost

The code handling OEM ID and OEM table ID fields had some duplicated
code and was using unnecessary dynamically allocated memory.
This change fixes that.

Signed-off-by: Marian Postevca <posteuca@mutex.one>
---
 hw/acpi/hmat.h                   |  2 +-
 hw/i386/acpi-common.h            |  2 +-
 include/hw/acpi/acpi-build-oem.h | 55 ++++++++++++++++++++++
 include/hw/acpi/aml-build.h      | 16 +++----
 include/hw/acpi/ghes.h           |  2 +-
 include/hw/acpi/pci.h            |  2 +-
 include/hw/acpi/vmgenid.h        |  2 +-
 include/hw/arm/virt.h            |  4 +-
 include/hw/i386/microvm.h        |  4 +-
 include/hw/i386/pc.h             |  5 +-
 include/hw/mem/nvdimm.h          |  4 +-
 hw/acpi/aml-build.c              | 27 ++++++-----
 hw/acpi/ghes.c                   |  5 +-
 hw/acpi/hmat.c                   |  4 +-
 hw/acpi/nvdimm.c                 | 22 +++++----
 hw/acpi/pci.c                    |  4 +-
 hw/acpi/vmgenid.c                |  6 ++-
 hw/arm/virt-acpi-build.c         | 40 ++++++----------
 hw/arm/virt.c                    | 16 +++----
 hw/i386/acpi-build.c             | 78 +++++++++++++++-----------------
 hw/i386/acpi-common.c            |  4 +-
 hw/i386/acpi-microvm.c           | 13 ++----
 hw/i386/microvm.c                | 16 +++----
 hw/i386/pc.c                     | 16 +++----
 24 files changed, 190 insertions(+), 159 deletions(-)
 create mode 100644 include/hw/acpi/acpi-build-oem.h

diff --git a/hw/acpi/hmat.h b/hw/acpi/hmat.h
index b57f0e7e80..39c42328bd 100644
--- a/hw/acpi/hmat.h
+++ b/hw/acpi/hmat.h
@@ -38,6 +38,6 @@
 #define HMAT_PROXIMITY_INITIATOR_VALID  0x1
 
 void build_hmat(GArray *table_data, BIOSLinker *linker, NumaState *numa_state,
-                const char *oem_id, const char *oem_table_id);
+                struct AcpiBuildOem *bld_oem);
 
 #endif
diff --git a/hw/i386/acpi-common.h b/hw/i386/acpi-common.h
index b12cd73ea5..27c2e5b6a9 100644
--- a/hw/i386/acpi-common.h
+++ b/hw/i386/acpi-common.h
@@ -10,6 +10,6 @@
 
 void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
                      X86MachineState *x86ms, AcpiDeviceIf *adev,
-                     const char *oem_id, const char *oem_table_id);
+                     struct AcpiBuildOem *bld_oem);
 
 #endif
diff --git a/include/hw/acpi/acpi-build-oem.h b/include/hw/acpi/acpi-build-oem.h
new file mode 100644
index 0000000000..eb0d700006
--- /dev/null
+++ b/include/hw/acpi/acpi-build-oem.h
@@ -0,0 +1,55 @@
+#ifndef QEMU_HW_ACPI_BUILD_OEM_H
+#define QEMU_HW_ACPI_BUILD_OEM_H
+
+/*
+ * Utilities for working with ACPI OEM ID and OEM TABLE ID fields
+ *
+ * Copyright (c) 2021 Marian Postevca
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#include "qemu/cutils.h"
+
+#define ACPI_BUILD_APPNAME6 "BOCHS "
+#define ACPI_BUILD_APPNAME8 "BXPC    "
+
+#define ACPI_BUILD_OEM_ID_SIZE 6
+#define ACPI_BUILD_OEM_TABLE_ID_SIZE 8
+
+struct AcpiBuildOem {
+    char oem_id[ACPI_BUILD_OEM_ID_SIZE + 1];
+    char oem_table_id[ACPI_BUILD_OEM_TABLE_ID_SIZE + 1];
+};
+
+#define ACPI_INIT_BUILD_OEM_ID(__bld_oem, __oem_id) do {       \
+        pstrcpy(__bld_oem.oem_id,                              \
+                sizeof __bld_oem.oem_id, __oem_id);            \
+} while (0)
+
+#define ACPI_INIT_BUILD_OEM_TABLE_ID(__bld_oem,  __oem_table_id) do {   \
+        pstrcpy(__bld_oem.oem_table_id,                                 \
+                sizeof __bld_oem.oem_table_id, __oem_table_id);         \
+} while (0)
+
+#define ACPI_INIT_BUILD_OEM(__bld_oem, __oem_id, __oem_table_id) do {   \
+        ACPI_INIT_BUILD_OEM_ID(__bld_oem, __oem_id);                    \
+        ACPI_INIT_BUILD_OEM_TABLE_ID(__bld_oem, __oem_table_id);        \
+} while (0)
+
+#define ACPI_INIT_DEFAULT_BUILD_OEM(__bld_oem) do {                     \
+        ACPI_INIT_BUILD_OEM(__bld_oem,                                  \
+                            ACPI_BUILD_APPNAME6, ACPI_BUILD_APPNAME8);  \
+} while (0)
+
+#endif /* QEMU_HW_ACPI_BUILD_OEM_H */
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 380d3e3924..65c26842ad 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -3,13 +3,11 @@
 
 #include "hw/acpi/acpi-defs.h"
 #include "hw/acpi/bios-linker-loader.h"
+#include "hw/acpi/acpi-build-oem.h"
 
 /* Reserve RAM space for tables: add another order of magnitude. */
 #define ACPI_BUILD_TABLE_MAX_SIZE         0x200000
 
-#define ACPI_BUILD_APPNAME6 "BOCHS "
-#define ACPI_BUILD_APPNAME8 "BXPC    "
-
 #define ACPI_BUILD_TABLE_FILE "etc/acpi/tables"
 #define ACPI_BUILD_RSDP_FILE "etc/acpi/rsdp"
 #define ACPI_BUILD_TPMLOG_FILE "etc/tpm/log"
@@ -416,7 +414,7 @@ void build_append_int_noprefix(GArray *table, uint64_t value, int size);
 void
 build_header(BIOSLinker *linker, GArray *table_data,
              AcpiTableHeader *h, const char *sig, int len, uint8_t rev,
-             const char *oem_id, const char *oem_table_id);
+             struct AcpiBuildOem *bld_oem);
 void *acpi_data_push(GArray *table_data, unsigned size);
 unsigned acpi_data_len(GArray *table);
 void acpi_add_table(GArray *table_offsets, GArray *table_data);
@@ -426,10 +424,10 @@ void
 build_rsdp(GArray *tbl, BIOSLinker *linker, AcpiRsdpData *rsdp_data);
 void
 build_rsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets,
-           const char *oem_id, const char *oem_table_id);
+           struct AcpiBuildOem *bld_oem);
 void
 build_xsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets,
-           const char *oem_id, const char *oem_table_id);
+           struct AcpiBuildOem *bld_oem);
 
 int
 build_append_named_dword(GArray *array, const char *name_format, ...)
@@ -460,11 +458,11 @@ void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
                        uint64_t len, int node, MemoryAffinityFlags flags);
 
 void build_slit(GArray *table_data, BIOSLinker *linker, MachineState *ms,
-                const char *oem_id, const char *oem_table_id);
+                struct AcpiBuildOem *bld_oem);
 
 void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
-                const char *oem_id, const char *oem_table_id);
+                struct AcpiBuildOem *bld_oem);
 
 void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog,
-                const char *oem_id, const char *oem_table_id);
+                struct AcpiBuildOem *bld_oem);
 #endif
diff --git a/include/hw/acpi/ghes.h b/include/hw/acpi/ghes.h
index 2ae8bc1ded..9a7b654c98 100644
--- a/include/hw/acpi/ghes.h
+++ b/include/hw/acpi/ghes.h
@@ -68,7 +68,7 @@ typedef struct AcpiGhesState {
 
 void build_ghes_error_table(GArray *hardware_errors, BIOSLinker *linker);
 void acpi_build_hest(GArray *table_data, BIOSLinker *linker,
-                     const char *oem_id, const char *oem_table_id);
+                     struct AcpiBuildOem *bld_oem);
 void acpi_ghes_add_fw_cfg(AcpiGhesState *vms, FWCfgState *s,
                           GArray *hardware_errors);
 int acpi_ghes_record_errors(uint8_t notify, uint64_t error_physical_addr);
diff --git a/include/hw/acpi/pci.h b/include/hw/acpi/pci.h
index e514f179d8..1a41c6dc8a 100644
--- a/include/hw/acpi/pci.h
+++ b/include/hw/acpi/pci.h
@@ -34,5 +34,5 @@ typedef struct AcpiMcfgInfo {
 } AcpiMcfgInfo;
 
 void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info,
-                const char *oem_id, const char *oem_table_id);
+                struct AcpiBuildOem *bld_oem);
 #endif
diff --git a/include/hw/acpi/vmgenid.h b/include/hw/acpi/vmgenid.h
index dc8bb3433e..388d6ebd59 100644
--- a/include/hw/acpi/vmgenid.h
+++ b/include/hw/acpi/vmgenid.h
@@ -31,7 +31,7 @@ static inline Object *find_vmgenid_dev(void)
 }
 
 void vmgenid_build_acpi(VmGenIdState *vms, GArray *table_data, GArray *guid,
-                        BIOSLinker *linker, const char *oem_id);
+                        BIOSLinker *linker, struct AcpiBuildOem *bld_oem);
 void vmgenid_add_fw_cfg(VmGenIdState *vms, FWCfgState *s, GArray *guid);
 
 #endif
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index ee9a93101e..6af0396294 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -38,6 +38,7 @@
 #include "sysemu/kvm.h"
 #include "hw/intc/arm_gicv3_common.h"
 #include "qom/object.h"
+#include "hw/acpi/acpi-build-oem.h"
 
 #define NUM_GICV2M_SPIS       64
 #define NUM_VIRTIO_TRANSPORTS 32
@@ -165,8 +166,7 @@ struct VirtMachineState {
     DeviceState *acpi_dev;
     Notifier powerdown_notifier;
     PCIBus *bus;
-    char *oem_id;
-    char *oem_table_id;
+    struct AcpiBuildOem bld_oem;
 };
 
 #define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM : VIRT_PCIE_ECAM)
diff --git a/include/hw/i386/microvm.h b/include/hw/i386/microvm.h
index 372b05774e..4a42502a38 100644
--- a/include/hw/i386/microvm.h
+++ b/include/hw/i386/microvm.h
@@ -25,6 +25,7 @@
 #include "hw/boards.h"
 #include "hw/i386/x86.h"
 #include "hw/acpi/acpi_dev_interface.h"
+#include "hw/acpi/acpi-build-oem.h"
 #include "hw/pci-host/gpex.h"
 #include "qom/object.h"
 
@@ -106,8 +107,7 @@ struct MicrovmMachineState {
     Notifier machine_done;
     Notifier powerdown_req;
     struct GPEXConfig gpex;
-    char *oem_id;
-    char *oem_table_id;
+    struct AcpiBuildOem bld_oem;
 };
 
 #define TYPE_MICROVM_MACHINE   MACHINE_TYPE_NAME("microvm")
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 5f93540a43..461b09d887 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -11,6 +11,7 @@
 #include "hw/acpi/acpi_dev_interface.h"
 #include "hw/hotplug.h"
 #include "qom/object.h"
+#include "hw/acpi/acpi-build-oem.h"
 
 #define HPET_INTCAP "hpet-intcap"
 
@@ -45,9 +46,7 @@ typedef struct PCMachineState {
     bool pit_enabled;
     bool hpet_enabled;
     uint64_t max_fw_size;
-    char *oem_id;
-    char *oem_table_id;
-
+    struct AcpiBuildOem bld_oem;
     /* NUMA information: */
     uint64_t numa_nodes;
     uint64_t *node_mem;
diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
index bcf62f825c..a7ab354cd1 100644
--- a/include/hw/mem/nvdimm.h
+++ b/include/hw/mem/nvdimm.h
@@ -154,8 +154,8 @@ void nvdimm_init_acpi_state(NVDIMMState *state, MemoryRegion *io,
 void nvdimm_build_srat(GArray *table_data);
 void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
                        BIOSLinker *linker, NVDIMMState *state,
-                       uint32_t ram_slots, const char *oem_id,
-                       const char *oem_table_id);
+                       uint32_t ram_slots,
+                       struct AcpiBuildOem *bld_oem);
 void nvdimm_plug(NVDIMMState *state);
 void nvdimm_acpi_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev);
 #endif
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index a2cd7a5830..164d02e0c9 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1667,7 +1667,7 @@ Aml *aml_object_type(Aml *object)
 void
 build_header(BIOSLinker *linker, GArray *table_data,
              AcpiTableHeader *h, const char *sig, int len, uint8_t rev,
-             const char *oem_id, const char *oem_table_id)
+             struct AcpiBuildOem *bld_oem)
 {
     unsigned tbl_offset = (char *)h - table_data->data;
     unsigned checksum_offset = (char *)&h->checksum - table_data->data;
@@ -1675,9 +1675,9 @@ build_header(BIOSLinker *linker, GArray *table_data,
     h->length = cpu_to_le32(len);
     h->revision = rev;
 
-    strpadcpy((char *)h->oem_id, sizeof h->oem_id, oem_id, ' ');
+    strpadcpy((char *)h->oem_id, sizeof h->oem_id, bld_oem->oem_id, ' ');
     strpadcpy((char *)h->oem_table_id, sizeof h->oem_table_id,
-              oem_table_id, ' ');
+              bld_oem->oem_table_id, ' ');
 
     h->oem_revision = cpu_to_le32(1);
     memcpy(h->asl_compiler_id, ACPI_BUILD_APPNAME8, 4);
@@ -1797,7 +1797,7 @@ build_rsdp(GArray *tbl, BIOSLinker *linker, AcpiRsdpData *rsdp_data)
 /* Build rsdt table */
 void
 build_rsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets,
-           const char *oem_id, const char *oem_table_id)
+           struct AcpiBuildOem *bld_oem)
 {
     int i;
     unsigned rsdt_entries_offset;
@@ -1818,13 +1818,13 @@ build_rsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets,
             ACPI_BUILD_TABLE_FILE, ref_tbl_offset);
     }
     build_header(linker, table_data,
-                 (void *)rsdt, "RSDT", rsdt_len, 1, oem_id, oem_table_id);
+                 (void *)rsdt, "RSDT", rsdt_len, 1, bld_oem);
 }
 
 /* Build xsdt table */
 void
 build_xsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets,
-           const char *oem_id, const char *oem_table_id)
+           struct AcpiBuildOem *bld_oem)
 {
     int i;
     unsigned xsdt_entries_offset;
@@ -1845,7 +1845,7 @@ build_xsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets,
             ACPI_BUILD_TABLE_FILE, ref_tbl_offset);
     }
     build_header(linker, table_data,
-                 (void *)xsdt, "XSDT", xsdt_len, 1, oem_id, oem_table_id);
+                 (void *)xsdt, "XSDT", xsdt_len, 1, bld_oem);
 }
 
 void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
@@ -1864,7 +1864,7 @@ void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
  * (Revision 2.0 or later)
  */
 void build_slit(GArray *table_data, BIOSLinker *linker, MachineState *ms,
-                const char *oem_id, const char *oem_table_id)
+                struct AcpiBuildOem *bld_oem)
 {
     int slit_start, i, j;
     slit_start = table_data->len;
@@ -1885,12 +1885,12 @@ void build_slit(GArray *table_data, BIOSLinker *linker, MachineState *ms,
     build_header(linker, table_data,
                  (void *)(table_data->data + slit_start),
                  "SLIT",
-                 table_data->len - slit_start, 1, oem_id, oem_table_id);
+                 table_data->len - slit_start, 1, bld_oem);
 }
 
 /* build rev1/rev3/rev5.1 FADT */
 void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
-                const char *oem_id, const char *oem_table_id)
+                struct AcpiBuildOem *bld_oem)
 {
     int off;
     int fadt_start = tbl->len;
@@ -2009,7 +2009,7 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
 
 build_hdr:
     build_header(linker, tbl, (void *)(tbl->data + fadt_start),
-                 "FACP", tbl->len - fadt_start, f->rev, oem_id, oem_table_id);
+                 "FACP", tbl->len - fadt_start, f->rev, bld_oem);
 }
 
 /*
@@ -2018,7 +2018,7 @@ build_hdr:
  * of TCG ACPI Specification, Family “1.2” and “2.0”, Version 1.2, Rev 8
  */
 void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog,
-                const char *oem_id, const char *oem_table_id)
+                struct AcpiBuildOem *bld_oem)
 {
     uint8_t start_method_params[12] = {};
     unsigned log_addr_offset, tpm2_start;
@@ -2067,8 +2067,7 @@ void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog,
                                    log_addr_offset, 8,
                                    ACPI_BUILD_TPMLOG_FILE, 0);
     build_header(linker, table_data,
-                 tpm2_ptr, "TPM2", table_data->len - tpm2_start, 4, oem_id,
-                 oem_table_id);
+                 tpm2_ptr, "TPM2", table_data->len - tpm2_start, 4, bld_oem);
 }
 
 Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set, uint32_t io_offset,
diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c
index a4dac6bf15..963d602b26 100644
--- a/hw/acpi/ghes.c
+++ b/hw/acpi/ghes.c
@@ -21,6 +21,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/units.h"
+#include "hw/acpi/acpi-build-oem.h"
 #include "hw/acpi/ghes.h"
 #include "hw/acpi/aml-build.h"
 #include "qemu/error-report.h"
@@ -360,7 +361,7 @@ static void build_ghes_v2(GArray *table_data, int source_id, BIOSLinker *linker)
 
 /* Build Hardware Error Source Table */
 void acpi_build_hest(GArray *table_data, BIOSLinker *linker,
-                     const char *oem_id, const char *oem_table_id)
+                     struct AcpiBuildOem *bld_oem)
 {
     uint64_t hest_start = table_data->len;
 
@@ -373,7 +374,7 @@ void acpi_build_hest(GArray *table_data, BIOSLinker *linker,
     build_ghes_v2(table_data, ACPI_HEST_SRC_ID_SEA, linker);
 
     build_header(linker, table_data, (void *)(table_data->data + hest_start),
-                 "HEST", table_data->len - hest_start, 1, oem_id, oem_table_id);
+                 "HEST", table_data->len - hest_start, 1, bld_oem);
 }
 
 void acpi_ghes_add_fw_cfg(AcpiGhesState *ags, FWCfgState *s,
diff --git a/hw/acpi/hmat.c b/hw/acpi/hmat.c
index edb3fd91b2..d8f594100f 100644
--- a/hw/acpi/hmat.c
+++ b/hw/acpi/hmat.c
@@ -254,7 +254,7 @@ static void hmat_build_table_structs(GArray *table_data, NumaState *numa_state)
 }
 
 void build_hmat(GArray *table_data, BIOSLinker *linker, NumaState *numa_state,
-                const char *oem_id, const char *oem_table_id)
+                struct AcpiBuildOem *bld_oem)
 {
     int hmat_start = table_data->len;
 
@@ -265,5 +265,5 @@ void build_hmat(GArray *table_data, BIOSLinker *linker, NumaState *numa_state,
 
     build_header(linker, table_data,
                  (void *)(table_data->data + hmat_start),
-                 "HMAT", table_data->len - hmat_start, 2, oem_id, oem_table_id);
+                 "HMAT", table_data->len - hmat_start, 2, bld_oem);
 }
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index e3d5fe1939..54b29fc424 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -403,7 +403,7 @@ void nvdimm_plug(NVDIMMState *state)
 
 static void nvdimm_build_nfit(NVDIMMState *state, GArray *table_offsets,
                               GArray *table_data, BIOSLinker *linker,
-                              const char *oem_id, const char *oem_table_id)
+                              struct AcpiBuildOem *bld_oem)
 {
     NvdimmFitBuffer *fit_buf = &state->fit_buf;
     unsigned int header;
@@ -418,8 +418,7 @@ static void nvdimm_build_nfit(NVDIMMState *state, GArray *table_offsets,
 
     build_header(linker, table_data,
                  (void *)(table_data->data + header), "NFIT",
-                 sizeof(NvdimmNfitHeader) + fit_buf->fit->len, 1, oem_id,
-                 oem_table_id);
+                 sizeof(NvdimmNfitHeader) + fit_buf->fit->len, 1, bld_oem);
 }
 
 #define NVDIMM_DSM_MEMORY_SIZE      4096
@@ -1280,9 +1279,11 @@ static void nvdimm_build_nvdimm_devices(Aml *root_dev, uint32_t ram_slots)
 static void nvdimm_build_ssdt(GArray *table_offsets, GArray *table_data,
                               BIOSLinker *linker,
                               NVDIMMState *nvdimm_state,
-                              uint32_t ram_slots, const char *oem_id)
+                              uint32_t ram_slots,
+                              struct AcpiBuildOem *bld_oem)
 {
     Aml *ssdt, *sb_scope, *dev;
+    struct AcpiBuildOem tmp_bld_oem;
     int mem_addr_offset, nvdimm_ssdt;
 
     acpi_add_table(table_offsets, table_data);
@@ -1331,9 +1332,11 @@ static void nvdimm_build_ssdt(GArray *table_offsets, GArray *table_data,
     bios_linker_loader_add_pointer(linker,
         ACPI_BUILD_TABLE_FILE, mem_addr_offset, sizeof(uint32_t),
         NVDIMM_DSM_MEM_FILE, 0);
+
+    ACPI_INIT_BUILD_OEM(tmp_bld_oem, bld_oem->oem_id, "NVDIMM");
     build_header(linker, table_data,
-        (void *)(table_data->data + nvdimm_ssdt),
-                 "SSDT", table_data->len - nvdimm_ssdt, 1, oem_id, "NVDIMM");
+                 (void *)(table_data->data + nvdimm_ssdt),
+                 "SSDT", table_data->len - nvdimm_ssdt, 1, &tmp_bld_oem);
     free_aml_allocator();
 }
 
@@ -1361,8 +1364,7 @@ void nvdimm_build_srat(GArray *table_data)
 
 void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
                        BIOSLinker *linker, NVDIMMState *state,
-                       uint32_t ram_slots, const char *oem_id,
-                       const char *oem_table_id)
+                       uint32_t ram_slots, struct AcpiBuildOem *bld_oem)
 {
     GSList *device_list;
 
@@ -1372,7 +1374,7 @@ void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
     }
 
     nvdimm_build_ssdt(table_offsets, table_data, linker, state,
-                      ram_slots, oem_id);
+                      ram_slots, bld_oem);
 
     device_list = nvdimm_get_device_list();
     /* no NVDIMM device is plugged. */
@@ -1381,6 +1383,6 @@ void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
     }
 
     nvdimm_build_nfit(state, table_offsets, table_data, linker,
-                      oem_id, oem_table_id);
+                      bld_oem);
     g_slist_free(device_list);
 }
diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
index ec455c3b25..0e7173a24d 100644
--- a/hw/acpi/pci.c
+++ b/hw/acpi/pci.c
@@ -29,7 +29,7 @@
 #include "hw/pci/pcie_host.h"
 
 void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info,
-                const char *oem_id, const char *oem_table_id)
+                struct AcpiBuildOem *bld_oem)
 {
     int mcfg_start = table_data->len;
 
@@ -57,6 +57,6 @@ void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info,
     build_append_int_noprefix(table_data, 0, 4);
 
     build_header(linker, table_data, (void *)(table_data->data + mcfg_start),
-                 "MCFG", table_data->len - mcfg_start, 1, oem_id, oem_table_id);
+                 "MCFG", table_data->len - mcfg_start, 1, bld_oem);
 }
 
diff --git a/hw/acpi/vmgenid.c b/hw/acpi/vmgenid.c
index 4f41a13ea0..bf3253d168 100644
--- a/hw/acpi/vmgenid.c
+++ b/hw/acpi/vmgenid.c
@@ -24,11 +24,12 @@
 #include "sysemu/reset.h"
 
 void vmgenid_build_acpi(VmGenIdState *vms, GArray *table_data, GArray *guid,
-                        BIOSLinker *linker, const char *oem_id)
+                        BIOSLinker *linker, struct AcpiBuildOem *bld_oem)
 {
     Aml *ssdt, *dev, *scope, *method, *addr, *if_ctx;
     uint32_t vgia_offset;
     QemuUUID guid_le;
+    struct AcpiBuildOem tmp_bld_oem;
 
     /* Fill in the GUID values.  These need to be converted to little-endian
      * first, since that's what the guest expects
@@ -116,9 +117,10 @@ void vmgenid_build_acpi(VmGenIdState *vms, GArray *table_data, GArray *guid,
         ACPI_BUILD_TABLE_FILE, vgia_offset, sizeof(uint32_t),
         VMGENID_GUID_FW_CFG_FILE, 0);
 
+    ACPI_INIT_BUILD_OEM(tmp_bld_oem, bld_oem->oem_id, "VMGENID");
     build_header(linker, table_data,
         (void *)(table_data->data + table_data->len - ssdt->buf->len),
-        "SSDT", ssdt->buf->len, 1, oem_id, "VMGENID");
+        "SSDT", ssdt->buf->len, 1, &tmp_bld_oem);
     free_aml_allocator();
 }
 
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index f9c9df916c..55d2f427c2 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -341,8 +341,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     iort->length = cpu_to_le32(iort_length);
 
     build_header(linker, table_data, (void *)(table_data->data + iort_start),
-                 "IORT", table_data->len - iort_start, 0, vms->oem_id,
-                 vms->oem_table_id);
+                 "IORT", table_data->len - iort_start, 0, &vms->bld_oem);
 }
 
 static void
@@ -376,8 +375,7 @@ build_spcr(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     spcr->pci_vendor_id = 0xffff;  /* PCI Vendor ID: not a PCI device */
 
     build_header(linker, table_data, (void *)(table_data->data + spcr_start),
-                 "SPCR", table_data->len - spcr_start, 2, vms->oem_id,
-                 vms->oem_table_id);
+                 "SPCR", table_data->len - spcr_start, 2, &vms->bld_oem);
 }
 
 static void
@@ -429,8 +427,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     }
 
     build_header(linker, table_data, (void *)(table_data->data + srat_start),
-                 "SRAT", table_data->len - srat_start, 3, vms->oem_id,
-                 vms->oem_table_id);
+                 "SRAT", table_data->len - srat_start, 3, &vms->bld_oem);
 }
 
 /* GTDT */
@@ -465,8 +462,7 @@ build_gtdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
 
     build_header(linker, table_data,
                  (void *)(table_data->data + gtdt_start), "GTDT",
-                 table_data->len - gtdt_start, 2, vms->oem_id,
-                 vms->oem_table_id);
+                 table_data->len - gtdt_start, 2, &vms->bld_oem);
 }
 
 /* MADT */
@@ -555,8 +551,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
 
     build_header(linker, table_data,
                  (void *)(table_data->data + madt_start), "APIC",
-                 table_data->len - madt_start, 3, vms->oem_id,
-                 vms->oem_table_id);
+                 table_data->len - madt_start, 3, &vms->bld_oem);
 }
 
 /* FADT */
@@ -586,7 +581,7 @@ static void build_fadt_rev5(GArray *table_data, BIOSLinker *linker,
         g_assert_not_reached();
     }
 
-    build_fadt(table_data, linker, &fadt, vms->oem_id, vms->oem_table_id);
+    build_fadt(table_data, linker, &fadt, &vms->bld_oem);
 }
 
 /* DSDT */
@@ -650,8 +645,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     g_array_append_vals(table_data, dsdt->buf->data, dsdt->buf->len);
     build_header(linker, table_data,
         (void *)(table_data->data + table_data->len - dsdt->buf->len),
-                 "DSDT", dsdt->buf->len, 2, vms->oem_id,
-                 vms->oem_table_id);
+                 "DSDT", dsdt->buf->len, 2, &vms->bld_oem);
     free_aml_allocator();
 }
 
@@ -710,8 +704,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
            .base = vms->memmap[VIRT_ECAM_ID(vms->highmem_ecam)].base,
            .size = vms->memmap[VIRT_ECAM_ID(vms->highmem_ecam)].size,
         };
-        build_mcfg(tables_blob, tables->linker, &mcfg, vms->oem_id,
-                   vms->oem_table_id);
+        build_mcfg(tables_blob, tables->linker, &mcfg, &vms->bld_oem);
     }
 
     acpi_add_table(table_offsets, tables_blob);
@@ -720,8 +713,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
     if (vms->ras) {
         build_ghes_error_table(tables->hardware_errors, tables->linker);
         acpi_add_table(table_offsets, tables_blob);
-        acpi_build_hest(tables_blob, tables->linker, vms->oem_id,
-                        vms->oem_table_id);
+        acpi_build_hest(tables_blob, tables->linker, &vms->bld_oem);
     }
 
     if (ms->numa_state->num_nodes > 0) {
@@ -729,15 +721,13 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
         build_srat(tables_blob, tables->linker, vms);
         if (ms->numa_state->have_numa_distance) {
             acpi_add_table(table_offsets, tables_blob);
-            build_slit(tables_blob, tables->linker, ms, vms->oem_id,
-                       vms->oem_table_id);
+            build_slit(tables_blob, tables->linker, ms, &vms->bld_oem);
         }
     }
 
     if (ms->nvdimms_state->is_enabled) {
         nvdimm_build_acpi(table_offsets, tables_blob, tables->linker,
-                          ms->nvdimms_state, ms->ram_slots, vms->oem_id,
-                          vms->oem_table_id);
+                          ms->nvdimms_state, ms->ram_slots, &vms->bld_oem);
     }
 
     if (its_class_name() && !vmc->no_its) {
@@ -747,20 +737,18 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
 
     if (tpm_get_version(tpm_find()) == TPM_VERSION_2_0) {
         acpi_add_table(table_offsets, tables_blob);
-        build_tpm2(tables_blob, tables->linker, tables->tcpalog, vms->oem_id,
-                   vms->oem_table_id);
+        build_tpm2(tables_blob, tables->linker, tables->tcpalog, &vms->bld_oem);
     }
 
     /* XSDT is pointed to by RSDP */
     xsdt = tables_blob->len;
-    build_xsdt(tables_blob, tables->linker, table_offsets, vms->oem_id,
-               vms->oem_table_id);
+    build_xsdt(tables_blob, tables->linker, table_offsets, &vms->bld_oem);
 
     /* RSDP is in FSEG memory, so allocate it separately */
     {
         AcpiRsdpData rsdp_data = {
             .revision = 2,
-            .oem_id = vms->oem_id,
+            .oem_id = vms->bld_oem.oem_id,
             .xsdt_tbl_offset = &xsdt,
             .rsdt_tbl_offset = NULL,
         };
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 371147f3ae..2cdf782fae 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2159,7 +2159,7 @@ static char *virt_get_oem_id(Object *obj, Error **errp)
 {
     VirtMachineState *vms = VIRT_MACHINE(obj);
 
-    return g_strdup(vms->oem_id);
+    return g_strdup(vms->bld_oem.oem_id);
 }
 
 static void virt_set_oem_id(Object *obj, const char *value, Error **errp)
@@ -2167,20 +2167,19 @@ static void virt_set_oem_id(Object *obj, const char *value, Error **errp)
     VirtMachineState *vms = VIRT_MACHINE(obj);
     size_t len = strlen(value);
 
-    if (len > 6) {
+    if (len > ACPI_BUILD_OEM_ID_SIZE) {
         error_setg(errp,
                    "User specified oem-id value is bigger than 6 bytes in size");
         return;
     }
-
-    strncpy(vms->oem_id, value, 6);
+    ACPI_INIT_BUILD_OEM_ID(vms->bld_oem, value);
 }
 
 static char *virt_get_oem_table_id(Object *obj, Error **errp)
 {
     VirtMachineState *vms = VIRT_MACHINE(obj);
 
-    return g_strdup(vms->oem_table_id);
+    return g_strdup(vms->bld_oem.oem_table_id);
 }
 
 static void virt_set_oem_table_id(Object *obj, const char *value,
@@ -2189,12 +2188,12 @@ static void virt_set_oem_table_id(Object *obj, const char *value,
     VirtMachineState *vms = VIRT_MACHINE(obj);
     size_t len = strlen(value);
 
-    if (len > 8) {
+    if (len > ACPI_BUILD_OEM_TABLE_ID_SIZE) {
         error_setg(errp,
                    "User specified oem-table-id value is bigger than 8 bytes in size");
         return;
     }
-    strncpy(vms->oem_table_id, value, 8);
+    ACPI_INIT_BUILD_OEM_TABLE_ID(vms->bld_oem, value);
 }
 
 
@@ -2706,8 +2705,7 @@ static void virt_instance_init(Object *obj)
 
     virt_flash_create(vms);
 
-    vms->oem_id = g_strndup(ACPI_BUILD_APPNAME6, 6);
-    vms->oem_table_id = g_strndup(ACPI_BUILD_APPNAME8, 8);
+    ACPI_INIT_DEFAULT_BUILD_OEM(vms->bld_oem);
 }
 
 static const TypeInfo virt_machine_info = {
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b9190b924a..a638e04165 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -24,6 +24,7 @@
 #include "qapi/error.h"
 #include "qapi/qmp/qnum.h"
 #include "acpi-build.h"
+#include "hw/acpi/acpi-build-oem.h"
 #include "acpi-common.h"
 #include "qemu/bitmap.h"
 #include "qemu/error-report.h"
@@ -1637,13 +1638,12 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
     g_array_append_vals(table_data, dsdt->buf->data, dsdt->buf->len);
     build_header(linker, table_data,
         (void *)(table_data->data + table_data->len - dsdt->buf->len),
-                 "DSDT", dsdt->buf->len, 1, pcms->oem_id, pcms->oem_table_id);
+                 "DSDT", dsdt->buf->len, 1, &pcms->bld_oem);
     free_aml_allocator();
 }
 
 static void
-build_hpet(GArray *table_data, BIOSLinker *linker, const char *oem_id,
-           const char *oem_table_id)
+build_hpet(GArray *table_data, BIOSLinker *linker, struct AcpiBuildOem *bld_oem)
 {
     Acpi20Hpet *hpet;
 
@@ -1654,12 +1654,12 @@ build_hpet(GArray *table_data, BIOSLinker *linker, const char *oem_id,
     hpet->timer_block_id = cpu_to_le32(0x8086a201);
     hpet->addr.address = cpu_to_le64(HPET_BASE);
     build_header(linker, table_data,
-                 (void *)hpet, "HPET", sizeof(*hpet), 1, oem_id, oem_table_id);
+                 (void *)hpet, "HPET", sizeof(*hpet), 1, bld_oem);
 }
 
 static void
 build_tpm_tcpa(GArray *table_data, BIOSLinker *linker, GArray *tcpalog,
-               const char *oem_id, const char *oem_table_id)
+               struct AcpiBuildOem *bld_oem)
 {
     Acpi20Tcpa *tcpa = acpi_data_push(table_data, sizeof *tcpa);
     unsigned log_addr_size = sizeof(tcpa->log_area_start_address);
@@ -1679,7 +1679,7 @@ build_tpm_tcpa(GArray *table_data, BIOSLinker *linker, GArray *tcpalog,
         ACPI_BUILD_TPMLOG_FILE, 0);
 
     build_header(linker, table_data,
-                 (void *)tcpa, "TCPA", sizeof(*tcpa), 2, oem_id, oem_table_id);
+                 (void *)tcpa, "TCPA", sizeof(*tcpa), 2, bld_oem);
 }
 
 #define HOLE_640K_START  (640 * KiB)
@@ -1814,8 +1814,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
     build_header(linker, table_data,
                  (void *)(table_data->data + srat_start),
                  "SRAT",
-                 table_data->len - srat_start, 1, pcms->oem_id,
-                 pcms->oem_table_id);
+                 table_data->len - srat_start, 1, &pcms->bld_oem);
 }
 
 /*
@@ -1823,8 +1822,8 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
  * (version Oct. 2014 or later)
  */
 static void
-build_dmar_q35(GArray *table_data, BIOSLinker *linker, const char *oem_id,
-               const char *oem_table_id)
+build_dmar_q35(GArray *table_data, BIOSLinker *linker,
+               struct AcpiBuildOem *bld_oem)
 {
     int dmar_start = table_data->len;
 
@@ -1874,7 +1873,7 @@ build_dmar_q35(GArray *table_data, BIOSLinker *linker, const char *oem_id,
     }
 
     build_header(linker, table_data, (void *)(table_data->data + dmar_start),
-                 "DMAR", table_data->len - dmar_start, 1, oem_id, oem_table_id);
+                 "DMAR", table_data->len - dmar_start, 1, bld_oem);
 }
 
 /*
@@ -1885,8 +1884,7 @@ build_dmar_q35(GArray *table_data, BIOSLinker *linker, const char *oem_id,
  * Helpful to speedup Windows guests and ignored by others.
  */
 static void
-build_waet(GArray *table_data, BIOSLinker *linker, const char *oem_id,
-           const char *oem_table_id)
+build_waet(GArray *table_data, BIOSLinker *linker, struct AcpiBuildOem *bld_oem)
 {
     int waet_start = table_data->len;
 
@@ -1902,7 +1900,7 @@ build_waet(GArray *table_data, BIOSLinker *linker, const char *oem_id,
     build_append_int_noprefix(table_data, 1 << 1 /* ACPI PM timer good */, 4);
 
     build_header(linker, table_data, (void *)(table_data->data + waet_start),
-                 "WAET", table_data->len - waet_start, 1, oem_id, oem_table_id);
+                 "WAET", table_data->len - waet_start, 1, bld_oem);
 }
 
 /*
@@ -2004,8 +2002,8 @@ ivrs_host_bridges(Object *obj, void *opaque)
 }
 
 static void
-build_amd_iommu(GArray *table_data, BIOSLinker *linker, const char *oem_id,
-                const char *oem_table_id)
+build_amd_iommu(GArray *table_data, BIOSLinker *linker,
+                struct AcpiBuildOem *bld_oem)
 {
     int ivhd_table_len = 24;
     int iommu_start = table_data->len;
@@ -2100,8 +2098,7 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker, const char *oem_id,
     }
 
     build_header(linker, table_data, (void *)(table_data->data + iommu_start),
-                 "IVRS", table_data->len - iommu_start, 1, oem_id,
-                 oem_table_id);
+                 "IVRS", table_data->len - iommu_start, 1, bld_oem);
 }
 
 typedef
@@ -2157,6 +2154,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
     GArray *tables_blob = tables->table_data;
     AcpiSlicOem slic_oem = { .id = NULL, .table_id = NULL };
     Object *vmgenid_dev;
+    struct AcpiBuildOem slic_bld_oem;
+    struct AcpiBuildOem *bld_oem;
     char *oem_id;
     char *oem_table_id;
 
@@ -2168,15 +2167,18 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
     if (slic_oem.id) {
         oem_id = slic_oem.id;
     } else {
-        oem_id = pcms->oem_id;
+        oem_id = pcms->bld_oem.oem_id;
     }
 
     if (slic_oem.table_id) {
         oem_table_id = slic_oem.table_id;
     } else {
-        oem_table_id = pcms->oem_table_id;
+        oem_table_id = pcms->bld_oem.oem_table_id;
     }
 
+    ACPI_INIT_BUILD_OEM(slic_bld_oem, oem_id, oem_table_id);
+    bld_oem = &pcms->bld_oem;
+
     table_offsets = g_array_new(false, true /* clear */,
                                         sizeof(uint32_t));
     ACPI_BUILD_DPRINTF("init ACPI tables\n");
@@ -2210,35 +2212,33 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
     pm.fadt.facs_tbl_offset = &facs;
     pm.fadt.dsdt_tbl_offset = &dsdt;
     pm.fadt.xdsdt_tbl_offset = &dsdt;
-    build_fadt(tables_blob, tables->linker, &pm.fadt, oem_id, oem_table_id);
+    build_fadt(tables_blob, tables->linker, &pm.fadt, &slic_bld_oem);
     aml_len += tables_blob->len - fadt;
 
     acpi_add_table(table_offsets, tables_blob);
     acpi_build_madt(tables_blob, tables->linker, x86ms,
-                    ACPI_DEVICE_IF(x86ms->acpi_dev), pcms->oem_id,
-                    pcms->oem_table_id);
+                    ACPI_DEVICE_IF(x86ms->acpi_dev), bld_oem);
 
     vmgenid_dev = find_vmgenid_dev();
     if (vmgenid_dev) {
         acpi_add_table(table_offsets, tables_blob);
         vmgenid_build_acpi(VMGENID(vmgenid_dev), tables_blob,
-                           tables->vmgenid, tables->linker, pcms->oem_id);
+                           tables->vmgenid, tables->linker, bld_oem);
     }
 
     if (misc.has_hpet) {
         acpi_add_table(table_offsets, tables_blob);
-        build_hpet(tables_blob, tables->linker, pcms->oem_id,
-                   pcms->oem_table_id);
+        build_hpet(tables_blob, tables->linker, bld_oem);
     }
     if (misc.tpm_version != TPM_VERSION_UNSPEC) {
         if (misc.tpm_version == TPM_VERSION_1_2) {
             acpi_add_table(table_offsets, tables_blob);
             build_tpm_tcpa(tables_blob, tables->linker, tables->tcpalog,
-                           pcms->oem_id, pcms->oem_table_id);
+                           bld_oem);
         } else { /* TPM_VERSION_2_0 */
             acpi_add_table(table_offsets, tables_blob);
             build_tpm2(tables_blob, tables->linker, tables->tcpalog,
-                       pcms->oem_id, pcms->oem_table_id);
+                       bld_oem);
         }
     }
     if (pcms->numa_nodes) {
@@ -2246,40 +2246,36 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
         build_srat(tables_blob, tables->linker, machine);
         if (machine->numa_state->have_numa_distance) {
             acpi_add_table(table_offsets, tables_blob);
-            build_slit(tables_blob, tables->linker, machine, pcms->oem_id,
-                       pcms->oem_table_id);
+            build_slit(tables_blob, tables->linker, machine, bld_oem);
         }
         if (machine->numa_state->hmat_enabled) {
             acpi_add_table(table_offsets, tables_blob);
             build_hmat(tables_blob, tables->linker, machine->numa_state,
-                       pcms->oem_id, pcms->oem_table_id);
+                       bld_oem);
         }
     }
     if (acpi_get_mcfg(&mcfg)) {
         acpi_add_table(table_offsets, tables_blob);
-        build_mcfg(tables_blob, tables->linker, &mcfg, pcms->oem_id,
-                   pcms->oem_table_id);
+        build_mcfg(tables_blob, tables->linker, &mcfg, bld_oem);
     }
     if (x86_iommu_get_default()) {
         IommuType IOMMUType = x86_iommu_get_type();
         if (IOMMUType == TYPE_AMD) {
             acpi_add_table(table_offsets, tables_blob);
-            build_amd_iommu(tables_blob, tables->linker, pcms->oem_id,
-                            pcms->oem_table_id);
+            build_amd_iommu(tables_blob, tables->linker, bld_oem);
         } else if (IOMMUType == TYPE_INTEL) {
             acpi_add_table(table_offsets, tables_blob);
-            build_dmar_q35(tables_blob, tables->linker, pcms->oem_id,
-                           pcms->oem_table_id);
+            build_dmar_q35(tables_blob, tables->linker, bld_oem);
         }
     }
     if (machine->nvdimms_state->is_enabled) {
         nvdimm_build_acpi(table_offsets, tables_blob, tables->linker,
                           machine->nvdimms_state, machine->ram_slots,
-                          pcms->oem_id, pcms->oem_table_id);
+                          bld_oem);
     }
 
     acpi_add_table(table_offsets, tables_blob);
-    build_waet(tables_blob, tables->linker, pcms->oem_id, pcms->oem_table_id);
+    build_waet(tables_blob, tables->linker, bld_oem);
 
     /* Add tables supplied by user (if any) */
     for (u = acpi_table_first(); u; u = acpi_table_next(u)) {
@@ -2292,13 +2288,13 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
     /* RSDT is pointed to by RSDP */
     rsdt = tables_blob->len;
     build_rsdt(tables_blob, tables->linker, table_offsets,
-               oem_id, oem_table_id);
+               &slic_bld_oem);
 
     /* RSDP is in FSEG memory, so allocate it separately */
     {
         AcpiRsdpData rsdp_data = {
             .revision = 0,
-            .oem_id = pcms->oem_id,
+            .oem_id = bld_oem->oem_id,
             .xsdt_tbl_offset = NULL,
             .rsdt_tbl_offset = &rsdt,
         };
diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
index 1f5947fcf9..c3a5e6f0b5 100644
--- a/hw/i386/acpi-common.c
+++ b/hw/i386/acpi-common.c
@@ -73,7 +73,7 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
 
 void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
                      X86MachineState *x86ms, AcpiDeviceIf *adev,
-                     const char *oem_id, const char *oem_table_id)
+                     struct AcpiBuildOem *bld_oem)
 {
     MachineClass *mc = MACHINE_GET_CLASS(x86ms);
     const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(x86ms));
@@ -158,6 +158,6 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
 
     build_header(linker, table_data,
                  (void *)(table_data->data + madt_start), "APIC",
-                 table_data->len - madt_start, 1, oem_id, oem_table_id);
+                 table_data->len - madt_start, 1, bld_oem);
 }
 
diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c
index 54b3af478a..a10851407f 100644
--- a/hw/i386/acpi-microvm.c
+++ b/hw/i386/acpi-microvm.c
@@ -149,7 +149,7 @@ build_dsdt_microvm(GArray *table_data, BIOSLinker *linker,
     g_array_append_vals(table_data, dsdt->buf->data, dsdt->buf->len);
     build_header(linker, table_data,
         (void *)(table_data->data + table_data->len - dsdt->buf->len),
-                 "DSDT", dsdt->buf->len, 2, mms->oem_id, mms->oem_table_id);
+                 "DSDT", dsdt->buf->len, 2, &mms->bld_oem);
     free_aml_allocator();
 }
 
@@ -201,24 +201,21 @@ static void acpi_build_microvm(AcpiBuildTables *tables,
     pmfadt.dsdt_tbl_offset = &dsdt;
     pmfadt.xdsdt_tbl_offset = &dsdt;
     acpi_add_table(table_offsets, tables_blob);
-    build_fadt(tables_blob, tables->linker, &pmfadt, mms->oem_id,
-               mms->oem_table_id);
+    build_fadt(tables_blob, tables->linker, &pmfadt, &mms->bld_oem);
 
     acpi_add_table(table_offsets, tables_blob);
     acpi_build_madt(tables_blob, tables->linker, X86_MACHINE(machine),
-                    ACPI_DEVICE_IF(x86ms->acpi_dev), mms->oem_id,
-                    mms->oem_table_id);
+                    ACPI_DEVICE_IF(x86ms->acpi_dev), &mms->bld_oem);
 
     xsdt = tables_blob->len;
-    build_xsdt(tables_blob, tables->linker, table_offsets, mms->oem_id,
-               mms->oem_table_id);
+    build_xsdt(tables_blob, tables->linker, table_offsets, &mms->bld_oem);
 
     /* RSDP is in FSEG memory, so allocate it separately */
     {
         AcpiRsdpData rsdp_data = {
             /* ACPI 2.0: 5.2.4.3 RSDP Structure */
             .revision = 2, /* xsdt needs v2 */
-            .oem_id = mms->oem_id,
+            .oem_id = mms->bld_oem.oem_id,
             .xsdt_tbl_offset = &xsdt,
             .rsdt_tbl_offset = NULL,
         };
diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 4e0cf4c522..1d9bbf2871 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -652,7 +652,7 @@ static char *microvm_machine_get_oem_id(Object *obj, Error **errp)
 {
     MicrovmMachineState *mms = MICROVM_MACHINE(obj);
 
-    return g_strdup(mms->oem_id);
+    return g_strdup(mms->bld_oem.oem_id);
 }
 
 static void microvm_machine_set_oem_id(Object *obj, const char *value,
@@ -661,21 +661,20 @@ static void microvm_machine_set_oem_id(Object *obj, const char *value,
     MicrovmMachineState *mms = MICROVM_MACHINE(obj);
     size_t len = strlen(value);
 
-    if (len > 6) {
+    if (len > ACPI_BUILD_OEM_ID_SIZE) {
         error_setg(errp,
           "User specified "MICROVM_MACHINE_OEM_ID" value is bigger than "
           "6 bytes in size");
         return;
     }
-
-    strncpy(mms->oem_id, value, 6);
+    ACPI_INIT_BUILD_OEM_ID(mms->bld_oem, value);
 }
 
 static char *microvm_machine_get_oem_table_id(Object *obj, Error **errp)
 {
     MicrovmMachineState *mms = MICROVM_MACHINE(obj);
 
-    return g_strdup(mms->oem_table_id);
+    return g_strdup(mms->bld_oem.oem_table_id);
 }
 
 static void microvm_machine_set_oem_table_id(Object *obj, const char *value,
@@ -684,13 +683,13 @@ static void microvm_machine_set_oem_table_id(Object *obj, const char *value,
     MicrovmMachineState *mms = MICROVM_MACHINE(obj);
     size_t len = strlen(value);
 
-    if (len > 8) {
+    if (len > ACPI_BUILD_OEM_TABLE_ID_SIZE) {
         error_setg(errp,
           "User specified "MICROVM_MACHINE_OEM_TABLE_ID" value is bigger than "
           "8 bytes in size");
         return;
     }
-    strncpy(mms->oem_table_id, value, 8);
+    ACPI_INIT_BUILD_OEM_TABLE_ID(mms->bld_oem, value);
 }
 
 static void microvm_machine_initfn(Object *obj)
@@ -715,8 +714,7 @@ static void microvm_machine_initfn(Object *obj)
     mms->powerdown_req.notify = microvm_powerdown_req;
     qemu_register_powerdown_notifier(&mms->powerdown_req);
 
-    mms->oem_id = g_strndup(ACPI_BUILD_APPNAME6, 6);
-    mms->oem_table_id = g_strndup(ACPI_BUILD_APPNAME8, 8);
+    ACPI_INIT_DEFAULT_BUILD_OEM(mms->bld_oem);
 }
 
 static void microvm_class_init(ObjectClass *oc, void *data)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 8aa85dec54..f0f0f58b97 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1615,7 +1615,7 @@ static char *pc_machine_get_oem_id(Object *obj, Error **errp)
 {
     PCMachineState *pcms = PC_MACHINE(obj);
 
-    return g_strdup(pcms->oem_id);
+    return g_strdup(pcms->bld_oem.oem_id);
 }
 
 static void pc_machine_set_oem_id(Object *obj, const char *value, Error **errp)
@@ -1623,21 +1623,20 @@ static void pc_machine_set_oem_id(Object *obj, const char *value, Error **errp)
     PCMachineState *pcms = PC_MACHINE(obj);
     size_t len = strlen(value);
 
-    if (len > 6) {
+    if (len > ACPI_BUILD_OEM_ID_SIZE) {
         error_setg(errp,
           "User specified "PC_MACHINE_OEM_ID" value is bigger than "
           "6 bytes in size");
         return;
     }
-
-    strncpy(pcms->oem_id, value, 6);
+    ACPI_INIT_BUILD_OEM_ID(pcms->bld_oem, value);
 }
 
 static char *pc_machine_get_oem_table_id(Object *obj, Error **errp)
 {
     PCMachineState *pcms = PC_MACHINE(obj);
 
-    return g_strdup(pcms->oem_table_id);
+    return g_strdup(pcms->bld_oem.oem_table_id);
 }
 
 static void pc_machine_set_oem_table_id(Object *obj, const char *value,
@@ -1646,13 +1645,13 @@ static void pc_machine_set_oem_table_id(Object *obj, const char *value,
     PCMachineState *pcms = PC_MACHINE(obj);
     size_t len = strlen(value);
 
-    if (len > 8) {
+    if (len > ACPI_BUILD_OEM_TABLE_ID_SIZE) {
         error_setg(errp,
           "User specified "PC_MACHINE_OEM_TABLE_ID" value is bigger than "
           "8 bytes in size");
         return;
     }
-    strncpy(pcms->oem_table_id, value, 8);
+    ACPI_INIT_BUILD_OEM_TABLE_ID(pcms->bld_oem, value);
 }
 
 static void pc_machine_initfn(Object *obj)
@@ -1667,8 +1666,7 @@ static void pc_machine_initfn(Object *obj)
     pcms->max_ram_below_4g = 0; /* use default */
     /* acpi build is enabled by default if machine supports it */
     pcms->acpi_build_enabled = PC_MACHINE_GET_CLASS(pcms)->has_acpi_build;
-    pcms->oem_id = g_strndup(ACPI_BUILD_APPNAME6, 6);
-    pcms->oem_table_id = g_strndup(ACPI_BUILD_APPNAME8, 8);
+    ACPI_INIT_DEFAULT_BUILD_OEM(pcms->bld_oem);
     pcms->smbus_enabled = true;
     pcms->sata_enabled = true;
     pcms->pit_enabled = true;
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] acpi: Remove duplicated code handling OEM ID and OEM table ID fields
  2021-02-13 10:22 [PATCH] acpi: Remove duplicated code handling OEM ID and OEM table ID fields Marian Postevca
@ 2021-02-15 17:44 ` Igor Mammedov
  2021-02-15 18:26   ` Marian Postevca
  0 siblings, 1 reply; 4+ messages in thread
From: Igor Mammedov @ 2021-02-15 17:44 UTC (permalink / raw)
  To: Marian Postevca
  Cc: Peter Maydell, Xiao Guangrong, Sergio Lopez, Michael S. Tsirkin,
	Richard Henderson, qemu-devel, Dongjiu Geng, Shannon Zhao,
	Xiang Zheng, open list:ACPI/HEST/GHES, Paolo Bonzini,
	Eduardo Habkost

On Sat, 13 Feb 2021 12:22:33 +0200
Marian Postevca <posteuca@mutex.one> wrote:

> The code handling OEM ID and OEM table ID fields had some duplicated
> code and was using unnecessary dynamically allocated memory.
> This change fixes that.
> 
> Signed-off-by: Marian Postevca <posteuca@mutex.one>
[...]
>  24 files changed, 190 insertions(+), 159 deletions(-)
hmm, looks like adding instead of removing

[...]
> --- a/include/hw/i386/microvm.h
> +++ b/include/hw/i386/microvm.h
> @@ -25,6 +25,7 @@
>  #include "hw/boards.h"
>  #include "hw/i386/x86.h"
>  #include "hw/acpi/acpi_dev_interface.h"
> +#include "hw/acpi/acpi-build-oem.h"
>  #include "hw/pci-host/gpex.h"
>  #include "qom/object.h"
>  
> @@ -106,8 +107,7 @@ struct MicrovmMachineState {
>      Notifier machine_done;
>      Notifier powerdown_req;
>      struct GPEXConfig gpex;
> -    char *oem_id;
> -    char *oem_table_id;
> +    struct AcpiBuildOem bld_oem;
have you considered, putting this field into X86MachineState?
(that way you will be able to handle both PC and microvm in one place,
without duplication io init/property setters)

>  };
>  
>  #define TYPE_MICROVM_MACHINE   MACHINE_TYPE_NAME("microvm")




^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] acpi: Remove duplicated code handling OEM ID and OEM table ID fields
  2021-02-15 17:44 ` Igor Mammedov
@ 2021-02-15 18:26   ` Marian Postevca
  2021-02-16 22:11     ` Igor Mammedov
  0 siblings, 1 reply; 4+ messages in thread
From: Marian Postevca @ 2021-02-15 18:26 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: Peter Maydell, Xiao Guangrong, Sergio Lopez, Michael S. Tsirkin,
	Richard Henderson, qemu-devel, Dongjiu Geng, Shannon Zhao,
	Xiang Zheng, open list:ACPI/HEST/GHES, Paolo Bonzini,
	Eduardo Habkost

Igor Mammedov <imammedo@redhat.com> writes:

> hmm, looks like adding instead of removing
>
Do you mean that the commit message does not describe the change
correctly, or that my refactoring is too extreme?

If it is the latter, I think I tried to simplify things, by creating
macros to be used in multiple places where this structure is created.
And passing the structure around instead of two parameters seemed simpler.

> have you considered, putting this field into X86MachineState?
> (that way you will be able to handle both PC and microvm in one place,
> without duplication io init/property setters)
>
I did not, will try this approach.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] acpi: Remove duplicated code handling OEM ID and OEM table ID fields
  2021-02-15 18:26   ` Marian Postevca
@ 2021-02-16 22:11     ` Igor Mammedov
  0 siblings, 0 replies; 4+ messages in thread
From: Igor Mammedov @ 2021-02-16 22:11 UTC (permalink / raw)
  To: Marian Postevca
  Cc: Peter Maydell, Xiao Guangrong, Sergio Lopez, Michael S. Tsirkin,
	Richard Henderson, qemu-devel, Dongjiu Geng, Shannon Zhao,
	Xiang Zheng, open list:ACPI/HEST/GHES, Paolo Bonzini,
	Eduardo Habkost

On Mon, 15 Feb 2021 20:26:10 +0200
Marian Postevca <posteuca@mutex.one> wrote:

> Igor Mammedov <imammedo@redhat.com> writes:
> 
> > hmm, looks like adding instead of removing
> >  
> Do you mean that the commit message does not describe the change
> correctly, or that my refactoring is too extreme?
I've meant that diff-stat shows that patch adds more lines than it removes
so I'm not sure it removes duplication.
Maybe rephrase commit message a little.


> If it is the latter, I think I tried to simplify things, by creating
> macros to be used in multiple places where this structure is created.
> And passing the structure around instead of two parameters seemed simpler.
> 
> > have you considered, putting this field into X86MachineState?
> > (that way you will be able to handle both PC and microvm in one place,
> > without duplication io init/property setters)
> >  
> I did not, will try this approach.
this would be pure deduplication, and simpler if you do it as separate patch
and then re-factoring to static fields on top of that.
(i.e. don't mix re-factoring and deduplication)



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-02-16 22:13 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-02-13 10:22 [PATCH] acpi: Remove duplicated code handling OEM ID and OEM table ID fields Marian Postevca
2021-02-15 17:44 ` Igor Mammedov
2021-02-15 18:26   ` Marian Postevca
2021-02-16 22:11     ` Igor Mammedov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).