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(+)

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;
+}
-- 
2.7.4

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
>