linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] PCI: Enable LTR/OBFF before device is used by driver
@ 2012-06-08  8:01 Xudong Hao
  2012-06-08  8:01 ` [PATCH 1/4] Add pci_obff_supported() function Xudong Hao
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Xudong Hao @ 2012-06-08  8:01 UTC (permalink / raw)
  To: bhelgaas
  Cc: linux-pci, linux-kernel, kvm, avi, alex.williamson, ddutile,
	myron.stowe, xiantao.zhang, xudong.hao

The series of patches enable LTR and OBFF before device is used by driver, and
introduce a couple of functions to save/restore LTR latency value.

Patch 1/4 introduce new function pci_obff_supported() as pci_ltr_support().

Patch 2/4 enable LTR(Latency tolerance reporting) before device is used by
driver.

Patch 3/4 enable OBFF(optimized buffer flush/fill) before device is used by
driver.

Patch 4/4 introduce a couple of functions pci_save_ltr_value() and
pci_restore_ltr_value() to save and restore LTR latency value, while device is
reset.

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

* [PATCH 1/4] Add pci_obff_supported() function.
  2012-06-08  8:01 [PATCH 0/4] PCI: Enable LTR/OBFF before device is used by driver Xudong Hao
@ 2012-06-08  8:01 ` Xudong Hao
  2012-06-08  8:01 ` [PATCH 2/4] Enable LTR before device is used by driver Xudong Hao
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Xudong Hao @ 2012-06-08  8:01 UTC (permalink / raw)
  To: bhelgaas
  Cc: linux-pci, linux-kernel, kvm, avi, alex.williamson, ddutile,
	myron.stowe, xiantao.zhang, xudong.hao

Add pci_obff_supported() function and use it by obff relative function.

Signed-off-by: Xudong Hao <xudong.hao@intel.com>
---
 drivers/pci/pci.c |   30 ++++++++++++++++++++++++------
 1 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 447e834..a70093b 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -39,6 +39,7 @@ EXPORT_SYMBOL(pci_pci_problems);
 
 unsigned int pci_pm_d3_delay;
 
+static bool pci_obff_supported(struct pci_dev *dev);
 static void pci_pme_list_scan(struct work_struct *work);
 
 static LIST_HEAD(pci_pme_list);
@@ -2071,6 +2072,27 @@ void pci_disable_ido(struct pci_dev *dev, unsigned long type)
 EXPORT_SYMBOL(pci_disable_ido);
 
 /**
+ * pci_obff_supported - check whether a device supports OBFF 
+ * @dev: PCI device
+ *
+ * RETURNS:
+ * True if @dev supports optimized buffer flush/fill, false otherwise.
+ */
+static bool pci_obff_supported(struct pci_dev *dev)
+{
+	int pos;
+	u32 cap;
+
+	pos = pci_pcie_cap(dev);
+	if (!pos)
+		return false;
+
+	pci_read_config_dword(dev, pos + PCI_EXP_DEVCAP2, &cap);
+
+	return cap & PCI_EXP_OBFF_MASK;
+}
+
+/**
  * pci_enable_obff - enable optimized buffer flush/fill
  * @dev: PCI device
  * @type: type of signaling to use
@@ -2096,17 +2118,13 @@ int pci_enable_obff(struct pci_dev *dev, enum pci_obff_signal_type type)
 	u16 ctrl;
 	int ret;
 
-	if (!pci_is_pcie(dev))
+	if (!pci_obff_supported(dev))
 		return -ENOTSUPP;
 
 	pos = pci_pcie_cap(dev);
 	if (!pos)
 		return -ENOTSUPP;
 
-	pci_read_config_dword(dev, pos + PCI_EXP_DEVCAP2, &cap);
-	if (!(cap & PCI_EXP_OBFF_MASK))
-		return -ENOTSUPP; /* no OBFF support at all */
-
 	/* Make sure the topology supports OBFF as well */
 	if (dev->bus) {
 		ret = pci_enable_obff(dev->bus->self, type);
@@ -2149,7 +2167,7 @@ void pci_disable_obff(struct pci_dev *dev)
 	int pos;
 	u16 ctrl;
 
-	if (!pci_is_pcie(dev))
+	if (!pci_obff_supported(dev))
 		return;
 
 	pos = pci_pcie_cap(dev);
-- 
1.5.5


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

* [PATCH 2/4] Enable LTR before device is used by driver
  2012-06-08  8:01 [PATCH 0/4] PCI: Enable LTR/OBFF before device is used by driver Xudong Hao
  2012-06-08  8:01 ` [PATCH 1/4] Add pci_obff_supported() function Xudong Hao
