[v2,1/3] PCI: endpoint: Handle 64-bit BARs properly

Message ID 20180208123346.20784-2-niklas.cassel@axis.com
State New
Headers show
Series
  • PCI endpoint 64-bit BAR fixes
Related show

Commit Message

Niklas Cassel Feb. 8, 2018, 12:33 p.m.
A 64-bit BAR uses the succeeding BAR for the upper bits, therefore
we cannot call pci_epc_set_bar() on a BAR that follows a 64-bit BAR.

If pci_epc_set_bar() is called with flag PCI_BASE_ADDRESS_MEM_TYPE_64,
it has to be up to the controller driver to write both BAR[x] and BAR[x+1]
(and BAR_mask[x] and BAR_mask[x+1]).

Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
 drivers/pci/endpoint/functions/pci-epf-test.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Kishon Vijay Abraham I Feb. 8, 2018, 12:47 p.m. | #1
Hi,

On Thursday 08 February 2018 06:03 PM, Niklas Cassel wrote:
> A 64-bit BAR uses the succeeding BAR for the upper bits, therefore
> we cannot call pci_epc_set_bar() on a BAR that follows a 64-bit BAR.
> 
> If pci_epc_set_bar() is called with flag PCI_BASE_ADDRESS_MEM_TYPE_64,

Not related to $patch. But I have a query on when PCI_BASE_ADDRESS_MEM_TYPE_64
should be set. Whether if the size is > 4G or if the address can be mapped
anywhere in the 64-bit PCIe address space or both?

Thanks
Kishon
> it has to be up to the controller driver to write both BAR[x] and BAR[x+1]
> (and BAR_mask[x] and BAR_mask[x+1]).
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
> ---
>  drivers/pci/endpoint/functions/pci-epf-test.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index 800da09d9005..eef85820f59e 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -382,6 +382,8 @@ static int pci_epf_test_set_bar(struct pci_epf *epf)
>  			if (bar == test_reg_bar)
>  				return ret;
>  		}
> +		if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64)
> +			bar++;
>  	}
>  
>  	return 0;
>
Niklas Cassel Feb. 8, 2018, 3:18 p.m. | #2
On Thu, Feb 08, 2018 at 06:17:32PM +0530, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Thursday 08 February 2018 06:03 PM, Niklas Cassel wrote:
> > A 64-bit BAR uses the succeeding BAR for the upper bits, therefore
> > we cannot call pci_epc_set_bar() on a BAR that follows a 64-bit BAR.
> > 
> > If pci_epc_set_bar() is called with flag PCI_BASE_ADDRESS_MEM_TYPE_64,
> 
> Not related to $patch. But I have a query on when PCI_BASE_ADDRESS_MEM_TYPE_64
> should be set. Whether if the size is > 4G or if the address can be mapped
> anywhere in the 64-bit PCIe address space or both?

Hello Kishon,

Since 32-bit BARs work fine on 64-bit CPUs,
and since 64-bit BARs work fine on 32 bit CPUs
(as long as we assign them an address <4G,
and their (combined) size is not too big),
perhaps the best way would be if pci-epf-test always defaults to 32-bit BARs,
and a module parameter says if we should test 64-bit BARs.

Just because a 64-bit BAR can be assigned an address >4G,
and have a size >4G, doesn't mean that we have to give it those properties.

This way we can have some testing of 64-bit BARs on 32-bit CPUs,
even though more extensive testing (e.g. having a BAR with a size >4G)
would require a 64-bit CPU.


Regards,
Niklas

> 
> Thanks
> Kishon
> > it has to be up to the controller driver to write both BAR[x] and BAR[x+1]
> > (and BAR_mask[x] and BAR_mask[x+1]).
> > 
> > Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
> > ---
> >  drivers/pci/endpoint/functions/pci-epf-test.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> > index 800da09d9005..eef85820f59e 100644
> > --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> > @@ -382,6 +382,8 @@ static int pci_epf_test_set_bar(struct pci_epf *epf)
> >  			if (bar == test_reg_bar)
> >  				return ret;
> >  		}
> > +		if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64)
> > +			bar++;
> >  	}
> >  
> >  	return 0;
> >
Bjorn Helgaas Feb. 8, 2018, 9:57 p.m. | #3
On Thu, Feb 08, 2018 at 06:17:32PM +0530, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Thursday 08 February 2018 06:03 PM, Niklas Cassel wrote:
> > A 64-bit BAR uses the succeeding BAR for the upper bits, therefore
> > we cannot call pci_epc_set_bar() on a BAR that follows a 64-bit BAR.
> > 
> > If pci_epc_set_bar() is called with flag PCI_BASE_ADDRESS_MEM_TYPE_64,
> 
> Not related to $patch. But I have a query on when
> PCI_BASE_ADDRESS_MEM_TYPE_64 should be set. Whether if the size is >
> 4G or if the address can be mapped anywhere in the 64-bit PCIe
> address space or both?

