Hi Nathan,

> -----Original Message-----
> From: Nathan Chen <nathanc@nvidia.com>
> Sent: Wednesday, November 20, 2024 11:59 PM
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> eric.auger@redhat.com; peter.maydell@linaro.org; jgg@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; Nicolin Chen <nicolinc@nvidia.com>
> Subject: Re: [RFC PATCH 0/5] hw/arm/virt: Add support for user-creatable
> nested SMMUv3
> 
> Hi Shameer,
> 
>  >  Attempt to add the HNS VF to a different SMMUv3 will result in,
>  >
>  > -device vfio-pci,host=0000:7d:02.2,bus=pcie.port3,iommufd=iommufd0:
> Unable to attach viommu
>  > -device vfio-pci,host=0000:7d:02.2,bus=pcie.port3,iommufd=iommufd0:
> vfio 0000:7d:02.2:
>  >    Failed to set iommu_device: [iommufd=29] error attach 0000:7d:02.2
> (38) to id=11: Invalid argument
>  >
>  > At present Qemu is not doing any extra validation other than the above
>  > failure to make sure the user configuration is correct or not. The
>  > assumption is libvirt will take care of this.
> Would you be able to elaborate what Qemu is validating with this error
> message? I'm not seeing these errors when assigning a GPU's
> pcie-root-port to different PXBs (with different associated SMMU nodes).

You should see that error when you have two devices that belongs to two
different physical SMMUv3s in the host kernel, is assigned to a single
PXB/SMMUv3 for Guest.

Something like,

-device pxb-pcie,id=pcie.1,bus_nr=8,bus=pcie.0 \
-device arm-smmuv3-nested,id=smmuv1,pci-bus=pcie.1 \
-device pcie-root-port,id=pcie.port1,bus=pcie.1,chassis=1 \
-device pcie-root-port,id=pcie.port2,bus=pcie.1,chassis=1 \
-device vfio-pci,host=0000:7d:02.1,bus=pcie.port1,iommufd=iommufd0 \  --> This device belongs to phys SMMUv3_0
-device vfio-pci,host=0000:75:02.1,bus=pcie.port2,iommufd=iommufd0 \  --> This device belongs to phys SMMUv3_1

So the assumption above is that libvirt will be able to detect which devices belongs
to the same physical SMMUv3 and do the assignment for Guests correctly.

> I launched a VM using my libvirt prototype code + your qemu branch and
> noted a few small things:

Thanks for giving this a spin with libvirt.

> 1. Are there plans to support "-device addr" for arm-smmuv3-nested's
> PCIe slot and function like any other device? If not I'll exclude it
> from my libvirt prototype.

Not at the moment. arm-smmuv3-nested at the moment is not making any use
of PCI slot and  func info specifically. I am not sure how that will be useful
for this though.

> 2. Is "id" for  "-device arm-smmuv3-nested" necessary for any sort of
> functionality? If so, I'll make a change to my libvirt prototype to
> support this. I was able to boot a VM and see a similar VM PCI topology
> as your example without specifying "id".

Yes, "id" not used and without it, it will work.

> Otherwise, the VM topology looks OK with your qemu branch + my libvirt
> prototype.

That is good to know.
 
> Also as a heads up, I've added support for auto-inserting PCIe switch
> between the PXB and GPUs in libvirt to attach multiple devices to a SMMU
> node per libvirt's documentation - "If you intend to plug multiple
> devices into a pcie-expander-bus, you must connect a
> pcie-switch-upstream-port to the pcie-root-port that is plugged into the
> pcie-expander-bus, and multiple pcie-switch-downstream-ports to the
> pcie-switch-upstream-port". Future unit-tests should follow this
> topology configuration.

Ok. Could you please give me an example Qemu equivalent command option,
if possible, for the above case. I am not that familiar with libvirt and I would
also like to test the above scenario.

Thanks,
Shameer

 >> Also as a heads up, I've added support for auto-inserting PCIe switch
 >> between the PXB and GPUs in libvirt to attach multiple devices to a SMMU
 >> node per libvirt's documentation - "If you intend to plug multiple
 >> devices into a pcie-expander-bus, you must connect a
 >> pcie-switch-upstream-port to the pcie-root-port that is plugged into the
 >> pcie-expander-bus, and multiple pcie-switch-downstream-ports to the
 >> pcie-switch-upstream-port". Future unit-tests should follow this
 >> topology configuration.
 >
 > Ok. Could you please give me an example Qemu equivalent command option,
 > if possible, for the above case. I am not that familiar with libvirt 
and I would
 > also like to test the above scenario.

