[Qemu-devel] [PATCH v3 03/13] tests: acpi: make pointer to RSDP 64bit

Igor Mammedov posted 13 patches 6 years, 6 months ago
Only 12 patches received!
There is a newer version of this series
[Qemu-devel] [PATCH v3 03/13] tests: acpi: make pointer to RSDP 64bit
Posted by Igor Mammedov 6 years, 6 months ago
In case of UEFI, RSDP doesn't have to be located in lowmem,
it could be placed at any address. Make sure that test won't
break if it is placed above the first 4Gb of address space.

PS:
While at it cleanup some local variables as we don't really
need them.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2:
  - s/In case of UEFI/In case of UEFI,/ (Laszlo Ersek <lersek@redhat.com>)
---
 tests/bios-tables-test.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 2ee044c..c29dcf4 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -26,7 +26,7 @@
 typedef struct {
     const char *machine;
     const char *variant;
-    uint32_t rsdp_addr;
+    uint64_t rsdp_addr;
     uint8_t rsdp_table[36 /* ACPI 2.0+ RSDP size */];
     GArray *tables;
     uint32_t smbios_ep_addr;
@@ -86,13 +86,11 @@ static void test_acpi_rsdp_address(test_data *data)
 
 static void test_acpi_rsdp_table(test_data *data)
 {
-    uint8_t *rsdp_table = data->rsdp_table, revision;
-    uint32_t addr = data->rsdp_addr;
+    uint8_t *rsdp_table = data->rsdp_table;
 
-    acpi_fetch_rsdp_table(data->qts, addr, rsdp_table);
-    revision = rsdp_table[15 /* Revision offset */];
+    acpi_fetch_rsdp_table(data->qts, data->rsdp_addr, rsdp_table);
 
-    switch (revision) {
+    switch (rsdp_table[15 /* Revision offset */]) {
     case 0: /* ACPI 1.0 RSDP */
         /* With rev 1, checksum is only for the first 20 bytes */
         g_assert(!acpi_calc_checksum(rsdp_table,  20));
-- 
2.7.4


Re: [Qemu-devel] [PATCH v3 03/13] tests: acpi: make pointer to RSDP 64bit
Posted by Wei Yang 6 years, 6 months ago
On Thu, Apr 25, 2019 at 07:34:39AM +0200, Igor Mammedov wrote:
>In case of UEFI, RSDP doesn't have to be located in lowmem,
>it could be placed at any address. Make sure that test won't
>break if it is placed above the first 4Gb of address space.
>
>PS:
>While at it cleanup some local variables as we don't really
>need them.
>
>Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>---
>v2:
>  - s/In case of UEFI/In case of UEFI,/ (Laszlo Ersek <lersek@redhat.com>)
>---
> tests/bios-tables-test.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
>diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
>index 2ee044c..c29dcf4 100644
>--- a/tests/bios-tables-test.c
>+++ b/tests/bios-tables-test.c
>@@ -26,7 +26,7 @@
> typedef struct {
>     const char *machine;
>     const char *variant;
>-    uint32_t rsdp_addr;
>+    uint64_t rsdp_addr;
>     uint8_t rsdp_table[36 /* ACPI 2.0+ RSDP size */];
>     GArray *tables;
>     uint32_t smbios_ep_addr;
>@@ -86,13 +86,11 @@ static void test_acpi_rsdp_address(test_data *data)
> 
> static void test_acpi_rsdp_table(test_data *data)
> {
>-    uint8_t *rsdp_table = data->rsdp_table, revision;
>-    uint32_t addr = data->rsdp_addr;
>+    uint8_t *rsdp_table = data->rsdp_table;
> 
>-    acpi_fetch_rsdp_table(data->qts, addr, rsdp_table);
>-    revision = rsdp_table[15 /* Revision offset */];
>+    acpi_fetch_rsdp_table(data->qts, data->rsdp_addr, rsdp_table);

Ok, I think this is the reason you change the second parameter from uint32_t
to uint64_t in 1st patch.

Maybe we need to move that on to this one?

> 
>-    switch (revision) {
>+    switch (rsdp_table[15 /* Revision offset */]) {
>     case 0: /* ACPI 1.0 RSDP */
>         /* With rev 1, checksum is only for the first 20 bytes */
>         g_assert(!acpi_calc_checksum(rsdp_table,  20));
>-- 
>2.7.4

-- 
Wei Yang
Help you, Help me

Re: [Qemu-devel] [PATCH v3 03/13] tests: acpi: make pointer to RSDP 64bit
Posted by Igor Mammedov 6 years, 6 months ago
On Thu, 25 Apr 2019 15:31:42 +0800
Wei Yang <richardw.yang@linux.intel.com> wrote:

> On Thu, Apr 25, 2019 at 07:34:39AM +0200, Igor Mammedov wrote:
> >In case of UEFI, RSDP doesn't have to be located in lowmem,
> >it could be placed at any address. Make sure that test won't
> >break if it is placed above the first 4Gb of address space.
> >
> >PS:
> >While at it cleanup some local variables as we don't really
> >need them.
> >
> >Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> >Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >---
> >v2:
> >  - s/In case of UEFI/In case of UEFI,/ (Laszlo Ersek <lersek@redhat.com>)
> >---
> > tests/bios-tables-test.c | 10 ++++------
> > 1 file changed, 4 insertions(+), 6 deletions(-)
> >
> >diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
> >index 2ee044c..c29dcf4 100644
> >--- a/tests/bios-tables-test.c
> >+++ b/tests/bios-tables-test.c
> >@@ -26,7 +26,7 @@
> > typedef struct {
> >     const char *machine;
> >     const char *variant;
> >-    uint32_t rsdp_addr;
> >+    uint64_t rsdp_addr;
> >     uint8_t rsdp_table[36 /* ACPI 2.0+ RSDP size */];
> >     GArray *tables;
> >     uint32_t smbios_ep_addr;
> >@@ -86,13 +86,11 @@ static void test_acpi_rsdp_address(test_data *data)
> > 
> > static void test_acpi_rsdp_table(test_data *data)
> > {
> >-    uint8_t *rsdp_table = data->rsdp_table, revision;
> >-    uint32_t addr = data->rsdp_addr;
> >+    uint8_t *rsdp_table = data->rsdp_table;
> > 
> >-    acpi_fetch_rsdp_table(data->qts, addr, rsdp_table);
> >-    revision = rsdp_table[15 /* Revision offset */];
> >+    acpi_fetch_rsdp_table(data->qts, data->rsdp_addr, rsdp_table);  
> 
> Ok, I think this is the reason you change the second parameter from uint32_t
> to uint64_t in 1st patch.
> 
> Maybe we need to move that on to this one?
Sure.
There is one additional path to make 64-bit table pointers support a bit more complete.
I'll respin series once travis build passes.

 
> > 
> >-    switch (revision) {
> >+    switch (rsdp_table[15 /* Revision offset */]) {
> >     case 0: /* ACPI 1.0 RSDP */
> >         /* With rev 1, checksum is only for the first 20 bytes */
> >         g_assert(!acpi_calc_checksum(rsdp_table,  20));
> >-- 
> >2.7.4  
>