Re: [Qemu-devel] [PATCH v4 05/15] tests: acpi: fetch X_DSDT if pointer to DSDT is 0

[Wei Yang]

On Tue, May 07, 2019 at 12:04:08PM +0200, Igor Mammedov wrote:
>On Sun, 5 May 2019 09:27:45 +0800
>Wei Yang <address@hidden> wrote:
>
>> On Thu, May 02, 2019 at 04:51:53PM +0200, Igor Mammedov wrote:
>> >that way it would be possible to test a DSDT pointed by
>> >64bit X_DSDT field in FADT.
>> >
>> >PS:
>> >it will allow to enable testing arm/virt board, which sets
>> >only newer X_DSDT field.
>> >
>> >Signed-off-by: Igor Mammedov <address@hidden>
>> >---
>> >v4:
>> > * dropping Reviewed-bys due to acpi_fetch_table() change
>> >   introduced by earlier patch:
>> >   "tests: acpi: make acpi_fetch_table() take size of fetched table pointer"
>> >v2:
>> >  add 'val = le32_to_cpu(val)' even if it doesn't necessary
>> >  it works as reminder that value copied from table is in
>> >  little-endian format (Philippe Mathieu-Daudé <address@hidden>)
>> >---
>> > tests/bios-tables-test.c | 11 ++++++++++-
>> > 1 file changed, 10 insertions(+), 1 deletion(-)
>> >
>> >diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
>> >index a164d27..d165a1b 100644
>> >--- a/tests/bios-tables-test.c
>> >+++ b/tests/bios-tables-test.c
>> >@@ -140,6 +140,9 @@ static void test_acpi_fadt_table(test_data *data)
>> >     AcpiSdtTable table = g_array_index(data->tables, typeof(table), 0);
>> >     uint8_t *fadt_aml = table.aml;
>> >     uint32_t fadt_len = table.aml_len;
>> >+    uint32_t val;
>> >+    int dsdt_offset = 40 /* DSDT */;
>> >+    int dsdt_entry_size = 4;
>> > 
>> >     g_assert(compare_signature(&table, "FACP"));
>> > 
>> >@@ -148,8 +151,14 @@ static void test_acpi_fadt_table(test_data *data)
>> >                      fadt_aml + 36 /* FIRMWARE_CTRL */, 4, "FACS", false);
>> >     g_array_append_val(data->tables, table);
>> > 
>> >+    memcpy(&val, fadt_aml + dsdt_offset, 4);
>> >+    val = le32_to_cpu(val);
>> >+    if (!val) {
>> >+        dsdt_offset = 140 /* X_DSDT */;
>> 
>> In case we can point out where we get it, e.g. ACPI 5, Table 5-34 FADT 
>> Format.
>> 
>> This may be more helpful for reviewing and maintaining.
>
>for fields we typically use only verbatim field name, so it would be easy
>to find by searching for it in spec. In this case it is obvious about which
>table it applies to, so reference to spec for a field probably excessive.
>
>Complete reference necessary for tables and API functions that implement
>ACPI primitive.
>

That's fine.

Reviewed-by: Wei Yang <address@hidden>

-- 
Wei Yang
Help you, Help me