[Qemu-devel] [PATCH 0/4] set the OEM fields in the RSDT and the FADT from the SLIC

[Qemu-devel] [PATCH 0/4] set the OEM fields in the RSDT and the FADT from the SLIC

[Laszlo Ersek]

Background:
- https://bugzilla.redhat.com/show_bug.cgi?id=1248758
- http://thread.gmane.org/gmane.comp.emulators.qemu/358854
- https://github.com/tianocore/edk2/issues/5

This matters mostly for Win7 installations that were converted with p2v.
Testing feedback from such environments would be appreciated.

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Aleksei Kovura <alex3kov@zoho.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Michael Tokarev <mjt@tls.msk.ru>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard W.M. Jones <rjones@redhat.com>
Cc: Shannon Zhao <zhaoshenglong@huawei.com>
Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com>

Thanks
Laszlo

Laszlo Ersek (4):
  acpi: take oem_id in build_header(), optionally
  acpi: expose oem_id and oem_table_id in build_rsdt()
  acpi: stash the OEM ID and OEM Table ID fields from an external SLIC
    table
  pc: set the OEM fields in the RSDT and the FADT from the SLIC

 include/hw/acpi/acpi.h      |  2 ++
 include/hw/acpi/aml-build.h |  5 +++--
 include/hw/i386/pc.h        |  2 ++
 hw/acpi/aml-build.c         | 14 ++++++++++----
 hw/acpi/core.c              | 18 ++++++++++++++++++
 hw/acpi/nvdimm.c            |  4 ++--
 hw/arm/virt-acpi-build.c    | 14 +++++++-------
 hw/i386/acpi-build.c        | 40 +++++++++++++++++++++++++++-------------
 hw/i386/pc.c                | 19 +++++++++++++++++++
 qemu-options.hx             | 10 +++++++++-
 10 files changed, 99 insertions(+), 29 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/4] acpi: take oem_id in build_header(), optionally

[Laszlo Ersek]

This patch is the continuation of commit 8870ca0e94f2 ("acpi: support
specified oem table id for build_header"). It will allow us to control the
OEM ID field too in the SDT header.

Cc: "Michael S. Tsirkin" <mst@redhat.com> (supporter:ACPI/SMBIOS)
Cc: Igor Mammedov <imammedo@redhat.com> (supporter:ACPI/SMBIOS)
Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com> (maintainer:NVDIMM)
Cc: Shannon Zhao <zhaoshenglong@huawei.com> (maintainer:ARM ACPI Subsystem)
Cc: Paolo Bonzini <pbonzini@redhat.com> (maintainer:X86)
Cc: Richard W.M. Jones <rjones@redhat.com>
Cc: Aleksei Kovura <alex3kov@zoho.com>
Cc: Michael Tokarev <mjt@tls.msk.ru>
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1248758
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 include/hw/acpi/aml-build.h |  2 +-
 hw/acpi/aml-build.c         | 11 ++++++++---
 hw/acpi/nvdimm.c            |  4 ++--
 hw/arm/virt-acpi-build.c    | 12 ++++++------
 hw/i386/acpi-build.c        | 20 ++++++++++----------
 5 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 6d6f705..c460bdd 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -357,7 +357,7 @@ Aml *aml_sizeof(Aml *arg);
 void
 build_header(GArray *linker, GArray *table_data,
              AcpiTableHeader *h, const char *sig, int len, uint8_t rev,
-             const char *oem_table_id);
+             const char *oem_id, const char *oem_table_id);
 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);
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 78e1290..05b8bd0 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1430,12 +1430,17 @@ Aml *aml_alias(const char *source_object, const char *alias_object)
 void
 build_header(GArray *linker, GArray *table_data,
              AcpiTableHeader *h, const char *sig, int len, uint8_t rev,
-             const char *oem_table_id)
+             const char *oem_id, const char *oem_table_id)
 {
     memcpy(&h->signature, sig, 4);
     h->length = cpu_to_le32(len);
     h->revision = rev;
-    memcpy(h->oem_id, ACPI_BUILD_APPNAME6, 6);
+
+    if (oem_id) {
+        strncpy((char *)h->oem_id, oem_id, sizeof h->oem_id);
+    } else {
+        memcpy(h->oem_id, ACPI_BUILD_APPNAME6, 6);
+    }
 
     if (oem_table_id) {
         strncpy((char *)h->oem_table_id, oem_table_id, sizeof(h->oem_table_id));
@@ -1510,5 +1515,5 @@ build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets)
                                        sizeof(uint32_t));
     }
     build_header(linker, table_data,
-                 (void *)rsdt, "RSDT", rsdt_len, 1, NULL);
+                 (void *)rsdt, "RSDT", rsdt_len, 1, NULL, NULL);
 }
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index df1b176..73749aa 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -365,7 +365,7 @@ static void nvdimm_build_nfit(GSList *device_list, GArray *table_offsets,
 
     build_header(linker, table_data,
                  (void *)(table_data->data + header), "NFIT",
-                 sizeof(NvdimmNfitHeader) + structures->len, 1, NULL);
+                 sizeof(NvdimmNfitHeader) + structures->len, 1, NULL, NULL);
     g_array_free(structures, true);
 }
 
@@ -470,7 +470,7 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets,
     g_array_append_vals(table_data, ssdt->buf->data, ssdt->buf->len);
     build_header(linker, table_data,
         (void *)(table_data->data + table_data->len - ssdt->buf->len),
-        "SSDT", ssdt->buf->len, 1, "NVDIMM");
+        "SSDT", ssdt->buf->len, 1, NULL, "NVDIMM");
     free_aml_allocator();
 }
 
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 0caf5ce..6b5aa12 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -424,7 +424,7 @@ build_spcr(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
     spcr->pci_vendor_id = 0xffff;  /* PCI Vendor ID: not a PCI device */
 
     build_header(linker, table_data, (void *)spcr, "SPCR", sizeof(*spcr), 2,
-                 NULL);
+                 NULL, NULL);
 }
 
 static void
@@ -443,7 +443,7 @@ build_mcfg(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
     mcfg->allocation[0].end_bus_number = (memmap[VIRT_PCIE_ECAM].size
                                           / PCIE_MMCFG_SIZE_MIN) - 1;
 
-    build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1, NULL);
+    build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1, NULL, NULL);
 }
 
 /* GTDT */
@@ -469,7 +469,7 @@ build_gtdt(GArray *table_data, GArray *linker)
 
     build_header(linker, table_data,
                  (void *)(table_data->data + gtdt_start), "GTDT",
-                 table_data->len - gtdt_start, 2, NULL);
+                 table_data->len - gtdt_start, 2, NULL, NULL);
 }
 
 /* MADT */
@@ -531,7 +531,7 @@ build_madt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info,
 
     build_header(linker, table_data,
                  (void *)(table_data->data + madt_start), "APIC",
-                 table_data->len - madt_start, 3, NULL);
+                 table_data->len - madt_start, 3, NULL, NULL);
 }
 
 /* FADT */
@@ -556,7 +556,7 @@ build_fadt(GArray *table_data, GArray *linker, unsigned dsdt)
                                    sizeof fadt->dsdt);
 
     build_header(linker, table_data,
-                 (void *)fadt, "FACP", sizeof(*fadt), 5, NULL);
+                 (void *)fadt, "FACP", sizeof(*fadt), 5, NULL, NULL);
 }
 
 /* DSDT */
@@ -592,7 +592,7 @@ build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
     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, NULL);
+        "DSDT", dsdt->buf->len, 2, NULL, NULL);
     free_aml_allocator();
 }
 
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 78758e2..e1ebd07 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -358,7 +358,7 @@ build_fadt(GArray *table_data, GArray *linker, AcpiPmInfo *pm,
     fadt_setup(fadt, pm);
 
     build_header(linker, table_data,
-                 (void *)fadt, "FACP", sizeof(*fadt), 1, NULL);
+                 (void *)fadt, "FACP", sizeof(*fadt), 1, NULL, NULL);
 }
 
 static void
