[RFC,0/4] PCI: Allow BAR movement during hotplug

Message

Sergei Miroshnichenko Sept. 14, 2018, 4:14 p.m. UTC

If the firmware or kernel has arranged memory for PCIe devices in a way that doesn't
provide enough space for BARs of a new hotplugged device, the kernel can pause the
drivers of the "obstructing" devices and move their BARs, so new BARs can fit into the
created hole.

When a driver is un-paused by the kernel after the PCIe rescan, it should check if its
BARs had changed and ioremap() them if needed.

BARs are moved not directly, but by releasing bridge resources, then sorting and
assigning them back, similarly to the initial PCIe topology scan during system boot
(when pci=realloc is passed).

Pausing drivers is performed via reset_prepare() and reset_done() callbacks of struct
pci_error_handlers. Drivers should pause during rescan not only because of potential
movement of their BARs, but also because of possible updating of the bridge windows.

This patchset is a part of our work on adding support for hotplugging bridges full of
NVME devices (without special requirement such as Hot-Plug Controller, reservation of
bus numbers and memory regions by firmware, etc.), should I also add here the patch that
adds support of moving BARs to the NVME driver?

Sergey Miroshnichenko (4):
  PCI: hotplug: Add parameter to put devices to reset during rescan
  PCI: Release and reassign resources from the root during rescan
  PCI: Invalidate the released BAR resources
  PCI: Fix writing invalid BARs during pci_restore_state()

 .../admin-guide/kernel-parameters.txt         |  6 ++
 drivers/pci/pci.c                             |  4 +-
 drivers/pci/pci.h                             |  8 ++
 drivers/pci/probe.c                           | 78 ++++++++++++++++++-
 drivers/pci/setup-bus.c                       | 33 +++++---
 include/linux/pci.h                           |  1 +
 6 files changed, 119 insertions(+), 11 deletions(-)

Comments

David Laight Sept. 18, 2018, 11:16 a.m. UTC | #1

From: Sergey Miroshnichenko
> Sent: 14 September 2018 17:14
> 
> If the firmware or kernel has arranged memory for PCIe devices in a way that doesn't
> provide enough space for BARs of a new hotplugged device, the kernel can pause the
> drivers of the "obstructing" devices and move their BARs, so new BARs can fit into the
> created hole.
> 
> When a driver is un-paused by the kernel after the PCIe rescan, it should check if its
> BARs had changed and ioremap() them if needed.
> 
> BARs are moved not directly, but by releasing bridge resources, then sorting and
> assigning them back, similarly to the initial PCIe topology scan during system boot
> (when pci=realloc is passed).
> 
> Pausing drivers is performed via reset_prepare() and reset_done() callbacks of struct
> pci_error_handlers. Drivers should pause during rescan not only because of potential
> movement of their BARs, but also because of possible updating of the bridge windows.
...

You are restricting this to GPL drivers (unless someone has opened up the error
handling functions since I last looked).

I also suspect you need something to indicate that the driver is willing for
all this to happen.

An error return from reset_prepare() will all be too late - other drivers
might have been stopped even though it is impossible to actually perform
the reorganisation.

There will also be drivers/devices that can handle being moved by a full
remove/rescan sequence, particularly if there are no current users of the
device.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Sergei Miroshnichenko Sept. 18, 2018, 5:07 p.m. UTC | #2

On 9/18/18 2:16 PM, David Laight wrote:
> From: Sergey Miroshnichenko
>> Sent: 14 September 2018 17:14
>>
>> If the firmware or kernel has arranged memory for PCIe devices in a way that doesn't
>> provide enough space for BARs of a new hotplugged device, the kernel can pause the
>> drivers of the "obstructing" devices and move their BARs, so new BARs can fit into the
>> created hole.
>>
>> When a driver is un-paused by the kernel after the PCIe rescan, it should check if its
>> BARs had changed and ioremap() them if needed.
>>
>> BARs are moved not directly, but by releasing bridge resources, then sorting and
>> assigning them back, similarly to the initial PCIe topology scan during system boot
>> (when pci=realloc is passed).
>>
>> Pausing drivers is performed via reset_prepare() and reset_done() callbacks of struct
>> pci_error_handlers. Drivers should pause during rescan not only because of potential
>> movement of their BARs, but also because of possible updating of the bridge windows.
> ...
> 
> You are restricting this to GPL drivers (unless someone has opened up the error
> handling functions since I last looked).
> 

The only GPL-related restrictions I was aware of are functions exported
with EXPORT_SYMBOL_GPL().

> I also suspect you need something to indicate that the driver is willing for
> all this to happen.
> 

I'm thinking about new callbacks in the struct pci_driver, would they
met the same restrictions?

> An error return from reset_prepare() will all be too late - other drivers
> might have been stopped even though it is impossible to actually perform
> the reorganisation.
> 
> There will also be drivers/devices that can handle being moved by a full
> remove/rescan sequence, particularly if there are no current users of the
> device.
> 

The implementation is going to be changed a bit:
 - if a device's driver doesn't have these callbacks (and so it can't
pause), its BARs are marked as immovable;
 - for every "movable" device that was working before the hotplug event,
it is guaranteed to arrange BARs;
 - if there is not enough memory or if the memory is too fragmented by
"immovable" devices, the BAR allocation for hotplugged devices can fail,
in this case the only way is to convert drivers to "movable".

The remove+rescan trick for unused "immovable" devices could be used as
a fallback, if BAR allocation has failed, but I'm not sure yet - what if
there are devices that are expected to do some job even without being
explicitly open()'ed?

> 	David
> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
> 

Best regards,
Serge