linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Sergei Miroshnichenko <s.miroshnichenko@yadro.com>
Cc: linux-pci@vger.kernel.org, Stefan Roese <sr@denx.de>, linux@yadro.com
Subject: Re: [PATCH v7 16/26] PCI: Ignore PCIBIOS_MIN_MEM
Date: Thu, 30 Jan 2020 17:52:44 -0600	[thread overview]
Message-ID: <20200130235244.GA149020@google.com> (raw)
In-Reply-To: <20200129152937.311162-17-s.miroshnichenko@yadro.com>

On Wed, Jan 29, 2020 at 06:29:27PM +0300, Sergei Miroshnichenko wrote:
> BARs and bridge windows are only allowed to be assigned to their
> parent bus's bridge windows, going up to the root complex's resources.
> So additional limitations on BAR address are not needed, and the
> PCIBIOS_MIN_MEM can be ignored.

This is theoretically true, but I don't think we have reliable
information about the host bridge windows in all cases, so
PCIBIOS_MIN_MEM/_IO is something of an approximation.

> Besides, the value of PCIBIOS_MIN_MEM reported by the BIOS 1.3 on
> Supermicro H11SSL-i via e820__setup_pci_gap():
> 
>   [mem 0xebff1000-0xfe9fffff] available for PCI devices
> 
> is only suitable for a single RC out of four:
> 
>   pci_bus 0000:00: root bus resource [mem 0xec000000-0xefffffff window]
>   pci_bus 0000:20: root bus resource [mem 0xeb800000-0xebefffff window]
>   pci_bus 0000:40: root bus resource [mem 0xeb200000-0xeb5fffff window]
>   pci_bus 0000:60: root bus resource [mem 0xe8b00000-0xeaffffff window]
> 
> , which makes the AMD EPYC 7251 unable to boot with this movable BARs
> patchset.

Something's wrong if this system booted before this patch set but not
after.  We shouldn't be doing *anything* with the BARs until we need
to, i.e., until we hot-add a device where we have to move things to
find space for it.

