2017-07-10 15:00:15

by Christian König

[permalink] [raw]
Subject: Resizeable PCI BAR support V8

Hi everyone,

This is the eighth incarnation of this set of patches. It enables device
drivers to resize and most likely also relocate the PCI BAR of devices
they manage to allow the CPU to access all of the device local memory at once.

This is very useful for GFX device drivers where the default PCI BAR is only
about 256MB in size for compatibility reasons, but the device easily have
multiple gigabyte of local memory.

Some changes since the last version:
1. Rebased on Linus current master tree, going to rebase on 4.13-rc1 as soon as that is out.
2. Some more documentation on how to use the new functionality in drivers.
3. Only re-enable decoding when everything looks good.
4. Rename the new amdgpu function to better match what it is doing.

Bjorn what's you'r feeling on getting this into 4.14?

Regards,
Christian.


2017-07-10 15:00:05

by Christian König

[permalink] [raw]
Subject: [PATCH v8 1/6] PCI: add a define for the PCI resource type mask v2

From: Christian König <[email protected]>

We use this mask multiple times in the bus setup.

v2: fix some style nit picks

Signed-off-by: Christian König <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
---
drivers/pci/pci.h | 3 +++
drivers/pci/setup-bus.c | 12 +++---------
2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 03e3d02..136a0f9 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -4,6 +4,9 @@
#define PCI_FIND_CAP_TTL 48

#define PCI_VSEC_ID_INTEL_TBT 0x1234 /* Thunderbolt */
+#define PCI_RES_TYPE_MASK \
+ (IORESOURCE_IO | IORESOURCE_MEM | IORESOURCE_PREFETCH |\
+ IORESOURCE_MEM_64)

