[PATCH kernel] powerpc/powernv: Update comment about shifting IOV BAR

[PATCH kernel] powerpc/powernv: Update comment about shifting IOV BAR

[Alexey Kardashevskiy]

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

From: Alexey Kardashevskiy <aik@ozlabs.ru>

This updates the comment about creating a hole in /proc/iomem which
should not be normally happening but it does in the powernv platform
due the way MMIO M64 BARs are organised in the IODA2-capable hardware.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---

It has been mentioned multiple times (last one -
https://www.spinics.net/lists/linux-pci/msg64084.html ) that the comment
is not informative enough for people not particularly familiar with
the POWER8 IO hardware.

This attempt aims to:
1. explain why we shift the resource
2. explain why nothing can use that hole as a resource while it is "free"
(I am not sure that this is the case actually)

Please comment, everyone, let's have this very well documented while
I remember these bits :) Thanks.
---
 arch/powerpc/platforms/powernv/pci-ioda.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 48de308224d6..c4a36ae78c95 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1002,9 +1002,13 @@ static int pnv_pci_vf_resource_shift(struct pci_dev *dev, int offset)
 	}
 
 	/*
-	 * After doing so, there would be a "hole" in the /proc/iomem when
-	 * offset is a positive value. It looks like the device return some
-	 * mmio back to the system, which actually no one could use it.
+	 * Since M64 BAR shares segments among all possible 256 PEs,
+	 * we have to shift the beginning of PF IOV BAR to make it start from
+	 * the segment which belongs to the PE number assigned to the first VF.
+	 * This creates a "hole" in the /proc/iomem which could be used for
+	 * allocating other resources, however this is not expected to happen
+	 * on IODA as the only possibility would be a PCI hotplug and IODA
+	 * hardware only allows it on a slot with dedicated PHB.
 	 */
 	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
 		res = &dev->resource[i + PCI_IOV_RESOURCES];
-- 
2.11.0

Re: [PATCH kernel] powerpc/powernv: Update comment about shifting IOV BAR

[Alexey Kardashevskiy]

On 31/08/17 13:34, Alexey Kardashevskiy wrote:
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Oops, this was not right :)

Anyway, Ben, please comment. Thanks.


> 
> From: Alexey Kardashevskiy <aik@ozlabs.ru>
> 
> This updates the comment about creating a hole in /proc/iomem which
> should not be normally happening but it does in the powernv platform
> due the way MMIO M64 BARs are organised in the IODA2-capable hardware.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> 
> It has been mentioned multiple times (last one -
> https://www.spinics.net/lists/linux-pci/msg64084.html ) that the comment
> is not informative enough for people not particularly familiar with
> the POWER8 IO hardware.
> 
> This attempt aims to:
> 1. explain why we shift the resource
> 2. explain why nothing can use that hole as a resource while it is "free"
> (I am not sure that this is the case actually)
> 
> Please comment, everyone, let's have this very well documented while
> I remember these bits :) Thanks.
> ---
>  arch/powerpc/platforms/powernv/pci-ioda.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> index 48de308224d6..c4a36ae78c95 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -1002,9 +1002,13 @@ static int pnv_pci_vf_resource_shift(struct pci_dev *dev, int offset)
>  	}
>  
>  	/*
> -	 * After doing so, there would be a "hole" in the /proc/iomem when
> -	 * offset is a positive value. It looks like the device return some
> -	 * mmio back to the system, which actually no one could use it.
> +	 * Since M64 BAR shares segments among all possible 256 PEs,
> +	 * we have to shift the beginning of PF IOV BAR to make it start from
> +	 * the segment which belongs to the PE number assigned to the first VF.
> +	 * This creates a "hole" in the /proc/iomem which could be used for
> +	 * allocating other resources, however this is not expected to happen
> +	 * on IODA as the only possibility would be a PCI hotplug and IODA
> +	 * hardware only allows it on a slot with dedicated PHB.
>  	 */
>  	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
>  		res = &dev->resource[i + PCI_IOV_RESOURCES];
> 


-- 
Alexey

Re: [PATCH kernel] powerpc/powernv: Update comment about shifting IOV BAR

[Benjamin Herrenschmidt]

On Thu, 2017-09-14 at 12:45 +1000, Alexey Kardashevskiy wrote:
> On 31/08/17 13:34, Alexey Kardashevskiy wrote:
> > From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> Oops, this was not right :)
> 
> Anyway, Ben, please comment. Thanks.

