On Wed, Aug 27, 2025 at 06:45:42AM +0000, Duan, Zhenzhong wrote:
> Hi
> 
> >-----Original Message-----
> >From: Nicolin Chen <nicol...@nvidia.com>
> >Subject: Re: [PATCH v5 07/21] intel_iommu: Introduce a new structure
> >VTDHostIOMMUDevice
> >
> >Hi Zhenzhong/Yi,
> >
> >On Fri, Aug 22, 2025 at 02:40:45AM -0400, Zhenzhong Duan wrote:
> >> @@ -4371,6 +4374,7 @@ static bool vtd_dev_set_iommu_device(PCIBus
> >*bus, void *opaque, int devfn,
> >>                                       HostIOMMUDevice *hiod,
> >Error **errp)
> >>  {
> >>      IntelIOMMUState *s = opaque;
> >> +    VTDHostIOMMUDevice *vtd_hiod;
> >>      struct vtd_as_key key = {
> >>          .bus = bus,
> >>          .devfn = devfn,
> >
> >I wonder if the bus/devfn here would always reflect the actual BDF
> >numbers in this function, on an x86 VM.
> 
> devfn is enumerated by QEMU, see do_pci_register_device(),

Oh, thanks for the direction.

> bus number is enumerated in BIOS or kernel.
> So we can't use BDF number as key, we use PCIBus pointer + devfn
> as the key instead.

Yea, I figured that out.

> >With ARM, when the device is attached to a pxb bus, the bus/devfn
> >here are both 0, so PCI_BUILD_BDF() using these two returns 0 too.
> >
> >QEMU command for the device:
> > -device pxb-pcie,id=pcie.1,bus_nr=1,bus=pcie.0 \
> > -device arm-smmuv3,primary-bus=pcie.1,id=smmuv3.1,accel=on \
> > -device pcie-root-port,id=pcie.port1,bus=pcie.1,chassis=1,io-reserve=0 \
> > -device
> >vfio-pci-nohotplug,host=0009:01:00.0,bus=pcie.port1,rombar=0,id=dev0,iom
> >mufd=iommufd0
> >
> >QEMU log:
> >smmuv3_accel_set_iommu_device: bus=0, devfn=0, sid=0
> 
> There is only one device under pcie.port1, devfn is initialized to 0,
> bus number isn't enumerated yet during realize() so 0.

That's a pain for ARM... It needs to set BDF number early for some
use case. Shameer's current solution is doing after the guest kernel
boots, very late. So we might want to move it forward..

So, it'd be very ideal to have BDF in the set_iommu_device callback.
Otherwise, we'd have to add something like set_iommu_vdevice op to
invoke in the PCI core.

> >The set_iommu_device op is invoked by vfio_pci_realize() where the
> >the BDF number won't get ready for this kind of PCI setup until a
> >later stage that I can't identify yet..
> >
> >Given that VTD wants the BDF number too, I start to wonder whether
> >the set_iommu_device op is invoked in the right place or not..
> >
> >Maybe VTD works because it saves the bus pointer v.s. bus_num(=0),
> >so its bus_num would be updated when later code calculates the BDF
> >number using the saved bus pointer (in the key). Nonetheless, the
> >saved devfn (in the key) is 0, which wouldn't be updated later as
> >the bus_num. So, if the device is supposed to have a devfn (!=0),
> >this wouldn't work?
> 
> Both PCIBus pointer and devfn are fixed value for a QEMU instance,
> never changed.

I see. devfn wouldn't be changed. Only the bus_num will be updated
in the later stage. So, it's not a problem for Intel.

Thanks
Nicolin

Reply via email to