@ 2012-06-08  8:01 ` Xudong Hao
  2012-06-08  8:01 ` [PATCH 3/4] Enable OBFF " Xudong Hao
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Xudong Hao @ 2012-06-08  8:01 UTC (permalink / raw)
  To: bhelgaas
  Cc: linux-pci, linux-kernel, kvm, avi, alex.williamson, ddutile,
	myron.stowe, xiantao.zhang, xudong.hao

Enable LTR(Latency tolerance reporting) in pci_enable_device(),
make sure this feature is enabled before the device is used by driver.

Signed-off-by: Xudong Hao <xudong.hao@intel.com>
---
 drivers/pci/pci.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index a70093b..5e5e3ad 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1136,6 +1136,21 @@ int pci_load_and_free_saved_state(struct pci_dev *dev,
 }
 EXPORT_SYMBOL_GPL(pci_load_and_free_saved_state);
 
+static void pci_enable_dev_caps(struct pci_dev *dev)
+{
+	/* 
+	 * LTR(Latency tolerance reporting) allows devices to send
+	 * messages to the root complex indicating their latency
+	 * tolerance for snooped & unsnooped memory transactions.
+	 */
+	pci_enable_ltr(dev);
+}
+
+static void pci_disable_dev_caps(struct pci_dev *dev)
+{
+	pci_disable_ltr(dev);
+}
+
 static int do_pci_enable_device(struct pci_dev *dev, int bars)
 {
 	int err;
@@ -1148,6 +1163,9 @@ static int do_pci_enable_device(struct pci_dev *dev, int bars)
 		return err;
 	pci_fixup_device(pci_fixup_enable, dev);
 
+	/* Enable some device capibility before it's used by driver. */
+	pci_enable_dev_caps(dev);
+
 	return 0;
 }
 
@@ -1363,6 +1381,7 @@ static void do_pci_disable_device(struct pci_dev *dev)
 	}
 
 	pcibios_disable_device(dev);
+	pci_disable_dev_caps(dev);
 }
 
 /**
-- 
1.5.5


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

* [PATCH 3/4] Enable OBFF before device is used by driver
  2012-06-08  8:01 [PATCH 0/4] PCI: Enable LTR/OBFF before device is used by driver Xudong Hao
  2012-06-08  8:01 ` [PATCH 1/4] Add pci_obff_supported() function Xudong Hao
  2012-06-08  8:01 ` [PATCH 2/4] Enable LTR before device is used by driver Xudong Hao
@ 2012-06-08  8:01 ` Xudong Hao
  2012-06-08  8:01 ` [PATCH 4/4] PCI: save/restore max Latency Value for device LTR Xudong Hao
  2012-06-08 17:31 ` [PATCH 0/4] PCI: Enable LTR/OBFF before device is used by driver Bjorn Helgaas
  4 siblings, 0 replies; 9+ messages in thread
From: Xudong Hao @ 2012-06-08  8:01 UTC (permalink / raw)
  To: bhelgaas
  Cc: linux-pci, linux-kernel, kvm, avi, alex.williamson, ddutile,
	myron.stowe, xiantao.zhang, xudong.hao

Enable OBFF(optimized buffer flush/fill) in pci_enable_device() function, make
sure this feature is enabled before device is used by driver.

Signed-off-by: Xudong Hao <xudong.hao@intel.com>
---
 drivers/pci/pci.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 5e5e3ad..35127c1 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1144,11 +1144,20 @@ static void pci_enable_dev_caps(struct pci_dev *dev)
 	 * tolerance for snooped & unsnooped memory transactions.
 	 */
 	pci_enable_ltr(dev);
+
+	/* 
+	 * OBFF (optimized buffer flush/fill), where supported,
+	 * can help improve energy efficiency by giving devices
+	 * information about when interrupts and other activity
+	 * will have a reduced power impact.
+	 */
+	pci_enable_obff(dev, PCI_EXP_OBFF_SIGNAL_L0);
 }
 
 static void pci_disable_dev_caps(struct pci_dev *dev)
 {
 	pci_disable_ltr(dev);
+	pci_disable_obff(dev);
 }
 
 static int do_pci_enable_device(struct pci_dev *dev, int bars)