You can use "-device x3130-upstream" for the upstream switch port, and
"-device xio3130-downstream" for the downstream port:

  -device pxb-pcie,bus_nr=250,id=pci.1,bus=pcie.0,addr=0x1 \
  -device pcie-root-port,id=pci.2,bus=pci.1,addr=0x0 \
  -device x3130-upstream,id=pci.3,bus=pci.2,addr=0x0 \
  -device xio3130-downstream,id=pci.4,bus=pci.3,addr=0x0,chassis=17,port=1 \
  -device vfio-pci,host=0009:01:00.0,id=hostdev0,bus=pci.4,addr=0x0 \
  -device arm-smmuv3-nested,pci-bus=pci.1

-Nathan

> -----Original Message-----
> From: Nathan Chen <nathanc@nvidia.com>
> Sent: Friday, November 22, 2024 1:42 AM
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> eric.auger@redhat.com; peter.maydell@linaro.org; jgg@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; Nicolin Chen <nicolinc@nvidia.com>
> Subject: Re: [RFC PATCH 0/5] hw/arm/virt: Add support for user-creatable
> nested SMMUv3
> 
>  >> Also as a heads up, I've added support for auto-inserting PCIe switch
>  >> between the PXB and GPUs in libvirt to attach multiple devices to a
> SMMU
>  >> node per libvirt's documentation - "If you intend to plug multiple
>  >> devices into a pcie-expander-bus, you must connect a
>  >> pcie-switch-upstream-port to the pcie-root-port that is plugged into the
>  >> pcie-expander-bus, and multiple pcie-switch-downstream-ports to the
>  >> pcie-switch-upstream-port". Future unit-tests should follow this
>  >> topology configuration.
>  >
>  > Ok. Could you please give me an example Qemu equivalent command
> option,
>  > if possible, for the above case. I am not that familiar with libvirt
> and I would
>  > also like to test the above scenario.
> 
> You can use "-device x3130-upstream" for the upstream switch port, and
> "-device xio3130-downstream" for the downstream port:
> 
>   -device pxb-pcie,bus_nr=250,id=pci.1,bus=pcie.0,addr=0x1 \
>   -device pcie-root-port,id=pci.2,bus=pci.1,addr=0x0 \
>   -device x3130-upstream,id=pci.3,bus=pci.2,addr=0x0 \
>   -device xio3130-
> downstream,id=pci.4,bus=pci.3,addr=0x0,chassis=17,port=1 \
>   -device vfio-pci,host=0009:01:00.0,id=hostdev0,bus=pci.4,addr=0x0 \
>   -device arm-smmuv3-nested,pci-bus=pci.1

Thanks. Just wondering why libvirt mandates usage of pcie-switch for multiple
device plugging rather than just using pcie-root-ports?

Please let me if there is any advantage in doing so that you are aware of.

Thanks,
Shameer

 >  >> Also as a heads up, I've added support for auto-inserting PCIe switch
 >  >> between the PXB and GPUs in libvirt to attach multiple devices to a
 > SMMU
 >  >> node per libvirt's documentation - "If you intend to plug multiple
 >  >> devices into a pcie-expander-bus, you must connect a
 >  >> pcie-switch-upstream-port to the pcie-root-port that is plugged 
into the
 >  >> pcie-expander-bus, and multiple pcie-switch-downstream-ports to the
 >  >> pcie-switch-upstream-port". Future unit-tests should follow this
 >  >> topology configuration.
 >  >
 > >  > Ok. Could you please give me an example Qemu equivalent command
 > > option,
 > >  > if possible, for the above case. I am not that familiar with libvirt
 > > and I would
 > >  > also like to test the above scenario.
 > >
 > > You can use "-device x3130-upstream" for the upstream switch port, and
 > > "-device xio3130-downstream" for the downstream port:
 > >
 > >   -device pxb-pcie,bus_nr=250,id=pci.1,bus=pcie.0,addr=0x1 \
 > >   -device pcie-root-port,id=pci.2,bus=pci.1,addr=0x0 \
 > >   -device x3130-upstream,id=pci.3,bus=pci.2,addr=0x0 \
 > >   -device xio3130-
 > > downstream,id=pci.4,bus=pci.3,addr=0x0,chassis=17,port=1 \
 > >   -device vfio-pci,host=0009:01:00.0,id=hostdev0,bus=pci.4,addr=0x0 \
 > >   -device arm-smmuv3-nested,pci-bus=pci.1
 >
 > Thanks. Just wondering why libvirt mandates usage of pcie-switch for 
multiple
 > device plugging rather than just using pcie-root-ports?
 >
 > Please let me if there is any advantage in doing so that you are 
aware > of.

Actually it seems like that documentation I quoted is out of date. That 
section of the documentation for pcie-expander-bus was written before a 
patch that revised libvirt's pxb to have 32 slots instead of just 1 
slot, and it wasn't updated afterwards.

With your branch and my libvirt prototype, I was still able to attach a 
passthrough device behind a PCIe switch and see it attached to a vSMMU 
in the VM, so I'm not sure if you need to make additional changes to 
your solution to support this. But I think we should still support/test 
the case where VFIO devices are behind a switch, otherwise we're placing 
a limitation on end users who have a use case for it.

-Nathan