On Tue, Apr 25, 2017 at 4:19 PM, Christian König deathsimple@vodafone.de wrote:
From: Christian König christian.koenig@amd.com
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.
If reprogramming the bridge BAR fails the old status is restored and -ENOSPC returned to the calling device driver.
+int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type) +{
const unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
IORESOURCE_PREFETCH | IORESOURCE_MEM_64;
Redundant.
struct pci_dev_resource *dev_res;
LIST_HEAD(saved);
LIST_HEAD(added);
LIST_HEAD(failed);
unsigned i;
unsigned int i;
int ret = 0;
/* Walk to the root BUS, releasing bridge BARs when possible */
while (1) {
This raises red flag. Care to refactor? Also do {} while () syntax allows faster to get that the loop body goes at least once.
for (i = PCI_BRIDGE_RESOURCES; i < PCI_BRIDGE_RESOURCE_END;
i++) {
struct resource *res = &bridge->resource[i];
if ((res->flags & type_mask) != (type & type_mask))
I would rather go with
if ((res->flags ^ type) & 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;
if (res->parent)
release_resource(res);
res->start = 0;
res->end = 0;
dev_info(&bridge->dev, "BAR %d: released %pR\n",
i, res);
I doesn't make much sense to me after zeroing a range.
break;
}
if (i == PCI_BRIDGE_RESOURCE_END)
break;
if (!bridge->bus || !bridge->bus->self)
break;
bridge = bridge->bus->self;
}
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;
}
Remove extra empty line.
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);
free_list(&failed);
return ret;
You might re-use two lines with below, but perhaps better to show which case returns 0 explicitly and drop assignment ret = 0 above.
+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_release_resource(struct pci_dev *dev, int resno) +{
struct resource *res = dev->resource + resno;
release_resource(res);
res->end = resource_size(res) - 1;
res->start = 0;
res->flags |= IORESOURCE_UNSET;
dev_info(&dev->dev, "BAR %d: released %pR\n", resno, res);
Makes little sense to me after you cleared information out.
+} +EXPORT_SYMBOL(pci_release_resource);
+int pci_resize_resource(struct pci_dev *dev, int resno, int size) +{
struct resource *res = dev->resource + resno;
u64 bytes = 1ULL << (size + 20);
res->end = res->start + (1ULL << (old + 20)) - 1;
Perhaps macro or helper?
static inline u64 rbar_size_to_bytes(size) { return 1ULL << (size + 20); }