@@ -428,7 +428,7 @@ build_madt(GArray *table_data, GArray *linker, AcpiCpuInfo *cpu,
 
     build_header(linker, table_data,
                  (void *)(table_data->data + madt_start), "APIC",
-                 table_data->len - madt_start, 1, NULL);
+                 table_data->len - madt_start, 1, NULL, NULL);
 }
 
 /* Assign BSEL property to all buses.  In the future, this can be changed
@@ -2197,7 +2197,7 @@ build_ssdt(GArray *table_data, GArray *linker,
     g_array_append_vals(table_data, ssdt->buf->data, ssdt->buf->len);
     build_header(linker, table_data,
         (void *)(table_data->data + table_data->len - ssdt->buf->len),
-        "SSDT", ssdt->buf->len, 1, NULL);
+        "SSDT", ssdt->buf->len, 1, NULL, NULL);
     free_aml_allocator();
 }
 
@@ -2213,7 +2213,7 @@ build_hpet(GArray *table_data, GArray *linker)
     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, NULL);
+                 (void *)hpet, "HPET", sizeof(*hpet), 1, NULL, NULL);
 }
 
 static void
@@ -2236,7 +2236,7 @@ build_tpm_tcpa(GArray *table_data, GArray *linker, GArray *tcpalog)
                                    sizeof(tcpa->log_area_start_address));
 
     build_header(linker, table_data,
-                 (void *)tcpa, "TCPA", sizeof(*tcpa), 2, NULL);
+                 (void *)tcpa, "TCPA", sizeof(*tcpa), 2, NULL, NULL);
 
     acpi_data_push(tcpalog, TPM_LOG_AREA_MINIMUM_SIZE);
 }
@@ -2253,7 +2253,7 @@ build_tpm2(GArray *table_data, GArray *linker)
     tpm2_ptr->start_method = cpu_to_le32(TPM2_START_METHOD_MMIO);
 
     build_header(linker, table_data,
-                 (void *)tpm2_ptr, "TPM2", sizeof(*tpm2_ptr), 4, NULL);
+                 (void *)tpm2_ptr, "TPM2", sizeof(*tpm2_ptr), 4, NULL, NULL);
 }
 
 typedef enum {
@@ -2367,7 +2367,7 @@ build_srat(GArray *table_data, GArray *linker, PcGuestInfo *guest_info)
     build_header(linker, table_data,
                  (void *)(table_data->data + srat_start),
                  "SRAT",
-                 table_data->len - srat_start, 1, NULL);
+                 table_data->len - srat_start, 1, NULL, NULL);
 }
 
 static void
@@ -2396,7 +2396,7 @@ build_mcfg_q35(GArray *table_data, GArray *linker, AcpiMcfgInfo *info)
     } else {
         sig = "MCFG";
     }
-    build_header(linker, table_data, (void *)mcfg, sig, len, 1, NULL);
+    build_header(linker, table_data, (void *)mcfg, sig, len, 1, NULL, NULL);
 }
 
 static void
@@ -2420,7 +2420,7 @@ build_dmar_q35(GArray *table_data, GArray *linker)
     drhd->address = cpu_to_le64(Q35_HOST_BRIDGE_IOMMU_ADDR);
 
     build_header(linker, table_data, (void *)(table_data->data + dmar_start),
-                 "DMAR", table_data->len - dmar_start, 1, NULL);
+                 "DMAR", table_data->len - dmar_start, 1, NULL, NULL);
 }
 
 static void
@@ -2529,7 +2529,7 @@ build_dsdt(GArray *table_data, GArray *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, NULL);
+        "DSDT", dsdt->buf->len, 1, NULL, NULL);
     free_aml_allocator();
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/4] acpi: expose oem_id and oem_table_id in build_rsdt()

[Laszlo Ersek]

Since build_rsdt() is implemented as common utility code (in
"hw/acpi/aml-build.c"), it should expose -- and forward -- the oem_id and
oem_table_id parameters between board code and the generic build_header()
function.

Cc: "Michael S. Tsirkin" <mst@redhat.com> (supporter:ACPI/SMBIOS)
Cc: Igor Mammedov <imammedo@redhat.com> (supporter:ACPI/SMBIOS)
Cc: Shannon Zhao <zhaoshenglong@huawei.com> (maintainer:ARM ACPI Subsystem)
Cc: Paolo Bonzini <pbonzini@redhat.com> (maintainer:X86)
Cc: Richard W.M. Jones <rjones@redhat.com>
Cc: Aleksei Kovura <alex3kov@zoho.com>
Cc: Michael Tokarev <mjt@tls.msk.ru>
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1248758
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 include/hw/acpi/aml-build.h | 3 ++-
 hw/acpi/aml-build.c         | 5 +++--
 hw/arm/virt-acpi-build.c    | 2 +-
 hw/i386/acpi-build.c        | 2 +-
 4 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index c460bdd..aa29d30 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -364,6 +364,7 @@ void acpi_add_table(GArray *table_offsets, GArray *table_data);
 void acpi_build_tables_init(AcpiBuildTables *tables);
 void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre);
 void
-build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets);
+build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets,
+           const char *oem_id, const char *oem_table_id);
 
 #endif
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 05b8bd0..ce7fe81 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1496,7 +1496,8 @@ void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre)
 
 /* Build rsdt table */
 void
-build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets)
+build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets,
+           const char *oem_id, const char *oem_table_id)
 {
     AcpiRsdtDescriptorRev1 *rsdt;
     size_t rsdt_len;
@@ -1515,5 +1516,5 @@ build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets)
                                        sizeof(uint32_t));
     }
     build_header(linker, table_data,
-                 (void *)rsdt, "RSDT", rsdt_len, 1, NULL, NULL);
+                 (void *)rsdt, "RSDT", rsdt_len, 1, oem_id, oem_table_id);
 }
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 6b5aa12..61b6f9a 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -656,7 +656,7 @@ void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
 
     /* RSDT is pointed to by RSDP */
     rsdt = tables_blob->len;
-    build_rsdt(tables_blob, tables->linker, table_offsets);
+    build_rsdt(tables_blob, tables->linker, table_offsets, NULL, NULL);
 
     /* RSDP is in FSEG memory, so allocate it separately */
     build_rsdp(tables->rsdp, tables->linker, rsdt);
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index e1ebd07..6408362 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2705,7 +2705,7 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
 
     /* RSDT is pointed to by RSDP */
     rsdt = tables_blob->len;
-    build_rsdt(tables_blob, tables->linker, table_offsets);
+    build_rsdt(tables_blob, tables->linker, table_offsets, NULL, NULL);
 
     /* RSDP is in FSEG memory, so allocate it separately */
     build_rsdp(tables->rsdp, tables->linker, rsdt);
-- 
1.8.3.1

[Qemu-devel] [PATCH 3/4] acpi: stash the OEM ID and OEM Table ID fields from an external SLIC table

[Laszlo Ersek]

The SLIC table is not generated by QEMU. If the user specifies an external
one however, then board-specific code might want to adapt other,
auto-generated tables to it. This patch saves the OEM ID and OEM Table ID
fields from the SLIC, and leaves the actual utilization to board code (the
next patch).

Cc: "Michael S. Tsirkin" <mst@redhat.com> (supporter:ACPI/SMBIOS)
Cc: Igor Mammedov <imammedo@redhat.com> (supporter:ACPI/SMBIOS)
Cc: Richard W.M. Jones <rjones@redhat.com>
Cc: Aleksei Kovura <alex3kov@zoho.com>
Cc: Michael Tokarev <mjt@tls.msk.ru>
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1248758
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 include/hw/acpi/acpi.h |  2 ++
 hw/acpi/core.c         | 18 ++++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
index b20bd55..407197a 100644
--- a/include/hw/acpi/acpi.h
+++ b/include/hw/acpi/acpi.h
@@ -189,6 +189,8 @@ void acpi_update_sci(ACPIREGS *acpi_regs, qemu_irq irq);
 extern int acpi_enabled;
 extern char unsigned *acpi_tables;
 extern size_t acpi_tables_len;
+extern char *acpi_slic_oem_id;
+extern char *acpi_slic_oem_table_id;
 
 uint8_t *acpi_table_first(void);
 uint8_t *acpi_table_next(uint8_t *current);
diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index 21e113d..7046035 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -54,6 +54,8 @@ static const char unsigned dfl_hdr[ACPI_TABLE_HDR_SIZE - ACPI_TABLE_PFX_SIZE] =
 
 char unsigned *acpi_tables;
 size_t acpi_tables_len;
+char *acpi_slic_oem_id;
+char *acpi_slic_oem_table_id;
 
 static QemuOptsList qemu_acpi_opts = {
     .name = "acpi",
@@ -227,6 +229,22 @@ static void acpi_table_install(const char unsigned *blob, size_t bloblen,
     /* recalculate checksum */
     ext_hdr->checksum = acpi_checksum((const char unsigned *)ext_hdr +
                                       ACPI_TABLE_PFX_SIZE, acpi_payload_size);
+
+    /* If the table signature is SLIC, stash the OEM ID and OEM Table ID
+     * fields, so we can later adapt the RSDT and the FADT.
+     */
+    if (memcmp(ext_hdr->sig, "SLIC", 4) == 0) {
+        g_free(acpi_slic_oem_id);
+        acpi_slic_oem_id = g_malloc(sizeof ext_hdr->oem_id + 1);
+        memcpy(acpi_slic_oem_id, ext_hdr->oem_id, sizeof ext_hdr->oem_id);
+        acpi_slic_oem_id[sizeof ext_hdr->oem_id] = '\0';
+
+        g_free(acpi_slic_oem_table_id);
+        acpi_slic_oem_table_id = g_malloc(sizeof ext_hdr->oem_table_id + 1);
+        memcpy(acpi_slic_oem_table_id, ext_hdr->oem_table_id,
+               sizeof ext_hdr->oem_table_id);
+        acpi_slic_oem_table_id[sizeof ext_hdr->oem_table_id] = '\0';
+    }
 }
 
 void acpi_table_add(const QemuOpts *opts, Error **errp)
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 3/4] acpi: stash the OEM ID and OEM Table ID fields from an external SLIC table

[Michael S. Tsirkin]

On Thu, Jan 14, 2016 at 02:36:56AM +0100, Laszlo Ersek wrote:
> The SLIC table is not generated by QEMU. If the user specifies an external
> one however, then board-specific code might want to adapt other,
> auto-generated tables to it. This patch saves the OEM ID and OEM Table ID
> fields from the SLIC, and leaves the actual utilization to board code (the
> next patch).
> 
> Cc: "Michael S. Tsirkin" <mst@redhat.com> (supporter:ACPI/SMBIOS)
> Cc: Igor Mammedov <imammedo@redhat.com> (supporter:ACPI/SMBIOS)
> Cc: Richard W.M. Jones <rjones@redhat.com>
> Cc: Aleksei Kovura <alex3kov@zoho.com>
> Cc: Michael Tokarev <mjt@tls.msk.ru>
> RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1248758
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  include/hw/acpi/acpi.h |  2 ++
>  hw/acpi/core.c         | 18 ++++++++++++++++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
> index b20bd55..407197a 100644
> --- a/include/hw/acpi/acpi.h
> +++ b/include/hw/acpi/acpi.h
> @@ -189,6 +189,8 @@ void acpi_update_sci(ACPIREGS *acpi_regs, qemu_irq irq);
>  extern int acpi_enabled;
>  extern char unsigned *acpi_tables;
>  extern size_t acpi_tables_len;
> +extern char *acpi_slic_oem_id;
> +extern char *acpi_slic_oem_table_id;
>  
>  uint8_t *acpi_table_first(void);
>  uint8_t *acpi_table_next(uint8_t *current);

