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.au...@redhat.com' <eric.au...@redhat.com>; qemu-
> > a...@nongnu.org; qemu-devel@nongnu.org
> > Cc: peter.mayd...@linaro.org; j...@nvidia.com; nicol...@nvidia.com;
> > ddut...@redhat.com; Linuxarm <linux...@huawei.com>; Wangzhou (B)
> > <wangzh...@hisilicon.com>; jiangkunkun <jiangkun...@huawei.com>;
> > Jonathan Cameron <jonathan.came...@huawei.com>;
> > zhangfei....@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
