Linux-PCI Archive on lore.kernel.org
 help / color / Atom feed
From: "Oliver O'Halloran" <oohall@gmail.com>
To: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
Cc: linux-pci@vger.kernel.org,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	Bjorn Helgaas <helgaas@kernel.org>,
	linux@yadro.com
Subject: Re: [PATCH v5 18/23] powerpc/pci: Handle BAR movement
Date: Tue, 10 Sep 2019 00:02:30 +1000
Message-ID: <CAOSf1CG-oaKazKzZCULUjntc+3-dztiQ3U=6tcWu+OGer_77Ag@mail.gmail.com> (raw)
In-Reply-To: <7af3d4cd-b786-19b1-1ddf-b93f9875976d@yadro.com>

On Sat, Sep 7, 2019 at 2:25 AM Sergey Miroshnichenko
<s.miroshnichenko@yadro.com> wrote:
>
> Hi Oliver,
>
> On 9/4/19 8:37 AM, Oliver O'Halloran wrote:
> > On Fri, 2019-08-16 at 19:50 +0300, Sergey Miroshnichenko wrote:
> >> Add pcibios_rescan_prepare()/_done() hooks for the powerpc platform. Now if
> >> the device's driver supports movable BARs, pcibios_rescan_prepare() will be
> >> called after the device is stopped, and pcibios_rescan_done() - before it
> >> resumes. There are no memory requests to this device between the hooks, so
> >> it it safe to rebuild the EEH address cache during that.
> >>
> >> CC: Oliver O'Halloran <oohall@gmail.com>
> >> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
> >> ---
> >>  arch/powerpc/kernel/pci-hotplug.c | 10 ++++++++++
> >>  1 file changed, 10 insertions(+)
> >>
> >> diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c
> >> index 0b0cf8168b47..18cf13bba228 100644
> >> --- a/arch/powerpc/kernel/pci-hotplug.c
> >> +++ b/arch/powerpc/kernel/pci-hotplug.c
> >> @@ -144,3 +144,13 @@ void pci_hp_add_devices(struct pci_bus *bus)
> >>      pcibios_finish_adding_to_bus(bus);
> >>  }
> >>  EXPORT_SYMBOL_GPL(pci_hp_add_devices);
> >> +
> >> +void pcibios_rescan_prepare(struct pci_dev *pdev)
> >> +{
> >> +    eeh_addr_cache_rmv_dev(pdev);
> >> +}
> >> +
> >> +void pcibios_rescan_done(struct pci_dev *pdev)
> >> +{
> >> +    eeh_addr_cache_insert_dev(pdev);
> >> +}
> >
> > Is this actually sufficent? The PE number for a device is largely
> > determined by the location of the MMIO BARs. If you move a BAR far
> > enough the PE number stored in the eeh_pe would need to be updated as
> > well.
> >
>
> Thanks for the hint! I've checked on our PowerNV: for bridges with MEM
> only it allocates PE numbers starting from 0xff down, and when there
> are MEM64 - starting from 0 up, one PE number per 4GiB.
>
> PEs are allocated during call to pnv_pci_setup_bridge(), and the I've
> added invocation of pci_setup_bridge() after a hotplug event in the
> "Recalculate all bridge windows during rescan" patch of this series.

Sort of.

On PHB3 both the 32bit and the 64bit MMIO windows are split into 256
segments each of which is mapped to a PE number. For the 32bit space
there's a remapping table in hardware that allows arbitrary mapping of
segments to PE numbers, but in the 64bit space the mapping is fixed
with the first segment being PE0, etc. If there's a 64 bit BAR under a
bridge the PE is really "allocated" during the BAR assignment process,
and the setup_bridge() step sets up the EEH state based on that.

It's worth pointing out that this is why the 64bit window is usually
4GB. Bridge windows need to be aligned to a segment boundary to ensure
the devices under them are placed into a unique PE.

> Currently, if a bus already has a PE, pnv_ioda_setup_bus_PE() takes it
> and returns. I can see two ways to change it, both are not difficult to
> implement:
>
>  a.1) check if MEM64 BARs appeared below the bus - allocate and assign
>       a new master PE with required number of slave PEs;
>
>  a.2) if the bus now has more MEM64 than before - check if more slave
>       PEs must be reserved;
>
>  b) release all the PEs before a PCI rescan and allocate+assign them
>     again after - with this approach the "Hook up the writes to
>     PCI_SECONDARY_BUS register" patch may be eliminated.
>
> Do you find any of these suitable?

