linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v8 0/5] Fix PF/VF dependency issue
@ 2019-09-05 19:31 Bjorn Helgaas
  2019-09-05 19:31 ` [PATCH 1/5] PCI/ATS: Handle sharing of PF PRI Capability with all VFs Bjorn Helgaas
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2019-09-05 19:31 UTC (permalink / raw)
  To: Kuppuswamy Sathyanarayanan
  Cc: Ashok Raj, Keith Busch, linux-pci, linux-kernel, Bjorn Helgaas

From: Bjorn Helgaas <bhelgaas@google.com>

The current implementation of ATS, PASID, and PRI does not handle VF
dependencies correctly.  These are essentially Kuppuswamy's patches; I
reordered and tweaked them slightly.

Please treat this as a proposal, not a done deal, and post a v9 with any
changes needed.

Changes since v7:
 * Moved PRI & PASID capability offset caching to last since they're just
   optimizations
 * Cached offsets at first use instead of adding _init() functions
 * Dropped complicated pci_prg_resp_pasid_required() #ifdefs by just
   searching for the capability again
 * Dropped EA/VF validation since it only enforces spec language without
   fixing any known issues

Changes since v6:
 * Removed locking interfaces used in v6.
 * Made necessary changes to PRI/PASID enable/disable calls to allow
   register changes only for PF.

Changes since v5:
 * Created new patches for PRI/PASID capability caching.
 * Removed individual locks (pri_lock, pasid_lock) and added common
   resource lock to synchronize PRI/PASID updates between PF/VF.
 * Addressed comments from Bjorn Helgaas.

Changes since v4:
 * Defined empty functions for pci_pri_init() and pci_pasid_init() for cases
   where CONFIG_PCI_PRI and CONFIG_PCI_PASID are not enabled.

Changes since v3:
 * Fixed critical path (lock context) in pci_restore_*_state functions.

Changes since v2:
 * Added locking mechanism to synchronize accessing PF registers in VF.
 * Removed spec compliance checks in patches.
 * Addressed comments from Bjorn Helgaas.

Changes since v1:
 * Added more details about the patches in commit log.
 * Removed bulk spec check patch.
 * Addressed comments from Bjorn Helgaas.


Link: https://lore.kernel.org/r/cover.1567029860.git.sathyanarayanan.kuppuswamy@linux.intel.com v7

Kuppuswamy Sathyanarayanan (5):
  PCI/ATS: Handle sharing of PF PRI Capability with all VFs
  PCI/ATS: Handle sharing of PF PASID Capability with all VFs
  PCI/ATS: Disable PF/VF ATS service independently
  PCI/ATS: Cache PRI Capability offset
  PCI/ATS: Cache PASID Capability offset

 drivers/pci/ats.c   | 153 +++++++++++++++++++++++++++-----------------
 include/linux/pci.h |   3 +-
 2 files changed, 96 insertions(+), 60 deletions(-)

-- 
2.23.0.187.g17f5b7556c-goog


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/5] PCI/ATS: Handle sharing of PF PRI Capability with all VFs
  2019-09-05 19:31 [PATCH v8 0/5] Fix PF/VF dependency issue Bjorn Helgaas
@ 2019-09-05 19:31 ` Bjorn Helgaas
  2019-09-05 19:31 ` [PATCH 2/5] PCI/ATS: Handle sharing of PF PASID " Bjorn Helgaas
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2019-09-05 19:31 UTC (permalink / raw)
  To: Kuppuswamy Sathyanarayanan
  Cc: Ashok Raj, Keith Busch, linux-pci, linux-kernel, Bjorn Helgaas

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

Per PCIe r5.0, sec 9.3.7.11, VFs must not implement the PRI Capability.  If
the PF implements PRI, it is shared by the VFs.  Since VFs don't have a PRI
Capability, pci_enable_pri() always failed, which caused IOMMU setup to
fail.

Update the PRI interfaces so for VFs they reflect the state of the PF PRI.

[bhelgaas: rebase without pri_cap caching, commit log]
Suggested-by: Ashok Raj <ashok.raj@intel.com>
Link: https://lore.kernel.org/r/b971e31f8695980da8e4a7f93e3b6a3edba3edaa.1567029860.git.sathyanarayanan.kuppuswamy@linux.intel.com
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Keith Busch <keith.busch@intel.com>
---
 drivers/pci/ats.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index e18499243f84..3b1c9a2305c1 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -182,6 +182,17 @@ int pci_enable_pri(struct pci_dev *pdev, u32 reqs)
 	u32 max_requests;
 	int pos;
 
+	/*
+	 * VFs must not implement the PRI Capability.  If their PF
+	 * implements PRI, it is shared by the VFs, so if the PF PRI is
+	 * enabled, it is also enabled for the VF.
+	 */
+	if (pdev->is_virtfn) {
+		if (pci_physfn(pdev)->pri_enabled)
+			return 0;
+		return -EINVAL;
+	}
+
 	if (WARN_ON(pdev->pri_enabled))
 		return -EBUSY;
 
@@ -218,6 +229,10 @@ void pci_disable_pri(struct pci_dev *pdev)
 	u16 control;
 	int pos;
 
+	/* VFs share the PF PRI */
+	if (pdev->is_virtfn)
+		return;
+
 	if (WARN_ON(!pdev->pri_enabled))
 		return;
 
@@ -243,6 +258,9 @@ void pci_restore_pri_state(struct pci_dev *pdev)
 	u32 reqs = pdev->pri_reqs_alloc;
 	int pos;
 
+	if (pdev->is_virtfn)
+		return;
+
 	if (!pdev->pri_enabled)
 		return;
 
@@ -267,6 +285,9 @@ int pci_reset_pri(struct pci_dev *pdev)
 	u16 control;
 	int pos;
 
+	if (pdev->is_virtfn)
+		return 0;
+
 	if (WARN_ON(pdev->pri_enabled))
 		return -EBUSY;
 
@@ -412,6 +433,9 @@ int pci_prg_resp_pasid_required(struct pci_dev *pdev)
 	u16 status;
 	int pos;
 
+	if (pdev->is_virtfn)
+		pdev = pci_physfn(pdev);
+
 	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI);
 	if (!pos)
 		return 0;
-- 
2.23.0.187.g17f5b7556c-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/5] PCI/ATS: Handle sharing of PF PASID Capability with all VFs
  2019-09-05 19:31 [PATCH v8 0/5] Fix PF/VF dependency issue Bjorn Helgaas
  2019-09-05 19:31 ` [PATCH 1/5] PCI/ATS: Handle sharing of PF PRI Capability with all VFs Bjorn Helgaas
