2012-06-20 19:56:03

by Rao, Nikhil

[permalink] [raw]
Subject: [PATCH] pci: 64bit resource fix in setup-res.c

size parameter of _pci_assign_resource() needs to be
of type resource_size_t rather than int

Signed-off-by: Nikhil P Rao <[email protected]>
---
drivers/pci/setup-res.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index eea85da..9f6bbec 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -206,7 +206,8 @@ static int pci_revert_fw_address(struct resource *res, struct pci_dev *dev,
return ret;
}

-static int _pci_assign_resource(struct pci_dev *dev, int resno, int size, resource_size_t min_align)
+static int _pci_assign_resource(struct pci_dev *dev, int resno,
+ resource_size_t size, resource_size_t min_align)
{
struct resource *res = dev->resource + resno;
struct pci_bus *bus;
--
1.7.1



2012-06-21 23:48:00

by Rao, Nikhil

[permalink] [raw]
Subject: [PATCH] pci: support alignments upto 8Gb in pbus_size_mem()

I ran into the "disabling BAR .." error message when
trying to use a 8Gb PCIe card on a system with a BIOS
that didnt have support for BAR size > 2Gb. This patch fixed
the problem, but I also see the code reading the IORESOURCE_MEM_64
flag so I am not sure what the right solution is

Signed-off-by: Nikhil P Rao <[email protected]>
---
drivers/pci/setup-bus.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 8fa2d4b..3b3601f 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -780,7 +780,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
{
struct pci_dev *dev;
resource_size_t min_align, align, size, size0, size1;
- resource_size_t aligns[12]; /* Alignments from 1Mb to 2Gb */
+ resource_size_t aligns[14]; /* Alignments from 1Mb to 8Gb */
int order, max_order;
struct resource *b_res = find_free_bus_resource(bus, type);
unsigned int mem64_mask = 0;
@@ -819,7 +819,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
/* For bridges size != alignment */
align = pci_resource_alignment(dev, r);
order = __ffs(align) - 20;
- if (order > 11) {
+ if (order >= ARRAY_SIZE(aligns)) {
dev_warn(&dev->dev, "disabling BAR %d: %pR "
"(bad alignment %#llx)\n", i, r,
(unsigned long long) align);
--
1.7.1


2012-06-23 18:16:06

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH] pci: support alignments upto 8Gb in pbus_size_mem()

On Thu, Jun 21, 2012 at 5:47 PM, Nikhil P Rao <[email protected]> wrote:
> I ran into the "disabling BAR .." error message when
> trying to use a 8Gb PCIe card on a system with a BIOS
> that didnt have support for BAR size > 2Gb.

So the BIOS left the 8Gb BAR unassigned, and you got the "disabling
BAR ... (bad alignment)" message when Linux tried to enable it?

How do we know 8Gb is the correct new limit? Are we going to be
fixing this again when we see a 16Gb or a 32Gb BAR? Do we need a
better algorithm that doesn't have a limit like this?

This patch fixed
> the problem, but I also see the code reading the IORESOURCE_MEM_64
> flag so I am not sure what the right solution is
>
> Signed-off-by: Nikhil P Rao <[email protected]>
> ---
> ?drivers/pci/setup-bus.c | ? ?4 ++--
> ?1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 8fa2d4b..3b3601f 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -780,7 +780,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
> ?{
> ? ? ? ?struct pci_dev *dev;
> ? ? ? ?resource_size_t min_align, align, size, size0, size1;
> - ? ? ? resource_size_t aligns[12]; ? ? /* Alignments from 1Mb to 2Gb */
> + ? ? ? resource_size_t aligns[14]; ? ? /* Alignments from 1Mb to 8Gb */
> ? ? ? ?int order, max_order;
> ? ? ? ?struct resource *b_res = find_free_bus_resource(bus, type);
> ? ? ? ?unsigned int mem64_mask = 0;
> @@ -819,7 +819,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
> ? ? ? ? ? ? ? ? ? ? ? ?/* For bridges size != alignment */
> ? ? ? ? ? ? ? ? ? ? ? ?align = pci_resource_alignment(dev, r);
> ? ? ? ? ? ? ? ? ? ? ? ?order = __ffs(align) - 20;
> - ? ? ? ? ? ? ? ? ? ? ? if (order > 11) {
> + ? ? ? ? ? ? ? ? ? ? ? if (order >= ARRAY_SIZE(aligns)) {
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?dev_warn(&dev->dev, "disabling BAR %d: %pR "
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "(bad alignment %#llx)\n", i, r,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (unsigned long long) align);
> --
> 1.7.1
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html

2012-06-25 20:54:49

by Rao, Nikhil

[permalink] [raw]
Subject: Re: [PATCH] pci: support alignments upto 8Gb in pbus_size_mem()

On Sat, 2012-06-23 at 12:15 -0600, Bjorn Helgaas wrote:
> On Thu, Jun 21, 2012 at 5:47 PM, Nikhil P Rao <[email protected]> wrote:
> > I ran into the "disabling BAR .." error message when
> > trying to use a 8Gb PCIe card on a system with a BIOS
> > that didnt have support for BAR size > 2Gb.
>
> So the BIOS left the 8Gb BAR unassigned, and you got the "disabling
> BAR ... (bad alignment)" message when Linux tried to enable it?

Yes.

> How do we know 8Gb is the correct new limit? Are we going to be
> fixing this again when we see a 16Gb or a 32Gb BAR? Do we need a
> better algorithm that doesn't have a limit like this?
>

The original error message seems applicable to 32bit archs. and not to
64 bit archs. How about the patch below - is aligns[44] (256bytes more)
acceptable ?


From: Nikhil P Rao <[email protected]>
Date: Mon, 25 Jun 2012 13:33:55 -0700
Subject: [PATCH] pci: fix resource size check

Support a PCI BAR alignment of > 2Gb, the original check was
only applicable to 32 bit kernels,

Signed-off-by: Nikhil P Rao <[email protected]>
---
drivers/pci/setup-bus.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 8fa2d4b..9f8d9ea 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -780,7 +780,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
{
struct pci_dev *dev;
resource_size_t min_align, align, size, size0, size1;
- resource_size_t aligns[12]; /* Alignments from 1Mb to 2Gb */
+ resource_size_t aligns[44]; /* Alignments from 1Mb to 2^63 */
int order, max_order;
struct resource *b_res = find_free_bus_resource(bus, type);
unsigned int mem64_mask = 0;
@@ -819,7 +819,8 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
/* For bridges size != alignment */
align = pci_resource_alignment(dev, r);
order = __ffs(align) - 20;
- if (order > 11) {
+ if ((sizeof(size_t) == 4 && order > 11) ||
+ (sizeof(size_t) == 8 && order > 43)) {
dev_warn(&dev->dev, "disabling BAR %d: %pR "
"(bad alignment %#llx)\n", i, r,
(unsigned long long) align);
--
1.7.1