[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 18:09:30 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 |
On 01/14/16 17:44, Laszlo Ersek wrote:
> On 01/14/16 11:24, Michael S. Tsirkin wrote:
>> On Thu, Jan 14, 2016 at 02:36:57AM +0100, Laszlo Ersek wrote:
>>> The Microsoft spec about the SLIC and MSDM ACPI tables at
>>> <http://go.microsoft.com/fwlink/p/?LinkId=234834> requires the OEM ID and
>>> OEM Table ID fields to be consistent between the SLIC and the RSDT/XSDT.
>>> That further affects the FADT, because a similar match between the FADT
>>> and the RSDT/XSDT is required by the ACPI spec in general.
>>>
>>> The stashed SLIC OEM identifiers can be ignored with the new
>>>
>>> -machine heed-slic-oem=no
>>>
>>> option.
>>
>> I'd prefer "use-slic-oem".
>
> That is, an option with the opposite meaning. Sounds doable.
>
> Would you like me to give that option a default value that had the
> equivalent effect? (That is, use-slic-oem=on by default?) If not, that
> might cause further trouble for users (they would have to figure out one
> more option).
Ugh, nevermind. You suggested another option name, but with the *same*
meaning. Okay, that's really easy to change.
But the more foundational question below remains open.
Thanks
Laszlo
>> But do we really expect people to use this?
>> Less knobs would be better ...
>
> I don't disagree; I only did this because you had suggested it in the
> earlier discussion:
>
> http://thread.gmane.org/gmane.comp.emulators.qemu/358854/focus=359239
>
> Perhaps I misunderstood. FWIW, the SLIC spec from Microsoft is
> unambiguous about the OEM identity between SLIC and RSDT/XSDT (and
> consequently the FADT): that requirement is not optional. Turning off
> the OEM matching is therefore arguably useless, if the user provides a SLIC.
>
> I can certainly drop this knob if you think that's best.
>
> Thanks!
> Laszlo
>
>>
>>> Cc: "Michael S. Tsirkin" <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
>
>
- [Qemu-devel] [PATCH 0/4] set the OEM fields in the RSDT and the FADT from the SLIC, Laszlo Ersek, 2016/01/13
- Re: [Qemu-devel] [PATCH 0/4] set the OEM fields in the RSDT and the FADT from the SLIC, Richard W.M. Jones, 2016/01/14
- Re: [Qemu-devel] [PATCH 0/4] set the OEM fields in the RSDT and the FADT from the SLIC, Alex, 2016/01/14
- Re: [Qemu-devel] [PATCH 0/4] set the OEM fields in the RSDT and the FADT from the SLIC, Richard W.M. Jones, 2016/01/21
- Re: [Qemu-devel] [PATCH 0/4] set the OEM fields in the RSDT and the FADT from the SLIC, Michael Tokarev, 2016/01/21
- Re: [Qemu-devel] [PATCH 0/4] set the OEM fields in the RSDT and the FADT from the SLIC, Richard W.M. Jones, 2016/01/21
- Re: [Qemu-devel] [PATCH 0/4] set the OEM fields in the RSDT and the FADT from the SLIC, Laszlo Ersek, 2016/01/21