@ 2019-09-05 19:31 ` Bjorn Helgaas
  2019-09-05 19:31 ` [PATCH 3/5] PCI/ATS: Disable PF/VF ATS service independently Bjorn Helgaas
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2019-09-05 19:31 UTC (permalink / raw)
  To: Kuppuswamy Sathyanarayanan
  Cc: Ashok Raj, Keith Busch, linux-pci, linux-kernel, Bjorn Helgaas

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

Per PCIe r5.0, sec 9.3.7.14, if a PF implements the PASID Capability, the
PF PASID configuration is shared by its VFs.  VFs must not implement their
own PASID Capability.  Since VFs don't have a PASID Capability,
pci_enable_pasid() always failed, which caused IOMMU setup to fail.

Update the PASID interfaces so for VFs they reflect the state of the PF
PASID.

[bhelgaas: rebase without pasid_cap caching, commit log]
Suggested-by: Ashok Raj <ashok.raj@intel.com>
Link: https://lore.kernel.org/r/8ba1ac192e4ac737508b6ac15002158e176bab91.1567029860.git.sathyanarayanan.kuppuswamy@linux.intel.com
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Keith Busch <keith.busch@intel.com>
---
 drivers/pci/ats.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index 3b1c9a2305c1..ab928f8267cf 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -318,6 +318,16 @@ int pci_enable_pasid(struct pci_dev *pdev, int features)
 	u16 control, supported;
 	int pos;
 