This is incorrect, we can do hotplug behind switches afaik.
> 
> > 
> > From: Alexey Kardashevskiy <aik@ozlabs.ru>
> > 
> > This updates the comment about creating a hole in /proc/iomem which
> > should not be normally happening but it does in the powernv platform
> > due the way MMIO M64 BARs are organised in the IODA2-capable hardware.
> > 
> > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> > ---
> > 
> > It has been mentioned multiple times (last one -
> > https://www.spinics.net/lists/linux-pci/msg64084.html ) that the comment
> > is not informative enough for people not particularly familiar with
> > the POWER8 IO hardware.
> > 
> > This attempt aims to:
> > 1. explain why we shift the resource
> > 2. explain why nothing can use that hole as a resource while it is "free"
> > (I am not sure that this is the case actually)
> > 
> > Please comment, everyone, let's have this very well documented while
> > I remember these bits :) Thanks.
> > ---
> >  arch/powerpc/platforms/powernv/pci-ioda.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> > index 48de308224d6..c4a36ae78c95 100644
> > --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> > +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> > @@ -1002,9 +1002,13 @@ static int pnv_pci_vf_resource_shift(struct pci_dev *dev, int offset)
> >  	}
> >  
> >  	/*
> > -	 * After doing so, there would be a "hole" in the /proc/iomem when
> > -	 * offset is a positive value. It looks like the device return some
> > -	 * mmio back to the system, which actually no one could use it.
> > +	 * Since M64 BAR shares segments among all possible 256 PEs,
> > +	 * we have to shift the beginning of PF IOV BAR to make it start from
> > +	 * the segment which belongs to the PE number assigned to the first VF.
> > +	 * This creates a "hole" in the /proc/iomem which could be used for
> > +	 * allocating other resources, however this is not expected to happen
> > +	 * on IODA as the only possibility would be a PCI hotplug and IODA
> > +	 * hardware only allows it on a slot with dedicated PHB.
> >  	 */
> >  	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
> >  		res = &dev->resource[i + PCI_IOV_RESOURCES];
> > 
> 
> 

Re: [PATCH kernel] powerpc/powernv: Update comment about shifting IOV BAR

[Alexey Kardashevskiy]

On 14/09/17 13:07, Benjamin Herrenschmidt wrote:
> On Thu, 2017-09-14 at 12:45 +1000, Alexey Kardashevskiy wrote:
>> On 31/08/17 13:34, Alexey Kardashevskiy wrote:
>>> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>
>> Oops, this was not right :)
>>
>> Anyway, Ben, please comment. Thanks.
> 
> This is incorrect, we can do hotplug behind switches afaik.

Do we have an actual system which allows this? Anyway, what we do now is
wrong and it needs what? Reserve that hole? I'd like to update the comment
for now, at least, and state what bad thing can happen and what we expect.


>>
>>>
>>> From: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>
>>> This updates the comment about creating a hole in /proc/iomem which
>>> should not be normally happening but it does in the powernv platform
>>> due the way MMIO M64 BARs are organised in the IODA2-capable hardware.
>>>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> ---
>>>
>>> It has been mentioned multiple times (last one -
>>> https://www.spinics.net/lists/linux-pci/msg64084.html ) that the comment
>>> is not informative enough for people not particularly familiar with
>>> the POWER8 IO hardware.
>>>
>>> This attempt aims to:
>>> 1. explain why we shift the resource
>>> 2. explain why nothing can use that hole as a resource while it is "free"
>>> (I am not sure that this is the case actually)
>>>
>>> Please comment, everyone, let's have this very well documented while
>>> I remember these bits :) Thanks.
>>> ---
>>>  arch/powerpc/platforms/powernv/pci-ioda.c | 10 +++++++---
>>>  1 file changed, 7 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
>>> index 48de308224d6..c4a36ae78c95 100644
>>> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
>>> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>>> @@ -1002,9 +1002,13 @@ static int pnv_pci_vf_resource_shift(struct pci_dev *dev, int offset)
>>>  	}
>>>  
>>>  	/*
>>> -	 * After doing so, there would be a "hole" in the /proc/iomem when
>>> -	 * offset is a positive value. It looks like the device return some
>>> -	 * mmio back to the system, which actually no one could use it.
>>> +	 * Since M64 BAR shares segments among all possible 256 PEs,
>>> +	 * we have to shift the beginning of PF IOV BAR to make it start from
>>> +	 * the segment which belongs to the PE number assigned to the first VF.
>>> +	 * This creates a "hole" in the /proc/iomem which could be used for
>>> +	 * allocating other resources, however this is not expected to happen
>>> +	 * on IODA as the only possibility would be a PCI hotplug and IODA
>>> +	 * hardware only allows it on a slot with dedicated PHB.
>>>  	 */
>>>  	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
>>>  		res = &dev->resource[i + PCI_IOV_RESOURCES];
>>>
>>
>>


