mbox

[PULL,v2,00/47] riscv-to-apply queue

Message ID 20240924221751.2688389-1-alistair.francis@wdc.com (mailing list archive)
State New, archived
Headers show

Pull-request

https://github.com/alistair23/qemu.git tags/pull-riscv-to-apply-20240925-1

Message

Alistair Francis Sept. 24, 2024, 10:17 p.m. UTC 
The following changes since commit 01dc65a3bc262ab1bec8fe89775e9bbfa627becb:

  Merge tag 'pull-target-arm-20240919' of https://git.linaro.org/people/pmaydell/qemu-arm into staging (2024-09-19 14:15:15 +0100)

are available in the Git repository at:

  https://github.com/alistair23/qemu.git tags/pull-riscv-to-apply-20240925-1

for you to fetch changes up to 6bfa92c5757fe7a9580e1f6e065076777cae650f:

  bsd-user: Add RISC-V 64-bit Target Configuration and Debug XML Files (2024-09-24 12:53:16 +1000)

----------------------------------------------------------------
RISC-V PR for 9.2

* Add a property to set vl to ceil(AVL/2)
* Enable numamem testing for RISC-V
* Consider MISA bit choice in implied rule
* Fix the za64rs priv spec requirements
* Enable Bit Manip for OpenTitan Ibex CPU
* Fix the group bit setting of AIA with KVM
* Stop timer with infinite timecmp
* Add 'fcsr' register to QEMU log as a part of F extension
* Fix riscv64 build on musl libc
* Add preliminary textra trigger CSR functions
* RISC-V IOMMU support
* RISC-V bsd-user support
* Respect firmware ELF entry point
* Add Svvptc extension support
* Fix masking of rv32 physical address
* Fix linking problem with semihosting disabled
* Fix IMSIC interrupt state updates

----------------------------------------------------------------
Alexandre Ghiti (1):
      target: riscv: Add Svvptc extension support

Alistair Francis (1):
      target: riscv: Enable Bit Manip for OpenTitan Ibex CPU

Alvin Chang (2):
      target/riscv: Preliminary textra trigger CSR writting support
      target/riscv: Add textra matching condition for the triggers

Andrew Jones (3):
      target/riscv/kvm: Fix the group bit setting of AIA
      target/riscv: Stop timer with infinite timecmp
      target/riscv32: Fix masking of physical address

Daniel Henrique Barboza (5):
      target/riscv/tcg/tcg-cpu.c: consider MISA bit choice in implied rule
      pci-ids.rst: add Red Hat pci-id for RISC-V IOMMU device
      test/qtest: add riscv-iommu-pci tests
      qtest/riscv-iommu-test: add init queues test
      docs/specs: add riscv-iommu

Haibo Xu (3):
      tests/acpi: Add empty ACPI SRAT data file for RISC-V
      tests/qtest/bios-tables-test.c: Enable numamem testing for RISC-V
      tests/acpi: Add expected ACPI SRAT AML file for RISC-V

Jason Chien (1):
      target/riscv: Add a property to set vl to ceil(AVL/2)

Maria Klauchek (1):
      target/riscv/cpu.c: Add 'fcsr' register to QEMU log as a part of F extension

Mark Corbin (15):
      bsd-user: Implement RISC-V CPU initialization and main loop
      bsd-user: Add RISC-V CPU execution loop and syscall handling
      bsd-user: Implement RISC-V CPU register cloning and reset functions
      bsd-user: Implement RISC-V TLS register setup
      bsd-user: Add RISC-V ELF definitions and hardware capability detection
      bsd-user: Define RISC-V register structures and register copying
      bsd-user: Add RISC-V signal trampoline setup function
      bsd-user: Implement RISC-V sysarch system call emulation
      bsd-user: Add RISC-V thread setup and initialization support
      bsd-user: Define RISC-V VM parameters and helper functions
      bsd-user: Define RISC-V system call structures and constants
      bsd-user: Define RISC-V signal handling structures and constants
      bsd-user: Implement RISC-V signal trampoline setup functions
      bsd-user: Implement 'get_mcontext' for RISC-V
      bsd-user: Implement set_mcontext and get_ucontext_sigreturn for RISCV

Milan P. Stanić (1):
      util/util/cpuinfo-riscv.c: fix riscv64 build on musl libc

Samuel Holland (1):
      hw/riscv: Respect firmware ELF entry point

Thomas Huth (1):
      target/riscv/cpu_helper: Fix linking problem with semihosting disabled

Tomasz Jeznach (9):
      exec/memtxattr: add process identifier to the transaction attributes
      hw/riscv: add riscv-iommu-bits.h
      hw/riscv: add RISC-V IOMMU base emulation
      hw/riscv: add riscv-iommu-pci reference device
      hw/riscv/virt.c: support for RISC-V IOMMU PCIDevice hotplug
      hw/riscv/riscv-iommu: add Address Translation Cache (IOATC)
      hw/riscv/riscv-iommu: add ATS support
      hw/riscv/riscv-iommu: add DBG support
      hw/intc: riscv-imsic: Fix interrupt state updates.

Vladimir Isaev (1):
      target/riscv: fix za64rs enabling

