* [PATCH] PCI: Don't update VF's BAR
@ 2015-06-30 7:37 Wei Yang
2015-07-14 22:15 ` Bjorn Helgaas
0 siblings, 1 reply; 6+ messages in thread
From: Wei Yang @ 2015-06-30 7:37 UTC (permalink / raw)
To: linux-pci, bhelgaas; +Cc: Wei Yang
VF BARs are RO zero, so updating VF BARs will not take any effect.
See the SR-IOV spec r1.1, sec 3.4.1.11.
Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
---
drivers/pci/setup-res.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index 232f925..334b394 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -37,6 +37,13 @@ void pci_update_resource(struct pci_dev *dev, int resno)
struct resource *res = dev->resource + resno;
/*
+ * Per SRIOV SPEC 3.4.1.11, VF BARs are RO zero.
+ * If this is a VF, just return.
+ */
+ if (dev->is_virtfn)
+ return;
+
+ /*
* Ignore resources for unimplemented BARs and unused resource slots
* for 64 bit BARs.
*/
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] PCI: Don't update VF's BAR
2015-06-30 7:37 [PATCH] PCI: Don't update VF's BAR Wei Yang
@ 2015-07-14 22:15 ` Bjorn Helgaas
2015-07-15 1:38 ` Wei Yang
2015-07-29 8:52 ` [PATCH v2] " Wei Yang
0 siblings, 2 replies; 6+ messages in thread
From: Bjorn Helgaas @ 2015-07-14 22:15 UTC (permalink / raw)
To: Wei Yang; +Cc: linux-pci
On Tue, Jun 30, 2015 at 03:37:14PM +0800, Wei Yang wrote:
> VF BARs are RO zero, so updating VF BARs will not take any effect.
> See the SR-IOV spec r1.1, sec 3.4.1.11.
>
> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
> ---
> drivers/pci/setup-res.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index 232f925..334b394 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -37,6 +37,13 @@ void pci_update_resource(struct pci_dev *dev, int resno)
> struct resource *res = dev->resource + resno;
>
> /*
> + * Per SRIOV SPEC 3.4.1.11, VF BARs are RO zero.
> + * If this is a VF, just return.
> + */
> + if (dev->is_virtfn)
> + return;
Does this fix a problem? Are you seeing the "BAR %d: error updating"
message?
I wouldn't think we would even call pci_update_resource() for VFs, except
for pci_restore_bars(). Maybe we should check there?
If the PCI core is assigning space directly to VF BARs and trying to update
them, I'd like to know about that rather than silently ignoring it.
> +
> + /*
> * Ignore resources for unimplemented BARs and unused resource slots
> * for 64 bit BARs.
> */
> --
> 1.7.9.5
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PCI: Don't update VF's BAR
2015-07-14 22:15 ` Bjorn Helgaas
@ 2015-07-15 1:38 ` Wei Yang
2015-07-29 8:52 ` [PATCH v2] " Wei Yang
1 sibling, 0 replies; 6+ messages in thread
From: Wei Yang @ 2015-07-15 1:38 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Wei Yang, linux-pci
On Tue, Jul 14, 2015 at 05:15:11PM -0500, Bjorn Helgaas wrote:
>On Tue, Jun 30, 2015 at 03:37:14PM +0800, Wei Yang wrote:
>> VF BARs are RO zero, so updating VF BARs will not take any effect.
>> See the SR-IOV spec r1.1, sec 3.4.1.11.
>>
>> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>> ---
>> drivers/pci/setup-res.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
>> index 232f925..334b394 100644
>> --- a/drivers/pci/setup-res.c
>> +++ b/drivers/pci/setup-res.c
>> @@ -37,6 +37,13 @@ void pci_update_resource(struct pci_dev *dev, int resno)
>> struct resource *res = dev->resource + resno;
>>
>> /*
>> + * Per SRIOV SPEC 3.4.1.11, VF BARs are RO zero.
>> + * If this is a VF, just return.
>> + */
>> + if (dev->is_virtfn)
>> + return;
>
>Does this fix a problem? Are you seeing the "BAR %d: error updating"
>message?
>
Yes, I see the "error updating" message when doing vfio pass through.
>I wouldn't think we would even call pci_update_resource() for VFs, except
>for pci_restore_bars(). Maybe we should check there?
>
This happens after commit <6eb7018705de> ("vfio-pci: Move idle devices to
D3hot power state"). When do vfio, it will set the power state, and which in
turn update the BAR.
Hmm... yes, maybe the pci_restore_bars() is a good place to check.
>If the PCI core is assigning space directly to VF BARs and trying to update
>them, I'd like to know about that rather than silently ignoring it.
>
Ok, so I would add a warning at this place.
>> +
>> + /*
>> * Ignore resources for unimplemented BARs and unused resource slots
>> * for 64 bit BARs.
>> */
>> --
>> 1.7.9.5
>>
--
Richard Yang
Help you, Help me
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] PCI: Don't update VF's BAR
2015-07-14 22:15 ` Bjorn Helgaas
2015-07-15 1:38 ` Wei Yang
@ 2015-07-29 8:52 ` Wei Yang
2015-08-27 17:27 ` Bjorn Helgaas
1 sibling, 1 reply; 6+ messages in thread
From: Wei Yang @ 2015-07-29 8:52 UTC (permalink / raw)
To: bhelgaas; +Cc: linux-pci, Wei Yang
VF BARs are RO zero, so updating VF BARs will not take any effect.
See the SR-IOV spec r1.1, sec 3.4.1.11.
Also this patch adds a warning in pci_update_resource() in case someone
really tries to update it.
Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
---
drivers/pci/pci.c | 7 +++++++
drivers/pci/setup-res.c | 5 +++++
2 files changed, 12 insertions(+)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 0008c95..93c0d24 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -483,6 +483,13 @@ static void pci_restore_bars(struct pci_dev *dev)
{
int i;
+ /*
+ * Per SRIOV SPEC 3.4.1.11, VF BARs are RO zero.
+ * If this is a VF, just return.
+ */
+ if (dev->is_virtfn)
+ return;
+
for (i = 0; i < PCI_BRIDGE_RESOURCES; i++)
pci_update_resource(dev, i);
}
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index 232f925..ebe57db 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -36,6 +36,11 @@ void pci_update_resource(struct pci_dev *dev, int resno)
enum pci_bar_type type;
struct resource *res = dev->resource + resno;
+ if (dev->is_virtfn) {
+ dev_warn(&dev->dev, "Trying to update VF BAR\n");
+ return;
+ }
+
/*
* Ignore resources for unimplemented BARs and unused resource slots
* for 64 bit BARs.
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] PCI: Don't update VF's BAR
2015-07-29 8:52 ` [PATCH v2] " Wei Yang
@ 2015-08-27 17:27 ` Bjorn Helgaas
2015-08-28 3:51 ` Richard Yang
0 siblings, 1 reply; 6+ messages in thread
From: Bjorn Helgaas @ 2015-08-27 17:27 UTC (permalink / raw)
To: Wei Yang; +Cc: linux-pci, Alex Williamson
[+cc Alex]
On Wed, Jul 29, 2015 at 04:52:58PM +0800, Wei Yang wrote:
> VF BARs are RO zero, so updating VF BARs will not take any effect.
> See the SR-IOV spec r1.1, sec 3.4.1.11.
>
> Also this patch adds a warning in pci_update_resource() in case someone
> really tries to update it.
>
> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
I applied this with some whitespace and changelog fixes to
pci-4.4/virtualization for v4.4, thanks! I will rebase this branch
v4.3-rc1 or later.
You mentioned the justification (avoids "error updating" messages) on the
mailing list. It helps me out if you include that in the changelog, but I
added it for you.
Bjorn
commit fa47e4466c567b43bcf8e152a4425277b6d033d9
Author: Wei Yang <weiyang@linux.vnet.ibm.com>
Date: Wed Jul 29 16:52:58 2015 +0800
PCI: Don't try to restore VF BARs
VF BARs are read-only zero, so updating VF BARs will not have any effect.
See the SR-IOV spec r1.1, sec 3.4.1.11.
Don't update VF BARs in pci_restore_bars().
This avoids spurious "BAR %d: error updating" messages that we see when
doing vfio pass-through after 6eb7018705de ("vfio-pci: Move idle devices to
D3hot power state").
[bhelgaas: changelog, fix whitespace]
Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 0008c95..1a682f8 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -473,7 +473,7 @@ int pci_wait_for_pending(struct pci_dev *dev, int pos, u16 mask)
}
/**
- * pci_restore_bars - restore a devices BAR values (e.g. after wake-up)
+ * pci_restore_bars - restore a device's BAR values (e.g. after wake-up)
* @dev: PCI device to have its BARs restored
*
* Restore the BAR values for a given device, so as to make it
@@ -483,6 +483,10 @@ static void pci_restore_bars(struct pci_dev *dev)
{
int i;
+ /* Per SR-IOV spec 3.4.1.11, VF BARs are RO zero */
+ if (dev->is_virtfn)
+ return;
+
for (i = 0; i < PCI_BRIDGE_RESOURCES; i++)
pci_update_resource(dev, i);
}
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index 232f925..152de5c 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -36,6 +36,11 @@ void pci_update_resource(struct pci_dev *dev, int resno)
enum pci_bar_type type;
struct resource *res = dev->resource + resno;
+ if (dev->is_virtfn) {
+ dev_warn(&dev->dev, "can't update VF BAR%d\n", resno);
+ return;
+ }
+
/*
* Ignore resources for unimplemented BARs and unused resource slots
* for 64 bit BARs.
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] PCI: Don't update VF's BAR
2015-08-27 17:27 ` Bjorn Helgaas
@ 2015-08-28 3:51 ` Richard Yang
0 siblings, 0 replies; 6+ messages in thread
From: Richard Yang @ 2015-08-28 3:51 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Wei Yang, linux-pci, Alex Williamson
On Thu, Aug 27, 2015 at 12:27:53PM -0500, Bjorn Helgaas wrote:
>[+cc Alex]
>
>On Wed, Jul 29, 2015 at 04:52:58PM +0800, Wei Yang wrote:
>> VF BARs are RO zero, so updating VF BARs will not take any effect.
>> See the SR-IOV spec r1.1, sec 3.4.1.11.
>>
>> Also this patch adds a warning in pci_update_resource() in case someone
>> really tries to update it.
>>
>> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>
>I applied this with some whitespace and changelog fixes to
>pci-4.4/virtualization for v4.4, thanks! I will rebase this branch
>v4.3-rc1 or later.
>
>You mentioned the justification (avoids "error updating" messages) on the
>mailing list. It helps me out if you include that in the changelog, but I
>added it for you.
Bjorn,
You are always nice to us :-)
Have a good weekend.
>
>Bjorn
>
>commit fa47e4466c567b43bcf8e152a4425277b6d033d9
>Author: Wei Yang <weiyang@linux.vnet.ibm.com>
>Date: Wed Jul 29 16:52:58 2015 +0800
>
> PCI: Don't try to restore VF BARs
>
> VF BARs are read-only zero, so updating VF BARs will not have any effect.
> See the SR-IOV spec r1.1, sec 3.4.1.11.
>
> Don't update VF BARs in pci_restore_bars().
>
> This avoids spurious "BAR %d: error updating" messages that we see when
> doing vfio pass-through after 6eb7018705de ("vfio-pci: Move idle devices to
> D3hot power state").
>
> [bhelgaas: changelog, fix whitespace]
> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>
>diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>index 0008c95..1a682f8 100644
>--- a/drivers/pci/pci.c
>+++ b/drivers/pci/pci.c
>@@ -473,7 +473,7 @@ int pci_wait_for_pending(struct pci_dev *dev, int pos, u16 mask)
> }
>
> /**
>- * pci_restore_bars - restore a devices BAR values (e.g. after wake-up)
>+ * pci_restore_bars - restore a device's BAR values (e.g. after wake-up)
> * @dev: PCI device to have its BARs restored
> *
> * Restore the BAR values for a given device, so as to make it
>@@ -483,6 +483,10 @@ static void pci_restore_bars(struct pci_dev *dev)
> {
> int i;
>
>+ /* Per SR-IOV spec 3.4.1.11, VF BARs are RO zero */
>+ if (dev->is_virtfn)
>+ return;
>+
> for (i = 0; i < PCI_BRIDGE_RESOURCES; i++)
> pci_update_resource(dev, i);
> }
>diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
>index 232f925..152de5c 100644
>--- a/drivers/pci/setup-res.c
>+++ b/drivers/pci/setup-res.c
>@@ -36,6 +36,11 @@ void pci_update_resource(struct pci_dev *dev, int resno)
> enum pci_bar_type type;
> struct resource *res = dev->resource + resno;
>
>+ if (dev->is_virtfn) {
>+ dev_warn(&dev->dev, "can't update VF BAR%d\n", resno);
>+ return;
>+ }
>+
> /*
> * Ignore resources for unimplemented BARs and unused resource slots
> * for 64 bit BARs.
--
Richard Yang
Help you, Help me
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-08-28 3:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-30 7:37 [PATCH] PCI: Don't update VF's BAR Wei Yang
2015-07-14 22:15 ` Bjorn Helgaas
2015-07-15 1:38 ` Wei Yang
2015-07-29 8:52 ` [PATCH v2] " Wei Yang
2015-08-27 17:27 ` Bjorn Helgaas
2015-08-28 3:51 ` Richard Yang
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).