Re: [PATCH 3/4] x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 30h-3fh) Processors v2

[Andy Shevchenko]

On Tue, Apr 25, 2017 at 4:19 PM, Christian König
 wrote:
> From: Christian König 
>
> Most BIOS don't enable this because of compatibility reasons.
>
> Manually enable a 64bit BAR of 64GB size so that we have
> enough room for PCI devices.

> +static void pci_amd_enable_64bit_bar(struct pci_dev *dev)
> +{

> +   uint32_t base, limit, high;

Is u32 or uint32_t used actually in this file?

> +   struct resource *res, *conflict;

> +   unsigned i;

unsigned int i;

Reversed tree order.

> +   for (i = 0; i < 8; ++i) {
> +   pci_read_config_dword(dev, 0x80 + i * 0x8, );
> +   pci_read_config_dword(dev, 0x180 + i * 0x4, );
> +
> +   /* Is this slot free? */
> +   if ((base & 0x3) == 0x0)
> +   break;
> +
> +   base >>= 8;
> +   base |= high << 24;
> +
> +   /* Abort if a slot already configures a 64bit BAR. */
> +   if (base > 0x1)
> +   return;
> +   }
> +   if (i == 8)
> +   return;
> +
> +   res = kzalloc(sizeof(*res), GFP_KERNEL);
> +   if (!res)
> +   return;
> +
> +   res->name = "PCI Bus :00";

> +   res->flags = IORESOURCE_PREFETCH | IORESOURCE_MEM |
> +   IORESOURCE_MEM_64 | IORESOURCE_WINDOW;

You are using this constant twice AFAICS, perhaps define it in generic
pci.h and re-use?

> +   res->start = 0x1;
> +   res->end = 0xfd - 1;

Perhaps you forgot ul / ull in many places in your code. Care to
compile for 32-bit and fix the warnings?

> +   dev_info(>dev, "adding root bus resource %pR\n", res);

Some of your messages I think are rather dev_dbg() than dev_info().

> +   base = ((res->start >> 8) & 0xff00) | 0x3;
> +   limit = ((res->end + 1) >> 8) & 0xff00;
> +   high = ((res->start >> 40) & 0xff) |
> +   res->end + 1) >> 40) & 0xff) << 16);

It would be nice to have the magics defined (just on top of this function) like

#define PCI_AMD_BAR_XXX_MASK GENMASK(...)
#define PCI_AMD_BAR_XXX_SHIFT nn

> +   pci_write_config_dword(dev, 0x180 + i * 0x4, high);
> +   pci_write_config_dword(dev, 0x84 + i * 0x8, limit);
> +   pci_write_config_dword(dev, 0x80 + i * 0x8, base);

Ditto for pos:s.

> +
> +   pci_bus_add_resource(dev->bus, res, 0);
> +}

-- 
With Best Regards,
Andy Shevchenko
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/4] x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 30h-3fh) Processors v2

[Alex Deucher]

On Tue, Apr 25, 2017 at 9:19 AM, Christian König
 wrote:
> From: Christian König 
>
> Most BIOS don't enable this because of compatibility reasons.
>
> Manually enable a 64bit BAR of 64GB size so that we have
> enough room for PCI devices.
>
> v2: style cleanups, increase size, add resource name, set correct flags,
> print message that windows was added
>
> Signed-off-by: Christian König 
> ---
>  arch/x86/pci/fixup.c | 53 
> 
>  1 file changed, 53 insertions(+)
>
> diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
> index 6d52b94..8d949c4 100644
> --- a/arch/x86/pci/fixup.c
> +++ b/arch/x86/pci/fixup.c
> @@ -571,3 +571,56 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2fc0, 
> pci_invalid_bar);
>  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6f60, pci_invalid_bar);
>  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fa0, pci_invalid_bar);
>  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fc0, pci_invalid_bar);
> +
> +static void pci_amd_enable_64bit_bar(struct pci_dev *dev)
> +{
> +   uint32_t base, limit, high;
> +   struct resource *res, *conflict;
> +   unsigned i;
> +
> +   for (i = 0; i < 8; ++i) {
> +   pci_read_config_dword(dev, 0x80 + i * 0x8, );
> +   pci_read_config_dword(dev, 0x180 + i * 0x4, );

Might be nice to define names for the register offsets.


> +
> +   /* Is this slot free? */
> +   if ((base & 0x3) == 0x0)
> +   break;
> +
> +   base >>= 8;
> +   base |= high << 24;
> +
> +   /* Abort if a slot already configures a 64bit BAR. */
> +   if (base > 0x1)
> +   return;
> +   }
> +   if (i == 8)
> +   return;
> +
> +   res = kzalloc(sizeof(*res), GFP_KERNEL);
> +   if (!res)
> +   return;
> +
> +   res->name = "PCI Bus :00";
> +   res->flags = IORESOURCE_PREFETCH | IORESOURCE_MEM |
> +   IORESOURCE_MEM_64 | IORESOURCE_WINDOW;
> +   res->start = 0x1;
> +   res->end = 0xfd - 1;
> +
> +   /* Just grab the free area behind system memory for this */
> +   while ((conflict = request_resource_conflict(_resource, res)))
> +   res->start = conflict->end + 1;
> +
> +   dev_info(>dev, "adding root bus resource %pR\n", res);
> +
> +   base = ((res->start >> 8) & 0xff00) | 0x3;
> +   limit = ((res->end + 1) >> 8) & 0xff00;
> +   high = ((res->start >> 40) & 0xff) |
> +   res->end + 1) >> 40) & 0xff) << 16);
> +
> +   pci_write_config_dword(dev, 0x180 + i * 0x4, high);
> +   pci_write_config_dword(dev, 0x84 + i * 0x8, limit);
> +   pci_write_config_dword(dev, 0x80 + i * 0x8, base);

Same here.  Either way:
Reviewed-by: Alex Deucher 

> +
> +   pci_bus_add_resource(dev->bus, res, 0);
> +}
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_AMD, 0x141b, pci_amd_enable_64bit_bar);
> --
> 2.7.4
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/4] x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 30h-3fh) Processors v2

[Christian König]

From: Christian König 

Most BIOS don't enable this because of compatibility reasons.

Manually enable a 64bit BAR of 64GB size so that we have
enough room for PCI devices.

v2: style cleanups, increase size, add resource name, set correct flags,
print message that windows was added

Signed-off-by: Christian König 
---
 arch/x86/pci/fixup.c | 53 
 1 file changed, 53 insertions(+)

diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index 6d52b94..8d949c4 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -571,3 +571,56 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2fc0, 
pci_invalid_bar);
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6f60, pci_invalid_bar);
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fa0, pci_invalid_bar);
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x6fc0, pci_invalid_bar);
+
+static void pci_amd_enable_64bit_bar(struct pci_dev *dev)
+{
+   uint32_t base, limit, high;
+   struct resource *res, *conflict;
+   unsigned i;
+
+   for (i = 0; i < 8; ++i) {
+   pci_read_config_dword(dev, 0x80 + i * 0x8, );
+   pci_read_config_dword(dev, 0x180 + i * 0x4, );
+
+   /* Is this slot free? */
+   if ((base & 0x3) == 0x0)
+   break;
+
+   base >>= 8;
+   base |= high << 24;
+
+   /* Abort if a slot already configures a 64bit BAR. */
+   if (base > 0x1)
+   return;
+   }
+   if (i == 8)
+   return;
+
+   res = kzalloc(sizeof(*res), GFP_KERNEL);
+   if (!res)
+   return;
+
+   res->name = "PCI Bus :00";
+   res->flags = IORESOURCE_PREFETCH | IORESOURCE_MEM |
+   IORESOURCE_MEM_64 | IORESOURCE_WINDOW;
+   res->start = 0x1;
+   res->end = 0xfd - 1;
+
+   /* Just grab the free area behind system memory for this */
+   while ((conflict = request_resource_conflict(_resource, res)))
+   res->start = conflict->end + 1;
+
+   dev_info(>dev, "adding root bus resource %pR\n", res);
+
+   base = ((res->start >> 8) & 0xff00) | 0x3;
+   limit = ((res->end + 1) >> 8) & 0xff00;
+   high = ((res->start >> 40) & 0xff) |
+   res->end + 1) >> 40) & 0xff) << 16);
+
+   pci_write_config_dword(dev, 0x180 + i * 0x4, high);
+   pci_write_config_dword(dev, 0x84 + i * 0x8, limit);
+   pci_write_config_dword(dev, 0x80 + i * 0x8, base);
+
+   pci_bus_add_resource(dev->bus, res, 0);
+}
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_AMD, 0x141b, pci_amd_enable_64bit_bar);
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel