Re: [PATCH] vfio/igd: Check host PCI address when probing

[Alex Williamson <alex.williamson@redhat.com>]

On Sun, 13 Apr 2025 19:30:10 +0800
Tomita Moeko <tomitamoeko@gmail.com> wrote:

> On 4/10/25 01:18, Alex Williamson wrote:
> > On Wed, 26 Mar 2025 01:22:39 +0800
> > Tomita Moeko <tomitamoeko@gmail.com> wrote:
> >   
> >> So far, all Intel VGA adapters, including discrete GPUs like A770 and
> >> B580, were treated as IGD devices. While this had no functional impact,
> >> a error about "unsupported IGD device" will be printed when passthrough
> >> Intel discrete GPUs.
> >>
> >> Since IGD devices must be at "0000:00:02.0", let's check the host PCI
> >> address when probing.
> >>
> >> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> >> ---
> >>  hw/vfio/igd.c | 23 +++++++++--------------
> >>  1 file changed, 9 insertions(+), 14 deletions(-)
> >>
> >> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
> >> index 265fffc2aa..ff250017b0 100644
> >> --- a/hw/vfio/igd.c
> >> +++ b/hw/vfio/igd.c
> >> @@ -53,6 +53,13 @@
> >>   * headless setup is desired, the OpRegion gets in the way of that.
> >>   */
> >>  
> >> +static bool vfio_is_igd(VFIOPCIDevice *vdev)
> >> +{
> >> +    return vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) &&
> >> +           vfio_is_vga(vdev) &&
> >> +           vfio_pci_host_match(&vdev->host, "0000:00:02.0");
> >> +}  
> > 
> > vfio-pci devices can also be specified via sysfsdev= rather than host=,
> > so at a minimum I think we'd need to test against vdev->vbasedev.name,
> > as other callers of vfio_pci_host_match do.  For example building a
> > local PCIHostDeviceAddress and comparing it to name.  This is also not
> > foolproof though if we start taking advantage of devices passed by fd.  
> 
> Sorry for my late reply. Yes `vfio_pci_host_match` does not work for
> sysfsdev of fd, I will drop this change.
> 
> > Could we instead rely PCIe capabilities?  A discrete GPU should
> > identify as either an endpoint or legacy endpoint and IGD should
> > identify as a root complex integrated endpoint, or maybe older versions
> > would lack the PCIe capability altogether.  
> 
> Older generations seems do not have PCIe capabilities in their config
> space, like the sandy bridge spec [1] does not mention it. I don't have
> a sandy bridge box for now to verify it :(

I have a Sandy Bridge i7-2760QM and can confirm there is no PCIe
capability:

# lspci -vvvs 02.0
00:02.0 VGA compatible controller: Intel Corporation 2nd Generation Core Processor Family Integrated Graphics Controller (rev 09) (prog-if 00 [VGA controller])
	Subsystem: Lenovo Device 21d1
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
	Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0
	Interrupt: pin A routed to IRQ 35
	Region 0: Memory at f0000000 (64-bit, non-prefetchable) [size=4M]
	Region 2: Memory at e0000000 (64-bit, prefetchable) [size=256M]
	Region 4: I/O ports at 5000 [size=64]
	Expansion ROM at 000c0000 [virtual] [disabled] [size=128K]
	Capabilities: [90] MSI: Enable+ Count=1/1 Maskable- 64bit-
		Address: fee00018  Data: 0000
	Capabilities: [d0] Power Management version 2
		Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
		Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
	Capabilities: [a4] PCI Advanced Features
		AFCap: TP+ FLR+
		AFCtrl: FLR-
		AFStatus: TP-
	Kernel driver in use: i915
	Kernel modules: i915

# lspci -xxxs 02.0
00:02.0 VGA compatible controller: Intel Corporation 2nd Generation Core Processor Family Integrated Graphics Controller (rev 09)
00: 86 80 26 01 07 04 90 00 09 00 00 03 00 00 00 00
10: 04 00 00 f0 00 00 00 00 0c 00 00 e0 00 00 00 00
20: 01 50 00 00 00 00 00 00 00 00 00 00 aa 17 d1 21
30: 00 00 00 00 90 00 00 00 00 00 00 00 0b 01 00 00
40: 09 00 0c 01 9d 21 00 e2 90 00 00 14 00 00 00 00
50: 11 02 00 00 11 00 00 00 00 00 00 00 01 00 a0 db
60: 00 00 02 00 00 00 00 00 00 00 00 00 00 00 00 00
70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
90: 05 d0 01 00 18 00 e0 fe 00 00 00 00 00 00 00 00
a0: 00 00 00 00 13 00 06 03 00 00 00 00 00 00 00 00
b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
d0: 01 a4 22 00 00 00 00 00 00 00 00 00 00 00 00 00
e0: 00 00 00 00 01 00 00 00 00 80 00 00 00 00 00 00
f0: 00 00 00 00 00 00 00 00 00 00 06 00 18 60 ef da

> > Also I think the comments that were dropped below are still valid and
> > useful to transfer to this new helper.  I think those are actually
> > referring to the guest address of 00:02.0 though, which should maybe be
> > a test as well.  Thanks,
> > 
> > Alex  
> 
> Guest address of 00:02.0 is not mandatory, newer drivers does not depend
> on the address (probably thanks to the discrete GPU, they removed these
> hardcode). Though putting it at guest 00:02.0 is still recommended.
> I would prefer not making this a limit.

I suppose it was never enforced beyond the comment, so if you vouch
that it's not mandatory, let's not make it a requirement now.  Thanks,

Alex