(And we don't want a bisection hole where this system can't boot until
this patch is applied, but I assume that's obvious.)

> Signed-off-by: Sergei Miroshnichenko <s.miroshnichenko@yadro.com>
> ---
>  drivers/pci/setup-res.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index a7d81816d1ea..4043aab021dd 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -246,12 +246,13 @@ static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev,
>  		int resno, resource_size_t size, resource_size_t align)
>  {
>  	struct resource *res = dev->resource + resno;
> -	resource_size_t min;
> +	resource_size_t min = 0;
>  	int ret;
>  	resource_size_t start = (resource_size_t)-1;
>  	resource_size_t end = 0;
>  
> -	min = (res->flags & IORESOURCE_IO) ? PCIBIOS_MIN_IO : PCIBIOS_MIN_MEM;
> +	if (!pci_can_move_bars)
> +		min = (res->flags & IORESOURCE_IO) ? PCIBIOS_MIN_IO : PCIBIOS_MIN_MEM;

I don't understand the connection here.  PCIBIOS_MIN_MEM and
PCIBIOS_MIN_IO are basically ways to say "we can't put PCI resources
below this address".

On ACPI systems, the devices in the ACPI namespace are supposed to
tell the OS what resources they use, and obviously the OS should not
assign those resources to anything else.  If Linux handled all those
ACPI resources correctly and in the absence of firmware defects, we
shouldn't need PCIBIOS_MIN_MEM/_IO at all.  But neither of those is
currently true.

It's true that we should be smarter about PCIBIOS_MIN_MEM/_IO, but I
don't think that has anything to do with whether we support *moving*
BARs.  We have to avoid the address space that's already in use in
*all* cases.

>  	if (pci_can_move_bars && dev->subordinate && resno >= PCI_BRIDGE_RESOURCES) {
>  		struct pci_bus *child_bus = dev->subordinate;
> -- 
> 2.24.1
> 

  reply	other threads:[~2020-01-30 23:52 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-29 15:29 [PATCH v7 00/26] PCI: Allow BAR movement during boot and hotplug Sergei Miroshnichenko
2020-01-29 15:29 ` [PATCH v7 01/26] PCI: Fix race condition in pci_enable/disable_device() Sergei Miroshnichenko
2020-01-29 15:29 ` [PATCH v7 02/26] PCI: Enable bridge's I/O and MEM access for hotplugged devices Sergei Miroshnichenko
2020-01-30 23:12   ` Bjorn Helgaas
2020-01-29 15:29 ` [PATCH v7 03/26] PCI: hotplug: Initial support of the movable BARs feature Sergei Miroshnichenko
2020-01-29 15:29 ` [PATCH v7 04/26] PCI: Add version of release_child_resources() aware of immovable BARs Sergei Miroshnichenko
2020-01-29 15:29 ` [PATCH v7 05/26] PCI: hotplug: Fix reassigning the released BARs Sergei Miroshnichenko
2020-01-29 15:29 ` [PATCH v7 06/26] PCI: hotplug: Recalculate every bridge window during rescan Sergei Miroshnichenko
2020-01-29 15:29 ` [PATCH v7 07/26] PCI: hotplug: Don't allow hot-added devices to steal resources Sergei Miroshnichenko
2020-01-29 15:29 ` [PATCH v7 08/26] PCI: hotplug: Try to reassign movable BARs only once Sergei Miroshnichenko
2020-01-29 15:29 ` [PATCH v7 09/26] PCI: hotplug: Calculate immovable parts of bridge windows Sergei Miroshnichenko
2020-01-30 21:26   ` Bjorn Helgaas
2020-01-31 16:59     ` Sergei Miroshnichenko
2020-01-31 20:10       ` Bjorn Helgaas
2020-01-29 15:29 ` [PATCH v7 10/26] PCI: Include fixed and immovable BARs into the bus size calculating Sergei Miroshnichenko
2020-01-29 15:29 ` [PATCH v7 11/26] PCI: hotplug: movable BARs: Compute limits for relocated bridge windows Sergei Miroshnichenko
2020-01-29 15:29 ` [PATCH v7 12/26] PCI: Make sure bridge windows include their fixed BARs Sergei Miroshnichenko
2020-01-29 15:29 ` [PATCH v7 13/26] PCI: hotplug: Add support of immovable BARs to pci_assign_resource() Sergei Miroshnichenko
2020-01-29 15:29 ` [PATCH v7 14/26] PCI: hotplug: Sort immovable BARs before assignment Sergei Miroshnichenko
2020-01-29 15:29 ` [PATCH v7 15/26] PCI: hotplug: Enable the movable BARs feature by default Sergei Miroshnichenko
2020-01-29 15:29 ` [PATCH v7 16/26] PCI: Ignore PCIBIOS_MIN_MEM Sergei Miroshnichenko
2020-01-30 23:52   ` Bjorn Helgaas [this message]
2020-01-31 18:19     ` Sergei Miroshnichenko
2020-01-31 20:27       ` Bjorn Helgaas
2020-02-05 13:04         ` Sergei Miroshnichenko
2020-02-05 16:32           ` Bjorn Helgaas
2020-02-12 12:16             ` Sergei Miroshnichenko
2020-02-12 14:13               ` Bjorn Helgaas
2020-01-29 15:29 ` [PATCH v7 17/26] PCI: hotplug: Ignore the MEM BAR offsets from BIOS/bootloader Sergei Miroshnichenko
2020-01-31 20:31   ` Bjorn Helgaas
2020-02-05 11:01     ` Sergei Miroshnichenko
2020-02-05 16:42       ` Bjorn Helgaas
2020-02-12 12:29         ` Sergei Miroshnichenko
2020-01-29 15:29 ` [PATCH v7 18/26] PCI: Treat VGA BARs as immovable Sergei Miroshnichenko
2020-01-29 15:29 ` [PATCH v7 19/26] PCI: hotplug: Configure MPS for hot-added bridges during bus rescan Sergei Miroshnichenko
2020-01-29 15:29 ` [PATCH v7 20/26] PNP: Don't reserve BARs for PCI when enabled movable BARs Sergei Miroshnichenko
2020-01-30 14:39   ` Rafael J. Wysocki
2020-01-30 21:14   ` Bjorn Helgaas
2020-01-31 15:48     ` Sergei Miroshnichenko
2020-01-31 19:39       ` Bjorn Helgaas
2020-01-29 15:29 ` [PATCH v7 21/26] PCI: hotplug: Don't disable the released bridge windows immediately Sergei Miroshnichenko
2020-01-29 15:29 ` [PATCH v7 22/26] PCI: pciehp: Trigger a domain rescan on hp events when enabled movable BARs Sergei Miroshnichenko
2020-01-29 15:29 ` [PATCH v7 23/26] PCI: Don't claim immovable BARs Sergei Miroshnichenko
2020-01-29 15:29 ` [PATCH v7 24/26] PCI: hotplug: Don't reserve bus space when enabled movable BARs Sergei Miroshnichenko
2020-01-29 15:29 ` [PATCH v7 25/26] nvme-pci: Handle " Sergei Miroshnichenko
2020-01-29 15:29 ` [PATCH v7 26/26] PCI/portdrv: Declare support of " Sergei Miroshnichenko
2020-01-30 23:37 ` [PATCH v7 00/26] PCI: Allow BAR movement during boot and hotplug Bjorn Helgaas
2020-02-03  4:56   ` Oliver O'Halloran

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200130235244.GA149020@google.com \
    --to=helgaas@kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux@yadro.com \
    --cc=s.miroshnichenko@yadro.com \
    --cc=sr@denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).