-- 
1.5.5


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

* [PATCH 4/4] PCI: save/restore max Latency Value for device LTR
  2012-06-08  8:01 [PATCH 0/4] PCI: Enable LTR/OBFF before device is used by driver Xudong Hao
                   ` (2 preceding siblings ...)
  2012-06-08  8:01 ` [PATCH 3/4] Enable OBFF " Xudong Hao
@ 2012-06-08  8:01 ` Xudong Hao
  2012-06-12 17:07   ` Bjorn Helgaas
  2012-06-08 17:31 ` [PATCH 0/4] PCI: Enable LTR/OBFF before device is used by driver Bjorn Helgaas
  4 siblings, 1 reply; 9+ messages in thread
From: Xudong Hao @ 2012-06-08  8:01 UTC (permalink / raw)
  To: bhelgaas
  Cc: linux-pci, linux-kernel, kvm, avi, alex.williamson, ddutile,
	myron.stowe, xiantao.zhang, xudong.hao

LTR: Save Max snoop/no-snoop Latency Value in pci_save_pcie_state, and restore
them in pci_restore_pcie_state.

Signed-off-by: Xudong Hao <xudong.hao@intel.com>
---
 drivers/pci/pci.c   |   71 ++++++++++++++++++++++++++++++++++++++++++++++++---
 drivers/pci/probe.c |    5 +++-
 2 files changed, 71 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 35127c1..7172f15 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -912,6 +912,54 @@ static void pci_restore_pcie_state(struct pci_dev *dev)
 		pci_write_config_word(dev, pos + PCI_EXP_SLTCTL2, cap[i++]);
 }
 
+#define PCI_LTR_SAVE_REGS	2
+
+static int pci_save_ltr_value(struct pci_dev *dev)
+{
+	int i = 0, pos;
+	struct pci_cap_saved_state *save_state;
+	u16 *cap;
+	u16 ctrl;
+
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_LTR);
+	if (!pos)
+		return -ENOTSUPP;
+
+	/* Check if LTR is eanbled before save value */
+	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL2, &ctrl);
+	if (!(ctrl & PCI_EXP_LTR_EN))
+		return -ENOTSUPP;
+
+	save_state = pci_find_saved_cap(dev, PCI_EXT_CAP_ID_LTR);
+	if (!save_state) {
+		dev_err(&dev->dev, "buffer not found in %s\n", __func__);
+		return -ENOMEM;
+	}
+	cap = (u16 *)&save_state->cap.data[0];
+
+	pci_read_config_word(dev, pos + PCI_LTR_MAX_SNOOP_LAT, &cap[i++]);
+	pci_read_config_word(dev, pos + PCI_LTR_MAX_NOSNOOP_LAT, &cap[i++]);
+	return 0;
+}
+
+static void pci_restore_ltr_value(struct pci_dev *dev)
+{
+	int i = 0, pos;
+	struct pci_cap_saved_state *save_state;
+	u16 *cap;
+
+	if (!pci_ltr_supported(dev))
+		return;
+
+	save_state = pci_find_saved_cap(dev, PCI_EXT_CAP_ID_LTR);
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_LTR);
+	if (!save_state || !pos)
+		return;
+	cap = (u16 *)&save_state->cap.data[0];
+
+	pci_write_config_word(dev, pos + PCI_LTR_MAX_SNOOP_LAT, cap[i++]);
+	pci_write_config_word(dev, pos + PCI_LTR_MAX_NOSNOOP_LAT, cap[i++]);
+}
 
 static int pci_save_pcix_state(struct pci_dev *dev)
 {
@@ -966,6 +1014,11 @@ pci_save_state(struct pci_dev *dev)
 		return i;
 	if ((i = pci_save_pcix_state(dev)) != 0)
 		return i;
+
+	if (pci_ltr_supported(dev))
+		if ((i = pci_save_ltr_value(dev)) != 0)
+			return i;
+
 	return 0;
 }
 
@@ -1034,6 +1087,7 @@ void pci_restore_state(struct pci_dev *dev)
 	pci_restore_pcix_state(dev);
 	pci_restore_msi_state(dev);
 	pci_restore_iov_state(dev);
+	pci_restore_ltr_value(dev);
 
 	dev->state_saved = false;
 }
@@ -1955,12 +2009,15 @@ static void pci_add_saved_cap(struct pci_dev *pci_dev,
  * @size: requested size of the buffer
  */
 static int pci_add_cap_save_buffer(
-	struct pci_dev *dev, char cap, unsigned int size)
+	struct pci_dev *dev, char cap, unsigned int size, bool is_extcap)
 {
 	int pos;
 	struct pci_cap_saved_state *save_state;
 
-	pos = pci_find_capability(dev, cap);
+	if (is_extcap)
+		pos = pci_find_ext_capability(dev, cap);
+	else
+		pos = pci_find_capability(dev, cap);
 	if (pos <= 0)
 		return 0;
 
@@ -1984,15 +2041,21 @@ void pci_allocate_cap_save_buffers(struct pci_dev *dev)
 	int error;
 
 	error = pci_add_cap_save_buffer(dev, PCI_CAP_ID_EXP,
-					PCI_EXP_SAVE_REGS * sizeof(u16));
+					PCI_EXP_SAVE_REGS * sizeof(u16), 0);
 	if (error)
 		dev_err(&dev->dev,
 			"unable to preallocate PCI Express save buffer\n");
 
-	error = pci_add_cap_save_buffer(dev, PCI_CAP_ID_PCIX, sizeof(u16));
+	error = pci_add_cap_save_buffer(dev, PCI_CAP_ID_PCIX, sizeof(u16), 0);
 	if (error)
 		dev_err(&dev->dev,
 			"unable to preallocate PCI-X save buffer\n");
+
+	error = pci_add_cap_save_buffer(dev, PCI_EXT_CAP_ID_LTR,
+					PCI_LTR_SAVE_REGS * sizeof(u16), 1);
+	if (error)
+		dev_err(&dev->dev,
+			"unable to preallocate LTR save buffer\n");
 }
 
 void pci_free_cap_save_buffers(struct pci_dev *dev)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 658ac97..a6eb509 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1232,7 +1232,10 @@ static void pci_init_capabilities(struct pci_dev *dev)
 	/* MSI/MSI-X list */
 	pci_msi_init_pci_dev(dev);
 
-	/* Buffers for saving PCIe and PCI-X capabilities */
+	/* 
+	 * Buffers for saving PCIe and PCI-X capabilities
+	 * and for saving LTR extented capabilities.
+	 */
 	pci_allocate_cap_save_buffers(dev);
 
 	/* Power Management */
-- 
1.5.5


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

* Re: [PATCH 0/4] PCI: Enable LTR/OBFF before device is used by driver
  2012-06-08  8:01 [PATCH 0/4] PCI: Enable LTR/OBFF before device is used by driver Xudong Hao
                   ` (3 preceding siblings ...)
  2012-06-08  8:01 ` [PATCH 4/4] PCI: save/restore max Latency Value for device LTR Xudong Hao
@ 2012-06-08 17:31 ` Bjorn Helgaas
  2012-06-08 18:02   ` Myron Stowe
  4 siblings, 1 reply; 9+ messages in thread
From: Bjorn Helgaas @ 2012-06-08 17:31 UTC (permalink / raw)
  To: Xudong Hao
  Cc: linux-pci, linux-kernel, kvm, avi, alex.williamson, ddutile,
	myron.stowe, xiantao.zhang

On Fri, Jun 8, 2012 at 1:01 AM, Xudong Hao <xudong.hao@intel.com> wrote:
> The series of patches enable LTR and OBFF before device is used by driver, and
> introduce a couple of functions to save/restore LTR latency value.
>
> Patch 1/4 introduce new function pci_obff_supported() as pci_ltr_support().
>
> Patch 2/4 enable LTR(Latency tolerance reporting) before device is used by
> driver.
>
> Patch 3/4 enable OBFF(optimized buffer flush/fill) before device is used by
> driver.
>
> Patch 4/4 introduce a couple of functions pci_save_ltr_value() and
> pci_restore_ltr_value() to save and restore LTR latency value, while device is
> reset.

We need some justification for these patches.  Why do we want them?
Do they improve performance?  Reduce power consumption?  How have they
been tested?  How can we be confident that these features work
correctly on hardware in the field?  Should or could the BIOS enable
them itself, based on OEM testing and desire to support these
features?

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

* Re: [PATCH 0/4] PCI: Enable LTR/OBFF before device is used by driver
  2012-06-08 17:31 ` [PATCH 0/4] PCI: Enable LTR/OBFF before device is used by driver Bjorn Helgaas
