ACPI: Add -acpifadt to allow FADT revision changes

This patch allows FADT to be built with different revisions. When the revision is greater than 1.0, 64-bit address fields may also be filled.

Note that FADT revision 2 has never been defined by the ACPI specification. So this patch only adds an option -acpifadt to allow revision 1,3,5 to be configured by the users.

1. Tested by booting a linux image, the 64-bit addresses are correctly filled in the dumped FADT.
2. Tested by booting a Windows image, no boot failure can be seen.

Signed-off-by Lv Zheng <[email protected]>

---
History:

v2: Fix coding style issues.

---
 hw/i386/acpi-build.c   |   62 ++++++++++++++++++++++++++++++++++++++++++------
 include/hw/acpi/acpi.h |    1 +
 qemu-options.hx        |    9 +++++++
 vl.c                   |   23 ++++++++++++++++++
 4 files changed, 88 insertions(+), 7 deletions(-)

hw/i386/acpi-build.c +55 -7

index ce7cbc5..4479695 100644
@@ -276,8 +276,22 @@ build_facs(GArray *table_data, BIOSLinker *linker)
     facs->length = cpu_to_le32(sizeof(*facs));
 }
 
+/* GAS */
+static void
+build_gas(struct AcpiGenericAddress *gas, uint8_t space_id,
+          uint8_t bit_width, uint8_t bit_offset,
+          uint8_t access_width, uint64_t address)
+{
+    gas->space_id = space_id;
+    gas->bit_width = bit_width;
+    gas->bit_offset = bit_offset;
+    gas->access_width = access_width;
+    gas->address = address;
+}
+
 /* Load chipset information in FADT */
-static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)
+static void fadt_setup(AcpiFadtDescriptorRev5_1 *fadt, AcpiPmInfo *pm,
+                       uint8_t revision)
 {
     fadt->model = 1;
     fadt->reserved1 = 0;

@@ -309,6 +323,25 @@ static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)
         fadt->flags |= cpu_to_le32(1 << ACPI_FADT_F_FORCE_APIC_CLUSTER_MODEL);
     }
     fadt->century = RTC_CENTURY;
+
+    /* Build ACPI 2.0 (FADT version 3+) GAS fields */
+    if (revision >= 3) {
+        /* EVT, CNT, TMR register matches hw/acpi/core.c */
+        build_gas(&fadt->xpm1a_event_block, AML_SYSTEM_IO,
+                  32, 0, 0, cpu_to_le64(pm->io_base));
+        build_gas(&fadt->xpm1a_control_block, AML_SYSTEM_IO,
+                  16, 0, 0, cpu_to_le64(pm->io_base + 0x04));
+        build_gas(&fadt->xpm_timer_block, AML_SYSTEM_IO,
+                  32, 0, 0, cpu_to_le64(pm->io_base + 0x08));
+        build_gas(&fadt->xgpe0_block, AML_SYSTEM_IO,
+                  pm->gpe0_blk_len << 3, 0, 0, cpu_to_le64(pm->gpe0_blk));
+    }
+
+    /* Build dummy ACPI 5.0 fields */
+    if (revision >= 5) {
+        build_gas(&fadt->sleep_control, AML_SYSTEM_MEMORY, 0, 0, 0, 0);
+        build_gas(&fadt->sleep_status, AML_SYSTEM_MEMORY, 0, 0, 0, 0);
+    }
 }
 
 

@@ -316,9 +349,9 @@ static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)
 static void
 build_fadt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm,
            unsigned facs_tbl_offset, unsigned dsdt_tbl_offset,
-           const char *oem_id, const char *oem_table_id)
+           const char *oem_id, const char *oem_table_id, uint8_t revision)
 {
-    AcpiFadtDescriptorRev1 *fadt = acpi_data_push(table_data, sizeof(*fadt));
+    AcpiFadtDescriptorRev5_1 *fadt = acpi_data_push(table_data, sizeof(*fadt));
     unsigned fw_ctrl_offset = (char *)&fadt->firmware_ctrl - table_data->data;
     unsigned dsdt_entry_offset = (char *)&fadt->dsdt - table_data->data;
 

@@ -328,13 +361,28 @@ build_fadt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm,
         ACPI_BUILD_TABLE_FILE, facs_tbl_offset);
 
     /* DSDT address to be filled by Guest linker */
-    fadt_setup(fadt, pm);
     bios_linker_loader_add_pointer(linker,
         ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt->dsdt),
         ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
 
-    build_header(linker, table_data,
-                 (void *)fadt, "FACP", sizeof(*fadt), 1, oem_id, oem_table_id);
+    if (revision > 2) {
+        fw_ctrl_offset = (char *)&fadt->Xfacs - table_data->data;
+        dsdt_entry_offset = (char *)&fadt->Xdsdt - table_data->data;
+
+        /* FACS address to be filled by Guest linker */
+        bios_linker_loader_add_pointer(linker,
+            ACPI_BUILD_TABLE_FILE, fw_ctrl_offset, sizeof(fadt->Xfacs),
+            ACPI_BUILD_TABLE_FILE, facs_tbl_offset);
+
+        /* DSDT address to be filled by Guest linker */
+        bios_linker_loader_add_pointer(linker,
+            ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt->Xdsdt),
+            ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
+    }
+
+    fadt_setup(fadt, pm, revision);
+    build_header(linker, table_data, (void *)fadt, "FACP", sizeof(*fadt),
+                 revision, oem_id, oem_table_id);
 }
 
 void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,

@@ -2681,7 +2729,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
     fadt = tables_blob->len;
     acpi_add_table(table_offsets, tables_blob);
     build_fadt(tables_blob, tables->linker, &pm, facs, dsdt,
-               slic_oem.id, slic_oem.table_id);
+               slic_oem.id, slic_oem.table_id, acpi_fadt_rev);
     aml_len += tables_blob->len - fadt;
 
     acpi_add_table(table_offsets, tables_blob);

include/hw/acpi/acpi.h +1 -0

index 7b3d93c..63df38d 100644
@@ -173,6 +173,7 @@ void acpi_update_sci(ACPIREGS *acpi_regs, qemu_irq irq);
 
 /* acpi.c */
 extern int acpi_enabled;
+extern uint8_t acpi_fadt_rev;
 extern char unsigned *acpi_tables;
 extern size_t acpi_tables_len;
 

qemu-options.hx +9 -0

index a71aaf8..cff34f6 100644
@@ -1492,6 +1492,15 @@ STEXI
 Disable HPET support.
 ETEXI
 
+DEF("acpifadt", HAS_ARG, QEMU_OPTION_acpifadt, \
+    "-acpifadt  n    configure FADT revision number (1, 3, 5)\n",
+    QEMU_ARCH_ALL)
+STEXI
+@item -acpifadt @var{n}
+@findex -acpifadt
+Configure FADT revision number, default is 1.
+ETEXI
+
 DEF("acpitable", HAS_ARG, QEMU_OPTION_acpitable,
     "-acpitable [sig=str][,rev=n][,oem_id=str][,oem_table_id=str][,oem_rev=n][,asl_compiler_id=str][,asl_compiler_rev=n][,{data|file}=file1[:file2]...]\n"
     "                ACPI table description\n", QEMU_ARCH_I386)

vl.c +23 -0

index e7c2c62..220e36d 100644
@@ -64,6 +64,7 @@ int main(int argc, char **argv)
 #include "hw/bt.h"
 #include "sysemu/watchdog.h"
 #include "hw/smbios/smbios.h"
+#include "hw/acpi/acpi.h"
 #include "hw/xen/xen.h"
 #include "hw/qdev.h"
 #include "hw/loader.h"

@@ -158,6 +159,7 @@ int max_cpus = 1;
 int smp_cores = 1;
 int smp_threads = 1;
 int acpi_enabled = 1;
+uint8_t acpi_fadt_rev = 1;
 int no_hpet = 0;
 int fd_bootchk = 1;
 static int no_reboot;

@@ -2301,6 +2303,24 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, Error **errp)
     return 0;
 }
 
+static void fadt_parse(const char *arg)
+{
+    unsigned long rev;
+    int err;
+
+    err = qemu_strtoul(arg, NULL, 10, &rev);
+    if (err) {
+        error_printf("Invalid FADT revision.\n");
+        exit(1);
+    }
+    if (rev != 1 && rev != 3 && rev != 5) {
+        error_printf("Unsupported FADT revision %lu, "
+                     "please specify 1,3,5.\n", rev);
+        exit(1);
+    }
+    acpi_fadt_rev = rev;
+}
+
 static int device_help_func(void *opaque, QemuOpts *opts, Error **errp)
 {
     return qdev_device_help(opts);

@@ -3622,6 +3642,9 @@ int main(int argc, char **argv, char **envp)
                 qdev_prop_register_global(&slew_lost_ticks);
                 break;
             }
+            case QEMU_OPTION_acpifadt:
+                fadt_parse(optarg);
+                break;
             case QEMU_OPTION_acpitable:
                 opts = qemu_opts_parse_noisily(qemu_find_opts("acpi"),
                                                optarg, true);

From: Paolo Bonzini [mailto:[email protected]] On Behalf Of Paolo Bonzini
Subject: Re: [PATCH v2] ACPI: Add -acpifadt to allow FADT revision changes

Quoted 1 line · Lv Zheng

This patch allows FADT to be built with different revisions. When the

revision

Quoted 3 lines

is greater than 1.0, 64-bit address fields may also be filled.

Note that FADT revision 2 has never been defined by the ACPI

specification. So

Quoted 4 lines

this patch only adds an option -acpifadt to allow revision 1,3,5 to be configured by the users.

1. Tested by booting a linux image, the 64-bit addresses are correctly

filled

Quoted 4 lines

in the dumped FADT.
2. Tested by booting a Windows image, no boot failure can be seen.

Signed-off-by Lv Zheng <[email protected]>

Hi, please make this a suboption of -acpitable instead. We try not to add new command-line options without supporting them in QemuOpts too.

Paolo

Quoted 6 lines

---
History:

v2: Fix coding style issues.

---
 hw/i386/acpi-build.c   |   62

++++++++++++++++++++++++++++++++++++++++++------

hw/i386/acpi-build.c build_facs(GArray *table_data, BIOSLinker

Quoted 10 lines

 include/hw/acpi/acpi.h |    1 +
 qemu-options.hx        |    9 +++++++
 vl.c                   |   23 ++++++++++++++++++
 4 files changed, 88 insertions(+), 7 deletions(-)

hw/i386/acpi-build.c +0 -0

index ce7cbc5..4479695 100644
@@ -276,8 +276,22 @@ build_facs(GArray *table_data, BIOSLinker

*linker)

Quoted 9 lines

hw/i386/acpi-build.c +6 -0

     facs->length = cpu_to_le32(sizeof(*facs));
 }
 
+/* GAS */
+static void
+build_gas(struct AcpiGenericAddress *gas, uint8_t space_id,
+          uint8_t bit_width, uint8_t bit_offset,
+          uint8_t access_width, uint64_t address)
+{

hw/i386/acpi-build.c build_facs(GArray *table_data, BIOSLinker *linker)

Quoted 10 lines

hw/i386/acpi-build.c +8 -1

+    gas->space_id = space_id;
+    gas->bit_width = bit_width;
+    gas->bit_offset = bit_offset;
+    gas->access_width = access_width;
+    gas->address = address;
+}
+
 /* Load chipset information in FADT */
-static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)
+static void fadt_setup(AcpiFadtDescriptorRev5_1 *fadt, AcpiPmInfo

*pm,

hw/i386/acpi-build.c static void fadt_setup(AcpiFadtDescriptorRev1

Quoted 5 lines

hw/i386/acpi-build.c +1 -0

+                       uint8_t revision)
 {
     fadt->model = 1;
     fadt->reserved1 = 0;

@@ -309,6 +323,25 @@ static void fadt_setup(AcpiFadtDescriptorRev1

*fadt, AcpiPmInfo *pm)

hw/i386/acpi-build.c static void fadt_setup(AcpiFadtDescriptorRev1 · line 323

Quoted 1 line

hw/i386/acpi-build.c +0 -0

         fadt->flags |= cpu_to_le32(1 <<

ACPI_FADT_F_FORCE_APIC_CLUSTER_MODEL);

Quoted 9 lines

hw/i386/acpi-build.c +7 -0

     }
     fadt->century = RTC_CENTURY;
+
+    /* Build ACPI 2.0 (FADT version 3+) GAS fields */
+    if (revision >= 3) {
+        /* EVT, CNT, TMR register matches hw/acpi/core.c */
+        build_gas(&fadt->xpm1a_event_block, AML_SYSTEM_IO,
+                  32, 0, 0, cpu_to_le64(pm->io_base));
+        build_gas(&fadt->xpm1a_control_block, AML_SYSTEM_IO,

hw/i386/acpi-build.c static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)

Quoted 10 lines

hw/i386/acpi-build.c +10 -0

+                  16, 0, 0, cpu_to_le64(pm->io_base + 0x04));
+        build_gas(&fadt->xpm_timer_block, AML_SYSTEM_IO,
+                  32, 0, 0, cpu_to_le64(pm->io_base + 0x08));
+        build_gas(&fadt->xgpe0_block, AML_SYSTEM_IO,
+                  pm->gpe0_blk_len << 3, 0, 0, cpu_to_le64(pm->gpe0_blk));
+    }
+
+    /* Build dummy ACPI 5.0 fields */
+    if (revision >= 5) {
+        build_gas(&fadt->sleep_control, AML_SYSTEM_MEMORY, 0, 0, 0,

0);

hw/i386/acpi-build.c static void fadt_setup(AcpiFadtDescriptorRev1

Quoted 6 lines

hw/i386/acpi-build.c +2 -0

+        build_gas(&fadt->sleep_status, AML_SYSTEM_MEMORY, 0, 0, 0, 0);
+    }
 }
 
 

@@ -316,9 +349,9 @@ static void fadt_setup(AcpiFadtDescriptorRev1

*fadt, AcpiPmInfo *pm)

hw/i386/acpi-build.c static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)

Quoted 7 lines

hw/i386/acpi-build.c +1 -2

 static void
 build_fadt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm,
            unsigned facs_tbl_offset, unsigned dsdt_tbl_offset,
-           const char *oem_id, const char *oem_table_id)
+           const char *oem_id, const char *oem_table_id, uint8_t revision)
 {
-    AcpiFadtDescriptorRev1 *fadt = acpi_data_push(table_data,

sizeof(*fadt));

hw/i386/acpi-build.c static void fadt_setup(AcpiFadtDescriptorRev1 · line 354

Quoted 1 line

hw/i386/acpi-build.c +1 -0

+    AcpiFadtDescriptorRev5_1 *fadt = acpi_data_push(table_data,

sizeof(*fadt));

hw/i386/acpi-build.c build_fadt(GArray *table_data, BIOSLinker

Quoted 5 lines

hw/i386/acpi-build.c +0 -0

     unsigned fw_ctrl_offset = (char *)&fadt->firmware_ctrl - table_data-
 data;
     unsigned dsdt_entry_offset = (char *)&fadt->dsdt - table_data->data;

@@ -328,13 +361,28 @@ build_fadt(GArray *table_data, BIOSLinker

*linker, AcpiPmInfo *pm,

hw/i386/acpi-build.c void acpi_build(AcpiBuildTables *tables,

Quoted 32 lines

hw/i386/acpi-build.c +18 -3

         ACPI_BUILD_TABLE_FILE, facs_tbl_offset);
 
     /* DSDT address to be filled by Guest linker */
-    fadt_setup(fadt, pm);
     bios_linker_loader_add_pointer(linker,
         ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt->dsdt),
         ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
 
-    build_header(linker, table_data,
-                 (void *)fadt, "FACP", sizeof(*fadt), 1, oem_id, oem_table_id);
+    if (revision > 2) {
+        fw_ctrl_offset = (char *)&fadt->Xfacs - table_data->data;
+        dsdt_entry_offset = (char *)&fadt->Xdsdt - table_data->data;
+
+        /* FACS address to be filled by Guest linker */
+        bios_linker_loader_add_pointer(linker,
+            ACPI_BUILD_TABLE_FILE, fw_ctrl_offset, sizeof(fadt->Xfacs),
+            ACPI_BUILD_TABLE_FILE, facs_tbl_offset);
+
+        /* DSDT address to be filled by Guest linker */
+        bios_linker_loader_add_pointer(linker,
+            ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt->Xdsdt),
+            ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
+    }
+
+    fadt_setup(fadt, pm, revision);
+    build_header(linker, table_data, (void *)fadt, "FACP", sizeof(*fadt),
+                 revision, oem_id, oem_table_id);
 }
 
 void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,

@@ -2681,7 +2729,7 @@ void acpi_build(AcpiBuildTables *tables,

MachineState *machine)

include/hw/acpi/acpi.h void acpi_update_sci(ACPIREGS *acpi_regs,

Quoted 13 lines

hw/i386/acpi-build.c +1 -1

     fadt = tables_blob->len;
     acpi_add_table(table_offsets, tables_blob);
     build_fadt(tables_blob, tables->linker, &pm, facs, dsdt,
-               slic_oem.id, slic_oem.table_id);
+               slic_oem.id, slic_oem.table_id, acpi_fadt_rev);
     aml_len += tables_blob->len - fadt;
 
     acpi_add_table(table_offsets, tables_blob);

include/hw/acpi/acpi.h +0 -0

index 7b3d93c..63df38d 100644
@@ -173,6 +173,7 @@ void acpi_update_sci(ACPIREGS *acpi_regs,

qemu_irq irq);

Quoted 16 lines

include/hw/acpi/acpi.h +1 -0

 /* acpi.c */
 extern int acpi_enabled;
+extern uint8_t acpi_fadt_rev;
 extern char unsigned *acpi_tables;
 extern size_t acpi_tables_len;
 

qemu-options.hx +1 -0

index a71aaf8..cff34f6 100644
@@ -1492,6 +1492,15 @@ STEXI
 Disable HPET support.
 ETEXI
 
+DEF("acpifadt", HAS_ARG, QEMU_OPTION_acpifadt, \

qemu-options.hx STEXI · line 1505

Quoted 10 lines

qemu-options.hx +8 -0

+    "-acpifadt  n    configure FADT revision number (1, 3, 5)\n",
+    QEMU_ARCH_ALL)
+STEXI
+@item -acpifadt @var{n}
+@findex -acpifadt
+Configure FADT revision number, default is 1.
+ETEXI
+
 DEF("acpitable", HAS_ARG, QEMU_OPTION_acpitable,
     "-acpitable

[sig=str][,rev=n][,oem_id=str][,oem_table_id=str][,oem_rev=n][,asl_compi ler_id=str][,asl_compiler_rev=n][,{data|file}=file1[:file2]...]\n"

vl.c static int parse_fw_cfg(void *opaque,

Quoted 22 lines

qemu-options.hx +0 -0

     "                ACPI table description\n", QEMU_ARCH_I386)

vl.c +2 -0

index e7c2c62..220e36d 100644
@@ -64,6 +64,7 @@ int main(int argc, char **argv)
 #include "hw/bt.h"
 #include "sysemu/watchdog.h"
 #include "hw/smbios/smbios.h"
+#include "hw/acpi/acpi.h"
 #include "hw/xen/xen.h"
 #include "hw/qdev.h"
 #include "hw/loader.h"

@@ -158,6 +159,7 @@ int max_cpus = 1;
 int smp_cores = 1;
 int smp_threads = 1;
 int acpi_enabled = 1;
+uint8_t acpi_fadt_rev = 1;
 int no_hpet = 0;
 int fd_bootchk = 1;
 static int no_reboot;

@@ -2301,6 +2303,24 @@ static int parse_fw_cfg(void *opaque,

QemuOpts *opts, Error **errp)

Quoted 12 lines

vl.c +9 -0

     return 0;
 }
 
+static void fadt_parse(const char *arg)
+{
+    unsigned long rev;
+    int err;
+
+    err = qemu_strtoul(arg, NULL, 10, &rev);
+    if (err) {
+        error_printf("Invalid FADT revision.\n");
+        exit(1);

vl.c static int parse_fw_cfg(void *opaque, QemuOpts *opts, Error **errp)

Quoted 10 lines

vl.c +9 -0

+    }
+    if (rev != 1 && rev != 3 && rev != 5) {
+        error_printf("Unsupported FADT revision %lu, "
+                     "please specify 1,3,5.\n", rev);
+        exit(1);
+    }
+    acpi_fadt_rev = rev;
+}
+
 static int device_help_func(void *opaque, QemuOpts *opts, Error

**errp)

Quoted 13 lines

vl.c +3 -0

 {
     return qdev_device_help(opts);

@@ -3622,6 +3642,9 @@ int main(int argc, char **argv, char **envp)
                 qdev_prop_register_global(&slew_lost_ticks);
                 break;
             }
+            case QEMU_OPTION_acpifadt:
+                fadt_parse(optarg);
+                break;
             case QEMU_OPTION_acpitable:
                 opts = qemu_opts_parse_noisily(qemu_find_opts("acpi"),
                                                optarg, true);

This patch allows FADT to be built with different revisions. When the revision is greater than 1.0, 64-bit address fields may also be filled.

Note that FADT revision 2 has never been defined by the ACPI specification. So this patch only adds an option -acpifadt to allow revision 1,3,5 to be configured by the users.

1. Tested by booting a linux image, the 64-bit addresses are correctly filled in the dumped FADT.
2. Tested by booting a Windows image, no boot failure can be seen.

Signed-off-by Lv Zheng <[email protected]>

---
 hw/i386/acpi-build.c   |   62 ++++++++++++++++++++++++++++++++++++++++++------
 include/hw/acpi/acpi.h |    1 +
 qemu-options.hx        |    9 +++++++
 vl.c                   |   18 ++++++++++++++
 4 files changed, 83 insertions(+), 7 deletions(-)

hw/i386/acpi-build.c +55 -7

index ce7cbc5..1f664de 100644
@@ -276,8 +276,22 @@ build_facs(GArray *table_data, BIOSLinker *linker)
     facs->length = cpu_to_le32(sizeof(*facs));
 }
 
+/* GAS */
+static void
+build_gas(struct AcpiGenericAddress *gas, uint8_t space_id,
+	  uint8_t bit_width, uint8_t bit_offset,
+	  uint8_t access_width, uint64_t address)
+{
+    gas->space_id = space_id;
+    gas->bit_width = bit_width;
+    gas->bit_offset = bit_offset;
+    gas->access_width= access_width;
+    gas->address = address;
+}
+
 /* Load chipset information in FADT */
-static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)
+static void fadt_setup(AcpiFadtDescriptorRev5_1 *fadt, AcpiPmInfo *pm,
+                       uint8_t revision)
 {
     fadt->model = 1;
     fadt->reserved1 = 0;

@@ -309,6 +323,25 @@ static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)
         fadt->flags |= cpu_to_le32(1 << ACPI_FADT_F_FORCE_APIC_CLUSTER_MODEL);
     }
     fadt->century = RTC_CENTURY;
+
+    /* Build ACPI 2.0 (FADT version 3+) GAS fields */
+    if (revision >= 3) {
+        /* EVT, CNT, TMR register matches hw/acpi/core.c */
+        build_gas(&fadt->xpm1a_event_block, AML_SYSTEM_IO,
+                  32, 0, 0, cpu_to_le64(pm->io_base));
+        build_gas(&fadt->xpm1a_control_block, AML_SYSTEM_IO,
+                  16, 0, 0, cpu_to_le64(pm->io_base + 0x04));
+        build_gas(&fadt->xpm_timer_block, AML_SYSTEM_IO,
+                  32, 0, 0, cpu_to_le64(pm->io_base + 0x08));
+        build_gas(&fadt->xgpe0_block, AML_SYSTEM_IO,
+                  pm->gpe0_blk_len << 3, 0, 0, cpu_to_le64(pm->gpe0_blk));
+    }
+
+    /* Build dummy ACPI 5.0 fields */
+    if (revision >= 5) {
+        build_gas(&fadt->sleep_control, AML_SYSTEM_MEMORY, 0, 0, 0, 0);
+        build_gas(&fadt->sleep_status, AML_SYSTEM_MEMORY, 0, 0, 0, 0);
+    }
 }
 
 

@@ -316,9 +349,9 @@ static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)
 static void
 build_fadt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm,
            unsigned facs_tbl_offset, unsigned dsdt_tbl_offset,
-           const char *oem_id, const char *oem_table_id)
+           const char *oem_id, const char *oem_table_id, uint8_t revision)
 {
-    AcpiFadtDescriptorRev1 *fadt = acpi_data_push(table_data, sizeof(*fadt));
+    AcpiFadtDescriptorRev5_1 *fadt = acpi_data_push(table_data, sizeof(*fadt));
     unsigned fw_ctrl_offset = (char *)&fadt->firmware_ctrl - table_data->data;
     unsigned dsdt_entry_offset = (char *)&fadt->dsdt - table_data->data;
 

@@ -328,13 +361,28 @@ build_fadt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm,
         ACPI_BUILD_TABLE_FILE, facs_tbl_offset);
 
     /* DSDT address to be filled by Guest linker */
-    fadt_setup(fadt, pm);
     bios_linker_loader_add_pointer(linker,
         ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt->dsdt),
         ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
 
-    build_header(linker, table_data,
-                 (void *)fadt, "FACP", sizeof(*fadt), 1, oem_id, oem_table_id);
+    if (revision > 2) {
+        fw_ctrl_offset = (char *)&fadt->Xfacs - table_data->data;
+        dsdt_entry_offset = (char *)&fadt->Xdsdt - table_data->data;
+
+        /* FACS address to be filled by Guest linker */
+        bios_linker_loader_add_pointer(linker,
+            ACPI_BUILD_TABLE_FILE, fw_ctrl_offset, sizeof(fadt->Xfacs),
+            ACPI_BUILD_TABLE_FILE, facs_tbl_offset);
+
+        /* DSDT address to be filled by Guest linker */
+        bios_linker_loader_add_pointer(linker,
+            ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt->Xdsdt),
+            ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
+    }
+
+    fadt_setup(fadt, pm, revision);
+    build_header(linker, table_data, (void *)fadt, "FACP", sizeof(*fadt),
+                 revision, oem_id, oem_table_id);
 }
 
 void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,

@@ -2681,7 +2729,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
     fadt = tables_blob->len;
     acpi_add_table(table_offsets, tables_blob);
     build_fadt(tables_blob, tables->linker, &pm, facs, dsdt,
-               slic_oem.id, slic_oem.table_id);
+               slic_oem.id, slic_oem.table_id, acpi_fadt_rev);
     aml_len += tables_blob->len - fadt;
 
     acpi_add_table(table_offsets, tables_blob);

include/hw/acpi/acpi.h +1 -0

index 7b3d93c..63df38d 100644
@@ -173,6 +173,7 @@ void acpi_update_sci(ACPIREGS *acpi_regs, qemu_irq irq);
 
 /* acpi.c */
 extern int acpi_enabled;
+extern uint8_t acpi_fadt_rev;
 extern char unsigned *acpi_tables;
 extern size_t acpi_tables_len;
 

qemu-options.hx +9 -0

index a71aaf8..cff34f6 100644
@@ -1492,6 +1492,15 @@ STEXI
 Disable HPET support.
 ETEXI
 
+DEF("acpifadt", HAS_ARG, QEMU_OPTION_acpifadt, \
+    "-acpifadt  n    configure FADT revision number (1, 3, 5)\n",
+    QEMU_ARCH_ALL)
+STEXI
+@item -acpifadt @var{n}
+@findex -acpifadt
+Configure FADT revision number, default is 1.
+ETEXI
+
 DEF("acpitable", HAS_ARG, QEMU_OPTION_acpitable,
     "-acpitable [sig=str][,rev=n][,oem_id=str][,oem_table_id=str][,oem_rev=n][,asl_compiler_id=str][,asl_compiler_rev=n][,{data|file}=file1[:file2]...]\n"
     "                ACPI table description\n", QEMU_ARCH_I386)

vl.c +18 -0

index e7c2c62..5fe2414 100644
@@ -64,6 +64,7 @@ int main(int argc, char **argv)
 #include "hw/bt.h"
 #include "sysemu/watchdog.h"
 #include "hw/smbios/smbios.h"
+#include "hw/acpi/acpi.h"
 #include "hw/xen/xen.h"
 #include "hw/qdev.h"
 #include "hw/loader.h"

@@ -158,6 +159,7 @@ int max_cpus = 1;
 int smp_cores = 1;
 int smp_threads = 1;
 int acpi_enabled = 1;
+uint8_t acpi_fadt_rev = 1;
 int no_hpet = 0;
 int fd_bootchk = 1;
 static int no_reboot;

@@ -2301,6 +2303,19 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, Error **errp)
     return 0;
 }
 
+static void fadt_parse(const char *arg)
+{
+    uint32_t rev;
+
+    rev = strtoul(arg, NULL, 0);
+    if (rev != 1 && rev != 3 && rev != 5) {
+        error_printf("Unsupported FADT revision %d,"
+                     "please specify 1,3,5.\n", rev);
+        exit(1);
+    }
+    acpi_fadt_rev = rev;
+}
+
 static int device_help_func(void *opaque, QemuOpts *opts, Error **errp)
 {
     return qdev_device_help(opts);

@@ -3622,6 +3637,9 @@ int main(int argc, char **argv, char **envp)
                 qdev_prop_register_global(&slew_lost_ticks);
                 break;
             }
+            case QEMU_OPTION_acpifadt:
+                fadt_parse(optarg);
+                break;
             case QEMU_OPTION_acpitable:
                 opts = qemu_opts_parse_noisily(qemu_find_opts("acpi"),
                                                optarg, true);

From: Zheng, Lv
Subject: [PATCH v4 1/2] ACPI: Cleanup -acpitable option code

In -acpitable options, at least/most one data/file sub-option is mandatory, this patch cleans up the code to reflect this in a managed manner so that the follow-up mandatory sub-options can be added to -acpitable.

Signed-off-by Lv Zheng <[email protected]>

---
 hw/acpi/core.c   |   32 +++++++++++++++++++++++---------
 qapi-schema.json |   27 ++++++++-------------------
 qemu-options.hx  |    7 +++++--
 3 files changed, 36 insertions(+), 30 deletions(-)

hw/acpi/core.c +0 -2

index e890a5d..cd1f9e4 100644
@@ -89,8 +89,6 @@ static int acpi_checksum(const uint8_t *data, int len)
  * It is valid to call this function with
  * (@blob == NULL && bloblen == 0 && !has_header).
  *
- * @hdrs->file and @hdrs->data are ignored.
- *
  * SIZE_MAX is considered "infinity" in this function.
  *
  * The number of tables that can be installed is not limited, but the 16-bit

hw/acpi/core.c +0 -0

 {
     AcpiTableOptions *hdrs = NULL;

     Error *err = NULL;

hw/acpi/core.c +1 -5

@@ -249,12 +248,8 @@ void acpi_table_add(const QemuOpts *opts,
 Error **errp)
     if (err) {
         goto out;
     }
-    if (hdrs->has_file == hdrs->has_data) {
-        error_setg(&err, "'-acpitable' requires one of 'data' or 'file'");
-        goto out;
-    }
 
-    pathnames = g_strsplit(hdrs->has_file ? hdrs->file : hdrs->data, ":", 0);
+    pathnames = g_strsplit(file, ":", 0);
     if (pathnames == NULL || pathnames[0] == NULL) {
         error_setg(&err, "'-acpitable' requires at least one pathname");

         goto out;

hw/acpi/core.c +1 -1

@@ -291,7 +286,7 @@ void acpi_table_add(const QemuOpts *opts, Error
 **errp)
         close(fd);
     }
 
-    acpi_table_install(blob, bloblen, hdrs->has_file, hdrs, &err);
+    acpi_table_install(blob, bloblen, has_header, hdrs, &err);
 
 out:

     g_free(blob);

hw/acpi/core.c +19 -0

@@ -301,6 +296,25 @@ out:
     error_propagate(errp, err);
 }
 
+void acpi_table_add(const QemuOpts *opts, Error **errp)
+{
+    const char *val;
+
+    val = qemu_opt_get((QemuOpts *)opts, "file");
+    if (val) {
+        acpi_table_from_file(true, val, hdrs, errp);
+        return;
+    }
+
+    val = qemu_opt_get((QemuOpts *)opts, "data");
+    if (val) {
+        acpi_table_from_file(false, val, hdrs, errp);
+        return;
+    }
+
+    error_setg(errp, "'-acpitable' requires one of 'data' or 'file'");
+}
+
 static bool acpi_table_builtin = false;
 
 void acpi_table_add_builtin(const QemuOpts *opts, Error **errp)

qapi-schema.json +1 -2

index 5658723..a5e219f 100644
@@ -3597,17 +3597,18 @@
 ##
 # @AcpiTableOptions
 #
-# Specify an ACPI table on the command line to load.
+# Specify ACPI table options for the table loaded on the command line.
 #
-# At most one of @file and @data can be specified. The list of files

specified
-# by any one of them is loaded and concatenated in order. If both are omitted,

qapi-schema.json +3 -1

-# @data is implied.
+# ACPI table can be loaded via 'file' and 'data' options. At most one of
+# 'file' and 'data' can be specified. The list of files specified by any one
+# of them is loaded and concatenated in order. If both # are omitted,

'data'

qapi-schema.json +1 -1

+# is implied.
 #
 # Other fields / optargs can be used to override fields of the generic ACPI
 # table header; refer to the ACPI specification 5.0, section 5.2.6 System
 # Description Table Header. If a header field is not overridden, then the
-# corresponding value from the concatenated blob is used (in case of

@file), or

qapi-schema.json +1 -1

-# it is filled in with a hard-coded value (in case of @data).
+# corresponding value from the concatenated blob is used (in case of

'file'),
+# or it is filled in with a hard-coded value (in case of 'data').
#
# String fields are copied into the matching ACPI member from lowest
address
# upwards, and silently truncated / NUL-padded to length.

qapi-schema.json +0 -0

@@ -3628,16 +3629,6 @@
 # @asl_compiler_rev: #optional revision number of the utility that

created the
# table (4 bytes)
#

qapi-schema.json +0 -7

-# @file: #optional colon (:) separated list of pathnames to load and
-#        concatenate as table data. The resultant binary blob is expected to
-#        have an ACPI table header. At least one file is required. This field
-#        excludes @data.
-#
-# @data: #optional colon (:) separated list of pathnames to load and
-#        concatenate as table data. The resultant binary blob must not have

an

qapi-schema.json +1 -6

-#        ACPI table header. At least one file is required. This field excludes
-#        @file.
-#
 # Since 1.5
 ##
 { 'struct': 'AcpiTableOptions',

@@ -3648,9 +3639,7 @@
     '*oem_table_id':      'str',
     '*oem_rev':           'uint32',
     '*asl_compiler_id':   'str',
-    '*asl_compiler_rev':  'uint32',
-    '*file':              'str',
-    '*data':              'str' }}
+    '*asl_compiler_rev':  'uint32' }}
 
 ##
 # @CommandLineParameterType:

qemu-options.hx +0 -1

index a71aaf8..5fe7f87 100644
@@ -1493,10 +1493,13 @@ Disable HPET support.
 ETEXI
 
 DEF("acpitable", HAS_ARG, QEMU_OPTION_acpitable,
-    "-acpitable

[sig=str][,rev=n][,oem_id=str][,oem_table_id=str][,oem_rev=n][,asl_compi ler_id=str][,asl_compiler_rev=n][,{data|file}=file1[:file2]...]\n"

qemu-options.hx +4 -1

+    "-acpitable {data|file}=file1[:file2]...\n"
+    "             [,sig=str][,rev=n]\n"
+    "             [,oem_id=str][,oem_table_id=str][,oem_rev=n]\n"
+    "             [,asl_compiler_id=str][,asl_compiler_rev=n]\n"
     "                ACPI table description\n", QEMU_ARCH_I386)
 STEXI
-@item -acpitable

[sig=@var{str}][,rev=@var{n}][,oem_id=@var{str}][,oem_table_id=@var{st r}][,oem_rev=@var{n}] [,asl_compiler_id=@var{str}][,asl_compiler_rev=@var{n}][,data=@var{file1 }[:@var{file2}]...]
+@item -acpitable data=@var{file1}[:@var{file2}]...[,sig=@var{str}][,rev=@var{n}][,oem_id=@ var{str}][,oem_table_id=@var{str}][,oem_rev=@var{n}] [,asl_compiler_id=@var{str}][,asl_compiler_rev=@var{n}]
@findex -acpitable
Add ACPI table with specified header fields and context from specified files. For file=, take whole ACPI table from the specified files, including all
--
1.7.10

In -acpitable options, at least/most one data/file sub-option is mandatory, this patch cleans up the code to reflect this in a managed manner so that the follow-up mandatory sub-options can be added to -acpitable.

Signed-off-by Lv Zheng <[email protected]>

---
 hw/acpi/core.c   |   32 +++++++++++++++++++++++---------
 qapi-schema.json |   26 +++++++-------------------
 qemu-options.hx  |    7 +++++--
 3 files changed, 35 insertions(+), 30 deletions(-)

hw/acpi/core.c +0 -0

index e890a5d..85e0e94 100644

hw/acpi/core.c +23 -7

  * SIZE_MAX is considered "infinity" in this function.
  *
  * The number of tables that can be installed is not limited, but the 16-bit

@@ -229,7 +227,8 @@ static void acpi_table_install(const char unsigned *blob, size_t bloblen,
                                       ACPI_TABLE_PFX_SIZE, acpi_payload_size);
 }
 
-void acpi_table_add(const QemuOpts *opts, Error **errp)
+static void acpi_table_from_file(bool has_header, const char *file,
+                                 const QemuOpts *opts, Error **errp)
 {
     AcpiTableOptions *hdrs = NULL;
     Error *err = NULL;

@@ -249,12 +248,8 @@ void acpi_table_add(const QemuOpts *opts, Error **errp)
     if (err) {
         goto out;
     }
-    if (hdrs->has_file == hdrs->has_data) {
-        error_setg(&err, "'-acpitable' requires one of 'data' or 'file'");
-        goto out;
-    }
 
-    pathnames = g_strsplit(hdrs->has_file ? hdrs->file : hdrs->data, ":", 0);
+    pathnames = g_strsplit(file, ":", 0);
     if (pathnames == NULL || pathnames[0] == NULL) {
         error_setg(&err, "'-acpitable' requires at least one pathname");
         goto out;

@@ -291,7 +286,7 @@ void acpi_table_add(const QemuOpts *opts, Error **errp)
         close(fd);
     }
 
-    acpi_table_install(blob, bloblen, hdrs->has_file, hdrs, &err);
+    acpi_table_install(blob, bloblen, has_header, hdrs, &err);
 
 out:
     g_free(blob);

@@ -301,6 +296,25 @@ out:
     error_propagate(errp, err);
 }
 
+void acpi_table_add(const QemuOpts *opts, Error **errp)
+{
+    const char *val;
+
+    val = qemu_opt_get((QemuOpts *)opts, "file");
+    if (val) {
+        acpi_table_from_file(true, val, opts, errp);
+        return;
+    }
+
+    val = qemu_opt_get((QemuOpts *)opts, "data");
+    if (val) {
+        acpi_table_from_file(false, val, opts, errp);
+        return;
+    }
+
+    error_setg(errp, "'-acpitable' requires one of 'data' or 'file'");
+}
+
 static bool acpi_table_builtin = false;
 
 void acpi_table_add_builtin(const QemuOpts *opts, Error **errp)

qapi-schema.json +0 -0

index 5658723..75b8b3b 100644
@@ -3597,17 +3597,17 @@
 ##

qapi-schema.json +5 -2

+# ACPI table can be loaded via 'file' and 'data' options. At most one of
+# 'file' and 'data' can be specified. The list of files specified by any one
+# of them is loaded and concatenated in order.
 #
 # Other fields / optargs can be used to override fields of the generic ACPI
 # table header; refer to the ACPI specification 5.0, section 5.2.6 System
 # Description Table Header. If a header field is not overridden, then the
-# corresponding value from the concatenated blob is used (in case of @file), or
-# it is filled in with a hard-coded value (in case of @data).
+# corresponding value from the concatenated blob is used (in case of 'file'),
+# or it is filled in with a hard-coded value (in case of 'data').
 #

# String fields are copied into the matching ACPI member from lowest address # upwards, and silently truncated / NUL-padded to length.