In general, PCI_BASE_ADDRESS_MEM_TYPE_64 should be set if the BAR is
64 bits wide.  IORESOURCE_MEM_64 is similar.

It doesn't matter what the current value of the BAR is.

It's easy to look at the current address or size of a resource if we
need to know where it is or how big it is.  But if we lose track of
the width of the BAR register, we have no way to know whether it's
*capable* of holding a 64-bit value.

> > it has to be up to the controller driver to write both BAR[x] and BAR[x+1]
> > (and BAR_mask[x] and BAR_mask[x+1]).
> > 
> > Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
> > ---
> >  drivers/pci/endpoint/functions/pci-epf-test.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> > index 800da09d9005..eef85820f59e 100644
> > --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> > @@ -382,6 +382,8 @@ static int pci_epf_test_set_bar(struct pci_epf *epf)
> >  			if (bar == test_reg_bar)
> >  				return ret;
> >  		}
> > +		if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64)
> > +			bar++;
> >  	}
> >  
> >  	return 0;
> >
Kishon Vijay Abraham I Feb. 9, 2018, 12:44 p.m. | #4
Hi Bjorn,

On Friday 09 February 2018 03:27 AM, Bjorn Helgaas wrote:
> On Thu, Feb 08, 2018 at 06:17:32PM +0530, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Thursday 08 February 2018 06:03 PM, Niklas Cassel wrote:
>>> A 64-bit BAR uses the succeeding BAR for the upper bits, therefore
>>> we cannot call pci_epc_set_bar() on a BAR that follows a 64-bit BAR.
>>>
>>> If pci_epc_set_bar() is called with flag PCI_BASE_ADDRESS_MEM_TYPE_64,
>>
>> Not related to $patch. But I have a query on when
>> PCI_BASE_ADDRESS_MEM_TYPE_64 should be set. Whether if the size is >
>> 4G or if the address can be mapped anywhere in the 64-bit PCIe
>> address space or both?
> 
> In general, PCI_BASE_ADDRESS_MEM_TYPE_64 should be set if the BAR is
> 64 bits wide.  IORESOURCE_MEM_64 is similar.

okay, if the HW support 64bit BAR, 64 bit flag should be set and not based on
size or anything else?

Thanks
Kishon
Lorenzo Pieralisi Feb. 13, 2018, 10:28 a.m. | #5
On Fri, Feb 09, 2018 at 06:14:49PM +0530, Kishon Vijay Abraham I wrote:
> Hi Bjorn,
> 
> On Friday 09 February 2018 03:27 AM, Bjorn Helgaas wrote:
> > On Thu, Feb 08, 2018 at 06:17:32PM +0530, Kishon Vijay Abraham I wrote:
> >> Hi,
> >>
> >> On Thursday 08 February 2018 06:03 PM, Niklas Cassel wrote:
> >>> A 64-bit BAR uses the succeeding BAR for the upper bits, therefore
> >>> we cannot call pci_epc_set_bar() on a BAR that follows a 64-bit BAR.
> >>>
> >>> If pci_epc_set_bar() is called with flag PCI_BASE_ADDRESS_MEM_TYPE_64,
> >>
> >> Not related to $patch. But I have a query on when
> >> PCI_BASE_ADDRESS_MEM_TYPE_64 should be set. Whether if the size is >
> >> 4G or if the address can be mapped anywhere in the 64-bit PCIe
> >> address space or both?
> > 
> > In general, PCI_BASE_ADDRESS_MEM_TYPE_64 should be set if the BAR is
> > 64 bits wide.  IORESOURCE_MEM_64 is similar.
> 
> okay, if the HW support 64bit BAR, 64 bit flag should be set and not based on
> size or anything else?

Yes, I completely agree with Bjorn. Actually it would be a good idea to
make the struct pci_epf->bar member array an array of struct resources
to simplify its handling.

Thanks,
Lorenzo

Patch

diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 800da09d9005..eef85820f59e 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -382,6 +382,8 @@  static int pci_epf_test_set_bar(struct pci_epf *epf)
 			if (bar == test_reg_bar)
 				return ret;
 		}
+		if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64)
+			bar++;
 	}
 
 	return 0;