Warner Losh (2):
      bsd-user: Add generic RISC-V64 target definitions
      bsd-user: Add RISC-V 64-bit Target Configuration and Debug XML Files

 docs/specs/index.rst                      |    1 +
 docs/specs/pci-ids.rst                    |    2 +
 docs/specs/riscv-iommu.rst                |   90 ++
 docs/system/riscv/virt.rst                |   13 +
 configs/targets/riscv64-bsd-user.mak      |    4 +
 meson.build                               |    1 +
 bsd-user/riscv/target.h                   |   20 +
 bsd-user/riscv/target_arch.h              |   27 +
 bsd-user/riscv/target_arch_cpu.h          |  148 ++
 bsd-user/riscv/target_arch_elf.h          |   42 +
 bsd-user/riscv/target_arch_reg.h          |   88 ++
 bsd-user/riscv/target_arch_signal.h       |   75 +
 bsd-user/riscv/target_arch_sigtramp.h     |   41 +
 bsd-user/riscv/target_arch_sysarch.h      |   41 +
 bsd-user/riscv/target_arch_thread.h       |   47 +
 bsd-user/riscv/target_arch_vmparam.h      |   53 +
 bsd-user/riscv/target_syscall.h           |   38 +
 hw/riscv/riscv-iommu-bits.h               |  421 +++++
 hw/riscv/riscv-iommu.h                    |  149 ++
 hw/riscv/trace.h                          |    1 +
 include/exec/memattrs.h                   |    5 +
 include/hw/pci/pci.h                      |    1 +
 include/hw/riscv/boot.h                   |    4 +-
 include/hw/riscv/iommu.h                  |   36 +
 target/riscv/cpu_bits.h                   |   10 +
 target/riscv/cpu_cfg.h                    |    2 +
 target/riscv/debug.h                      |    3 +
 tests/qtest/libqos/riscv-iommu.h          |  101 ++
 bsd-user/riscv/signal.c                   |  170 ++
 bsd-user/riscv/target_arch_cpu.c          |   29 +
 hw/intc/riscv_imsic.c                     |   50 +-
 hw/riscv/boot.c                           |   11 +-
 hw/riscv/microchip_pfsoc.c                |    2 +-
 hw/riscv/opentitan.c                      |    3 +-
 hw/riscv/riscv-iommu-pci.c                |  202 +++
 hw/riscv/riscv-iommu.c                    | 2431 +++++++++++++++++++++++++++++
 hw/riscv/shakti_c.c                       |   13 +-
 hw/riscv/sifive_u.c                       |    4 +-
 hw/riscv/spike.c                          |    5 +-
 hw/riscv/virt.c                           |   37 +-
 target/riscv/cpu.c                        |   16 +-
 target/riscv/cpu_helper.c                 |    8 +-
 target/riscv/debug.c                      |  114 +-
 target/riscv/kvm/kvm-cpu.c                |    4 +-
 target/riscv/tcg/tcg-cpu.c                |   13 +-
 target/riscv/time_helper.c                |    1 +
 target/riscv/vector_helper.c              |    2 +
 tests/qtest/bios-tables-test.c            |   28 +
 tests/qtest/libqos/riscv-iommu.c          |   76 +
 tests/qtest/riscv-iommu-test.c            |  220 +++
 util/cpuinfo-riscv.c                      |    1 +
 hw/riscv/Kconfig                          |    4 +
 hw/riscv/meson.build                      |    1 +
 hw/riscv/trace-events                     |   17 +
 target/riscv/Kconfig                      |    4 +-
 tests/data/acpi/riscv64/virt/SRAT.numamem |  Bin 0 -> 108 bytes
 tests/qtest/libqos/meson.build            |    4 +
 tests/qtest/meson.build                   |    1 +
 58 files changed, 4877 insertions(+), 58 deletions(-)
 create mode 100644 docs/specs/riscv-iommu.rst
 create mode 100644 configs/targets/riscv64-bsd-user.mak
 create mode 100644 bsd-user/riscv/target.h
 create mode 100644 bsd-user/riscv/target_arch.h
 create mode 100644 bsd-user/riscv/target_arch_cpu.h
 create mode 100644 bsd-user/riscv/target_arch_elf.h
 create mode 100644 bsd-user/riscv/target_arch_reg.h
 create mode 100644 bsd-user/riscv/target_arch_signal.h
 create mode 100644 bsd-user/riscv/target_arch_sigtramp.h
 create mode 100644 bsd-user/riscv/target_arch_sysarch.h
 create mode 100644 bsd-user/riscv/target_arch_thread.h
 create mode 100644 bsd-user/riscv/target_arch_vmparam.h
 create mode 100644 bsd-user/riscv/target_syscall.h
 create mode 100644 hw/riscv/riscv-iommu-bits.h
 create mode 100644 hw/riscv/riscv-iommu.h
 create mode 100644 hw/riscv/trace.h
 create mode 100644 include/hw/riscv/iommu.h
 create mode 100644 tests/qtest/libqos/riscv-iommu.h
 create mode 100644 bsd-user/riscv/signal.c
 create mode 100644 bsd-user/riscv/target_arch_cpu.c
 create mode 100644 hw/riscv/riscv-iommu-pci.c
 create mode 100644 hw/riscv/riscv-iommu.c
 create mode 100644 tests/qtest/libqos/riscv-iommu.c
 create mode 100644 tests/qtest/riscv-iommu-test.c
 create mode 100644 hw/riscv/trace-events
 create mode 100644 tests/data/acpi/riscv64/virt/SRAT.numamem

Comments