extern const unsigned char pcie_link_speed[];

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 958da7d..37450d9 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1523,8 +1523,6 @@ static void pci_bridge_release_resources(struct pci_bus *bus,
{
struct pci_dev *dev = bus->self;
struct resource *r;
- unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
- IORESOURCE_PREFETCH | IORESOURCE_MEM_64;
unsigned old_flags = 0;
struct resource *b_res;
int idx = 1;
@@ -1567,7 +1565,7 @@ static void pci_bridge_release_resources(struct pci_bus *bus,
*/
release_child_resources(r);
if (!release_resource(r)) {
- type = old_flags = r->flags & type_mask;
+ type = old_flags = r->flags & PCI_RES_TYPE_MASK;
dev_printk(KERN_DEBUG, &dev->dev, "resource %d %pR released\n",
PCI_BRIDGE_RESOURCES + idx, r);
/* keep the old size */
@@ -1758,8 +1756,6 @@ void pci_assign_unassigned_root_bus_resources(struct pci_bus *bus)
enum release_type rel_type = leaf_only;
LIST_HEAD(fail_head);
struct pci_dev_resource *fail_res;
- unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
- IORESOURCE_PREFETCH | IORESOURCE_MEM_64;
int pci_try_num = 1;
enum enable_type enable_local;

@@ -1818,7 +1814,7 @@ void pci_assign_unassigned_root_bus_resources(struct pci_bus *bus)
*/
list_for_each_entry(fail_res, &fail_head, list)
pci_bus_release_bridge_resources(fail_res->dev->bus,
- fail_res->flags & type_mask,
+ fail_res->flags & PCI_RES_TYPE_MASK,
rel_type);

/* restore size and flags */
@@ -1862,8 +1858,6 @@ void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
LIST_HEAD(fail_head);
struct pci_dev_resource *fail_res;
int retval;
- unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
- IORESOURCE_PREFETCH | IORESOURCE_MEM_64;

again:
__pci_bus_size_bridges(parent, &add_list);
@@ -1889,7 +1883,7 @@ void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
*/
list_for_each_entry(fail_res, &fail_head, list)
pci_bus_release_bridge_resources(fail_res->dev->bus,
- fail_res->flags & type_mask,
+ fail_res->flags & PCI_RES_TYPE_MASK,
whole_subtree);

/* restore size and flags */
--
2.7.4

2017-07-10 15:00:09

by Christian König

[permalink] [raw]
Subject: [PATCH v8 3/6] PCI: add functionality for resizing resources v7

From: Christian König <[email protected]>

This allows device drivers to request resizing their BARs.

The function only tries to reprogram the windows of the bridge directly above
the requesting device and only the BAR of the same type (usually mem, 64bit,
prefetchable). This is done to make sure not to disturb other drivers by
changing the BARs of their devices.

Drivers should use the following sequence to resize their BARs:
1. Disable memory decoding of your device using the PCI cfg dword.
2. Use pci_release_resource() to release all BARs which can move during the
resize. Including the one you want to resize.
3. Call pci_resize_resource() for each BAR you want to resize.
4. Call pci_assign_unassigned_bus_resources() to reassign new locations
for all BARs which are not resized, but could move.
5. If everything worked as expected enable memory decoding in your device again
using the PCI cfg dword.

v2: rebase on changes in rbar support
v3: style cleanups, fail if memory decoding is enabled or resources
still allocated, resize all unused bridge BARs,
drop calling pci_reenable_device
v4: print resources before releasing them, style cleanups,
use pci_rbar_size_to_bytes, use PCI_RES_TYPE_MASK
v5: use next pointer to simplify loop
v6: move reassigning resources on error to driver side
v7: Document in the commit message how to use the new function.

Signed-off-by: Christian König <[email protected]>
---
drivers/pci/setup-bus.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++++
drivers/pci/setup-res.c | 58 +++++++++++++++++++++++++++++
include/linux/pci.h | 3 ++
3 files changed, 159 insertions(+)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 37450d9..03af25b 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1908,6 +1908,104 @@ void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
}
EXPORT_SYMBOL_GPL(pci_assign_unassigned_bridge_resources);

+int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type)
+{
+ struct pci_dev_resource *dev_res;
+ struct pci_dev *next;
+ LIST_HEAD(saved);
+ LIST_HEAD(added);
+ LIST_HEAD(failed);
+ unsigned int i;
+ int ret;
+
+ /* Walk to the root hub, releasing bridge BARs when possible */
+ next = bridge;
+ do {
+ bridge = next;
+ for (i = PCI_BRIDGE_RESOURCES; i < PCI_BRIDGE_RESOURCE_END;
+ i++) {
+ struct resource *res = &bridge->resource[i];
+
+ if ((res->flags ^ type) & PCI_RES_TYPE_MASK)
+ continue;
+
+ /* Ignore BARs which are still in use */
+ if (res->child)
+ continue;
+
+ ret = add_to_list(&saved, bridge, res, 0, 0);
+ if (ret)
+ goto cleanup;
+
+ dev_info(&bridge->dev, "BAR %d: releasing %pR\n",
+ i, res);
+
+ if (res->parent)
+ release_resource(res);
+ res->start = 0;
+ res->end = 0;
+ break;
+ }
+ if (i == PCI_BRIDGE_RESOURCE_END)
+ break;
+
+ next = bridge->bus ? bridge->bus->self : NULL;
+ } while (next);
+
+ if (list_empty(&saved))
+ return -ENOENT;
+
+ __pci_bus_size_bridges(bridge->subordinate, &added);
+ __pci_bridge_assign_resources(bridge, &added, &failed);
+ BUG_ON(!list_empty(&added));
+
+ if (!list_empty(&failed)) {
+ ret = -ENOSPC;
+ goto cleanup;
+ }
+
+ list_for_each_entry(dev_res, &saved, list) {
+ /* Skip the bridge we just assigned resources for. */
+ if (bridge == dev_res->dev)
+ continue;
+
+ bridge = dev_res->dev;
+ pci_setup_bridge(bridge->subordinate);
+ }
+
+ free_list(&saved);
+ return 0;
+
+cleanup:
+ /* restore size and flags */
+ list_for_each_entry(dev_res, &failed, list) {
+ struct resource *res = dev_res->res;
+
+ res->start = dev_res->start;
+ res->end = dev_res->end;
+ res->flags = dev_res->flags;
+ }
+ free_list(&failed);
+
+ /* Revert to the old configuration */
+ list_for_each_entry(dev_res, &saved, list) {
+ struct resource *res = dev_res->res;
+
+ bridge = dev_res->dev;
+ i = res - bridge->resource;
+
+ res->start = dev_res->start;
+ res->end = dev_res->end;
+ res->flags = dev_res->flags;
+
+ pci_claim_resource(bridge, i);
+ pci_setup_bridge(bridge->subordinate);
+ }
+ free_list(&saved);
+
+ return ret;
+}
+
void pci_assign_unassigned_bus_resources(struct pci_bus *bus)
{
struct pci_dev *dev;
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index 85774b7..1d558e1 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -383,6 +383,64 @@ int pci_reassign_resource(struct pci_dev *dev, int resno, resource_size_t addsiz
return 0;
}

+void pci_release_resource(struct pci_dev *dev, int resno)
+{
+ struct resource *res = dev->resource + resno;
+
+ dev_info(&dev->dev, "BAR %d: releasing %pR\n", resno, res);
+ release_resource(res);
+ res->end = resource_size(res) - 1;
+ res->start = 0;
+ res->flags |= IORESOURCE_UNSET;
+}
+EXPORT_SYMBOL(pci_release_resource);
+
+int pci_resize_resource(struct pci_dev *dev, int resno, int size)
+{
+ struct resource *res = dev->resource + resno;
+ int old, ret;
+ u32 sizes;
+ u16 cmd;
+
+ /* Make sure the resource isn't assigned before resizing it. */
+ if (!(res->flags & IORESOURCE_UNSET))
+ return -EBUSY;
+
+ pci_read_config_word(dev, PCI_COMMAND, &cmd);
+ if (cmd & PCI_COMMAND_MEMORY)
+ return -EBUSY;
+
+ sizes = pci_rbar_get_possible_sizes(dev, resno);
+ if (!sizes)
+ return -ENOTSUPP;
+
+ if (!(sizes & BIT(size)))
+ return -EINVAL;
+
+ old = pci_rbar_get_current_size(dev, resno);
+ if (old < 0)
+ return old;
+
+ ret = pci_rbar_set_size(dev, resno, size);
+ if (ret)
+ return ret;
+
+ res->end = res->start + pci_rbar_size_to_bytes(size) - 1;
+
+ /* Check if the new config works by trying to assign everything. */
+ ret = pci_reassign_bridge_resources(dev->bus->self, res->flags);
+ if (ret)
+ goto error_resize;
+
+ return 0;
+
+error_resize:
+ pci_rbar_set_size(dev, resno, old);
+ res->end = res->start + pci_rbar_size_to_bytes(old) - 1;
+ return ret;
+}
+EXPORT_SYMBOL(pci_resize_resource);
+
int pci_enable_resources(struct pci_dev *dev, int mask)
{
u16 cmd, old_cmd;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 4869e66..7e486e3 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1080,6 +1080,8 @@ void pci_reset_bridge_secondary_bus(struct pci_dev *dev);
void pci_update_resource(struct pci_dev *dev, int resno);
int __must_check pci_assign_resource(struct pci_dev *dev, int i);
int __must_check pci_reassign_resource(struct pci_dev *dev, int i, resource_size_t add_size, resource_size_t align);
+void pci_release_resource(struct pci_dev *dev, int resno);
+int __must_check pci_resize_resource(struct pci_dev *dev, int i, int size);
int pci_select_bars(struct pci_dev *dev, unsigned long flags);
bool pci_device_is_present(struct pci_dev *pdev);
void pci_ignore_hotplug(struct pci_dev *dev);
@@ -1158,6 +1160,7 @@ void pci_assign_unassigned_resources(void);
void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge);
void pci_assign_unassigned_bus_resources(struct pci_bus *bus);
void pci_assign_unassigned_root_bus_resources(struct pci_bus *bus);
+int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type);
void pdev_enable_device(struct pci_dev *);
int pci_enable_resources(struct pci_dev *, int mask);
void pci_fixup_irqs(u8 (*)(struct pci_dev *, u8 *),
--
2.7.4

2017-07-10 15:00:17

by Christian König

[permalink] [raw]
Subject: [PATCH v8 6/6] drm/amdgpu: resize VRAM BAR for CPU access v4

From: Christian König <[email protected]>

Try to resize BAR0 to let CPU access all of VRAM.

v2: rebased, style cleanups, disable mem decode before resize,
handle gmc_v9 as well, round size up to power of two.
v3: handle gmc_v6 as well, release and reassign all BARs in the driver.
v4: rename new function to amdgpu_device_resize_fb_bar,
reenable mem decoding only if all resources are assigned.

Signed-off-by: Christian König <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 46 ++++++++++++++++++++++++++++++
drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 8 ++++--
drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 8 ++++--
drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 8 ++++--
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 10 ++++---
6 files changed, 68 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index ff7bf1a..5809e0e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1944,6 +1944,7 @@ uint64_t amdgpu_ttm_tt_pte_flags(struct amdgpu_device *adev, struct ttm_tt *ttm,
struct ttm_mem_reg *mem);
void amdgpu_vram_location(struct amdgpu_device *adev, struct amdgpu_mc *mc, u64 base);
void amdgpu_gtt_location(struct amdgpu_device *adev, struct amdgpu_mc *mc);
+void amdgpu_device_resize_fb_bar(struct amdgpu_device *adev);
void amdgpu_ttm_set_active_vram_size(struct amdgpu_device *adev, u64 size);
int amdgpu_ttm_init(struct amdgpu_device *adev);
void amdgpu_ttm_fini(struct amdgpu_device *adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 7d1a04a..a5123f5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -428,6 +428,9 @@ static int amdgpu_doorbell_init(struct amdgpu_device *adev)
return 0;
}

+ if (pci_resource_flags(adev->pdev, 2) & IORESOURCE_UNSET)
+ return -EINVAL;
+
/* doorbell bar mapping */
adev->doorbell.base = pci_resource_start(adev->pdev, 2);
adev->doorbell.size = pci_resource_len(adev->pdev, 2);
@@ -709,6 +712,49 @@ void amdgpu_gtt_location(struct amdgpu_device *adev, struct amdgpu_mc *mc)
mc->gtt_size >> 20, mc->gtt_start, mc->gtt_end);
}

+/**
+ * amdgpu_device_resize_fb_bar - try to resize FB BAR
+ *
+ * @adev: amdgpu_device pointer
+ *
+ * Try to resize FB BAR to make all VRAM CPU accessible.
+ */
+void amdgpu_device_resize_fb_bar(struct amdgpu_device *adev)
+{
+ u64 space_needed = roundup_pow_of_two(adev->mc.real_vram_size);
+ u32 rbar_size = order_base_2(((space_needed >> 20) | 1)) - 1;
+ u16 cmd;
+ int r;
+
+ /* Disable memory decoding while we change the BAR addresses and size */
+ pci_read_config_word(adev->pdev, PCI_COMMAND, &cmd);
+ pci_write_config_word(adev->pdev, PCI_COMMAND,
+ cmd & ~PCI_COMMAND_MEMORY);
+
+ /* Free the VRAM and doorbell BAR, we most likely need to move both. */
+ amdgpu_doorbell_fini(adev);
+ pci_release_resource(adev->pdev, 0);
+ if (adev->asic_type >= CHIP_BONAIRE)
+ pci_release_resource(adev->pdev, 2);
+
+ r = pci_resize_resource(adev->pdev, 0, rbar_size);
+ if (r == -ENOSPC)
+ DRM_INFO("Not enough PCI address space for a large BAR.");
+ else if (r && r != -ENOTSUPP)
+ DRM_ERROR("Problem resizing BAR0 (%d).", r);
+
+ pci_assign_unassigned_bus_resources(adev->pdev->bus);
+
+ /* When the doorbell or fb BAR isn't available we have no chance of
+ * using the device.
+ */
+ r = amdgpu_doorbell_init(adev);
+ if ((pci_resource_flags(adev->pdev, 0) & IORESOURCE_UNSET) || r)
+ BUG();
+ else
+ pci_write_config_word(adev->pdev, PCI_COMMAND, cmd);
+}
+
/*
* GPU helpers function.
*/
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
index d0214d9..89bb2e4 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
@@ -334,12 +334,14 @@ static int gmc_v6_0_mc_init(struct amdgpu_device *adev)
break;
}
adev->mc.vram_width = numchan * chansize;
- /* Could aper size report 0 ? */
- adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
- adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
/* size in MB on si */
adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL * 1024ULL;
adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL * 1024ULL;
+
+ if (!(adev->flags & AMD_IS_APU))
+ amdgpu_device_resize_fb_bar(adev);
+ adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
+ adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
adev->mc.visible_vram_size = adev->mc.aper_size;