This seems rather messy.
How about an API to find SLIC and return the IDs
from the installed tables?

> diff --git a/hw/acpi/core.c b/hw/acpi/core.c
> index 21e113d..7046035 100644
> --- a/hw/acpi/core.c
> +++ b/hw/acpi/core.c
> @@ -54,6 +54,8 @@ static const char unsigned dfl_hdr[ACPI_TABLE_HDR_SIZE - ACPI_TABLE_PFX_SIZE] =
>  
>  char unsigned *acpi_tables;
>  size_t acpi_tables_len;
> +char *acpi_slic_oem_id;
> +char *acpi_slic_oem_table_id;
>  
>  static QemuOptsList qemu_acpi_opts = {
>      .name = "acpi",
> @@ -227,6 +229,22 @@ static void acpi_table_install(const char unsigned *blob, size_t bloblen,
>      /* recalculate checksum */
>      ext_hdr->checksum = acpi_checksum((const char unsigned *)ext_hdr +
>                                        ACPI_TABLE_PFX_SIZE, acpi_payload_size);
> +
> +    /* If the table signature is SLIC, stash the OEM ID and OEM Table ID
> +     * fields, so we can later adapt the RSDT and the FADT.
> +     */
> +    if (memcmp(ext_hdr->sig, "SLIC", 4) == 0) {
> +        g_free(acpi_slic_oem_id);
> +        acpi_slic_oem_id = g_malloc(sizeof ext_hdr->oem_id + 1);
> +        memcpy(acpi_slic_oem_id, ext_hdr->oem_id, sizeof ext_hdr->oem_id);
> +        acpi_slic_oem_id[sizeof ext_hdr->oem_id] = '\0';
> +
> +        g_free(acpi_slic_oem_table_id);
> +        acpi_slic_oem_table_id = g_malloc(sizeof ext_hdr->oem_table_id + 1);
> +        memcpy(acpi_slic_oem_table_id, ext_hdr->oem_table_id,
> +               sizeof ext_hdr->oem_table_id);
> +        acpi_slic_oem_table_id[sizeof ext_hdr->oem_table_id] = '\0';
> +    }
>  }
>  
>  void acpi_table_add(const QemuOpts *opts, Error **errp)
> -- 
> 1.8.3.1
> 

Re: [Qemu-devel] [PATCH 3/4] acpi: stash the OEM ID and OEM Table ID fields from an external SLIC table

[Laszlo Ersek]

On 01/14/16 11:21, Michael S. Tsirkin wrote:
> On Thu, Jan 14, 2016 at 02:36:56AM +0100, Laszlo Ersek wrote:
>> The SLIC table is not generated by QEMU. If the user specifies an external
>> one however, then board-specific code might want to adapt other,
>> auto-generated tables to it. This patch saves the OEM ID and OEM Table ID
>> fields from the SLIC, and leaves the actual utilization to board code (the
>> next patch).
>>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com> (supporter:ACPI/SMBIOS)
>> Cc: Igor Mammedov <imammedo@redhat.com> (supporter:ACPI/SMBIOS)
>> Cc: Richard W.M. Jones <rjones@redhat.com>
>> Cc: Aleksei Kovura <alex3kov@zoho.com>
>> Cc: Michael Tokarev <mjt@tls.msk.ru>
>> RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1248758
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>  include/hw/acpi/acpi.h |  2 ++
>>  hw/acpi/core.c         | 18 ++++++++++++++++++
>>  2 files changed, 20 insertions(+)
>>
>> diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
>> index b20bd55..407197a 100644
>> --- a/include/hw/acpi/acpi.h
>> +++ b/include/hw/acpi/acpi.h
>> @@ -189,6 +189,8 @@ void acpi_update_sci(ACPIREGS *acpi_regs, qemu_irq irq);
>>  extern int acpi_enabled;
>>  extern char unsigned *acpi_tables;
>>  extern size_t acpi_tables_len;
>> +extern char *acpi_slic_oem_id;
>> +extern char *acpi_slic_oem_table_id;
>>  
>>  uint8_t *acpi_table_first(void);
>>  uint8_t *acpi_table_next(uint8_t *current);
> 
> This seems rather messy.
> How about an API to find SLIC and return the IDs
> from the installed tables?

If testing confirms this idea is worthwhile, I won't object to your
suggestion. I admit I don't readily recall how the externally provided
tables are represented in that big concatenated blob, but perhaps I can
look at acpi_table_first() / acpi_table_next(). (Or just stare at the
code longer.)

So: fair enough, thanks.

Laszlo

> 
>> diff --git a/hw/acpi/core.c b/hw/acpi/core.c
>> index 21e113d..7046035 100644
>> --- a/hw/acpi/core.c
>> +++ b/hw/acpi/core.c
>> @@ -54,6 +54,8 @@ static const char unsigned dfl_hdr[ACPI_TABLE_HDR_SIZE - ACPI_TABLE_PFX_SIZE] =
>>  
>>  char unsigned *acpi_tables;
>>  size_t acpi_tables_len;
>> +char *acpi_slic_oem_id;
>> +char *acpi_slic_oem_table_id;
>>  
>>  static QemuOptsList qemu_acpi_opts = {
>>      .name = "acpi",
>> @@ -227,6 +229,22 @@ static void acpi_table_install(const char unsigned *blob, size_t bloblen,
>>      /* recalculate checksum */
>>      ext_hdr->checksum = acpi_checksum((const char unsigned *)ext_hdr +
>>                                        ACPI_TABLE_PFX_SIZE, acpi_payload_size);
>> +
>> +    /* If the table signature is SLIC, stash the OEM ID and OEM Table ID
>> +     * fields, so we can later adapt the RSDT and the FADT.
>> +     */
>> +    if (memcmp(ext_hdr->sig, "SLIC", 4) == 0) {
>> +        g_free(acpi_slic_oem_id);
>> +        acpi_slic_oem_id = g_malloc(sizeof ext_hdr->oem_id + 1);
>> +        memcpy(acpi_slic_oem_id, ext_hdr->oem_id, sizeof ext_hdr->oem_id);
>> +        acpi_slic_oem_id[sizeof ext_hdr->oem_id] = '\0';
>> +
>> +        g_free(acpi_slic_oem_table_id);
>> +        acpi_slic_oem_table_id = g_malloc(sizeof ext_hdr->oem_table_id + 1);
>> +        memcpy(acpi_slic_oem_table_id, ext_hdr->oem_table_id,
>> +               sizeof ext_hdr->oem_table_id);
>> +        acpi_slic_oem_table_id[sizeof ext_hdr->oem_table_id] = '\0';
>> +    }
>>  }
>>  
>>  void acpi_table_add(const QemuOpts *opts, Error **errp)
>> -- 
>> 1.8.3.1
>>

[Qemu-devel] [PATCH 4/4] pc: set the OEM fields in the RSDT and the FADT from the SLIC

[Laszlo Ersek]

The Microsoft spec about the SLIC and MSDM ACPI tables at
<http://go.microsoft.com/fwlink/p/?LinkId=234834> requires the OEM ID and
OEM Table ID fields to be consistent between the SLIC and the RSDT/XSDT.
That further affects the FADT, because a similar match between the FADT
and the RSDT/XSDT is required by the ACPI spec in general.

The stashed SLIC OEM identifiers can be ignored with the new

  -machine heed-slic-oem=no

option.

Cc: "Michael S. Tsirkin" <mst@redhat.com> (supporter:ACPI/SMBIOS)
Cc: Igor Mammedov <imammedo@redhat.com> (supporter:ACPI/SMBIOS)
Cc: Paolo Bonzini <pbonzini@redhat.com> (maintainer:X86)
Cc: Richard W.M. Jones <rjones@redhat.com>
Cc: Aleksei Kovura <alex3kov@zoho.com>
Cc: Michael Tokarev <mjt@tls.msk.ru>
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1248758
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 include/hw/i386/pc.h |  2 ++
 hw/i386/acpi-build.c | 22 ++++++++++++++++++----
 hw/i386/pc.c         | 19 +++++++++++++++++++
 qemu-options.hx      | 10 +++++++++-
 4 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 588a33c..a762c29 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -56,6 +56,7 @@ struct PCMachineState {
     OnOffAuto vmport;
     OnOffAuto smm;
     bool nvdimm;
+    bool heed_slic_oem;
 
     /* RAM information (sizes, addresses, configuration): */
     ram_addr_t below_4g_mem_size, above_4g_mem_size;
@@ -67,6 +68,7 @@ struct PCMachineState {
 #define PC_MACHINE_VMPORT           "vmport"
 #define PC_MACHINE_SMM              "smm"
 #define PC_MACHINE_NVDIMM           "nvdimm"
+#define PC_MACHINE_HEED_SLIC_OEM    "heed-slic-oem"
 
 /**
  * PCMachineClass:
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 6408362..cf2aafc 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -337,7 +337,8 @@ static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)
 /* FADT */
 static void
 build_fadt(GArray *table_data, GArray *linker, AcpiPmInfo *pm,
-           unsigned facs, unsigned dsdt)
+           unsigned facs, unsigned dsdt,
+           const char *oem_id, const char *oem_table_id)
 {
     AcpiFadtDescriptorRev1 *fadt = acpi_data_push(table_data, sizeof(*fadt));
 
@@ -358,7 +359,7 @@ build_fadt(GArray *table_data, GArray *linker, AcpiPmInfo *pm,
     fadt_setup(fadt, pm);
 
     build_header(linker, table_data,
-                 (void *)fadt, "FACP", sizeof(*fadt), 1, NULL, NULL);
+                 (void *)fadt, "FACP", sizeof(*fadt), 1, oem_id, oem_table_id);
 }
 
 static void
@@ -2621,6 +2622,17 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
     uint8_t *u;
     size_t aml_len = 0;
     GArray *tables_blob = tables->table_data;
+    char *slic_oem_id = NULL;
+    char *slic_oem_table_id = NULL;
+    PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
+    bool heed_slic_oem = object_property_get_bool(OBJECT(pcms),
+                                                  PC_MACHINE_HEED_SLIC_OEM,
+                                                  &error_abort);
+
+    if (heed_slic_oem) {
+        slic_oem_id = acpi_slic_oem_id;
+        slic_oem_table_id = acpi_slic_oem_table_id;
+    }
 
     acpi_get_cpu_info(&cpu);
     acpi_get_pm_info(&pm);
@@ -2654,7 +2666,8 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
 
     /* ACPI tables pointed to by RSDT */
     acpi_add_table(table_offsets, tables_blob);
-    build_fadt(tables_blob, tables->linker, &pm, facs, dsdt);
+    build_fadt(tables_blob, tables->linker, &pm, facs, dsdt,
+               slic_oem_id, slic_oem_table_id);
 
     ssdt = tables_blob->len;
     acpi_add_table(table_offsets, tables_blob);
@@ -2705,7 +2718,8 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
 
     /* RSDT is pointed to by RSDP */
     rsdt = tables_blob->len;
-    build_rsdt(tables_blob, tables->linker, table_offsets, NULL, NULL);
+    build_rsdt(tables_blob, tables->linker, table_offsets,
+               slic_oem_id, slic_oem_table_id);
 
     /* RSDP is in FSEG memory, so allocate it separately */
     build_rsdp(tables->rsdp, tables->linker, rsdt);
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index c36b8cf..3e7a72a 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1887,6 +1887,20 @@ static void pc_machine_set_nvdimm(Object *obj, bool value, Error **errp)
     pcms->nvdimm = value;
 }
 
+static bool pc_machine_get_heed_slic_oem(Object *obj, Error **errp)
+{
+    PCMachineState *pcms = PC_MACHINE(obj);
+
+    return pcms->heed_slic_oem;
+}
+
+static void pc_machine_set_heed_slic_oem(Object *obj, bool value, Error **errp)
+{
+    PCMachineState *pcms = PC_MACHINE(obj);
+
+    pcms->heed_slic_oem = value;
+}
+
 static void pc_machine_initfn(Object *obj)
 {
     PCMachineState *pcms = PC_MACHINE(obj);
@@ -1926,6 +1940,11 @@ static void pc_machine_initfn(Object *obj)
     pcms->nvdimm = false;
     object_property_add_bool(obj, PC_MACHINE_NVDIMM, pc_machine_get_nvdimm,
                              pc_machine_set_nvdimm, &error_abort);
+
+    pcms->heed_slic_oem = true;
+    object_property_add_bool(obj, PC_MACHINE_HEED_SLIC_OEM,
+                             pc_machine_get_heed_slic_oem,
+                             pc_machine_set_heed_slic_oem, &error_abort);
 }
 
 static void pc_machine_reset(void)
diff --git a/qemu-options.hx b/qemu-options.hx
index 215d00d..e49964c 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -43,7 +43,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
     "                aes-key-wrap=on|off controls support for AES key wrapping (default=on)\n"
     "                dea-key-wrap=on|off controls support for DEA key wrapping (default=on)\n"
     "                suppress-vmdesc=on|off disables self-describing migration (default=off)\n"
-    "                nvdimm=on|off controls NVDIMM support (default=off)\n",
+    "                nvdimm=on|off controls NVDIMM support (default=off)\n"
+    "                heed_slic_oem=on|off adapts RSDT and FADT OEM identifiers to external SLIC (default=on)\n",
     QEMU_ARCH_ALL)
 STEXI
 @item -machine [type=]@var{name}[,prop=@var{value}[,...]]
@@ -84,6 +85,13 @@ controls whether DEA wrapping keys will be created to allow
 execution of DEA cryptographic functions.  The default is on.
 @item nvdimm=on|off
 Enables or disables NVDIMM support. The default is off.
+@item heed_slic_oem=on|off
+If the user provides an external SLIC ACPI table with the -acpitable option,
+then heed_slic_oem=on will adapt the OEM ID and OEM Table ID fields of the
+auto-generated RSDT and FADT tables to the same fields in the external SLIC.
+When heed_slic_oem is turned off, the RSDT and FADT tables will have general,
+QEMU-branded OEM ID and OEM Table ID values. The default is on. heed_slic_oem
+makes no difference if no SLIC table is provided by the user.
 @end table
 ETEXI
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 4/4] pc: set the OEM fields in the RSDT and the FADT from the SLIC

