| Message ID | 1547566866-129386-2-git-send-email-imammedo@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | tests: acpi: add UEFI (ARM) testing support | expand |
Hi Igor, On 01/15/19 16:40, Igor Mammedov wrote: > introduce UEFI specific counterpart to acpi_find_rsdp_address() > that will help to find RSDP address when [OA]VMF is used as > firmware. It requires a [OA]VMF built with PcdAcpiTestSupport=TRUE, > to locate RSDP address within 1Mb aligned ACPI test structure, tagged > with GUID AB87A6B1-2034-BDA0-71BD-375007757785 > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > tests/acpi-utils.h | 1 + > tests/acpi-utils.c | 43 +++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 44 insertions(+) I'm not promising to review all of this patch set (Phil, feel free to chime in); I'll just make some quick comments below: > diff --git a/tests/acpi-utils.h b/tests/acpi-utils.h > index ef388bb..3b11f47 100644 > --- a/tests/acpi-utils.h > +++ b/tests/acpi-utils.h > @@ -46,6 +46,7 @@ typedef struct { > > uint8_t acpi_calc_checksum(const uint8_t *data, int len); > uint32_t acpi_find_rsdp_address(QTestState *qts); > +uint64_t uefi_find_rsdp_addr(QTestState *qts, uint64_t start, uint64_t size); I think it would make sense to keep the "acpi_find_rsdp_address" prefix for the new function name; maybe append "_uefi"? Because now "acpi" is replaced with "uefi , plus "address" is truncated to "addr"; those don't seem overly logical. Anyway, up to you. > uint64_t acpi_get_xsdt_address(uint8_t *rsdp_table); > void acpi_parse_rsdp_table(QTestState *qts, uint32_t addr, uint8_t *rsdp_table); > void acpi_fetch_table(QTestState *qts, uint8_t **aml, uint32_t *aml_len, > diff --git a/tests/acpi-utils.c b/tests/acpi-utils.c > index cc33b46..b9ff9df 100644 > --- a/tests/acpi-utils.c > +++ b/tests/acpi-utils.c > @@ -111,3 +111,46 @@ void acpi_fetch_table(QTestState *qts, uint8_t **aml, uint32_t *aml_len, > g_assert(!acpi_calc_checksum(*aml, *aml_len)); > } > } > + > +#define GUID_SIZE 16 > +static uint8_t AcpiTestSupportGuid[GUID_SIZE] = > + { 0xb1, 0xa6, 0x87, 0xab, > + 0x34, 0x20, > + 0xa0, 0xbd, > + 0x71, 0xbd, 0x37, 0x50, 0x07, 0x75, 0x77, 0x85 }; I think this is generally good. QEMU has some utilities/helpers for working with UUIDs; however, for the test infrastructure, I think this should be good enough. Suggestion: make the GUID "const" as well. > + > +typedef struct { > + uint8_t signature_guid[16]; s/16/GUID_SIZE/? > + uint64_t rsdp10; > + uint64_t rsdp20; > +} __attribute__((packed)) UefiTestSupport; > + > +/* Wait at most 600 seconds (test is slow with TCI and --enable-debug) */ Do you specifically mean "Tiny Code Interpreter" here? > +#define TEST_DELAY (1 * G_USEC_PER_SEC / 10) > +#define TEST_CYCLES MAX((600 * G_USEC_PER_SEC / TEST_DELAY), 1) > +#define MB 0x100000ULL > +uint64_t uefi_find_rsdp_addr(QTestState *qts, uint64_t start, uint64_t size) > +{ > + int i, j; > + uint8_t data[GUID_SIZE]; > + > + for (i = 0; i < TEST_CYCLES; ++i) { > + for (j = 0; j < size / MB; j++) { > + /* look for GUID at every 1Mb block */ > + uint64_t addr = start + j * MB; > + > + qtest_memread(qts, addr, data, sizeof(data)); > + if (!memcmp(AcpiTestSupportGuid, data, sizeof(data))) { > + UefiTestSupport ret; > + > + qtest_memread(qts, addr, &ret, sizeof(ret)); > + ret.rsdp10 = le64_to_cpu(ret.rsdp10); > + ret.rsdp20 = le64_to_cpu(ret.rsdp20); > + return ret.rsdp20 ? ret.rsdp20 : ret.rsdp10; > + } > + } > + g_usleep(TEST_DELAY); > + } > + g_assert_not_reached(); > + return 0; > +} > Apart from my hair-splitting, it looks good. If you update 16-->GUID_SIZE, then you can add Reviewed-by: Laszlo Ersek <lersek@redhat.com> Thanks, Laszlo
On Tue, 15 Jan 2019 21:06:16 +0100 Laszlo Ersek <lersek@redhat.com> wrote: > Hi Igor, > > On 01/15/19 16:40, Igor Mammedov wrote: > > introduce UEFI specific counterpart to acpi_find_rsdp_address() > > that will help to find RSDP address when [OA]VMF is used as > > firmware. It requires a [OA]VMF built with PcdAcpiTestSupport=TRUE, > > to locate RSDP address within 1Mb aligned ACPI test structure, tagged > > with GUID AB87A6B1-2034-BDA0-71BD-375007757785 > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > --- > > tests/acpi-utils.h | 1 + > > tests/acpi-utils.c | 43 +++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 44 insertions(+) > > I'm not promising to review all of this patch set (Phil, feel free to > chime in); I'll just make some quick comments below: > > > diff --git a/tests/acpi-utils.h b/tests/acpi-utils.h > > index ef388bb..3b11f47 100644 > > --- a/tests/acpi-utils.h > > +++ b/tests/acpi-utils.h > > @@ -46,6 +46,7 @@ typedef struct { > > > > uint8_t acpi_calc_checksum(const uint8_t *data, int len); > > uint32_t acpi_find_rsdp_address(QTestState *qts); > > +uint64_t uefi_find_rsdp_addr(QTestState *qts, uint64_t start, uint64_t size); > > I think it would make sense to keep the "acpi_find_rsdp_address" prefix > for the new function name; maybe append "_uefi"? Because now "acpi" is > replaced with "uefi , plus "address" is truncated to "addr"; those don't > seem overly logical. > > Anyway, up to you. > > > uint64_t acpi_get_xsdt_address(uint8_t *rsdp_table); > > void acpi_parse_rsdp_table(QTestState *qts, uint32_t addr, uint8_t *rsdp_table); > > void acpi_fetch_table(QTestState *qts, uint8_t **aml, uint32_t *aml_len, > > diff --git a/tests/acpi-utils.c b/tests/acpi-utils.c > > index cc33b46..b9ff9df 100644 > > --- a/tests/acpi-utils.c > > +++ b/tests/acpi-utils.c > > @@ -111,3 +111,46 @@ void acpi_fetch_table(QTestState *qts, uint8_t **aml, uint32_t *aml_len, > > g_assert(!acpi_calc_checksum(*aml, *aml_len)); > > } > > } > > + > > +#define GUID_SIZE 16 > > +static uint8_t AcpiTestSupportGuid[GUID_SIZE] = > > + { 0xb1, 0xa6, 0x87, 0xab, > > + 0x34, 0x20, > > + 0xa0, 0xbd, > > + 0x71, 0xbd, 0x37, 0x50, 0x07, 0x75, 0x77, 0x85 }; > > I think this is generally good. QEMU has some utilities/helpers for > working with UUIDs; however, for the test infrastructure, I think this > should be good enough. > > Suggestion: make the GUID "const" as well. > > > + > > +typedef struct { > > + uint8_t signature_guid[16]; > > s/16/GUID_SIZE/? > > > + uint64_t rsdp10; > > + uint64_t rsdp20; > > +} __attribute__((packed)) UefiTestSupport; > > + > > +/* Wait at most 600 seconds (test is slow with TCI and --enable-debug) */ > > Do you specifically mean "Tiny Code Interpreter" here? it's typo, I've meant TCG. I'll fix it. > > +#define TEST_DELAY (1 * G_USEC_PER_SEC / 10) > > +#define TEST_CYCLES MAX((600 * G_USEC_PER_SEC / TEST_DELAY), 1) > > +#define MB 0x100000ULL > > +uint64_t uefi_find_rsdp_addr(QTestState *qts, uint64_t start, uint64_t size) > > +{ > > + int i, j; > > + uint8_t data[GUID_SIZE]; > > + > > + for (i = 0; i < TEST_CYCLES; ++i) { > > + for (j = 0; j < size / MB; j++) { > > + /* look for GUID at every 1Mb block */ > > + uint64_t addr = start + j * MB; > > + > > + qtest_memread(qts, addr, data, sizeof(data)); > > + if (!memcmp(AcpiTestSupportGuid, data, sizeof(data))) { > > + UefiTestSupport ret; > > + > > + qtest_memread(qts, addr, &ret, sizeof(ret)); > > + ret.rsdp10 = le64_to_cpu(ret.rsdp10); > > + ret.rsdp20 = le64_to_cpu(ret.rsdp20); > > + return ret.rsdp20 ? ret.rsdp20 : ret.rsdp10; > > + } > > + } > > + g_usleep(TEST_DELAY); > > + } > > + g_assert_not_reached(); > > + return 0; > > +} > > > > Apart from my hair-splitting, it looks good. If you update > 16-->GUID_SIZE, then you can add Thanks, I'll apply all of your suggestions for v2 respin > > Reviewed-by: Laszlo Ersek <lersek@redhat.com> > > Thanks, > Laszlo >
diff --git a/tests/acpi-utils.h b/tests/acpi-utils.h index ef388bb..3b11f47 100644 --- a/tests/acpi-utils.h +++ b/tests/acpi-utils.h @@ -46,6 +46,7 @@ typedef struct { uint8_t acpi_calc_checksum(const uint8_t *data, int len); uint32_t acpi_find_rsdp_address(QTestState *qts); +uint64_t uefi_find_rsdp_addr(QTestState *qts, uint64_t start, uint64_t size); uint64_t acpi_get_xsdt_address(uint8_t *rsdp_table); void acpi_parse_rsdp_table(QTestState *qts, uint32_t addr, uint8_t *rsdp_table); void acpi_fetch_table(QTestState *qts, uint8_t **aml, uint32_t *aml_len, diff --git a/tests/acpi-utils.c b/tests/acpi-utils.c index cc33b46..b9ff9df 100644 --- a/tests/acpi-utils.c +++ b/tests/acpi-utils.c @@ -111,3 +111,46 @@ void acpi_fetch_table(QTestState *qts, uint8_t **aml, uint32_t *aml_len, g_assert(!acpi_calc_checksum(*aml, *aml_len)); } } + +#define GUID_SIZE 16 +static uint8_t AcpiTestSupportGuid[GUID_SIZE] = + { 0xb1, 0xa6, 0x87, 0xab, + 0x34, 0x20, + 0xa0, 0xbd, + 0x71, 0xbd, 0x37, 0x50, 0x07, 0x75, 0x77, 0x85 }; + +typedef struct { + uint8_t signature_guid[16]; + uint64_t rsdp10; + uint64_t rsdp20; +} __attribute__((packed)) UefiTestSupport; + +/* Wait at most 600 seconds (test is slow with TCI and --enable-debug) */ +#define TEST_DELAY (1 * G_USEC_PER_SEC / 10) +#define TEST_CYCLES MAX((600 * G_USEC_PER_SEC / TEST_DELAY), 1) +#define MB 0x100000ULL +uint64_t uefi_find_rsdp_addr(QTestState *qts, uint64_t start, uint64_t size) +{ + int i, j; + uint8_t data[GUID_SIZE]; + + for (i = 0; i < TEST_CYCLES; ++i) { + for (j = 0; j < size / MB; j++) { + /* look for GUID at every 1Mb block */ + uint64_t addr = start + j * MB; + + qtest_memread(qts, addr, data, sizeof(data)); + if (!memcmp(AcpiTestSupportGuid, data, sizeof(data))) { + UefiTestSupport ret; + + qtest_memread(qts, addr, &ret, sizeof(ret)); + ret.rsdp10 = le64_to_cpu(ret.rsdp10); + ret.rsdp20 = le64_to_cpu(ret.rsdp20); + return ret.rsdp20 ? ret.rsdp20 : ret.rsdp10; + } + } + g_usleep(TEST_DELAY); + } + g_assert_not_reached(); + return 0; +}
introduce UEFI specific counterpart to acpi_find_rsdp_address() that will help to find RSDP address when [OA]VMF is used as firmware. It requires a [OA]VMF built with PcdAcpiTestSupport=TRUE, to locate RSDP address within 1Mb aligned ACPI test structure, tagged with GUID AB87A6B1-2034-BDA0-71BD-375007757785 Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- tests/acpi-utils.h | 1 + tests/acpi-utils.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+)