I'm not sure a) would work, but even if it does b) is preferable.
There's a lot of strangeness in the powerpc PCI code as-is without
adding extra code paths to deal with. Keeping what happens at hotplug
consistent with what happens at boot will help keep things sane.

FYI in the next few days I'm going to post a series that rips out the
use of pci_dn in powernv and the generic parts of EEH (pseries still
uses it). Assuming Bjorn isn't picking this up for 5.4 you might want
to wait for that before getting too deep into this.

Oliver

  reply index

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-16 16:50 [PATCH v5 00/23] PCI: Allow BAR movement during hotplug Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 01/23] PCI: Fix race condition in pci_enable/disable_device() Sergey Miroshnichenko
2019-08-22 12:37   ` Marta Rybczynska
2019-09-27 21:59   ` Bjorn Helgaas
2019-09-30  8:53     ` Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 02/23] PCI: Enable bridge's I/O and MEM access for hotplugged devices Sergey Miroshnichenko
2019-09-27 22:01   ` Bjorn Helgaas
2019-08-16 16:50 ` [PATCH v5 03/23] PCI: hotplug: Add a flag for the movable BARs feature Sergey Miroshnichenko
2019-09-27 22:02   ` Bjorn Helgaas
2019-09-30  8:44     ` David Laight
2019-09-30 16:17       ` Sergey Miroshnichenko
2019-09-30 12:59     ` Sergey Miroshnichenko
2019-10-15 22:14       ` Bjorn Helgaas
2019-10-16 15:50         ` Sergey Miroshnichenko
2019-10-16 17:29           ` Bjorn Helgaas
2019-08-16 16:50 ` [PATCH v5 04/23] PCI: Define PCI-specific version of the release_child_resources() Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 05/23] PCI: hotplug: movable BARs: Fix reassigning the released bridge windows Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 06/23] PCI: hotplug: movable BARs: Recalculate all bridge windows during rescan Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 07/23] PCI: hotplug: movable BARs: Don't allow added devices to steal resources Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 08/23] PCI: Include fixed and immovable BARs into the bus size calculating Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 09/23] PCI: Prohibit assigning BARs and bridge windows to non-direct parents Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 10/23] PCI: hotplug: movable BARs: Try to assign unassigned resources only once Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 11/23] PCI: hotplug: movable BARs: Calculate immovable parts of bridge windows Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 12/23] PCI: hotplug: movable BARs: Compute limits for relocated " Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 13/23] PCI: Make sure bridge windows include their fixed BARs Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 14/23] PCI: Fix assigning the fixed prefetchable resources Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 15/23] PCI: hotplug: movable BARs: Assign fixed and immovable BARs before others Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 16/23] PCI: hotplug: movable BARs: Don't reserve IO/mem bus space Sergey Miroshnichenko
2019-09-04  5:42   ` Oliver O'Halloran
2019-09-04 11:22     ` Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 17/23] powerpc/pci: Fix crash with enabled movable BARs Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 18/23] powerpc/pci: Handle BAR movement Sergey Miroshnichenko
2019-09-04  5:37   ` Oliver O'Halloran
2019-09-06 16:24     ` Sergey Miroshnichenko
2019-09-09 14:02       ` Oliver O'Halloran [this message]
2019-08-16 16:50 ` [PATCH v5 19/23] PCI: hotplug: Configure MPS for hot-added bridges during bus rescan Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 20/23] PCI: hotplug: movable BARs: Enable the feature by default Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 21/23] nvme-pci: Handle movable BARs Sergey Miroshnichenko
2019-08-16 16:51 ` [PATCH v5 22/23] PCI/portdrv: Declare support of " Sergey Miroshnichenko
2019-08-16 16:51 ` [PATCH v5 23/23] PCI: pciehp: movable BARs: Trigger a domain rescan on hp events Sergey Miroshnichenko

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='CAOSf1CG-oaKazKzZCULUjntc+3-dztiQ3U=6tcWu+OGer_77Ag@mail.gmail.com' \
    --to=oohall@gmail.com \
    --cc=helgaas@kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux@yadro.com \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=s.miroshnichenko@yadro.com \
    /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

Linux-PCI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pci/0 linux-pci/git/0.git

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

Example config snippet for mirrors

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


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