Peter Maydell Sept. 28, 2024, 11:34 a.m. UTC | #1 
On Tue, 24 Sept 2024 at 23:18, Alistair Francis <alistair23@gmail.com> wrote:
>
> The following changes since commit 01dc65a3bc262ab1bec8fe89775e9bbfa627becb:
>
>   Merge tag 'pull-target-arm-20240919' of https://git.linaro.org/people/pmaydell/qemu-arm into staging (2024-09-19 14:15:15 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/alistair23/qemu.git tags/pull-riscv-to-apply-20240925-1
>
> for you to fetch changes up to 6bfa92c5757fe7a9580e1f6e065076777cae650f:
>
>   bsd-user: Add RISC-V 64-bit Target Configuration and Debug XML Files (2024-09-24 12:53:16 +1000)
>
> ----------------------------------------------------------------
> RISC-V PR for 9.2
>
> * Add a property to set vl to ceil(AVL/2)
> * Enable numamem testing for RISC-V
> * Consider MISA bit choice in implied rule
> * Fix the za64rs priv spec requirements
> * Enable Bit Manip for OpenTitan Ibex CPU
> * Fix the group bit setting of AIA with KVM
> * Stop timer with infinite timecmp
> * Add 'fcsr' register to QEMU log as a part of F extension
> * Fix riscv64 build on musl libc
> * Add preliminary textra trigger CSR functions
> * RISC-V IOMMU support
> * RISC-V bsd-user support
> * Respect firmware ELF entry point
> * Add Svvptc extension support
> * Fix masking of rv32 physical address
> * Fix linking problem with semihosting disabled
> * Fix IMSIC interrupt state updates
>

This fails the riscv qos-tests on s390x. My guess is that the new
IOMMU support has endianness bugs and fails on bigendian hosts.

https://gitlab.com/qemu-project/qemu/-/jobs/7942189143

full test log (4MB) at
https://qemu-project.gitlab.io/-/qemu/-/jobs/7942189143/artifacts/build/meson-logs/testlog.txt

The assertion failure is
ERROR:../tests/qtest/riscv-iommu-test.c:72:test_reg_reset: assertion
failed (cap & RISCV_IOMMU_CAP_VERSION == 0x10): (0 == 16)

but there are a lot of virtio errors before that so the
problem probably happened rather earlier.

thanks
-- PMM
Peter Maydell Sept. 28, 2024, 8:23 p.m. UTC | #2 
On Sat, 28 Sept 2024 at 12:34, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 24 Sept 2024 at 23:18, Alistair Francis <alistair23@gmail.com> wrote:
> >
> > The following changes since commit 01dc65a3bc262ab1bec8fe89775e9bbfa627becb:
> >
> >   Merge tag 'pull-target-arm-20240919' of https://git.linaro.org/people/pmaydell/qemu-arm into staging (2024-09-19 14:15:15 +0100)
> >
> > are available in the Git repository at:
> >
> >   https://github.com/alistair23/qemu.git tags/pull-riscv-to-apply-20240925-1
> >
> > for you to fetch changes up to 6bfa92c5757fe7a9580e1f6e065076777cae650f:
> >
> >   bsd-user: Add RISC-V 64-bit Target Configuration and Debug XML Files (2024-09-24 12:53:16 +1000)
> >
> > ----------------------------------------------------------------
> > RISC-V PR for 9.2
> >
> > * Add a property to set vl to ceil(AVL/2)
> > * Enable numamem testing for RISC-V
> > * Consider MISA bit choice in implied rule
> > * Fix the za64rs priv spec requirements
> > * Enable Bit Manip for OpenTitan Ibex CPU
> > * Fix the group bit setting of AIA with KVM
> > * Stop timer with infinite timecmp
> > * Add 'fcsr' register to QEMU log as a part of F extension
> > * Fix riscv64 build on musl libc
> > * Add preliminary textra trigger CSR functions
> > * RISC-V IOMMU support
> > * RISC-V bsd-user support
> > * Respect firmware ELF entry point
> > * Add Svvptc extension support
> > * Fix masking of rv32 physical address
> > * Fix linking problem with semihosting disabled
> > * Fix IMSIC interrupt state updates
> >
>
> This fails the riscv qos-tests on s390x. My guess is that the new
> IOMMU support has endianness bugs and fails on bigendian hosts.

I have also noticed that the license text comments in the iommu
patches are confused. That must be fixed before we can accept them.
(see my reply to patch 16.)

thanks
-- PMM
Daniel Henrique Barboza Sept. 28, 2024, 8:40 p.m. UTC | #3 
On 9/28/24 8:34 AM, Peter Maydell wrote:
> On Tue, 24 Sept 2024 at 23:18, Alistair Francis <alistair23@gmail.com> wrote:
>>
>> The following changes since commit 01dc65a3bc262ab1bec8fe89775e9bbfa627becb:
>>
>>    Merge tag 'pull-target-arm-20240919' of https://git.linaro.org/people/pmaydell/qemu-arm into staging (2024-09-19 14:15:15 +0100)
>>
>> are available in the Git repository at:
>>
>>    https://github.com/alistair23/qemu.git tags/pull-riscv-to-apply-20240925-1
>>
>> for you to fetch changes up to 6bfa92c5757fe7a9580e1f6e065076777cae650f:
>>
>>    bsd-user: Add RISC-V 64-bit Target Configuration and Debug XML Files (2024-09-24 12:53:16 +1000)
>>
>> ----------------------------------------------------------------
>> RISC-V PR for 9.2
>>
>> * Add a property to set vl to ceil(AVL/2)
>> * Enable numamem testing for RISC-V
>> * Consider MISA bit choice in implied rule
>> * Fix the za64rs priv spec requirements
>> * Enable Bit Manip for OpenTitan Ibex CPU
>> * Fix the group bit setting of AIA with KVM
>> * Stop timer with infinite timecmp
>> * Add 'fcsr' register to QEMU log as a part of F extension
>> * Fix riscv64 build on musl libc
>> * Add preliminary textra trigger CSR functions
>> * RISC-V IOMMU support
>> * RISC-V bsd-user support
>> * Respect firmware ELF entry point
>> * Add Svvptc extension support
>> * Fix masking of rv32 physical address
>> * Fix linking problem with semihosting disabled
>> * Fix IMSIC interrupt state updates
>>
> 
> This fails the riscv qos-tests on s390x. My guess is that the new
> IOMMU support has endianness bugs and fails on bigendian hosts.
> 
> https://gitlab.com/qemu-project/qemu/-/jobs/7942189143
> 
> full test log (4MB) at
> https://qemu-project.gitlab.io/-/qemu/-/jobs/7942189143/artifacts/build/meson-logs/testlog.txt
> 
> The assertion failure is
> ERROR:../tests/qtest/riscv-iommu-test.c:72:test_reg_reset: assertion
> failed (cap & RISCV_IOMMU_CAP_VERSION == 0x10): (0 == 16)

