From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Return-Path: From: "Raslan, KarimAllah" To: "helgaas@kernel.org" CC: "linux-kernel@vger.kernel.org" , "linux-pci@vger.kernel.org" , "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 Message-ID: <1520051649.28771.3.camel@amazon.de> References: <1519939897-14596-1-git-send-email-karahmed@amazon.de> <1519939897-14596-2-git-send-email-karahmed@amazon.de> <20180302214857.GD202062@bhelgaas-glaptop.roam.corp.google.com> In-Reply-To: <20180302214857.GD202062@bhelgaas-glaptop.roam.corp.google.com> Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 List-ID: 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== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751754AbeCCEeU (ORCPT ); Fri, 2 Mar 2018 23:34:20 -0500 Received: from smtp-fw-33001.amazon.com ([207.171.190.10]:7281 "EHLO smtp-fw-33001.amazon.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751692AbeCCEeS (ORCPT ); Fri, 2 Mar 2018 23:34:18 -0500 X-IronPort-AV: E=Sophos;i="5.47,415,1515456000"; d="scan'208";a="722536129" From: "Raslan, KarimAllah" To: "helgaas@kernel.org" CC: "linux-kernel@vger.kernel.org" , "linux-pci@vger.kernel.org" , "bhelgaas@google.com" Subject: Re: [PATCH v3 2/2] PCI/IOV: Use the cached VF BARs size instead of re-reading them Thread-Topic: [PATCH v3 2/2] PCI/IOV: Use the cached VF BARs size instead of re-reading them Thread-Index: AQHTsaS1TwIm3iGB3UmhQpx81kZ7sqO9fT6AgABxNoA= Date: Sat, 3 Mar 2018 04:34:10 +0000 Message-ID: <1520051649.28771.3.camel@amazon.de> References: <1519939897-14596-1-git-send-email-karahmed@amazon.de> <1519939897-14596-2-git-send-email-karahmed@amazon.de> <20180302214857.GD202062@bhelgaas-glaptop.roam.corp.google.com> In-Reply-To: <20180302214857.GD202062@bhelgaas-glaptop.roam.corp.google.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.43.164.111] Content-Type: text/plain; charset="utf-8" Content-ID: <0A7983D3A3C88A43ADB3958C028B793A@amazon.com> MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id w234YUFC021153 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 > > Cc: linux-pci@vger.kernel.org > > Cc: linux-kernel@vger.kernel.org > > Signed-off-by: KarimAllah Ahmed > > --- > > 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