@ 2012-06-08 18:02   ` Myron Stowe
  2012-06-08 18:10     ` Don Dutile
  0 siblings, 1 reply; 9+ messages in thread
From: Myron Stowe @ 2012-06-08 18:02 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Xudong Hao, linux-pci, linux-kernel, kvm, avi, alex.williamson,
	ddutile, myron.stowe, xiantao.zhang

On Fri, Jun 8, 2012 at 11:31 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Fri, Jun 8, 2012 at 1:01 AM, Xudong Hao <xudong.hao@intel.com> wrote:
>> The series of patches enable LTR and OBFF before device is used by driver, and
>> introduce a couple of functions to save/restore LTR latency value.
>>
>> Patch 1/4 introduce new function pci_obff_supported() as pci_ltr_support().
>>
>> Patch 2/4 enable LTR(Latency tolerance reporting) before device is used by
>> driver.
>>
>> Patch 3/4 enable OBFF(optimized buffer flush/fill) before device is used by
>> driver.
>>
>> Patch 4/4 introduce a couple of functions pci_save_ltr_value() and
>> pci_restore_ltr_value() to save and restore LTR latency value, while device is
>> reset.
>
> We need some justification for these patches.  Why do we want them?
> Do they improve performance?  Reduce power consumption?  How have they
> been tested?  How can we be confident that these features work
> correctly on hardware in the field?  Should or could the BIOS enable
> them itself, based on OEM testing and desire to support these
> features?