The root cause is that the qtests I added aren't considering the endianess of the
host. The RISC-V IOMMU is being implemented as LE only and all regs are being
read/written in memory as LE. The qtest read/write helpers must take the qtest
endianess into account. We make this type of handling in other qtest archs like
ppc64.

I have a fix for the tests but I'm unable to run the ubuntu-22.04-s390x-all-system
job to verify it, even after setting Cirrus like Thomas taught me a week ago. In
fact I have no 'ubuntu-22-*' jobs available to run.

If there's a way to run these ubuntu s390x tests, let me know. Otherwise I'm inclined
to remove the IOMMU qtests for now until I'm able to verify that they'll work in a
BE host (I'll install a BE VM to verify).

I also saw that you made comments in one of the patches w.r.t license text and other
changes in the base code. I'll get to it.


Thanks,


Daniel



> 
> but there are a lot of virtio errors before that so the
> problem probably happened rather earlier.
> 
> thanks
> -- PMM
>
Peter Maydell Sept. 29, 2024, 3:38 p.m. UTC | #4 
On Sat, 28 Sept 2024 at 21:40, Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
>
>
> On 9/28/24 8:34 AM, Peter Maydell wrote:
> > The assertion failure is
> > ERROR:../tests/qtest/riscv-iommu-test.c:72:test_reg_reset: assertion
> > failed (cap & RISCV_IOMMU_CAP_VERSION == 0x10): (0 == 16)
>
> The root cause is that the qtests I added aren't considering the endianess of the
> host. The RISC-V IOMMU is being implemented as LE only and all regs are being
> read/written in memory as LE. The qtest read/write helpers must take the qtest
> endianess into account. We make this type of handling in other qtest archs like
> ppc64.
>
> I have a fix for the tests but I'm unable to run the ubuntu-22.04-s390x-all-system
> job to verify it, even after setting Cirrus like Thomas taught me a week ago. In
> fact I have no 'ubuntu-22-*' jobs available to run.

It's on the private s390 VM we have, so it's set up only to
be available on the main CI run (there's not enough capacity
on the machine to do any more than that). If you want to point
me at a gitlab branch I can do a quick "make check" on that
if you like.

thanks
-- PMM
Daniel Henrique Barboza Sept. 29, 2024, 8:53 p.m. UTC | #5 
On 9/29/24 12:38 PM, Peter Maydell wrote:
> On Sat, 28 Sept 2024 at 21:40, Daniel Henrique Barboza
> <dbarboza@ventanamicro.com> wrote:
>>
>>
>>
>> On 9/28/24 8:34 AM, Peter Maydell wrote:
>>> The assertion failure is
>>> ERROR:../tests/qtest/riscv-iommu-test.c:72:test_reg_reset: assertion
>>> failed (cap & RISCV_IOMMU_CAP_VERSION == 0x10): (0 == 16)
>>
>> The root cause is that the qtests I added aren't considering the endianess of the
>> host. The RISC-V IOMMU is being implemented as LE only and all regs are being
>> read/written in memory as LE. The qtest read/write helpers must take the qtest
>> endianess into account. We make this type of handling in other qtest archs like
>> ppc64.
>>
>> I have a fix for the tests but I'm unable to run the ubuntu-22.04-s390x-all-system
>> job to verify it, even after setting Cirrus like Thomas taught me a week ago. In
>> fact I have no 'ubuntu-22-*' jobs available to run.
> 
> It's on the private s390 VM we have, so it's set up only to
> be available on the main CI run (there's not enough capacity
> on the machine to do any more than that). If you want to point
> me at a gitlab branch I can do a quick "make check" on that
> if you like.

I appreciate it. This is the repo:

https://gitlab.com/danielhb/qemu/-/tree/pull_fix

If this is enough to fix the tests, I'll amend it in the new IOMMU version.
If we still failing then I'll need to set this s390 VM.

By the way, if you have any recipe/pointers to set this s390 VM to share,
that would be great.


Thanks,


Daniel



> 
> thanks
> -- PMM
Peter Maydell Sept. 30, 2024, 10:48 a.m. UTC | #6 
On Sun, 29 Sept 2024 at 21:53, Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
>
>
> On 9/29/24 12:38 PM, Peter Maydell wrote:
> > On Sat, 28 Sept 2024 at 21:40, Daniel Henrique Barboza
> > <dbarboza@ventanamicro.com> wrote:
> >>
> >>
> >>
> >> On 9/28/24 8:34 AM, Peter Maydell wrote:
> >>> The assertion failure is
> >>> ERROR:../tests/qtest/riscv-iommu-test.c:72:test_reg_reset: assertion
> >>> failed (cap & RISCV_IOMMU_CAP_VERSION == 0x10): (0 == 16)
> >>
> >> The root cause is that the qtests I added aren't considering the endianess of the
> >> host. The RISC-V IOMMU is being implemented as LE only and all regs are being
> >> read/written in memory as LE. The qtest read/write helpers must take the qtest
> >> endianess into account. We make this type of handling in other qtest archs like
> >> ppc64.
> >>
> >> I have a fix for the tests but I'm unable to run the ubuntu-22.04-s390x-all-system
> >> job to verify it, even after setting Cirrus like Thomas taught me a week ago. In
> >> fact I have no 'ubuntu-22-*' jobs available to run.
> >
> > It's on the private s390 VM we have, so it's set up only to
> > be available on the main CI run (there's not enough capacity
> > on the machine to do any more than that). If you want to point
> > me at a gitlab branch I can do a quick "make check" on that
> > if you like.
>
> I appreciate it. This is the repo:
>
> https://gitlab.com/danielhb/qemu/-/tree/pull_fix

This doesn't fix the assertion. This is because the test (now) does:

  qpci_memread(&r_iommu->dev, r_iommu->reg_bar, reg_offset,
               &reg, sizeof(reg));

  if (riscv_iommu_qtest_big_endian()) {
      reg = bswap32(reg);
  }

where riscv_iommu_qtest_big_endian() is a wrapper for
qtest_big_endian(). But qtest_big_endian() queries the
endianness of the *guest*, and so for riscv it will
always return false and we will never bswap.

If you need to do swapping inline in a test you can use
  reg = le32_to_cpu(reg);
which swaps an LE value read from the guest to the host
CPU's endianness ordering (and similarly with cpu_to_le32
on the write path).