qapi-schema.json +0 -10

@@ -3628,16 +3628,6 @@
 # @asl_compiler_rev: #optional revision number of the utility that created the
 #                    table (4 bytes)
 #
-# @file: #optional colon (:) separated list of pathnames to load and
-#        concatenate as table data. The resultant binary blob is expected to
-#        have an ACPI table header. At least one file is required. This field
-#        excludes @data.
-#
-# @data: #optional colon (:) separated list of pathnames to load and
-#        concatenate as table data. The resultant binary blob must not have an
-#        ACPI table header. At least one file is required. This field excludes
-#        @file.
-#
 # Since 1.5
 ##
 { 'struct': 'AcpiTableOptions',

qapi-schema.json +0 -0

 ##
 # @CommandLineParameterType:

qemu-options.hx +5 -2

index a71aaf8..5fe7f87 100644
@@ -1493,10 +1493,13 @@ Disable HPET support.
 ETEXI
 
 DEF("acpitable", HAS_ARG, QEMU_OPTION_acpitable,
-    "-acpitable [sig=str][,rev=n][,oem_id=str][,oem_table_id=str][,oem_rev=n][,asl_compiler_id=str][,asl_compiler_rev=n][,{data|file}=file1[:file2]...]\n"
+    "-acpitable {data|file}=file1[:file2]...\n"
+    "             [,sig=str][,rev=n]\n"
+    "             [,oem_id=str][,oem_table_id=str][,oem_rev=n]\n"
+    "             [,asl_compiler_id=str][,asl_compiler_rev=n]\n"
     "                ACPI table description\n", QEMU_ARCH_I386)
 STEXI
-@item -acpitable [sig=@var{str}][,rev=@var{n}][,oem_id=@var{str}][,oem_table_id=@var{str}][,oem_rev=@var{n}] [,asl_compiler_id=@var{str}][,asl_compiler_rev=@var{n}][,data=@var{file1}[:@var{file2}]...]
+@item -acpitable data=@var{file1}[:@var{file2}]...[,sig=@var{str}][,rev=@var{n}][,oem_id=@var{str}][,oem_table_id=@var{str}][,oem_rev=@var{n}] [,asl_compiler_id=@var{str}][,asl_compiler_rev=@var{n}]
 @findex -acpitable
 Add ACPI table with specified header fields and context from specified files.
 For file=, take whole ACPI table from the specified files, including all

From: Igor Mammedov [mailto:[email protected]]
Subject: Re: [PATCH v5 1/2] ACPI: Cleanup -acpitable option code

Quoted 11 lines · On Thu, 11 Aug 2016 17:36:38 +0800 Lv Zheng <[email protected]> wrote:

In -acpitable options, at least/most one data/file sub-option is mandatory, this patch cleans up the code to reflect this in a managed manner so that the follow-up mandatory sub-options can be added to -acpitable.

Signed-off-by Lv Zheng <[email protected]>

---
 hw/acpi/core.c   |   32 +++++++++++++++++++++++---------
 qapi-schema.json |   26 +++++++-------------------
 qemu-options.hx  |    7 +++++--
 3 files changed, 35 insertions(+), 30 deletions(-)

hw/acpi/core.c static int acpi_checksum(const uint8_t *data, int len) · line 93

Quoted 10 lines

hw/acpi/core.c +0 -2

index e890a5d..85e0e94 100644
@@ -89,8 +89,6 @@ static int acpi_checksum(const uint8_t *data, int len)
  * It is valid to call this function with
  * (@blob == NULL && bloblen == 0 && !has_header).
  *
- * @hdrs->file and @hdrs->data are ignored.
- *

unrelated change?

Quoted 76 lines

hw/acpi/core.c +23 -7

  * SIZE_MAX is considered "infinity" in this function.
  *
  * The number of tables that can be installed is not limited, but the 16-bit

@@ -229,7 +227,8 @@ static void acpi_table_install(const char unsigned *blob, size_t bloblen,
                                       ACPI_TABLE_PFX_SIZE, acpi_payload_size);
 }
 
-void acpi_table_add(const QemuOpts *opts, Error **errp)
+static void acpi_table_from_file(bool has_header, const char *file,
+                                 const QemuOpts *opts, Error **errp)
 {
     AcpiTableOptions *hdrs = NULL;
     Error *err = NULL;

@@ -249,12 +248,8 @@ void acpi_table_add(const QemuOpts *opts, Error **errp)
     if (err) {
         goto out;
     }
-    if (hdrs->has_file == hdrs->has_data) {
-        error_setg(&err, "'-acpitable' requires one of 'data' or 'file'");
-        goto out;
-    }
 
-    pathnames = g_strsplit(hdrs->has_file ? hdrs->file : hdrs->data, ":", 0);
+    pathnames = g_strsplit(file, ":", 0);
     if (pathnames == NULL || pathnames[0] == NULL) {
         error_setg(&err, "'-acpitable' requires at least one pathname");
         goto out;

@@ -291,7 +286,7 @@ void acpi_table_add(const QemuOpts *opts, Error **errp)
         close(fd);
     }
 
-    acpi_table_install(blob, bloblen, hdrs->has_file, hdrs, &err);
+    acpi_table_install(blob, bloblen, has_header, hdrs, &err);
 
 out:
     g_free(blob);

@@ -301,6 +296,25 @@ out:
     error_propagate(errp, err);
 }
 
+void acpi_table_add(const QemuOpts *opts, Error **errp)
+{
+    const char *val;
+
+    val = qemu_opt_get((QemuOpts *)opts, "file");
+    if (val) {
+        acpi_table_from_file(true, val, opts, errp);
+        return;
+    }
+
+    val = qemu_opt_get((QemuOpts *)opts, "data");
+    if (val) {
+        acpi_table_from_file(false, val, opts, errp);
+        return;
+    }
+
+    error_setg(errp, "'-acpitable' requires one of 'data' or 'file'");
+}
+
 static bool acpi_table_builtin = false;
 
 void acpi_table_add_builtin(const QemuOpts *opts, Error **errp)

qapi-schema.json +1 -3

index 5658723..75b8b3b 100644
@@ -3597,17 +3597,17 @@
 ##
 # @AcpiTableOptions
 #
-# Specify an ACPI table on the command line to load.
+# Specify ACPI table options for the table loaded on the command line.
 #
-# At most one of @file and @data can be specified. The list of files specified
-# by any one of them is loaded and concatenated in order.

qapi-schema.json line 3604

Quoted 1 line

qapi-schema.json +0 -1

-# If both are omitted, @data is implied.

You are removing this bit of documentation, which is still true

Quoted 35 lines

qapi-schema.json +5 -12

+# ACPI table can be loaded via 'file' and 'data' options. At most one of
+# 'file' and 'data' can be specified. The list of files specified by any one
+# of them is loaded and concatenated in order.
 #
 # Other fields / optargs can be used to override fields of the generic ACPI
 # table header; refer to the ACPI specification 5.0, section 5.2.6 System
 # Description Table Header. If a header field is not overridden, then the
-# corresponding value from the concatenated blob is used (in case of @file), or
-# it is filled in with a hard-coded value (in case of @data).
+# corresponding value from the concatenated blob is used (in case of 'file'),
+# or it is filled in with a hard-coded value (in case of 'data').
 #
 # String fields are copied into the matching ACPI member from lowest address
 # upwards, and silently truncated / NUL-padded to length.

@@ -3628,16 +3628,6 @@
 # @asl_compiler_rev: #optional revision number of the utility that created the
 #                    table (4 bytes)
 #
-# @file: #optional colon (:) separated list of pathnames to load and
-#        concatenate as table data. The resultant binary blob is expected to
-#        have an ACPI table header. At least one file is required. This field
-#        excludes @data.
-#
-# @data: #optional colon (:) separated list of pathnames to load and
-#        concatenate as table data. The resultant binary blob must not have an
-#        ACPI table header. At least one file is required. This field excludes
-#        @file.
-#
 # Since 1.5
 ##
 { 'struct': 'AcpiTableOptions',

@@ -3648,9 +3638,7 @@
     '*oem_table_id':      'str',

qapi-schema.json line 3641

Quoted 6 lines

qapi-schema.json +1 -3

     '*oem_rev':           'uint32',
     '*asl_compiler_id':   'str',
-    '*asl_compiler_rev':  'uint32',
-    '*file':              'str',
-    '*data':              'str' }}
+    '*asl_compiler_rev':  'uint32' }}

it's probably is not ok to remove fields here
as it might break existing users that expect them

qemu-options.hx Disable HPET support. · line 1496

Quoted 13 lines

qapi-schema.json +0 -0

 ##
 # @CommandLineParameterType:

qemu-options.hx +0 -1

index a71aaf8..5fe7f87 100644
@@ -1493,10 +1493,13 @@ Disable HPET support.
 ETEXI
 
 DEF("acpitable", HAS_ARG, QEMU_OPTION_acpitable,
-    "-acpitable

[sig=str][,rev=n][,oem_id=str][,oem_table_id=str][,oem_rev=n][,asl_compiler_id=str][,asl_compiler_rev= n][,{data|file}=file1[:file2]...]\n"

qemu-options.hx Disable HPET support.

Quoted 7 lines

qemu-options.hx +4 -1

+    "-acpitable {data|file}=file1[:file2]...\n"
+    "             [,sig=str][,rev=n]\n"
+    "             [,oem_id=str][,oem_table_id=str][,oem_rev=n]\n"
+    "             [,asl_compiler_id=str][,asl_compiler_rev=n]\n"
     "                ACPI table description\n", QEMU_ARCH_I386)
 STEXI
-@item -acpitable

[sig=@var{str}][,rev=@var{n}][,oem_id=@var{str}][,oem_table_id=@var{str}][,oem_rev=@var{n}] [,asl_compiler_id=@var{str}][,asl_compiler_rev=@var{n}][,data=@var{file1}[:@var{file2}]...]

qemu-options.hx Disable HPET support. · line 1502

Quoted 1 line

qemu-options.hx +1 -0

+@item -acpitable

data=@var{file1}[:@var{file2}]...[,sig=@var{str}][,rev=@var{n}][,oem_id=@var{str}][,oem_table_id=@var{ str}][,oem_rev=@var{n}] [,asl_compiler_id=@var{str}][,asl_compiler_rev=@var{n}]

Quoted 3 lines

qemu-options.hx +0 -0

 @findex -acpitable
 Add ACPI table with specified header fields and context from specified files.
 For file=, take whole ACPI table from the specified files, including all

This patch allows FADT to be built with different revisions. When the revision is greater than 1.0, 64-bit address fields may also be filled.

Note that FADT revision 2 has never been defined by the ACPI specification. So this patch only adds an option -acpitable fadt= to allow revision 1,3,5 to be configured by the users.

1. Tested by booting a linux image, the 64-bit addresses are correctly filled in the dumped FADT.
2. Tested by booting a Windows image, no boot failure can be seen.

Signed-off-by Lv Zheng <[email protected]>

---
 hw/acpi/core.c         |   20 ++++++++++++-
 hw/i386/acpi-build.c   |   76 ++++++++++++++++++++++++++++++++++++++++++------
 include/hw/acpi/acpi.h |    1 +
 qemu-options.hx        |    8 ++++-
 4 files changed, 94 insertions(+), 11 deletions(-)

hw/acpi/core.c +19 -1

index 85e0e94..832c86b 100644
@@ -19,6 +19,7 @@
  * GNU GPL, version 2 or (at your option) any later version.
  */
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 #include "sysemu/sysemu.h"
 #include "hw/hw.h"
 #include "hw/i386/pc.h"

@@ -54,6 +55,7 @@ static const char unsigned dfl_hdr[ACPI_TABLE_HDR_SIZE - ACPI_TABLE_PFX_SIZE] =
 
 char unsigned *acpi_tables;
 size_t acpi_tables_len;
+uint8_t acpi_fadt_rev = 1;
 
 static QemuOptsList qemu_acpi_opts = {
     .name = "acpi",

@@ -312,7 +314,23 @@ void acpi_table_add(const QemuOpts *opts, Error **errp)
         return;
     }
 
-    error_setg(errp, "'-acpitable' requires one of 'data' or 'file'");
+    val = qemu_opt_get((QemuOpts *)opts, "fadt");
+    if (val) {
+        unsigned long rev;
+        int err;
+
+        err = qemu_strtoul(val, NULL, 10, &rev);
+        if (err ||
+            (rev != 1 && rev != 3 && rev != 5)) {
+            error_setg(errp, "Unsupported FADT revision %s, "
+                       "please specify 1,3,5", val);
+            return;
+        }
+        acpi_fadt_rev = rev;
+        return;
+    }
+
+    error_setg(errp, "'-acpitable' requires one of 'data','file' or 'fadt'");
 }
 
 static bool acpi_table_builtin = false;

hw/i386/acpi-build.c +67 -9

index ce7cbc5..8be6578 100644
@@ -276,8 +276,22 @@ build_facs(GArray *table_data, BIOSLinker *linker)
     facs->length = cpu_to_le32(sizeof(*facs));
 }
 
+/* GAS */
+static void
+build_gas(struct AcpiGenericAddress *gas, uint8_t space_id,
+          uint8_t bit_width, uint8_t bit_offset,
+          uint8_t access_width, uint64_t address)
+{
+    gas->space_id = space_id;
+    gas->bit_width = bit_width;
+    gas->bit_offset = bit_offset;
+    gas->access_width = access_width;
+    gas->address = address;
+}
+
 /* Load chipset information in FADT */