[Michael S. Tsirkin]

On Thu, Jan 14, 2016 at 02:36:57AM +0100, Laszlo Ersek wrote:
> The Microsoft spec about the SLIC and MSDM ACPI tables at
> <http://go.microsoft.com/fwlink/p/?LinkId=234834> requires the OEM ID and
> OEM Table ID fields to be consistent between the SLIC and the RSDT/XSDT.
> That further affects the FADT, because a similar match between the FADT
> and the RSDT/XSDT is required by the ACPI spec in general.
> 
> The stashed SLIC OEM identifiers can be ignored with the new
> 
>   -machine heed-slic-oem=no
> 
> option.

I'd prefer "use-slic-oem".

But do we really expect people to use this?
Less knobs would be better ...

> Cc: "Michael S. Tsirkin" <mst@redhat.com> (supporter:ACPI/SMBIOS)
> Cc: Igor Mammedov <imammedo@redhat.com> (supporter:ACPI/SMBIOS)
> Cc: Paolo Bonzini <pbonzini@redhat.com> (maintainer:X86)
> Cc: Richard W.M. Jones <rjones@redhat.com>
> Cc: Aleksei Kovura <alex3kov@zoho.com>
> Cc: Michael Tokarev <mjt@tls.msk.ru>
> RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1248758
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  include/hw/i386/pc.h |  2 ++
>  hw/i386/acpi-build.c | 22 ++++++++++++++++++----
>  hw/i386/pc.c         | 19 +++++++++++++++++++
>  qemu-options.hx      | 10 +++++++++-
>  4 files changed, 48 insertions(+), 5 deletions(-)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 588a33c..a762c29 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -56,6 +56,7 @@ struct PCMachineState {
>      OnOffAuto vmport;
>      OnOffAuto smm;
>      bool nvdimm;
> +    bool heed_slic_oem;
>  
>      /* RAM information (sizes, addresses, configuration): */
>      ram_addr_t below_4g_mem_size, above_4g_mem_size;
> @@ -67,6 +68,7 @@ struct PCMachineState {
>  #define PC_MACHINE_VMPORT           "vmport"
>  #define PC_MACHINE_SMM              "smm"
>  #define PC_MACHINE_NVDIMM           "nvdimm"
> +#define PC_MACHINE_HEED_SLIC_OEM    "heed-slic-oem"
>  
>  /**
>   * PCMachineClass:
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 6408362..cf2aafc 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -337,7 +337,8 @@ static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)
>  /* FADT */
>  static void
>  build_fadt(GArray *table_data, GArray *linker, AcpiPmInfo *pm,
> -           unsigned facs, unsigned dsdt)
> +           unsigned facs, unsigned dsdt,
> +           const char *oem_id, const char *oem_table_id)
>  {
>      AcpiFadtDescriptorRev1 *fadt = acpi_data_push(table_data, sizeof(*fadt));
>  
> @@ -358,7 +359,7 @@ build_fadt(GArray *table_data, GArray *linker, AcpiPmInfo *pm,
>      fadt_setup(fadt, pm);
>  
>      build_header(linker, table_data,
> -                 (void *)fadt, "FACP", sizeof(*fadt), 1, NULL, NULL);
> +                 (void *)fadt, "FACP", sizeof(*fadt), 1, oem_id, oem_table_id);
>  }
>  
>  static void
> @@ -2621,6 +2622,17 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>      uint8_t *u;
>      size_t aml_len = 0;
>      GArray *tables_blob = tables->table_data;
> +    char *slic_oem_id = NULL;
> +    char *slic_oem_table_id = NULL;
> +    PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
> +    bool heed_slic_oem = object_property_get_bool(OBJECT(pcms),
> +                                                  PC_MACHINE_HEED_SLIC_OEM,
> +                                                  &error_abort);
> +
> +    if (heed_slic_oem) {
> +        slic_oem_id = acpi_slic_oem_id;
> +        slic_oem_table_id = acpi_slic_oem_table_id;
> +    }
>  
>      acpi_get_cpu_info(&cpu);
>      acpi_get_pm_info(&pm);
> @@ -2654,7 +2666,8 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>  
>      /* ACPI tables pointed to by RSDT */
>      acpi_add_table(table_offsets, tables_blob);
> -    build_fadt(tables_blob, tables->linker, &pm, facs, dsdt);
> +    build_fadt(tables_blob, tables->linker, &pm, facs, dsdt,
> +               slic_oem_id, slic_oem_table_id);
>  
>      ssdt = tables_blob->len;
>      acpi_add_table(table_offsets, tables_blob);
> @@ -2705,7 +2718,8 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>  
>      /* RSDT is pointed to by RSDP */
>      rsdt = tables_blob->len;
> -    build_rsdt(tables_blob, tables->linker, table_offsets, NULL, NULL);
> +    build_rsdt(tables_blob, tables->linker, table_offsets,
> +               slic_oem_id, slic_oem_table_id);
>  
>      /* RSDP is in FSEG memory, so allocate it separately */
>      build_rsdp(tables->rsdp, tables->linker, rsdt);
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index c36b8cf..3e7a72a 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1887,6 +1887,20 @@ static void pc_machine_set_nvdimm(Object *obj, bool value, Error **errp)
>      pcms->nvdimm = value;
>  }
>  
> +static bool pc_machine_get_heed_slic_oem(Object *obj, Error **errp)
> +{
> +    PCMachineState *pcms = PC_MACHINE(obj);
> +
> +    return pcms->heed_slic_oem;
> +}
> +
> +static void pc_machine_set_heed_slic_oem(Object *obj, bool value, Error **errp)
> +{
> +    PCMachineState *pcms = PC_MACHINE(obj);
> +
> +    pcms->heed_slic_oem = value;
> +}
> +
>  static void pc_machine_initfn(Object *obj)
>  {
>      PCMachineState *pcms = PC_MACHINE(obj);
> @@ -1926,6 +1940,11 @@ static void pc_machine_initfn(Object *obj)
>      pcms->nvdimm = false;
>      object_property_add_bool(obj, PC_MACHINE_NVDIMM, pc_machine_get_nvdimm,
>                               pc_machine_set_nvdimm, &error_abort);
> +
> +    pcms->heed_slic_oem = true;
> +    object_property_add_bool(obj, PC_MACHINE_HEED_SLIC_OEM,
> +                             pc_machine_get_heed_slic_oem,
> +                             pc_machine_set_heed_slic_oem, &error_abort);
>  }
>  
>  static void pc_machine_reset(void)
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 215d00d..e49964c 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -43,7 +43,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
>      "                aes-key-wrap=on|off controls support for AES key wrapping (default=on)\n"
>      "                dea-key-wrap=on|off controls support for DEA key wrapping (default=on)\n"
>      "                suppress-vmdesc=on|off disables self-describing migration (default=off)\n"
> -    "                nvdimm=on|off controls NVDIMM support (default=off)\n",
> +    "                nvdimm=on|off controls NVDIMM support (default=off)\n"
> +    "                heed_slic_oem=on|off adapts RSDT and FADT OEM identifiers to external SLIC (default=on)\n",
>      QEMU_ARCH_ALL)
>  STEXI
>  @item -machine [type=]@var{name}[,prop=@var{value}[,...]]
> @@ -84,6 +85,13 @@ controls whether DEA wrapping keys will be created to allow
>  execution of DEA cryptographic functions.  The default is on.
>  @item nvdimm=on|off
>  Enables or disables NVDIMM support. The default is off.
> +@item heed_slic_oem=on|off
> +If the user provides an external SLIC ACPI table with the -acpitable option,
> +then heed_slic_oem=on will adapt the OEM ID and OEM Table ID fields of the
> +auto-generated RSDT and FADT tables to the same fields in the external SLIC.
> +When heed_slic_oem is turned off, the RSDT and FADT tables will have general,
> +QEMU-branded OEM ID and OEM Table ID values. The default is on. heed_slic_oem
> +makes no difference if no SLIC table is provided by the user.
>  @end table
>  ETEXI
>  
> -- 
> 1.8.3.1

