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

Marek Marczykowski-Górecki posted 1 patch 1 year ago
Test gitlab-ci passed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20230314013221.124930-1-marmarek@invisiblethingslab.com
xen/drivers/passthrough/vtd/dmar.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH v2] IOMMU/VT-d: Fix iommu=no-igfx if the IOMMU scope contains phantom device
Posted by Marek Marczykowski-Górecki 1 year ago
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.

Fixes: 2d7f191b392e ("VT-d: generalize and correct "iommu=no-igfx" handling")
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
v2:
 - move existence check one level up

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()
 - pci_device_detect() in acpi_parse_one_drhd() is skipped in case of
   (implicit or explicit) iommu=force
---
 xen/drivers/passthrough/vtd/dmar.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/vtd/dmar.c
index 78c8bad1515a..a8c09c0c84f6 100644
--- a/xen/drivers/passthrough/vtd/dmar.c
+++ b/xen/drivers/passthrough/vtd/dmar.c
@@ -389,7 +389,7 @@ static int __init acpi_parse_dev_scope(
                 printk(VTDPREFIX " endpoint: %pp\n",
                        &PCI_SBDF(seg, bus, path->dev, path->fn));
 
-            if ( drhd )
+            if ( drhd && pci_device_detect(seg, bus, path->dev, path->fn) )
             {
                 if ( (seg == 0) && (bus == 0) && (path->dev == 2) &&
                      (path->fn == 0) )
-- 
2.39.2


RE: [PATCH v2] IOMMU/VT-d: Fix iommu=no-igfx if the IOMMU scope contains phantom device
Posted by Tian, Kevin 1 year ago
> From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> Sent: Tuesday, March 14, 2023 9:32 AM
> 
> 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.
> 
> Fixes: 2d7f191b392e ("VT-d: generalize and correct "iommu=no-igfx"
> handling")
> Signed-off-by: Marek Marczykowski-Górecki
> <marmarek@invisiblethingslab.com>

this should be rebased on top of Jan's patch.

Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Re: [PATCH v2] IOMMU/VT-d: Fix iommu=no-igfx if the IOMMU scope contains phantom device
Posted by Jan Beulich 1 year ago
On 14.03.2023 02:50, Tian, Kevin wrote:
>> From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>> Sent: Tuesday, March 14, 2023 9:32 AM
>>
>> 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.
>>
>> Fixes: 2d7f191b392e ("VT-d: generalize and correct "iommu=no-igfx"
>> handling")
>> Signed-off-by: Marek Marczykowski-Górecki
>> <marmarek@invisiblethingslab.com>
> 
> this should be rebased on top of Jan's patch.

Right - I guess I could take care of that while applying. But I wonder
whether the description wouldn't then want adjusting some as well. Or
wait, with the v2 change it should actually have been adjusted already,
as the igd_drhd_address determination is now (intentionally) also
affected.

Jan

> Reviewed-by: Kevin Tian <kevin.tian@intel.com>


Re: [PATCH v2] IOMMU/VT-d: Fix iommu=no-igfx if the IOMMU scope contains phantom device
Posted by Jan Beulich 1 year ago
On 14.03.2023 11:24, Jan Beulich wrote:
> On 14.03.2023 02:50, Tian, Kevin wrote:
>>> From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>>> Sent: Tuesday, March 14, 2023 9:32 AM
>>>
>>> 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.
>>>
>>> Fixes: 2d7f191b392e ("VT-d: generalize and correct "iommu=no-igfx"
>>> handling")
>>> Signed-off-by: Marek Marczykowski-Górecki
>>> <marmarek@invisiblethingslab.com>
>>
>> this should be rebased on top of Jan's patch.
> 
> Right - I guess I could take care of that while applying. But I wonder
> whether the description wouldn't then want adjusting some as well. Or
> wait, with the v2 change it should actually have been adjusted already,
> as the igd_drhd_address determination is now (intentionally) also
> affected.

Since it wasn't clear if/when a v3 would appear, I've done the adjustments
myself even if originally I didn't mean to. I've also adjusted the title a
little, first and foremost to replace "phantom device" (which has an
entirely different meaning in PCI).

Jan