/* unless the user had overridden it, set the gart
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
index 7e9ea53..91d1b0c 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
@@ -372,13 +372,15 @@ static int gmc_v7_0_mc_init(struct amdgpu_device *adev)
}
adev->mc.vram_width = numchan * chansize;
}
- /* Could aper size report 0 ? */
- adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
- adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
/* size in MB on si */
adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL * 1024ULL;
adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL * 1024ULL;

+ if (!(adev->flags & AMD_IS_APU))
+ amdgpu_device_resize_fb_bar(adev);
+ adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
+ adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
+
#ifdef CONFIG_X86_64
if (adev->flags & AMD_IS_APU) {
adev->mc.aper_base = ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22;
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
index cc9f880..cd04d59 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -534,13 +534,15 @@ static int gmc_v8_0_mc_init(struct amdgpu_device *adev)
}
adev->mc.vram_width = numchan * chansize;
}
- /* Could aper size report 0 ? */
- adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
- adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
/* size in MB on si */
adev->mc.mc_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL * 1024ULL;
adev->mc.real_vram_size = RREG32(mmCONFIG_MEMSIZE) * 1024ULL * 1024ULL;

+ if (!(adev->flags & AMD_IS_APU))
+ amdgpu_device_resize_fb_bar(adev);
+ adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
+ adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
+
#ifdef CONFIG_X86_64
if (adev->flags & AMD_IS_APU) {
adev->mc.aper_base = ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22;
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 175ba5f..99fb632 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -480,17 +480,19 @@ static int gmc_v9_0_mc_init(struct amdgpu_device *adev)
}
adev->mc.vram_width = numchan * chansize;

- /* Could aper size report 0 ? */
- adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
- adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
/* size in MB on si */
adev->mc.mc_vram_size =
((adev->flags & AMD_IS_APU) ? nbio_v7_0_get_memsize(adev) :
nbio_v6_1_get_memsize(adev)) * 1024ULL * 1024ULL;
adev->mc.real_vram_size = adev->mc.mc_vram_size;
- adev->mc.visible_vram_size = adev->mc.aper_size;
+
+ if (!(adev->flags & AMD_IS_APU))
+ amdgpu_device_resize_fb_bar(adev);
+ adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
+ adev->mc.aper_size = pci_resource_len(adev->pdev, 0);

/* In case the PCI BAR is larger than the actual amount of vram */
+ adev->mc.visible_vram_size = adev->mc.aper_size;
if (adev->mc.visible_vram_size > adev->mc.real_vram_size)
adev->mc.visible_vram_size = adev->mc.real_vram_size;

--
2.7.4

2017-07-10 15:00:50

by Christian König

[permalink] [raw]
Subject: [PATCH v8 4/6] x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 30h-3fh) Processors v4

From: Christian König <[email protected]>

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
v3: add defines for all the magic numbers, style cleanups
v4: add some comment that the BIOS should actually allow this using
_PRS and _SRS.

Signed-off-by: Christian König <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
---
arch/x86/pci/fixup.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 76 insertions(+)

diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index 11e4074..0f4088b 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -618,3 +618,79 @@ static void quirk_apple_mbp_poweroff(struct pci_dev *pdev)
dev_info(dev, "can't work around MacBook Pro poweroff issue\n");
}
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x8c10, quirk_apple_mbp_poweroff);
+
+#define AMD_141b_MMIO_BASE(x) (0x80 + (x) * 0x8)
+#define AMD_141b_MMIO_BASE_RE_MASK BIT(0)
+#define AMD_141b_MMIO_BASE_WE_MASK BIT(1)
+#define AMD_141b_MMIO_BASE_MMIOBASE_MASK GENMASK(31,8)
+
+#define AMD_141b_MMIO_LIMIT(x) (0x84 + (x) * 0x8)
+#define AMD_141b_MMIO_LIMIT_MMIOLIMIT_MASK GENMASK(31,8)
+
+#define AMD_141b_MMIO_HIGH(x) (0x180 + (x) * 0x4)
+#define AMD_141b_MMIO_HIGH_MMIOBASE_MASK GENMASK(7,0)
+#define AMD_141b_MMIO_HIGH_MMIOLIMIT_SHIFT 16
+#define AMD_141b_MMIO_HIGH_MMIOLIMIT_MASK GENMASK(23,16)
+
+/*
+ * The PCI Firmware Spec, rev 3.2 notes that ACPI should optionally allow
+ * configuring host bridge windows using the _PRS and _SRS methods.
+ *
+ * But this is rarely implemented, so we manually enable a large 64bit BAR for
+ * PCIe device on AMD Family 15h (Models 30h-3fh) Processors here.
+ */
+static void pci_amd_enable_64bit_bar(struct pci_dev *dev)
+{
+ struct resource *res, *conflict;
+ u32 base, limit, high;
+ unsigned i;
+
+ for (i = 0; i < 8; ++i) {
+ pci_read_config_dword(dev, AMD_141b_MMIO_BASE(i), &base);
+ pci_read_config_dword(dev, AMD_141b_MMIO_HIGH(i), &high);
+
+ /* Is this slot free? */
+ if (!(base & (AMD_141b_MMIO_BASE_RE_MASK |
+ AMD_141b_MMIO_BASE_WE_MASK)))
+ break;
+
+ base >>= 8;
+ base |= high << 24;
+
+ /* Abort if a slot already configures a 64bit BAR. */
+ if (base > 0x10000)
+ return;
+ }
+ if (i == 8)
+ return;
+
+ res = kzalloc(sizeof(*res), GFP_KERNEL);
+ if (!res)
+ return;
+
+ res->name = "PCI Bus 0000:00";
+ res->flags = IORESOURCE_PREFETCH | IORESOURCE_MEM |
+ IORESOURCE_MEM_64 | IORESOURCE_WINDOW;
+ res->start = 0x100000000ull;
+ res->end = 0xfd00000000ull - 1;
+
+ /* Just grab the free area behind system memory for this */
+ while ((conflict = request_resource_conflict(&iomem_resource, res)))
+ res->start = conflict->end + 1;
+
+ dev_info(&dev->dev, "adding root bus resource %pR\n", res);
+
+ base = ((res->start >> 8) & AMD_141b_MMIO_BASE_MMIOBASE_MASK) |
+ AMD_141b_MMIO_BASE_RE_MASK | AMD_141b_MMIO_BASE_WE_MASK;
+ limit = ((res->end + 1) >> 8) & AMD_141b_MMIO_LIMIT_MMIOLIMIT_MASK;
+ high = ((res->start >> 40) & AMD_141b_MMIO_HIGH_MMIOBASE_MASK) |
+ ((((res->end + 1) >> 40) << AMD_141b_MMIO_HIGH_MMIOLIMIT_SHIFT)
+ & AMD_141b_MMIO_HIGH_MMIOLIMIT_MASK);
+
+ pci_write_config_dword(dev, AMD_141b_MMIO_HIGH(i), high);
+ pci_write_config_dword(dev, AMD_141b_MMIO_LIMIT(i), limit);
+ pci_write_config_dword(dev, AMD_141b_MMIO_BASE(i), 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

2017-07-10 15:01:08

by Christian König

[permalink] [raw]
Subject: [PATCH v8 5/6] drm/amdgpu: move hw generation check into amdgpu_doorbell_init v2

From: Christian König <[email protected]>

This way we can safely call it on SI as well.

v2: fix type in commit message

Signed-off-by: Christian König <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 4a8fc15..7d1a04a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -419,6 +419,15 @@ void amdgpu_pci_config_reset(struct amdgpu_device *adev)
*/
static int amdgpu_doorbell_init(struct amdgpu_device *adev)
{
+ /* No doorbell on SI hardware generation */
+ if (adev->asic_type < CHIP_BONAIRE) {
+ adev->doorbell.base = 0;
+ adev->doorbell.size = 0;
+ adev->doorbell.num_doorbells = 0;
+ adev->doorbell.ptr = NULL;
+ return 0;
+ }
+
/* doorbell bar mapping */
adev->doorbell.base = pci_resource_start(adev->pdev, 2);
adev->doorbell.size = pci_resource_len(adev->pdev, 2);
@@ -2099,9 +2108,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
DRM_INFO("register mmio base: 0x%08X\n", (uint32_t)adev->rmmio_base);
DRM_INFO("register mmio size: %u\n", (unsigned)adev->rmmio_size);

- if (adev->asic_type >= CHIP_BONAIRE)
- /* doorbell bar mapping */
- amdgpu_doorbell_init(adev);
+ /* doorbell bar mapping */
+ amdgpu_doorbell_init(adev);

/* io port mapping */
for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
@@ -2307,8 +2315,7 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
adev->rio_mem = NULL;
iounmap(adev->rmmio);
adev->rmmio = NULL;
- if (adev->asic_type >= CHIP_BONAIRE)
- amdgpu_doorbell_fini(adev);
+ amdgpu_doorbell_fini(adev);
amdgpu_debugfs_regs_cleanup(adev);
}

--
2.7.4

2017-07-10 15:01:38

by Christian König

[permalink] [raw]
Subject: [PATCH v8 2/6] PCI: add resizeable BAR infrastructure v5

From: Christian König <[email protected]>

Just the defines and helper functions to read the possible sizes of a BAR and
update it's size.

See https://pcisig.com/sites/default/files/specification_documents/ECN_Resizable-BAR_24Apr2008.pdf
and PCIe r3.1, sec 7.22.

This is useful for hardware with large local storage (mostly GFX) which only
expose 256MB BARs initially to be compatible with 32bit systems.

v2: provide read helper as well
v3: improve function names, use unsigned values, add better comments.
v4: move definition, improve commit message, s/bar/BAR/
v5: split out helper to find ctrl reg pos, style fixes, comment fixes,
add pci_rbar_size_to_bytes as well

Signed-off-by: Christian König <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
---
drivers/pci/pci.c | 104 ++++++++++++++++++++++++++++++++++++++++++
drivers/pci/pci.h | 8 ++++
include/uapi/linux/pci_regs.h | 11 ++++-
3 files changed, 121 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index d88edf5..647b799 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2957,6 +2957,110 @@ bool pci_acs_path_enabled(struct pci_dev *start,
}

/**
+ * pci_rbar_find_pos - find position of resize ctrl reg for BAR
+ * @dev: PCI device
+ * @bar: BAR to find
+ *
+ * Helper to find the postion of the ctrl register for a BAR.
+ * Returns -ENOTSUPP of resizeable BARs are not supported at all.
+ * Returns -ENOENT if not ctrl register for the BAR could be found.
+ */
+static int pci_rbar_find_pos(struct pci_dev *pdev, int bar)
+{
+ unsigned int pos, nbars;
+ unsigned int i;
+ u32 ctrl;
+
+ pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
+ if (!pos)
+ return -ENOTSUPP;
+
+ pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
+ nbars = (ctrl & PCI_REBAR_CTRL_NBAR_MASK) >> PCI_REBAR_CTRL_NBAR_SHIFT;
+
+ for (i = 0; i < nbars; ++i, pos += 8) {
+ int bar_idx;
+
+ pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
+ bar_idx = (ctrl & PCI_REBAR_CTRL_BAR_IDX_MASK) >>
+ PCI_REBAR_CTRL_BAR_IDX_SHIFT;
+ if (bar_idx == bar)
+ return pos;
+ }
+
+ return -ENOENT;
+}
+
+/**
+ * pci_rbar_get_possible_sizes - get possible sizes for BAR
+ * @dev: PCI device
+ * @bar: BAR to query
+ *
+ * Get the possible sizes of a resizeable BAR as bitmask defined in the spec
+ * (bit 0=1MB, bit 19=512GB). Returns 0 if BAR isn't resizeable.
+ */
+u32 pci_rbar_get_possible_sizes(struct pci_dev *pdev, int bar)
+{
+ u32 cap;
+ int pos;
+
+ pos = pci_rbar_find_pos(pdev, bar);
+ if (pos < 0)
+ return 0;
+
+ pci_read_config_dword(pdev, pos + PCI_REBAR_CAP, &cap);
+ return (cap & PCI_REBAR_CTRL_SIZES_MASK) >>
+ PCI_REBAR_CTRL_SIZES_SHIFT;
+}
+
+/**
+ * pci_rbar_get_current_size - get the current size of a BAR
+ * @dev: PCI device
+ * @bar: BAR to set size to
+ *
+ * Read the size of a BAR from the resizeable BAR config.
+ * Returns size if found or negative error code.
+ */
+int pci_rbar_get_current_size(struct pci_dev *pdev, int bar)
+{
+ u32 ctrl;
+ int pos;
+
+ pos = pci_rbar_find_pos(pdev, bar);
+ if (pos < 0)
+ return pos;
+
+ pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
+ return (ctrl & PCI_REBAR_CTRL_BAR_SIZE_MASK) >>
+ PCI_REBAR_CTRL_BAR_SIZE_SHIFT;
+}
+
+/**
+ * pci_rbar_set_size - set a new size for a BAR
+ * @dev: PCI device
+ * @bar: BAR to set size to
+ * @size: new size as defined in the spec (0=1MB, 19=512GB)
+ *
+ * Set the new size of a BAR as defined in the spec.
+ * Returns zero if resizing was successful, error code otherwise.
+ */
+int pci_rbar_set_size(struct pci_dev *pdev, int bar, int size)
+{
+ u32 ctrl;
+ int pos;
+
+ pos = pci_rbar_find_pos(pdev, bar);
+ if (pos < 0)
+ return pos;
+
+ pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
+ ctrl &= ~PCI_REBAR_CTRL_BAR_SIZE_MASK;
+ ctrl |= size << PCI_REBAR_CTRL_BAR_SIZE_SHIFT;
+ pci_write_config_dword(pdev, pos + PCI_REBAR_CTRL, ctrl);
+ return 0;
+}
+
+/**
* pci_swizzle_interrupt_pin - swizzle INTx for device behind bridge
* @dev: the PCI device
* @pin: the INTx pin (1=INTA, 2=INTB, 3=INTC, 4=INTD)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 136a0f9..e494f1e 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -367,4 +367,12 @@ int acpi_get_rc_resources(struct device *dev, const char *hid, u16 segment,
struct resource *res);
#endif

+u32 pci_rbar_get_possible_sizes(struct pci_dev *pdev, int bar);
+int pci_rbar_get_current_size(struct pci_dev *pdev, int bar);
+int pci_rbar_set_size(struct pci_dev *pdev, int bar, int size);
+static inline u64 pci_rbar_size_to_bytes(int size)
+{
+ return 1ULL << (size + 20);
+}
+
#endif /* DRIVERS_PCI_H */
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index c22d3eb..d6a075d 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -943,9 +943,16 @@
#define PCI_SATA_SIZEOF_LONG 16

/* Resizable BARs */
+#define PCI_REBAR_CAP 4 /* capability register */
+#define PCI_REBAR_CTRL_SIZES_MASK (0xFFFFF << 4) /* mask for sizes */
+#define PCI_REBAR_CTRL_SIZES_SHIFT 4 /* shift for sizes */
#define PCI_REBAR_CTRL 8 /* control register */
-#define PCI_REBAR_CTRL_NBAR_MASK (7 << 5) /* mask for # bars */
-#define PCI_REBAR_CTRL_NBAR_SHIFT 5 /* shift for # bars */
+#define PCI_REBAR_CTRL_BAR_IDX_MASK (7 << 0) /* mask for BAR index */
+#define PCI_REBAR_CTRL_BAR_IDX_SHIFT 0 /* shift for BAR index */
+#define PCI_REBAR_CTRL_NBAR_MASK (7 << 5) /* mask for # BARs */
+#define PCI_REBAR_CTRL_NBAR_SHIFT 5 /* shift for # BARs */
+#define PCI_REBAR_CTRL_BAR_SIZE_MASK (0x1F << 8) /* mask for BAR size */
+#define PCI_REBAR_CTRL_BAR_SIZE_SHIFT 8 /* shift for BAR size */

/* Dynamic Power Allocation */
#define PCI_DPA_CAP 4 /* capability register */
--
2.7.4

2017-07-10 21:47:27

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v8 1/6] PCI: add a define for the PCI resource type mask v2

If you post this again, you can drop the "v2", "v5", etc at the end of
the subject lines. I don't think it's useful to merge those.

On Mon, Jul 10, 2017 at 04:59:49PM +0200, Christian K?nig wrote:
> From: Christian K?nig <[email protected]>
>
> We use this mask multiple times in the bus setup.
>
> v2: fix some style nit picks

And you can put these "v2: ..." notes after the "---" because I don't
think they're useful after merging either.

> Signed-off-by: Christian K?nig <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
> ---
> drivers/pci/pci.h | 3 +++
> drivers/pci/setup-bus.c | 12 +++---------
> 2 files changed, 6 insertions(+), 9 deletions(-)

2017-07-10 22:23:41

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v8 6/6] drm/amdgpu: resize VRAM BAR for CPU access v4