+	/*
+	 * VFs must not implement the PASID Capability, but if a PF
+	 * supports PASID, its VFs share the PF PASID configuration.
+	 */
+	if (pdev->is_virtfn) {
+		if (pci_physfn(pdev)->pasid_enabled)
+			return 0;
+		return -EINVAL;
+	}
+
 	if (WARN_ON(pdev->pasid_enabled))
 		return -EBUSY;
 
@@ -355,6 +365,10 @@ void pci_disable_pasid(struct pci_dev *pdev)
 	u16 control = 0;
 	int pos;
 
+	/* VFs share the PF PASID configuration */
+	if (pdev->is_virtfn)
+		return;
+
 	if (WARN_ON(!pdev->pasid_enabled))
 		return;
 
@@ -377,6 +391,9 @@ void pci_restore_pasid_state(struct pci_dev *pdev)
 	u16 control;
 	int pos;
 
+	if (pdev->is_virtfn)
+		return;
+
 	if (!pdev->pasid_enabled)
 		return;
 
@@ -404,6 +421,9 @@ int pci_pasid_features(struct pci_dev *pdev)
 	u16 supported;
 	int pos;
 
+	if (pdev->is_virtfn)
+		pdev = pci_physfn(pdev);
+
 	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PASID);
 	if (!pos)
 		return -EINVAL;
@@ -463,6 +483,9 @@ int pci_max_pasids(struct pci_dev *pdev)
 	u16 supported;
 	int pos;
 
+	if (pdev->is_virtfn)
+		pdev = pci_physfn(pdev);
+
 	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PASID);
 	if (!pos)
 		return -EINVAL;
-- 
2.23.0.187.g17f5b7556c-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/5] PCI/ATS: Disable PF/VF ATS service independently
  2019-09-05 19:31 [PATCH v8 0/5] Fix PF/VF dependency issue Bjorn Helgaas
  2019-09-05 19:31 ` [PATCH 1/5] PCI/ATS: Handle sharing of PF PRI Capability with all VFs Bjorn Helgaas
  2019-09-05 19:31 ` [PATCH 2/5] PCI/ATS: Handle sharing of PF PASID " Bjorn Helgaas
@ 2019-09-05 19:31 ` Bjorn Helgaas
  2019-09-05 19:31 ` [PATCH 4/5] PCI/ATS: Cache PRI Capability offset Bjorn Helgaas
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2019-09-05 19:31 UTC (permalink / raw)
  To: Kuppuswamy Sathyanarayanan
  Cc: Ashok Raj, Keith Busch, linux-pci, linux-kernel, Bjorn Helgaas

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

Previously we didn't disable the PF ATS until all associated VFs had
disabled it.  But per PCIe spec r5.0, sec 9.3.7.8, the ATS Capability in
VFs and associated PFs may be enabled independently.  Leaving ATS enabled
in the PF unnecessarily may have power and performance impacts.

Remove this dependency logic in the ATS enable/disable code.

[bhelgaas: commit log]
Suggested-by: Ashok Raj <ashok.raj@intel.com>
Link: https://lore.kernel.org/r/8163ab8fa66afd2cba514ae95d29ab12104781aa.1567029860.git.sathyanarayanan.kuppuswamy@linux.intel.com
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Keith Busch <keith.busch@intel.com>
---
 drivers/pci/ats.c   | 11 -----------
 include/linux/pci.h |  1 -
 2 files changed, 12 deletions(-)

diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index ab928f8267cf..920deeccf38d 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -60,8 +60,6 @@ int pci_enable_ats(struct pci_dev *dev, int ps)
 		pdev = pci_physfn(dev);
 		if (pdev->ats_stu != ps)
 			return -EINVAL;
