On 3/26/19 10:24 PM, Bjorn Helgaas wrote:
> On Mon, Mar 11, 2019 at 04:31:06PM +0300, Sergey Miroshnichenko wrote:
>> If a new PCIe device has been hot-plugged between the two active ones
>> without big enough gap between their BARs,
>
> Just to speak precisely here, a hot-added device is not "between" two
> active ones because the new device has zeros in its BARs.
>
> BARs from different devices can be interleaved arbitrarily, subject to
> bridge window constraints, so we can really only speak about a *BAR*
> (not the entire device) being between two other BARs.
>
> Also, I don't think there's anything here that is PCIe-specific, so we
> should talk about "PCI", not "PCIe".
>
I agree, that should be rephrased. This patchset intends to solve the problem
when a
bridge window is not big enough (or fragmented too much) to fit new BARs, and
it can't be
expanded enough because blocked by "neighboring" BARs.
>> these BARs should be moved
>> if their drivers support this feature. The drivers should be notified
>> and paused during the procedure:
>>
>> 1) dev 8 (new)
>> |
>> v
>> .. | dev 3 | dev 3 | dev 5 | dev 7 |
>> .. | BAR 0 | BAR 1 | BAR 0 | BAR 0 |
>>
>> 2) dev 8
>> |
>> v
>> .. | dev 3 | dev 3 | --> --> | dev 5 | dev 7 |
>> .. | BAR 0 | BAR 1 | --> --> | BAR 0 | BAR 0 |
>>
>> 3)
>>
>> .. | dev 3 | dev 3 | dev 8 | dev 8 | dev 5 | dev 7 |
>> .. | BAR 0 | BAR 1 | BAR 0 | BAR 1 | BAR 0 | BAR 0 |
>>
>> Thus, prior reservation of memory regions by BIOS/bootloader/firmware
>> is not required anymore for the PCIe hotplug.
>>
>> The PCI_MOVABLE_BARS flag is set by the platform is this feature is
>> supported and tested, but can be overridden by the following command
>> line option:
>> pcie_movable_bars={ off | force }
>
> A chicken switch to turn this functionality off is OK, but I think it
> should be enabled by default. There isn't anything about this that's
> platform-specific, is there?
>
I'm a bit afraid to suppose that; I was once surprised that bus numbers can't
be assigned
arbitrarily on some platforms [1], so probably there are some similar
restrictions on BARs
too.
Was going to propose adding pci_add_flags(PCI_MOVABLE_BARS) into
arch/.../init.c for
tested platforms, so there will be less upset people with their BARs suddenly
broken. But
this logic can be reversed: pci_clear_flags(PCI_MOVABLE_BARS) for platforms
where movable
BARs can't work.
Serge
[1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2018-September/178103.html
>> Signed-off-by: Sergey Miroshnichenko <s.miroshniche...@yadro.com>
>> ---
>> .../admin-guide/kernel-parameters.txt | 7 ++++++
>> drivers/pci/pci.c | 24 +++++++++++++++++++
>> include/linux/pci.h | 2 ++
>> 3 files changed, 33 insertions(+)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt
>> b/Documentation/admin-guide/kernel-parameters.txt
>> index 2b8ee90bb644..d40eaf993f80 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -3417,6 +3417,13 @@
>> nomsi Do not use MSI for native PCIe PME signaling (this makes
>> all PCIe root ports use INTx for all services).
>>
>> + pcie_movable_bars=[PCIE]
>> + Override the movable BARs support detection:
>> + off
>> + Disable even if supported by the platform
>> + force
>> + Enable even if not explicitly declared as supported
>> +
>> pcmv= [HW,PCMCIA] BadgePAD 4
>>
>> pd_ignore_unused
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index 69898fe5255e..4dac49a887ec 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -139,6 +139,30 @@ static int __init pcie_port_pm_setup(char *str)
>> }
>> __setup("pcie_port_pm=", pcie_port_pm_setup);
>>
>> +static bool pcie_movable_bars_off;
>> +static bool pcie_movable_bars_force;
>> +static int __init pcie_movable_bars_setup(char *str)
>> +{
>> + if (!strcmp(str, "off"))
>> + pcie_movable_bars_off = true;
>> + else if (!strcmp(str, "force"))
>> + pcie_movable_bars_force = true;
>> + return 1;
>> +}
>> +__setup("pcie_movable_bars=", pcie_movable_bars_setup);
>> +
>> +bool pci_movable_bars_enabled(void)
>> +{
>> + if (pcie_movable_bars_off)
>> + return false;
>> +
>> + if (pcie_movable_bars_force)
>> + return true;
>> +
>> + return pci_has_flag(PCI_MOVABLE_BARS);
>> +}
>> +EXPORT_SYMBOL(pci_movable_bars_enabled);
>> +
>> /* Time to wait after a reset for device to become responsive */
>> #define PCIE_RESET_READY_POLL_MS 60000
>>
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index cb2760a31fe2..cbe661aff9f5 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -866,6 +866,7 @@ enum {
>> PCI_ENABLE_PROC_DOMAINS = 0x00000010, /* Enable domains in /proc */
>> PCI_COMPAT_DOMAIN_0 = 0x00000020, /* ... except domain 0 */
>> PCI_SCAN_ALL_PCIE_DEVS = 0x00000040, /* Scan all, not just dev 0 */
>> + PCI_MOVABLE_BARS = 0x00000080, /* Runtime BAR reassign after
>> hotplug */
>> };
>>
>> /* These external functions are only available when PCI support is enabled
>> */
>> @@ -1345,6 +1346,7 @@ unsigned char pci_bus_max_busnr(struct pci_bus *bus);
>> void pci_setup_bridge(struct pci_bus *bus);
>> resource_size_t pcibios_window_alignment(struct pci_bus *bus,
>> unsigned long type);
>> +bool pci_movable_bars_enabled(void);
>>
>> #define PCI_VGA_STATE_CHANGE_BRIDGE (1 << 0)
>> #define PCI_VGA_STATE_CHANGE_DECODES (1 << 1)
>> --
>> 2.20.1
>>
