lkml.org 
[lkml]   [2016]   [Sep]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/2] PCI: add CRS support to error handling path
On Thu, Sep 01, 2016 at 07:00:01PM -0400, Sinan Kaya wrote:
> The PCIE spec allows an endpoint device to extend the initialization time
> beyond 1 second by issuing Configuration Request Retry Status (CRS) for a
> vendor ID read request.
>
> This basically means "I'm busy now, please call me back later".
>
> There are two moving parts to CRS support from the SW perspective. One part
> is to determine if CRS is supported or not. The second part is to set the
> CRS visibility register.
>
> As part of the probe, the Linux kernel sets the above two conditions in
> pci_enable_crs function. The kernel is also honoring the returned CRS in
> pci_bus_read_dev_vendor_id function if supported. The function will poll up
> to specified amount of time while endpoint is returning CRS response.
>
> The PCIe spec also allows CRS to be issued during cold, warm, hot and FLR
> resets.
>
> The hot reset is initiated by starting a secondary bus reset. This patch is
> adding vendor ID read immediately after a bus reset so that the
> initialization procedure can be extended by the amount of time endpoint
> requires.
>
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
> drivers/pci/pci.c | 39 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index b209378..ebd0fc6 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3829,6 +3829,44 @@ static int pci_pm_reset(struct pci_dev *dev, int probe)
> return 0;
> }
>
> +/*
> + * Mostly copy paste from pci_walk_bus with the exceptions of hard coded
> + * work and removed locks.
> + */
> +static void pci_bus_probe_crs(struct pci_bus *top)
> +{
> + struct pci_dev *dev;
> + struct pci_bus *bus;
> + struct list_head *next;
> + int retval;
> + u32 l;
> +
> + bus = top;
> + next = top->devices.next;
> + for (;;) {
> + if (next == &bus->devices) {
> + /* end of this bus, go up or finish */
> + if (bus == top)
> + break;
> + next = bus->self->bus_list.next;
> + bus = bus->self->bus;
> + continue;
> + }
> + dev = list_entry(next, struct pci_dev, bus_list);
> + if (dev->subordinate) {
> + /* this is a pci-pci bridge, do its devices next */
> + next = dev->subordinate->devices.next;
> + bus = dev->subordinate;
> + } else
> + next = dev->bus_list.next;
> +
> + retval = pci_bus_read_dev_vendor_id(dev->bus, dev->devfn, &l,
> + 60 * 1000);
> + if (retval)
> + break;
> + }
> +}

Sigh. Man, this is ugly. Maybe we're locked into the current
strategy and don't really have a choice, but I really don't like it.

> +
> void pci_reset_secondary_bus(struct pci_dev *dev)
> {
> u16 ctrl;
> @@ -4361,6 +4399,7 @@ void pci_reset_bridge_secondary_bus(struct pci_dev *dev)
> pci_bus_save_and_disable(dev->bus);
> pcibios_reset_secondary_bus(dev);
> pci_bus_restore(dev->bus);
> + pci_bus_probe_crs(dev->subordinate);

This looks backwards -- pci_bus_restore() uses config accesses, so surely
you want to do the CRS check *before* that, right? Oh, never mind, I see
you already caught this.

You mentioned several kinds of reset where CRS is allowed. Doesn't this
fix only one of them? I know we support at least FLR reset also.

> }
> EXPORT_SYMBOL_GPL(pci_reset_bridge_secondary_bus);
>
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html

\
 
 \ /
  Last update: 2016-09-17 09:59    [W:0.090 / U:1.288 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site