But it turns out that libqos provides already functions
to read/write 32 and 64 bit values from PCI devices:
   reg = qpci_io_readl(&r_iommu->dev, r_iommu->reg_bar, reg_offset);
which do the byteswap for you.
Similarly qpci_io_writel() etc. (The functions work for
both IO and MEM PCI BARs.)

> If this is enough to fix the tests, I'll amend it in the new IOMMU version.
> If we still failing then I'll need to set this s390 VM.
>
> By the way, if you have any recipe/pointers to set this s390 VM to share,
> that would be great.

It's a VM provided by IBM under their "Community Cloud"
umbrella: https://community.ibm.com/zsystems/l1cc/

thanks
-- PMM
Thomas Huth Sept. 30, 2024, 10:58 a.m. UTC | #7 
On 29/09/2024 22.53, Daniel Henrique Barboza wrote:
> 
> 
> On 9/29/24 12:38 PM, Peter Maydell wrote:
>> On Sat, 28 Sept 2024 at 21:40, Daniel Henrique Barboza
>> <dbarboza@ventanamicro.com> wrote:
>>>
>>>
>>>
>>> On 9/28/24 8:34 AM, Peter Maydell wrote:
>>>> The assertion failure is
>>>> ERROR:../tests/qtest/riscv-iommu-test.c:72:test_reg_reset: assertion
>>>> failed (cap & RISCV_IOMMU_CAP_VERSION == 0x10): (0 == 16)
>>>
>>> The root cause is that the qtests I added aren't considering the 
>>> endianess of the
>>> host. The RISC-V IOMMU is being implemented as LE only and all regs are 
>>> being
>>> read/written in memory as LE. The qtest read/write helpers must take the 
>>> qtest
>>> endianess into account. We make this type of handling in other qtest 
>>> archs like
>>> ppc64.
>>>
>>> I have a fix for the tests but I'm unable to run the ubuntu-22.04-s390x- 
>>> all-system
>>> job to verify it, even after setting Cirrus like Thomas taught me a week 
>>> ago. In
>>> fact I have no 'ubuntu-22-*' jobs available to run.
>>
>> It's on the private s390 VM we have, so it's set up only to
>> be available on the main CI run (there's not enough capacity
>> on the machine to do any more than that). If you want to point
>> me at a gitlab branch I can do a quick "make check" on that
>> if you like.
> 
> I appreciate it. This is the repo:
> 
> https://gitlab.com/danielhb/qemu/-/tree/pull_fix
> 
> If this is enough to fix the tests, I'll amend it in the new IOMMU version.
> If we still failing then I'll need to set this s390 VM.
> 
> By the way, if you have any recipe/pointers to set this s390 VM to share,
> that would be great.

You can also use Travis-CI for testing QEMU on a s390x host, see e.g. my 
runs here:

  https://app.travis-ci.com/github/huth/qemu

You just need a github account and set up the Travis-CI for that repository 
there - only caveat: in case you run out of open source credits, you've got 
to ask the Travis support for granting you new credits (seems like one has 
to do it a year from what I experienced in the past).

  HTH,
   Thomas