I too am a little nervous about these changes due to Jesse's earlier response
(see http://marc.info/?l=linux-pci&m=133372610102933&w=2) where he indicated:
  "Given how device specific these extensions are, I'd expect you'd need
   to know about each specific device anyway, which is why I think the
   control belongs in the driver."

Having these features enabled by default may be too aggressive.  Not saying it
is not correct - something you may be able to inform us about, especially since
you are with Intel - just make me nervous without further information.

Myron

> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

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

* Re: [PATCH 0/4] PCI: Enable LTR/OBFF before device is used by driver
  2012-06-08 18:02   ` Myron Stowe
@ 2012-06-08 18:10     ` Don Dutile
  0 siblings, 0 replies; 9+ messages in thread
From: Don Dutile @ 2012-06-08 18:10 UTC (permalink / raw)
  To: Myron Stowe
  Cc: Bjorn Helgaas, Xudong Hao, linux-pci, linux-kernel, kvm, avi,
	alex.williamson, myron.stowe, xiantao.zhang

On 06/08/2012 02:02 PM, Myron Stowe wrote:
> On Fri, Jun 8, 2012 at 11:31 AM, Bjorn Helgaas<bhelgaas@google.com>  wrote:
>> On Fri, Jun 8, 2012 at 1:01 AM, Xudong Hao<xudong.hao@intel.com>  wrote:
>>> The series of patches enable LTR and OBFF before device is used by driver, and
>>> introduce a couple of functions to save/restore LTR latency value.
>>>
>>> Patch 1/4 introduce new function pci_obff_supported() as pci_ltr_support().
>>>
>>> Patch 2/4 enable LTR(Latency tolerance reporting) before device is used by
>>> driver.
>>>
>>> Patch 3/4 enable OBFF(optimized buffer flush/fill) before device is used by
>>> driver.
>>>
>>> Patch 4/4 introduce a couple of functions pci_save_ltr_value() and
>>> pci_restore_ltr_value() to save and restore LTR latency value, while device is
>>> reset.
>>
>> We need some justification for these patches.  Why do we want them?
>> Do they improve performance?  Reduce power consumption?  How have they
>> been tested?  How can we be confident that these features work
>> correctly on hardware in the field?  Should or could the BIOS enable
>> them itself, based on OEM testing and desire to support these
>> features?
>
> I too am a little nervous about these changes due to Jesse's earlier response
> (see http://marc.info/?l=linux-pci&m=133372610102933&w=2) where he indicated:
>    "Given how device specific these extensions are, I'd expect you'd need
>     to know about each specific device anyway, which is why I think the
>     control belongs in the driver."
>
> Having these features enabled by default may be too aggressive.  Not saying it
> is not correct - something you may be able to inform us about, especially since
> you are with Intel - just make me nervous without further information.
>
> Myron
>
+1; like AER, I prefer the enablement be in the driver; when/if the
feature has proven itself reliable, then the kernel can enable it by default
In the case of the kernel & driver doing an enable, it won't hurt.
If want hook to disable by boot parameter, the kernel would have to clear
on scan, and put the disable *after* driver probe.

>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

* Re: [PATCH 4/4] PCI: save/restore max Latency Value for device LTR
  2012-06-08  8:01 ` [PATCH 4/4] PCI: save/restore max Latency Value for device LTR Xudong Hao
@ 2012-06-12 17:07   ` Bjorn Helgaas
  0 siblings, 0 replies; 9+ messages in thread
From: Bjorn Helgaas @ 2012-06-12 17:07 UTC (permalink / raw)
  To: Xudong Hao
  Cc: linux-pci, linux-kernel, kvm, avi, alex.williamson, ddutile,
	myron.stowe, xiantao.zhang

On Fri, Jun 8, 2012 at 2:01 AM, Xudong Hao <xudong.hao@intel.com> wrote:
> LTR: Save Max snoop/no-snoop Latency Value in pci_save_pcie_state, and restore
> them in pci_restore_pcie_state.
>
> Signed-off-by: Xudong Hao <xudong.hao@intel.com>
> ---
>  drivers/pci/pci.c   |   71 ++++++++++++++++++++++++++++++++++++++++++++++++---
>  drivers/pci/probe.c |    5 +++-
>  2 files changed, 71 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 35127c1..7172f15 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -912,6 +912,54 @@ static void pci_restore_pcie_state(struct pci_dev *dev)
>                pci_write_config_word(dev, pos + PCI_EXP_SLTCTL2, cap[i++]);
>  }
>
> +#define PCI_LTR_SAVE_REGS      2
> +
> +static int pci_save_ltr_value(struct pci_dev *dev)
> +{
> +       int i = 0, pos;
> +       struct pci_cap_saved_state *save_state;
> +       u16 *cap;
> +       u16 ctrl;
> +
> +       pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_LTR);
> +       if (!pos)
> +               return -ENOTSUPP;
> +
> +       /* Check if LTR is eanbled before save value */
> +       pci_read_config_word(dev, pos + PCI_EXP_DEVCTL2, &ctrl);

This is totally broken.  "pos" is the LTR Extended Capability (spec
section 7.25).  PCI_EXP_DEVCTL2 is a field in the PCI Express
Capability structure (section 7.8).

> +       if (!(ctrl & PCI_EXP_LTR_EN))
> +               return -ENOTSUPP;
> +
> +       save_state = pci_find_saved_cap(dev, PCI_EXT_CAP_ID_LTR);
> +       if (!save_state) {
> +               dev_err(&dev->dev, "buffer not found in %s\n", __func__);
> +               return -ENOMEM;
> +       }
> +       cap = (u16 *)&save_state->cap.data[0];
> +
> +       pci_read_config_word(dev, pos + PCI_LTR_MAX_SNOOP_LAT, &cap[i++]);
> +       pci_read_config_word(dev, pos + PCI_LTR_MAX_NOSNOOP_LAT, &cap[i++]);
> +       return 0;
> +}
> +
> +static void pci_restore_ltr_value(struct pci_dev *dev)
> +{
> +       int i = 0, pos;
> +       struct pci_cap_saved_state *save_state;
> +       u16 *cap;
> +
> +       if (!pci_ltr_supported(dev))
> +               return;
> +
> +       save_state = pci_find_saved_cap(dev, PCI_EXT_CAP_ID_LTR);
> +       pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_LTR);
> +       if (!save_state || !pos)
> +               return;
> +       cap = (u16 *)&save_state->cap.data[0];
> +
> +       pci_write_config_word(dev, pos + PCI_LTR_MAX_SNOOP_LAT, cap[i++]);
> +       pci_write_config_word(dev, pos + PCI_LTR_MAX_NOSNOOP_LAT, cap[i++]);
> +}
>
>  static int pci_save_pcix_state(struct pci_dev *dev)
>  {
> @@ -966,6 +1014,11 @@ pci_save_state(struct pci_dev *dev)
>                return i;
>        if ((i = pci_save_pcix_state(dev)) != 0)
>                return i;
> +
> +       if (pci_ltr_supported(dev))
> +               if ((i = pci_save_ltr_value(dev)) != 0)
> +                       return i;
> +
>        return 0;
>  }
>
> @@ -1034,6 +1087,7 @@ void pci_restore_state(struct pci_dev *dev)
>        pci_restore_pcix_state(dev);
>        pci_restore_msi_state(dev);
>        pci_restore_iov_state(dev);
> +       pci_restore_ltr_value(dev);
>
>        dev->state_saved = false;
>  }
> @@ -1955,12 +2009,15 @@ static void pci_add_saved_cap(struct pci_dev *pci_dev,
>  * @size: requested size of the buffer
>  */
>  static int pci_add_cap_save_buffer(
> -       struct pci_dev *dev, char cap, unsigned int size)
> +       struct pci_dev *dev, char cap, unsigned int size, bool is_extcap)
>  {
>        int pos;
>        struct pci_cap_saved_state *save_state;
>
> -       pos = pci_find_capability(dev, cap);
> +       if (is_extcap)
> +               pos = pci_find_ext_capability(dev, cap);
> +       else
> +               pos = pci_find_capability(dev, cap);
>        if (pos <= 0)
>                return 0;
>
> @@ -1984,15 +2041,21 @@ void pci_allocate_cap_save_buffers(struct pci_dev *dev)
>        int error;
>
>        error = pci_add_cap_save_buffer(dev, PCI_CAP_ID_EXP,
> -                                       PCI_EXP_SAVE_REGS * sizeof(u16));
> +                                       PCI_EXP_SAVE_REGS * sizeof(u16), 0);
>        if (error)
>                dev_err(&dev->dev,
>                        "unable to preallocate PCI Express save buffer\n");
>
> -       error = pci_add_cap_save_buffer(dev, PCI_CAP_ID_PCIX, sizeof(u16));
> +       error = pci_add_cap_save_buffer(dev, PCI_CAP_ID_PCIX, sizeof(u16), 0);
>        if (error)
>                dev_err(&dev->dev,
>                        "unable to preallocate PCI-X save buffer\n");
> +
> +       error = pci_add_cap_save_buffer(dev, PCI_EXT_CAP_ID_LTR,
> +                                       PCI_LTR_SAVE_REGS * sizeof(u16), 1);
> +       if (error)
> +               dev_err(&dev->dev,
> +                       "unable to preallocate LTR save buffer\n");
>  }
>
>  void pci_free_cap_save_buffers(struct pci_dev *dev)
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 658ac97..a6eb509 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1232,7 +1232,10 @@ static void pci_init_capabilities(struct pci_dev *dev)
>        /* MSI/MSI-X list */
>        pci_msi_init_pci_dev(dev);
>
> -       /* Buffers for saving PCIe and PCI-X capabilities */
> +       /*
> +        * Buffers for saving PCIe and PCI-X capabilities
> +        * and for saving LTR extented capabilities.
> +        */
>        pci_allocate_cap_save_buffers(dev);
>
>        /* Power Management */
> --
> 1.5.5
>

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

end of thread, other threads:[~2012-06-12 17:08 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-08  8:01 [PATCH 0/4] PCI: Enable LTR/OBFF before device is used by driver Xudong Hao
2012-06-08  8:01 ` [PATCH 1/4] Add pci_obff_supported() function Xudong Hao
2012-06-08  8:01 ` [PATCH 2/4] Enable LTR before device is used by driver Xudong Hao
2012-06-08  8:01 ` [PATCH 3/4] Enable OBFF " Xudong Hao
2012-06-08  8:01 ` [PATCH 4/4] PCI: save/restore max Latency Value for device LTR Xudong Hao
2012-06-12 17:07   ` Bjorn Helgaas
2012-06-08 17:31 ` [PATCH 0/4] PCI: Enable LTR/OBFF before device is used by driver Bjorn Helgaas
2012-06-08 18:02   ` Myron Stowe
2012-06-08 18:10     ` Don Dutile

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).