Re: [PATCH v3 00/13] riscv: QEMU RISC-V IOMMU Support

[Andrew Jones]

On June 10, 2024 2:34:58 AM GMT+02:00, Alistair Francis <alistair23@gmail.com> 
wrote:
>On Fri, May 24, 2024 at 3:43 AM Daniel Henrique Barboza
><dbarboza@ventanamicro.com> wrote:
>>
>> Hi,
>>
>> In this new version a lot of changes were made throughout all the code,
>> most notably on patch 3. Link for the previous version is [1].
>>
>> * How it was tested *
>>
>> This series was tested using an emulated QEMU RISC-V host booting a QEMU
>> KVM guest, passing through an emulated e1000 network card from the host
>> to the guest. I can provide more details (e.g. QEMU command lines) if
>> required, just let me know. For now this cover-letter is too much of an
>> essay as is.
>
>It would probably be helpful to document these somewhere, so others
>can use them as a starting point for running this
>

I've written up a testing procedure which I shared internally with Daniel. I'll 
sanitize it and post it somewhere public.

Thanks,
drew

>Alistair
>
>>
>> The Linux kernel used for tests can be found here:
>>
>> https://github.com/tjeznach/linux/tree/riscv_iommu_v6-rc3
>>
>> This is a newer version of the following work from Tomasz:
>>
>> https://lore.kernel.org/linux-riscv/cover.1715708679.git.tjeznach@rivosinc.com/
>> ("[PATCH v5 0/7] Linux RISC-V IOMMU Support")
>>
>> The v5 wasn't enough for the testing being done. v6-rc3 did the trick.
>>
>> Note that to test this work using riscv-iommu-pci we'll need to provide
>> the Rivos PCI ID in the command line. More details down below.
>>
>> * Highlights of this version *
>>
>> - patches removed from v2: platform driver (riscv-iommu-sys, former
>> patch 05) and the EDU changes (patches 14 and 15). The platform driver
>> will be sent later with a working example on the 'virt' machine,
>> either on a newer version of this series or via a follow-up series. We
>> already have a PoC on [2] created by Sunil. More tests are needed, so
>> it'll be left behind for now. The EDU changes will be sent in separate
>> after I finish the doc changes that Frank cited in v2.
>>
>> - patch 3 contains the bulk of changes made from v2. Please give special
>> attention to the following functions since this is entirely new code I
>> ended up adding:
>>
>>  - riscv_iommu_report_fault()
>>  - riscv_iommu_validate_device_ctx()
>>  - riscv_iommu_update_ipsr()
>>
>>   Aside from these helpers most of the changes made in this patch 3 were
>> punctual.
>>
>> - Red HAT PCI ID related changes. A new patch (4) that introduces a
>> generic RISC-V IOMMU PCI ID was added. This PCI ID was gracefully given
>> to us by Red Hat and Gerd Hoffman from their ID space. The
>> riscv-iommu-pci device now defaults to this PCI ID instead of Rivos PCI
>> ID. The device was changed slightly to allow vendor-id and device-id to
>> be set in the command-line, so it's now possible to use this reference
>> device as another RISC-V IOMMU PCI device to ease the burden of
>> testing/development.
>>
>>   To instantiate the riscv-iommu-pci device using the previous Rivos PCI
>> ID, use the following cmd line:
>>
>>   -device riscv-iommu-pci,vendor-id=0x1efd,device-id=0xedf1
>>
>>   I'm using these options to test the series with the existing Linux RISC-V
>> IOMMU support that uses just a Rivos ID to identify the device.
>>
>>
>> Series based on alistair/riscv-to-apply.next. It's also applicable on
>> current QEMU master. It can also be fetched from:
>>
>> https://gitlab.com/danielhb/qemu/-/tree/riscv_iommu_v3
>>
>>
>> Patches missing reviews/acks: 3, 5, 9, 10, 11.
>>
>> Changes from v2 [1]:
>> - patch 05 (hw/riscv: add riscv-iommu-sys platform device): dropped
>>   - will be reintroduced in a later review or as a follow-up series
>>
>> - patches 14 and 15: dropped
>>   - will be sent in separate
>>
>> - patches 2, 3, 4 and 5:
>>   - removed all 'Ziommu' references
>>
>> - patch 2:
>>   - added extra bits that patch 3 ended up using
>>
>> - patch 3:
>>   - fixed blank line at EOF in hw/riscv/trace.h
>>   - added a riscv_iommu_report_fault() helper to report faults. The helper 
>> checks if
>>     a given fault is eligible to be reported if DTF is 1
>>   - Use riscv_iommu_report_fault() in riscv_iommu_ctx() and 
>> riscv_iommu_translate()
>>     to avoid code repetition
>>   - added a riscv_iommu_validate_device_ctx() helper to validate the device 
>> context
>>     as specified in "Device configuration checks" section. This helper is 
>> being used
>>     in riscv_iommu_ctx_fetch()
>>   - added a new riscv_iommu_update_ipsr() helper to handle IPSR updates
>>     in riscv_iommu_mmio_write()
>>   - riscv_iommmu_msi_write() now reports a fault in all error paths
>>   - check for fctl.WSI before issuing a MSI interrupt in riscv_iommu_notify()
>>   - change riscv-iommu region name to 'riscv-iommu'
>>   - change address_space_init() name for PCI devices to 'name' instead of 
>> using TYPE_RISCV_IOMMU_PCI
>>   - changed riscv_iommu_mmio_ops min_access_size to 4
>>   - do not check for min and max sizes on riscv_iommu_mmio_write()
>>   - changed riscv_iommu_trap_ops  min_access_size to 4
>>   - removed IOMMU qemu_thread thread:
>>     - riscv_iommu_mmio_write() will now execute a riscv_iommu_process_fn by 
>> holding
>>       'core_lock'
>>   - init FSCR as zero explicitly
>>   - check for bus->iommu_opaque == NULL before calling pci_setup_iommu()
>>
>> - patch 4 (new):
>>   - add Red-Hat PCI RISC-V IOMMU ID
>>
>> - patch 5 (former 4):
>>   - create vendor-id and device-id properties
>>   - set Red-hat PCI RISC-V IOMMU ID as default ID
>>
>> - patch 8:
>>   - use IOMMU_NONE instead of '0' in relevant 'iot->perm = 0' instances
>>
>> - patch 9:
>>   - add s-stage and g-stage steps in riscv_iommu_validate_device_ctx()
>>   - removed 'gpa' boolean from riscv_iommu_spa_fetch()
>>   - 'en_s' is no longer used for early MSI address match
>>
>> - patch 10:
>>   - add ATS steps in riscv_iommu_validate_device_ctx()
>>   - check for 's->enable_ats' before adding RISCV_IOMMU_DC_TC_EN_ATS in 
>> device context
>>   - check for 's->enable_ats' before processing ATS commands in 
>> riscv_iommu_process_cq_tail()
>>   - remove ambiguous trace_riscv_iommu_ats() from riscv_iommu_translate()
>>
>> - patch 11:
>>   - removed unused bits
>>   - added RISCV_IOMMU_TR_REQ_CTL_NW and RISCV_IOMMU_TR_RESPONSE_S
>>     bits
>>   - set IOMMUTLBEntry 'perm' using RISCV_IOMMU_TR_REQ_CTL_NW in 
>> riscv_iommu_process_dbg()
>>   - clear RISCV_IOMMU_TR_RESPONSE_S in riscv_iommu_process_dbg(). Added a 
>> comment talking about the (lack of) superpage support
>>
>> [1] 
>> https://lore.kernel.org/qemu-riscv/20240307160319.675044-1-dbarboza@ventanamicro.com/
>> [2] https://github.com/vlsunil/qemu/commits/acpi_rimt_poc_v1/
>>
>> Andrew Jones (1):
>>   hw/riscv/riscv-iommu: Add another irq for mrif notifications
>>
>> Daniel Henrique Barboza (3):
>>   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
>>
>> 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 s-stage and g-stage support
>>   hw/riscv/riscv-iommu: add ATS support
>>   hw/riscv/riscv-iommu: add DBG support
>>
>>  docs/specs/pci-ids.rst           |    2 +
>>  hw/riscv/Kconfig                 |    4 +
>>  hw/riscv/meson.build             |    1 +
>>  hw/riscv/riscv-iommu-bits.h      |  416 ++++++
>>  hw/riscv/riscv-iommu-pci.c       |  177 +++
>>  hw/riscv/riscv-iommu.c           | 2283 ++++++++++++++++++++++++++++++
>>  hw/riscv/riscv-iommu.h           |  146 ++
>>  hw/riscv/trace-events            |   15 +
>>  hw/riscv/trace.h                 |    1 +
>>  hw/riscv/virt.c                  |   33 +-
>>  include/exec/memattrs.h          |    5 +
>>  include/hw/pci/pci.h             |    1 +
>>  include/hw/riscv/iommu.h         |   36 +
>>  meson.build                      |    1 +
>>  tests/qtest/libqos/meson.build   |    4 +
>>  tests/qtest/libqos/riscv-iommu.c |   76 +
>>  tests/qtest/libqos/riscv-iommu.h |  100 ++
>>  tests/qtest/meson.build          |    1 +
>>  tests/qtest/riscv-iommu-test.c   |  234 +++
>>  19 files changed, 3535 insertions(+), 1 deletion(-)
>>  create mode 100644 hw/riscv/riscv-iommu-bits.h
>>  create mode 100644 hw/riscv/riscv-iommu-pci.c
>>  create mode 100644 hw/riscv/riscv-iommu.c
>>  create mode 100644 hw/riscv/riscv-iommu.h
>>  create mode 100644 hw/riscv/trace-events
>>  create mode 100644 hw/riscv/trace.h
>>  create mode 100644 include/hw/riscv/iommu.h
>>  create mode 100644 tests/qtest/libqos/riscv-iommu.c
>>  create mode 100644 tests/qtest/libqos/riscv-iommu.h
>>  create mode 100644 tests/qtest/riscv-iommu-test.c
>>
>> --
>> 2.44.0
>>
>>