-static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)
+static void fadt_setup(AcpiFadtDescriptorRev5_1 *fadt, AcpiPmInfo *pm,
+                       uint8_t revision)
 {
     fadt->model = 1;
     fadt->reserved1 = 0;

@@ -309,6 +323,25 @@ static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)
         fadt->flags |= cpu_to_le32(1 << ACPI_FADT_F_FORCE_APIC_CLUSTER_MODEL);
     }
     fadt->century = RTC_CENTURY;
+
+    /* Build ACPI 2.0 (FADT version 3+) GAS fields */
+    if (revision >= 3) {
+        /* EVT, CNT, TMR register matches hw/acpi/core.c */
+        build_gas(&fadt->xpm1a_event_block, AML_SYSTEM_IO,
+                  32, 0, 0, cpu_to_le64(pm->io_base));
+        build_gas(&fadt->xpm1a_control_block, AML_SYSTEM_IO,
+                  16, 0, 0, cpu_to_le64(pm->io_base + 0x04));
+        build_gas(&fadt->xpm_timer_block, AML_SYSTEM_IO,
+                  32, 0, 0, cpu_to_le64(pm->io_base + 0x08));
+        build_gas(&fadt->xgpe0_block, AML_SYSTEM_IO,
+                  pm->gpe0_blk_len << 3, 0, 0, cpu_to_le64(pm->gpe0_blk));
+    }
+
+    /* Build dummy ACPI 5.0 fields */
+    if (revision >= 5) {
+        build_gas(&fadt->sleep_control, AML_SYSTEM_MEMORY, 0, 0, 0, 0);
+        build_gas(&fadt->sleep_status, AML_SYSTEM_MEMORY, 0, 0, 0, 0);
+    }
 }
 
 

@@ -316,11 +349,21 @@ static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)
 static void
 build_fadt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm,
            unsigned facs_tbl_offset, unsigned dsdt_tbl_offset,
-           const char *oem_id, const char *oem_table_id)
+           const char *oem_id, const char *oem_table_id, uint8_t revision)
 {
-    AcpiFadtDescriptorRev1 *fadt = acpi_data_push(table_data, sizeof(*fadt));
-    unsigned fw_ctrl_offset = (char *)&fadt->firmware_ctrl - table_data->data;
-    unsigned dsdt_entry_offset = (char *)&fadt->dsdt - table_data->data;
+    AcpiFadtDescriptorRev5_1 *fadt;
+    unsigned fw_ctrl_offset;
+    unsigned dsdt_entry_offset;
+    unsigned fadt_len;
+
+    if (revision == 1) {
+        fadt_len = sizeof(AcpiFadtDescriptorRev1);
+    } else {
+        fadt_len = sizeof(AcpiFadtDescriptorRev5_1);
+    }
+    fadt = acpi_data_push(table_data, fadt_len);
+    fw_ctrl_offset = (char *)&fadt->firmware_ctrl - table_data->data;
+    dsdt_entry_offset = (char *)&fadt->dsdt - table_data->data;
 
     /* FACS address to be filled by Guest linker */
     bios_linker_loader_add_pointer(linker,

@@ -328,13 +371,28 @@ build_fadt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm,
         ACPI_BUILD_TABLE_FILE, facs_tbl_offset);
 
     /* DSDT address to be filled by Guest linker */
-    fadt_setup(fadt, pm);
     bios_linker_loader_add_pointer(linker,
         ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt->dsdt),
         ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
 
-    build_header(linker, table_data,
-                 (void *)fadt, "FACP", sizeof(*fadt), 1, oem_id, oem_table_id);
+    if (revision > 2) {
+        fw_ctrl_offset = (char *)&fadt->Xfacs - table_data->data;
+        dsdt_entry_offset = (char *)&fadt->Xdsdt - table_data->data;
+
+        /* FACS address to be filled by Guest linker */
+        bios_linker_loader_add_pointer(linker,
+            ACPI_BUILD_TABLE_FILE, fw_ctrl_offset, sizeof(fadt->Xfacs),
+            ACPI_BUILD_TABLE_FILE, facs_tbl_offset);
+
+        /* DSDT address to be filled by Guest linker */
+        bios_linker_loader_add_pointer(linker,
+            ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt->Xdsdt),
+            ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
+    }
+
+    fadt_setup(fadt, pm, revision);
+    build_header(linker, table_data, (void *)fadt, "FACP", fadt_len,
+                 revision, oem_id, oem_table_id);
 }
 
 void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,

@@ -2681,7 +2739,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
     fadt = tables_blob->len;
     acpi_add_table(table_offsets, tables_blob);
     build_fadt(tables_blob, tables->linker, &pm, facs, dsdt,
-               slic_oem.id, slic_oem.table_id);
+               slic_oem.id, slic_oem.table_id, acpi_fadt_rev);
     aml_len += tables_blob->len - fadt;
 
     acpi_add_table(table_offsets, tables_blob);

include/hw/acpi/acpi.h +1 -0

index 7b3d93c..63df38d 100644
@@ -173,6 +173,7 @@ void acpi_update_sci(ACPIREGS *acpi_regs, qemu_irq irq);
 
 /* acpi.c */
 extern int acpi_enabled;
+extern uint8_t acpi_fadt_rev;
 extern char unsigned *acpi_tables;
 extern size_t acpi_tables_len;
 

qemu-options.hx +7 -1

index 5fe7f87..d61dd92 100644
@@ -1497,7 +1497,10 @@ DEF("acpitable", HAS_ARG, QEMU_OPTION_acpitable,
     "             [,sig=str][,rev=n]\n"
     "             [,oem_id=str][,oem_table_id=str][,oem_rev=n]\n"
     "             [,asl_compiler_id=str][,asl_compiler_rev=n]\n"
-    "                ACPI table description\n", QEMU_ARCH_I386)
+    "                ACPI table description\n"
+    "-acpitable fadt=n\n"
+    "                Configure FADT revision\n",
+    QEMU_ARCH_I386)
 STEXI
 @item -acpitable data=@var{file1}[:@var{file2}]...[,sig=@var{str}][,rev=@var{n}][,oem_id=@var{str}][,oem_table_id=@var{str}][,oem_rev=@var{n}] [,asl_compiler_id=@var{str}][,asl_compiler_rev=@var{n}]
 @findex -acpitable

@@ -1511,6 +1514,9 @@ If a SLIC table is supplied to QEMU, then the SLIC's oem_id and oem_table_id
 fields will override the same in the RSDT and the FADT (a.k.a. FACP), in order
 to ensure the field matches required by the Microsoft SLIC spec and the ACPI
 spec.
+
+@item -acpitable fadt=@var{n}
+Configure FADT revision to 1, 3, 5, default 1.
 ETEXI
 
 DEF("smbios", HAS_ARG, QEMU_OPTION_smbios,

From: Igor Mammedov [mailto:[email protected]]
Subject: Re: [PATCH v5 2/2] ACPI: Add -acpitable fadt= to allow FADT revision changes

Quoted 1 line · On Thu, 11 Aug 2016 17:36:45 +0800 Lv Zheng <[email protected]> wrote:

This patch allows FADT to be built with different revisions. When the

revision

Quoted 3 lines

is greater than 1.0, 64-bit address fields may also be filled.

Note that FADT revision 2 has never been defined by the ACPI

specification. So

Quoted 1 line

this patch only adds an option -acpitable fadt= to allow revision 1,3,5 to

be

Quoted 3 lines

configured by the users.

1. Tested by booting a linux image, the 64-bit addresses are correctly

filled

Quoted 2 lines

in the dumped FADT.
2. Tested by booting a Windows image, no boot failure can be seen.

series still doesn't say why do you need 64-bit addresses in FADT, current Seabios/OVMF allocates tables below 4Gb and gpe/pm register blocks are below 4gb as well so there is no point in accepting theses patches and adding extra CLI options as these patches do.

If we'd ever need to bump FADT revision, I'd first try to increase it unconditionally and test/see if it would not upset legacy guests Windows starting from XP, RHEL4/5/6
If it works then these patches are not necessary,
(this approach worked just fine for bumping up DSDT revision to 2 it would be possible to use 64-bit math in ASL/AML).

if it doesn't, I still won't do it this way but rather:
- make new revision used by default
- add compat property to piix4_pm/ich9-lpc to force legacy revision
- turn on legacy revision for old machine types using added above
property

That way we won't regress existing setups as old machines will stay the same and only new machine types will be affected. And users that actually want to play with legacy revision on new machine types could force it by using above property.

Quoted 5 lines

Signed-off-by Lv Zheng <[email protected]>

---
 hw/acpi/core.c         |   20 ++++++++++++-
 hw/i386/acpi-build.c   |   76

++++++++++++++++++++++++++++++++++++++++++------

hw/acpi/core.c static const char unsigned

Quoted 17 lines

 include/hw/acpi/acpi.h |    1 +
 qemu-options.hx        |    8 ++++-
 4 files changed, 94 insertions(+), 11 deletions(-)

hw/acpi/core.c +1 -0

index 85e0e94..832c86b 100644
@@ -19,6 +19,7 @@
  * GNU GPL, version 2 or (at your option) any later version.
  */
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 #include "sysemu/sysemu.h"
 #include "hw/hw.h"
 #include "hw/i386/pc.h"

@@ -54,6 +55,7 @@ static const char unsigned

dfl_hdr[ACPI_TABLE_HDR_SIZE - ACPI_TABLE_PFX_SIZE] =

hw/acpi/core.c void acpi_table_add(const QemuOpts *opts,

Quoted 8 lines

hw/acpi/core.c +1 -0

 char unsigned *acpi_tables;
 size_t acpi_tables_len;
+uint8_t acpi_fadt_rev = 1;
 
 static QemuOptsList qemu_acpi_opts = {
     .name = "acpi",

@@ -312,7 +314,23 @@ void acpi_table_add(const QemuOpts *opts,

Error **errp)

hw/i386/acpi-build.c build_facs(GArray *table_data, BIOSLinker

Quoted 29 lines

hw/acpi/core.c +17 -1

         return;
     }
 
-    error_setg(errp, "'-acpitable' requires one of 'data' or 'file'");
+    val = qemu_opt_get((QemuOpts *)opts, "fadt");
+    if (val) {
+        unsigned long rev;
+        int err;
+
+        err = qemu_strtoul(val, NULL, 10, &rev);
+        if (err ||
+            (rev != 1 && rev != 3 && rev != 5)) {
+            error_setg(errp, "Unsupported FADT revision %s, "
+                       "please specify 1,3,5", val);
+            return;
+        }
+        acpi_fadt_rev = rev;
+        return;
+    }
+
+    error_setg(errp, "'-acpitable' requires one of 'data','file' or 'fadt'");
 }
 
 static bool acpi_table_builtin = false;

hw/i386/acpi-build.c +0 -0

index ce7cbc5..8be6578 100644
@@ -276,8 +276,22 @@ build_facs(GArray *table_data, BIOSLinker

*linker)

Quoted 9 lines

hw/i386/acpi-build.c +6 -0

     facs->length = cpu_to_le32(sizeof(*facs));
 }
 
+/* GAS */
+static void
+build_gas(struct AcpiGenericAddress *gas, uint8_t space_id,
+          uint8_t bit_width, uint8_t bit_offset,
+          uint8_t access_width, uint64_t address)
+{

hw/i386/acpi-build.c build_facs(GArray *table_data, BIOSLinker *linker)

Quoted 10 lines

hw/i386/acpi-build.c +8 -1

+    gas->space_id = space_id;
+    gas->bit_width = bit_width;
+    gas->bit_offset = bit_offset;
+    gas->access_width = access_width;
+    gas->address = address;
+}
+
 /* Load chipset information in FADT */
-static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)
+static void fadt_setup(AcpiFadtDescriptorRev5_1 *fadt, AcpiPmInfo

*pm,

hw/i386/acpi-build.c static void fadt_setup(AcpiFadtDescriptorRev1

Quoted 5 lines

hw/i386/acpi-build.c +1 -0

+                       uint8_t revision)
 {
     fadt->model = 1;
     fadt->reserved1 = 0;

@@ -309,6 +323,25 @@ static void fadt_setup(AcpiFadtDescriptorRev1

*fadt, AcpiPmInfo *pm)

hw/i386/acpi-build.c static void fadt_setup(AcpiFadtDescriptorRev1 · line 323

Quoted 1 line

hw/i386/acpi-build.c +0 -0

         fadt->flags |= cpu_to_le32(1 <<

ACPI_FADT_F_FORCE_APIC_CLUSTER_MODEL);

Quoted 9 lines

hw/i386/acpi-build.c +7 -0

     }
     fadt->century = RTC_CENTURY;
+
+    /* Build ACPI 2.0 (FADT version 3+) GAS fields */
+    if (revision >= 3) {
+        /* EVT, CNT, TMR register matches hw/acpi/core.c */
+        build_gas(&fadt->xpm1a_event_block, AML_SYSTEM_IO,
+                  32, 0, 0, cpu_to_le64(pm->io_base));
+        build_gas(&fadt->xpm1a_control_block, AML_SYSTEM_IO,

hw/i386/acpi-build.c static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)

Quoted 10 lines

hw/i386/acpi-build.c +10 -0

+                  16, 0, 0, cpu_to_le64(pm->io_base + 0x04));
+        build_gas(&fadt->xpm_timer_block, AML_SYSTEM_IO,
+                  32, 0, 0, cpu_to_le64(pm->io_base + 0x08));
+        build_gas(&fadt->xgpe0_block, AML_SYSTEM_IO,
+                  pm->gpe0_blk_len << 3, 0, 0, cpu_to_le64(pm->gpe0_blk));
+    }
+
+    /* Build dummy ACPI 5.0 fields */
+    if (revision >= 5) {
+        build_gas(&fadt->sleep_control, AML_SYSTEM_MEMORY, 0, 0, 0,

0);

hw/i386/acpi-build.c static void

Quoted 6 lines

hw/i386/acpi-build.c +2 -0

+        build_gas(&fadt->sleep_status, AML_SYSTEM_MEMORY, 0, 0, 0, 0);
+    }
 }
 
 