On Mon, Jul 10, 2017 at 04:59:54PM +0200, Christian K?nig wrote:
> From: Christian K?nig <[email protected]>
>
> Try to resize BAR0 to let CPU access all of VRAM.
>
> v2: rebased, style cleanups, disable mem decode before resize,
> handle gmc_v9 as well, round size up to power of two.
> v3: handle gmc_v6 as well, release and reassign all BARs in the driver.
> v4: rename new function to amdgpu_device_resize_fb_bar,
> reenable mem decoding only if all resources are assigned.
>
> Signed-off-by: Christian K?nig <[email protected]>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 46 ++++++++++++++++++++++++++++++
> drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 8 ++++--
> drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 8 ++++--
> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 8 ++++--
> drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 10 ++++---
> 6 files changed, 68 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index ff7bf1a..5809e0e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1944,6 +1944,7 @@ uint64_t amdgpu_ttm_tt_pte_flags(struct amdgpu_device *adev, struct ttm_tt *ttm,
> struct ttm_mem_reg *mem);
> void amdgpu_vram_location(struct amdgpu_device *adev, struct amdgpu_mc *mc, u64 base);
> void amdgpu_gtt_location(struct amdgpu_device *adev, struct amdgpu_mc *mc);
> +void amdgpu_device_resize_fb_bar(struct amdgpu_device *adev);
> void amdgpu_ttm_set_active_vram_size(struct amdgpu_device *adev, u64 size);
> int amdgpu_ttm_init(struct amdgpu_device *adev);
> void amdgpu_ttm_fini(struct amdgpu_device *adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 7d1a04a..a5123f5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -428,6 +428,9 @@ static int amdgpu_doorbell_init(struct amdgpu_device *adev)
> return 0;
> }
>
> + if (pci_resource_flags(adev->pdev, 2) & IORESOURCE_UNSET)
> + return -EINVAL;
> +
> /* doorbell bar mapping */
> adev->doorbell.base = pci_resource_start(adev->pdev, 2);
> adev->doorbell.size = pci_resource_len(adev->pdev, 2);
> @@ -709,6 +712,49 @@ void amdgpu_gtt_location(struct amdgpu_device *adev, struct amdgpu_mc *mc)
> mc->gtt_size >> 20, mc->gtt_start, mc->gtt_end);
> }
>
> +/**
> + * amdgpu_device_resize_fb_bar - try to resize FB BAR
> + *
> + * @adev: amdgpu_device pointer
> + *
> + * Try to resize FB BAR to make all VRAM CPU accessible.
> + */
> +void amdgpu_device_resize_fb_bar(struct amdgpu_device *adev)
> +{
> + u64 space_needed = roundup_pow_of_two(adev->mc.real_vram_size);
> + u32 rbar_size = order_base_2(((space_needed >> 20) | 1)) - 1;
> + u16 cmd;
> + int r;
> +
> + /* Disable memory decoding while we change the BAR addresses and size */
> + pci_read_config_word(adev->pdev, PCI_COMMAND, &cmd);
> + pci_write_config_word(adev->pdev, PCI_COMMAND,
> + cmd & ~PCI_COMMAND_MEMORY);
> +
> + /* Free the VRAM and doorbell BAR, we most likely need to move both. */
> + amdgpu_doorbell_fini(adev);
> + pci_release_resource(adev->pdev, 0);
> + if (adev->asic_type >= CHIP_BONAIRE)
> + pci_release_resource(adev->pdev, 2);

The related pieces should be together, e.g.,

amdgpu_doorbell_fini(adev);
if (adev->asic_type >= CHIP_BONAIRE)
pci_release_resource(adev->pdev, 2);

pci_release_resource(adev->pdev, 0);

> + r = pci_resize_resource(adev->pdev, 0, rbar_size);
> + if (r == -ENOSPC)
> + DRM_INFO("Not enough PCI address space for a large BAR.");
> + else if (r && r != -ENOTSUPP)
> + DRM_ERROR("Problem resizing BAR0 (%d).", r);
> +
> + pci_assign_unassigned_bus_resources(adev->pdev->bus);
> +
> + /* When the doorbell or fb BAR isn't available we have no chance of
> + * using the device.
> + */
> + r = amdgpu_doorbell_init(adev);
> + if ((pci_resource_flags(adev->pdev, 0) & IORESOURCE_UNSET) || r)
> + BUG();

Seems extreme to panic the system in this case.

I don't think IORESOURCE_UNSET should appear in a driver. I think that's
an internal property that should be handled in the core.

What if you added something like this:

int pci_enable_mem_decoding(struct pci_dev *dev)
{
unsigned int bars = 0;

for (i = 0; i <= PCI_ROM_RESOURCE; i++)
if (dev->resource[i].flags & IORESOURCE_MEM)
bars |= (1 << i);

return pci_enable_resources(pdev, bars);
}

> + else
> + pci_write_config_word(adev->pdev, PCI_COMMAND, cmd);
> +}