-- 
Alexey

Re: [PATCH kernel] powerpc/powernv: Update comment about shifting IOV BAR

[Benjamin Herrenschmidt]

On Thu, 2017-09-14 at 13:18 +1000, Alexey Kardashevskiy wrote:
> On 14/09/17 13:07, Benjamin Herrenschmidt wrote:
> > On Thu, 2017-09-14 at 12:45 +1000, Alexey Kardashevskiy wrote:
> > > On 31/08/17 13:34, Alexey Kardashevskiy wrote:
> > > > From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > 
> > > Oops, this was not right :)
> > > 
> > > Anyway, Ben, please comment. Thanks.
> > 
> > This is incorrect, we can do hotplug behind switches afaik.
> 
> Do we have an actual system which allows this? 

Tuleta no ?

> Anyway, what we do now is
> wrong and it needs what? Reserve that hole? I'd like to update the comment
> for now, at least, and state what bad thing can happen and what we expect.

The hole should be reserved unless another SR-IOV device can use it ...

> 
> > > 
> > > > 
> > > > From: Alexey Kardashevskiy <aik@ozlabs.ru>
> > > > 
> > > > This updates the comment about creating a hole in /proc/iomem which
> > > > should not be normally happening but it does in the powernv platform
> > > > due the way MMIO M64 BARs are organised in the IODA2-capable hardware.
> > > > 
> > > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> > > > ---
> > > > 
> > > > It has been mentioned multiple times (last one -
> > > > https://www.spinics.net/lists/linux-pci/msg64084.html ) that the comment
> > > > is not informative enough for people not particularly familiar with
> > > > the POWER8 IO hardware.
> > > > 
> > > > This attempt aims to:
> > > > 1. explain why we shift the resource
> > > > 2. explain why nothing can use that hole as a resource while it is "free"
> > > > (I am not sure that this is the case actually)
> > > > 
> > > > Please comment, everyone, let's have this very well documented while
> > > > I remember these bits :) Thanks.
> > > > ---
> > > >  arch/powerpc/platforms/powernv/pci-ioda.c | 10 +++++++---
> > > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> > > > index 48de308224d6..c4a36ae78c95 100644
> > > > --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> > > > +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> > > > @@ -1002,9 +1002,13 @@ static int pnv_pci_vf_resource_shift(struct pci_dev *dev, int offset)
> > > >  	}
> > > >  
> > > >  	/*
> > > > -	 * After doing so, there would be a "hole" in the /proc/iomem when
> > > > -	 * offset is a positive value. It looks like the device return some
> > > > -	 * mmio back to the system, which actually no one could use it.
> > > > +	 * Since M64 BAR shares segments among all possible 256 PEs,
> > > > +	 * we have to shift the beginning of PF IOV BAR to make it start from
> > > > +	 * the segment which belongs to the PE number assigned to the first VF.
> > > > +	 * This creates a "hole" in the /proc/iomem which could be used for
> > > > +	 * allocating other resources, however this is not expected to happen
> > > > +	 * on IODA as the only possibility would be a PCI hotplug and IODA
> > > > +	 * hardware only allows it on a slot with dedicated PHB.
> > > >  	 */
> > > >  	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
> > > >  		res = &dev->resource[i + PCI_IOV_RESOURCES];
> > > > 
> > > 
> > > 
> 
> 

RE: [PATCH kernel] powerpc/powernv: Update comment about shifting IOV BAR

[David Laight]

