mbox series

[v6,00/13] RISC-V: ACPI: Enable AIA, PLIC and update RHCT

Message ID 20231102170223.2619260-1-sunilvl@ventanamicro.com (mailing list archive)
Headers show
Series RISC-V: ACPI: Enable AIA, PLIC and update RHCT | expand

Message

Sunil V L Nov. 2, 2023, 5:02 p.m. UTC 
This series primarily enables external interrupt controllers (AIA and PLIC)
in ACPI tables for RISC-V virt platform. It also updates RHCT with CMO and
MMU related information.

Below ECRs for these changes are approved by ASWG and will be
available in next ACPI spec release.

1) MADT (AIA) - https://drive.google.com/file/d/1oMGPyOD58JaPgMl1pKasT-VKsIKia7zR/view?usp=sharing
2) RHCT - https://drive.google.com/file/d/1sKbOa8m1UZw1JkquZYe3F1zQBN1xXsaf/view?usp=sharing

First two patches in this series are to migrate a couple of functions from
ARM architecture to common code so that RISC-V doesn't need to duplicate
the same.

The patch set is based on Alistair's riscv-to-apply.next branch.

These changes are also available in  riscv_acpi_b2_v6 branch at:
https://github.com/vlsunil/qemu/

Changes since v5:
	1) Fixed the issue in PATCH 2 reported by Daniel found when built with
	   clang + --enable-debug.

Changes since v4:
	1) Updated copyright for new files as per SPDX format suggested by Drew.
	2) Updated RINTC patch to avoid code duplication as suggested by Drew.
	3) Moved mmu offset below cmo in MMU patch as suggested by Drew.
	4) Updated tags.

Changes since v3:
	1) Addressed comments from Daniel and Drew.
	2) Added a new patch in microvm to use common function for virtio in DSDT.
	3) Rebased to latest riscv-to-apply.next branch and added tags.

Changes since v2:
        1) Rebased to latest riscv-to-apply.next branch which needed
           changing ext_icboz to ext_zicboz in CMO patch.
        2) Fixed node type in MMU node.
        3) Added latest tags.

Changes since v1:
        1) As per Igor's suggestion, migrated fw_cfg and virtio creation
           functions to device specific file instead of generic aml-build.c.
           Since ACPI is optional, new files are created and enabled for
           build only when CONFIG_ACPI is enabled.
        2) As per Igor's suggestion, properties are added to the GPEX PCI
           host to indicate MMIO ranges. The platform fw can initialize
           these to appropriate values and the DSDT generator can fetch
           the information from the host bus itself. This makes the code
           generic instead of machine specific.
        3) Added PLIC patch from Haibo.
        4) Rebased to latest riscv-to-apply.next and added RB tags as
           appropriate.
Sunil V L (13):
  hw/arm/virt-acpi-build.c: Migrate fw_cfg creation to common location
  hw/arm/virt-acpi-build.c: Migrate virtio creation to common location
  hw/i386/acpi-microvm.c: Use common function to add virtio in DSDT
  hw/riscv: virt: Make few IMSIC macros and functions public
  hw/riscv/virt-acpi-build.c: Add AIA support in RINTC
  hw/riscv/virt-acpi-build.c: Add IMSIC in the MADT
  hw/riscv/virt-acpi-build.c: Add APLIC in the MADT
  hw/riscv/virt-acpi-build.c: Add CMO information in RHCT
  hw/riscv/virt-acpi-build.c: Add MMU node in RHCT
  hw/pci-host/gpex: Define properties for MMIO ranges
  hw/riscv/virt: Update GPEX MMIO related properties
  hw/riscv/virt-acpi-build.c: Add IO controllers and devices
  hw/riscv/virt-acpi-build.c: Add PLIC in MADT

 hw/arm/virt-acpi-build.c        |  51 +----
 hw/i386/acpi-microvm.c          |  15 +-
 hw/nvram/fw_cfg-acpi.c          |  23 +++
 hw/nvram/meson.build            |   1 +
 hw/pci-host/gpex-acpi.c         |  13 ++
 hw/pci-host/gpex.c              |  12 ++
 hw/riscv/Kconfig                |   1 +
 hw/riscv/virt-acpi-build.c      | 323 +++++++++++++++++++++++++++++---
 hw/riscv/virt.c                 |  72 ++++---
 hw/virtio/meson.build           |   1 +
 hw/virtio/virtio-acpi.c         |  32 ++++
 include/hw/nvram/fw_cfg_acpi.h  |  15 ++
 include/hw/pci-host/gpex.h      |  28 ++-
 include/hw/riscv/virt.h         |  26 +++
 include/hw/virtio/virtio-acpi.h |  16 ++
 15 files changed, 498 insertions(+), 131 deletions(-)
 create mode 100644 hw/nvram/fw_cfg-acpi.c
 create mode 100644 hw/virtio/virtio-acpi.c
 create mode 100644 include/hw/nvram/fw_cfg_acpi.h
 create mode 100644 include/hw/virtio/virtio-acpi.h

