Re: [PATCH RFC v4 11/21] PCI: Release and reassign the root bridge resources during rescan
On 3/26/19 11:41 PM, Bjorn Helgaas wrote:
> On Mon, Mar 11, 2019 at 04:31:12PM +0300, Sergey Miroshnichenko wrote:
>> When the movable BARs feature is enabled, don't rely on the memory gaps
>> reserved by the BIOS/bootloader/firmware, but instead rearrange the BARs
>> and bridge windows starting from the root.
>>
>> Endpoint device's BARs, after being released, are resorted and written
>> back by the pci_assign_unassigned_root_bus_resources().
>>
>> The last step of writing the recalculated windows to the bridges is done
>> by the new pci_setup_bridges() function.
>>
>> Signed-off-by: Sergey Miroshnichenko
>> ---
>> drivers/pci/pci.h | 1 +
>> drivers/pci/probe.c | 22 ++
>> drivers/pci/setup-bus.c | 11 ++-
>> 3 files changed, 33 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
>> index 224d88634115..e06e8692a7b1 100644
>> --- a/drivers/pci/pci.h
>> +++ b/drivers/pci/pci.h
>> @@ -248,6 +248,7 @@ void __pci_bus_assign_resources(const struct pci_bus
>> *bus,
>> struct list_head *realloc_head,
>> struct list_head *fail_head);
>> bool pci_bus_clip_resource(struct pci_dev *dev, int idx);
>> +void pci_bus_release_root_bridge_resources(struct pci_bus *bus);
>>
>> void pci_reassigndev_resource_alignment(struct pci_dev *dev);
>> void pci_disable_bridge_window(struct pci_dev *dev);
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index 1cf6ec960236..692752c71f71 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -3299,6 +3299,25 @@ static void pci_bus_rescan_done(struct pci_bus *bus)
>> pm_runtime_put(&bus->dev);
>> }
>>
>> +static void pci_setup_bridges(struct pci_bus *bus)
>> +{
>> +struct pci_dev *dev;
>> +
>> +list_for_each_entry(dev, &bus->devices, bus_list) {
>> +struct pci_bus *child;
>> +
>> +if (!pci_dev_is_added(dev) || pci_dev_is_ignored(dev))
>> +continue;
>> +
>> +child = dev->subordinate;
>> +if (child)
>> +pci_setup_bridges(child);
>> +}
>> +
>> +if (bus->self)
>> +pci_setup_bridge(bus);
>> +}
>> +
>> /**
>> * pci_rescan_bus - Scan a PCI bus for devices
>> * @bus: PCI bus to scan
>> @@ -3321,8 +3340,11 @@ unsigned int pci_rescan_bus(struct pci_bus *bus)
>> pci_bus_rescan_prepare(root);
>>
>> max = pci_scan_child_bus(root);
>> +
>> +pci_bus_release_root_bridge_resources(root);
>> pci_assign_unassigned_root_bus_resources(root);
>>
>> +pci_setup_bridges(root);
>> pci_bus_rescan_done(root);
>> } else {
>> max = pci_scan_child_bus(bus);
>> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
>> index be7d4e6d7b65..36a1907d9509 100644
>> --- a/drivers/pci/setup-bus.c
>> +++ b/drivers/pci/setup-bus.c
>> @@ -1584,7 +1584,7 @@ static void pci_bridge_release_resources(struct
>> pci_bus *bus,
>> pci_printk(KERN_DEBUG, dev, "resource %d %pR released\n",
>> PCI_BRIDGE_RESOURCES + idx, r);
>> /* keep the old size */
>> -r->end = resource_size(r) - 1;
>> +r->end = pci_movable_bars_enabled() ? 0 : (resource_size(r) -
>> 1);
>
> Doesn't this mean we're throwing away the information about the BAR
> size, and we'll have to size the BAR again somewhere? I would like to
> avoid that. But I don't know yet where you rely on this, so maybe
> it's not possible to avoid it.
>
This resource is not a BAR, but a bridge window, I'm freeing it intentionally,
so
pbus_size_mem() can later recalculate a new size.
Serge
>> r->start = 0;
>> r->flags = 0;
>>
>> @@ -1637,6 +1637,15 @@ static void pci_bus_release_bridge_resources(struct
>> pci_bus *bus,
>> pci_bridge_release_resources(bus, type);
>> }
>>
>> +void pci_bus_release_root_bridge_resources(struct pci_bus *root_bus)
>> +{
>> +pci_bus_release_bridge_resources(root_bus, IORESOURCE_IO,
>> whole_subtree);
>> +pci_bus_release_bridge_resources(root_bus, IORESOURCE_MEM,
>> whole_subtree);
>> +pci_bus_release_bridge_resources(root_bus,
>> + IORESOURCE_MEM_64 |
>> IORESOURCE_PREFETCH,
>> + whole_subtree);
>> +}
>> +
>> static void pci_bus_dump_res(struct pci_bus *bus)
>> {
>> struct resource *res;
>> --
>> 2.20.1
>>
Re: [PATCH RFC v4 11/21] PCI: Release and reassign the root bridge resources during rescan
On Mon, Mar 11, 2019 at 04:31:12PM +0300, Sergey Miroshnichenko wrote:
> When the movable BARs feature is enabled, don't rely on the memory gaps
> reserved by the BIOS/bootloader/firmware, but instead rearrange the BARs
> and bridge windows starting from the root.
>
> Endpoint device's BARs, after being released, are resorted and written
> back by the pci_assign_unassigned_root_bus_resources().
>
> The last step of writing the recalculated windows to the bridges is done
> by the new pci_setup_bridges() function.
>
> Signed-off-by: Sergey Miroshnichenko
> ---
> drivers/pci/pci.h | 1 +
> drivers/pci/probe.c | 22 ++
> drivers/pci/setup-bus.c | 11 ++-
> 3 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 224d88634115..e06e8692a7b1 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -248,6 +248,7 @@ void __pci_bus_assign_resources(const struct pci_bus *bus,
> struct list_head *realloc_head,
> struct list_head *fail_head);
> bool pci_bus_clip_resource(struct pci_dev *dev, int idx);
> +void pci_bus_release_root_bridge_resources(struct pci_bus *bus);
>
> void pci_reassigndev_resource_alignment(struct pci_dev *dev);
> void pci_disable_bridge_window(struct pci_dev *dev);
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 1cf6ec960236..692752c71f71 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -3299,6 +3299,25 @@ static void pci_bus_rescan_done(struct pci_bus *bus)
> pm_runtime_put(&bus->dev);
> }
>
> +static void pci_setup_bridges(struct pci_bus *bus)
> +{
> + struct pci_dev *dev;
> +
> + list_for_each_entry(dev, &bus->devices, bus_list) {
> + struct pci_bus *child;
> +
> + if (!pci_dev_is_added(dev) || pci_dev_is_ignored(dev))
> + continue;
> +
> + child = dev->subordinate;
> + if (child)
> + pci_setup_bridges(child);
> + }
> +
> + if (bus->self)
> + pci_setup_bridge(bus);
> +}
> +
> /**
> * pci_rescan_bus - Scan a PCI bus for devices
> * @bus: PCI bus to scan
> @@ -3321,8 +3340,11 @@ unsigned int pci_rescan_bus(struct pci_bus *bus)
> pci_bus_rescan_prepare(root);
>
> max = pci_scan_child_bus(root);
> +
> + pci_bus_release_root_bridge_resources(root);
> pci_assign_unassigned_root_bus_resources(root);
>
> + pci_setup_bridges(root);
> pci_bus_rescan_done(root);
> } else {
> max = pci_scan_child_bus(bus);
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index be7d4e6d7b65..36a1907d9509 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1584,7 +1584,7 @@ static void pci_bridge_release_resources(struct pci_bus
> *bus,
> pci_printk(KERN_DEBUG, dev, "resource %d %pR released\n",
> PCI_BRIDGE_RESOURCES + idx, r);
> /* keep the old size */
> - r->end = resource_size(r) - 1;
> + r->end = pci_movable_bars_enabled() ? 0 : (resource_size(r) -
> 1);
Doesn't this mean we're throwing away the information about the BAR
size, and we'll have to size the BAR again somewhere? I would like to
avoid that. But I don't know yet where you rely on this, so maybe
it's not possible to avoid it.
> r->start = 0;
> r->flags = 0;
>
> @@ -1637,6 +1637,15 @@ static void pci_bus_release_bridge_resources(struct
> pci_bus *bus,
> pci_bridge_release_resources(bus, type);
> }
>
> +void pci_bus_release_root_bridge_resources(struct pci_bus *root_bus)
> +{
> + pci_bus_release_bridge_resources(root_bus, IORESOURCE_IO,
> whole_subtree);
> + pci_bus_release_bridge_resources(root_bus, IORESOURCE_MEM,
> whole_subtree);
> + pci_bus_release_bridge_resources(root_bus,
> + IORESOURCE_MEM_64 |
> IORESOURCE_PREFETCH,
> + whole_subtree);
> +}
> +
> static void pci_bus_dump_res(struct pci_bus *bus)
> {
> struct resource *res;
> --
> 2.20.1
>
[PATCH RFC v4 11/21] PCI: Release and reassign the root bridge resources during rescan
When the movable BARs feature is enabled, don't rely on the memory gaps
reserved by the BIOS/bootloader/firmware, but instead rearrange the BARs
and bridge windows starting from the root.
Endpoint device's BARs, after being released, are resorted and written
back by the pci_assign_unassigned_root_bus_resources().
The last step of writing the recalculated windows to the bridges is done
by the new pci_setup_bridges() function.
Signed-off-by: Sergey Miroshnichenko
---
drivers/pci/pci.h | 1 +
drivers/pci/probe.c | 22 ++
drivers/pci/setup-bus.c | 11 ++-
3 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 224d88634115..e06e8692a7b1 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -248,6 +248,7 @@ void __pci_bus_assign_resources(const struct pci_bus *bus,
struct list_head *realloc_head,
struct list_head *fail_head);
bool pci_bus_clip_resource(struct pci_dev *dev, int idx);
+void pci_bus_release_root_bridge_resources(struct pci_bus *bus);
void pci_reassigndev_resource_alignment(struct pci_dev *dev);
void pci_disable_bridge_window(struct pci_dev *dev);
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 1cf6ec960236..692752c71f71 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -3299,6 +3299,25 @@ static void pci_bus_rescan_done(struct pci_bus *bus)
pm_runtime_put(&bus->dev);
}
+static void pci_setup_bridges(struct pci_bus *bus)
+{
+ struct pci_dev *dev;
+
+ list_for_each_entry(dev, &bus->devices, bus_list) {
+ struct pci_bus *child;
+
+ if (!pci_dev_is_added(dev) || pci_dev_is_ignored(dev))
+ continue;
+
+ child = dev->subordinate;
+ if (child)
+ pci_setup_bridges(child);
+ }
+
+ if (bus->self)
+ pci_setup_bridge(bus);
+}
+
/**
* pci_rescan_bus - Scan a PCI bus for devices
* @bus: PCI bus to scan
@@ -3321,8 +3340,11 @@ unsigned int pci_rescan_bus(struct pci_bus *bus)
pci_bus_rescan_prepare(root);
max = pci_scan_child_bus(root);
+
+ pci_bus_release_root_bridge_resources(root);
pci_assign_unassigned_root_bus_resources(root);
+ pci_setup_bridges(root);
pci_bus_rescan_done(root);
} else {
max = pci_scan_child_bus(bus);
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index be7d4e6d7b65..36a1907d9509 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1584,7 +1584,7 @@ static void pci_bridge_release_resources(struct pci_bus
*bus,
pci_printk(KERN_DEBUG, dev, "resource %d %pR released\n",
PCI_BRIDGE_RESOURCES + idx, r);
/* keep the old size */
- r->end = resource_size(r) - 1;
+ r->end = pci_movable_bars_enabled() ? 0 : (resource_size(r) -
1);
r->start = 0;
r->flags = 0;
@@ -1637,6 +1637,15 @@ static void pci_bus_release_bridge_resources(struct
pci_bus *bus,
pci_bridge_release_resources(bus, type);
}
+void pci_bus_release_root_bridge_resources(struct pci_bus *root_bus)
+{
+ pci_bus_release_bridge_resources(root_bus, IORESOURCE_IO,
whole_subtree);
+ pci_bus_release_bridge_resources(root_bus, IORESOURCE_MEM,
whole_subtree);
+ pci_bus_release_bridge_resources(root_bus,
+IORESOURCE_MEM_64 |
IORESOURCE_PREFETCH,
+whole_subtree);
+}
+
static void pci_bus_dump_res(struct pci_bus *bus)
{
struct resource *res;
--
2.20.1
