Skip to contentSkip to conversation

VMD fixups

PATCH In review Jon Derrick 3 participants 9 messages Aug 17, 2017 – Aug 30, 2017

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

v2 cover VMD fixups

Mostly just cleanup in this revision, eg, trying to limit scope of vmd code to x86

Previous:
https://patchwork.kernel.org/patch/9886095/
https://patchwork.kernel.org/patch/9886097/
https://patchwork.kernel.org/patch/9886101/

Jon Derrick (4):
MAINTAINERS: Add Jonathan Derrick as VMD maintainer pci/x86: Move VMD quirks to x86 fixups
x86/PCI: Use is_vmd rather than relying on the domain number
iommu: Prevent VMD child devices from being remapping targets

 MAINTAINERS                 |  1 +
 arch/x86/pci/fixup.c        | 18 ++++++++++++++++++
 drivers/iommu/intel-iommu.c |  5 +++++
 drivers/pci/quirks.c        | 17 -----------------
 4 files changed, 24 insertions(+), 17 deletions(-)
Signature · 1 line
2.9.4
#1 lore raw

PATCH v2 1/4 MAINTAINERS: Add Jonathan Derrick as VMD maintainer

Add myself as VMD maintainer

---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)
MAINTAINERS +1 -0
index f66488d..3ec39df 100644
@@ -10090,6 +10090,7 @@ F:	drivers/pci/dwc/*imx6*
 
 PCI DRIVER FOR INTEL VOLUME MANAGEMENT DEVICE (VMD)
 M:	Keith Busch <[email protected]>
+M:	Jonathan Derrick <[email protected]>
 L:	[email protected]
 S:	Supported
 F:	drivers/pci/host/vmd.c
Signature · 1 line
2.9.4
#2 lore raw

PATCH v2 2/4 pci/x86: Move VMD quirks to x86 fixups

In reply to Jon Derrick

VMD currently only exists for Intel x86 products

---
 arch/x86/pci/fixup.c | 18 ++++++++++++++++++
 drivers/pci/quirks.c | 17 -----------------
 2 files changed, 18 insertions(+), 17 deletions(-)
arch/x86/pci/fixup.c +18 -0
index 11e4074..ca4b02e5 100644
@@ -618,3 +618,21 @@ static void quirk_apple_mbp_poweroff(struct pci_dev *pdev)
 		dev_info(dev, "can't work around MacBook Pro poweroff issue\n");
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x8c10, quirk_apple_mbp_poweroff);
+
+/*
+ * VMD-enabled root ports will change the source ID for all messages
+ * to the VMD device. Rather than doing device matching with the source
+ * ID, the AER driver should traverse the child device tree, reading
+ * AER registers to find the faulting device.
+ */
+static void quirk_no_aersid(struct pci_dev *pdev)
+{
+	/* VMD Domain */
+	if (pdev->bus->sysdata && pci_domain_nr(pdev->bus) >= 0x10000)
+		pdev->bus->bus_flags |= PCI_BUS_FLAGS_NO_AERSID;
+}
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2030, quirk_no_aersid);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2031, quirk_no_aersid);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2032, quirk_no_aersid);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2033, quirk_no_aersid);
+
drivers/pci/quirks.c +0 -17
index f1c9852..073baba 100644
@@ -4671,23 +4671,6 @@ static void quirk_intel_qat_vf_cap(struct pci_dev *pdev)
 }
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x443, quirk_intel_qat_vf_cap);
 
-/*
- * VMD-enabled root ports will change the source ID for all messages
- * to the VMD device. Rather than doing device matching with the source
- * ID, the AER driver should traverse the child device tree, reading
- * AER registers to find the faulting device.
- */
-static void quirk_no_aersid(struct pci_dev *pdev)
-{
-	/* VMD Domain */
-	if (pdev->bus->sysdata && pci_domain_nr(pdev->bus) >= 0x10000)
-		pdev->bus->bus_flags |= PCI_BUS_FLAGS_NO_AERSID;
-}
-DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2030, quirk_no_aersid);
-DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2031, quirk_no_aersid);
-DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2032, quirk_no_aersid);
-DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2033, quirk_no_aersid);
-
 /* FLR may cause some 82579 devices to hang. */
 static void quirk_intel_no_flr(struct pci_dev *dev)
 {
Signature · 1 line
2.9.4
#3 lore raw

PATCH v2 3/4 x86/PCI: Use is_vmd rather than relying on the domain number

In reply to Jon Derrick
---
 arch/x86/pci/fixup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/pci/fixup.c +1 -1
index ca4b02e5..1ed2fbf 100644
@@ -628,7 +628,7 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x8c10, quirk_apple_mbp_poweroff);
 static void quirk_no_aersid(struct pci_dev *pdev)
 {
 	/* VMD Domain */
-	if (pdev->bus->sysdata && pci_domain_nr(pdev->bus) >= 0x10000)
+	if (is_vmd(pdev->bus))
 		pdev->bus->bus_flags |= PCI_BUS_FLAGS_NO_AERSID;
 }
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2030, quirk_no_aersid);
Signature · 1 line
2.9.4
#4 lore raw

