Re: [PATCH v9 00/11] hw/arm/virt: Add support for user creatable SMMUv3 device

[Donald Dutile <ddutile@redhat.com>]

On 8/29/25 4:25 AM, Shameer Kolothum wrote:
> Hi,
> 
> Changes from v8:
> https://lore.kernel.org/qemu-devel/20250711084749.18300-1-shameerali.kolothum.thodi@huawei.com/
> 
> 1.Dropped previous patch #1 as that one is now already in.
> 2.Rebased and updated DSDT in patch #11 to make bios table tests happy.
>    The DSDT has changed since Eric's PCI hotplug series work.
> 3.Added T-by tags from Nicolin. Thanks!.
> 
> I think this is in a good shape now. Please take a look.
> 
> Thanks,
> Shameer
> 
> Changes from v7:
> https://lore.kernel.org/qemu-devel/20250708154055.101012-1-shameerali.kolothum.thodi@huawei.com/
> 
> 1. Rebased to latest target-arm.next(I have included patch#1
>     as I can't find that after a git pull of latest)
> 2. Addressed comments from Nicolin and added R-by tags. Thanks!
> 
> Changes from v6:
> https://lore.kernel.org/qemu-devel/20250703084643.85740-1-shameerali.kolothum.thodi@huawei.com/
> 
> 1. Fixed the warning case for DT support, reported by Eric(patch #1).
> 2. Picked up R-by's and T-by's. Thanks!
> 
> Changes from v5:
> https://lore.kernel.org/qemu-devel/20250623094230.76084-1-shameerali.kolothum.thodi@huawei.com/
> 
> 1. Rebased to target-arm.next and resolved conflicts with the series
>     [PATCH-for-10.1 v6 0/9] hw/arm: GIC 'its=off'.
> 2. While at it, noticed an issue with RC id mappings creation
>     and patch #1 is a fix for that.
> 3. Patches 3 and 4 have changes because of the conflict resolution with the
>     above series. I have retained the R-by tags, but encourage all to take
>     another look in case I missed anything.
> 4. Collected R-by and T-by tags. Thanks!.
> 
> Changes from v4:
> https://lore.kernel.org/qemu-devel/20250613144449.60156-1-shameerali.kolothum.thodi@huawei.com/
> 
> Major changes from v4:
> 
> 1. Added stricter validation for PCI buses associated with the SMMU.
>     Only the default PCIe Root Complex (pcie.0) and additional root
>     complexes created using pxb-pcie (see patch #1) are allowed.
> 
> 2. While testing this series with a setup involving multiple PCIe root
>     complexes using pxb-pcie, I encountered an issue related to IOMMU
>     ops resolution. Consider the below configuration, where an
>     arm-smmuv3 device is associated with the default root complex pcie.0,
>     and an additional pxb-pcie-based root complex (pcie.1) is added
>     without any associated SMMU:
> 
>     -device arm-smmuv3,primary-bus=pcie.0,id=smmuv3.1 \
>     ...
>     -device pxb-pcie,id=pcie.1,bus_nr=8,bus=pcie.0 \
>     -device pcie-root-port,id=pcie.port1,chassis=2,bus=pcie.1 \
>     -device arm-smmuv3,primary-bus=pcie.1,id=smmuv3.2 \
>     ...
>     -device virtio-net-pci,bus=pcie.0,netdev=net0,id=virtionet.0 \
>     -device virtio-net-pci,bus=pcie.port1,netdev=net1,id=virtionet.1
> 
>     The guest boots fine, and virtionet.0(behind the SMMUV3) bring up
>     is successful. However, attempting to bring up virtionet.1
>     (behind pcie.1, which lacks a connected SMMU) results in a failure:
> 
>     root@ubuntu:/# dhclient enp9s0
>     arm-smmu-v3 arm-smmu-v3.0.auto: event 0x02 received:
>     arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000090000000002
>     arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000
>     arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000
>     arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000
>     arm-smmu-v3 arm-smmu-v3.0.auto: event: C_BAD_STREAMID client: (unassigned sid) sid: 0x900 ssid: 0x0
>     virtio_net virtio1 enp9s0: NETDEV WATCHDOG: CPU: 2: transmit queue 0 timed out 5172 ms
>     virtio_net virtio1 enp9s0: TX timeout on queue: 0, sq: output.0, vq: 0x1, name: output.0, 5172000 usecs ago
>     ...
> 
>     Debug shows that QEMU currently registers IOMMU ops for bus using
>     pci_setup_iommu(). However, when retrieving IOMMU ops for a device
>     via pci_device_get_iommu_bus_devfn(), the function walks up to the
>     parent_dev and fetches the IOMMU ops from the parent, even if the
>     current root bus has none configured.
> 
>     This works today because existing IOMMU models in QEMU are globally
>     scoped, and pxb-pcie based extra root complexes can use the
>     bypass_iommu property to skip translation as needed.
> 
>     However, with this series introducing support for associating
>     arm-smmuv3 devices with specific PCIe root complexes, this
>     becomes problematic. In QEMU, pxb-pcie is implemented as a synthetic
>     root complex whose parent_dev is pcie.0. As a result, even though
>     pcie.1 has no SMMU attached, pci_device_get_iommu_bus_devfn()
>     incorrectly returns the IOMMU ops associated with pcie.0 due to
>     the fallback mechanism via parent_dev. This causes devices on
>     pcie.1 to erroneously use the address space from pcie.0's SMMU,
>     leading to failures like the one above.
> 
>     To address this, patch #6 in the series introduces a new helper
>     function pci_setup_iommu_per_bus(), which explicitly sets the
>     iommu_per_bus field in the PCIBus structure. This allows
>     pci_device_get_iommu_bus_devfn() to retrieve IOMMU ops based
>     on the specific bus.
> 
>     Not sure this is the correct approach or not. If there is a better
>     way to handle this, please let me know .
> 
> 3. Picked up few R-by tags where the patch content has not changed much.
> 
> 4. Dropped T-by from Nathan for some patches as things have changed a bit.
>     @Nathan, apprecaite if you have time to rerun the tests.
> 
> 5. Added a bios table tests for both legacy SMMUv3 and new SMMMv3 devices.
>     See last few patches.
> 
> Cover letter:
> 
> This patch series introduces support for a user-creatable SMMUv3 device
> (-device arm-smmuv3) in QEMU.
> 
> The implementation is based on feedback received from the RFCv2[0]:
> "hw/arm/virt: Add support for user-creatable accelerated SMMUv3"
> 
> Currently, QEMU's SMMUv3 emulation (iommu=smmuv3) is tied to the machine
> and does not support instantiating multiple SMMUv3 devices—each associated
> with a separate PCIe root complex. In contrast, real-world ARM systems
> often include multiple SMMUv3 instances, each bound to a different PCIe
> root complex.
> 
> This series allows to specify multiple SMMUv3 instances as below,
> 
>   -device arm-smmuv3,primary-bus=pcie.0,id=smmuv3.0
>    ...
>   -device arm-smmuv3,primary-bus=pcie.1,,id=smmuv3.1
> 
> The multiple SMMUv3 instance support lays the groundwork for supporting
> accelerated SMMUv3, as proposed in the aforementioned RFCv2[0]. The
> proposed accelerated support will be an optional property like below,
> -device arm-smmuv3,primary-bus=pcie.1,accel=on,..
> 
> Please note, the accelerated SMMUv3 support is not part of this series
> and will be sent out as a separate series later on top of this one.
> 
> This series also,
> 
> -Supports either the legacy iommu=smmuv3 option or the new
>    "-device arm-smmuv3" model.
>    -Adds device tree bindings for the new SMMUv3 device on the arm/virt
>     machine only, and only for the default pcie.0 root complex.
>     (Note: pxb-pcie root complexes are currently not supported with the
>      device tree due to known limitations[1].)
> 
> Example usage:
>    -device arm-smmuv3,primary-bus=pcie.0,id=smmuv3.0
>    -device virtio-net-pci,bus=pcie.0
>    -device pxb-pcie,id=pcie.1,bus_nr=2
>    -device arm-smmuv3,primary-bus=pcie.1,id=smmuv3.1
>    -device pcie-root-port,id=pcie.port1,bus=pcie.1
>    -device virtio-net-pci,bus=pcie.port1
> 
> Please take a look and let me know your feedback.
> 
> Thanks,
> Shameer
> [0]:https://lore.kernel.org/qemu-devel/20250311141045.66620-1-shameerali.kolothum.thodi@huawei.com/
> [1]:https://lore.kernel.org/qemu-devel/20230421165037.2506-1-Jonathan.Cameron@huawei.com/
> 
> Nicolin Chen (1):
>    hw/arm/virt: Add an SMMU_IO_LEN macro
> 
> Shameer Kolothum (10):
>    hw/arm/smmu-common: Check SMMU has PCIe Root Complex association
>    hw/arm/virt-acpi-build: Re-arrange SMMUv3 IORT build
>    hw/arm/virt-acpi-build: Update IORT for multiple smmuv3 devices
>    hw/arm/virt: Factor out common SMMUV3 dt bindings code
>    hw/pci: Introduce pci_setup_iommu_per_bus() for per-bus IOMMU ops
>      retrieval
>    hw/arm/virt: Allow user-creatable SMMUv3 dev instantiation
>    qemu-options.hx: Document the arm-smmuv3 device
>    bios-tables-test: Allow for smmuv3 test data.
>    qtest/bios-tables-test: Add tests for legacy smmuv3 and smmuv3 device
>    qtest/bios-tables-test: Update tables for smmuv3 tests
> 
>   hw/arm/smmu-common.c                          |  37 +++-
>   hw/arm/smmuv3.c                               |   2 +
>   hw/arm/virt-acpi-build.c                      | 201 ++++++++++++++----
>   hw/arm/virt.c                                 | 111 +++++++---
>   hw/core/sysbus-fdt.c                          |   3 +
>   hw/pci-bridge/pci_expander_bridge.c           |   1 -
>   hw/pci/pci.c                                  |  31 +++
>   include/hw/arm/smmu-common.h                  |   1 +
>   include/hw/arm/virt.h                         |   1 +
>   include/hw/pci/pci.h                          |   2 +
>   include/hw/pci/pci_bridge.h                   |   1 +
>   include/hw/pci/pci_bus.h                      |   1 +
>   qemu-options.hx                               |   7 +
>   tests/data/acpi/aarch64/virt/DSDT.smmuv3-dev  | Bin 0 -> 10230 bytes
>   .../data/acpi/aarch64/virt/DSDT.smmuv3-legacy | Bin 0 -> 10230 bytes
>   tests/data/acpi/aarch64/virt/IORT.smmuv3-dev  | Bin 0 -> 364 bytes
>   .../data/acpi/aarch64/virt/IORT.smmuv3-legacy | Bin 0 -> 276 bytes
>   tests/qtest/bios-tables-test.c                |  86 ++++++++
>   18 files changed, 410 insertions(+), 75 deletions(-)
>   create mode 100644 tests/data/acpi/aarch64/virt/DSDT.smmuv3-dev
>   create mode 100644 tests/data/acpi/aarch64/virt/DSDT.smmuv3-legacy
>   create mode 100644 tests/data/acpi/aarch64/virt/IORT.smmuv3-dev
>   create mode 100644 tests/data/acpi/aarch64/virt/IORT.smmuv3-legacy
> 
Apologies for delayed response; lots to catch up on after extended PTO.

Final cleanup from v8 looks good, and +1 on Nicolin's testing!

for series:
Reviewed-by: Donald Dutile <ddutile@redhat.com>