Re: [Qemu-devel] [PATCH 4/4] pc: set the OEM fields in the RSDT and the FADT from the SLIC

[Laszlo Ersek]

On 01/14/16 11:24, Michael S. Tsirkin wrote:
> On Thu, Jan 14, 2016 at 02:36:57AM +0100, Laszlo Ersek wrote:
>> The Microsoft spec about the SLIC and MSDM ACPI tables at
>> <http://go.microsoft.com/fwlink/p/?LinkId=234834> requires the OEM ID and
>> OEM Table ID fields to be consistent between the SLIC and the RSDT/XSDT.
>> That further affects the FADT, because a similar match between the FADT
>> and the RSDT/XSDT is required by the ACPI spec in general.
>>
>> The stashed SLIC OEM identifiers can be ignored with the new
>>
>>   -machine heed-slic-oem=no
>>
>> option.
> 
> I'd prefer "use-slic-oem".

That is, an option with the opposite meaning. Sounds doable.

Would you like me to give that option a default value that had the
equivalent effect? (That is, use-slic-oem=on by default?) If not, that
might cause further trouble for users (they would have to figure out one
more option).

> But do we really expect people to use this?
> Less knobs would be better ...

I don't disagree; I only did this because you had suggested it in the
earlier discussion:

http://thread.gmane.org/gmane.comp.emulators.qemu/358854/focus=359239

Perhaps I misunderstood. FWIW, the SLIC spec from Microsoft is
unambiguous about the OEM identity between SLIC and RSDT/XSDT (and
consequently the FADT): that requirement is not optional. Turning off
the OEM matching is therefore arguably useless, if the user provides a SLIC.

I can certainly drop this knob if you think that's best.

Thanks!
Laszlo

> 
>> Cc: "Michael S. Tsirkin" <mst@redhat.com> (supporter:ACPI/SMBIOS)
>> Cc: Igor Mammedov <imammedo@redhat.com> (supporter:ACPI/SMBIOS)
>> Cc: Paolo Bonzini <pbonzini@redhat.com> (maintainer:X86)
>> Cc: Richard W.M. Jones <rjones@redhat.com>
>> Cc: Aleksei Kovura <alex3kov@zoho.com>
>> Cc: Michael Tokarev <mjt@tls.msk.ru>
>> RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1248758
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>  include/hw/i386/pc.h |  2 ++
>>  hw/i386/acpi-build.c | 22 ++++++++++++++++++----
>>  hw/i386/pc.c         | 19 +++++++++++++++++++
>>  qemu-options.hx      | 10 +++++++++-
>>  4 files changed, 48 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>> index 588a33c..a762c29 100644
>> --- a/include/hw/i386/pc.h
>> +++ b/include/hw/i386/pc.h
>> @@ -56,6 +56,7 @@ struct PCMachineState {
>>      OnOffAuto vmport;
>>      OnOffAuto smm;
>>      bool nvdimm;
>> +    bool heed_slic_oem;
>>  
>>      /* RAM information (sizes, addresses, configuration): */
>>      ram_addr_t below_4g_mem_size, above_4g_mem_size;
>> @@ -67,6 +68,7 @@ struct PCMachineState {
>>  #define PC_MACHINE_VMPORT           "vmport"
>>  #define PC_MACHINE_SMM              "smm"
>>  #define PC_MACHINE_NVDIMM           "nvdimm"
>> +#define PC_MACHINE_HEED_SLIC_OEM    "heed-slic-oem"
>>  
>>  /**
>>   * PCMachineClass:
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index 6408362..cf2aafc 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -337,7 +337,8 @@ static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)
>>  /* FADT */
>>  static void
>>  build_fadt(GArray *table_data, GArray *linker, AcpiPmInfo *pm,
>> -           unsigned facs, unsigned dsdt)
>> +           unsigned facs, unsigned dsdt,
>> +           const char *oem_id, const char *oem_table_id)
>>  {
>>      AcpiFadtDescriptorRev1 *fadt = acpi_data_push(table_data, sizeof(*fadt));
>>  
>> @@ -358,7 +359,7 @@ build_fadt(GArray *table_data, GArray *linker, AcpiPmInfo *pm,
>>      fadt_setup(fadt, pm);
>>  
>>      build_header(linker, table_data,
>> -                 (void *)fadt, "FACP", sizeof(*fadt), 1, NULL, NULL);
>> +                 (void *)fadt, "FACP", sizeof(*fadt), 1, oem_id, oem_table_id);
>>  }
>>  
>>  static void
>> @@ -2621,6 +2622,17 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>>      uint8_t *u;
>>      size_t aml_len = 0;
>>      GArray *tables_blob = tables->table_data;
>> +    char *slic_oem_id = NULL;
>> +    char *slic_oem_table_id = NULL;
>> +    PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
>> +    bool heed_slic_oem = object_property_get_bool(OBJECT(pcms),
>> +                                                  PC_MACHINE_HEED_SLIC_OEM,
>> +                                                  &error_abort);
>> +
>> +    if (heed_slic_oem) {
>> +        slic_oem_id = acpi_slic_oem_id;
>> +        slic_oem_table_id = acpi_slic_oem_table_id;
>> +    }
>>  
>>      acpi_get_cpu_info(&cpu);
>>      acpi_get_pm_info(&pm);
>> @@ -2654,7 +2666,8 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>>  
>>      /* ACPI tables pointed to by RSDT */
>>      acpi_add_table(table_offsets, tables_blob);
>> -    build_fadt(tables_blob, tables->linker, &pm, facs, dsdt);
>> +    build_fadt(tables_blob, tables->linker, &pm, facs, dsdt,
>> +               slic_oem_id, slic_oem_table_id);
>>  
>>      ssdt = tables_blob->len;
>>      acpi_add_table(table_offsets, tables_blob);
>> @@ -2705,7 +2718,8 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>>  
>>      /* RSDT is pointed to by RSDP */
>>      rsdt = tables_blob->len;
>> -    build_rsdt(tables_blob, tables->linker, table_offsets, NULL, NULL);
>> +    build_rsdt(tables_blob, tables->linker, table_offsets,
>> +               slic_oem_id, slic_oem_table_id);
>>  
>>      /* RSDP is in FSEG memory, so allocate it separately */
>>      build_rsdp(tables->rsdp, tables->linker, rsdt);
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index c36b8cf..3e7a72a 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -1887,6 +1887,20 @@ static void pc_machine_set_nvdimm(Object *obj, bool value, Error **errp)
>>      pcms->nvdimm = value;
>>  }
>>  
>> +static bool pc_machine_get_heed_slic_oem(Object *obj, Error **errp)
>> +{
>> +    PCMachineState *pcms = PC_MACHINE(obj);
>> +
>> +    return pcms->heed_slic_oem;
>> +}
>> +
>> +static void pc_machine_set_heed_slic_oem(Object *obj, bool value, Error **errp)
>> +{
>> +    PCMachineState *pcms = PC_MACHINE(obj);
>> +
>> +    pcms->heed_slic_oem = value;
>> +}
>> +
>>  static void pc_machine_initfn(Object *obj)
>>  {
>>      PCMachineState *pcms = PC_MACHINE(obj);
>> @@ -1926,6 +1940,11 @@ static void pc_machine_initfn(Object *obj)
>>      pcms->nvdimm = false;
>>      object_property_add_bool(obj, PC_MACHINE_NVDIMM, pc_machine_get_nvdimm,
>>                               pc_machine_set_nvdimm, &error_abort);
>> +
>> +    pcms->heed_slic_oem = true;
>> +    object_property_add_bool(obj, PC_MACHINE_HEED_SLIC_OEM,
>> +                             pc_machine_get_heed_slic_oem,
>> +                             pc_machine_set_heed_slic_oem, &error_abort);
>>  }
>>  
>>  static void pc_machine_reset(void)
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 215d00d..e49964c 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -43,7 +43,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
>>      "                aes-key-wrap=on|off controls support for AES key wrapping (default=on)\n"
>>      "                dea-key-wrap=on|off controls support for DEA key wrapping (default=on)\n"
>>      "                suppress-vmdesc=on|off disables self-describing migration (default=off)\n"
>> -    "                nvdimm=on|off controls NVDIMM support (default=off)\n",
>> +    "                nvdimm=on|off controls NVDIMM support (default=off)\n"
>> +    "                heed_slic_oem=on|off adapts RSDT and FADT OEM identifiers to external SLIC (default=on)\n",
>>      QEMU_ARCH_ALL)
>>  STEXI
>>  @item -machine [type=]@var{name}[,prop=@var{value}[,...]]
>> @@ -84,6 +85,13 @@ controls whether DEA wrapping keys will be created to allow
>>  execution of DEA cryptographic functions.  The default is on.
>>  @item nvdimm=on|off
>>  Enables or disables NVDIMM support. The default is off.
>> +@item heed_slic_oem=on|off
>> +If the user provides an external SLIC ACPI table with the -acpitable option,
>> +then heed_slic_oem=on will adapt the OEM ID and OEM Table ID fields of the
>> +auto-generated RSDT and FADT tables to the same fields in the external SLIC.
>> +When heed_slic_oem is turned off, the RSDT and FADT tables will have general,
>> +QEMU-branded OEM ID and OEM Table ID values. The default is on. heed_slic_oem
>> +makes no difference if no SLIC table is provided by the user.
>>  @end table
>>  ETEXI
>>  
>> -- 
>> 1.8.3.1