Comments

Daniel Henrique Barboza Nov. 2, 2023, 9 p.m. UTC | #1 
Sunil,


While doing unrelated work (running Gitlab on my series built on top of
current riscv-to-apply.next), I hit the following error:

https://gitlab.com/danielhb/qemu/-/jobs/5448178994

==========

4/257 ERROR:../tests/qtest/bios-tables-test.c:535:test_acpi_asl: assertion failed: (all_tables_match) ERROR
   4/257 qemu:qtest+qtest-i386 / qtest-i386/bios-tables-test                ERROR           7.77s   killed by signal 6 SIGABRT
>>> G_TEST_DBUS_DAEMON=/builds/danielhb/qemu/tests/dbus-vmstate-daemon.sh PYTHON=/builds/danielhb/qemu/build/pyvenv/bin/python3 MALLOC_PERTURB_=159 QTEST_QEMU_BINARY=./qemu-system-i386 /builds/danielhb/qemu/build/tests/qtest/bios-tables-test --tap -k

acpi-test: Warning! DSDT binary file mismatch. Actual [aml:/tmp/aml-IOYVD2], Expected [aml:tests/data/acpi/microvm/DSDT].
See source file tests/qtest/bios-tables-test.c for instructions on how to update expected files.
to see ASL diff between mismatched files install IASL, rebuild QEMU from scratch and re-run tests with V=1 environment variable set**
ERROR:../tests/qtest/bios-tables-test.c:535:test_acpi_asl: assertion failed: (all_tables_match)
(test program exited with status code -6)

(...)

Summary of Failures:
4571  4/257 qemu:qtest+qtest-i386 / qtest-i386/bios-tables-test                ERROR           7.77s   killed by signal 6 SIGABRT
4572  7/257 qemu:qtest+qtest-aarch64 / qtest-aarch64/bios-tables-test          ERROR          66.00s   killed by signal 6 SIGABRT
4573  Ok:                 247
4574  Expected Fail:      0
4575  Fail:               2
4576  Unexpected Pass:    0
4577  Skipped:            8
4578  Timeout:            0

==========