2017-07-11 12:07:59

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v8 4/6] x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 30h-3fh) Processors v4

Hi Christian,

[auto build test WARNING on pci/next]
[also build test WARNING on next-20170710]
[cannot apply to v4.12]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Christian-K-nig/PCI-add-a-define-for-the-PCI-resource-type-mask-v2/20170711-104904
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386

All warnings (new ones prefixed by >>):

arch/x86/pci/fixup.c: In function 'pci_amd_enable_64bit_bar':
>> arch/x86/pci/fixup.c:674:15: warning: large integer implicitly truncated to unsigned type [-Woverflow]
res->start = 0x100000000ull;
^~~~~~~~~~~~~~
arch/x86/pci/fixup.c:675:13: warning: large integer implicitly truncated to unsigned type [-Woverflow]
res->end = 0xfd00000000ull - 1;
^~~~~~~~~~~~~~~
>> arch/x86/pci/fixup.c:686:22: warning: right shift count >= width of type [-Wshift-count-overflow]
high = ((res->start >> 40) & AMD_141b_MMIO_HIGH_MMIOBASE_MASK) |
^~
arch/x86/pci/fixup.c:687:21: warning: right shift count >= width of type [-Wshift-count-overflow]
((((res->end + 1) >> 40) << AMD_141b_MMIO_HIGH_MMIOLIMIT_SHIFT)
^~

vim +674 arch/x86/pci/fixup.c

634
635 /*
636 * The PCI Firmware Spec, rev 3.2 notes that ACPI should optionally allow
637 * configuring host bridge windows using the _PRS and _SRS methods.
638 *
639 * But this is rarely implemented, so we manually enable a large 64bit BAR for
640 * PCIe device on AMD Family 15h (Models 30h-3fh) Processors here.
641 */
642 static void pci_amd_enable_64bit_bar(struct pci_dev *dev)
643 {
644 struct resource *res, *conflict;
645 u32 base, limit, high;
646 unsigned i;
647
648 for (i = 0; i < 8; ++i) {
649 pci_read_config_dword(dev, AMD_141b_MMIO_BASE(i), &base);
650 pci_read_config_dword(dev, AMD_141b_MMIO_HIGH(i), &high);
651
652 /* Is this slot free? */
653 if (!(base & (AMD_141b_MMIO_BASE_RE_MASK |
654 AMD_141b_MMIO_BASE_WE_MASK)))
655 break;
656
657 base >>= 8;
658 base |= high << 24;
659
660 /* Abort if a slot already configures a 64bit BAR. */
661 if (base > 0x10000)
662 return;
663 }
664 if (i == 8)
665 return;
666
667 res = kzalloc(sizeof(*res), GFP_KERNEL);
668 if (!res)
669 return;
670
671 res->name = "PCI Bus 0000:00";
672 res->flags = IORESOURCE_PREFETCH | IORESOURCE_MEM |
673 IORESOURCE_MEM_64 | IORESOURCE_WINDOW;
> 674 res->start = 0x100000000ull;
675 res->end = 0xfd00000000ull - 1;
676
677 /* Just grab the free area behind system memory for this */
678 while ((conflict = request_resource_conflict(&iomem_resource, res)))
679 res->start = conflict->end + 1;
680
681 dev_info(&dev->dev, "adding root bus resource %pR\n", res);
682
683 base = ((res->start >> 8) & AMD_141b_MMIO_BASE_MMIOBASE_MASK) |
684 AMD_141b_MMIO_BASE_RE_MASK | AMD_141b_MMIO_BASE_WE_MASK;
685 limit = ((res->end + 1) >> 8) & AMD_141b_MMIO_LIMIT_MMIOLIMIT_MASK;
> 686 high = ((res->start >> 40) & AMD_141b_MMIO_HIGH_MMIOBASE_MASK) |

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (3.54 kB)
.config.gz (59.42 kB)
Download all attachments

2017-07-11 12:39:49

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v8 2/6] PCI: add resizeable BAR infrastructure v5

Hi Christian,

[auto build test WARNING on pci/next]
[also build test WARNING on v4.12 next-20170710]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Christian-K-nig/PCI-add-a-define-for-the-PCI-resource-type-mask-v2/20170711-104904
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick (https://www.imagemagick.org)
arch/x86/include/asm/uaccess_32.h:1: warning: no structured comments found
include/linux/init.h:1: warning: no structured comments found
include/linux/mod_devicetable.h:687: warning: Excess struct/union/enum/typedef member 'ver_major' description in 'fsl_mc_device_id'
include/linux/mod_devicetable.h:687: warning: Excess struct/union/enum/typedef member 'ver_minor' description in 'fsl_mc_device_id'
kernel/sched/core.c:2080: warning: No description found for parameter 'rf'
kernel/sched/core.c:2080: warning: Excess function parameter 'cookie' description in 'try_to_wake_up_local'
include/linux/wait.h:555: warning: No description found for parameter 'wq'
include/linux/wait.h:555: warning: Excess function parameter 'wq_head' description in 'wait_event_interruptible_hrtimeout'
include/linux/wait.h:759: warning: No description found for parameter 'wq_head'
include/linux/wait.h:759: warning: Excess function parameter 'wq' description in 'wait_event_killable'
include/linux/kthread.h:26: warning: Excess function parameter '...' description in 'kthread_create'
kernel/sys.c:1: warning: no structured comments found
Error: Cannot open file kernel/rcu/srcu.c
Error: Cannot open file kernel/rcu/srcu.c
WARNING: kernel-doc 'scripts/kernel-doc -rst -enable-lineno -export kernel/rcu/srcu.c' failed with return code 2
include/linux/device.h:968: warning: No description found for parameter 'dma_ops'
drivers/dma-buf/seqno-fence.c:1: warning: no structured comments found
include/linux/iio/iio.h:603: warning: No description found for parameter 'trig_readonly'
include/linux/iio/trigger.h:151: warning: No description found for parameter 'indio_dev'
include/linux/iio/trigger.h:151: warning: No description found for parameter 'trig'
include/linux/device.h:969: warning: No description found for parameter 'dma_ops'
drivers/ata/libata-eh.c:1449: warning: No description found for parameter 'link'
drivers/ata/libata-eh.c:1449: warning: Excess function parameter 'ap' description in 'ata_eh_done'
drivers/ata/libata-eh.c:1590: warning: No description found for parameter 'qc'
drivers/ata/libata-eh.c:1590: warning: Excess function parameter 'dev' description in 'ata_eh_request_sense'
>> drivers/pci/pci.c:2970: warning: No description found for parameter 'pdev'
>> drivers/pci/pci.c:2970: warning: Excess function parameter 'dev' description in 'pci_rbar_find_pos'
drivers/pci/pci.c:3004: warning: No description found for parameter 'pdev'
>> drivers/pci/pci.c:3004: warning: Excess function parameter 'dev' description in 'pci_rbar_get_possible_sizes'
drivers/pci/pci.c:3026: warning: No description found for parameter 'pdev'
>> drivers/pci/pci.c:3026: warning: Excess function parameter 'dev' description in 'pci_rbar_get_current_size'
drivers/pci/pci.c:3049: warning: No description found for parameter 'pdev'
>> drivers/pci/pci.c:3049: warning: Excess function parameter 'dev' description in 'pci_rbar_set_size'
arch/s390/include/asm/cmb.h:1: warning: no structured comments found
drivers/scsi/scsi_lib.c:1116: warning: No description found for parameter 'rq'
drivers/scsi/constants.c:1: warning: no structured comments found
include/linux/usb/gadget.h:230: warning: No description found for parameter 'claimed'
include/linux/usb/gadget.h:230: warning: No description found for parameter 'enabled'
include/linux/usb/gadget.h:412: warning: No description found for parameter 'quirk_altset_not_supp'
include/linux/usb/gadget.h:412: warning: No description found for parameter 'quirk_stall_not_supp'
include/linux/usb/gadget.h:412: warning: No description found for parameter 'quirk_zlp_not_supp'
fs/inode.c:1666: warning: No description found for parameter 'rcu'
include/linux/jbd2.h:443: warning: No description found for parameter 'i_transaction'
include/linux/jbd2.h:443: warning: No description found for parameter 'i_next_transaction'
include/linux/jbd2.h:443: warning: No description found for parameter 'i_list'
include/linux/jbd2.h:443: warning: No description found for parameter 'i_vfs_inode'
include/linux/jbd2.h:443: warning: No description found for parameter 'i_flags'
include/linux/jbd2.h:497: warning: No description found for parameter 'h_rsv_handle'
include/linux/jbd2.h:497: warning: No description found for parameter 'h_reserved'
include/linux/jbd2.h:497: warning: No description found for parameter 'h_type'
include/linux/jbd2.h:497: warning: No description found for parameter 'h_line_no'
include/linux/jbd2.h:497: warning: No description found for parameter 'h_start_jiffies'
include/linux/jbd2.h:497: warning: No description found for parameter 'h_requested_credits'
include/linux/jbd2.h:497: warning: No description found for parameter 'saved_alloc_context'
include/linux/jbd2.h:1050: warning: No description found for parameter 'j_chkpt_bhs'
include/linux/jbd2.h:1050: warning: No description found for parameter 'j_devname'
include/linux/jbd2.h:1050: warning: No description found for parameter 'j_average_commit_time'
include/linux/jbd2.h:1050: warning: No description found for parameter 'j_min_batch_time'
include/linux/jbd2.h:1050: warning: No description found for parameter 'j_max_batch_time'
include/linux/jbd2.h:1050: warning: No description found for parameter 'j_commit_callback'
include/linux/jbd2.h:1050: warning: No description found for parameter 'j_failed_commit'
include/linux/jbd2.h:1050: warning: No description found for parameter 'j_chksum_driver'
include/linux/jbd2.h:1050: warning: No description found for parameter 'j_csum_seed'
fs/jbd2/transaction.c:511: warning: No description found for parameter 'type'
fs/jbd2/transaction.c:511: warning: No description found for parameter 'line_no'
fs/jbd2/transaction.c:641: warning: No description found for parameter 'gfp_mask'
include/drm/drm_drv.h:537: warning: No description found for parameter 'set_busid'
include/drm/drm_drv.h:537: warning: No description found for parameter 'debugfs_init'
include/drm/drm_drv.h:537: warning: No description found for parameter 'gem_open_object'
include/drm/drm_drv.h:537: warning: No description found for parameter 'gem_close_object'
include/drm/drm_drv.h:537: warning: No description found for parameter 'prime_handle_to_fd'
include/drm/drm_drv.h:537: warning: No description found for parameter 'prime_fd_to_handle'
include/drm/drm_drv.h:537: warning: No description found for parameter 'gem_prime_export'
include/drm/drm_drv.h:537: warning: No description found for parameter 'gem_prime_import'
include/drm/drm_drv.h:537: warning: No description found for parameter 'gem_prime_pin'
include/drm/drm_drv.h:537: warning: No description found for parameter 'gem_prime_unpin'
include/drm/drm_drv.h:537: warning: No description found for parameter 'gem_prime_res_obj'
include/drm/drm_drv.h:537: warning: No description found for parameter 'gem_prime_get_sg_table'
include/drm/drm_drv.h:537: warning: No description found for parameter 'gem_prime_import_sg_table'
include/drm/drm_drv.h:537: warning: No description found for parameter 'gem_prime_vmap'
include/drm/drm_drv.h:537: warning: No description found for parameter 'gem_prime_vunmap'
include/drm/drm_drv.h:537: warning: No description found for parameter 'gem_prime_mmap'
include/drm/drm_drv.h:537: warning: No description found for parameter 'gem_vm_ops'
include/drm/drm_drv.h:537: warning: No description found for parameter 'major'
include/drm/drm_drv.h:537: warning: No description found for parameter 'minor'
include/drm/drm_drv.h:537: warning: No description found for parameter 'patchlevel'
include/drm/drm_drv.h:537: warning: No description found for parameter 'name'
include/drm/drm_drv.h:537: warning: No description found for parameter 'desc'
include/drm/drm_drv.h:537: warning: No description found for parameter 'date'
include/drm/drm_drv.h:537: warning: No description found for parameter 'driver_features'
include/drm/drm_drv.h:537: warning: No description found for parameter 'ioctls'
include/drm/drm_drv.h:537: warning: No description found for parameter 'num_ioctls'
include/drm/drm_drv.h:537: warning: No description found for parameter 'fops'
include/drm/drm_color_mgmt.h:1: warning: no structured comments found
drivers/gpu/drm/drm_syncobj.c:341: warning: Excess function parameter 'dev' description in 'drm_syncobj_open'
drivers/gpu/drm/drm_syncobj.c:366: warning: Excess function parameter 'dev' description in 'drm_syncobj_release'
include/drm/drm_syncobj.h:1: warning: no structured comments found
drivers/gpu/drm/drm_syncobj.c:342: warning: Excess function parameter 'dev' description in 'drm_syncobj_open'
drivers/gpu/drm/drm_syncobj.c:367: warning: Excess function parameter 'dev' description in 'drm_syncobj_release'
drivers/gpu/host1x/bus.c:50: warning: Excess function parameter 'driver' description in 'host1x_subdev_add'
include/net/sock.h:479: warning: No description found for parameter 'sk_tsq_flags'
include/net/sock.h:479: warning: No description found for parameter '__sk_flags_offset'
include/net/sock.h:479: warning: No description found for parameter 'sk_uid'
Documentation/doc-guide/sphinx.rst:121: ERROR: Unknown target name: "sphinx c domain".
kernel/sched/fair.c:7584: WARNING: Inline emphasis start-string without end-string.
kernel/time/timer.c:1200: ERROR: Unexpected indentation.
kernel/time/timer.c:1202: ERROR: Unexpected indentation.
kernel/time/timer.c:1203: WARNING: Block quote ends without a blank line; unexpected unindent.
include/linux/wait.h:108: WARNING: Block quote ends without a blank line; unexpected unindent.
include/linux/wait.h:111: ERROR: Unexpected indentation.
include/linux/wait.h:113: WARNING: Block quote ends without a blank line; unexpected unindent.
kernel/time/hrtimer.c:991: WARNING: Block quote ends without a blank line; unexpected unindent.
kernel/signal.c:323: WARNING: Inline literal start-string without end-string.
kernel/rcu/tree.c:3187: ERROR: Unexpected indentation.
kernel/rcu/tree.c:3214: ERROR: Unexpected indentation.
kernel/rcu/tree.c:3215: WARNING: Bullet list ends without a blank line; unexpected unindent.
include/linux/iio/iio.h:219: ERROR: Unexpected indentation.
include/linux/iio/iio.h:220: WARNING: Block quote ends without a blank line; unexpected unindent.
include/linux/iio/iio.h:226: WARNING: Definition list ends without a blank line; unexpected unindent.
drivers/iio/industrialio-core.c:633: ERROR: Unknown target name: "iio_val".
drivers/iio/industrialio-core.c:640: ERROR: Unknown target name: "iio_val".
drivers/ata/libata-core.c:5906: ERROR: Unknown target name: "hw".
drivers/message/fusion/mptbase.c:5051: WARNING: Definition list ends without a blank line; unexpected unindent.
drivers/tty/serial/serial_core.c:1897: WARNING: Definition list ends without a blank line; unexpected unindent.
drivers/pci/pci.c:3574: ERROR: Unexpected indentation.
include/linux/regulator/driver.h:271: ERROR: Unknown target name: "regulator_regmap_x_voltage".
include/linux/spi/spi.h:373: ERROR: Unexpected indentation.
drivers/w1/w1_io.c:196: WARNING: Definition list ends without a blank line; unexpected unindent.
block/bio.c:407: ERROR: Unknown target name: "gfp".
drivers/gpu/drm/drm_scdc_helper.c:203: ERROR: Unexpected indentation.
drivers/gpu/drm/drm_scdc_helper.c:204: WARNING: Block quote ends without a blank line; unexpected unindent.
drivers/gpu/drm/drm_ioctl.c:702: WARNING: Definition list ends without a blank line; unexpected unindent.
Documentation/gpu/todo.rst:111: ERROR: Unknown target name: "drm_fb".

vim +/pdev +2970 drivers/pci/pci.c

2958
2959 /**
2960 * pci_rbar_find_pos - find position of resize ctrl reg for BAR
2961 * @dev: PCI device
2962 * @bar: BAR to find
2963 *
2964 * Helper to find the postion of the ctrl register for a BAR.
2965 * Returns -ENOTSUPP of resizeable BARs are not supported at all.
2966 * Returns -ENOENT if not ctrl register for the BAR could be found.
2967 */
2968 static int pci_rbar_find_pos(struct pci_dev *pdev, int bar)
2969 {
> 2970 unsigned int pos, nbars;
2971 unsigned int i;
2972 u32 ctrl;
2973
2974 pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
2975 if (!pos)
2976 return -ENOTSUPP;
2977
2978 pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
2979 nbars = (ctrl & PCI_REBAR_CTRL_NBAR_MASK) >> PCI_REBAR_CTRL_NBAR_SHIFT;
2980
2981 for (i = 0; i < nbars; ++i, pos += 8) {
2982 int bar_idx;
2983
2984 pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
2985 bar_idx = (ctrl & PCI_REBAR_CTRL_BAR_IDX_MASK) >>
2986 PCI_REBAR_CTRL_BAR_IDX_SHIFT;
2987 if (bar_idx == bar)
2988 return pos;
2989 }
2990
2991 return -ENOENT;
2992 }
2993
2994 /**
2995 * pci_rbar_get_possible_sizes - get possible sizes for BAR
2996 * @dev: PCI device
2997 * @bar: BAR to query
2998 *
2999 * Get the possible sizes of a resizeable BAR as bitmask defined in the spec
3000 * (bit 0=1MB, bit 19=512GB). Returns 0 if BAR isn't resizeable.
3001 */
3002 u32 pci_rbar_get_possible_sizes(struct pci_dev *pdev, int bar)
3003 {
> 3004 u32 cap;
3005 int pos;
3006
3007 pos = pci_rbar_find_pos(pdev, bar);
3008 if (pos < 0)
3009 return 0;
3010
3011 pci_read_config_dword(pdev, pos + PCI_REBAR_CAP, &cap);
3012 return (cap & PCI_REBAR_CTRL_SIZES_MASK) >>
3013 PCI_REBAR_CTRL_SIZES_SHIFT;
3014 }
3015
3016 /**
3017 * pci_rbar_get_current_size - get the current size of a BAR
3018 * @dev: PCI device
3019 * @bar: BAR to set size to
3020 *
3021 * Read the size of a BAR from the resizeable BAR config.
3022 * Returns size if found or negative error code.
3023 */
3024 int pci_rbar_get_current_size(struct pci_dev *pdev, int bar)
3025 {
> 3026 u32 ctrl;
3027 int pos;
3028
3029 pos = pci_rbar_find_pos(pdev, bar);
3030 if (pos < 0)
3031 return pos;
3032
3033 pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
3034 return (ctrl & PCI_REBAR_CTRL_BAR_SIZE_MASK) >>
3035 PCI_REBAR_CTRL_BAR_SIZE_SHIFT;
3036 }
3037
3038 /**
3039 * pci_rbar_set_size - set a new size for a BAR
3040 * @dev: PCI device
3041 * @bar: BAR to set size to
3042 * @size: new size as defined in the spec (0=1MB, 19=512GB)
3043 *
3044 * Set the new size of a BAR as defined in the spec.
3045 * Returns zero if resizing was successful, error code otherwise.
3046 */
3047 int pci_rbar_set_size(struct pci_dev *pdev, int bar, int size)
3048 {
> 3049 u32 ctrl;
3050 int pos;
3051
3052 pos = pci_rbar_find_pos(pdev, bar);
3053 if (pos < 0)
3054 return pos;
3055
3056 pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
3057 ctrl &= ~PCI_REBAR_CTRL_BAR_SIZE_MASK;
3058 ctrl |= size << PCI_REBAR_CTRL_BAR_SIZE_SHIFT;
3059 pci_write_config_dword(pdev, pos + PCI_REBAR_CTRL, ctrl);
3060 return 0;
3061 }
3062

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (15.58 kB)
.config.gz (6.54 kB)
Download all attachments

2017-07-11 17:50:22

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v8 4/6] x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 30h-3fh) Processors v4

