LKML Archive on lore.kernel.org
 help / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: "Lee, Chun-Yi" <joeyli.kernel@gmail.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H . Peter Anvin" <hpa@zytor.com>,
	x86@kernel.org, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org, "Lee, Chun-Yi" <jlee@suse.com>
Subject: Re: [PATCH] x86/PCI: Claim the resources of firmware enabled IOAPIC before children bus
Date: Mon, 6 Aug 2018 16:48:07 -0500
Message-ID: <20180806214807.GE30691@bhelgaas-glaptop.roam.corp.google.com> (raw)
In-Reply-To: <20180724110144.16442-1-jlee@suse.com>

On Tue, Jul 24, 2018 at 07:01:44PM +0800, Lee, Chun-Yi wrote:
> I got a machine that the resource of firmware enabled IOAPIC conflicts
> with the resource of a children bus when the PCI host bus be hotplug.
> 
> [ 3182.243325] PCI host bridge to bus 0001:40
> [ 3182.243328] pci_bus 0001:40: root bus resource [io  0xc000-0xdfff window]
> [ 3182.243330] pci_bus 0001:40: root bus resource [mem 0xdc000000-0xebffffff window]
> [ 3182.243331] pci_bus 0001:40: root bus resource [mem 0x212400000000-0x2125ffffffff window]
> [ 3182.243334] pci_bus 0001:40: root bus resource [bus 40-7e]
> ...
> [ 3182.244737] pci 0001:40:05.4: [8086:6f2c] type 00 class 0x080020
> [ 3182.244746] pci 0001:40:05.4: reg 0x10: [mem 0xdc000000-0xdc000fff]
> ...
> [ 3182.246697] pci 0001:40:02.0: PCI bridge to [bus 41]
> [ 3182.246702] pci 0001:40:02.0:   bridge window [mem 0xdc000000-0xdc7fffff]
> ...
> pci 0001:40:05.4: can't claim BAR 0 [mem 0xdc000000-0xdc000fff]: address conflict with PCI Bus 0001:41 [mem 0xdc000000-0xdc7fffff]
> 
> The bus topology:
> 
>  +-[0001:40]-+-02.0-[41]--
>  |           +-03.0-[41]--
>  |           +-03.2-[41]--+-00.0  Intel Corporation I350 Gigabit Network Connection
>  |           |            +-00.1  Intel Corporation I350 Gigabit Network Connection
>  |           |            +-00.2  Intel Corporation I350 Gigabit Network Connection
>  |           |            \-00.3  Intel Corporation I350 Gigabit Network Connection
>  |           +-05.0  Intel Corporation Xeon E7 v4/Xeon E5 v4/Xeon E3 v4/Xeon D Map/VTd_Misc/System Management
>  |           +-05.1  Intel Corporation Xeon E7 v4/Xeon E5 v4/Xeon E3 v4/Xeon D IIO Hot Plug
>  |           +-05.2  Intel Corporation Xeon E7 v4/Xeon E5 v4/Xeon E3 v4/Xeon D IIO RAS/Control Status/Global Errors
>  |           \-05.4  Intel Corporation Xeon E7 v4/Xeon E5 v4/Xeon E3 v4/Xeon D I/O APIC
> 
> This problem causes that the NIC behine the child bus was not available
> after PCI host bridge hotpluged.
> 
> Kernel does not want to change resource of firmware enabled IOAPIC, but
> the priority of children bus's resources are higher than any other devices.
> So this conflict can not be handled by the reassigning logic of kernel.

I don't understand this paragraph.  AFAIK, if two devices on a bus
both decode the same address, as the IOAPIC and the bridge do here,
the only real "priority" in PCI would be subtractive decode.  But I
don't think that applies here since the bridge is using a positive
decode window.

I would expect that a read to the [mem 0xdc000000-0xdc000fff] range
would potentially receive two read completions, which would cause an
Unexpected Completion error.

Maybe you mean that we don't want to change the IOAPIC resources, but
it's OK if we move the bridge window, so in that sense, the IOAPIC is
"higher priority" than the bridge window?  This is the reverse of what
your paragraph seems to say.

> This patch claims the resources of firmware enabled IOAPIC before
> children bus. Then kernel gets a chance to reassign the resources of
> children bus to avoid the conflict.

Can you open a report at https://bugzilla.kernel.org, attach the
before and after dmesg logs, and include the URL in the commit log?

> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
> ---
>  arch/x86/pci/i386.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c
> index ed4ac215305d..6413eda87c72 100644
> --- a/arch/x86/pci/i386.c
> +++ b/arch/x86/pci/i386.c
> @@ -230,13 +230,40 @@ static void pcibios_allocate_bridge_resources(struct pci_dev *dev)
>  	}
>  }
>  
> +static bool ioapic_firmware_enabled(struct pci_dev *dev)
> +{
> +	u16 class = dev->class >> 8;
> +
> +	if (class == PCI_CLASS_SYSTEM_PIC) {
> +		u16 command;
> +
> +		pci_read_config_word(dev, PCI_COMMAND, &command);
> +		if (command & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY))
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
> +static void pcibios_allocate_dev_resources(struct pci_dev *dev, int pass);
> +
>  static void pcibios_allocate_bus_resources(struct pci_bus *bus)
>  {
>  	struct pci_bus *child;
> +	struct pci_dev *dev;
>  
>  	/* Depth-First Search on bus tree */
>  	if (bus->self)
>  		pcibios_allocate_bridge_resources(bus->self);
> +
> +	/* allocate firmware enabled APIC before children bus */
> +	list_for_each_entry(dev, &bus->devices, bus_list) {
> +		if (ioapic_firmware_enabled(dev)) {
> +			pcibios_allocate_dev_resources(dev, 0);
> +			pcibios_allocate_dev_resources(dev, 1);
> +		}
> +	}
> +
>  	list_for_each_entry(child, &bus->children, node)
>  		pcibios_allocate_bus_resources(child);
>  }
> -- 
> 2.13.6
> 

  reply index

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-24 11:01 Lee, Chun-Yi
2018-08-06 21:48 ` Bjorn Helgaas [this message]
2018-08-08 15:53   ` joeyli
2018-08-08 21:23     ` Bjorn Helgaas
2018-08-10  9:25       ` joeyli
2018-08-10 13:58         ` Bjorn Helgaas
2018-08-12  0:15           ` joeyli
2018-08-13 18:45             ` Bjorn Helgaas

Reply instructions:

You may reply publically 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=20180806214807.GE30691@bhelgaas-glaptop.roam.corp.google.com \
    --to=helgaas@kernel.org \
    --cc=bhelgaas@google.com \
    --cc=hpa@zytor.com \
    --cc=jlee@suse.com \
    --cc=joeyli.kernel@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox