Re: [RFC PATCH 0/5] hw/arm/virt: Add support for user-creatable nested SMMUv3

[Daniel P . Berrangé]

On Mon, Nov 18, 2024 at 11:50:46AM +0100, Eric Auger wrote:
> Hi Shameer,
> 
> On 11/8/24 13:52, Shameer Kolothum wrote:
> > Hi,
> >
> > This series adds initial support for a user-creatable "arm-smmuv3-nested"
> > device to Qemu. At present the Qemu ARM SMMUv3 emulation is per machine
> > and cannot support multiple SMMUv3s.
> >
> > In order to support vfio-pci dev assignment with vSMMUv3, the physical
> > SMMUv3 has to be configured in nested mode. Having a pluggable
> > "arm-smmuv3-nested" device enables us to have multiple vSMMUv3 for Guests
> > running on a host with multiple physical SMMUv3s. A few benefits of doing
> > this are,
> >
> > 1. Avoid invalidation broadcast or lookup in case devices are behind
> >    multiple phys SMMUv3s.
> > 2. Makes it easy to handle phys SMMUv3s that differ in features.
> > 3. Easy to handle future requirements such as vCMDQ support.
> >
> > This is based on discussions/suggestions received for a previous RFC by
> > Nicolin here[0].
> >
> > This series includes,
> >  -Adds support for "arm-smmuv3-nested" device. At present only virt is
> >   supported and is using _plug_cb() callback to hook the sysbus mem
> >   and irq (Not sure this has any negative repercussions). Patch #3.
> >  -Provides a way to associate a pci-bus(pxb-pcie) to the above device.
> >   Patch #3.
> >  -The last patch is adding RMR support for MSI doorbell handling. Patch #5.
> >   This may change in future[1].
> >
> > This RFC is for initial discussion/test purposes only and includes patches
> > that are only relevant for adding the "arm-smmuv3-nested" support. For the
> > complete branch please find,
> > https://github.com/hisilicon/qemu/tree/private-smmuv3-nested-dev-rfc-v1
> >
> > Few ToDos to note,
> > 1. At present default-bus-bypass-iommu=on should be set when
> >    arm-smmuv3-nested dev is specified. Otherwise you may get an IORT
> >    related boot error.  Requires fixing.
> > 2. Hot adding a device is not working at the moment. Looks like pcihp irq 
> > issue.
> >    Could be a bug in IORT id mappings.
> > 3. The above branch doesn't support vSVA yet.
> >
> > Hopefully this is helpful in taking the discussion forward. Please take a
> > look and let me know.
> >
> > How to use it(Eg:):
> >
> > On a HiSilicon platform that has multiple physical SMMUv3s, the ACC ZIP VF
> > devices and HNS VF devices are behind different SMMUv3s. So for a Guest,
> > specify two smmuv3-nested devices each behind a pxb-pcie as below,
> >
> > ./qemu-system-aarch64 -machine 
> > virt,gic-version=3,default-bus-bypass-iommu=on \
> > -enable-kvm -cpu host -m 4G -smp cpus=8,maxcpus=8 \
> > -object iommufd,id=iommufd0 \
> > -bios QEMU_EFI.fd \
> > -kernel Image \
> > -device virtio-blk-device,drive=fs \
> > -drive if=none,file=rootfs.qcow2,id=fs \
> > -device pxb-pcie,id=pcie.1,bus_nr=8,bus=pcie.0 \
> > -device pcie-root-port,id=pcie.port1,bus=pcie.1,chassis=1 \
> > -device arm-smmuv3-nested,id=smmuv1,pci-bus=pcie.1 \
> > -device vfio-pci,host=0000:7d:02.1,bus=pcie.port1,iommufd=iommufd0 \
> > -device pxb-pcie,id=pcie.2,bus_nr=16,bus=pcie.0 \
> > -device pcie-root-port,id=pcie.port2,bus=pcie.2,chassis=2 \
> > -device arm-smmuv3-nested,id=smmuv2,pci-bus=pcie.2 \
> > -device vfio-pci,host=0000:75:00.1,bus=pcie.port2,iommufd=iommufd0 \
> > -append "rdinit=init console=ttyAMA0 root=/dev/vda2 rw 
> > earlycon=pl011,0x9000000" \
> > -device virtio-9p-pci,fsdev=p9fs2,mount_tag=p9,bus=pcie.0 \
> > -fsdev local,id=p9fs2,path=p9root,security_model=mapped \
> This kind of instantiation matches what I had in mind. It is
> questionable whether the legacy SMMU shouldn't be migrated to that mode
> too (instead of using a machine option setting), depending on Peter's
> feedbacks and also comments from Libvirt guys. Adding Andrea in the loop.

Yeah, looking at the current config I'm pretty surprised to see it
configured with '-machine virt,iommu=ssmuv3', where 'smmuv3' is a
type name. This effectively a back-door reinvention of the '-device'
arg.

I think it'd make more sense to deprecate the 'iommu' property
on the machine, and allow '-device ssmu3,pci-bus=pcie.0' to
associate the IOMMU with the PCI root bus, so we have consistent
approaches for all SMMU impls.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|