On Wed, 11 Mar 2020 19:08:26 +0200
Liran Alon <liran.a...@oracle.com> wrote:
> From: Elad Gabay <elad.ga...@oracle.com>
>
> Microsoft introduced this ACPI table to avoid Windows guests performing
> various workarounds for device erratas. As the virtual device emulated
> by VMM may not have the errata.
>
> Currently, WAET allows hypervisor to inform guest about two
> specific behaviors: One for RTC and the other for ACPI PM Timer.
>
> Support for WAET have been introduced since Windows Vista. This ACPI
> table is also exposed by other hypervisors, such as VMware, by default.
>
> This patch adds WAET ACPI Table to QEMU. It also makes sure to introduce
> the new ACPI table only for new machine-types.
in addition to comments made by Michael ...
>
> Signed-off-by: Elad Gabay <elad.ga...@oracle.com>
> Co-developed-by: Liran Alon <liran.a...@oracle.com>
> Signed-off-by: Liran Alon <liran.a...@oracle.com>
> ---
> hw/i386/acpi-build.c | 18 ++++++++++++++++++
> hw/i386/pc_piix.c | 2 ++
> hw/i386/pc_q35.c | 2 ++
> include/hw/acpi/acpi-defs.h | 25 +++++++++++++++++++++++++
> include/hw/i386/pc.h | 1 +
> 5 files changed, 48 insertions(+)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 9c4e46fa7466..29f70741cd96 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2512,6 +2512,19 @@ build_dmar_q35(GArray *table_data, BIOSLinker *linker)
> build_header(linker, table_data, (void *)(table_data->data + dmar_start),
> "DMAR", table_data->len - dmar_start, 1, NULL, NULL);
> }
> +
> +static void
> +build_waet(GArray *table_data, BIOSLinker *linker)
see build_hmat_lb() for example how to doc comment for such function
should look like. Use earliest spec version where table was introduced.
> +{
> + AcpiTableWaet *waet;
> +
> + waet = acpi_data_push(table_data, sizeof(*waet));
> + waet->emulated_device_flags = cpu_to_le32(ACPI_WAET_PM_TIMER_GOOD);
we don't use packed structures for building ACPI tables anymore (there is
old code that still does but that's being converted when we touch it)
pls use build_append_int_noprefix() api instead, see build_amd_iommu() as
an example how to build binary tables using it and how to use comments
to document fields.
Basic idea is that api makes function building a table match table's
description in spec (each call represents a row in spec) and comment
belonging to a row should contain verbatim field name as used by spec
so reader could copy/past and grep it easily.
> +
> + build_header(linker, table_data,
> + (void *)waet, "WAET", sizeof(*waet), 1, NULL, NULL);
> +}
> +
> /*
> * IVRS table as specified in AMD IOMMU Specification v2.62, Section 5.2
> * accessible here http://support.amd.com/TechDocs/48882_IOMMU.pdf
> @@ -2859,6 +2872,11 @@ void acpi_build(AcpiBuildTables *tables, MachineState
> *machine)
> machine->nvdimms_state, machine->ram_slots);
> }
>
> + if (!pcmc->do_not_add_waet_acpi) {
> + acpi_add_table(table_offsets, tables_blob);
> + build_waet(tables_blob, tables->linker);
> + }
we typically do not version ACPI table changes (there might be exceptions
but it should be a justified one).
ACPI tables are considered to be a part of firmware (even though they are
generated by QEMU) so on QEMU upgrade user gets a new firmware along with
new ACPI tables.
> +
> /* Add tables supplied by user (if any) */
> for (u = acpi_table_first(); u; u = acpi_table_next(u)) {
> unsigned len = acpi_table_len(u);
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 9088db8fb601..2d11a8b50a9c 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -432,9 +432,11 @@ DEFINE_I440FX_MACHINE(v5_0, "pc-i440fx-5.0", NULL,
>
> static void pc_i440fx_4_2_machine_options(MachineClass *m)
> {
> + PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> pc_i440fx_5_0_machine_options(m);
> m->alias = NULL;
> m->is_default = false;
> + pcmc->do_not_add_waet_acpi = true;
> compat_props_add(m->compat_props, hw_compat_4_2, hw_compat_4_2_len);
> compat_props_add(m->compat_props, pc_compat_4_2, pc_compat_4_2_len);
> }
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 84cf925cf43a..1e0a726b27a7 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -361,8 +361,10 @@ DEFINE_Q35_MACHINE(v5_0, "pc-q35-5.0", NULL,
>
> static void pc_q35_4_2_machine_options(MachineClass *m)
> {
> + PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> pc_q35_5_0_machine_options(m);
> m->alias = NULL;
> + pcmc->do_not_add_waet_acpi = true;
> compat_props_add(m->compat_props, hw_compat_4_2, hw_compat_4_2_len);
> compat_props_add(m->compat_props, pc_compat_4_2, pc_compat_4_2_len);
> }
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index 57a3f58b0c9a..803c904471d5 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -634,4 +634,29 @@ struct AcpiIortRC {
> } QEMU_PACKED;
> typedef struct AcpiIortRC AcpiIortRC;
>
> +/*
> + * Windows ACPI Emulated Devices Table.
> + * Specification:
> + *
> http://download.microsoft.com/download/7/E/7/7E7662CF-CBEA-470B-A97E-CE7CE0D98DC2/WAET.docx
> + */
> +
> +/*
> + * Indicates whether the RTC has been enhanced not to require acknowledgment
> + * after it asserts an interrupt. With this bit set, an interrupt handler can
> + * bypass reading the RTC register C to unlatch the pending interrupt.
> + */
> +#define ACPI_WAET_RTC_GOOD (1 << 0)
> +/*
> + * Indicates whether the ACPI PM timer has been enhanced not to require
> + * multiple reads. With this bit set, only one read of the ACPI PM timer is
> + * necessary to obtain a reliable value.
> + */
> +#define ACPI_WAET_PM_TIMER_GOOD (1 << 1)
> +
> +struct AcpiTableWaet {
> + ACPI_TABLE_HEADER_DEF
> + uint32_t emulated_device_flags;
> +} QEMU_PACKED;
> +typedef struct AcpiTableWaet AcpiTableWaet;
> +
> #endif
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 60c988c4a5aa..f1f64e8f45c8 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -100,6 +100,7 @@ typedef struct PCMachineClass {
> int legacy_acpi_table_size;
> unsigned acpi_data_size;
> bool do_not_add_smb_acpi;
> + bool do_not_add_waet_acpi;
>
> /* SMBIOS compat: */
> bool smbios_defaults;