Re: [Qemu-devel] [PATCH 4/4] pc: set the OEM fields in the RSDT and the FADT from the SLIC

[Laszlo Ersek]

On 01/14/16 17:44, Laszlo Ersek wrote:
> On 01/14/16 11:24, Michael S. Tsirkin wrote:
>> On Thu, Jan 14, 2016 at 02:36:57AM +0100, Laszlo Ersek wrote:
>>> The Microsoft spec about the SLIC and MSDM ACPI tables at
>>> <http://go.microsoft.com/fwlink/p/?LinkId=234834> requires the OEM ID and
>>> OEM Table ID fields to be consistent between the SLIC and the RSDT/XSDT.
>>> That further affects the FADT, because a similar match between the FADT
>>> and the RSDT/XSDT is required by the ACPI spec in general.
>>>
>>> The stashed SLIC OEM identifiers can be ignored with the new
>>>
>>>   -machine heed-slic-oem=no
>>>
>>> option.
>>
>> I'd prefer "use-slic-oem".
> 
> That is, an option with the opposite meaning. Sounds doable.
> 
> Would you like me to give that option a default value that had the
> equivalent effect? (That is, use-slic-oem=on by default?) If not, that
> might cause further trouble for users (they would have to figure out one
> more option).

Ugh, nevermind. You suggested another option name, but with the *same*
meaning. Okay, that's really easy to change.

But the more foundational question below remains open.

Thanks
Laszlo

>> But do we really expect people to use this?
>> Less knobs would be better ...
> 
> I don't disagree; I only did this because you had suggested it in the
> earlier discussion:
> 
> http://thread.gmane.org/gmane.comp.emulators.qemu/358854/focus=359239
> 
> Perhaps I misunderstood. FWIW, the SLIC spec from Microsoft is
> unambiguous about the OEM identity between SLIC and RSDT/XSDT (and
> consequently the FADT): that requirement is not optional. Turning off
> the OEM matching is therefore arguably useless, if the user provides a SLIC.
> 
> I can certainly drop this knob if you think that's best.
> 
> Thanks!
> Laszlo
> 
>>
>>> Cc: "Michael S. Tsirkin" <mst@redhat.com> (supporter:ACPI/SMBIOS)
>>> Cc: Igor Mammedov <imammedo@redhat.com> (supporter:ACPI/SMBIOS)
>>> Cc: Paolo Bonzini <pbonzini@redhat.com> (maintainer:X86)
>>> Cc: Richard W.M. Jones <rjones@redhat.com>
>>> Cc: Aleksei Kovura <alex3kov@zoho.com>
>>> Cc: Michael Tokarev <mjt@tls.msk.ru>
>>> RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1248758
>>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>>> ---
>>>  include/hw/i386/pc.h |  2 ++
>>>  hw/i386/acpi-build.c | 22 ++++++++++++++++++----
>>>  hw/i386/pc.c         | 19 +++++++++++++++++++
>>>  qemu-options.hx      | 10 +++++++++-
>>>  4 files changed, 48 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>>> index 588a33c..a762c29 100644
>>> --- a/include/hw/i386/pc.h
>>> +++ b/include/hw/i386/pc.h
>>> @@ -56,6 +56,7 @@ struct PCMachineState {
>>>      OnOffAuto vmport;
>>>      OnOffAuto smm;
>>>      bool nvdimm;
>>> +    bool heed_slic_oem;
>>>  
>>>      /* RAM information (sizes, addresses, configuration): */
>>>      ram_addr_t below_4g_mem_size, above_4g_mem_size;
>>> @@ -67,6 +68,7 @@ struct PCMachineState {
>>>  #define PC_MACHINE_VMPORT           "vmport"
>>>  #define PC_MACHINE_SMM              "smm"
>>>  #define PC_MACHINE_NVDIMM           "nvdimm"
>>> +#define PC_MACHINE_HEED_SLIC_OEM    "heed-slic-oem"
>>>  
>>>  /**
>>>   * PCMachineClass:
>>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>>> index 6408362..cf2aafc 100644
>>> --- a/hw/i386/acpi-build.c
>>> +++ b/hw/i386/acpi-build.c
>>> @@ -337,7 +337,8 @@ static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)
>>>  /* FADT */
>>>  static void
>>>  build_fadt(GArray *table_data, GArray *linker, AcpiPmInfo *pm,
>>> -           unsigned facs, unsigned dsdt)
>>> +           unsigned facs, unsigned dsdt,
>>> +           const char *oem_id, const char *oem_table_id)
>>>  {
>>>      AcpiFadtDescriptorRev1 *fadt = acpi_data_push(table_data, sizeof(*fadt));
>>>  
>>> @@ -358,7 +359,7 @@ build_fadt(GArray *table_data, GArray *linker, AcpiPmInfo *pm,
>>>      fadt_setup(fadt, pm);
>>>  
>>>      build_header(linker, table_data,
>>> -                 (void *)fadt, "FACP", sizeof(*fadt), 1, NULL, NULL);
>>> +                 (void *)fadt, "FACP", sizeof(*fadt), 1, oem_id, oem_table_id);
>>>  }
>>>  
>>>  static void
>>> @@ -2621,6 +2622,17 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>>>      uint8_t *u;
>>>      size_t aml_len = 0;
>>>      GArray *tables_blob = tables->table_data;
>>> +    char *slic_oem_id = NULL;
>>> +    char *slic_oem_table_id = NULL;
>>> +    PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
>>> +    bool heed_slic_oem = object_property_get_bool(OBJECT(pcms),
>>> +                                                  PC_MACHINE_HEED_SLIC_OEM,
>>> +                                                  &error_abort);
>>> +
>>> +    if (heed_slic_oem) {
>>> +        slic_oem_id = acpi_slic_oem_id;
>>> +        slic_oem_table_id = acpi_slic_oem_table_id;
>>> +    }
>>>  
>>>      acpi_get_cpu_info(&cpu);
>>>      acpi_get_pm_info(&pm);
>>> @@ -2654,7 +2666,8 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>>>  
>>>      /* ACPI tables pointed to by RSDT */
>>>      acpi_add_table(table_offsets, tables_blob);
>>> -    build_fadt(tables_blob, tables->linker, &pm, facs, dsdt);
>>> +    build_fadt(tables_blob, tables->linker, &pm, facs, dsdt,
>>> +               slic_oem_id, slic_oem_table_id);
>>>  
>>>      ssdt = tables_blob->len;
>>>      acpi_add_table(table_offsets, tables_blob);
>>> @@ -2705,7 +2718,8 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>>>  
>>>      /* RSDT is pointed to by RSDP */
>>>      rsdt = tables_blob->len;
>>> -    build_rsdt(tables_blob, tables->linker, table_offsets, NULL, NULL);
>>> +    build_rsdt(tables_blob, tables->linker, table_offsets,
>>> +               slic_oem_id, slic_oem_table_id);
>>>  
>>>      /* RSDP is in FSEG memory, so allocate it separately */
>>>      build_rsdp(tables->rsdp, tables->linker, rsdt);
>>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>>> index c36b8cf..3e7a72a 100644
>>> --- a/hw/i386/pc.c
>>> +++ b/hw/i386/pc.c
>>> @@ -1887,6 +1887,20 @@ static void pc_machine_set_nvdimm(Object *obj, bool value, Error **errp)
>>>      pcms->nvdimm = value;
>>>  }
>>>  
>>> +static bool pc_machine_get_heed_slic_oem(Object *obj, Error **errp)
>>> +{
>>> +    PCMachineState *pcms = PC_MACHINE(obj);
>>> +
>>> +    return pcms->heed_slic_oem;
>>> +}
>>> +
>>> +static void pc_machine_set_heed_slic_oem(Object *obj, bool value, Error **errp)
>>> +{
>>> +    PCMachineState *pcms = PC_MACHINE(obj);
>>> +
>>> +    pcms->heed_slic_oem = value;
>>> +}
>>> +
>>>  static void pc_machine_initfn(Object *obj)
>>>  {
>>>      PCMachineState *pcms = PC_MACHINE(obj);
>>> @@ -1926,6 +1940,11 @@ static void pc_machine_initfn(Object *obj)
>>>      pcms->nvdimm = false;
>>>      object_property_add_bool(obj, PC_MACHINE_NVDIMM, pc_machine_get_nvdimm,
>>>                               pc_machine_set_nvdimm, &error_abort);
>>> +
>>> +    pcms->heed_slic_oem = true;
>>> +    object_property_add_bool(obj, PC_MACHINE_HEED_SLIC_OEM,
>>> +                             pc_machine_get_heed_slic_oem,
>>> +                             pc_machine_set_heed_slic_oem, &error_abort);
>>>  }
>>>  
>>>  static void pc_machine_reset(void)
>>> diff --git a/qemu-options.hx b/qemu-options.hx
>>> index 215d00d..e49964c 100644
>>> --- a/qemu-options.hx
>>> +++ b/qemu-options.hx
>>> @@ -43,7 +43,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
>>>      "                aes-key-wrap=on|off controls support for AES key wrapping (default=on)\n"
>>>      "                dea-key-wrap=on|off controls support for DEA key wrapping (default=on)\n"
>>>      "                suppress-vmdesc=on|off disables self-describing migration (default=off)\n"
>>> -    "                nvdimm=on|off controls NVDIMM support (default=off)\n",
>>> +    "                nvdimm=on|off controls NVDIMM support (default=off)\n"
>>> +    "                heed_slic_oem=on|off adapts RSDT and FADT OEM identifiers to external SLIC (default=on)\n",
>>>      QEMU_ARCH_ALL)
>>>  STEXI
>>>  @item -machine [type=]@var{name}[,prop=@var{value}[,...]]
>>> @@ -84,6 +85,13 @@ controls whether DEA wrapping keys will be created to allow
>>>  execution of DEA cryptographic functions.  The default is on.
>>>  @item nvdimm=on|off
>>>  Enables or disables NVDIMM support. The default is off.
>>> +@item heed_slic_oem=on|off
>>> +If the user provides an external SLIC ACPI table with the -acpitable option,
>>> +then heed_slic_oem=on will adapt the OEM ID and OEM Table ID fields of the
>>> +auto-generated RSDT and FADT tables to the same fields in the external SLIC.
>>> +When heed_slic_oem is turned off, the RSDT and FADT tables will have general,
>>> +QEMU-branded OEM ID and OEM Table ID values. The default is on. heed_slic_oem
>>> +makes no difference if no SLIC table is provided by the user.
>>>  @end table
>>>  ETEXI
>>>  
>>> -- 
>>> 1.8.3.1
> 
> 

