linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bjorn Helgaas <bjorn.helgaas@gmail.com>
To: "Derrick, Jonathan" <jonathan.derrick@intel.com>
Cc: "helgaas@kernel.org" <helgaas@kernel.org>,
	"mr.nuke.me@gmail.com" <mr.nuke.me@gmail.com>,
	"hch@lst.de" <hch@lst.de>,
	"Shevchenko, Andriy" <andriy.shevchenko@intel.com>,
	"lorenzo.pieralisi@arm.com" <lorenzo.pieralisi@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"mika.westerberg@linux.intel.com"
	<mika.westerberg@linux.intel.com>,
	"Baldysiak, Pawel" <pawel.baldysiak@intel.com>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	"lukas@wunner.de" <lukas@wunner.de>,
	"okaya@kernel.org" <okaya@kernel.org>,
	"kbusch@kernel.org" <kbusch@kernel.org>,
	"stuart.w.hayes@gmail.com" <stuart.w.hayes@gmail.com>
Subject: Re: [RFC 0/9] PCIe Hotplug Slot Emulation driver
Date: Wed, 1 Apr 2020 16:45:49 -0500	[thread overview]
Message-ID: <CABhMZUWZOc8n0mU4wL7VX88w936HGaLJHFYVEpRdSUKqhbsdrw@mail.gmail.com> (raw)
In-Reply-To: <97b916ad6ad03f39ccdf5b62fe7d7b9e10190708.camel@intel.com>

On Mon, Mar 30, 2020 at 12:43 PM Derrick, Jonathan
<jonathan.derrick@intel.com> wrote:
> On Sat, 2020-03-28 at 16:51 -0500, Bjorn Helgaas wrote:
> > On Fri, Feb 07, 2020 at 04:59:58PM -0700, Jon Derrick wrote:
> > > This set adds an emulation driver for PCIe Hotplug. There may be platforms with
> > > specific configurations that can support hotplug but don't provide the logical
> > > slot hotplug hardware. For instance, the platform may use an
> > > electrically-tolerant interposer between the slot and the device.
> > > ...

> > There's a lot of good work in here, and I don't claim to understand
> > the use case and all the benefits.
> I've received more info that the customer use case is an AIC that
> breaks out 1-4 M.2 cards which have been made hotplug tolerant.
>
> > But it seems like quite a lot of additional code and complexity in an
> > area that's already pretty subtle, so I'm not yet convinced that it's
> > all worthwhile.
> >
> > It seems like this would rely on Data Link Layer Link Active
> > Reporting.  Is that something we could add to pciehp as a generic
> > feature without making a separate driver for it?  I haven't looked at
> > this for a while, but I would assume that if we find out that a link
> > went down, pciehp could/should be smart enough to notice that even if
> > it didn't come via the usual pciehp Slot Status path.
> I had a plan to do V2 by intercepting bus_ops rather than indirecting
> slot_ops in pciehp. That should touch /a lot/ less code.

I assume this is something like pci_bus_set_ops() or
pci_bus_set_aer_ops()?  Probably touches less code, but I'm not really
a fan of either of those current situations because they make things
magical -- there's a lot of stuff going on behind the curtain, and it
makes another thing to consider when we evaluate every pciehp change.

> The problem I saw with adding DLLLA as a primary signal in pciehp is
> that most of the pciehp boilerplate relies on valid Slot register
> logic. I don't know how reliable pciehp will be if there's no backing
> slot register logic, emulated or real. Consider how many slot
> capability reads are in hpc.
>
> I could add a non-slot flag check to each of those callers, but it
> might be worse than the emulation alternative.

I see what you mean -- there are quite a few reads of PCI_EXP_SLTSTA.
I'm not 100% sure all of those really need to exist.  I would expect
that we'd read it once in the ISR and then operate on that value.  So
maybe there's some middle ground of restructuring to remove some of
those reads and making the remaining few more generic.

All that being said, I'm also sympathetic to Christoph's concern about
cluttering pciehp to deal with non-standard topologies.  At some point
if you need to do non-standard things you may have to write and
maintain your own drivers.  I don't know where that point is yet.

Bjorn

      parent reply	other threads:[~2020-04-01 21:46 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-07 23:59 [RFC 0/9] PCIe Hotplug Slot Emulation driver Jon Derrick
2020-02-07 23:59 ` [RFC 1/9] PCI: pci-bridge-emul: Update PCIe register behaviors Jon Derrick
2020-02-08  9:55   ` Andy Shevchenko
2020-03-28 21:42   ` Bjorn Helgaas
2020-02-08  0:00 ` [RFC 2/9] PCI: pci-bridge-emul: Eliminate reserved member Jon Derrick
2020-03-28 21:43   ` Bjorn Helgaas
2020-02-08  0:00 ` [RFC 3/9] PCI: pci-bridge-emul: Provide a helper to set behavior Jon Derrick
2020-02-08  0:00 ` [RFC 4/9] PCI: pciehp: Indirect slot register operations Jon Derrick
2020-02-08  0:00 ` [RFC 5/9] PCI: Add pcie_port_slot_emulated stub Jon Derrick
2020-02-08  0:00 ` [RFC 6/9] PCI: pciehp: Expose the poll loop to other drivers Jon Derrick
2020-02-08  0:00 ` [RFC 7/9] PCI: Move pci_dev_str_match to search.c Jon Derrick
2020-02-08  0:00 ` [RFC 8/9] PCI: pciehp: Add hotplug slot emulation driver Jon Derrick
2020-02-08  0:00 ` [RFC 9/9] PCI: pciehp: Wire up pcie_port_emulate_slot and pciehp_emul Jon Derrick
2020-02-10  7:01 ` [RFC 0/9] PCIe Hotplug Slot Emulation driver Christoph Hellwig
2020-02-10 15:05   ` Derrick, Jonathan
2020-02-10 16:58     ` hch
2020-02-10 17:09       ` Derrick, Jonathan
2020-03-28 21:51 ` Bjorn Helgaas
2020-03-30 17:43   ` Derrick, Jonathan
2020-03-30 17:49     ` hch
2020-04-01 21:45     ` Bjorn Helgaas [this message]

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=CABhMZUWZOc8n0mU4wL7VX88w936HGaLJHFYVEpRdSUKqhbsdrw@mail.gmail.com \
    --to=bjorn.helgaas@gmail.com \
    --cc=andriy.shevchenko@intel.com \
    --cc=bjorn@helgaas.com \
    --cc=hch@lst.de \
    --cc=helgaas@kernel.org \
    --cc=jonathan.derrick@intel.com \
    --cc=kbusch@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=lukas@wunner.de \
    --cc=mika.westerberg@linux.intel.com \
    --cc=mr.nuke.me@gmail.com \
    --cc=okaya@kernel.org \
    --cc=pawel.baldysiak@intel.com \
    --cc=stuart.w.hayes@gmail.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
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).