diff mbox series

[v3,14/15] tests/qtest/bios-tables-test.c: Enable basic testing for RISC-V

Message ID 20240621115906.1049832-15-sunilvl@ventanamicro.com (mailing list archive)
State New, archived
Headers show
Series Add support for RISC-V ACPI tests | expand

Commit Message

Sunil V L June 21, 2024, 11:59 a.m. UTC 
Add basic ACPI table test case for RISC-V.

Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 tests/qtest/bios-tables-test.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

Comments

Igor Mammedov June 25, 2024, 11:19 a.m. UTC | #1 
On Fri, 21 Jun 2024 17:29:05 +0530
Sunil V L <sunilvl@ventanamicro.com> wrote:

> Add basic ACPI table test case for RISC-V.
> 
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  tests/qtest/bios-tables-test.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index f4c4704bab..0f9c654e96 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -1977,6 +1977,28 @@ static void test_acpi_microvm_acpi_erst(void)
>  }
>  #endif /* CONFIG_POSIX */
>  
> +static void test_acpi_riscv64_virt_tcg(void)
> +{
> +    test_data data = {
> +        .machine = "virt",
> +        .arch = "riscv64",
> +        .tcg_only = true,
> +        .uefi_fl1 = "pc-bios/edk2-riscv-code.fd",
> +        .uefi_fl2 = "pc-bios/edk2-riscv-vars.fd",
> +        .cd = "tests/data/uefi-boot-images/bios-tables-test.riscv64.iso.qcow2",
> +        .ram_start = 0x80000000ULL,
> +        .scan_len = 128ULL * 1024 * 1024,
> +    };
> +
> +    /*
> +     * RHCT will have ISA string encoded. To reduce the effort
> +     * of updating expected AML file for any new default ISA extension,
> +     * use the profile rva22s64.
> +     */
> +    test_acpi_one("-cpu rva22s64 ", &data);
> +    free_test_data(&data);
> +}
> +
>  static void test_acpi_aarch64_virt_tcg(void)
>  {
>      test_data data = {
> @@ -2455,6 +2477,10 @@ int main(int argc, char *argv[])
>                  qtest_add_func("acpi/virt/viot", test_acpi_aarch64_virt_viot);
>              }
>          }
> +    } else if (strcmp(arch, "riscv64") == 0) {
> +        if (has_tcg && qtest_has_device("virtio-blk-pci")) {
> +            qtest_add_func("acpi/virt", test_acpi_riscv64_virt_tcg);
> +        }
>      }
>      ret = g_test_run();
>      boot_sector_cleanup(disk);
Igor Mammedov June 25, 2024, 12:05 p.m. UTC | #2 
On Tue, 25 Jun 2024 13:19:59 +0200
Igor Mammedov <imammedo@redhat.com> wrote:

> On Fri, 21 Jun 2024 17:29:05 +0530
> Sunil V L <sunilvl@ventanamicro.com> wrote:
> 
> > Add basic ACPI table test case for RISC-V.
> > 
> > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>  
> 
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>

I take ack back for now, since patch most likely to cause failures on weaker test host (CI infra)

test case never finishes and timeouts on my x86 host while consuming 100%,

======
QTEST_QEMU_BINARY=./qemu-system-riscv64 /tmp/qemu_build/tests/qtest/bios-tables-test
# random seed: R02Sd870403ff62b08e48122105b2700f660
# starting QEMU: exec ./qemu-system-riscv64 -qtest unix:/tmp/qtest-2873960.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-2873960.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -machine none -accel qtest
1..1
# Start of riscv64 tests
# Start of acpi tests
# starting QEMU: exec ./qemu-system-riscv64 -qtest unix:/tmp/qtest-2873960.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-2873960.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -machine virt  -accel tcg -nodefaults -nographic -drive if=pflash,format=raw,file=pc-bios/edk2-riscv-code.fd,readonly=on -drive if=pflash,format=raw,file=pc-bios/edk2-riscv-vars.fd,snapshot=on -cdrom tests/data/uefi-boot-images/bios-tables-test.riscv64.iso.qcow2 -cpu rva22s64  -accel qtest



