From: Bjorn Helgaas <helgaas@kernel.org>
To: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Cc: Austin.Bolen@dell.com, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, ashok.raj@intel.com
Subject: Re: [PATCH v17 09/12] PCI/AER: Allow clearing Error Status Register in FF mode
Date: Thu, 12 Mar 2020 17:36:51 -0500 [thread overview]
Message-ID: <20200312223651.GA202733@google.com> (raw)
In-Reply-To: <e710fd4c-4c0e-448a-6791-beed334536ce@linux.intel.com>
On Thu, Mar 12, 2020 at 03:02:07PM -0700, Kuppuswamy Sathyanarayanan wrote:
> On 3/12/20 2:52 PM, Bjorn Helgaas wrote:
> > On Thu, Mar 12, 2020 at 02:29:58PM -0700, Kuppuswamy Sathyanarayanan wrote:
> > > On 3/12/20 2:02 PM, Austin.Bolen@dell.com wrote:
> > > > On 3/12/2020 2:53 PM, Bjorn Helgaas wrote:
> > > > > On Wed, Mar 11, 2020 at 04:07:59PM -0700, Kuppuswamy Sathyanarayanan wrote:
> > > > > > On 3/11/20 3:23 PM, Bjorn Helgaas wrote:
> > > > > > > Is any synchronization needed here between the EDR path and the
> > > > > > > hotplug/enumeration path?
> > > > > > If we want to follow the implementation note step by step (in
> > > > > > sequence) then we need some synchronization between EDR path and
> > > > > > enumeration path. But if it's OK to achieve the same end result by
> > > > > > following steps out of sequence then we don't need to create any
> > > > > > dependency between EDR and enumeration paths. Currently we follow
> > > > > > the latter approach.
> > > > > What would the synchronization look like?
> > > > >
> > > > > Ideally I think it would be better to follow the order in the
> > > > > flowchart if it's not too onerous. That will make the code easier to
> > > > > understand. The current situation with this dependency on pciehp and
> > > > > what it will do leaves a lot of things implicit.
> > > > >
> > > > > What happens if CONFIG_PCIE_EDR=y but CONFIG_HOTPLUG_PCI_PCIE=n?
> > > > >
> > > > > IIUC, when DPC triggers, pciehp is what fields the DLLSC interrupt and
> > > > > unbinds the drivers and removes the devices. If that doesn't happen,
> > > > > and Linux clears the DPC trigger to bring the link back up, will those
> > > > > drivers try to operate uninitialized devices?
> > > > >
> > > > > Does EDR need a dependency on CONFIG_HOTPLUG_PCI_PCIE?
> > > > From one of Sathya's other responses:
> > > >
> > > > "If hotplug is not supported then there is support to enumerate
> > > > devices via polling or ACPI events. But a point to note
> > > > here is, enumeration path is independent of error handler path, and
> > > > hence there is no explicit trigger or event from error handler path
> > > > to enumeration path to kick start the enumeration."
> > > >
> > > > The EDR standard doesn't have any dependency on hot-plug. It sounds like
> > > > in the current implementation there's some manual intervention needed if
> > > > hot-plug is not supported?
> > >
> > > No, there is no need for manual intervention even in non hotplug
> > > cases.
> > >
> > > For ACPI events case, we would rely on ACPI event to kick start the
> > > enumeration. And for polling model, there is an independent polling
> > > thread which will kick start the enumeration.
>
> > I'm guessing the ACPI case works via hotplug_is_native(): if
> > CONFIG_HOTPLUG_PCI_PCIE=n, pciehp_is_native() returns false, and
> > acpiphp manages hotplug.
> >
> > What if CONFIG_HOTPLUG_PCI_ACPI=n also?
>
> If none of the auto scans are enabled then we might need some
> manual trigger (rescan). But this would be needed in native
> DPC case as well.
> >
> > Where is the polling thread?
>
> drivers/pci/hotplug/pciehp_hpc.c
Only if CONFIG_HOTPLUG_PCI_PCIE=y, obviously. My question is about
what happens when CONFIG_HOTPLUG_PCI_PCIE=n.
I'm not as concerned about requiring a manual rescan. That's
inconvenient, but doesn't seem like a big deal because that's what you
expect with no hotplug driver.
What I *am* worried about is calling driver callbacks on a device that
has been reset but not initialized. That could cause all sorts of
havoc because the driver thinks it can trust BARs and other
configuration.
next prev parent reply other threads:[~2020-03-12 22:36 UTC|newest]
Thread overview: 67+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-04 2:36 [PATCH v17 00/12] Add Error Disconnect Recover (EDR) support sathyanarayanan.kuppuswamy
2020-03-04 2:36 ` [PATCH v17 01/12] PCI/ERR: Update error status after reset_link() sathyanarayanan.kuppuswamy
2020-03-04 2:36 ` [PATCH v17 02/12] PCI/AER: Move pci_cleanup_aer_error_status_regs() declaration to pci.h sathyanarayanan.kuppuswamy
2020-03-04 2:36 ` [PATCH v17 03/12] PCI/ERR: Remove service dependency in pcie_do_recovery() sathyanarayanan.kuppuswamy
2020-03-17 14:40 ` Christoph Hellwig
2020-03-04 2:36 ` [PATCH v17 04/12] PCI: portdrv: remove unnecessary pcie_port_find_service() sathyanarayanan.kuppuswamy
2020-03-04 2:36 ` [PATCH v17 05/12] PCI: portdrv: remove reset_link member from pcie_port_service_driver sathyanarayanan.kuppuswamy
2020-03-17 14:41 ` Christoph Hellwig
2020-03-17 14:55 ` Kuppuswamy, Sathyanarayanan
2020-03-04 2:36 ` [PATCH v17 06/12] Documentation: PCI: Remove reset_link references sathyanarayanan.kuppuswamy
2020-03-17 14:42 ` Christoph Hellwig
2020-03-17 15:05 ` Kuppuswamy, Sathyanarayanan
2020-03-17 15:07 ` Christoph Hellwig
2020-03-17 16:03 ` Bjorn Helgaas
2020-03-17 17:06 ` Christoph Hellwig
2020-03-19 22:52 ` Bjorn Helgaas
2020-03-04 2:36 ` [PATCH v17 07/12] PCI/ERR: Return status of pcie_do_recovery() sathyanarayanan.kuppuswamy
2020-03-04 2:36 ` [PATCH v17 08/12] PCI/DPC: Cache DPC capabilities in pci_init_capabilities() sathyanarayanan.kuppuswamy
2020-03-04 2:36 ` [PATCH v17 09/12] PCI/AER: Allow clearing Error Status Register in FF mode sathyanarayanan.kuppuswamy
2020-03-06 5:45 ` Kuppuswamy, Sathyanarayanan
2020-03-06 16:04 ` Bjorn Helgaas
2020-03-06 16:11 ` Kuppuswamy, Sathyanarayanan
2020-03-06 16:41 ` Bjorn Helgaas
2020-03-10 2:40 ` Bjorn Helgaas
2020-03-10 4:28 ` Kuppuswamy, Sathyanarayanan
2020-03-10 18:14 ` Austin.Bolen
2020-03-10 19:32 ` Bjorn Helgaas
2020-03-10 20:06 ` Austin.Bolen
2020-03-10 20:41 ` Kuppuswamy Sathyanarayanan
2020-03-10 20:49 ` Austin.Bolen
2020-03-11 14:45 ` Bjorn Helgaas
2020-03-11 15:19 ` Austin.Bolen
2020-03-11 17:12 ` Bjorn Helgaas
2020-03-11 17:27 ` Austin.Bolen
2020-03-11 20:33 ` Bjorn Helgaas
2020-03-11 21:25 ` Kuppuswamy Sathyanarayanan
2020-03-11 21:53 ` Austin.Bolen
2020-03-11 22:11 ` Kuppuswamy Sathyanarayanan
2020-03-11 22:23 ` Bjorn Helgaas
2020-03-11 23:07 ` Kuppuswamy Sathyanarayanan
2020-03-12 19:53 ` Bjorn Helgaas
2020-03-12 21:02 ` Austin.Bolen
2020-03-12 21:29 ` Kuppuswamy Sathyanarayanan
2020-03-12 21:52 ` Bjorn Helgaas
2020-03-12 22:02 ` Kuppuswamy Sathyanarayanan
2020-03-12 22:36 ` Bjorn Helgaas [this message]
2020-03-12 21:59 ` Kuppuswamy Sathyanarayanan
2020-03-12 22:32 ` Bjorn Helgaas
2020-03-13 6:22 ` Kuppuswamy, Sathyanarayanan
2020-03-13 19:28 ` Bjorn Helgaas
2020-03-13 20:26 ` Kuppuswamy Sathyanarayanan
2020-03-19 23:03 ` Bjorn Helgaas
2020-03-19 23:20 ` Kuppuswamy, Sathyanarayanan
2020-03-11 22:13 ` Bjorn Helgaas
2020-03-11 22:41 ` Kuppuswamy Sathyanarayanan
2020-03-11 18:12 ` Kuppuswamy Sathyanarayanan
2020-03-11 22:05 ` Bjorn Helgaas
2020-03-04 2:36 ` [PATCH v17 10/12] PCI/DPC: Export DPC error recovery functions sathyanarayanan.kuppuswamy
2020-03-17 14:43 ` Christoph Hellwig
2020-03-04 2:36 ` [PATCH v17 11/12] PCI/DPC: Add Error Disconnect Recover (EDR) support sathyanarayanan.kuppuswamy
2020-03-06 3:47 ` Bjorn Helgaas
2020-03-06 6:32 ` Kuppuswamy, Sathyanarayanan
2020-03-06 21:00 ` Bjorn Helgaas
2020-03-06 22:42 ` Kuppuswamy Sathyanarayanan
2020-03-06 23:23 ` Bjorn Helgaas
2020-03-07 0:19 ` Kuppuswamy Sathyanarayanan
2020-03-04 2:36 ` [PATCH v17 12/12] PCI/ACPI: Enable EDR support sathyanarayanan.kuppuswamy
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=20200312223651.GA202733@google.com \
--to=helgaas@kernel.org \
--cc=Austin.Bolen@dell.com \
--cc=ashok.raj@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=sathyanarayanan.kuppuswamy@linux.intel.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).