@@ -316,11 +349,21 @@ static void

fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)

hw/i386/acpi-build.c static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)

Quoted 7 lines

hw/i386/acpi-build.c +1 -2

 static void
 build_fadt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm,
            unsigned facs_tbl_offset, unsigned dsdt_tbl_offset,
-           const char *oem_id, const char *oem_table_id)
+           const char *oem_id, const char *oem_table_id, uint8_t revision)
 {
-    AcpiFadtDescriptorRev1 *fadt = acpi_data_push(table_data,

sizeof(*fadt));

hw/i386/acpi-build.c build_fadt(GArray *table_data, BIOSLinker

Quoted 20 lines

hw/i386/acpi-build.c +13 -2

-    unsigned fw_ctrl_offset = (char *)&fadt->firmware_ctrl - table_data-
 data;
-    unsigned dsdt_entry_offset = (char *)&fadt->dsdt - table_data->data;
+    AcpiFadtDescriptorRev5_1 *fadt;
+    unsigned fw_ctrl_offset;
+    unsigned dsdt_entry_offset;
+    unsigned fadt_len;
+
+    if (revision == 1) {
+        fadt_len = sizeof(AcpiFadtDescriptorRev1);
+    } else {
+        fadt_len = sizeof(AcpiFadtDescriptorRev5_1);
+    }
+    fadt = acpi_data_push(table_data, fadt_len);
+    fw_ctrl_offset = (char *)&fadt->firmware_ctrl - table_data->data;
+    dsdt_entry_offset = (char *)&fadt->dsdt - table_data->data;
 
     /* FACS address to be filled by Guest linker */

bios_linker_loader_add_pointer(linker,

hw/i386/acpi-build.c +0 -0

@@ -328,13 +371,28 @@ build_fadt(GArray *table_data, BIOSLinker

*linker, AcpiPmInfo *pm,

hw/i386/acpi-build.c void acpi_build(AcpiBuildTables *tables,

Quoted 32 lines

hw/i386/acpi-build.c +18 -3

         ACPI_BUILD_TABLE_FILE, facs_tbl_offset);
 
     /* DSDT address to be filled by Guest linker */
-    fadt_setup(fadt, pm);
     bios_linker_loader_add_pointer(linker,
         ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt->dsdt),
         ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
 
-    build_header(linker, table_data,
-                 (void *)fadt, "FACP", sizeof(*fadt), 1, oem_id, oem_table_id);
+    if (revision > 2) {
+        fw_ctrl_offset = (char *)&fadt->Xfacs - table_data->data;
+        dsdt_entry_offset = (char *)&fadt->Xdsdt - table_data->data;
+
+        /* FACS address to be filled by Guest linker */
+        bios_linker_loader_add_pointer(linker,
+            ACPI_BUILD_TABLE_FILE, fw_ctrl_offset, sizeof(fadt->Xfacs),
+            ACPI_BUILD_TABLE_FILE, facs_tbl_offset);
+
+        /* DSDT address to be filled by Guest linker */
+        bios_linker_loader_add_pointer(linker,
+            ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt->Xdsdt),
+            ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
+    }
+
+    fadt_setup(fadt, pm, revision);
+    build_header(linker, table_data, (void *)fadt, "FACP", fadt_len,
+                 revision, oem_id, oem_table_id);
 }
 
 void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,

@@ -2681,7 +2739,7 @@ void acpi_build(AcpiBuildTables *tables,

MachineState *machine)

include/hw/acpi/acpi.h void acpi_update_sci(ACPIREGS *acpi_regs,

Quoted 13 lines

hw/i386/acpi-build.c +1 -1

     fadt = tables_blob->len;
     acpi_add_table(table_offsets, tables_blob);
     build_fadt(tables_blob, tables->linker, &pm, facs, dsdt,
-               slic_oem.id, slic_oem.table_id);
+               slic_oem.id, slic_oem.table_id, acpi_fadt_rev);
     aml_len += tables_blob->len - fadt;
 
     acpi_add_table(table_offsets, tables_blob);

include/hw/acpi/acpi.h +0 -0

index 7b3d93c..63df38d 100644
@@ -173,6 +173,7 @@ void acpi_update_sci(ACPIREGS *acpi_regs,

qemu_irq irq);

qemu-options.hx DEF("acpitable", HAS_ARG,

Quoted 12 lines

include/hw/acpi/acpi.h +1 -0

 /* acpi.c */
 extern int acpi_enabled;
+extern uint8_t acpi_fadt_rev;
 extern char unsigned *acpi_tables;
 extern size_t acpi_tables_len;
 

qemu-options.hx +0 -0

index 5fe7f87..d61dd92 100644
@@ -1497,7 +1497,10 @@ DEF("acpitable", HAS_ARG,

QEMU_OPTION_acpitable,

qemu-options.hx DEF("acpitable", HAS_ARG, QEMU_OPTION_acpitable,

Quoted 10 lines

qemu-options.hx +4 -1

     "             [,sig=str][,rev=n]\n"
     "             [,oem_id=str][,oem_table_id=str][,oem_rev=n]\n"
     "             [,asl_compiler_id=str][,asl_compiler_rev=n]\n"
-    "                ACPI table description\n", QEMU_ARCH_I386)
+    "                ACPI table description\n"
+    "-acpitable fadt=n\n"
+    "                Configure FADT revision\n",
+    QEMU_ARCH_I386)
 STEXI
 @item -acpitable

data=@var{file1}[:@var{file2}]...[,sig=@var{str}][,rev=@var{n}][,oem_id=@ var{str}][,oem_table_id=@var{str}][,oem_rev=@var{n}] [,asl_compiler_id=@var{str}][,asl_compiler_rev=@var{n}]

qemu-options.hx If a SLIC table is supplied to QEMU, then the

Quoted 2 lines

qemu-options.hx +0 -0

 @findex -acpitable

@@ -1511,6 +1514,9 @@ If a SLIC table is supplied to QEMU, then the

SLIC's oem_id and oem_table_id

qemu-options.hx If a SLIC table is supplied to QEMU, then the · line 1514

Quoted 1 line

qemu-options.hx +0 -0

 fields will override the same in the RSDT and the FADT (a.k.a. FACP), in

order

qemu-options.hx If a SLIC table is supplied to QEMU, then the · line 1515

Quoted 1 line

qemu-options.hx +0 -0

 to ensure the field matches required by the Microsoft SLIC spec and the

ACPI

Quoted 7 lines

qemu-options.hx +3 -0

 spec.
+
+@item -acpitable fadt=@var{n}
+Configure FADT revision to 1, 3, 5, default 1.
 ETEXI
 
 DEF("smbios", HAS_ARG, QEMU_OPTION_smbios,

Hi, Igor

Thanks for the review.

Quoted 19 lines

From: Igor Mammedov [mailto:[email protected]]
Subject: Re: [PATCH v5 2/2] ACPI: Add -acpitable fadt= to allow FADT revision changes

Quoted 1 line · On Thu, 11 Aug 2016 17:36:45 +0800 Lv Zheng <[email protected]> wrote:

This patch allows FADT to be built with different revisions. When the

revision

Quoted 3 lines

is greater than 1.0, 64-bit address fields may also be filled.

Note that FADT revision 2 has never been defined by the ACPI

specification. So

Quoted 1 line

this patch only adds an option -acpitable fadt= to allow revision 1,3,5 to

be

Quoted 3 lines

configured by the users.

1. Tested by booting a linux image, the 64-bit addresses are correctly

filled

Quoted 8 lines

Quoted 2 lines

in the dumped FADT.
2. Tested by booting a Windows image, no boot failure can be seen.

series still doesn't say why do you need 64-bit addresses in FADT, current Seabios/OVMF allocates tables below 4Gb and gpe/pm register blocks are below 4gb as well so there is no point in accepting theses patches and adding extra CLI options as these patches do.

[Lv Zheng]
This patch is used by us to probe Windows FADT behavior.
Current known behavior includes:
1. On x86, Windows requires BIOS to have 2 FADTs, 1 is in RSDT, and probably is a v1 table, the other is in XSDT, and probably is a v3+ table.

2. v2 FADT never exists in the spec, it is used in very early years, by IA64 prototype machines.
3. If the FADT in RSDT takes effective, then Windows actually ignores 64-bit GAS fields, but uses 32-bit register fields. But there are still many uncertainties:
1. What if the FADT in XSDT takes effective?
2. What's the behavior on IA64/ARM platforms?
If we could have an option here, we could continue the probing work, trying every combination.

Thus the original purpose of this commit is - probing de-facto standard behavior.

Quoted 14 lines

If we'd ever need to bump FADT revision, I'd first try to increase it unconditionally and test/see if it would not upset legacy guests Windows starting from XP, RHEL4/5/6
If it works then these patches are not necessary,
(this approach worked just fine for bumping up DSDT revision to 2 it would be possible to use 64-bit math in ASL/AML).

if it doesn't, I still won't do it this way but rather:
- make new revision used by default
- add compat property to piix4_pm/ich9-lpc to force legacy revision
- turn on legacy revision for old machine types using added above
property

Quoted 5 lines

That way we won't regress existing setups as old machines will stay the same and only new machine types will be affected. And users that actually want to play with legacy revision on new machine types could force it by using above property.

[Lv Zheng]
But it seems the code in this commit contains useful stuffs for qemu.

As I said previously, it seems Windows requires 2 FADTs.
So having a revision parameter for build_fadt() seems to be necessary. Because if qemu wants to increase FADT revision, qemu may do this in this way:
1. Invoke build_fadt() with revision 1, and put it into RSDT;
2. Invoke build_fadt() with latest revision, and put it into XSDT. Also we know latest revision FADT is useful, it contains reduced hardware support. And some of new ACPI features rely this model.

So I'm sure that this commit contains useful stuffs for qemu to do future extension.

But I don't know if you want to keep the new -acpitable fadt= option.
Let me ask:
1. Are there any issues in PATCH 01? Maybe my understanding of qemu option code is not correct.

2. IMO, this option should be useful for a long period because we need it to help probing Windows behavior, so can we have it upstreamed?

Best regards
Lv

Quoted 234 lines

Quoted 5 lines

Signed-off-by Lv Zheng <[email protected]>

---
 hw/acpi/core.c         |   20 ++++++++++++-
 hw/i386/acpi-build.c   |   76

++++++++++++++++++++++++++++++++++++++++++------

hw/acpi/core.c static const char unsigned

Quoted 17 lines

 include/hw/acpi/acpi.h |    1 +
 qemu-options.hx        |    8 ++++-
 4 files changed, 94 insertions(+), 11 deletions(-)

hw/acpi/core.c +1 -0

index 85e0e94..832c86b 100644
@@ -19,6 +19,7 @@
  * GNU GPL, version 2 or (at your option) any later version.
  */
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 #include "sysemu/sysemu.h"
 #include "hw/hw.h"
 #include "hw/i386/pc.h"

@@ -54,6 +55,7 @@ static const char unsigned

dfl_hdr[ACPI_TABLE_HDR_SIZE - ACPI_TABLE_PFX_SIZE] =

hw/acpi/core.c void acpi_table_add(const QemuOpts *opts,

Quoted 8 lines

hw/acpi/core.c +1 -0

 char unsigned *acpi_tables;
 size_t acpi_tables_len;
+uint8_t acpi_fadt_rev = 1;
 
 static QemuOptsList qemu_acpi_opts = {
     .name = "acpi",

@@ -312,7 +314,23 @@ void acpi_table_add(const QemuOpts *opts,

Error **errp)

hw/i386/acpi-build.c build_facs(GArray *table_data, BIOSLinker

Quoted 29 lines

hw/acpi/core.c +17 -1

         return;
     }
 
-    error_setg(errp, "'-acpitable' requires one of 'data' or 'file'");
+    val = qemu_opt_get((QemuOpts *)opts, "fadt");
+    if (val) {
+        unsigned long rev;
+        int err;
+
+        err = qemu_strtoul(val, NULL, 10, &rev);
+        if (err ||
+            (rev != 1 && rev != 3 && rev != 5)) {
+            error_setg(errp, "Unsupported FADT revision %s, "
+                       "please specify 1,3,5", val);
+            return;
+        }
+        acpi_fadt_rev = rev;
+        return;
+    }
+
+    error_setg(errp, "'-acpitable' requires one of 'data','file' or 'fadt'");
 }
 
 static bool acpi_table_builtin = false;

hw/i386/acpi-build.c +0 -0

index ce7cbc5..8be6578 100644
@@ -276,8 +276,22 @@ build_facs(GArray *table_data, BIOSLinker

*linker)

Quoted 9 lines

hw/i386/acpi-build.c +6 -0

     facs->length = cpu_to_le32(sizeof(*facs));
 }
 
+/* GAS */
+static void
+build_gas(struct AcpiGenericAddress *gas, uint8_t space_id,
+          uint8_t bit_width, uint8_t bit_offset,
+          uint8_t access_width, uint64_t address)
+{

hw/i386/acpi-build.c build_facs(GArray *table_data, BIOSLinker *linker)

Quoted 10 lines

hw/i386/acpi-build.c +8 -1

+    gas->space_id = space_id;
+    gas->bit_width = bit_width;
+    gas->bit_offset = bit_offset;
+    gas->access_width = access_width;
+    gas->address = address;
+}
+
 /* Load chipset information in FADT */
-static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)
+static void fadt_setup(AcpiFadtDescriptorRev5_1 *fadt, AcpiPmInfo

*pm,

hw/i386/acpi-build.c static void fadt_setup(AcpiFadtDescriptorRev1

Quoted 5 lines

hw/i386/acpi-build.c +1 -0

+                       uint8_t revision)
 {
     fadt->model = 1;
     fadt->reserved1 = 0;

@@ -309,6 +323,25 @@ static void fadt_setup(AcpiFadtDescriptorRev1

*fadt, AcpiPmInfo *pm)

hw/i386/acpi-build.c static void fadt_setup(AcpiFadtDescriptorRev1 · line 323

Quoted 1 line

hw/i386/acpi-build.c +0 -0

         fadt->flags |= cpu_to_le32(1 <<

ACPI_FADT_F_FORCE_APIC_CLUSTER_MODEL);

Quoted 9 lines

hw/i386/acpi-build.c +7 -0

     }
     fadt->century = RTC_CENTURY;
+
+    /* Build ACPI 2.0 (FADT version 3+) GAS fields */
+    if (revision >= 3) {
+        /* EVT, CNT, TMR register matches hw/acpi/core.c */
+        build_gas(&fadt->xpm1a_event_block, AML_SYSTEM_IO,
+                  32, 0, 0, cpu_to_le64(pm->io_base));
+        build_gas(&fadt->xpm1a_control_block, AML_SYSTEM_IO,

hw/i386/acpi-build.c static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)

Quoted 10 lines

hw/i386/acpi-build.c +10 -0

+                  16, 0, 0, cpu_to_le64(pm->io_base + 0x04));
+        build_gas(&fadt->xpm_timer_block, AML_SYSTEM_IO,
+                  32, 0, 0, cpu_to_le64(pm->io_base + 0x08));
+        build_gas(&fadt->xgpe0_block, AML_SYSTEM_IO,
+                  pm->gpe0_blk_len << 3, 0, 0, cpu_to_le64(pm->gpe0_blk));
+    }
+
+    /* Build dummy ACPI 5.0 fields */
+    if (revision >= 5) {
+        build_gas(&fadt->sleep_control, AML_SYSTEM_MEMORY, 0, 0, 0,

0);

hw/i386/acpi-build.c static void

Quoted 6 lines

hw/i386/acpi-build.c +2 -0

+        build_gas(&fadt->sleep_status, AML_SYSTEM_MEMORY, 0, 0, 0, 0);
+    }
 }
 
 

@@ -316,11 +349,21 @@ static void

fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)

hw/i386/acpi-build.c static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)

Quoted 7 lines

hw/i386/acpi-build.c +1 -2

 static void
 build_fadt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm,
            unsigned facs_tbl_offset, unsigned dsdt_tbl_offset,
-           const char *oem_id, const char *oem_table_id)
+           const char *oem_id, const char *oem_table_id, uint8_t revision)
 {
-    AcpiFadtDescriptorRev1 *fadt = acpi_data_push(table_data,

sizeof(*fadt));

hw/i386/acpi-build.c build_fadt(GArray *table_data, BIOSLinker

Quoted 20 lines

hw/i386/acpi-build.c +13 -2

-    unsigned fw_ctrl_offset = (char *)&fadt->firmware_ctrl - table_data-
 data;
-    unsigned dsdt_entry_offset = (char *)&fadt->dsdt - table_data->data;
+    AcpiFadtDescriptorRev5_1 *fadt;
+    unsigned fw_ctrl_offset;
+    unsigned dsdt_entry_offset;
+    unsigned fadt_len;
+
+    if (revision == 1) {
+        fadt_len = sizeof(AcpiFadtDescriptorRev1);
+    } else {
+        fadt_len = sizeof(AcpiFadtDescriptorRev5_1);
+    }
+    fadt = acpi_data_push(table_data, fadt_len);
+    fw_ctrl_offset = (char *)&fadt->firmware_ctrl - table_data->data;
+    dsdt_entry_offset = (char *)&fadt->dsdt - table_data->data;
 
     /* FACS address to be filled by Guest linker */

bios_linker_loader_add_pointer(linker,

hw/i386/acpi-build.c +0 -0

@@ -328,13 +371,28 @@ build_fadt(GArray *table_data, BIOSLinker

*linker, AcpiPmInfo *pm,

hw/i386/acpi-build.c void acpi_build(AcpiBuildTables *tables,

Quoted 32 lines

hw/i386/acpi-build.c +18 -3

         ACPI_BUILD_TABLE_FILE, facs_tbl_offset);
 
     /* DSDT address to be filled by Guest linker */
-    fadt_setup(fadt, pm);
     bios_linker_loader_add_pointer(linker,
         ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt->dsdt),
         ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
 
-    build_header(linker, table_data,
-                 (void *)fadt, "FACP", sizeof(*fadt), 1, oem_id, oem_table_id);
+    if (revision > 2) {
+        fw_ctrl_offset = (char *)&fadt->Xfacs - table_data->data;
+        dsdt_entry_offset = (char *)&fadt->Xdsdt - table_data->data;
+
+        /* FACS address to be filled by Guest linker */
+        bios_linker_loader_add_pointer(linker,
+            ACPI_BUILD_TABLE_FILE, fw_ctrl_offset, sizeof(fadt->Xfacs),
+            ACPI_BUILD_TABLE_FILE, facs_tbl_offset);
+
+        /* DSDT address to be filled by Guest linker */
+        bios_linker_loader_add_pointer(linker,
+            ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt->Xdsdt),
+            ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
+    }
+
+    fadt_setup(fadt, pm, revision);
+    build_header(linker, table_data, (void *)fadt, "FACP", fadt_len,
+                 revision, oem_id, oem_table_id);
 }
 
 void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,

@@ -2681,7 +2739,7 @@ void acpi_build(AcpiBuildTables *tables,

MachineState *machine)

include/hw/acpi/acpi.h void acpi_update_sci(ACPIREGS *acpi_regs,

Quoted 13 lines

hw/i386/acpi-build.c +1 -1

     fadt = tables_blob->len;
     acpi_add_table(table_offsets, tables_blob);
     build_fadt(tables_blob, tables->linker, &pm, facs, dsdt,
-               slic_oem.id, slic_oem.table_id);
+               slic_oem.id, slic_oem.table_id, acpi_fadt_rev);
     aml_len += tables_blob->len - fadt;
 
     acpi_add_table(table_offsets, tables_blob);

include/hw/acpi/acpi.h +0 -0

index 7b3d93c..63df38d 100644
@@ -173,6 +173,7 @@ void acpi_update_sci(ACPIREGS *acpi_regs,

qemu_irq irq);

qemu-options.hx DEF("acpitable", HAS_ARG,

Quoted 12 lines

include/hw/acpi/acpi.h +1 -0

 /* acpi.c */
 extern int acpi_enabled;
+extern uint8_t acpi_fadt_rev;
 extern char unsigned *acpi_tables;
 extern size_t acpi_tables_len;
 

qemu-options.hx +0 -0

index 5fe7f87..d61dd92 100644
@@ -1497,7 +1497,10 @@ DEF("acpitable", HAS_ARG,

QEMU_OPTION_acpitable,

qemu-options.hx DEF("acpitable", HAS_ARG, QEMU_OPTION_acpitable,

Quoted 10 lines

qemu-options.hx +4 -1

     "             [,sig=str][,rev=n]\n"
     "             [,oem_id=str][,oem_table_id=str][,oem_rev=n]\n"
     "             [,asl_compiler_id=str][,asl_compiler_rev=n]\n"
-    "                ACPI table description\n", QEMU_ARCH_I386)
+    "                ACPI table description\n"
+    "-acpitable fadt=n\n"
+    "                Configure FADT revision\n",
+    QEMU_ARCH_I386)
 STEXI
 @item -acpitable

data=@var{file1}[:@var{file2}]...[,sig=@var{str}][,rev=@var{n}][,oem_id=@ var{str}][,oem_table_id=@var{str}][,oem_rev=@var{n}] [,asl_compiler_id=@var{str}][,asl_compiler_rev=@var{n}]

qemu-options.hx If a SLIC table is supplied to QEMU, then the

Quoted 2 lines

qemu-options.hx +0 -0

 @findex -acpitable

@@ -1511,6 +1514,9 @@ If a SLIC table is supplied to QEMU, then the

SLIC's oem_id and oem_table_id

qemu-options.hx If a SLIC table is supplied to QEMU, then the · line 1514

Quoted 1 line

qemu-options.hx +0 -0

 fields will override the same in the RSDT and the FADT (a.k.a. FACP), in

order

qemu-options.hx If a SLIC table is supplied to QEMU, then the · line 1515

Quoted 1 line

qemu-options.hx +0 -0

 to ensure the field matches required by the Microsoft SLIC spec and the

ACPI

Quoted 7 lines

qemu-options.hx +3 -0

 spec.
+
+@item -acpitable fadt=@var{n}
+Configure FADT revision to 1, 3, 5, default 1.
 ETEXI
 
 DEF("smbios", HAS_ARG, QEMU_OPTION_smbios,

From: Michael S. Tsirkin [mailto:[email protected]]
Subject: Re: [PATCH v5 2/2] ACPI: Add -acpitable fadt= to allow FADT revision changes

Quoted 18 lines · Lv

Hi, Igor

Thanks for the review.

Quoted 14 lines

From: Igor Mammedov [mailto:[email protected]]
Subject: Re: [PATCH v5 2/2] ACPI: Add -acpitable fadt= to allow FADT revision changes

Quoted 1 line · On Thu, 11 Aug 2016 17:36:45 +0800 Lv Zheng <[email protected]> wrote:

This patch allows FADT to be built with different revisions. When the

revision

Quoted 3 lines

is greater than 1.0, 64-bit address fields may also be filled.

Note that FADT revision 2 has never been defined by the ACPI

specification. So

Quoted 1 line

this patch only adds an option -acpitable fadt= to allow revision 1,3,5

to

Quoted 13 lines

Quoted 13 lines

be

Quoted 3 lines

configured by the users.

1. Tested by booting a linux image, the 64-bit addresses are correctly

filled

Quoted 2 lines

in the dumped FADT.
2. Tested by booting a Windows image, no boot failure can be seen.

series still doesn't say why do you need 64-bit addresses in FADT, current Seabios/OVMF allocates tables below 4Gb and gpe/pm register blocks are below 4gb as well so there is no point in accepting theses patches and adding extra CLI options as these patches do.

Quoted 4 lines

[Lv Zheng]
This patch is used by us to probe Windows FADT behavior. Current known behavior includes:
1. On x86, Windows requires BIOS to have 2 FADTs, 1 is in RSDT, and

probably is a v1 table, the other is in XSDT, and probably is a v3+ table.

What does "requires" mean here? It seems to work fine without.

Quoted 2 lines

2. v2 FADT never exists in the spec, it is used in very early years, by IA64

prototype machines.

Quoted 1 line

3. If the FADT in RSDT takes effective, then Windows actually ignores 64-

bit GAS fields, but uses 32-bit register fields.

Quoted 4 lines

But there are still many uncertainties:
1. What if the FADT in XSDT takes effective?
2. What's the behavior on IA64/ARM platforms?
If we could have an option here, we could continue the probing work,

trying every combination.

Quoted 2 lines

Thus the original purpose of this commit is - probing de-facto standard

behavior.

Nice, but why do we need PATCH 2 upstream? Is it hard for you to maintain
out of tree?

Quoted 20 lines

Quoted 15 lines

If we'd ever need to bump FADT revision, I'd first try to increase it unconditionally and test/see if it would not upset legacy guests Windows starting from XP, RHEL4/5/6
If it works then these patches are not necessary,
(this approach worked just fine for bumping up DSDT revision to 2 it would be possible to use 64-bit math in ASL/AML).

if it doesn't, I still won't do it this way but rather:
- make new revision used by default
- add compat property to piix4_pm/ich9-lpc to force legacy revision
- turn on legacy revision for old machine types using added above
property

Quoted 5 lines

That way we won't regress existing setups as old machines will stay the same and only new machine types will be affected. And users that actually want to play with legacy revision on new machine types could force it by using above property.

Quoted 6 lines

[Lv Zheng]
But it seems the code in this commit contains useful stuffs for qemu.

As I said previously, it seems Windows requires 2 FADTs.
So having a revision parameter for build_fadt() seems to be necessary. Because if qemu wants to increase FADT revision, qemu may do this in

this way:

Quoted 3 lines

1. Invoke build_fadt() with revision 1, and put it into RSDT;
2. Invoke build_fadt() with latest revision, and put it into XSDT. Also we know latest revision FADT is useful, it contains reduced

hardware support.

Quoted 3 lines

And some of new ACPI features rely this model.

So I'm sure that this commit contains useful stuffs for qemu to do future

extension.

Quoted 4 lines

But I don't know if you want to keep the new -acpitable fadt= option. Let me ask:
1. Are there any issues in PATCH 01? Maybe my understanding of qemu

option code is not correct.

Didn't review yet, I'll let Igor review first.

Quoted 1 line

2. IMO, this option should be useful for a long period because we need it

to help probing Windows behavior, so can we have it upstreamed?

ATM PATCH 2 does not seem generally useful.

Quoted 30 lines

Best regards
Lv

Quoted 25 lines

Quoted 5 lines

Signed-off-by Lv Zheng <[email protected]>

---
 hw/acpi/core.c         |   20 ++++++++++++-
 hw/i386/acpi-build.c   |   76

++++++++++++++++++++++++++++++++++++++++++------

hw/acpi/core.c static const char unsigned

Quoted 17 lines

 include/hw/acpi/acpi.h |    1 +
 qemu-options.hx        |    8 ++++-
 4 files changed, 94 insertions(+), 11 deletions(-)

hw/acpi/core.c +1 -0

index 85e0e94..832c86b 100644
@@ -19,6 +19,7 @@
  * GNU GPL, version 2 or (at your option) any later version.
  */
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 #include "sysemu/sysemu.h"
 #include "hw/hw.h"
 #include "hw/i386/pc.h"

@@ -54,6 +55,7 @@ static const char unsigned

dfl_hdr[ACPI_TABLE_HDR_SIZE - ACPI_TABLE_PFX_SIZE] =

Quoted 1 line

Quoted 7 lines

Quoted 7 lines

hw/acpi/core.c void acpi_table_add(const QemuOpts

Quoted 7 lines

char unsigned *acpi_tables;
size_t acpi_tables_len;

hw/acpi/core.c +1 -0

+uint8_t acpi_fadt_rev = 1;
 
 static QemuOptsList qemu_acpi_opts = {
     .name = "acpi",

@@ -312,7 +314,23 @@ void acpi_table_add(const QemuOpts

*opts,

Quoted 34 lines

Quoted 34 lines

Error **errp)

hw/i386/acpi-build.c build_facs(GArray *table_data, BIOSLinker

Quoted 29 lines

hw/acpi/core.c +17 -1

         return;
     }
 
-    error_setg(errp, "'-acpitable' requires one of 'data' or 'file'");
+    val = qemu_opt_get((QemuOpts *)opts, "fadt");
+    if (val) {
+        unsigned long rev;
+        int err;
+
+        err = qemu_strtoul(val, NULL, 10, &rev);
+        if (err ||
+            (rev != 1 && rev != 3 && rev != 5)) {
+            error_setg(errp, "Unsupported FADT revision %s, "
+                       "please specify 1,3,5", val);
+            return;
+        }
+        acpi_fadt_rev = rev;
+        return;
+    }
+
+    error_setg(errp, "'-acpitable' requires one of 'data','file' or 'fadt'");
 }
 
 static bool acpi_table_builtin = false;

hw/i386/acpi-build.c +0 -0

index ce7cbc5..8be6578 100644
@@ -276,8 +276,22 @@ build_facs(GArray *table_data, BIOSLinker

*linker)

Quoted 3 lines

hw/i386/acpi-build.c +0 -0

     facs->length = cpu_to_le32(sizeof(*facs));
 }
 

Quoted 15 lines

Quoted 7 lines

Quoted 7 lines

hw/i386/acpi-build.c +7 -0

+/* GAS */
+static void
+build_gas(struct AcpiGenericAddress *gas, uint8_t space_id,
+          uint8_t bit_width, uint8_t bit_offset,
+          uint8_t access_width, uint64_t address)
+{
+    gas->space_id = space_id;

Quoted 8 lines

hw/i386/acpi-build.c build_facs(GArray *table_data, BIOSLinker *linker)

Quoted 8 lines

hw/i386/acpi-build.c +6 -1

+    gas->bit_width = bit_width;
+    gas->bit_offset = bit_offset;
+    gas->access_width = access_width;
+    gas->address = address;
+}
+
 /* Load chipset information in FADT */
-static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo

*pm)

Quoted 7 lines

Quoted 7 lines

hw/i386/acpi-build.c build_facs(GArray *table_data, BIOSLinker · line 293

Quoted 1 line

hw/i386/acpi-build.c +1 -0

+static void fadt_setup(AcpiFadtDescriptorRev5_1 *fadt, AcpiPmInfo

*pm,

hw/i386/acpi-build.c static void

Quoted 5 lines

hw/i386/acpi-build.c +1 -0

+                       uint8_t revision)
 {
     fadt->model = 1;
     fadt->reserved1 = 0;

@@ -309,6 +323,25 @@ static void

fadt_setup(AcpiFadtDescriptorRev1

Quoted 17 lines

Quoted 17 lines

*fadt, AcpiPmInfo *pm)

hw/i386/acpi-build.c static void · line 323

Quoted 1 line

hw/i386/acpi-build.c +0 -0

         fadt->flags |= cpu_to_le32(1 <<

ACPI_FADT_F_FORCE_APIC_CLUSTER_MODEL);

Quoted 14 lines

hw/i386/acpi-build.c +12 -0

     }
     fadt->century = RTC_CENTURY;
+
+    /* Build ACPI 2.0 (FADT version 3+) GAS fields */
+    if (revision >= 3) {
+        /* EVT, CNT, TMR register matches hw/acpi/core.c */
+        build_gas(&fadt->xpm1a_event_block, AML_SYSTEM_IO,
+                  32, 0, 0, cpu_to_le64(pm->io_base));
+        build_gas(&fadt->xpm1a_control_block, AML_SYSTEM_IO,
+                  16, 0, 0, cpu_to_le64(pm->io_base + 0x04));
+        build_gas(&fadt->xpm_timer_block, AML_SYSTEM_IO,
+                  32, 0, 0, cpu_to_le64(pm->io_base + 0x08));
+        build_gas(&fadt->xgpe0_block, AML_SYSTEM_IO,
+                  pm->gpe0_blk_len << 3, 0, 0, cpu_to_le64(pm-

Quoted 6 lines

gpe0_blk));

Quoted 5 lines

Quoted 5 lines

hw/i386/acpi-build.c +5 -0

+    }
+
+    /* Build dummy ACPI 5.0 fields */
+    if (revision >= 5) {
+        build_gas(&fadt->sleep_control, AML_SYSTEM_MEMORY, 0, 0,

0,

Quoted 2 lines

Quoted 2 lines

0);

Quoted 1 line

hw/i386/acpi-build.c +1 -0

+        build_gas(&fadt->sleep_status, AML_SYSTEM_MEMORY, 0, 0,

0, 0);

Quoted 11 lines

Quoted 11 lines

hw/i386/acpi-build.c static void

Quoted 5 lines

hw/i386/acpi-build.c +1 -0

+    }
 }
 
 

@@ -316,11 +349,21 @@ static void

fadt_setup(AcpiFadtDescriptorRev1 *fadt, AcpiPmInfo *pm)

Quoted 5 lines

hw/i386/acpi-build.c +1 -1

 static void
 build_fadt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm,
            unsigned facs_tbl_offset, unsigned dsdt_tbl_offset,
-           const char *oem_id, const char *oem_table_id)
+           const char *oem_id, const char *oem_table_id, uint8_t

revision)

Quoted 4 lines

Quoted 4 lines

hw/i386/acpi-build.c static void · line 321

Quoted 2 lines

hw/i386/acpi-build.c +0 -1

 {
-    AcpiFadtDescriptorRev1 *fadt = acpi_data_push(table_data,

sizeof(*fadt));

Quoted 1 line

hw/i386/acpi-build.c +0 -1

-    unsigned fw_ctrl_offset = (char *)&fadt->firmware_ctrl -

table_data-

Quoted 17 lines

Quoted 2 lines

hw/i386/acpi-build.c static void · line 324

Quoted 2 lines

hw/i386/acpi-build.c +0 -1

 data;
-    unsigned dsdt_entry_offset = (char *)&fadt->dsdt - table_data-

data;

Quoted 14 lines

Quoted 14 lines

hw/i386/acpi-build.c +13 -0

+    AcpiFadtDescriptorRev5_1 *fadt;
+    unsigned fw_ctrl_offset;
+    unsigned dsdt_entry_offset;
+    unsigned fadt_len;
+
+    if (revision == 1) {
+        fadt_len = sizeof(AcpiFadtDescriptorRev1);
+    } else {
+        fadt_len = sizeof(AcpiFadtDescriptorRev5_1);
+    }
+    fadt = acpi_data_push(table_data, fadt_len);
+    fw_ctrl_offset = (char *)&fadt->firmware_ctrl - table_data->data;
+    dsdt_entry_offset = (char *)&fadt->dsdt - table_data->data;
 

Quoted 3 lines

Quoted 3 lines

hw/i386/acpi-build.c build_fadt(GArray *table_data,

Quoted 3 lines

hw/i386/acpi-build.c +0 -0

     /* FACS address to be filled by Guest linker */

bios_linker_loader_add_pointer(linker,

hw/i386/acpi-build.c +0 -0

@@ -328,13 +371,28 @@ build_fadt(GArray *table_data,

BIOSLinker

Quoted 11 lines

Quoted 11 lines

*linker, AcpiPmInfo *pm,

Quoted 10 lines

hw/i386/acpi-build.c +0 -3

         ACPI_BUILD_TABLE_FILE, facs_tbl_offset);
 
     /* DSDT address to be filled by Guest linker */
-    fadt_setup(fadt, pm);
     bios_linker_loader_add_pointer(linker,
         ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt->dsdt),
         ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
 
-    build_header(linker, table_data,
-                 (void *)fadt, "FACP", sizeof(*fadt), 1, oem_id,

oem_table_id);

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

hw/i386/acpi-build.c +5 -0

+    if (revision > 2) {
+        fw_ctrl_offset = (char *)&fadt->Xfacs - table_data->data;
+        dsdt_entry_offset = (char *)&fadt->Xdsdt - table_data->data;
+
+        /* FACS address to be filled by Guest linker */

Quoted 57 lines

Quoted 7 lines

hw/i386/acpi-build.c build_fadt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm,

Quoted 7 lines

hw/i386/acpi-build.c +7 -0

+        bios_linker_loader_add_pointer(linker,
+            ACPI_BUILD_TABLE_FILE, fw_ctrl_offset, sizeof(fadt->Xfacs),
+            ACPI_BUILD_TABLE_FILE, facs_tbl_offset);
+
+        /* DSDT address to be filled by Guest linker */
+        bios_linker_loader_add_pointer(linker,
+            ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt-

Xdsdt),

Quoted 49 lines

hw/i386/acpi-build.c void acpi_build(AcpiBuildTables *tables,

Quoted 10 lines

hw/i386/acpi-build.c +6 -0

+            ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
+    }
+
+    fadt_setup(fadt, pm, revision);
+    build_header(linker, table_data, (void *)fadt, "FACP", fadt_len,
+                 revision, oem_id, oem_table_id);
 }
 
 void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,

@@ -2681,7 +2739,7 @@ void acpi_build(AcpiBuildTables *tables,

MachineState *machine)

include/hw/acpi/acpi.h void acpi_update_sci(ACPIREGS *acpi_regs,

Quoted 13 lines

hw/i386/acpi-build.c +1 -1

     fadt = tables_blob->len;
     acpi_add_table(table_offsets, tables_blob);
     build_fadt(tables_blob, tables->linker, &pm, facs, dsdt,
-               slic_oem.id, slic_oem.table_id);
+               slic_oem.id, slic_oem.table_id, acpi_fadt_rev);
     aml_len += tables_blob->len - fadt;
 
     acpi_add_table(table_offsets, tables_blob);

include/hw/acpi/acpi.h +0 -0

index 7b3d93c..63df38d 100644
@@ -173,6 +173,7 @@ void acpi_update_sci(ACPIREGS *acpi_regs,

qemu_irq irq);

qemu-options.hx DEF("acpitable", HAS_ARG,

Quoted 12 lines

include/hw/acpi/acpi.h +1 -0

 /* acpi.c */
 extern int acpi_enabled;
+extern uint8_t acpi_fadt_rev;
 extern char unsigned *acpi_tables;
 extern size_t acpi_tables_len;
 

qemu-options.hx +0 -0

index 5fe7f87..d61dd92 100644
@@ -1497,7 +1497,10 @@ DEF("acpitable", HAS_ARG,

QEMU_OPTION_acpitable,

Quoted 10 lines

qemu-options.hx +4 -1

     "             [,sig=str][,rev=n]\n"
     "             [,oem_id=str][,oem_table_id=str][,oem_rev=n]\n"
     "             [,asl_compiler_id=str][,asl_compiler_rev=n]\n"
-    "                ACPI table description\n", QEMU_ARCH_I386)
+    "                ACPI table description\n"
+    "-acpitable fadt=n\n"
+    "                Configure FADT revision\n",
+    QEMU_ARCH_I386)
 STEXI
 @item -acpitable

data=@var{file1}[:@var{file2}]...[,sig=@var{str}][,rev=@var{n}][,oem_id=@

Quoted 4 lines

Quoted 4 lines

var{str}][,oem_table_id=@var{str}][,oem_rev=@var{n}] [,asl_compiler_id=@var{str}][,asl_compiler_rev=@var{n}]

qemu-options.hx If a SLIC table is supplied to QEMU, then

Quoted 2 lines

qemu-options.hx +0 -0

 @findex -acpitable

@@ -1511,6 +1514,9 @@ If a SLIC table is supplied to QEMU, then

the

Quoted 2 lines

Quoted 2 lines

SLIC's oem_id and oem_table_id

Quoted 1 line

qemu-options.hx +0 -0

 fields will override the same in the RSDT and the FADT (a.k.a. FACP),

in

Quoted 2 lines

Quoted 2 lines

order

Quoted 1 line

qemu-options.hx +0 -0

 to ensure the field matches required by the Microsoft SLIC spec and

the

Quoted 8 lines

Quoted 8 lines

ACPI

Quoted 7 lines

qemu-options.hx +3 -0

 spec.
+
+@item -acpitable fadt=@var{n}
+Configure FADT revision to 1, 3, 5, default 1.
 ETEXI
 
 DEF("smbios", HAS_ARG, QEMU_OPTION_smbios,