Re: [PATCH v7 09/26] PCI: hotplug: Calculate immovable parts of bridge windows

[Bjorn Helgaas <helgaas@kernel.org>]

On Wed, Jan 29, 2020 at 06:29:20PM +0300, Sergei Miroshnichenko wrote:
> When movable BARs are enabled, and if a bridge contains a device with fixed
> (IORESOURCE_PCI_FIXED) or immovable BARs, the corresponding windows can't

What is the difference between fixed (IORESOURCE_PCI_FIXED) and
immovable BARs?  I'm hesitant to add a new concept ("immovable") with
a different name but very similar meaning.

I understand that in the case of bridge windows, you may need to track
only part of the window, as opposed to a BAR where the entire BAR has
to be either fixed or movable, but I think adding a new term will be
confusing.

> be moved too far away from their original positions - they must still
> contain all the fixed/immovable BARs, like that:
> 
> 1) Window position before a bus rescan:
> 
>    | <--                    root bridge window                        --> |
>    |                                                                      |
>    | | <--     bridge window    --> |                                     |
>    | | movable BARs | **fixed BAR** |                                     |
>        ^^^^^^^^^^^^
> 
> 2) Possible valid outcome after rescan and move:
> 
>    | <--                    root bridge window                        --> |
>    |                                                                      |
>    |                | <--     bridge window    --> |                      |
>    |                | **fixed BAR** | Movable BARs |                      |
>                                       ^^^^^^^^^^^^
> 
> An immovable area of a bridge window is a range that covers all the fixed
> and immovable BARs of direct children, and all the fixed area of children
> bridges:
> 
>    | <--                    root bridge window                        --> |
>    |                                                                      |
>    |  | <--                  bridge window level 1                --> |   |
>    |  | ******************** immovable area *******************       |   |
>    |  |                                                               |   |
>    |  | **fixed BAR** | <--      bridge window level 2     --> | BARs |   |
>    |  |               | *********** immovable area *********** |      |   |
>    |  |               |                                        |      |   |
>    |  |               | **fixed BAR** |  BARs  | **fixed BAR** |      |   |
>                                          ^^^^
> 
> To store these areas, the .immovable_range field has been added to struct
> pci_bus for every bridge window type: IO, MEM and PREFETCH. It is filled
> recursively from leaves to the root before a rescan.
> 
> Signed-off-by: Sergei Miroshnichenko <s.miroshnichenko@yadro.com>
> ---
>  drivers/pci/pci.h   | 14 ++++++++
>  drivers/pci/probe.c | 88 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/pci.h |  6 ++++
>  3 files changed, 108 insertions(+)
> 
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 3b4c982772d3..5f2051c8531c 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -404,6 +404,20 @@ static inline bool pci_dev_is_disconnected(const struct pci_dev *dev)
>  	return dev->error_state == pci_channel_io_perm_failure;
>  }
>  
> +static inline int pci_get_bridge_resource_idx(struct resource *r)
> +{
> +	int idx = 1;
> +
> +	if (r->flags & IORESOURCE_IO)
> +		idx = 0;
> +	else if (!(r->flags & IORESOURCE_PREFETCH))
> +		idx = 1;
> +	else if (r->flags & IORESOURCE_MEM_64)
> +		idx = 2;

Random nit: No variables or elses required:

  if (r->flags & IORESOURCE_IO)
    return 0;
  if (!(r->flags & IORESOURCE_PREFETCH))
    return 1;
  ...

> +	return idx;
> +}