Thomas Huth Sept. 30, 2024, 11:35 a.m. UTC | #8 
On 30/09/2024 12.58, Thomas Huth wrote:
> On 29/09/2024 22.53, Daniel Henrique Barboza wrote:
>>
>>
>> On 9/29/24 12:38 PM, Peter Maydell wrote:
>>> On Sat, 28 Sept 2024 at 21:40, Daniel Henrique Barboza
>>> <dbarboza@ventanamicro.com> wrote:
>>>>
>>>>
>>>>
>>>> On 9/28/24 8:34 AM, Peter Maydell wrote:
>>>>> The assertion failure is
>>>>> ERROR:../tests/qtest/riscv-iommu-test.c:72:test_reg_reset: assertion
>>>>> failed (cap & RISCV_IOMMU_CAP_VERSION == 0x10): (0 == 16)
>>>>
>>>> The root cause is that the qtests I added aren't considering the 
>>>> endianess of the
>>>> host. The RISC-V IOMMU is being implemented as LE only and all regs are 
>>>> being
>>>> read/written in memory as LE. The qtest read/write helpers must take the 
>>>> qtest
>>>> endianess into account. We make this type of handling in other qtest 
>>>> archs like
>>>> ppc64.
>>>>
>>>> I have a fix for the tests but I'm unable to run the ubuntu-22.04-s390x- 
>>>> all-system
>>>> job to verify it, even after setting Cirrus like Thomas taught me a week 
>>>> ago. In
>>>> fact I have no 'ubuntu-22-*' jobs available to run.
>>>
>>> It's on the private s390 VM we have, so it's set up only to
>>> be available on the main CI run (there's not enough capacity
>>> on the machine to do any more than that). If you want to point
>>> me at a gitlab branch I can do a quick "make check" on that
>>> if you like.
>>
>> I appreciate it. This is the repo:
>>
>> https://gitlab.com/danielhb/qemu/-/tree/pull_fix
>>
>> If this is enough to fix the tests, I'll amend it in the new IOMMU version.
>> If we still failing then I'll need to set this s390 VM.
>>
>> By the way, if you have any recipe/pointers to set this s390 VM to share,
>> that would be great.
> 
> You can also use Travis-CI for testing QEMU on a s390x host, see e.g. my 
> runs here:
> 
>   https://app.travis-ci.com/github/huth/qemu

... apart from the fact, that it currently seems to be completely broken 
again :-( ... I guess that means: Never mind, sorry for the distraction!

  Thomas
Daniel Henrique Barboza Sept. 30, 2024, 12:05 p.m. UTC | #9 
On 9/30/24 7:48 AM, Peter Maydell wrote:
> On Sun, 29 Sept 2024 at 21:53, Daniel Henrique Barboza
> <dbarboza@ventanamicro.com> wrote:
>>
>>
>>
>> On 9/29/24 12:38 PM, Peter Maydell wrote:
>>> On Sat, 28 Sept 2024 at 21:40, Daniel Henrique Barboza
>>> <dbarboza@ventanamicro.com> wrote:
>>>>
>>>>
>>>>
>>>> On 9/28/24 8:34 AM, Peter Maydell wrote:
>>>>> The assertion failure is
>>>>> ERROR:../tests/qtest/riscv-iommu-test.c:72:test_reg_reset: assertion
>>>>> failed (cap & RISCV_IOMMU_CAP_VERSION == 0x10): (0 == 16)
>>>>
>>>> The root cause is that the qtests I added aren't considering the endianess of the
>>>> host. The RISC-V IOMMU is being implemented as LE only and all regs are being
>>>> read/written in memory as LE. The qtest read/write helpers must take the qtest
>>>> endianess into account. We make this type of handling in other qtest archs like
>>>> ppc64.
>>>>
>>>> I have a fix for the tests but I'm unable to run the ubuntu-22.04-s390x-all-system
>>>> job to verify it, even after setting Cirrus like Thomas taught me a week ago. In
>>>> fact I have no 'ubuntu-22-*' jobs available to run.
>>>
>>> It's on the private s390 VM we have, so it's set up only to
>>> be available on the main CI run (there's not enough capacity
>>> on the machine to do any more than that). If you want to point
>>> me at a gitlab branch I can do a quick "make check" on that
>>> if you like.
>>
>> I appreciate it. This is the repo:
>>
>> https://gitlab.com/danielhb/qemu/-/tree/pull_fix
> 
> This doesn't fix the assertion. This is because the test (now) does:
> 
>    qpci_memread(&r_iommu->dev, r_iommu->reg_bar, reg_offset,
>                 &reg, sizeof(reg));
> 
>    if (riscv_iommu_qtest_big_endian()) {
>        reg = bswap32(reg);
>    }
> 
> where riscv_iommu_qtest_big_endian() is a wrapper for
> qtest_big_endian(). But qtest_big_endian() queries the
> endianness of the *guest*, and so for riscv it will
> always return false and we will never bswap.

Ooops. My bad.

> 
> If you need to do swapping inline in a test you can use
>    reg = le32_to_cpu(reg);
> which swaps an LE value read from the guest to the host
> CPU's endianness ordering (and similarly with cpu_to_le32
> on the write path).
> 
> But it turns out that libqos provides already functions
> to read/write 32 and 64 bit values from PCI devices:
>     reg = qpci_io_readl(&r_iommu->dev, r_iommu->reg_bar, reg_offset);
> which do the byteswap for you.
> Similarly qpci_io_writel() etc. (The functions work for
> both IO and MEM PCI BARs.)

I'll convert the tests to use qcpi_io_readl/writel and friends. Hopefully this
will fix it.

> 
>> If this is enough to fix the tests, I'll amend it in the new IOMMU version.
>> If we still failing then I'll need to set this s390 VM.
>>
>> By the way, if you have any recipe/pointers to set this s390 VM to share,
>> that would be great.
> 
> It's a VM provided by IBM under their "Community Cloud"
> umbrella: https://community.ibm.com/zsystems/l1cc/


I'll see if I can get something going with the IBM community cloud, but I wonder
how hard it is to launch a s390x TCG guest with Ubuntu. The image is freely
available:

https://cdimage.ubuntu.com/releases/jammy/release/ubuntu-22.04.5-live-server-s390x.iso


But I'm unsure of whether we need some secret sauce/paid key to do the install. Thomas,
do you have more info?


Thanks,


Daniel

> 
> thanks
> -- PMM
Ilya Leoshkevich Sept. 30, 2024, 12:10 p.m. UTC | #10 
On Sat, 2024-09-28 at 17:40 -0300, Daniel Henrique Barboza wrote:
> 
> 
> On 9/28/24 8:34 AM, Peter Maydell wrote:
> > On Tue, 24 Sept 2024 at 23:18, Alistair Francis
> > <alistair23@gmail.com> wrote:
> > > 
> > > The following changes since commit
> > > 01dc65a3bc262ab1bec8fe89775e9bbfa627becb:
> > > 
> > >    Merge tag 'pull-target-arm-20240919' of
> > > https://git.linaro.org/people/pmaydell/qemu-arm into staging
> > > (2024-09-19 14:15:15 +0100)
> > > 
> > > are available in the Git repository at:
> > > 
> > >    https://github.com/alistair23/qemu.git tags/pull-riscv-to-
> > > apply-20240925-1
> > > 
> > > for you to fetch changes up to
> > > 6bfa92c5757fe7a9580e1f6e065076777cae650f:
> > > 
> > >    bsd-user: Add RISC-V 64-bit Target Configuration and Debug XML
> > > Files (2024-09-24 12:53:16 +1000)
> > > 
> > > ----------------------------------------------------------------
> > > RISC-V PR for 9.2
> > > 
> > > * Add a property to set vl to ceil(AVL/2)
> > > * Enable numamem testing for RISC-V
> > > * Consider MISA bit choice in implied rule
> > > * Fix the za64rs priv spec requirements
> > > * Enable Bit Manip for OpenTitan Ibex CPU
> > > * Fix the group bit setting of AIA with KVM
> > > * Stop timer with infinite timecmp
> > > * Add 'fcsr' register to QEMU log as a part of F extension
> > > * Fix riscv64 build on musl libc
> > > * Add preliminary textra trigger CSR functions
> > > * RISC-V IOMMU support
> > > * RISC-V bsd-user support
> > > * Respect firmware ELF entry point
> > > * Add Svvptc extension support
> > > * Fix masking of rv32 physical address
> > > * Fix linking problem with semihosting disabled
> > > * Fix IMSIC interrupt state updates
> > > 
> > 
> > This fails the riscv qos-tests on s390x. My guess is that the new
> > IOMMU support has endianness bugs and fails on bigendian hosts.
> > 
> > https://gitlab.com/qemu-project/qemu/-/jobs/7942189143
> > 
> > full test log (4MB) at
> > https://qemu-project.gitlab.io/-/qemu/-/jobs/7942189143/artifacts/build/meson-logs/testlog.txt
> > 
> > The assertion failure is
> > ERROR:../tests/qtest/riscv-iommu-test.c:72:test_reg_reset:
> > assertion
> > failed (cap & RISCV_IOMMU_CAP_VERSION == 0x10): (0 == 16)
> 
> The root cause is that the qtests I added aren't considering the
> endianess of the
> host. The RISC-V IOMMU is being implemented as LE only and all regs
> are being
> read/written in memory as LE. The qtest read/write helpers must take
> the qtest
> endianess into account. We make this type of handling in other qtest
> archs like
> ppc64.
> 
> I have a fix for the tests but I'm unable to run the ubuntu-22.04-
> s390x-all-system
> job to verify it, even after setting Cirrus like Thomas taught me a
> week ago. In
> fact I have no 'ubuntu-22-*' jobs available to run.
> 
> If there's a way to run these ubuntu s390x tests, let me know.
> Otherwise I'm inclined
> to remove the IOMMU qtests for now until I'm able to verify that
> they'll work in a
> BE host (I'll install a BE VM to verify).

You can get a free s390x VM here:

https://linuxone.cloud.marist.edu/#/register?flag=VM

Best regards,
Ilya

[...]
Daniel Henrique Barboza Sept. 30, 2024, 11:04 p.m. UTC | #11 
On 9/30/24 9:10 AM, Ilya Leoshkevich wrote:
> On Sat, 2024-09-28 at 17:40 -0300, Daniel Henrique Barboza wrote:
>>
>>
>> On 9/28/24 8:34 AM, Peter Maydell wrote:
>>> On Tue, 24 Sept 2024 at 23:18, Alistair Francis
>>> <alistair23@gmail.com> wrote:
>>>>
>>>> The following changes since commit
>>>> 01dc65a3bc262ab1bec8fe89775e9bbfa627becb:
>>>>
>>>>     Merge tag 'pull-target-arm-20240919' of
>>>> https://git.linaro.org/people/pmaydell/qemu-arm into staging
>>>> (2024-09-19 14:15:15 +0100)
>>>>
>>>> are available in the Git repository at:
>>>>
>>>>     https://github.com/alistair23/qemu.git tags/pull-riscv-to-
>>>> apply-20240925-1
>>>>
>>>> for you to fetch changes up to
>>>> 6bfa92c5757fe7a9580e1f6e065076777cae650f:
>>>>
>>>>     bsd-user: Add RISC-V 64-bit Target Configuration and Debug XML
>>>> Files (2024-09-24 12:53:16 +1000)
>>>>
>>>> ----------------------------------------------------------------
>>>> RISC-V PR for 9.2
>>>>
>>>> * Add a property to set vl to ceil(AVL/2)
>>>> * Enable numamem testing for RISC-V
>>>> * Consider MISA bit choice in implied rule
>>>> * Fix the za64rs priv spec requirements
>>>> * Enable Bit Manip for OpenTitan Ibex CPU
>>>> * Fix the group bit setting of AIA with KVM
>>>> * Stop timer with infinite timecmp
>>>> * Add 'fcsr' register to QEMU log as a part of F extension
>>>> * Fix riscv64 build on musl libc
>>>> * Add preliminary textra trigger CSR functions
>>>> * RISC-V IOMMU support
>>>> * RISC-V bsd-user support
>>>> * Respect firmware ELF entry point
>>>> * Add Svvptc extension support
>>>> * Fix masking of rv32 physical address
>>>> * Fix linking problem with semihosting disabled
>>>> * Fix IMSIC interrupt state updates
>>>>
>>>
>>> This fails the riscv qos-tests on s390x. My guess is that the new
>>> IOMMU support has endianness bugs and fails on bigendian hosts.
>>>
>>> https://gitlab.com/qemu-project/qemu/-/jobs/7942189143
>>>
>>> full test log (4MB) at
>>> https://qemu-project.gitlab.io/-/qemu/-/jobs/7942189143/artifacts/build/meson-logs/testlog.txt
>>>
>>> The assertion failure is
>>> ERROR:../tests/qtest/riscv-iommu-test.c:72:test_reg_reset:
>>> assertion
>>> failed (cap & RISCV_IOMMU_CAP_VERSION == 0x10): (0 == 16)
>>
>> The root cause is that the qtests I added aren't considering the
>> endianess of the
>> host. The RISC-V IOMMU is being implemented as LE only and all regs
>> are being
>> read/written in memory as LE. The qtest read/write helpers must take
>> the qtest
>> endianess into account. We make this type of handling in other qtest
>> archs like
>> ppc64.
>>
>> I have a fix for the tests but I'm unable to run the ubuntu-22.04-
>> s390x-all-system
>> job to verify it, even after setting Cirrus like Thomas taught me a
>> week ago. In
>> fact I have no 'ubuntu-22-*' jobs available to run.
>>
>> If there's a way to run these ubuntu s390x tests, let me know.
>> Otherwise I'm inclined
>> to remove the IOMMU qtests for now until I'm able to verify that
>> they'll work in a
>> BE host (I'll install a BE VM to verify).
> 
> You can get a free s390x VM here:
> 
> https://linuxone.cloud.marist.edu/#/register?flag=VM

Thanks! This was surprisingly easy to set up and run. Please send my best regards
to the LinuxOne Community Cloud team.

Peter, in fact there were some endianness problems in the code like you hinted
before. I fixed them up and the tests are now (apparently) passing. I'll clean
stuff up and see if I can send a new version of the whole series in the next
few days.


Thanks,

Daniel

> 
> Best regards,
> Ilya
> 
> [...]
Thomas Huth Oct. 21, 2024, 4:17 p.m. UTC | #12 
On 30/09/2024 13.35, Thomas Huth wrote:
> On 30/09/2024 12.58, Thomas Huth wrote:
>> On 29/09/2024 22.53, Daniel Henrique Barboza wrote:
>>>
>>>
>>> On 9/29/24 12:38 PM, Peter Maydell wrote:
>>>> On Sat, 28 Sept 2024 at 21:40, Daniel Henrique Barboza
>>>> <dbarboza@ventanamicro.com> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 9/28/24 8:34 AM, Peter Maydell wrote:
>>>>>> The assertion failure is
>>>>>> ERROR:../tests/qtest/riscv-iommu-test.c:72:test_reg_reset: assertion
>>>>>> failed (cap & RISCV_IOMMU_CAP_VERSION == 0x10): (0 == 16)
>>>>>
>>>>> The root cause is that the qtests I added aren't considering the 
>>>>> endianess of the
>>>>> host. The RISC-V IOMMU is being implemented as LE only and all regs are 
>>>>> being
>>>>> read/written in memory as LE. The qtest read/write helpers must take 
>>>>> the qtest
>>>>> endianess into account. We make this type of handling in other qtest 
>>>>> archs like
>>>>> ppc64.
>>>>>
>>>>> I have a fix for the tests but I'm unable to run the ubuntu-22.04- 
>>>>> s390x- all-system
>>>>> job to verify it, even after setting Cirrus like Thomas taught me a 
>>>>> week ago. In
>>>>> fact I have no 'ubuntu-22-*' jobs available to run.
>>>>
>>>> It's on the private s390 VM we have, so it's set up only to
>>>> be available on the main CI run (there's not enough capacity
>>>> on the machine to do any more than that). If you want to point
>>>> me at a gitlab branch I can do a quick "make check" on that
>>>> if you like.
>>>
>>> I appreciate it. This is the repo:
>>>
>>> https://gitlab.com/danielhb/qemu/-/tree/pull_fix
>>>
>>> If this is enough to fix the tests, I'll amend it in the new IOMMU version.
>>> If we still failing then I'll need to set this s390 VM.
>>>
>>> By the way, if you have any recipe/pointers to set this s390 VM to share,
>>> that would be great.
>>
>> You can also use Travis-CI for testing QEMU on a s390x host, see e.g. my 
>> runs here:
>>
>>   https://app.travis-ci.com/github/huth/qemu
> 
> ... apart from the fact, that it currently seems to be completely broken 
> again :-( ... I guess that means: Never mind, sorry for the distraction!

FWIW, it seems to be working again:

  https://app.travis-ci.com/github/huth/qemu/builds/272812225

  Thomas