Re: [RFC PATCH 4/5] hw/arm/virt-acpi-build: Build IORT with multiple SMMU nodes

[Nicolin Chen <nicolinc@nvidia.com>]

On Thu, Nov 21, 2024 at 09:46:16AM +0000, Shameerali Kolothum Thodi wrote:
> Hi Eric,
> 
> > -----Original Message-----
> > From: Shameerali Kolothum Thodi
> > Sent: Wednesday, November 20, 2024 4:26 PM
> > To: 'eric.auger@redhat.com' <eric.auger@redhat.com>; qemu-
> > arm@nongnu.org; qemu-devel@nongnu.org
> > Cc: peter.maydell@linaro.org; jgg@nvidia.com; nicolinc@nvidia.com;
> > ddutile@redhat.com; Linuxarm <linuxarm@huawei.com>; Wangzhou (B)
> > <wangzhou1@hisilicon.com>; jiangkunkun <jiangkunkun@huawei.com>;
> > Jonathan Cameron <jonathan.cameron@huawei.com>;
> > zhangfei.gao@linaro.org
> > Subject: RE: [RFC PATCH 4/5] hw/arm/virt-acpi-build: Build IORT with
> > multiple SMMU nodes
> > 
> > > > I think I have an idea why the hot add was not working.
> > > >
> > > > When we have the PCIe topology as something like below,
> > > >
> > > > -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
> > > > pcie-root-port,id=pcie.port2,bus=pcie.1,chassis=2 \ -device
> > > > arm-smmuv3-nested,id=smmuv1,pci-bus=pcie.1 \ ...
> > > >
> > > > The current IORT generation includes the pcie-root-port dev ids also
> > > > in the SMMUv3 node idmaps.
> > > >
> > > > Hence, when Guest kernel loads, pcieport is also behind the SMMUv3.
> > > >
> > > > [    1.466670] pcieport 0000:64:00.0: Adding to iommu group 1
> > > > ...
> > > > [    1.448205] pcieport 0000:64:01.0: Adding to iommu group 2
> > >
> > > But it should be the same without multi-instantiation, no? I would
> > > have expected this as normal. Has you tested hot-plug without the
> > > series laterly? Do you have the same pb?
> > 
> > That is a good question. I will give it a try soon and update.
> 
> I tried hot add with the current SMMUv3(iommu=smmuv3) and hot add
> works when I added a virtio dev to pcie-root-port connected to a pxb-pcie.
> 
> And now I think I know(hopefully) the reason why it is not working with
> smmuv3-nested case. I think the root cause is this commit here,
> 
> (series: " cover-letter: Add HW accelerated nesting support for arm SMMUv3")

> This changes the way address space is returned for the devices.
> 
> static AddressSpace *smmu_find_add_as(PCIBus *bus, void *opaque, int devfn)
> {
>     SMMUState *s = opaque;
>     SMMUPciBus *sbus = smmu_get_sbus(s, bus);
>     SMMUDevice *sdev = smmu_get_sdev(s, sbus, bus, devfn);
> 
>     /* Return the system as if the device uses stage-2 only */
>     if (s->nested && !sdev->s1_hwpt) {
>         return &sdev->as_sysmem;
>     } else {
>         return &sdev->as;
>     }
> }
> 
> If we have entries in the SMMUv3 idmap for bus:devfn, then I think we should
> return IOMMU address space here. But the logic above returns sysmem
> address space for anything other than vfio/iommufd devices.
>
> The hot add works when I hacked the logic to return IOMMU address space
> for pcie root port devices.

That is to bypass the "if (memory_region_is_iommu(section->mr))"
in vfio_listener_region_add(), when the device gets initially
attached to the default container.

Once a device reaches to the pci_device_set_iommu_device() call,
it should be attached to an IDENTIY/bypass proxy s1_hwpt, so the
smmu_find_add_as() will return the iommu as.

So, the fact that your hack is working means the hotplug routine
is likely missing a pci_device_set_iommu_device() call, IMHO, or
probably it should do pci_device_iommu_address_space() after the
device finishes pci_device_set_iommu_device() instead..

Nicolin