On Tue, Jul 11, 2017 at 3:07 PM, kbuild test robot <[email protected]> wrote:

> make ARCH=i386

Yeah, either this code shouldn't have been built on 32-bit arch at
all, or be portable.

> arch/x86/pci/fixup.c: In function 'pci_amd_enable_64bit_bar':
>>> arch/x86/pci/fixup.c:674:15: warning: large integer implicitly truncated to unsigned type [-Woverflow]
> res->start = 0x100000000ull;
> ^~~~~~~~~~~~~~
> arch/x86/pci/fixup.c:675:13: warning: large integer implicitly truncated to unsigned type [-Woverflow]
> res->end = 0xfd00000000ull - 1;
> ^~~~~~~~~~~~~~~
>>> arch/x86/pci/fixup.c:686:22: warning: right shift count >= width of type [-Wshift-count-overflow]

I suppose explicit casting will help.

> high = ((res->start >> 40) & AMD_141b_MMIO_HIGH_MMIOBASE_MASK) |
> ^~
> arch/x86/pci/fixup.c:687:21: warning: right shift count >= width of type [-Wshift-count-overflow]
> ((((res->end + 1) >> 40) << AMD_141b_MMIO_HIGH_MMIOLIMIT_SHIFT)
> ^~

These can be done via upper_32_bits() + shift.

--
With Best Regards,
Andy Shevchenko

2017-09-14 08:06:53

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: Resizeable PCI BAR support V8

On Mon, Jul 10, 2017 at 04:59:48PM +0200, Christian K?nig wrote:
> Hi everyone,
>
> This is the eighth incarnation of this set of patches. It enables device
> drivers to resize and most likely also relocate the PCI BAR of devices
> they manage to allow the CPU to access all of the device local memory at once.
>
> This is very useful for GFX device drivers where the default PCI BAR is only
> about 256MB in size for compatibility reasons, but the device easily have
> multiple gigabyte of local memory.
>
> Some changes since the last version:
> 1. Rebased on Linus current master tree, going to rebase on 4.13-rc1 as soon as that is out.
> 2. Some more documentation on how to use the new functionality in drivers.
> 3. Only re-enable decoding when everything looks good.
> 4. Rename the new amdgpu function to better match what it is doing.
>
> Bjorn what's you'r feeling on getting this into 4.14?

This seems 99.9% done, but there were some kbuild issues that need to be
resolved. Or did I miss the resolution of those?

Bjorn