**
ERROR:../../builds/imammedo/qemu/tests/qtest/acpi-utils.c:158:acpi_find_rsdp_address_uefi: code should not be reached
Bail out! ERROR:../../builds/imammedo/qemu/tests/qtest/acpi-utils.c:158:acpi_find_rsdp_address_uefi: code should not be reached
========


 
> 
> > ---
> >  tests/qtest/bios-tables-test.c | 26 ++++++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> > 
> > diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> > index f4c4704bab..0f9c654e96 100644
> > --- a/tests/qtest/bios-tables-test.c
> > +++ b/tests/qtest/bios-tables-test.c
> > @@ -1977,6 +1977,28 @@ static void test_acpi_microvm_acpi_erst(void)
> >  }
> >  #endif /* CONFIG_POSIX */
> >  
> > +static void test_acpi_riscv64_virt_tcg(void)
> > +{
> > +    test_data data = {
> > +        .machine = "virt",
> > +        .arch = "riscv64",
> > +        .tcg_only = true,
> > +        .uefi_fl1 = "pc-bios/edk2-riscv-code.fd",
> > +        .uefi_fl2 = "pc-bios/edk2-riscv-vars.fd",
> > +        .cd = "tests/data/uefi-boot-images/bios-tables-test.riscv64.iso.qcow2",
> > +        .ram_start = 0x80000000ULL,
> > +        .scan_len = 128ULL * 1024 * 1024,
> > +    };
> > +
> > +    /*
> > +     * RHCT will have ISA string encoded. To reduce the effort
> > +     * of updating expected AML file for any new default ISA extension,
> > +     * use the profile rva22s64.
> > +     */
> > +    test_acpi_one("-cpu rva22s64 ", &data);
> > +    free_test_data(&data);
> > +}
> > +
> >  static void test_acpi_aarch64_virt_tcg(void)
> >  {
> >      test_data data = {
> > @@ -2455,6 +2477,10 @@ int main(int argc, char *argv[])
> >                  qtest_add_func("acpi/virt/viot", test_acpi_aarch64_virt_viot);
> >              }
> >          }
> > +    } else if (strcmp(arch, "riscv64") == 0) {
> > +        if (has_tcg && qtest_has_device("virtio-blk-pci")) {
> > +            qtest_add_func("acpi/virt", test_acpi_riscv64_virt_tcg);
> > +        }
> >      }
> >      ret = g_test_run();
> >      boot_sector_cleanup(disk);  
>
Sunil V L June 25, 2024, 12:29 p.m. UTC | #3 
On Tue, Jun 25, 2024 at 02:05:58PM +0200, Igor Mammedov wrote:
> On Tue, 25 Jun 2024 13:19:59 +0200
> Igor Mammedov <imammedo@redhat.com> wrote:
> 
> > On Fri, 21 Jun 2024 17:29:05 +0530
> > Sunil V L <sunilvl@ventanamicro.com> wrote:
> > 
> > > Add basic ACPI table test case for RISC-V.
> > > 
> > > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>  
> > 
> > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> 
> I take ack back for now, since patch most likely to cause failures on weaker test host (CI infra)
> 
> test case never finishes and timeouts on my x86 host while consuming 100%,
> 
Hi Igor,

Many thanks for your kind review!. I think you are missing the patch [1]
(which I mentioned in cover letter as well). This patch became a
dependency since your suggestion to use -cdrom option needed this fix.

gitlab CI tests also passed for me with that patch included.

[1] - https://mail.gnu.org/archive/html/qemu-devel/2024-06/msg03683.html
 
Thanks,
Sunil
Igor Mammedov June 25, 2024, 2:06 p.m. UTC | #4 
On Tue, 25 Jun 2024 17:59:33 +0530
Sunil V L <sunilvl@ventanamicro.com> wrote:

> On Tue, Jun 25, 2024 at 02:05:58PM +0200, Igor Mammedov wrote:
> > On Tue, 25 Jun 2024 13:19:59 +0200
> > Igor Mammedov <imammedo@redhat.com> wrote:
> >   
> > > On Fri, 21 Jun 2024 17:29:05 +0530
> > > Sunil V L <sunilvl@ventanamicro.com> wrote:
> > >   
> > > > Add basic ACPI table test case for RISC-V.
> > > > 
> > > > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> > > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>    
> > > 
> > > Reviewed-by: Igor Mammedov <imammedo@redhat.com>  
> > 
> > I take ack back for now, since patch most likely to cause failures on weaker test host (CI infra)
> > 
> > test case never finishes and timeouts on my x86 host while consuming 100%,
> >   
> Hi Igor,
> 
> Many thanks for your kind review!. I think you are missing the patch [1]
> (which I mentioned in cover letter as well). This patch became a
> dependency since your suggestion to use -cdrom option needed this fix.
> 
> gitlab CI tests also passed for me with that patch included.
> 
> [1] - https://mail.gnu.org/archive/html/qemu-devel/2024-06/msg03683.html

