* [PATCH 0/3] PCI endpoint 64-bit BAR fixes
@ 2018-02-01 16:11 Niklas Cassel
2018-02-01 16:11 ` [PATCH 1/3] PCI: endpoint: Handle 64-bit BARs properly Niklas Cassel
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Niklas Cassel @ 2018-02-01 16:11 UTC (permalink / raw)
To: kishon, linux-pci; +Cc: Niklas Cassel, linux-kernel
PCI endpoint fixes to improve the way 64-bit BARs are handled.
There are still future improvements that could be made:
pci-epf-test.c always allocates space for
6 BARs, even when using 64-bit BARs (which
really only requires us to allocate 3 BARs).
pcitest.sh will print "NOT OKAY" for BAR1,
BAR3, and BAR5 when using 64-bit BARs.
This could probably be improved to say
something like "N/A (64-bit BAR)".
Niklas Cassel (3):
PCI: endpoint: Handle 64-bit BARs properly
misc: pci_endpoint_test: Handle 64-bit BARs properly
PCI: designware-ep: Return an error when requesting a too large BAR
size
drivers/misc/pci_endpoint_test.c | 2 ++
drivers/pci/dwc/pcie-designware-ep.c | 5 +++++
drivers/pci/endpoint/functions/pci-epf-test.c | 2 ++
3 files changed, 9 insertions(+)
--
2.14.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/3] PCI: endpoint: Handle 64-bit BARs properly
2018-02-01 16:11 [PATCH 0/3] PCI endpoint 64-bit BAR fixes Niklas Cassel
@ 2018-02-01 16:11 ` Niklas Cassel
2018-02-01 16:11 ` [PATCH 2/3] misc: pci_endpoint_test: " Niklas Cassel
2018-02-01 16:11 ` [PATCH 3/3] PCI: designware-ep: Return an error when requesting a too large BAR size Niklas Cassel
2 siblings, 0 replies; 9+ messages in thread
From: Niklas Cassel @ 2018-02-01 16:11 UTC (permalink / raw)
To: kishon, Lorenzo Pieralisi, Bjorn Helgaas, Sekhar Nori,
Cyrille Pitchen, Niklas Cassel, John Keeping, Shawn Lin
Cc: Niklas Cassel, linux-pci, linux-kernel
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(+)
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;
--
2.14.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] misc: pci_endpoint_test: Handle 64-bit BARs properly
2018-02-01 16:11 [PATCH 0/3] PCI endpoint 64-bit BAR fixes Niklas Cassel
2018-02-01 16:11 ` [PATCH 1/3] PCI: endpoint: Handle 64-bit BARs properly Niklas Cassel
@ 2018-02-01 16:11 ` Niklas Cassel
2018-02-01 16:11 ` [PATCH 3/3] PCI: designware-ep: Return an error when requesting a too large BAR size Niklas Cassel
2 siblings, 0 replies; 9+ messages in thread
From: Niklas Cassel @ 2018-02-01 16:11 UTC (permalink / raw)
To: kishon, Lorenzo Pieralisi, Arnd Bergmann, Greg Kroah-Hartman
Cc: Niklas Cassel, linux-pci, linux-kernel
A 64-bit BAR uses the succeeding BAR for the upper bits,
so we cannot simply call pci_ioremap_bar() on every single BAR.
Ignore BARs that does not have a valid resource length.
pci 0000:01:00.0: BAR 4: assigned [mem 0xc0300000-0xc031ffff 64bit]
pci 0000:01:00.0: BAR 2: assigned [mem 0xc0320000-0xc03203ff 64bit]
pci 0000:01:00.0: BAR 0: assigned [mem 0xc0320400-0xc03204ff 64bit]
pci-endpoint-test 0000:01:00.0: can't ioremap BAR 1: [??? 0x00000000 flags 0x0]
pci-endpoint-test 0000:01:00.0: failed to read BAR1
pci-endpoint-test 0000:01:00.0: can't ioremap BAR 3: [??? 0x00000000 flags 0x0]
pci-endpoint-test 0000:01:00.0: failed to read BAR3
pci-endpoint-test 0000:01:00.0: can't ioremap BAR 5: [??? 0x00000000 flags 0x0]
pci-endpoint-test 0000:01:00.0: failed to read BAR5
Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
Lorenzo/Bjorn: pci_resource_len() seems to fix my problem,
but is it the correct function to use here?
If BAR[x] is a 64-bit BAR, I'm assuming that pci_resource_len() on BAR[x+1]
will always return 0 (since BAR[x+1] cannot have any prefetchable/type bits
when BAR[x] is 64-bit).
drivers/misc/pci_endpoint_test.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
index 320276f42653..3af31bfdcfdd 100644
--- a/drivers/misc/pci_endpoint_test.c
+++ b/drivers/misc/pci_endpoint_test.c
@@ -534,6 +534,8 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev,
}
for (bar = BAR_0; bar <= BAR_5; bar++) {
+ if (pci_resource_len(pdev, bar) == 0)
+ continue;
base = pci_ioremap_bar(pdev, bar);
if (!base) {
dev_err(dev, "failed to read BAR%d\n", bar);
--
2.14.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] PCI: designware-ep: Return an error when requesting a too large BAR size
2018-02-01 16:11 [PATCH 0/3] PCI endpoint 64-bit BAR fixes Niklas Cassel
2018-02-01 16:11 ` [PATCH 1/3] PCI: endpoint: Handle 64-bit BARs properly Niklas Cassel
2018-02-01 16:11 ` [PATCH 2/3] misc: pci_endpoint_test: " Niklas Cassel
@ 2018-02-01 16:11 ` Niklas Cassel
2018-02-01 18:57 ` Andy Shevchenko
2 siblings, 1 reply; 9+ messages in thread
From: Niklas Cassel @ 2018-02-01 16:11 UTC (permalink / raw)
To: kishon, Jingoo Han, Joao Pinto, Lorenzo Pieralisi, Bjorn Helgaas
Cc: Niklas Cassel, linux-pci, linux-kernel
pci_epc_set_bar() can be called with flag PCI_BASE_ADDRESS_MEM_TYPE_64,
and can thus request a BAR size larger than 4 GB.
However, the pcie-designware-ep.c driver currently doesn't handle
BAR sizes larger than 4 GB. (Since we are only writing the BAR_mask[x]
register and not the BAR_mask[x+1] register.)
For now, return an error when requesting a BAR size larger than 4 GB.
Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
drivers/pci/dwc/pcie-designware-ep.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c
index 3a6feeff5f5b..4a0085ead1e3 100644
--- a/drivers/pci/dwc/pcie-designware-ep.c
+++ b/drivers/pci/dwc/pcie-designware-ep.c
@@ -126,6 +126,11 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no,
enum dw_pcie_as_type as_type;
u32 reg = PCI_BASE_ADDRESS_0 + (4 * bar);
+ if (size > 0x100000000ULL) {
+ dev_err(pci->dev, "can't handle BAR larger than 4GB\n");
+ return -EINVAL;
+ }
+
if (!(flags & PCI_BASE_ADDRESS_SPACE))
as_type = DW_PCIE_AS_MEM;
else
--
2.14.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] PCI: designware-ep: Return an error when requesting a too large BAR size
2018-02-01 16:11 ` [PATCH 3/3] PCI: designware-ep: Return an error when requesting a too large BAR size Niklas Cassel
@ 2018-02-01 18:57 ` Andy Shevchenko
2018-02-01 19:00 ` Jingoo Han
0 siblings, 1 reply; 9+ messages in thread
From: Andy Shevchenko @ 2018-02-01 18:57 UTC (permalink / raw)
To: Niklas Cassel
Cc: Kishon Vijay Abraham I, Jingoo Han, Joao Pinto,
Lorenzo Pieralisi, Bjorn Helgaas, Niklas Cassel, linux-pci,
Linux Kernel Mailing List
On Thu, Feb 1, 2018 at 6:11 PM, Niklas Cassel <niklas.cassel@axis.com> wrote:
include/linux/sizes.h:
+SZ_4G 0x100000000ULL
> + if (size > 0x100000000ULL) {
#include <linux/sizes.h>
if (size > SZ_4G) {
?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] PCI: designware-ep: Return an error when requesting a too large BAR size
2018-02-01 18:57 ` Andy Shevchenko
@ 2018-02-01 19:00 ` Jingoo Han
2018-02-05 16:25 ` Niklas Cassel
0 siblings, 1 reply; 9+ messages in thread
From: Jingoo Han @ 2018-02-01 19:00 UTC (permalink / raw)
To: 'Andy Shevchenko', 'Niklas Cassel'
Cc: 'Kishon Vijay Abraham I', 'Joao Pinto',
'Lorenzo Pieralisi', 'Bjorn Helgaas',
'Niklas Cassel',
linux-pci, 'Linux Kernel Mailing List'
On Thursday, February 1, 2018 1:58 PM, Andy Shevchenko wrote:
>
> On Thu, Feb 1, 2018 at 6:11 PM, Niklas Cassel <niklas.cassel@axis.com>
> wrote:
>
> include/linux/sizes.h:
>
> +SZ_4G 0x100000000ULL
>
> > + if (size > 0x100000000ULL) {
>
> #include <linux/sizes.h>
>
> if (size > SZ_4G) {
I like this one for the readability.
Thank you.
Best regards,
Jingoo Han
>
> ?
>
> --
> With Best Regards,
> Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] PCI: designware-ep: Return an error when requesting a too large BAR size
2018-02-01 19:00 ` Jingoo Han
@ 2018-02-05 16:25 ` Niklas Cassel
2018-02-06 19:38 ` Andy Shevchenko
0 siblings, 1 reply; 9+ messages in thread
From: Niklas Cassel @ 2018-02-05 16:25 UTC (permalink / raw)
To: Jingoo Han
Cc: 'Andy Shevchenko', 'Kishon Vijay Abraham I',
'Joao Pinto', 'Lorenzo Pieralisi',
'Bjorn Helgaas',
linux-pci, 'Linux Kernel Mailing List'
On Thu, Feb 01, 2018 at 02:00:40PM -0500, Jingoo Han wrote:
> On Thursday, February 1, 2018 1:58 PM, Andy Shevchenko wrote:
> >
> > On Thu, Feb 1, 2018 at 6:11 PM, Niklas Cassel <niklas.cassel@axis.com>
> > wrote:
> >
> > include/linux/sizes.h:
> >
> > +SZ_4G 0x100000000ULL
> >
> > > + if (size > 0x100000000ULL) {
> >
> > #include <linux/sizes.h>
> >
> > if (size > SZ_4G) {
>
> I like this one for the readability.
> Thank you.
>
I liked it too, however both variants
if (size > 0x100000000ULL) {
if (size > SZ_4G) {
result in:
drivers/pci/dwc/pcie-designware-ep.c:131:11: warning:
comparison is always false due to limited range of data type [-Wtype-limits]
when compiling with W=1 on a platform with 32-bit size_t.
The annoying thing here is that a BAR can be 64-bit,
yet the parameter size is defined as a size_t,
so the error will only show on 32-bit and not on 64-bit.
What do you think about:
if (upper_32_bits(size)) {
dev_err(pci->dev, "can't handle BAR larger than 4GB\n");
return -EINVAL;
}
That should compile without warnings for both
32-bit size_t and 64-bit size_t.
Regards,
Niklas
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] PCI: designware-ep: Return an error when requesting a too large BAR size
2018-02-05 16:25 ` Niklas Cassel
@ 2018-02-06 19:38 ` Andy Shevchenko
2018-02-08 12:33 ` Niklas Cassel
0 siblings, 1 reply; 9+ messages in thread
From: Andy Shevchenko @ 2018-02-06 19:38 UTC (permalink / raw)
To: Niklas Cassel
Cc: Jingoo Han, Kishon Vijay Abraham I, Joao Pinto,
Lorenzo Pieralisi, Bjorn Helgaas, linux-pci,
Linux Kernel Mailing List
On Mon, Feb 5, 2018 at 6:25 PM, Niklas Cassel <niklas.cassel@axis.com> wrote:
> On Thu, Feb 01, 2018 at 02:00:40PM -0500, Jingoo Han wrote:
>> On Thursday, February 1, 2018 1:58 PM, Andy Shevchenko wrote:
>> >
>> > On Thu, Feb 1, 2018 at 6:11 PM, Niklas Cassel <niklas.cassel@axis.com>
>> > wrote:
>> >
>> > include/linux/sizes.h:
>> >
>> > +SZ_4G 0x100000000ULL
>> >
>> > > + if (size > 0x100000000ULL) {
>> >
>> > #include <linux/sizes.h>
>> >
>> > if (size > SZ_4G) {
>>
>> I like this one for the readability.
>> Thank you.
>>
>
> I liked it too, however both variants
>
> if (size > 0x100000000ULL) {
>
> if (size > SZ_4G) {
>
> result in:
>
> drivers/pci/dwc/pcie-designware-ep.c:131:11: warning:
> comparison is always false due to limited range of data type [-Wtype-limits]
>
> when compiling with W=1 on a platform with 32-bit size_t.
>
>
> The annoying thing here is that a BAR can be 64-bit,
> yet the parameter size is defined as a size_t,
> so the error will only show on 32-bit and not on 64-bit.
Oh, indeed. And it looks moving to u64 or alike is not a solution
(because if would not describe real hardware in that case).
> What do you think about:
> if (upper_32_bits(size)) {
> dev_err(pci->dev, "can't handle BAR larger than 4GB\n");
> return -EINVAL;
> }
>
> That should compile without warnings for both
> 32-bit size_t and 64-bit size_t.
Can you derive some helper based on the code in __pci_read_base() code?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] PCI: designware-ep: Return an error when requesting a too large BAR size
2018-02-06 19:38 ` Andy Shevchenko
@ 2018-02-08 12:33 ` Niklas Cassel
0 siblings, 0 replies; 9+ messages in thread
From: Niklas Cassel @ 2018-02-08 12:33 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jingoo Han, Kishon Vijay Abraham I, Joao Pinto,
Lorenzo Pieralisi, Bjorn Helgaas, linux-pci,
Linux Kernel Mailing List
On Tue, Feb 06, 2018 at 09:38:09PM +0200, Andy Shevchenko wrote:
> On Mon, Feb 5, 2018 at 6:25 PM, Niklas Cassel <niklas.cassel@axis.com> wrote:
> > On Thu, Feb 01, 2018 at 02:00:40PM -0500, Jingoo Han wrote:
> >> On Thursday, February 1, 2018 1:58 PM, Andy Shevchenko wrote:
> >> >
> >> > On Thu, Feb 1, 2018 at 6:11 PM, Niklas Cassel <niklas.cassel@axis.com>
> >> > wrote:
> >> >
> >> > include/linux/sizes.h:
> >> >
> >> > +SZ_4G 0x100000000ULL
> >> >
> >> > > + if (size > 0x100000000ULL) {
> >> >
> >> > #include <linux/sizes.h>
> >> >
> >> > if (size > SZ_4G) {
> >>
> >> I like this one for the readability.
> >> Thank you.
> >>
> >
> > I liked it too, however both variants
> >
> > if (size > 0x100000000ULL) {
> >
> > if (size > SZ_4G) {
> >
> > result in:
> >
> > drivers/pci/dwc/pcie-designware-ep.c:131:11: warning:
> > comparison is always false due to limited range of data type [-Wtype-limits]
> >
> > when compiling with W=1 on a platform with 32-bit size_t.
> >
> >
> > The annoying thing here is that a BAR can be 64-bit,
> > yet the parameter size is defined as a size_t,
> > so the error will only show on 32-bit and not on 64-bit.
>
> Oh, indeed. And it looks moving to u64 or alike is not a solution
> (because if would not describe real hardware in that case).
>
> > What do you think about:
>
> > if (upper_32_bits(size)) {
> > dev_err(pci->dev, "can't handle BAR larger than 4GB\n");
> > return -EINVAL;
> > }
> >
> > That should compile without warnings for both
> > 32-bit size_t and 64-bit size_t.
>
> Can you derive some helper based on the code in __pci_read_base() code?
Hello Andy,
I guess that would be possible, however I think that
simply checking upper_32_bits(size) is simpler.
If someone is ever to fix dw_pcie_ep_set_bar()
so that it works with 64-bit BARs, the function will
probably need to check upper_32_bits(size) anyway.
Regards,
Niklas
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-02-08 12:33 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-01 16:11 [PATCH 0/3] PCI endpoint 64-bit BAR fixes Niklas Cassel
2018-02-01 16:11 ` [PATCH 1/3] PCI: endpoint: Handle 64-bit BARs properly Niklas Cassel
2018-02-01 16:11 ` [PATCH 2/3] misc: pci_endpoint_test: " Niklas Cassel
2018-02-01 16:11 ` [PATCH 3/3] PCI: designware-ep: Return an error when requesting a too large BAR size Niklas Cassel
2018-02-01 18:57 ` Andy Shevchenko
2018-02-01 19:00 ` Jingoo Han
2018-02-05 16:25 ` Niklas Cassel
2018-02-06 19:38 ` Andy Shevchenko
2018-02-08 12:33 ` Niklas Cassel
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.