[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 4/4] pc: set the OEM fields in the RSDT and the
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-devel] [PATCH 4/4] pc: set the OEM fields in the RSDT and the FADT from the SLIC |
Date: |
Thu, 14 Jan 2016 17:44:53 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 |
On 01/14/16 11:24, Michael S. Tsirkin wrote:
> On Thu, Jan 14, 2016 at 02:36:57AM +0100, Laszlo Ersek wrote:
>> The Microsoft spec about the SLIC and MSDM ACPI tables at
>> <http://go.microsoft.com/fwlink/p/?LinkId=234834> requires the OEM ID and
>> OEM Table ID fields to be consistent between the SLIC and the RSDT/XSDT.
>> That further affects the FADT, because a similar match between the FADT
>> and the RSDT/XSDT is required by the ACPI spec in general.
>>
>> The stashed SLIC OEM identifiers can be ignored with the new
>>
>> -machine heed-slic-oem=no
>>
>> option.
>
> I'd prefer "use-slic-oem".
That is, an option with the opposite meaning. Sounds doable.
Would you like me to give that option a default value that had the
equivalent effect? (That is, use-slic-oem=on by default?) If not, that
might cause further trouble for users (they would have to figure out one
more option).
> But do we really expect people to use this?
> Less knobs would be better ...
I don't disagree; I only did this because you had suggested it in the
earlier discussion:
http://thread.gmane.org/gmane.comp.emulators.qemu/358854/focus=359239
Perhaps I misunderstood. FWIW, the SLIC spec from Microsoft is
unambiguous about the OEM identity between SLIC and RSDT/XSDT (and
consequently the FADT): that requirement is not optional. Turning off
the OEM matching is therefore arguably useless, if the user provides a SLIC.
I can certainly drop this knob if you think that's best.
Thanks!
Laszlo
>
>> Cc: "Michael S. Tsirkin" <address@hidden> (supporter:ACPI/SMBIOS)
>> Cc: Igor Mammedov <address@hidden> (supporter:ACPI/SMBIOS)
>> Cc: Paolo Bonzini <address@hidden> (maintainer:X86)
>> Cc: Richard W.M. Jones <address@hidden>
>> Cc: Aleksei Kovura <address@hidden>
>> Cc: Michael Tokarev <address@hidden>
>> RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1248758
>> Signed-off-by: Laszlo Ersek <address@hidden>
>> ---
>> include/hw/i386/pc.h | 2 ++
>> hw/i386/acpi-build.c | 22 ++++++++++++++++++----
>> hw/i386/pc.c | 19 +++++++++++++++++++
>> qemu-options.hx | 10 +++++++++-
>> 4 files changed, 48 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>> index 588a33c..a762c29 100644
>> --- a/include/hw/i386/pc.h
>> +++ b/include/hw/i386/pc.h
>> @@ -56,6 +56,7 @@ struct PCMachineState {
>> OnOffAuto vmport;
>> OnOffAuto smm;
>> bool nvdimm;
>> + bool heed_slic_oem;
>>
>> /* RAM information (sizes, addresses, configuration): */
>> ram_addr_t below_4g_mem_size, above_4g_mem_size;
>> @@ -67,6 +68,7 @@ struct PCMachineState {
>> #define PC_MACHINE_VMPORT "vmport"
>> #define PC_MACHINE_SMM "smm"
>> #define PC_MACHINE_NVDIMM "nvdimm"
>> +#define PC_MACHINE_HEED_SLIC_OEM "heed-slic-oem"
>>
>> /**
>> * PCMachineClass:
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index 6408362..cf2aafc 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -337,7 +337,8 @@ static void fadt_setup(AcpiFadtDescriptorRev1 *fadt,
>> AcpiPmInfo *pm)
>> /* FADT */
>> static void
>> build_fadt(GArray *table_data, GArray *linker, AcpiPmInfo *pm,
>> - unsigned facs, unsigned dsdt)
>> + unsigned facs, unsigned dsdt,
>> + const char *oem_id, const char *oem_table_id)
>> {
>> AcpiFadtDescriptorRev1 *fadt = acpi_data_push(table_data,
>> sizeof(*fadt));
>>
>> @@ -358,7 +359,7 @@ build_fadt(GArray *table_data, GArray *linker,
>> AcpiPmInfo *pm,
>> fadt_setup(fadt, pm);
>>
>> build_header(linker, table_data,
>> - (void *)fadt, "FACP", sizeof(*fadt), 1, NULL, NULL);
>> + (void *)fadt, "FACP", sizeof(*fadt), 1, oem_id,
>> oem_table_id);
>> }
>>
>> static void
>> @@ -2621,6 +2622,17 @@ void acpi_build(PcGuestInfo *guest_info,
>> AcpiBuildTables *tables)
>> uint8_t *u;
>> size_t aml_len = 0;
>> GArray *tables_blob = tables->table_data;
>> + char *slic_oem_id = NULL;
>> + char *slic_oem_table_id = NULL;
>> + PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
>> + bool heed_slic_oem = object_property_get_bool(OBJECT(pcms),
>> + PC_MACHINE_HEED_SLIC_OEM,
>> + &error_abort);
>> +
>> + if (heed_slic_oem) {
>> + slic_oem_id = acpi_slic_oem_id;
>> + slic_oem_table_id = acpi_slic_oem_table_id;
>> + }
>>
>> acpi_get_cpu_info(&cpu);
>> acpi_get_pm_info(&pm);
>> @@ -2654,7 +2666,8 @@ void acpi_build(PcGuestInfo *guest_info,
>> AcpiBuildTables *tables)
>>
>> /* ACPI tables pointed to by RSDT */
>> acpi_add_table(table_offsets, tables_blob);
>> - build_fadt(tables_blob, tables->linker, &pm, facs, dsdt);
>> + build_fadt(tables_blob, tables->linker, &pm, facs, dsdt,
>> + slic_oem_id, slic_oem_table_id);
>>
>> ssdt = tables_blob->len;
>> acpi_add_table(table_offsets, tables_blob);
>> @@ -2705,7 +2718,8 @@ void acpi_build(PcGuestInfo *guest_info,
>> AcpiBuildTables *tables)
>>
>> /* RSDT is pointed to by RSDP */
>> rsdt = tables_blob->len;
>> - build_rsdt(tables_blob, tables->linker, table_offsets, NULL, NULL);
>> + build_rsdt(tables_blob, tables->linker, table_offsets,
>> + slic_oem_id, slic_oem_table_id);
>>
>> /* RSDP is in FSEG memory, so allocate it separately */
>> build_rsdp(tables->rsdp, tables->linker, rsdt);
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index c36b8cf..3e7a72a 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -1887,6 +1887,20 @@ static void pc_machine_set_nvdimm(Object *obj, bool
>> value, Error **errp)
>> pcms->nvdimm = value;
>> }
>>
>> +static bool pc_machine_get_heed_slic_oem(Object *obj, Error **errp)
>> +{
>> + PCMachineState *pcms = PC_MACHINE(obj);
>> +
>> + return pcms->heed_slic_oem;
>> +}
>> +
>> +static void pc_machine_set_heed_slic_oem(Object *obj, bool value, Error
>> **errp)
>> +{
>> + PCMachineState *pcms = PC_MACHINE(obj);
>> +
>> + pcms->heed_slic_oem = value;
>> +}
>> +
>> static void pc_machine_initfn(Object *obj)
>> {
>> PCMachineState *pcms = PC_MACHINE(obj);
>> @@ -1926,6 +1940,11 @@ static void pc_machine_initfn(Object *obj)
>> pcms->nvdimm = false;
>> object_property_add_bool(obj, PC_MACHINE_NVDIMM, pc_machine_get_nvdimm,
>> pc_machine_set_nvdimm, &error_abort);
>> +
>> + pcms->heed_slic_oem = true;
>> + object_property_add_bool(obj, PC_MACHINE_HEED_SLIC_OEM,
>> + pc_machine_get_heed_slic_oem,
>> + pc_machine_set_heed_slic_oem, &error_abort);
>> }
>>
>> static void pc_machine_reset(void)
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 215d00d..e49964c 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -43,7 +43,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
>> " aes-key-wrap=on|off controls support for AES key
>> wrapping (default=on)\n"
>> " dea-key-wrap=on|off controls support for DEA key
>> wrapping (default=on)\n"
>> " suppress-vmdesc=on|off disables self-describing
>> migration (default=off)\n"
>> - " nvdimm=on|off controls NVDIMM support (default=off)\n",
>> + " nvdimm=on|off controls NVDIMM support (default=off)\n"
>> + " heed_slic_oem=on|off adapts RSDT and FADT OEM
>> identifiers to external SLIC (default=on)\n",
>> QEMU_ARCH_ALL)
>> STEXI
>> @item -machine address@hidden,address@hidden,...]]
>> @@ -84,6 +85,13 @@ controls whether DEA wrapping keys will be created to
>> allow
>> execution of DEA cryptographic functions. The default is on.
>> @item nvdimm=on|off
>> Enables or disables NVDIMM support. The default is off.
>> address@hidden heed_slic_oem=on|off
>> +If the user provides an external SLIC ACPI table with the -acpitable option,
>> +then heed_slic_oem=on will adapt the OEM ID and OEM Table ID fields of the
>> +auto-generated RSDT and FADT tables to the same fields in the external SLIC.
>> +When heed_slic_oem is turned off, the RSDT and FADT tables will have
>> general,
>> +QEMU-branded OEM ID and OEM Table ID values. The default is on.
>> heed_slic_oem
>> +makes no difference if no SLIC table is provided by the user.
>> @end table
>> ETEXI
>>
>> --
>> 1.8.3.1