(qtest-aarch64/bios-tables-test' fails with the same error message as 'qtest-i386/bios-tables-test')


To be sure, since I ran a hacked version of your patches in that Gitlab pipeline, I
removed your ACPI patches from riscv-to-apply.next, applied this new version, and then
tried to run the x86 qtest locally. It still fails, same error:

$ rm -rf build && ./configure --target-list=x86_64-softmmu
$ make -j && QTEST_QEMU_BINARY=./build/qemu-system-x86_64 ./build/tests/qtest/bios-tables-test

(...)
# starting QEMU: exec ./build/qemu-system-x86_64 -qtest unix:/tmp/qtest-508977.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-508977.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -machine microvm -accel kvm -accel tcg -net none  -machine microvm,acpi=on,ioapic2=off,rtc=off -drive id=hd0,if=none,file=tests/acpi-test-disk-eyxyvN,format=raw -device virtio-blk-device,drive=hd0  -accel qtest
acpi-test: Warning! DSDT binary file mismatch. Actual [aml:/tmp/aml-H9IUD2], Expected [aml:tests/data/acpi/microvm/DSDT].
See source file tests/qtest/bios-tables-test.c for instructions on how to update expected files.
to see ASL diff between mismatched files install IASL, rebuild QEMU from scratch and re-run tests with V=1 environment variable set**
ERROR:../tests/qtest/bios-tables-test.c:535:test_acpi_asl: assertion failed: (all_tables_match)
not ok /x86_64/acpi/microvm - ERROR:../tests/qtest/bios-tables-test.c:535:test_acpi_asl: assertion failed: (all_tables_match)


I did a 'git bisect' and it pointed to the following patch:


$ git bisect good
e63248d45e8f8488706ed13a7b83266e578deafd is the first bad commit
commit e63248d45e8f8488706ed13a7b83266e578deafd
Author: Sunil V L <sunilvl@ventanamicro.com>
Date:   Thu Nov 2 22:32:13 2023 +0530

     hw/i386/acpi-microvm.c: Use common function to add virtio in DSDT
     
     With common function to add virtio in DSDT created now, update microvm
     code also to use it instead of duplicate code.
     
     Suggested-by: Andrew Jones <ajones@ventanamicro.com>
     Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
     Acked-by: Alistair Francis <alistair.francis@wdc.com>
     Acked-by: Michael S. Tsirkin <mst@redhat.com>

  hw/i386/acpi-microvm.c | 15 ++-------------
  1 file changed, 2 insertions(+), 13 deletions(-)
$

This suggests a problem in patch 03. I suggest doing a bisect on your own to be sure.

Unfortunately these tests will block the pipeline, forbidding us to merge it. I'm afraid
you'll need to fix it and send a v7.



Thanks,

Daniel


On 11/2/23 14:02, Sunil V L wrote:
> This series primarily enables external interrupt controllers (AIA and PLIC)
> in ACPI tables for RISC-V virt platform. It also updates RHCT with CMO and
> MMU related information.
> 
> Below ECRs for these changes are approved by ASWG and will be
> available in next ACPI spec release.
> 
> 1) MADT (AIA) - https://drive.google.com/file/d/1oMGPyOD58JaPgMl1pKasT-VKsIKia7zR/view?usp=sharing
> 2) RHCT - https://drive.google.com/file/d/1sKbOa8m1UZw1JkquZYe3F1zQBN1xXsaf/view?usp=sharing
> 
> First two patches in this series are to migrate a couple of functions from
> ARM architecture to common code so that RISC-V doesn't need to duplicate
> the same.
> 
> The patch set is based on Alistair's riscv-to-apply.next branch.
> 
> These changes are also available in  riscv_acpi_b2_v6 branch at:
> https://github.com/vlsunil/qemu/
> 
> Changes since v5:
> 	1) Fixed the issue in PATCH 2 reported by Daniel found when built with
> 	   clang + --enable-debug.
> 
> Changes since v4:
> 	1) Updated copyright for new files as per SPDX format suggested by Drew.
> 	2) Updated RINTC patch to avoid code duplication as suggested by Drew.
> 	3) Moved mmu offset below cmo in MMU patch as suggested by Drew.
> 	4) Updated tags.
> 
> Changes since v3:
> 	1) Addressed comments from Daniel and Drew.
> 	2) Added a new patch in microvm to use common function for virtio in DSDT.
> 	3) Rebased to latest riscv-to-apply.next branch and added tags.
> 
> Changes since v2:
>          1) Rebased to latest riscv-to-apply.next branch which needed
>             changing ext_icboz to ext_zicboz in CMO patch.
>          2) Fixed node type in MMU node.
>          3) Added latest tags.
> 
> Changes since v1:
>          1) As per Igor's suggestion, migrated fw_cfg and virtio creation
>             functions to device specific file instead of generic aml-build.c.
>             Since ACPI is optional, new files are created and enabled for
>             build only when CONFIG_ACPI is enabled.
>          2) As per Igor's suggestion, properties are added to the GPEX PCI
>             host to indicate MMIO ranges. The platform fw can initialize
>             these to appropriate values and the DSDT generator can fetch
>             the information from the host bus itself. This makes the code
>             generic instead of machine specific.
>          3) Added PLIC patch from Haibo.
>          4) Rebased to latest riscv-to-apply.next and added RB tags as
>             appropriate.
> Sunil V L (13):
>    hw/arm/virt-acpi-build.c: Migrate fw_cfg creation to common location
>    hw/arm/virt-acpi-build.c: Migrate virtio creation to common location
>    hw/i386/acpi-microvm.c: Use common function to add virtio in DSDT
>    hw/riscv: virt: Make few IMSIC macros and functions public
>    hw/riscv/virt-acpi-build.c: Add AIA support in RINTC
>    hw/riscv/virt-acpi-build.c: Add IMSIC in the MADT
>    hw/riscv/virt-acpi-build.c: Add APLIC in the MADT
>    hw/riscv/virt-acpi-build.c: Add CMO information in RHCT
>    hw/riscv/virt-acpi-build.c: Add MMU node in RHCT
>    hw/pci-host/gpex: Define properties for MMIO ranges
>    hw/riscv/virt: Update GPEX MMIO related properties
>    hw/riscv/virt-acpi-build.c: Add IO controllers and devices
>    hw/riscv/virt-acpi-build.c: Add PLIC in MADT
> 
>   hw/arm/virt-acpi-build.c        |  51 +----
>   hw/i386/acpi-microvm.c          |  15 +-
>   hw/nvram/fw_cfg-acpi.c          |  23 +++
>   hw/nvram/meson.build            |   1 +
>   hw/pci-host/gpex-acpi.c         |  13 ++
>   hw/pci-host/gpex.c              |  12 ++
>   hw/riscv/Kconfig                |   1 +
>   hw/riscv/virt-acpi-build.c      | 323 +++++++++++++++++++++++++++++---
>   hw/riscv/virt.c                 |  72 ++++---
>   hw/virtio/meson.build           |   1 +
>   hw/virtio/virtio-acpi.c         |  32 ++++
>   include/hw/nvram/fw_cfg_acpi.h  |  15 ++
>   include/hw/pci-host/gpex.h      |  28 ++-
>   include/hw/riscv/virt.h         |  26 +++
>   include/hw/virtio/virtio-acpi.h |  16 ++
>   15 files changed, 498 insertions(+), 131 deletions(-)
>   create mode 100644 hw/nvram/fw_cfg-acpi.c
>   create mode 100644 hw/virtio/virtio-acpi.c
>   create mode 100644 include/hw/nvram/fw_cfg_acpi.h
>   create mode 100644 include/hw/virtio/virtio-acpi.h
>
Sunil V L Nov. 3, 2023, 3:21 a.m. UTC | #2 
On Thu, Nov 02, 2023 at 06:00:22PM -0300, Daniel Henrique Barboza wrote:
> Sunil,
> 
> 
> While doing unrelated work (running Gitlab on my series built on top of
> current riscv-to-apply.next), I hit the following error:
> 
> https://gitlab.com/danielhb/qemu/-/jobs/5448178994
> 
> ==========
> 
> 4/257 ERROR:../tests/qtest/bios-tables-test.c:535:test_acpi_asl: assertion failed: (all_tables_match) ERROR
>   4/257 qemu:qtest+qtest-i386 / qtest-i386/bios-tables-test                ERROR           7.77s   killed by signal 6 SIGABRT
> > > > G_TEST_DBUS_DAEMON=/builds/danielhb/qemu/tests/dbus-vmstate-daemon.sh PYTHON=/builds/danielhb/qemu/build/pyvenv/bin/python3 MALLOC_PERTURB_=159 QTEST_QEMU_BINARY=./qemu-system-i386 /builds/danielhb/qemu/build/tests/qtest/bios-tables-test --tap -k
> 
> acpi-test: Warning! DSDT binary file mismatch. Actual [aml:/tmp/aml-IOYVD2], Expected [aml:tests/data/acpi/microvm/DSDT].
> See source file tests/qtest/bios-tables-test.c for instructions on how to update expected files.
> to see ASL diff between mismatched files install IASL, rebuild QEMU from scratch and re-run tests with V=1 environment variable set**
> ERROR:../tests/qtest/bios-tables-test.c:535:test_acpi_asl: assertion failed: (all_tables_match)
> (test program exited with status code -6)
> 
> (...)
> 
> Summary of Failures:
> 4571  4/257 qemu:qtest+qtest-i386 / qtest-i386/bios-tables-test                ERROR           7.77s   killed by signal 6 SIGABRT
> 4572  7/257 qemu:qtest+qtest-aarch64 / qtest-aarch64/bios-tables-test          ERROR          66.00s   killed by signal 6 SIGABRT
> 4573  Ok:                 247
> 4574  Expected Fail:      0
> 4575  Fail:               2
> 4576  Unexpected Pass:    0
> 4577  Skipped:            8
> 4578  Timeout:            0
> 
> ==========
> 
Thanks! Daniel. I sent v7 with this issue fixed. With v7, here is the
qtest report I got.

$ make check-qtest-i386 V=1
...
Ok:                 60  
Expected Fail:      0   
Fail:               0   
Unexpected Pass:    0   
Skipped:            3   
Timeout:            0

$ make check-qtest-aarch64 V=1
...
Ok:                 20  
Expected Fail:      0   
Fail:               0   
Unexpected Pass:    0   
Skipped:            2   
Timeout:            0

$ make check
...
Ok:                 765
Expected Fail:      0   
Fail:               0   
Unexpected Pass:    0   
Skipped:            71  
Timeout:            0

While I don't see any failures now, there are some tests skipped which
look expected. Let me know if I need to run any other tests.

Thanks!
Sunil