Re: [Qemu-devel] [PATCH 0/4] set the OEM fields in the RSDT and the FADT from the SLIC

[Richard W.M. Jones]

On Thu, Jan 14, 2016 at 02:36:53AM +0100, Laszlo Ersek wrote:
> This matters mostly for Win7 installations that were converted with p2v.
> Testing feedback from such environments would be appreciated.

Just to note first that no RHEL customer has ever reported this bug to
us.

To test this is going to require a spare UEFI-capable physical machine
where I can install Windows 7, and I don't have such a machine.

I could test it using a virt-p2v conversion from a VM, but then I
guess we'd have to apply these patches to qemu before conversion, and
so it wouldn't be clear to me exactly what we were testing.

Anyway I'll keep an eye on this and if I get hardware capable of
testing it, then I'll do so.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

Re: [Qemu-devel] [PATCH 0/4] set the OEM fields in the RSDT and the FADT from the SLIC

[Alex]

[-- Attachment #1: Type: text/plain, Size: 1314 bytes --]

Richard, I just posted HW test results to 
https://bugzilla.redhat.com/show_bug.cgi?id=1248758.
Should I do it here instead?
I'm not a RHEL customer, but I am semi-responsible for raising this fuss :)

------------------------------------------------------------------------

*From:* Richard W.m. Jones
*Sent:* Thursday, January 14, 2016 1:03PM
*To:* Laszlo Ersek
*Cc:* Qemu-devel, Michael S. Tsirkin, Aleksei Kovura, Igor Mammedov, 
Michael Tokarev, Paolo Bonzini, Shannon Zhao, Xiao Guangrong
*Subject:* Re: [PATCH 0/4] set the OEM fields in the RSDT and the FADT 
from the SLIC

On Thu, Jan 14, 2016 at 02:36:53AM +0100, Laszlo Ersek wrote:

> This matters mostly for Win7 installations that were converted with p2v.
> Testing feedback from such environments would be appreciated.

Just to note first that no RHEL customer has ever reported this bug to
us.

To test this is going to require a spare UEFI-capable physical machine
where I can install Windows 7, and I don't have such a machine.

I could test it using a virt-p2v conversion from a VM, but then I
guess we'd have to apply these patches to qemu before conversion, and
so it wouldn't be clear to me exactly what we were testing.

Anyway I'll keep an eye on this and if I get hardware capable of
testing it, then I'll do so.

Rich.

/--Regards, Alex/

[-- Attachment #2: Type: text/html, Size: 1934 bytes --]

Re: [Qemu-devel] [PATCH 0/4] set the OEM fields in the RSDT and the FADT from the SLIC

[Richard W.M. Jones]

On Thu, Jan 14, 2016 at 01:06:05PM +0300, Alex wrote:
> Richard, I just posted HW test results to
> https://bugzilla.redhat.com/show_bug.cgi?id=1248758.
> Should I do it here instead?

I saw that.  Testing a virt-p2v conversion is a lot more involved.  It
would involve something like this:

(1) Install Win7 on a UEFI-based physical machine, ensuring that Win7
is using UEFI to boot (not CSM or BIOS).

(2) Install a recent Fedora on a second machine (second machine may be
a VM).  'dnf install virt-v2v' on this machine.

(3) Boot virt-p2v ISO (http://oirase.annexia.org/virt-p2v/) on the
first physical machine.  Perform a P2V conversion
(http://libguestfs.org/virt-p2v.1.html).

(4) Boot the converted Win7 VM on the target qemu.  Reproduce the
original bug.  We have never had the original bug reported to us by
any customer.

(5) Patch qemu on the target.

(6) Boot virt-p2v ISO again, and perform a second conversion.

(7) Verify that the bug (step 4) has been fixed.

While I was looking at Laszlo's patches just now, I realized that I
have an AMD box that uses UEFI (actually - it uses CSM right now, but
I think I can make it boot using pure UEFI).  I'll have to swap some
disks around but I may be able to try this out today or tomorrow if I
can find a spare hard disk.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

Re: [Qemu-devel] [PATCH 0/4] set the OEM fields in the RSDT and the FADT from the SLIC

[Laszlo Ersek]

On 01/14/16 11:23, Richard W.M. Jones wrote:
> On Thu, Jan 14, 2016 at 01:06:05PM +0300, Alex wrote:
>> Richard, I just posted HW test results to
>> https://bugzilla.redhat.com/show_bug.cgi?id=1248758.
>> Should I do it here instead?
> 
> I saw that.  Testing a virt-p2v conversion is a lot more involved.  It
> would involve something like this:
> 
> (1) Install Win7 on a UEFI-based physical machine, ensuring that Win7
> is using UEFI to boot (not CSM or BIOS).
> 
> (2) Install a recent Fedora on a second machine (second machine may be
> a VM).  'dnf install virt-v2v' on this machine.
> 
> (3) Boot virt-p2v ISO (http://oirase.annexia.org/virt-p2v/) on the
> first physical machine.  Perform a P2V conversion
> (http://libguestfs.org/virt-p2v.1.html).
> 
> (4) Boot the converted Win7 VM on the target qemu.  Reproduce the
> original bug.  We have never had the original bug reported to us by
> any customer.
> 
> (5) Patch qemu on the target.
> 
> (6) Boot virt-p2v ISO again, and perform a second conversion.
> 
> (7) Verify that the bug (step 4) has been fixed.

Very good description, thank you.

> While I was looking at Laszlo's patches just now, I realized that I
> have an AMD box that uses UEFI (actually - it uses CSM right now, but
> I think I can make it boot using pure UEFI).  I'll have to swap some
> disks around but I may be able to try this out today or tomorrow if I
> can find a spare hard disk.

That would be awesome, yes. Thanks!

Cheers
Laszlo

Re: [Qemu-devel] [PATCH 0/4] set the OEM fields in the RSDT and the FADT from the SLIC

[Richard W.M. Jones]

On Thu, Jan 14, 2016 at 05:35:21PM +0100, Laszlo Ersek wrote:
> On 01/14/16 11:23, Richard W.M. Jones wrote:
> > On Thu, Jan 14, 2016 at 01:06:05PM +0300, Alex wrote:
> >> Richard, I just posted HW test results to
> >> https://bugzilla.redhat.com/show_bug.cgi?id=1248758.
> >> Should I do it here instead?
> > 
> > I saw that.  Testing a virt-p2v conversion is a lot more involved.  It
> > would involve something like this:
> > 
> > (1) Install Win7 on a UEFI-based physical machine, ensuring that Win7
> > is using UEFI to boot (not CSM or BIOS).
> > 
> > (2) Install a recent Fedora on a second machine (second machine may be
> > a VM).  'dnf install virt-v2v' on this machine.
> > 
> > (3) Boot virt-p2v ISO (http://oirase.annexia.org/virt-p2v/) on the
> > first physical machine.  Perform a P2V conversion
> > (http://libguestfs.org/virt-p2v.1.html).
> > 
> > (4) Boot the converted Win7 VM on the target qemu.  Reproduce the
> > original bug.  We have never had the original bug reported to us by
> > any customer.
> > 
> > (5) Patch qemu on the target.
> > 
> > (6) Boot virt-p2v ISO again, and perform a second conversion.
> > 
> > (7) Verify that the bug (step 4) has been fixed.
> 
> Very good description, thank you.

I made a good faith attempt to test this, but my attempts to get
Windows installed failed at the first step.  I don't have an optical
drive (who does?) and the MSFT tool that I tried for converting the
MSDN-supplied Win7 ISO into a USB key resulted only in USB keys that
did not boot at all.  It'll need to wait until I find an optical
drive.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

Re: [Qemu-devel] [PATCH 0/4] set the OEM fields in the RSDT and the FADT from the SLIC

[Alex]

On 15/01/16 19:07, Richard W.M. Jones wrote:
> On Thu, Jan 14, 2016 at 05:35:21PM +0100, Laszlo Ersek wrote:
>> On 01/14/16 11:23, Richard W.M. Jones wrote:
>>> On Thu, Jan 14, 2016 at 01:06:05PM +0300, Alex wrote:
>>>> Richard, I just posted HW test results to
>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1248758.
>>>> Should I do it here instead?
>>>
>>> I saw that.  Testing a virt-p2v conversion is a lot more involved.  It
>>> would involve something like this:
>>>
>>> (1) Install Win7 on a UEFI-based physical machine, ensuring that Win7
>>> is using UEFI to boot (not CSM or BIOS).
>>>
>>> (2) Install a recent Fedora on a second machine (second machine may be
>>> a VM).  'dnf install virt-v2v' on this machine.
>>>
>>> (3) Boot virt-p2v ISO (http://oirase.annexia.org/virt-p2v/) on the
>>> first physical machine.  Perform a P2V conversion
>>> (http://libguestfs.org/virt-p2v.1.html).
>>>
>>> (4) Boot the converted Win7 VM on the target qemu.  Reproduce the
>>> original bug.  We have never had the original bug reported to us by
>>> any customer.
>>>
>>> (5) Patch qemu on the target.
>>>
>>> (6) Boot virt-p2v ISO again, and perform a second conversion.
>>>
>>> (7) Verify that the bug (step 4) has been fixed.
>>
>> Very good description, thank you.
>
> I made a good faith attempt to test this, but my attempts to get
> Windows installed failed at the first step.  I don't have an optical
> drive (who does?) and the MSFT tool that I tried for converting the
> MSDN-supplied Win7 ISO into a USB key resulted only in USB keys that
> did not boot at all.  It'll need to wait until I find an optical
> drive.
>
> Rich.
>

Had this problem as well - this program http://rufus.akeo.ie/ can create 
UEFI-bootable USB stick from Win7 ISO.
-- 
/--Regards, Alex/

Re: [Qemu-devel] [PATCH 0/4] set the OEM fields in the RSDT and the FADT from the SLIC

[Richard W.M. Jones]

On Thu, Jan 14, 2016 at 05:35:21PM +0100, Laszlo Ersek wrote:
> On 01/14/16 11:23, Richard W.M. Jones wrote:
> > On Thu, Jan 14, 2016 at 01:06:05PM +0300, Alex wrote:
> >> Richard, I just posted HW test results to
> >> https://bugzilla.redhat.com/show_bug.cgi?id=1248758.
> >> Should I do it here instead?
> > 
> > I saw that.  Testing a virt-p2v conversion is a lot more involved.  It
> > would involve something like this:
> > 
> > (1) Install Win7 on a UEFI-based physical machine, ensuring that Win7
> > is using UEFI to boot (not CSM or BIOS).
> > 
> > (2) Install a recent Fedora on a second machine (second machine may be
> > a VM).  'dnf install virt-v2v' on this machine.
> > 
> > (3) Boot virt-p2v ISO (http://oirase.annexia.org/virt-p2v/) on the
> > first physical machine.  Perform a P2V conversion
> > (http://libguestfs.org/virt-p2v.1.html).
> > 
> > (4) Boot the converted Win7 VM on the target qemu.  Reproduce the
> > original bug.  We have never had the original bug reported to us by
> > any customer.
> > 
> > (5) Patch qemu on the target.
> > 
> > (6) Boot virt-p2v ISO again, and perform a second conversion.
> > 
> > (7) Verify that the bug (step 4) has been fixed.
> 
> Very good description, thank you.
> 
> > While I was looking at Laszlo's patches just now, I realized that I
> > have an AMD box that uses UEFI (actually - it uses CSM right now, but
> > I think I can make it boot using pure UEFI).  I'll have to swap some
> > disks around but I may be able to try this out today or tomorrow if I
> > can find a spare hard disk.
> 
> That would be awesome, yes. Thanks!

I'm afraid I gave up on this -- did give it my best.  It turns out
that the machine that I thought supported UEFI boot does not.  I'll
keep an eye out for such a machine and test this in future.

All was not lost because I did discover a few bugs in virt-p2v along
the way:

https://github.com/libguestfs/libguestfs/commit/7e2f2b0b2410587b81fd42bf741e3a36a5e75f6f
https://github.com/libguestfs/libguestfs/commit/c3ebc0a83761c552e6c19163b6d87044ae1ca635
https://github.com/libguestfs/libguestfs/commit/3f376fa5137d73df681cc5eaaa9b5e4206d57fce
https://github.com/libguestfs/libguestfs/commit/d723b352f8c10fb5244f17889cf68e13dd85c037

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

Re: [Qemu-devel] [PATCH 0/4] set the OEM fields in the RSDT and the FADT from the SLIC

[Michael Tokarev]

21.01.2016 14:37, Richard W.M. Jones wrote:

> I'm afraid I gave up on this -- did give it my best.  It turns out
> that the machine that I thought supported UEFI boot does not.  I'll
> keep an eye out for such a machine and test this in future.

BTW, why do you guys refer to UEFI boot all the time?  The whole thing
is equally useful on traditional BIOS-booting machines too.  Windows7
uses the same mechanism (looking at RSDT) for offline activation no
matter if it is EFI system or not.

When I did first version of my patch (based on someone else's version,
but details escapes my memory already), there was no UEFI support for
qemu whatsoever, and it allowed me to use my OEM version of Windows7
inside a qemu virtual machine without second activation.

Thanks,

/mjt

Re: [Qemu-devel] [PATCH 0/4] set the OEM fields in the RSDT and the FADT from the SLIC

[Richard W.M. Jones]

On Thu, Jan 21, 2016 at 02:42:02PM +0300, Michael Tokarev wrote:
> 21.01.2016 14:37, Richard W.M. Jones wrote:
> 
> > I'm afraid I gave up on this -- did give it my best.  It turns out
> > that the machine that I thought supported UEFI boot does not.  I'll
> > keep an eye out for such a machine and test this in future.
> 
> BTW, why do you guys refer to UEFI boot all the time?  The whole thing
> is equally useful on traditional BIOS-booting machines too.  Windows7
> uses the same mechanism (looking at RSDT) for offline activation no
> matter if it is EFI system or not.
> 
> When I did first version of my patch (based on someone else's version,
> but details escapes my memory already), there was no UEFI support for
> qemu whatsoever, and it allowed me to use my OEM version of Windows7
> inside a qemu virtual machine without second activation.

Could really do with detailed steps to reproduce the problem.  We have
never observed it, and I know next to nothing about how "activation"
works.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

Re: [Qemu-devel] [PATCH 0/4] set the OEM fields in the RSDT and the FADT from the SLIC

[Laszlo Ersek]

On 01/21/16 12:42, Michael Tokarev wrote:
> 21.01.2016 14:37, Richard W.M. Jones wrote:
> 
>> I'm afraid I gave up on this -- did give it my best.  It turns out
>> that the machine that I thought supported UEFI boot does not.  I'll
>> keep an eye out for such a machine and test this in future.
> 
> BTW, why do you guys refer to UEFI boot all the time?  The whole thing
> is equally useful on traditional BIOS-booting machines too.  Windows7
> uses the same mechanism (looking at RSDT) for offline activation no
> matter if it is EFI system or not.
> 
> When I did first version of my patch (based on someone else's version,
> but details escapes my memory already), there was no UEFI support for
> qemu whatsoever, and it allowed me to use my OEM version of Windows7
> inside a qemu virtual machine without second activation.

I've been referring to UEFI because my interest was renewed in this
topic by the following OVMF problem report:

https://github.com/tianocore/edk2/issues/5

Of course, the issue is not specific to OVMF (that's why I posted the
patches for QEMU), but I guess it stuck in my mind. Sorry.

Laszlo