[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 01/10] ACPI: Add a function for building name
From: |
Ben Warren |
Subject: |
Re: [Qemu-devel] [PATCH v5 01/10] ACPI: Add a function for building named qword entries |
Date: |
Wed, 8 Feb 2017 12:24:10 -0800 |
> On Feb 8, 2017, at 2:53 AM, Laszlo Ersek <address@hidden> wrote:
>
> On 02/08/17 11:43, Igor Mammedov wrote:
>> On Tue, 7 Feb 2017 21:09:27 +0100
>> Laszlo Ersek <address@hidden> wrote:
>>
>>> On 02/07/17 14:51, Igor Mammedov wrote:
>>>> On Sun, 5 Feb 2017 01:11:56 -0800
>>>> address@hidden wrote:
>>>>
>>>>> From: Ben Warren <address@hidden>
>>>>>
>>>>> This is initially used to patch a 64-bit address into
>>>>> the VM Generation ID SSDT
>>>>>
>>>>> Signed-off-by: Ben Warren <address@hidden>
>>>>> ---
>>>> ...
>>>>> +int
>>>>> +build_append_named_qword(GArray *array, const char *name_format, ...)
>>>> it ain't used anywhere, I'd just drop this patch.
>>>
>>> Ben and I discussed this under
>>> - msgid <address@hidden>
>>> https://www.mail-archive.com/address@hidden/msg425496.html
>>> - msgid <address@hidden>
>>> https://www.mail-archive.com/address@hidden/msg425519.html
>>>
>>> On 01/26/17 06:35, Ben Warren wrote:
>>>> I propose to still include this patch but touch up the comments as
>>>> requested by Laszlo. This way it will be in the toolbox for future
>>>> users and has been tested. [...]
>>>
>>> I generally agree that dead code is undesirable, but this function has
>>> surfaced several times until now, and we get to review it every single
>>> time. Ben tested it, I support its inclusion.
>>>
>>> OTOH I also pointed it out to Ben
>>>
>>> https://www.mail-archive.com/address@hidden/msg425218.html
>>>
>>> that he should expect disagreement between his reviewers :) Given that
>>> I'm observing this series more from the sidelines and you maintain /
>>> support ACPI gen in QEMU, I certainly defer to you on this.
>> It's not only dead, having patchable QWORD ready for use in QEMU
>> would tempt someone to use it and that would lead to XP BSOD
>> if it slips screening at review time.
>
> Good point.
>
>> I'd delay this patch until
>> we announce that ACPI 1.0 (XP based) guest no more supported by
>> new QEMU.
>>
>> Anyway I won't object to merging this if you insist and give it your RB.
>
> No, you are entirely right. As long as we consider Windows XP guests
> first class citizens (i.e., we even reject the idea of additional
> command line options), then keeping QWORD "out of sight" is safer.
>
> Follow up question: when are we going to drop Windows XP? Do we have a
> plan? :) It's quite a struggle to support both ends of the spectrum with
> the same machine types and options (decades-old DOS and Windows XP vs.
> supercomputer-class NUMA setups). I think this limits development.
>
> (Just curious; I'm not suggesting to drop Windows XP "soon".)
>
> Thanks!
> Laszlo
>
OK, I’ll drop this patch from the set. I guess it will always be around if we
want to resurrect.
>
>>>
>>>
>>>>> +{
>>>>> + int offset;
>>>>> + va_list ap;
>>>>> +
>>>>> + build_append_byte(array, 0x08); /* NameOp */
>>>>> + va_start(ap, name_format);
>>>>> + build_append_namestringv(array, name_format, ap);
>>>>> + va_end(ap);
>>>>> +
>>>>> + build_append_byte(array, 0x0E); /* QWordPrefix */
>>>>> +
>>>>> + offset = array->len;
>>>>> + build_append_int_noprefix(array, 0x0000000000000000, 8);
>>>>> + assert(array->len == offset + 8);
>>>>> +
>>>>> + return offset;
>>>>> +}
>>>>> +
>>>>> static GPtrArray *alloc_list;
>>>>>
>>>>> static Aml *aml_alloc(void)
>>>>> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
>>>>> index 559326c..dbf63cf 100644
>>>>> --- a/include/hw/acpi/aml-build.h
>>>>> +++ b/include/hw/acpi/aml-build.h
>>>>> @@ -385,6 +385,10 @@ int
>>>>> build_append_named_dword(GArray *array, const char *name_format, ...)
>>>>> GCC_FMT_ATTR(2, 3);
>>>>>
>>>>> +int
>>>>> +build_append_named_qword(GArray *array, const char *name_format, ...)
>>>>> +GCC_FMT_ATTR(2, 3);
>>>>> +
>>>>> void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
>>>>> uint64_t len, int node, MemoryAffinityFlags flags);
smime.p7s
Description: S/MIME cryptographic signature
- Re: [Qemu-devel] [PATCH v5 02/10] linker-loader: Add new 'write pointer' command, (continued)
[Qemu-devel] [PATCH v5 01/10] ACPI: Add a function for building named qword entries, ben, 2017/02/05
[Qemu-devel] [PATCH v5 04/10] ACPI: Add vmgenid storage entries to the build tables, ben, 2017/02/05
[Qemu-devel] [PATCH v5 03/10] docs: VM Generation ID device description, ben, 2017/02/05
[Qemu-devel] [PATCH v5 05/10] ACPI: Add Virtual Machine Generation ID support, ben, 2017/02/05