-
-		atomic_inc(&pdev->ats_ref_cnt);  /* count enabled VFs */
 	} else {
 		dev->ats_stu = ps;
 		ctrl |= PCI_ATS_CTRL_STU(dev->ats_stu - PCI_ATS_MIN_STU);
@@ -79,20 +77,11 @@ EXPORT_SYMBOL_GPL(pci_enable_ats);
  */
 void pci_disable_ats(struct pci_dev *dev)
 {
-	struct pci_dev *pdev;
 	u16 ctrl;
 
 	if (WARN_ON(!dev->ats_enabled))
 		return;
 
-	if (atomic_read(&dev->ats_ref_cnt))
-		return;		/* VFs still enabled */
-
-	if (dev->is_virtfn) {
-		pdev = pci_physfn(dev);
-		atomic_dec(&pdev->ats_ref_cnt);
-	}
-
 	pci_read_config_word(dev, dev->ats_cap + PCI_ATS_CTRL, &ctrl);
 	ctrl &= ~PCI_ATS_CTRL_ENABLE;
 	pci_write_config_word(dev, dev->ats_cap + PCI_ATS_CTRL, ctrl);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 9e700d9f9f28..a73e8d28a896 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -452,7 +452,6 @@ struct pci_dev {
 	};
 	u16		ats_cap;	/* ATS Capability offset */
 	u8		ats_stu;	/* ATS Smallest Translation Unit */
-	atomic_t	ats_ref_cnt;	/* Number of VFs with ATS enabled */
 #endif
 #ifdef CONFIG_PCI_PRI
 	u32		pri_reqs_alloc; /* Number of PRI requests allocated */
-- 
2.23.0.187.g17f5b7556c-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 4/5] PCI/ATS: Cache PRI Capability offset
  2019-09-05 19:31 [PATCH v8 0/5] Fix PF/VF dependency issue Bjorn Helgaas
                   ` (2 preceding siblings ...)
  2019-09-05 19:31 ` [PATCH 3/5] PCI/ATS: Disable PF/VF ATS service independently Bjorn Helgaas
@ 2019-09-05 19:31 ` Bjorn Helgaas
  2019-09-05 19:31 ` [PATCH 5/5] PCI/ATS: Cache PASID " Bjorn Helgaas
  2019-09-27 10:30 ` [PATCH v8 0/5] Fix PF/VF dependency issue Bjorn Helgaas
  5 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2019-09-05 19:31 UTC (permalink / raw)
  To: Kuppuswamy Sathyanarayanan
  Cc: Ashok Raj, Keith Busch, linux-pci, linux-kernel, Bjorn Helgaas

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

Previously each PRI interface searched for the PRI Capability.  Cache the
capability offset the first time we use it instead of searching each time.

[bhelgaas: commit log, reorder patch to later, save offset directly in
pci_enable_pri() rather than adding pci_pri_init()]
Link: https://lore.kernel.org/r/0c5495d376faf6dbb8eb2165204c474438aaae65.156
7029860.git.sathyanarayanan.kuppuswamy@linux.intel.com
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/ats.c   | 52 ++++++++++++++++++++++-----------------------
 include/linux/pci.h |  1 +
 2 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index 920deeccf38d..bc463e2ecc61 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -169,7 +169,7 @@ int pci_enable_pri(struct pci_dev *pdev, u32 reqs)
 {
 	u16 control, status;
 	u32 max_requests;
-	int pos;
+	int pri = pdev->pri_cap;
 
 	/*
 	 * VFs must not implement the PRI Capability.  If their PF
@@ -185,21 +185,24 @@ int pci_enable_pri(struct pci_dev *pdev, u32 reqs)
 	if (WARN_ON(pdev->pri_enabled))
 		return -EBUSY;
 
-	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI);
-	if (!pos)
-		return -EINVAL;
+	if (!pri) {
+		pri = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI);
+		if (!pri)
+			return -EINVAL;
+		pdev->pri_cap = pri;
+	}
 
-	pci_read_config_word(pdev, pos + PCI_PRI_STATUS, &status);
+	pci_read_config_word(pdev, pri + PCI_PRI_STATUS, &status);
 	if (!(status & PCI_PRI_STATUS_STOPPED))
 		return -EBUSY;
 
-	pci_read_config_dword(pdev, pos + PCI_PRI_MAX_REQ, &max_requests);
+	pci_read_config_dword(pdev, pri + PCI_PRI_MAX_REQ, &max_requests);
 	reqs = min(max_requests, reqs);
 	pdev->pri_reqs_alloc = reqs;
-	pci_write_config_dword(pdev, pos + PCI_PRI_ALLOC_REQ, reqs);
+	pci_write_config_dword(pdev, pri + PCI_PRI_ALLOC_REQ, reqs);
 
 	control = PCI_PRI_CTRL_ENABLE;
-	pci_write_config_word(pdev, pos + PCI_PRI_CTRL, control);
+	pci_write_config_word(pdev, pri + PCI_PRI_CTRL, control);
 
 	pdev->pri_enabled = 1;
 
@@ -216,7 +219,7 @@ EXPORT_SYMBOL_GPL(pci_enable_pri);
 void pci_disable_pri(struct pci_dev *pdev)
 {
 	u16 control;
-	int pos;
+	int pri = pdev->pri_cap;
 
 	/* VFs share the PF PRI */
 	if (pdev->is_virtfn)
@@ -225,13 +228,12 @@ void pci_disable_pri(struct pci_dev *pdev)
 	if (WARN_ON(!pdev->pri_enabled))
 		return;
 
-	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI);
-	if (!pos)
+	if (!pri)
 		return;
 
-	pci_read_config_word(pdev, pos + PCI_PRI_CTRL, &control);
+	pci_read_config_word(pdev, pri + PCI_PRI_CTRL, &control);
 	control &= ~PCI_PRI_CTRL_ENABLE;
-	pci_write_config_word(pdev, pos + PCI_PRI_CTRL, control);
+	pci_write_config_word(pdev, pri + PCI_PRI_CTRL, control);
 
 	pdev->pri_enabled = 0;
 }
@@ -245,7 +247,7 @@ void pci_restore_pri_state(struct pci_dev *pdev)
 {
 	u16 control = PCI_PRI_CTRL_ENABLE;
 	u32 reqs = pdev->pri_reqs_alloc;
-	int pos;
+	int pri = pdev->pri_cap;
 
 	if (pdev->is_virtfn)
 		return;
@@ -253,12 +255,11 @@ void pci_restore_pri_state(struct pci_dev *pdev)
 	if (!pdev->pri_enabled)
 		return;
 
-	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI);
-	if (!pos)
+	if (!pri)
 		return;
 
-	pci_write_config_dword(pdev, pos + PCI_PRI_ALLOC_REQ, reqs);
-	pci_write_config_word(pdev, pos + PCI_PRI_CTRL, control);
+	pci_write_config_dword(pdev, pri + PCI_PRI_ALLOC_REQ, reqs);
+	pci_write_config_word(pdev, pri + PCI_PRI_CTRL, control);
 }
 EXPORT_SYMBOL_GPL(pci_restore_pri_state);
 
@@ -272,7 +273,7 @@ EXPORT_SYMBOL_GPL(pci_restore_pri_state);
 int pci_reset_pri(struct pci_dev *pdev)
 {
 	u16 control;
-	int pos;
+	int pri = pdev->pri_cap;
 
 	if (pdev->is_virtfn)
 		return 0;
@@ -280,12 +281,11 @@ int pci_reset_pri(struct pci_dev *pdev)
 	if (WARN_ON(pdev->pri_enabled))
 		return -EBUSY;
 
-	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI);
-	if (!pos)
+	if (!pri)
 		return -EINVAL;
 
 	control = PCI_PRI_CTRL_RESET;
-	pci_write_config_word(pdev, pos + PCI_PRI_CTRL, control);
+	pci_write_config_word(pdev, pri + PCI_PRI_CTRL, control);
 
 	return 0;
 }
@@ -440,16 +440,16 @@ EXPORT_SYMBOL_GPL(pci_pasid_features);
 int pci_prg_resp_pasid_required(struct pci_dev *pdev)
 {
 	u16 status;
-	int pos;
+	int pri;
 
 	if (pdev->is_virtfn)
 		pdev = pci_physfn(pdev);
 
-	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI);
-	if (!pos)
+	pri = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI);
+	if (!pri)
 		return 0;
 
-	pci_read_config_word(pdev, pos + PCI_PRI_STATUS, &status);
+	pci_read_config_word(pdev, pri + PCI_PRI_STATUS, &status);
 
 	if (status & PCI_PRI_STATUS_PASID)
 		return 1;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index a73e8d28a896..c81a24172b14 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -454,6 +454,7 @@ struct pci_dev {
 	u8		ats_stu;	/* ATS Smallest Translation Unit */
 #endif
 #ifdef CONFIG_PCI_PRI
+	u16		pri_cap;	/* PRI Capability offset */
 	u32		pri_reqs_alloc; /* Number of PRI requests allocated */
 #endif
 #ifdef CONFIG_PCI_PASID
-- 
2.23.0.187.g17f5b7556c-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 5/5] PCI/ATS: Cache PASID Capability offset
  2019-09-05 19:31 [PATCH v8 0/5] Fix PF/VF dependency issue Bjorn Helgaas
                   ` (3 preceding siblings ...)
  2019-09-05 19:31 ` [PATCH 4/5] PCI/ATS: Cache PRI Capability offset Bjorn Helgaas
@ 2019-09-05 19:31 ` Bjorn Helgaas
  2019-09-27 10:30 ` [PATCH v8 0/5] Fix PF/VF dependency issue Bjorn Helgaas
  5 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2019-09-05 19:31 UTC (permalink / raw)
  To: Kuppuswamy Sathyanarayanan
  Cc: Ashok Raj, Keith Busch, linux-pci, linux-kernel, Bjorn Helgaas

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

Previously each PASID interface searched for the PASID Capability.  Cache
the capability offset the first time we use it instead of searching each
time.

[bhelgaas: commit log, reorder patch to later, save offset directly in
pci_enable_pasid() rather than adding pci_pasid_init()]
Link: https://lore.kernel.org/r/4957778959fa34eab3e8b3065d1951989c61cb0f.1567029860.git.sathyanarayanan.kuppuswamy@linux.intel.com
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/ats.c   | 43 +++++++++++++++++++++----------------------
 include/linux/pci.h |  1 +
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index bc463e2ecc61..cb4f62da7b8a 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -305,7 +305,7 @@ EXPORT_SYMBOL_GPL(pci_reset_pri);
 int pci_enable_pasid(struct pci_dev *pdev, int features)
 {
 	u16 control, supported;
-	int pos;
+	int pasid = pdev->pasid_cap;
 
 	/*
 	 * VFs must not implement the PASID Capability, but if a PF
@@ -323,11 +323,14 @@ int pci_enable_pasid(struct pci_dev *pdev, int features)
 	if (!pdev->eetlp_prefix_path)
 		return -EINVAL;
 
-	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PASID);
-	if (!pos)
-		return -EINVAL;
+	if (!pasid) {
+		pasid = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PASID);
+		if (!pasid)
+			return -EINVAL;
+		pdev->pasid_cap = pasid;
+	}
 
-	pci_read_config_word(pdev, pos + PCI_PASID_CAP, &supported);
+	pci_read_config_word(pdev, pasid + PCI_PASID_CAP, &supported);
 	supported &= PCI_PASID_CAP_EXEC | PCI_PASID_CAP_PRIV;
 
 	/* User wants to enable anything unsupported? */
@@ -337,7 +340,7 @@ int pci_enable_pasid(struct pci_dev *pdev, int features)
 	control = PCI_PASID_CTRL_ENABLE | features;
 	pdev->pasid_features = features;
 
-	pci_write_config_word(pdev, pos + PCI_PASID_CTRL, control);
+	pci_write_config_word(pdev, pasid + PCI_PASID_CTRL, control);
 
 	pdev->pasid_enabled = 1;
 
@@ -352,7 +355,7 @@ EXPORT_SYMBOL_GPL(pci_enable_pasid);
 void pci_disable_pasid(struct pci_dev *pdev)
 {
 	u16 control = 0;
-	int pos;
+	int pasid = pdev->pasid_cap;
 
 	/* VFs share the PF PASID configuration */
 	if (pdev->is_virtfn)
@@ -361,11 +364,10 @@ void pci_disable_pasid(struct pci_dev *pdev)
 	if (WARN_ON(!pdev->pasid_enabled))
 		return;
 
-	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PASID);
-	if (!pos)
+	if (!pasid)
 		return;
 
-	pci_write_config_word(pdev, pos + PCI_PASID_CTRL, control);
+	pci_write_config_word(pdev, pasid + PCI_PASID_CTRL, control);
 
 	pdev->pasid_enabled = 0;
 }
@@ -378,7 +380,7 @@ EXPORT_SYMBOL_GPL(pci_disable_pasid);
 void pci_restore_pasid_state(struct pci_dev *pdev)
 {
 	u16 control;
-	int pos;
+	int pasid = pdev->pasid_cap;
 
 	if (pdev->is_virtfn)
 		return;
@@ -386,12 +388,11 @@ void pci_restore_pasid_state(struct pci_dev *pdev)
 	if (!pdev->pasid_enabled)
 		return;
 
-	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PASID);
-	if (!pos)
+	if (!pasid)
 		return;
 
 	control = PCI_PASID_CTRL_ENABLE | pdev->pasid_features;
-	pci_write_config_word(pdev, pos + PCI_PASID_CTRL, control);
+	pci_write_config_word(pdev, pasid + PCI_PASID_CTRL, control);
 }
 EXPORT_SYMBOL_GPL(pci_restore_pasid_state);
 
@@ -408,16 +409,15 @@ EXPORT_SYMBOL_GPL(pci_restore_pasid_state);
 int pci_pasid_features(struct pci_dev *pdev)
 {
 	u16 supported;
-	int pos;
+	int pasid = pdev->pasid_cap;
 
 	if (pdev->is_virtfn)
 		pdev = pci_physfn(pdev);
 
-	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PASID);
-	if (!pos)
+	if (!pasid)
 		return -EINVAL;
 
-	pci_read_config_word(pdev, pos + PCI_PASID_CAP, &supported);
+	pci_read_config_word(pdev, pasid + PCI_PASID_CAP, &supported);
 
 	supported &= PCI_PASID_CAP_EXEC | PCI_PASID_CAP_PRIV;
 
@@ -470,16 +470,15 @@ EXPORT_SYMBOL_GPL(pci_prg_resp_pasid_required);
 int pci_max_pasids(struct pci_dev *pdev)
 {
 	u16 supported;
-	int pos;
+	int pasid = pdev->pasid_cap;
 
 	if (pdev->is_virtfn)
 		pdev = pci_physfn(pdev);
 
-	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PASID);
-	if (!pos)
+	if (!pasid)
 		return -EINVAL;
 
-	pci_read_config_word(pdev, pos + PCI_PASID_CAP, &supported);
+	pci_read_config_word(pdev, pasid + PCI_PASID_CAP, &supported);
 
 	supported = (supported & PASID_NUMBER_MASK) >> PASID_NUMBER_SHIFT;
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index c81a24172b14..7ddbb6445e1a 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -458,6 +458,7 @@ struct pci_dev {
 	u32		pri_reqs_alloc; /* Number of PRI requests allocated */
 #endif
 #ifdef CONFIG_PCI_PASID
+	u16		pasid_cap;	/* PASID Capability offset */
 	u16		pasid_features;
 #endif
 #ifdef CONFIG_PCI_P2PDMA
-- 
2.23.0.187.g17f5b7556c-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v8 0/5] Fix PF/VF dependency issue
  2019-09-05 19:31 [PATCH v8 0/5] Fix PF/VF dependency issue Bjorn Helgaas
                   ` (4 preceding siblings ...)
  2019-09-05 19:31 ` [PATCH 5/5] PCI/ATS: Cache PASID " Bjorn Helgaas
@ 2019-09-27 10:30 ` Bjorn Helgaas
  5 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2019-09-27 10:30 UTC (permalink / raw)
  To: Kuppuswamy Sathyanarayanan
  Cc: Ashok Raj, Keith Busch, linux-pci, linux-kernel

On Thu, Sep 05, 2019 at 02:31:41PM -0500, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
> 
> The current implementation of ATS, PASID, and PRI does not handle VF
> dependencies correctly.  These are essentially Kuppuswamy's patches; I
> reordered and tweaked them slightly.
> 
> Please treat this as a proposal, not a done deal, and post a v9 with any
> changes needed.
> 
> Changes since v7:
>  * Moved PRI & PASID capability offset caching to last since they're just
>    optimizations
>  * Cached offsets at first use instead of adding _init() functions
>  * Dropped complicated pci_prg_resp_pasid_required() #ifdefs by just
>    searching for the capability again
>  * Dropped EA/VF validation since it only enforces spec language without
>    fixing any known issues
> 
> Changes since v6:
>  * Removed locking interfaces used in v6.
>  * Made necessary changes to PRI/PASID enable/disable calls to allow
>    register changes only for PF.
> 
> Changes since v5:
>  * Created new patches for PRI/PASID capability caching.
>  * Removed individual locks (pri_lock, pasid_lock) and added common
>    resource lock to synchronize PRI/PASID updates between PF/VF.
>  * Addressed comments from Bjorn Helgaas.
> 
> Changes since v4:
>  * Defined empty functions for pci_pri_init() and pci_pasid_init() for cases
>    where CONFIG_PCI_PRI and CONFIG_PCI_PASID are not enabled.
> 
> Changes since v3:
>  * Fixed critical path (lock context) in pci_restore_*_state functions.
> 
> Changes since v2:
>  * Added locking mechanism to synchronize accessing PF registers in VF.
>  * Removed spec compliance checks in patches.
>  * Addressed comments from Bjorn Helgaas.
> 
> Changes since v1:
>  * Added more details about the patches in commit log.
>  * Removed bulk spec check patch.
>  * Addressed comments from Bjorn Helgaas.
> 
> 
> Link: https://lore.kernel.org/r/cover.1567029860.git.sathyanarayanan.kuppuswamy@linux.intel.com v7
> 
> Kuppuswamy Sathyanarayanan (5):
>   PCI/ATS: Handle sharing of PF PRI Capability with all VFs
>   PCI/ATS: Handle sharing of PF PASID Capability with all VFs
>   PCI/ATS: Disable PF/VF ATS service independently
>   PCI/ATS: Cache PRI Capability offset
>   PCI/ATS: Cache PASID Capability offset
> 
>  drivers/pci/ats.c   | 153 +++++++++++++++++++++++++++-----------------
>  include/linux/pci.h |   3 +-
>  2 files changed, 96 insertions(+), 60 deletions(-)

Applied to pci/virtualization for v5.5.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-09-27 10:30 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-05 19:31 [PATCH v8 0/5] Fix PF/VF dependency issue Bjorn Helgaas
2019-09-05 19:31 ` [PATCH 1/5] PCI/ATS: Handle sharing of PF PRI Capability with all VFs Bjorn Helgaas
2019-09-05 19:31 ` [PATCH 2/5] PCI/ATS: Handle sharing of PF PASID " Bjorn Helgaas
2019-09-05 19:31 ` [PATCH 3/5] PCI/ATS: Disable PF/VF ATS service independently Bjorn Helgaas
2019-09-05 19:31 ` [PATCH 4/5] PCI/ATS: Cache PRI Capability offset Bjorn Helgaas
2019-09-05 19:31 ` [PATCH 5/5] PCI/ATS: Cache PASID " Bjorn Helgaas
2019-09-27 10:30 ` [PATCH v8 0/5] Fix PF/VF dependency issue Bjorn Helgaas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).