Re: [Qemu-devel] [PATCH v5 01/10] ACPI: Add a function for building named qword entries

[Laszlo Ersek]

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.

Thanks
Laszlo


>> +{
>> +    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);
>>  
> 
>