ok, keep my RB but respin series with that patch included to make it complete.
(there is no harm if it gets merged 1st through another tree. but it makes 
life of reviewers/maintainers easier)

>  
> Thanks,
> Sunil
>
Sunil V L June 25, 2024, 3:18 p.m. UTC | #5 
On Tue, Jun 25, 2024 at 04:06:58PM +0200, Igor Mammedov wrote:
> On Tue, 25 Jun 2024 17:59:33 +0530
> Sunil V L <sunilvl@ventanamicro.com> wrote:
> 
> > On Tue, Jun 25, 2024 at 02:05:58PM +0200, Igor Mammedov wrote:
> > > On Tue, 25 Jun 2024 13:19:59 +0200
> > > Igor Mammedov <imammedo@redhat.com> wrote:
> > >   
> > > > On Fri, 21 Jun 2024 17:29:05 +0530
> > > > Sunil V L <sunilvl@ventanamicro.com> wrote:
> > > >   
> > > > > Add basic ACPI table test case for RISC-V.
> > > > > 
> > > > > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> > > > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>    
> > > > 
> > > > Reviewed-by: Igor Mammedov <imammedo@redhat.com>  
> > > 
> > > I take ack back for now, since patch most likely to cause failures on weaker test host (CI infra)
> > > 
> > > test case never finishes and timeouts on my x86 host while consuming 100%,
> > >   
> > Hi Igor,
> > 
> > Many thanks for your kind review!. I think you are missing the patch [1]
> > (which I mentioned in cover letter as well). This patch became a
> > dependency since your suggestion to use -cdrom option needed this fix.
> > 
> > gitlab CI tests also passed for me with that patch included.
> > 
> > [1] - https://mail.gnu.org/archive/html/qemu-devel/2024-06/msg03683.html
> 
> ok, keep my RB but respin series with that patch included to make it complete.
> (there is no harm if it gets merged 1st through another tree. but it makes 
> life of reviewers/maintainers easier)
> 
Fair enough. Sorry about that. I have sent v4 as per your recommendation.
It looks like the series has got sufficient reviews. May be it would be
better if it goes via Alistair's tree since the dependent patch is
already merged in his next branch but I will leave it to the
maintainers.

Thanks!
Sunil
diff mbox series

Patch

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index f4c4704bab..0f9c654e96 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1977,6 +1977,28 @@  static void test_acpi_microvm_acpi_erst(void)
 }
 #endif /* CONFIG_POSIX */
 
+static void test_acpi_riscv64_virt_tcg(void)
+{
+    test_data data = {
+        .machine = "virt",
+        .arch = "riscv64",
+        .tcg_only = true,
+        .uefi_fl1 = "pc-bios/edk2-riscv-code.fd",
+        .uefi_fl2 = "pc-bios/edk2-riscv-vars.fd",
+        .cd = "tests/data/uefi-boot-images/bios-tables-test.riscv64.iso.qcow2",
+        .ram_start = 0x80000000ULL,
+        .scan_len = 128ULL * 1024 * 1024,
+    };
+
+    /*
+     * RHCT will have ISA string encoded. To reduce the effort
+     * of updating expected AML file for any new default ISA extension,
+     * use the profile rva22s64.
+     */
+    test_acpi_one("-cpu rva22s64 ", &data);
+    free_test_data(&data);
+}
+
 static void test_acpi_aarch64_virt_tcg(void)
 {
     test_data data = {
@@ -2455,6 +2477,10 @@  int main(int argc, char *argv[])
                 qtest_add_func("acpi/virt/viot", test_acpi_aarch64_virt_viot);
             }
         }
+    } else if (strcmp(arch, "riscv64") == 0) {
+        if (has_tcg && qtest_has_device("virtio-blk-pci")) {
+            qtest_add_func("acpi/virt", test_acpi_riscv64_virt_tcg);
+        }
     }
     ret = g_test_run();
     boot_sector_cleanup(disk);