From: "Raslan, KarimAllah" <karahmed@amazon.de>
To: "helgaas@kernel.org" <helgaas@kernel.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
"bhelgaas@google.com" <bhelgaas@google.com>
Subject: Re: [PATCH v3 2/2] PCI/IOV: Use the cached VF BARs size instead of re-reading them
Date: Sat, 3 Mar 2018 04:34:10 +0000 [thread overview]
Message-ID: <1520051649.28771.3.camel@amazon.de> (raw)
In-Reply-To: <20180302214857.GD202062@bhelgaas-glaptop.roam.corp.google.com>
T24gRnJpLCAyMDE4LTAzLTAyIGF0IDE1OjQ4IC0wNjAwLCBCam9ybiBIZWxnYWFzIHdyb3RlOg0K
PiBPbiBUaHUsIE1hciAwMSwgMjAxOCBhdCAxMDozMTozN1BNICswMTAwLCBLYXJpbUFsbGFoIEFo
bWVkIHdyb3RlOg0KPiA+IA0KPiA+IFVzZSB0aGUgY2FjaGVkIFZGIEJBUnMgc2l6ZSBpbnN0ZWFk
IG9mIHJlLXJlYWRpbmcgdGhlbSBmcm9tIHRoZSBoYXJkd2FyZS4NCj4gPiBUaGF0IGF2b2lkcyBk
b2luZyB1bm5lY2Vzc2FyaWx5IGJ1cyB0cmFuc2FjdGlvbnMgd2hpY2ggaXMgc3BlY2lhbGx5DQo+
ID4gbm90aWNhYmxlIHdoZW4geW91IGhhdmUgYSBQRiB3aXRoIGEgbGFyZ2UgbnVtYmVyIG9mIFZG
cy4NCj4gDQo+IFRoYW5rcyBhIGxvdCBmb3IgYnJlYWtpbmcgdGhpcyBvdXQhICBJdCBzZWVtcyB0
cml2aWFsLCBidXQgaXQgZGlkIG1ha2UgaXQNCj4gbXVjaCBlYXNpZXIgZm9yIG1lIHRvIHRoaW5r
IGFib3V0IHRoaXMgb25lLg0KPiANCj4gPiANCj4gPiBDYzogQmpvcm4gSGVsZ2FhcyA8YmhlbGdh
YXNAZ29vZ2xlLmNvbT4NCj4gPiBDYzogbGludXgtcGNpQHZnZXIua2VybmVsLm9yZw0KPiA+IENj
OiBsaW51eC1rZXJuZWxAdmdlci5rZXJuZWwub3JnDQo+ID4gU2lnbmVkLW9mZi1ieTogS2FyaW1B
bGxhaCBBaG1lZCA8a2FyYWhtZWRAYW1hem9uLmRlPg0KPiA+IC0tLQ0KPiA+ICBkcml2ZXJzL3Bj
aS9wcm9iZS5jIHwgMjQgKysrKysrKysrKysrKysrKysrLS0tLS0tDQo+ID4gIDEgZmlsZSBjaGFu
Z2VkLCAxOCBpbnNlcnRpb25zKCspLCA2IGRlbGV0aW9ucygtKQ0KPiA+IA0KPiA+IGRpZmYgLS1n
aXQgYS9kcml2ZXJzL3BjaS9wcm9iZS5jIGIvZHJpdmVycy9wY2kvcHJvYmUuYw0KPiA+IGluZGV4
IGE5NjgzN2UuLmFlYWExMGEgMTAwNjQ0DQo+ID4gLS0tIGEvZHJpdmVycy9wY2kvcHJvYmUuYw0K
PiA+ICsrKyBiL2RyaXZlcnMvcGNpL3Byb2JlLmMNCj4gPiBAQCAtMTgwLDYgKzE4MCw3IEBAIHN0
YXRpYyBpbmxpbmUgdW5zaWduZWQgbG9uZyBkZWNvZGVfYmFyKHN0cnVjdCBwY2lfZGV2ICpkZXYs
IHUzMiBiYXIpDQo+ID4gIGludCBfX3BjaV9yZWFkX2Jhc2Uoc3RydWN0IHBjaV9kZXYgKmRldiwg
ZW51bSBwY2lfYmFyX3R5cGUgdHlwZSwNCj4gPiAgCQkgICAgc3RydWN0IHJlc291cmNlICpyZXMs
IHVuc2lnbmVkIGludCBwb3MpDQo+ID4gIHsNCj4gPiArCWludCBiYXIgPSByZXMgLSBkZXYtPnJl
c291cmNlOw0KPiA+ICAJdTMyIGwgPSAwLCBzeiA9IDAsIG1hc2s7DQo+ID4gIAl1NjQgbDY0LCBz
ejY0LCBtYXNrNjQ7DQo+ID4gIAl1MTYgb3JpZ19jbWQ7DQo+ID4gQEAgLTE5OSw5ICsyMDAsMTMg
QEAgaW50IF9fcGNpX3JlYWRfYmFzZShzdHJ1Y3QgcGNpX2RldiAqZGV2LCBlbnVtIHBjaV9iYXJf
dHlwZSB0eXBlLA0KPiA+ICAJcmVzLT5uYW1lID0gcGNpX25hbWUoZGV2KTsNCj4gPiAgDQo+ID4g
IAlwY2lfcmVhZF9jb25maWdfZHdvcmQoZGV2LCBwb3MsICZsKTsNCj4gPiAtCXBjaV93cml0ZV9j
b25maWdfZHdvcmQoZGV2LCBwb3MsIGwgfCBtYXNrKTsNCj4gPiAtCXBjaV9yZWFkX2NvbmZpZ19k
d29yZChkZXYsIHBvcywgJnN6KTsNCj4gPiAtCXBjaV93cml0ZV9jb25maWdfZHdvcmQoZGV2LCBw
b3MsIGwpOw0KPiA+ICsJaWYgKGRldi0+aXNfdmlydGZuKSB7DQo+ID4gKwkJc3ogPSBkZXYtPnBo
eXNmbi0+c3Jpb3YtPmJhcnN6W2Jhcl0gJiAweGZmZmZmZmZmOw0KPiA+ICsJfSBlbHNlIHsNCj4g
PiArCQlwY2lfd3JpdGVfY29uZmlnX2R3b3JkKGRldiwgcG9zLCBsIHwgbWFzayk7DQo+ID4gKwkJ
cGNpX3JlYWRfY29uZmlnX2R3b3JkKGRldiwgcG9zLCAmc3opOw0KPiA+ICsJCXBjaV93cml0ZV9j
b25maWdfZHdvcmQoZGV2LCBwb3MsIGwpOw0KPiA+ICsJfQ0KPiANCj4gSSBkb24ndCBxdWl0ZSB1
bmRlcnN0YW5kIHRoaXMuICBUaGlzIGlzIHJlYWRpbmcgdGhlIHJlZ3VsYXIgQkFScyAoY29uZmln
DQo+IG9mZnNldHMgMHgxMCwgMHgxNCwgLi4uLCAweDI0KS4gIFBlciBzZWMgOS4zLjQuMS4xMSwg
dGhlc2UgYXJlIGFsbCBSTyBaZXJvDQo+IGZvciBWRnMuICBUaGF0IHNob3VsZCBtYWtlIHRoZW0g
bG9vayBsaWtlIHRoZXkncmUgYWxsIHVuaW1wbGVtZW50ZWQuDQo+IA0KPiBCdXQgdGhpcyBwYXRj
aCBtYWtlcyB1cyB1c2UgdGhlIHNpemUgd2UgZGlzY292ZXJlZCBmcm9tIHRoZSBQRidzIFZGIEJB
Um4NCj4gcmVnaXN0ZXJzIGluIGl0cyBTUi1JT1YgY2FwYWJpbGl0eS4gIFdvbid0IHRoYXQgY2F1
c2UgdXMgdG8gZmlsbCBpbiB0aGUNCj4gVkYncyBkZXYtPnJlc291cmNlW25dLCB3aGVuIHdlIGRp
ZG4ndCBkbyBpdCBiZWZvcmU/DQoNCk9oIC4uIHRoYXQgaXMgY29ycmVjdCEgSSBkaWQgbm90IG5v
dGljZSB0aGlzIHBhcnQgZnJvbSB0aGUgc3BlYyA6KQ0KDQo+IA0KPiA+IA0KPiA+ICAJLyoNCj4g
PiAgCSAqIEFsbCBiaXRzIHNldCBpbiBzeiBtZWFucyB0aGUgZGV2aWNlIGlzbid0IHdvcmtpbmcg
cHJvcGVybHkuDQo+ID4gQEAgLTI0MSw5ICsyNDYsMTQgQEAgaW50IF9fcGNpX3JlYWRfYmFzZShz
dHJ1Y3QgcGNpX2RldiAqZGV2LCBlbnVtIHBjaV9iYXJfdHlwZSB0eXBlLA0KPiA+ICANCj4gPiAg
CWlmIChyZXMtPmZsYWdzICYgSU9SRVNPVVJDRV9NRU1fNjQpIHsNCj4gPiAgCQlwY2lfcmVhZF9j
b25maWdfZHdvcmQoZGV2LCBwb3MgKyA0LCAmbCk7DQo+ID4gLQkJcGNpX3dyaXRlX2NvbmZpZ19k
d29yZChkZXYsIHBvcyArIDQsIH4wKTsNCj4gPiAtCQlwY2lfcmVhZF9jb25maWdfZHdvcmQoZGV2
LCBwb3MgKyA0LCAmc3opOw0KPiA+IC0JCXBjaV93cml0ZV9jb25maWdfZHdvcmQoZGV2LCBwb3Mg
KyA0LCBsKTsNCj4gPiArDQo+ID4gKwkJaWYgKGRldi0+aXNfdmlydGZuKSB7DQo+ID4gKwkJCXN6
ID0gKGRldi0+cGh5c2ZuLT5zcmlvdi0+YmFyc3pbYmFyXSA+PiAzMikgJiAweGZmZmZmZmZmOw0K
PiA+ICsJCX0gZWxzZSB7DQo+ID4gKwkJCXBjaV93cml0ZV9jb25maWdfZHdvcmQoZGV2LCBwb3Mg
KyA0LCB+MCk7DQo+ID4gKwkJCXBjaV9yZWFkX2NvbmZpZ19kd29yZChkZXYsIHBvcyArIDQsICZz
eik7DQo+ID4gKwkJCXBjaV93cml0ZV9jb25maWdfZHdvcmQoZGV2LCBwb3MgKyA0LCBsKTsNCj4g
PiArCQl9DQo+ID4gIA0KPiA+ICAJCWw2NCB8PSAoKHU2NClsIDw8IDMyKTsNCj4gPiAgCQlzejY0
IHw9ICgodTY0KXN6IDw8IDMyKTsNCj4gPiBAQCAtMzMyLDYgKzM0Miw4IEBAIHN0YXRpYyB2b2lk
IHBjaV9yZWFkX2Jhc2VzKHN0cnVjdCBwY2lfZGV2ICpkZXYsIHVuc2lnbmVkIGludCBob3dtYW55
LCBpbnQgcm9tKQ0KPiA+ICAJZm9yIChwb3MgPSAwOyBwb3MgPCBob3dtYW55OyBwb3MrKykgew0K
PiA+ICAJCXN0cnVjdCByZXNvdXJjZSAqcmVzID0gJmRldi0+cmVzb3VyY2VbcG9zXTsNCj4gPiAg
CQlyZWcgPSBQQ0lfQkFTRV9BRERSRVNTXzAgKyAocG9zIDw8IDIpOw0KPiA+ICsJCWlmIChkZXYt
PmlzX3ZpcnRmbiAmJiBkZXYtPnBoeXNmbi0+c3Jpb3YtPmJhcnN6W3Bvc10gPT0gMCkNCj4gPiAr
CQkJY29udGludWU7DQo+IA0KPiBTaW5jZSB3ZSBrbm93IHRoZSBWRiBCQVJzIGFyZSBhbGwgemVy
byAodGhlIG9uZXMgaW4gdGhlIFZGIGNvbmZpZyBzcGFjZSwNCj4gbm90IHRoZSBvbmVzIGluIHRo
ZSBQRiBTUi1JT1YgY2FwYWJpbGl0eSksIGluY2x1ZGluZyB0aGUgVkYgUk9NIEJBUiwgaXQNCj4g
d291bGQgbWFrZSBzZW5zZSB0byBtZSB0byB0b3RhbGx5IHNraXAgdGhpcyB3aG9sZSBmdW5jdGlv
biwgZS5nLiwNCj4gDQo+ICAgaWYgKGRldi0+bm9uX2NvbXBsaWFudF9iYXJzKQ0KPiAgICAgcmV0
dXJuOw0KPiANCj4gICBpZiAoZGV2LT5pc192aXJ0Zm4pDQo+ICAgICByZXR1cm47DQo+IA0KDQpD
b3JyZWN0ISBEb25lLg0KDQo+ID4gDQo+ID4gIAkJcG9zICs9IF9fcGNpX3JlYWRfYmFzZShkZXYs
IHBjaV9iYXJfdW5rbm93biwgcmVzLCByZWcpOw0KPiA+ICAJfQ0KPiA+ICANCj4gPiAtLSANCj4g
PiAyLjcuNA0KPiA+IA0KPiANCkFtYXpvbiBEZXZlbG9wbWVudCBDZW50ZXIgR2VybWFueSBHbWJI
CkJlcmxpbiAtIERyZXNkZW4gLSBBYWNoZW4KbWFpbiBvZmZpY2U6IEtyYXVzZW5zdHIuIDM4LCAx
MDExNyBCZXJsaW4KR2VzY2hhZWZ0c2Z1ZWhyZXI6IERyLiBSYWxmIEhlcmJyaWNoLCBDaHJpc3Rp
YW4gU2NobGFlZ2VyClVzdC1JRDogREUyODkyMzc4NzkKRWluZ2V0cmFnZW4gYW0gQW10c2dlcmlj
aHQgQ2hhcmxvdHRlbmJ1cmcgSFJCIDE0OTE3MyBCCg==
WARNING: multiple messages have this Message-ID (diff)
From: "Raslan, KarimAllah" <karahmed@amazon.de>
To: "helgaas@kernel.org" <helgaas@kernel.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
"bhelgaas@google.com" <bhelgaas@google.com>
Subject: Re: [PATCH v3 2/2] PCI/IOV: Use the cached VF BARs size instead of re-reading them
Date: Sat, 3 Mar 2018 04:34:10 +0000 [thread overview]
Message-ID: <1520051649.28771.3.camel@amazon.de> (raw)
In-Reply-To: <20180302214857.GD202062@bhelgaas-glaptop.roam.corp.google.com>
On Fri, 2018-03-02 at 15:48 -0600, Bjorn Helgaas wrote:
> On Thu, Mar 01, 2018 at 10:31:37PM +0100, KarimAllah Ahmed wrote:
> >
> > Use the cached VF BARs size instead of re-reading them from the hardware.
> > That avoids doing unnecessarily bus transactions which is specially
> > noticable when you have a PF with a large number of VFs.
>
> Thanks a lot for breaking this out! It seems trivial, but it did make it
> much easier for me to think about this one.
>
> >
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: linux-pci@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: KarimAllah Ahmed <karahmed@amazon.de>
> > ---
> > drivers/pci/probe.c | 24 ++++++++++++++++++------
> > 1 file changed, 18 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index a96837e..aeaa10a 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -180,6 +180,7 @@ static inline unsigned long decode_bar(struct pci_dev *dev, u32 bar)
> > int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
> > struct resource *res, unsigned int pos)
> > {
> > + int bar = res - dev->resource;
> > u32 l = 0, sz = 0, mask;
> > u64 l64, sz64, mask64;
> > u16 orig_cmd;
> > @@ -199,9 +200,13 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
> > res->name = pci_name(dev);
> >
> > pci_read_config_dword(dev, pos, &l);
> > - pci_write_config_dword(dev, pos, l | mask);
> > - pci_read_config_dword(dev, pos, &sz);
> > - pci_write_config_dword(dev, pos, l);
> > + if (dev->is_virtfn) {
> > + sz = dev->physfn->sriov->barsz[bar] & 0xffffffff;
> > + } else {
> > + pci_write_config_dword(dev, pos, l | mask);
> > + pci_read_config_dword(dev, pos, &sz);
> > + pci_write_config_dword(dev, pos, l);
> > + }
>
> I don't quite understand this. This is reading the regular BARs (config
> offsets 0x10, 0x14, ..., 0x24). Per sec 9.3.4.1.11, these are all RO Zero
> for VFs. That should make them look like they're all unimplemented.
>
> But this patch makes us use the size we discovered from the PF's VF BARn
> registers in its SR-IOV capability. Won't that cause us to fill in the
> VF's dev->resource[n], when we didn't do it before?
Oh .. that is correct! I did not notice this part from the spec :)
>
> >
> > /*
> > * All bits set in sz means the device isn't working properly.
> > @@ -241,9 +246,14 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
> >
> > if (res->flags & IORESOURCE_MEM_64) {
> > pci_read_config_dword(dev, pos + 4, &l);
> > - pci_write_config_dword(dev, pos + 4, ~0);
> > - pci_read_config_dword(dev, pos + 4, &sz);
> > - pci_write_config_dword(dev, pos + 4, l);
> > +
> > + if (dev->is_virtfn) {
> > + sz = (dev->physfn->sriov->barsz[bar] >> 32) & 0xffffffff;
> > + } else {
> > + pci_write_config_dword(dev, pos + 4, ~0);
> > + pci_read_config_dword(dev, pos + 4, &sz);
> > + pci_write_config_dword(dev, pos + 4, l);
> > + }
> >
> > l64 |= ((u64)l << 32);
> > sz64 |= ((u64)sz << 32);
> > @@ -332,6 +342,8 @@ static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom)
> > for (pos = 0; pos < howmany; pos++) {
> > struct resource *res = &dev->resource[pos];
> > reg = PCI_BASE_ADDRESS_0 + (pos << 2);
> > + if (dev->is_virtfn && dev->physfn->sriov->barsz[pos] == 0)
> > + continue;
>
> Since we know the VF BARs are all zero (the ones in the VF config space,
> not the ones in the PF SR-IOV capability), including the VF ROM BAR, it
> would make sense to me to totally skip this whole function, e.g.,
>
> if (dev->non_compliant_bars)
> return;
>
> if (dev->is_virtfn)
> return;
>
Correct! Done.
> >
> > pos += __pci_read_base(dev, pci_bar_unknown, res, reg);
> > }
> >
> > --
> > 2.7.4
> >
>
Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
next prev parent reply other threads:[~2018-03-03 4:34 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-01 21:31 [PATCH v3 1/2] PCI/IOV: Store more data about VFs into the SRIOV struct KarimAllah Ahmed
2018-03-01 21:31 ` [PATCH v3 2/2] PCI/IOV: Use the cached VF BARs size instead of re-reading them KarimAllah Ahmed
2018-03-02 21:48 ` Bjorn Helgaas
2018-03-03 4:34 ` Raslan, KarimAllah [this message]
2018-03-03 4:34 ` Raslan, KarimAllah
2018-03-02 21:36 ` [PATCH v3 1/2] PCI/IOV: Store more data about VFs into the SRIOV struct Bjorn Helgaas
2018-03-06 10:37 ` Raslan, KarimAllah
2018-03-06 10:37 ` Raslan, KarimAllah
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1520051649.28771.3.camel@amazon.de \
--to=karahmed@amazon.de \
--cc=bhelgaas@google.com \
--cc=helgaas@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.