Re: [PATCH] IOMMU/VT-d: Fix iommu=no-igfx if the IOMMU scope contains phantom device

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.02.2023 01:08, Marek Marczykowski-Górecki wrote:
> If the scope for IGD's IOMMU contains additional device that doesn't
> actually exist, iommu=no-igfx would not disable that IOMMU. In this
> particular case (Thinkpad x230) it included
> 00:02.1, but there is no such device on this platform.
> Consider only existing devices for "gfx only" check.
> 

Hmm, perhaps

Fixes: 2d7f191b392e ('VT-d: generalize and correct "iommu=no-igfx" handling')

?

> Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> ---
> I have looked at existence check acpi_parse_one_drhd(), but re-using
> that one wouldn't work for two reasons:
>  - gfx_only logic is very much tied to acpi_parse_dev_scope()

I think this one could be dealt with, but ...

>  - pci_device_detect() in acpi_parse_one_drhd() is skipped in case of
>    (implicit or explicit) iommu=force

... I agree this is a good reason to put the check in acpi_parse_dev_scope().

> --- a/xen/drivers/passthrough/vtd/dmar.c
> +++ b/xen/drivers/passthrough/vtd/dmar.c
> @@ -396,6 +396,7 @@ static int __init acpi_parse_dev_scope(
>                      igd_drhd_address = drhd->address;
>  
>                  if ( gfx_only &&
> +                     pci_device_detect(seg, bus, path->dev, path->fn) &&
>                       pci_conf_read8(PCI_SBDF(seg, bus, path->dev, path->fn),
>                                      PCI_CLASS_DEVICE + 1) != 0x03
>                                      /* PCI_BASE_CLASS_DISPLAY */ )

If we're adding an existence check, then maybe better in the surrounding
if(): Setting igd_drhd_address when there's not really a device at the
designated address isn't very sensible either. (In fact I think I'm going
to alter the inner part of that if() again as well.)

Jan