* [PATCH 2/3] pci: Generalize is_vmd behavior
2017-08-07 19:57 [PATCH 1/3] MAINTAINERS: Add Jonathan Derrick as VMD maintainer Jon Derrick
@ 2017-08-07 19:57 ` Jon Derrick
2017-08-11 17:03 ` Bjorn Helgaas
2017-08-07 19:57 ` [PATCH 3/3] iommu: prevent VMD child devices from being remapping targets Jon Derrick
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Jon Derrick @ 2017-08-07 19:57 UTC (permalink / raw)
To: linux-pci, iommu
Cc: Bjorn Helgaas, David Woodhouse, Keith Busch, Scott Bauer,
David Fugate, linux-kernel, Jon Derrick
Generalize is_vmd behavior to remove dependency on domain number
checking in pci quirks.
Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
arch/x86/include/asm/pci.h | 8 +++-----
arch/x86/pci/common.c | 2 +-
drivers/pci/quirks.c | 2 +-
include/linux/pci.h | 4 ++++
4 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
index 473a729..5c5d54a 100644
--- a/arch/x86/include/asm/pci.h
+++ b/arch/x86/include/asm/pci.h
@@ -60,16 +60,14 @@ static inline void *_pci_root_bus_fwnode(struct pci_bus *bus)
#define pci_root_bus_fwnode _pci_root_bus_fwnode
#endif
-static inline bool is_vmd(struct pci_bus *bus)
-{
#if IS_ENABLED(CONFIG_VMD)
+static inline bool pci_bus_is_vmd(struct pci_bus *bus)
+{
struct pci_sysdata *sd = bus->sysdata;
return sd->vmd_domain;
-#else
- return false;
-#endif
}
+#endif
/* Can be used to override the logic in pci_scan_bus for skipping
already-configured bus numbers - to be used for buggy BIOSes
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index dbe2132..18b2277 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -662,7 +662,7 @@ static void set_dma_domain_ops(struct pci_dev *pdev) {}
static void set_dev_domain_options(struct pci_dev *pdev)
{
- if (is_vmd(pdev->bus))
+ if (pci_bus_is_vmd(pdev->bus))
pdev->hotplug_user_indicators = 1;
}
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 6967c6b..ba47995 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -4666,7 +4666,7 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x443, quirk_intel_qat_vf_cap);
static void quirk_no_aersid(struct pci_dev *pdev)
{
/* VMD Domain */
- if (pdev->bus->sysdata && pci_domain_nr(pdev->bus) >= 0x10000)
+ if (pci_bus_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);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 4869e66..0299d8b 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1471,6 +1471,10 @@ static inline int pci_proc_domain(struct pci_bus *bus) { return 0; }
static inline int pci_get_new_domain_nr(void) { return -ENOSYS; }
#endif /* CONFIG_PCI_DOMAINS */
+#if !IS_ENABLED(CONFIG_VMD)
+static inline bool pci_bus_is_vmd(struct pci_bus *bus) { return false; }
+#endif
+
/*
* Generic implementation for PCI domain support. If your
* architecture does not need custom management of PCI
--
2.9.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 2/3] pci: Generalize is_vmd behavior
2017-08-07 19:57 ` [PATCH 2/3] pci: Generalize is_vmd behavior Jon Derrick
@ 2017-08-11 17:03 ` Bjorn Helgaas
[not found] ` <20170811170319.GB17039-1RhO1Y9PlrlHTL0Zs8A6p5iNqAH0jzoTYJqu5kTmcBRl57MIdRCFDg@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Bjorn Helgaas @ 2017-08-11 17:03 UTC (permalink / raw)
To: Jon Derrick
Cc: linux-pci, iommu, David Woodhouse, Keith Busch, Scott Bauer,
David Fugate, linux-kernel
On Mon, Aug 07, 2017 at 01:57:12PM -0600, Jon Derrick wrote:
> Generalize is_vmd behavior to remove dependency on domain number
> checking in pci quirks.
>
> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> ---
> arch/x86/include/asm/pci.h | 8 +++-----
> arch/x86/pci/common.c | 2 +-
> drivers/pci/quirks.c | 2 +-
> include/linux/pci.h | 4 ++++
> 4 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
> index 473a729..5c5d54a 100644
> --- a/arch/x86/include/asm/pci.h
> +++ b/arch/x86/include/asm/pci.h
> @@ -60,16 +60,14 @@ static inline void *_pci_root_bus_fwnode(struct pci_bus *bus)
> #define pci_root_bus_fwnode _pci_root_bus_fwnode
> #endif
>
> -static inline bool is_vmd(struct pci_bus *bus)
> -{
> #if IS_ENABLED(CONFIG_VMD)
> +static inline bool pci_bus_is_vmd(struct pci_bus *bus)
> +{
> struct pci_sysdata *sd = bus->sysdata;
>
> return sd->vmd_domain;
> -#else
> - return false;
> -#endif
> }
> +#endif
>
> /* Can be used to override the logic in pci_scan_bus for skipping
> already-configured bus numbers - to be used for buggy BIOSes
> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
> index dbe2132..18b2277 100644
> --- a/arch/x86/pci/common.c
> +++ b/arch/x86/pci/common.c
> @@ -662,7 +662,7 @@ static void set_dma_domain_ops(struct pci_dev *pdev) {}
>
> static void set_dev_domain_options(struct pci_dev *pdev)
> {
> - if (is_vmd(pdev->bus))
> + if (pci_bus_is_vmd(pdev->bus))
> pdev->hotplug_user_indicators = 1;
> }
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 6967c6b..ba47995 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -4666,7 +4666,7 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x443, quirk_intel_qat_vf_cap);
> static void quirk_no_aersid(struct pci_dev *pdev)
> {
> /* VMD Domain */
> - if (pdev->bus->sysdata && pci_domain_nr(pdev->bus) >= 0x10000)
> + if (pci_bus_is_vmd(pdev->bus))
I like this part ...
> pdev->bus->bus_flags |= PCI_BUS_FLAGS_NO_AERSID;
> }
> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2030, quirk_no_aersid);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 4869e66..0299d8b 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1471,6 +1471,10 @@ static inline int pci_proc_domain(struct pci_bus *bus) { return 0; }
> static inline int pci_get_new_domain_nr(void) { return -ENOSYS; }
> #endif /* CONFIG_PCI_DOMAINS */
>
> +#if !IS_ENABLED(CONFIG_VMD)
> +static inline bool pci_bus_is_vmd(struct pci_bus *bus) { return false; }
> +#endif
But not so much this part. VMD is (AFAIK) still fundamentally an
x86-only thing, so I'd like it better if this could all be within
arch/x86. Could this be done by moving quirk_no_aersid() to
arch/x86/pci/fixup.c?
BTW, CONFIG_VMD in drivers/pci/host/Kconfig is currently "depends on
SRCU". I'm not a Kconfig expert, but that doesn't seem like an
intuitive connection. And it's the only such dependency on SRCU in
the tree -- most other places use "select SRCU", which makes more
sense to me.
> /*
> * Generic implementation for PCI domain support. If your
> * architecture does not need custom management of PCI
> --
> 2.9.4
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] iommu: prevent VMD child devices from being remapping targets
2017-08-07 19:57 [PATCH 1/3] MAINTAINERS: Add Jonathan Derrick as VMD maintainer Jon Derrick
2017-08-07 19:57 ` [PATCH 2/3] pci: Generalize is_vmd behavior Jon Derrick
@ 2017-08-07 19:57 ` Jon Derrick
2017-08-11 17:17 ` Bjorn Helgaas
[not found] ` <20170807195713.10963-3-jonathan.derrick-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2017-08-11 17:20 ` [PATCH 1/3] MAINTAINERS: Add Jonathan Derrick as VMD maintainer Bjorn Helgaas
2017-08-11 18:23 ` Keith Busch
3 siblings, 2 replies; 10+ messages in thread
From: Jon Derrick @ 2017-08-07 19:57 UTC (permalink / raw)
To: linux-pci, iommu
Cc: Bjorn Helgaas, David Woodhouse, Keith Busch, Scott Bauer,
David Fugate, linux-kernel, Jon Derrick
VMD child devices must use the VMD endpoint's ID as the DMA source.
Because of this, there needs to be a way to link the parent VMD
endpoint's DMAR domain to the VMD child devices' DMAR domain such that
attaching and detaching child devices modify the endpoint's DMAR mapping
and prevents early detaching.
This is outside the scope of VMD, so disable binding child devices to
prevent unforeseen issues. This functionality may be implemented in the
future.
This patch prevents VMD child devices from returning an IOMMU, which
prevents it from exposing iommu_group sysfs directories and subsequent
binding by userspace-access drivers such as VFIO.
Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
drivers/iommu/intel-iommu.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 687f18f..651a6cd 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -905,6 +905,11 @@ static struct intel_iommu *device_to_iommu(struct device *dev, u8 *bus, u8 *devf
* the PF instead to find the IOMMU. */
pf_pdev = pci_physfn(pdev);
dev = &pf_pdev->dev;
+
+ /* VMD child devices currently cannot be handled individually */
+ if (pci_bus_is_vmd(pdev->bus))
+ return NULL;
+
segment = pci_domain_nr(pdev->bus);
} else if (has_acpi_companion(dev))
dev = &ACPI_COMPANION(dev)->dev;
--
2.9.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 3/3] iommu: prevent VMD child devices from being remapping targets
2017-08-07 19:57 ` [PATCH 3/3] iommu: prevent VMD child devices from being remapping targets Jon Derrick
@ 2017-08-11 17:17 ` Bjorn Helgaas
[not found] ` <20170807195713.10963-3-jonathan.derrick-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
1 sibling, 0 replies; 10+ messages in thread
From: Bjorn Helgaas @ 2017-08-11 17:17 UTC (permalink / raw)
To: Jon Derrick
Cc: linux-pci, iommu, David Woodhouse, Keith Busch, Scott Bauer,
David Fugate, linux-kernel
On Mon, Aug 07, 2017 at 01:57:13PM -0600, Jon Derrick wrote:
> VMD child devices must use the VMD endpoint's ID as the DMA source.
> Because of this, there needs to be a way to link the parent VMD
> endpoint's DMAR domain to the VMD child devices' DMAR domain such that
> attaching and detaching child devices modify the endpoint's DMAR mapping
> and prevents early detaching.
>
> This is outside the scope of VMD, so disable binding child devices to
> prevent unforeseen issues. This functionality may be implemented in the
> future.
>
> This patch prevents VMD child devices from returning an IOMMU, which
> prevents it from exposing iommu_group sysfs directories and subsequent
> binding by userspace-access drivers such as VFIO.
>
> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> ---
> drivers/iommu/intel-iommu.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 687f18f..651a6cd 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -905,6 +905,11 @@ static struct intel_iommu *device_to_iommu(struct device *dev, u8 *bus, u8 *devf
> * the PF instead to find the IOMMU. */
> pf_pdev = pci_physfn(pdev);
> dev = &pf_pdev->dev;
> +
> + /* VMD child devices currently cannot be handled individually */
> + if (pci_bus_is_vmd(pdev->bus))
Aah, this is the bigger reason why you want a general "is VMD" thing.
But this is still basically x86-specific code (I do see the ia64 in
Kconfig, so I guess maybe ia64 as well).
But pci_bus_is_vmd() doesn't feel like a generally useful concept. I
see why you need things like this quirk, but it's not clear what
useful things other code could do with pci_bus_is_vmd() because we
don't have a generic concept of how VMD buses are different than other
PCI buses.
> + return NULL;
> +
> segment = pci_domain_nr(pdev->bus);
> } else if (has_acpi_companion(dev))
> dev = &ACPI_COMPANION(dev)->dev;
> --
> 2.9.4
>
^ permalink raw reply [flat|nested] 10+ messages in thread[parent not found: <20170807195713.10963-3-jonathan.derrick-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH 3/3] iommu: prevent VMD child devices from being remapping targets
[not found] ` <20170807195713.10963-3-jonathan.derrick-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
@ 2017-08-11 18:25 ` Robin Murphy
[not found] ` <fda8e2c5-11da-7189-794c-59a68273e4d8-5wv7dgnIgG8@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Robin Murphy @ 2017-08-11 18:25 UTC (permalink / raw)
To: Jon Derrick, linux-pci-u79uwXL29TY76Z2rM5mHXA,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Keith Busch, Bjorn Helgaas,
David Fugate, David Woodhouse, Scott Bauer
On 07/08/17 20:57, Jon Derrick wrote:
> VMD child devices must use the VMD endpoint's ID as the DMA source.
> Because of this, there needs to be a way to link the parent VMD
> endpoint's DMAR domain to the VMD child devices' DMAR domain such that
> attaching and detaching child devices modify the endpoint's DMAR mapping
> and prevents early detaching.
That sounds like either pci_device_group() needs modifying, or perhaps
that intel-iommu needs its own extended iommu_ops::device_group
implementation, to ensure that VMD child devices get put in the same
group as their parent - if they share requester IDs they can't feasibly
be attached to different domains anyway.
Robin.
> This is outside the scope of VMD, so disable binding child devices to
> prevent unforeseen issues. This functionality may be implemented in the
> future.
>
> This patch prevents VMD child devices from returning an IOMMU, which
> prevents it from exposing iommu_group sysfs directories and subsequent
> binding by userspace-access drivers such as VFIO.
>
> Signed-off-by: Jon Derrick <jonathan.derrick-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
> drivers/iommu/intel-iommu.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 687f18f..651a6cd 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -905,6 +905,11 @@ static struct intel_iommu *device_to_iommu(struct device *dev, u8 *bus, u8 *devf
> * the PF instead to find the IOMMU. */
> pf_pdev = pci_physfn(pdev);
> dev = &pf_pdev->dev;
> +
> + /* VMD child devices currently cannot be handled individually */
> + if (pci_bus_is_vmd(pdev->bus))
> + return NULL;
> +
> segment = pci_domain_nr(pdev->bus);
> } else if (has_acpi_companion(dev))
> dev = &ACPI_COMPANION(dev)->dev;
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] MAINTAINERS: Add Jonathan Derrick as VMD maintainer
2017-08-07 19:57 [PATCH 1/3] MAINTAINERS: Add Jonathan Derrick as VMD maintainer Jon Derrick
2017-08-07 19:57 ` [PATCH 2/3] pci: Generalize is_vmd behavior Jon Derrick
2017-08-07 19:57 ` [PATCH 3/3] iommu: prevent VMD child devices from being remapping targets Jon Derrick
@ 2017-08-11 17:20 ` Bjorn Helgaas
2017-08-11 18:23 ` Keith Busch
3 siblings, 0 replies; 10+ messages in thread
From: Bjorn Helgaas @ 2017-08-11 17:20 UTC (permalink / raw)
To: Jon Derrick
Cc: linux-pci, iommu, David Woodhouse, Keith Busch, Scott Bauer,
David Fugate, linux-kernel
On Mon, Aug 07, 2017 at 01:57:11PM -0600, Jon Derrick wrote:
> Add myself as VMD maintainer
>
> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
Keith, I'm looking for an ack from you since you're currently
listed in MAINTAINERS.
> ---
> MAINTAINERS | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f66488d..3ec39df 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10090,6 +10090,7 @@ F: drivers/pci/dwc/*imx6*
>
> PCI DRIVER FOR INTEL VOLUME MANAGEMENT DEVICE (VMD)
> M: Keith Busch <keith.busch@intel.com>
> +M: Jonathan Derrick <jonathan.derrick@intel.com>
> L: linux-pci@vger.kernel.org
> S: Supported
> F: drivers/pci/host/vmd.c
> --
> 2.9.4
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] MAINTAINERS: Add Jonathan Derrick as VMD maintainer
2017-08-07 19:57 [PATCH 1/3] MAINTAINERS: Add Jonathan Derrick as VMD maintainer Jon Derrick
` (2 preceding siblings ...)
2017-08-11 17:20 ` [PATCH 1/3] MAINTAINERS: Add Jonathan Derrick as VMD maintainer Bjorn Helgaas
@ 2017-08-11 18:23 ` Keith Busch
3 siblings, 0 replies; 10+ messages in thread
From: Keith Busch @ 2017-08-11 18:23 UTC (permalink / raw)
To: Jon Derrick
Cc: linux-pci, iommu, Bjorn Helgaas, David Woodhouse, Scott Bauer,
David Fugate, linux-kernel
On Mon, Aug 07, 2017 at 01:57:11PM -0600, Jon Derrick wrote:
> Add myself as VMD maintainer
>
> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
Thanks for adding.
Acked-by: Keith Busch <keith.busch@intel.com>
> ---
> MAINTAINERS | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f66488d..3ec39df 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10090,6 +10090,7 @@ F: drivers/pci/dwc/*imx6*
>
> PCI DRIVER FOR INTEL VOLUME MANAGEMENT DEVICE (VMD)
> M: Keith Busch <keith.busch@intel.com>
> +M: Jonathan Derrick <jonathan.derrick@intel.com>
> L: linux-pci@vger.kernel.org
> S: Supported
> F: drivers/pci/host/vmd.c
> --
> 2.9.4
>
^ permalink raw reply [flat|nested] 10+ messages in thread