PATCH v2 4/4 iommu: Prevent VMD child devices from being remapping targets

In reply to Jon Derrick

VMD child devices must use the VMD endpoint's ID as the requester. Because of this, there needs to be a way to link the parent VMD endpoint's iommu group and associated mappings to the VMD child devices such that attaching and detaching child devices modify the endpoint's mappings, while preventing early detaching on a singular device removal or unbinding.

The reassignment of individual VMD child devices devices to VMs is outside the scope of VMD, but may be implemented in the future. For now it is best to prevent any such attempts.

This patch prevents VMD child devices from returning an IOMMU, which prevents it from exposing an iommu_group sysfs directories and allowing subsequent binding by userspace-access drivers such as VFIO.

---
 drivers/iommu/intel-iommu.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/iommu/intel-iommu.c +5 -0
index 687f18f..94353a6e 100644
@@ -901,6 +901,11 @@ static struct intel_iommu *device_to_iommu(struct device *dev, u8 *bus, u8 *devf
 		struct pci_dev *pf_pdev;
 
 		pdev = to_pci_dev(dev);
+
+		/* VMD child devices currently cannot be handled individually */
+		if (is_vmd(pdev->bus))
+			return NULL;
+
 		/* VFs aren't listed in scope tables; we need to look up
 		 * the PF instead to find the IOMMU. */
 		pf_pdev = pci_physfn(pdev);
Signature · 1 line
2.9.4
Acked-by on v2 4/4
#5 lore raw

Re: [PATCH v2 4/4] iommu: Prevent VMD child devices from being remapping targets

[+cc Robin]

This series looks fine to me as far as PCI is concerned, and I'd be happy to take it via my tree given an ack from David for this IOMMU piece. Alternatively, you can add my

to the other patches if you want to take it via another tree.

Robin raised a question about basically this same patch the first time around. Not sure whether there's still an objection there.

Quoted 39 lines · Jon Derrick

VMD child devices must use the VMD endpoint's ID as the requester. Because of this, there needs to be a way to link the parent VMD endpoint's iommu group and associated mappings to the VMD child devices such that attaching and detaching child devices modify the endpoint's mappings, while preventing early detaching on a singular device removal or unbinding.

The reassignment of individual VMD child devices devices to VMs is outside the scope of VMD, but may be implemented in the future. For now it is best to prevent any such attempts.

This patch prevents VMD child devices from returning an IOMMU, which prevents it from exposing an iommu_group sysfs directories and allowing subsequent binding by userspace-access drivers such as VFIO.

---
 drivers/iommu/intel-iommu.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/iommu/intel-iommu.c +5 -0
index 687f18f..94353a6e 100644
@@ -901,6 +901,11 @@ static struct intel_iommu *device_to_iommu(struct device *dev, u8 *bus, u8 *devf
 		struct pci_dev *pf_pdev;
 
 		pdev = to_pci_dev(dev);
+
+		/* VMD child devices currently cannot be handled individually */
+		if (is_vmd(pdev->bus))
+			return NULL;
+
 		/* VFs aren't listed in scope tables; we need to look up
 		 * the PF instead to find the IOMMU. */
 		pf_pdev = pci_physfn(pdev);
Signature · 1 line
2.9.4
reviewer
#6 lore raw

Re: [PATCH v2 4/4] iommu: Prevent VMD child devices from being remapping targets

Quoted 6 lines · Bjorn Helgaas

[+cc Robin]

This series looks fine to me as far as PCI is concerned, and I'd be happy to take it via my tree given an ack from David for this IOMMU piece. Alternatively, you can add my

Quoted 6 lines

to the other patches if you want to take it via another tree.

Robin raised a question about basically this same patch the first time around. Not sure whether there's still an objection there.

Oh, I don't actually mind the patch as-is - implementing proper IOMMU group support for VMD almost certainly should be the longer-term goal, but for all I know that may be non-trivial so in the meantime a quick and simple way to make things safe is good, however heavy-handed.

Robin.

Quoted 40 lines · Jon Derrick
Quoted 39 lines

VMD child devices must use the VMD endpoint's ID as the requester. Because of this, there needs to be a way to link the parent VMD endpoint's iommu group and associated mappings to the VMD child devices such that attaching and detaching child devices modify the endpoint's mappings, while preventing early detaching on a singular device removal or unbinding.

The reassignment of individual VMD child devices devices to VMs is outside the scope of VMD, but may be implemented in the future. For now it is best to prevent any such attempts.

This patch prevents VMD child devices from returning an IOMMU, which prevents it from exposing an iommu_group sysfs directories and allowing subsequent binding by userspace-access drivers such as VFIO.

---
 drivers/iommu/intel-iommu.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/iommu/intel-iommu.c +5 -0
index 687f18f..94353a6e 100644
@@ -901,6 +901,11 @@ static struct intel_iommu *device_to_iommu(struct device *dev, u8 *bus, u8 *devf
 		struct pci_dev *pf_pdev;
 
 		pdev = to_pci_dev(dev);
+
+		/* VMD child devices currently cannot be handled individually */
+		if (is_vmd(pdev->bus))
+			return NULL;
+
 		/* VFs aren't listed in scope tables; we need to look up
 		 * the PF instead to find the IOMMU. */
 		pf_pdev = pci_physfn(pdev);
Signature · 1 line
2.9.4
#7 lore raw

Re: [PATCH v2 4/4] iommu: Prevent VMD child devices from being remapping targets

In reply to Bjorn Helgaas
Quoted 6 lines · Bjorn Helgaas

[+cc Robin]

This series looks fine to me as far as PCI is concerned, and I'd be happy to take it via my tree given an ack from David for this IOMMU piece. Alternatively, you can add my

Quoted 6 lines

to the other patches if you want to take it via another tree.

Robin raised a question about basically this same patch the first time around. Not sure whether there's still an objection there.

Ping, David, any thoughts on this patch?

Quoted 40 lines · Jon Derrick
Quoted 39 lines

VMD child devices must use the VMD endpoint's ID as the requester. Because of this, there needs to be a way to link the parent VMD endpoint's iommu group and associated mappings to the VMD child devices such that attaching and detaching child devices modify the endpoint's mappings, while preventing early detaching on a singular device removal or unbinding.

The reassignment of individual VMD child devices devices to VMs is outside the scope of VMD, but may be implemented in the future. For now it is best to prevent any such attempts.

This patch prevents VMD child devices from returning an IOMMU, which prevents it from exposing an iommu_group sysfs directories and allowing subsequent binding by userspace-access drivers such as VFIO.

---
 drivers/iommu/intel-iommu.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/iommu/intel-iommu.c +5 -0
index 687f18f..94353a6e 100644
@@ -901,6 +901,11 @@ static struct intel_iommu *device_to_iommu(struct device *dev, u8 *bus, u8 *devf
 		struct pci_dev *pf_pdev;
 
 		pdev = to_pci_dev(dev);
+
+		/* VMD child devices currently cannot be handled individually */
+		if (is_vmd(pdev->bus))
+			return NULL;
+
 		/* VFs aren't listed in scope tables; we need to look up
 		 * the PF instead to find the IOMMU. */
 		pf_pdev = pci_physfn(pdev);
Signature · 1 line
2.9.4
#8 lore raw
In reply to Jon Derrick

[+cc Joerg]

Quoted 15 lines · Jon Derrick

Mostly just cleanup in this revision, eg, trying to limit scope of vmd code to x86

Previous:
https://patchwork.kernel.org/patch/9886095/
https://patchwork.kernel.org/patch/9886097/
https://patchwork.kernel.org/patch/9886101/

Jon Derrick (4):
MAINTAINERS: Add Jonathan Derrick as VMD maintainer pci/x86: Move VMD quirks to x86 fixups
x86/PCI: Use is_vmd rather than relying on the domain number
iommu: Prevent VMD child devices from being remapping targets

Quoted 5 lines
 MAINTAINERS                 |  1 +
 arch/x86/pci/fixup.c        | 18 ++++++++++++++++++
 drivers/iommu/intel-iommu.c |  5 +++++
 drivers/pci/quirks.c        | 17 -----------------
 4 files changed, 24 insertions(+), 17 deletions(-)

I haven't heard anything from IOMMU folks, so I applied these to pci/host-vmd for v4.14. Let me know if there's any objection.