RnJvbTogQmVuamFtaW4gSGVycmVuc2NobWlkdA0KPiBTZW50OiAxNCBTZXB0ZW1iZXIgMjAxNyAw
NDo0MA0KPiBPbiBUaHUsIDIwMTctMDktMTQgYXQgMTM6MTggKzEwMDAsIEFsZXhleSBLYXJkYXNo
ZXZza2l5IHdyb3RlOg0KPiA+IE9uIDE0LzA5LzE3IDEzOjA3LCBCZW5qYW1pbiBIZXJyZW5zY2ht
aWR0IHdyb3RlOg0KPiA+ID4gT24gVGh1LCAyMDE3LTA5LTE0IGF0IDEyOjQ1ICsxMDAwLCBBbGV4
ZXkgS2FyZGFzaGV2c2tpeSB3cm90ZToNCj4gPiA+ID4gT24gMzEvMDgvMTcgMTM6MzQsIEFsZXhl
eSBLYXJkYXNoZXZza2l5IHdyb3RlOg0KPiA+ID4gPiA+IEZyb206IEJlbmphbWluIEhlcnJlbnNj
aG1pZHQgPGJlbmhAa2VybmVsLmNyYXNoaW5nLm9yZz4NCj4gPiA+ID4NCj4gPiA+ID4gT29wcywg
dGhpcyB3YXMgbm90IHJpZ2h0IDopDQo+ID4gPiA+DQo+ID4gPiA+IEFueXdheSwgQmVuLCBwbGVh
c2UgY29tbWVudC4gVGhhbmtzLg0KPiA+ID4NCj4gPiA+IFRoaXMgaXMgaW5jb3JyZWN0LCB3ZSBj
YW4gZG8gaG90cGx1ZyBiZWhpbmQgc3dpdGNoZXMgYWZhaWsuDQo+ID4NCj4gPiBEbyB3ZSBoYXZl
IGFuIGFjdHVhbCBzeXN0ZW0gd2hpY2ggYWxsb3dzIHRoaXM/DQo+IA0KPiBUdWxldGEgbm8gPw0K
DQpZb3UgY2FuIGxvZ2ljYWxseSAnaG90cGx1ZycgUENJKGUpIG9uIGFueSBzeXN0ZW0gWzFdLg0K
DQpUaGUgJ3Byb2JsZW0nIGlzIHRoYXQgd2hhdGV2ZXIgZW51bWVyYXRlcyB0aGUgUENJKGUpIGF0
IHN5c3RlbQ0KcG93ZXJ1cCBkb2Vzbid0IG5vcm1hbGx5IGFzc2lnbiBleHRyYSByZXNvdXJjZXMg
dG8gYnJpZGdlcyB0byBhbGxvdw0KZm9yIGRldmljZXMgdGhhdCBhcmVuJ3QgcHJlc2VudCBhdCBi
b290IHRpbWUuDQpTbyB5b3UgY2FuIG5vcm1hbGx5IG9ubHkgcmVwbGFjZSBjYXJkcyB3aXRoIG9u
ZXMgdGhhdCB1c2UgdGhlIHNhbWUNCihvciBsZXNzKSByZXNvdXJjZXMsIG9yIHRoYXQgYXJlIG5v
dCBiZWhpbmQgYW55IGJyaWRnZXMuDQpUaGlzIGlzIHByb2JsZW1hdGljIGlmIHlvdSBoYXZlIGEg
ZG9ja2luZyBzdGF0aW9uIGNvbm5lY3RlZCB2aWENCmEgYnJpZGdlLg0KDQpbMV0gQXBhcnQgZnJv
bSBzb21lIGFubm95aW5nIHg4NiBEZWxsIHNlcnZlcnMgd2UgaGF2ZSB3aGljaCBnZW5lcmF0ZQ0K
YW4gTk1JIHdoZW4gdGhlIFBDSWUgbGluayBnb2VzIGRvd24gKHdoZW4gd2UgcmVwcm9ncmFtIHRo
ZSBmcGdhKS4NClRoZXkgYWxzbyBmYWlsIHRvIGJvb3QgaWYgYSBsaW5rIGRvZXNuJ3QgY29tZSB1
cC4uLg0KDQoJRGF2aWQNCg0K

Re: [PATCH kernel] powerpc/powernv: Update comment about shifting IOV BAR

[Benjamin Herrenschmidt]

On Thu, 2017-09-14 at 09:27 +0000, David Laight wrote:
> You can logically 'hotplug' PCI(e) on any system [1].
> 
> The 'problem' is that whatever enumerates the PCI(e) at system
> powerup doesn't normally assign extra resources to bridges to allow
> for devices that aren't present at boot time.
> So you can normally only replace cards with ones that use the same
> (or less) resources, or that are not behind any bridges.
> This is problematic if you have a docking station connected via
> a bridge.

There's also the problem of Max Payload Size. If you can hotplug behind
a bridge then the standard algorithm of finding the max of all devices
behind a host bridge doesn't work anymore and you have to clamp
everybody to 128 bytes.

> [1] Apart from some annoying x86 Dell servers we have which generate
> an NMI when the PCIe link goes down (when we reprogram the fpga).
> They also fail to boot if a link doesn't come up...
> 
>         David