| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20170124165945-mutt-send-email-mst@kernel.org>
Date: Tue, 24 Jan 2017 17:00:34 +0200
From: "Michael S. Tsirkin" <mst@...hat.com>
To: Doug Ledford <dledford@...hat.com>
Cc: linux-kernel@...r.kernel.org,
Linas Vepstas <linasvepstas@...il.com>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Jonathan Corbet <corbet@....net>,
Mike Marciniszyn <mike.marciniszyn@...el.com>,
Dennis Dalessandro <dennis.dalessandro@...el.com>,
Sean Hefty <sean.hefty@...el.com>,
Hal Rosenstock <hal.rosenstock@...il.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
linux-pci@...r.kernel.org, linux-doc@...r.kernel.org,
linux-rdma@...r.kernel.org, linux-media@...r.kernel.org
Subject: Re: [PATCH] pci: drop link_reset
On Wed, Jan 18, 2017 at 07:19:48PM -0500, Doug Ledford wrote:
> On Wed, 2017-01-18 at 23:39 +0200, Michael S. Tsirkin wrote:
> > No hardware seems to actually call link_reset, and
> > no driver implements it as more than a nop stub.
> >
> > This drops the mentions of the callback from everywhere.
> > It's dropped from the documentation as well, but
> > the doc really needs to be updated to reflect
> > reality better (e.g. on pcie slot reset is the link reset).
> >
> > This will be done in a later patch.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@...hat.com>
>
> This is going to conflict with the two patches I have in my for-next
> branch related to this same thing (it drops the stubs from qib and
> hfi1). It would be easiest if I just added this to my for-next and
> fixed up the conflicts prior to submission.
>
> > ---
> > Documentation/PCI/pci-error-recovery.txt | 24 +++-------------------
> > --
> > drivers/infiniband/hw/hfi1/pcie.c | 10 ----------
> > drivers/infiniband/hw/qib/qib_pcie.c | 8 --------
> > drivers/media/pci/ngene/ngene-cards.c | 7 -------
> > include/linux/pci.h | 3 ---
> > 5 files changed, 3 insertions(+), 49 deletions(-)
> >
> > diff --git a/Documentation/PCI/pci-error-recovery.txt
> > b/Documentation/PCI/pci-error-recovery.txt
> > index ac26869..da3b217 100644
> > --- a/Documentation/PCI/pci-error-recovery.txt
> > +++ b/Documentation/PCI/pci-error-recovery.txt
> > @@ -78,7 +78,6 @@ struct pci_error_handlers
> > {
> > int (*error_detected)(struct pci_dev *dev, enum
> > pci_channel_state);
> > int (*mmio_enabled)(struct pci_dev *dev);
> > - int (*link_reset)(struct pci_dev *dev);
> > int (*slot_reset)(struct pci_dev *dev);
> > void (*resume)(struct pci_dev *dev);
> > };
> > @@ -104,8 +103,7 @@ if it implements any, it must implement
> > error_detected(). If a callback
> > is not implemented, the corresponding feature is considered
> > unsupported.
> > For example, if mmio_enabled() and resume() aren't there, then it
> > is assumed that the driver is not doing any direct recovery and
> > requires
> > -a slot reset. If link_reset() is not implemented, the card is
> > assumed to
> > -not care about link resets. Typically a driver will want to know
> > about
> > +a slot reset. Typically a driver will want to know about
> > a slot_reset().
> >
> > The actual steps taken by a platform to recover from a PCI error
> > @@ -232,25 +230,9 @@ proceeds to STEP 4 (Slot Reset)
> >
> > STEP 3: Link Reset
> > ------------------
> > -The platform resets the link, and then calls the link_reset()
> > callback
> > -on all affected device drivers. This is a PCI-Express specific
> > state
> > +The platform resets the link. This is a PCI-Express specific step
> > and is done whenever a non-fatal error has been detected that can be
> > -"solved" by resetting the link. This call informs the driver of the
> > -reset and the driver should check to see if the device appears to be
> > -in working condition.
> > -
> > -The driver is not supposed to restart normal driver I/O operations
> > -at this point. It should limit itself to "probing" the device to
> > -check its recoverability status. If all is right, then the platform
> > -will call resume() once all drivers have ack'd link_reset().
> > -
> > - Result codes:
> > - (identical to STEP 3 (MMIO Enabled)
> > -
> > -The platform then proceeds to either STEP 4 (Slot Reset) or STEP 5
> > -(Resume Operations).
> > -
> > ->>> The current powerpc implementation does not implement this
> > callback.
> > +"solved" by resetting the link.
> >
> > STEP 4: Slot Reset
> > ------------------
> > diff --git a/drivers/infiniband/hw/hfi1/pcie.c
> > b/drivers/infiniband/hw/hfi1/pcie.c
> > index 4ac8f33..ebd941f 100644
> > --- a/drivers/infiniband/hw/hfi1/pcie.c
> > +++ b/drivers/infiniband/hw/hfi1/pcie.c
> > @@ -598,15 +598,6 @@ pci_slot_reset(struct pci_dev *pdev)
> > return PCI_ERS_RESULT_CAN_RECOVER;
> > }
> >
> > -static pci_ers_result_t
> > -pci_link_reset(struct pci_dev *pdev)
> > -{
> > - struct hfi1_devdata *dd = pci_get_drvdata(pdev);
> > -
> > - dd_dev_info(dd, "HFI1 link_reset function called,
> > ignored\n");
> > - return PCI_ERS_RESULT_CAN_RECOVER;
> > -}
> > -
> > static void
> > pci_resume(struct pci_dev *pdev)
> > {
> > @@ -625,7 +616,6 @@ pci_resume(struct pci_dev *pdev)
> > const struct pci_error_handlers hfi1_pci_err_handler = {
> > .error_detected = pci_error_detected,
> > .mmio_enabled = pci_mmio_enabled,
> > - .link_reset = pci_link_reset,
> > .slot_reset = pci_slot_reset,
> > .resume = pci_resume,
> > };
> > diff --git a/drivers/infiniband/hw/qib/qib_pcie.c
> > b/drivers/infiniband/hw/qib/qib_pcie.c
> > index 6abe1c6..c379b83 100644
> > --- a/drivers/infiniband/hw/qib/qib_pcie.c
> > +++ b/drivers/infiniband/hw/qib/qib_pcie.c
> > @@ -682,13 +682,6 @@ qib_pci_slot_reset(struct pci_dev *pdev)
> > return PCI_ERS_RESULT_CAN_RECOVER;
> > }
> >
> > -static pci_ers_result_t
> > -qib_pci_link_reset(struct pci_dev *pdev)
> > -{
> > - qib_devinfo(pdev, "QIB link_reset function called,
> > ignored\n");
> > - return PCI_ERS_RESULT_CAN_RECOVER;
> > -}
> > -
> > static void
> > qib_pci_resume(struct pci_dev *pdev)
> > {
> > @@ -707,7 +700,6 @@ qib_pci_resume(struct pci_dev *pdev)
> > const struct pci_error_handlers qib_pci_err_handler = {
> > .error_detected = qib_pci_error_detected,
> > .mmio_enabled = qib_pci_mmio_enabled,
> > - .link_reset = qib_pci_link_reset,
> > .slot_reset = qib_pci_slot_reset,
> > .resume = qib_pci_resume,
> > };
> > diff --git a/drivers/media/pci/ngene/ngene-cards.c
> > b/drivers/media/pci/ngene/ngene-cards.c
> > index 423e8c8..8438c1c 100644
> > --- a/drivers/media/pci/ngene/ngene-cards.c
> > +++ b/drivers/media/pci/ngene/ngene-cards.c
> > @@ -781,12 +781,6 @@ static pci_ers_result_t
> > ngene_error_detected(struct pci_dev *dev,
> > return PCI_ERS_RESULT_CAN_RECOVER;
> > }
> >
> > -static pci_ers_result_t ngene_link_reset(struct pci_dev *dev)
> > -{
> > - printk(KERN_INFO DEVICE_NAME ": link reset\n");
> > - return 0;
> > -}
> > -
> > static pci_ers_result_t ngene_slot_reset(struct pci_dev *dev)
> > {
> > printk(KERN_INFO DEVICE_NAME ": slot reset\n");
> > @@ -800,7 +794,6 @@ static void ngene_resume(struct pci_dev *dev)
> >
> > static const struct pci_error_handlers ngene_errors = {
> > .error_detected = ngene_error_detected,
> > - .link_reset = ngene_link_reset,
> > .slot_reset = ngene_slot_reset,
> > .resume = ngene_resume,
> > };
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index 30d6c16..316379c 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -661,9 +661,6 @@ struct pci_error_handlers {
> > /* MMIO has been re-enabled, but not DMA */
> > pci_ers_result_t (*mmio_enabled)(struct pci_dev *dev);
> >
> > - /* PCI Express link has been reset */
> > - pci_ers_result_t (*link_reset)(struct pci_dev *dev);
> > -
> > /* PCI slot has been reset */
> > pci_ers_result_t (*slot_reset)(struct pci_dev *dev);
> >
> --
> Doug Ledford <dledford@...hat.com>
> GPG KeyID: B826A3330E572FDD
>
> Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
Linus resolves such conflicts easily, but if Bjorn acks,
that's fine with me too.
--
MST
Powered by blists - more mailing lists