* [PATCH 1/2] PCI/AER: Remove aerdriver.nosourceid kernel parameter
2016-09-06 22:19 [PATCH 0/2] PCI/AER: Remove module parameters Bjorn Helgaas
@ 2016-09-06 22:19 ` Bjorn Helgaas
2016-09-06 22:20 ` [PATCH 2/2] PCI/AER: Remove aerdriver.forceload " Bjorn Helgaas
2016-09-14 20:30 ` [PATCH 0/2] PCI/AER: Remove module parameters Bjorn Helgaas
2 siblings, 0 replies; 6+ messages in thread
From: Bjorn Helgaas @ 2016-09-06 22:19 UTC (permalink / raw)
To: linux-pci
Cc: Lance Ortiz, Betty Dall, Hidetoshi Seto, Chen Gong,
KeithBuschkeith.busch, Rafael J. Wysocki, linux-kernel,
Shuah Khan, Huang Ying, Vijay Mohan Pandarathil, Jon Derrick
The aerdriver.nosourceid kernel parameter was intended for working around
broken chipsets don't supply the source ID for AER events. We recently
added PCI_BUS_FLAGS_NO_AERSID, which can be set by quirks for the same
purpose.
Remove the aerdriver.nosourceid kernel parameter. For anything other than
debugging, asking users to find and use kernel parameters is a poor user
experience. Instead, we should add PCI_BUS_FLAGS_NO_AERSID quirks for any
hardware that needs it.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
Documentation/PCI/pcieaer-howto.txt | 4 ----
drivers/pci/pcie/aer/aerdrv_core.c | 12 ++++--------
2 files changed, 4 insertions(+), 12 deletions(-)
diff --git a/Documentation/PCI/pcieaer-howto.txt b/Documentation/PCI/pcieaer-howto.txt
index b4987c0..4956df3 100644
--- a/Documentation/PCI/pcieaer-howto.txt
+++ b/Documentation/PCI/pcieaer-howto.txt
@@ -61,10 +61,6 @@ be initiated although firmwares have no _OSC support. To enable the
walkaround, pls. add aerdriver.forceload=y to kernel boot parameter line
when booting kernel. Note that forceload=n by default.
-nosourceid, another parameter of type bool, can be used when broken
-hardware (mostly chipsets) has root ports that cannot obtain the reporting
-source ID. nosourceid=n by default.
-
2.3 AER error output
When a PCI-E AER error is captured, an error message will be outputted to
console. If it's a correctable error, it is outputted as a warning.
diff --git a/drivers/pci/pcie/aer/aerdrv_core.c b/drivers/pci/pcie/aer/aerdrv_core.c
index 8f5e14c..f8a9b17 100644
--- a/drivers/pci/pcie/aer/aerdrv_core.c
+++ b/drivers/pci/pcie/aer/aerdrv_core.c
@@ -28,9 +28,7 @@
#include "aerdrv.h"
static bool forceload;
-static bool nosourceid;
module_param(forceload, bool, 0);
-module_param(nosourceid, bool, 0);
#define PCI_EXP_AER_FLAGS (PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE | \
PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE)
@@ -132,8 +130,7 @@ static bool is_error_source(struct pci_dev *dev, struct aer_err_info *e_info)
* When bus id is equal to 0, it might be a bad id
* reported by root port.
*/
- if (!nosourceid &&
- (PCI_BUS_NUM(e_info->id) != 0) &&
+ if ((PCI_BUS_NUM(e_info->id) != 0) &&
!(dev->bus->bus_flags & PCI_BUS_FLAGS_NO_AERSID)) {
/* Device ID match? */
if (e_info->id == ((dev->bus->number << 8) | dev->devfn))
@@ -146,11 +143,10 @@ static bool is_error_source(struct pci_dev *dev, struct aer_err_info *e_info)
/*
* When either
- * 1) nosourceid==y;
- * 2) bus id is equal to 0. Some ports might lose the bus
+ * 1) bus id is equal to 0. Some ports might lose the bus
* id of error source id;
- * 3) bus flag PCI_BUS_FLAGS_NO_AERSID is set
- * 4) There are multiple errors and prior ID comparing fails;
+ * 2) bus flag PCI_BUS_FLAGS_NO_AERSID is set
+ * 3) There are multiple errors and prior ID comparing fails;
* We check AER status registers to find possible reporter.
*/
if (atomic_read(&dev->enable_cnt) == 0)
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] PCI/AER: Remove aerdriver.forceload kernel parameter
2016-09-06 22:19 [PATCH 0/2] PCI/AER: Remove module parameters Bjorn Helgaas
2016-09-06 22:19 ` [PATCH 1/2] PCI/AER: Remove aerdriver.nosourceid kernel parameter Bjorn Helgaas
@ 2016-09-06 22:20 ` Bjorn Helgaas
2016-09-14 20:30 ` [PATCH 0/2] PCI/AER: Remove module parameters Bjorn Helgaas
2 siblings, 0 replies; 6+ messages in thread
From: Bjorn Helgaas @ 2016-09-06 22:20 UTC (permalink / raw)
To: linux-pci
Cc: Lance Ortiz, Betty Dall, Hidetoshi Seto, Chen Gong,
KeithBuschkeith.busch, Rafael J. Wysocki, linux-kernel,
Shuah Khan, Huang Ying, Vijay Mohan Pandarathil, Jon Derrick
Per the PCI Firmware spec, r3.0, sec 4.5.1, on ACPI systems, the OS must
not use AER unless _OSC is present and _OSC grants AER control to the OS.
The aerdriver.forceload kernel parameter was a way to enable Linux AER
support on ACPI systems that lack _OSC or fail to grant control the the OS.
Enabling Linux AER support when the firmware doesn't want us to is a recipe
for problems, e.g., the firmware might be handling AER itself.
Remove the aerdriver.forceload kernel parameter and related supporting
code.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
Documentation/PCI/pcieaer-howto.txt | 22 +++++++++-------------
drivers/pci/pcie/aer/aerdrv.c | 7 +------
drivers/pci/pcie/aer/aerdrv.h | 8 --------
drivers/pci/pcie/aer/aerdrv_core.c | 19 -------------------
4 files changed, 10 insertions(+), 46 deletions(-)
diff --git a/Documentation/PCI/pcieaer-howto.txt b/Documentation/PCI/pcieaer-howto.txt
index 4956df3..ea8cafb 100644
--- a/Documentation/PCI/pcieaer-howto.txt
+++ b/Documentation/PCI/pcieaer-howto.txt
@@ -49,21 +49,17 @@ depends on CONFIG_PCIEPORTBUS, so pls. set CONFIG_PCIEPORTBUS=y and
CONFIG_PCIEAER = y.
2.2 Load PCI Express AER Root Driver
-There is a case where a system has AER support in BIOS. Enabling the AER
-Root driver and having AER support in BIOS may result unpredictable
-behavior. To avoid this conflict, a successful load of the AER Root driver
-requires ACPI _OSC support in the BIOS to allow the AER Root driver to
-request for native control of AER. See the PCI FW 3.0 Specification for
-details regarding OSC usage. Currently, lots of firmwares don't provide
-_OSC support while they use PCI Express. To support such firmwares,
-forceload, a parameter of type bool, could enable AER to continue to
-be initiated although firmwares have no _OSC support. To enable the
-walkaround, pls. add aerdriver.forceload=y to kernel boot parameter line
-when booting kernel. Note that forceload=n by default.
+
+Some systems have AER support in firmware. Enabling Linux AER support at
+the same time the firmware handles AER may result in unpredictable
+behavior. Therefore, Linux does not handle AER events unless the firmware
+grants AER control to the OS via the ACPI _OSC method. See the PCI FW 3.0
+Specification for details regarding _OSC usage.
2.3 AER error output
-When a PCI-E AER error is captured, an error message will be outputted to
-console. If it's a correctable error, it is outputted as a warning.
+
+When a PCIe AER error is captured, an error message will be output to
+console. If it's a correctable error, it is output as a warning.
Otherwise, it is printed as an error. So users could choose different
log level to filter out correctable error messages.
diff --git a/drivers/pci/pcie/aer/aerdrv.c b/drivers/pci/pcie/aer/aerdrv.c
index 48d21e0..08ce257 100644
--- a/drivers/pci/pcie/aer/aerdrv.c
+++ b/drivers/pci/pcie/aer/aerdrv.c
@@ -70,7 +70,7 @@ static int pcie_aer_disable;
void pci_no_aer(void)
{
- pcie_aer_disable = 1; /* has priority over 'forceload' */
+ pcie_aer_disable = 1;
}
bool pci_aer_available(void)
@@ -304,11 +304,6 @@ static int aer_probe(struct pcie_device *dev)
struct aer_rpc *rpc;
struct device *device = &dev->device;
- /* Init */
- status = aer_init(dev);
- if (status)
- return status;
-
/* Alloc rpc data structure */
rpc = aer_alloc_rpc(dev);
if (!rpc) {
diff --git a/drivers/pci/pcie/aer/aerdrv.h b/drivers/pci/pcie/aer/aerdrv.h
index 945c939..f15ca8d 100644
--- a/drivers/pci/pcie/aer/aerdrv.h
+++ b/drivers/pci/pcie/aer/aerdrv.h
@@ -105,7 +105,6 @@ static inline pci_ers_result_t merge_result(enum pci_ers_result orig,
}
extern struct bus_type pcie_port_bus_type;
-int aer_init(struct pcie_device *dev);
void aer_isr(struct work_struct *work);
void aer_print_error(struct pci_dev *dev, struct aer_err_info *info);
void aer_print_port_info(struct pci_dev *dev, struct aer_err_info *info);
@@ -121,11 +120,4 @@ static inline int pcie_aer_get_firmware_first(struct pci_dev *pci_dev)
return 0;
}
#endif
-
-static inline void pcie_aer_force_firmware_first(struct pci_dev *pci_dev,
- int enable)
-{
- pci_dev->__aer_firmware_first = !!enable;
- pci_dev->__aer_firmware_first_valid = 1;
-}
#endif /* _AERDRV_H_ */
diff --git a/drivers/pci/pcie/aer/aerdrv_core.c b/drivers/pci/pcie/aer/aerdrv_core.c
index f8a9b17..8262527 100644
--- a/drivers/pci/pcie/aer/aerdrv_core.c
+++ b/drivers/pci/pcie/aer/aerdrv_core.c
@@ -27,9 +27,6 @@
#include <linux/kfifo.h>
#include "aerdrv.h"
-static bool forceload;
-module_param(forceload, bool, 0);
-
#define PCI_EXP_AER_FLAGS (PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE | \
PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE)
@@ -811,19 +808,3 @@ void aer_isr(struct work_struct *work)
aer_isr_one_error(p_device, &e_src);
mutex_unlock(&rpc->rpc_mutex);
}
-
-/**
- * aer_init - provide AER initialization
- * @dev: pointer to AER pcie device
- *
- * Invoked when AER service driver is loaded.
- */
-int aer_init(struct pcie_device *dev)
-{
- if (forceload) {
- dev_printk(KERN_DEBUG, &dev->device,
- "aerdrv forceload requested.\n");
- pcie_aer_force_firmware_first(dev->port, 0);
- }
- return 0;
-}
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 0/2] PCI/AER: Remove module parameters
2016-09-06 22:19 [PATCH 0/2] PCI/AER: Remove module parameters Bjorn Helgaas
2016-09-06 22:19 ` [PATCH 1/2] PCI/AER: Remove aerdriver.nosourceid kernel parameter Bjorn Helgaas
2016-09-06 22:20 ` [PATCH 2/2] PCI/AER: Remove aerdriver.forceload " Bjorn Helgaas
@ 2016-09-14 20:30 ` Bjorn Helgaas
2016-09-17 4:49 ` Tang, CQ
2 siblings, 1 reply; 6+ messages in thread
From: Bjorn Helgaas @ 2016-09-14 20:30 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, Lance Ortiz, Betty Dall, Hidetoshi Seto, Chen Gong,
KeithBuschkeith.busch, Rafael J. Wysocki, linux-kernel,
Shuah Khan, Huang Ying, Vijay Mohan Pandarathil, Jon Derrick
On Tue, Sep 06, 2016 at 05:19:22PM -0500, Bjorn Helgaas wrote:
> I'd like to remove the aerdriver.nosourceid and aerdriver.forceload kernel
> parameters. They seem like basically debugging things that are not really
> supportable upstream.
>
> "nosourceid" may have some legitimate uses, but I'd rather it were done via
> quirks with the flag Jon Derrick recently added so it happens
> automatically, without users having to specify the parameter.
>
> I don't *know* of any actual users of either of these, but obviously that
> doesn't mean there are none. Comments welcome.
>
> ---
>
> Bjorn Helgaas (2):
> PCI/AER: Remove aerdriver.nosourceid kernel parameter
> PCI/AER: Remove aerdriver.forceload kernel parameter
>
>
> Documentation/PCI/pcieaer-howto.txt | 26 +++++++++-----------------
> drivers/pci/pcie/aer/aerdrv.c | 7 +------
> drivers/pci/pcie/aer/aerdrv.h | 8 --------
> drivers/pci/pcie/aer/aerdrv_core.c | 31 ++++---------------------------
> 4 files changed, 14 insertions(+), 58 deletions(-)
I'm going to go out on a limb a little bit and apply these to pci/aer for
v4.9. Please let me know if you trip over any issues.
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH 0/2] PCI/AER: Remove module parameters
2016-09-14 20:30 ` [PATCH 0/2] PCI/AER: Remove module parameters Bjorn Helgaas
@ 2016-09-17 4:49 ` Tang, CQ
2016-09-19 18:11 ` Bjorn Helgaas
0 siblings, 1 reply; 6+ messages in thread
From: Tang, CQ @ 2016-09-17 4:49 UTC (permalink / raw)
To: Bjorn Helgaas, Bjorn Helgaas
Cc: linux-pci@vger.kernel.org, Lance Ortiz, Betty Dall,
Hidetoshi Seto, Chen Gong, KeithBuschkeith.busch@intel.com,
Rafael J. Wysocki, linux-kernel@vger.kernel.org, Shuah Khan,
Huang, Ying, Vijay Mohan Pandarathil, Derrick, Jonathan
Bjorn,
Can we extend this aerdriver to support AER from a root complex integr=
ated endpoint? Root complex event collector has the same job as root port i=
n this case.
It looks that both port driver and aer driver check PCI_EXP_TYPE_ROOT_=
PORT/PCI_EXP_TYPE_UPSTREAM/PCI_EXP_TYPE_DOWNSTREAM only.
portdrv_pci.c:127, aerdrv.c:86
Is it just a simple addition of PCI_EXP_TYPE_RC_EC to above two place=
s? Is there any difference to recover a root complex integrated endpoint? =
(compared with devices connected to root port).
Thanks.
--CQ
> -----Original Message-----
> From: linux-pci-owner@vger.kernel.org [mailto:linux-pci-
> owner@vger.kernel.org] On Behalf Of Bjorn Helgaas
> Sent: Wednesday, September 14, 2016 1:30 PM
> To: Bjorn Helgaas <bhelgaas@google.com>
> Cc: linux-pci@vger.kernel.org; Lance Ortiz <lance.ortiz@hpe.com>; Betty D=
all
> <betty.dall@hpe.com>; Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>;
> Chen Gong <gong.chen@linux.intel.com>; KeithBuschkeith.busch@intel.com;
> Rafael J. Wysocki <rjw@rjwysocki.net>; linux-kernel@vger.kernel.org; Shua=
h
> Khan <shuahkh@osg.samsung.com>; Huang, Ying <ying.huang@intel.com>;
> Vijay Mohan Pandarathil <vijaymohan.pandarathil@hpe.com>; Derrick,
> Jonathan <jonathan.derrick@intel.com>
> Subject: Re: [PATCH 0/2] PCI/AER: Remove module parameters
>=20
> On Tue, Sep 06, 2016 at 05:19:22PM -0500, Bjorn Helgaas wrote:
> > I'd like to remove the aerdriver.nosourceid and aerdriver.forceload
> > kernel parameters. They seem like basically debugging things that are
> > not really supportable upstream.
> >
> > "nosourceid" may have some legitimate uses, but I'd rather it were
> > done via quirks with the flag Jon Derrick recently added so it happens
> > automatically, without users having to specify the parameter.
> >
> > I don't *know* of any actual users of either of these, but obviously
> > that doesn't mean there are none. Comments welcome.
> >
> > ---
> >
> > Bjorn Helgaas (2):
> > PCI/AER: Remove aerdriver.nosourceid kernel parameter
> > PCI/AER: Remove aerdriver.forceload kernel parameter
> >
> >
> > Documentation/PCI/pcieaer-howto.txt | 26 +++++++++-----------------
> > drivers/pci/pcie/aer/aerdrv.c | 7 +------
> > drivers/pci/pcie/aer/aerdrv.h | 8 --------
> > drivers/pci/pcie/aer/aerdrv_core.c | 31 ++++-----------------------=
----
> > 4 files changed, 14 insertions(+), 58 deletions(-)
>=20
> I'm going to go out on a limb a little bit and apply these to pci/aer for=
v4.9.
> Please let me know if you trip over any issues.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in t=
he
> body of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 0/2] PCI/AER: Remove module parameters
2016-09-17 4:49 ` Tang, CQ
@ 2016-09-19 18:11 ` Bjorn Helgaas
0 siblings, 0 replies; 6+ messages in thread
From: Bjorn Helgaas @ 2016-09-19 18:11 UTC (permalink / raw)
To: Tang, CQ
Cc: Bjorn Helgaas, linux-pci@vger.kernel.org, Lance Ortiz, Betty Dall,
Hidetoshi Seto, Chen Gong, KeithBuschkeith.busch@intel.com,
Rafael J. Wysocki, linux-kernel@vger.kernel.org, Shuah Khan,
Huang, Ying, Vijay Mohan Pandarathil, Derrick, Jonathan
On Sat, Sep 17, 2016 at 04:49:11AM +0000, Tang, CQ wrote:
> Bjorn,
> Can we extend this aerdriver to support AER from a root complex integrated endpoint? Root complex event collector has the same job as root port in this case.
Seems like a reasonable idea.
> It looks that both port driver and aer driver check PCI_EXP_TYPE_ROOT_PORT/PCI_EXP_TYPE_UPSTREAM/PCI_EXP_TYPE_DOWNSTREAM only.
> portdrv_pci.c:127, aerdrv.c:86
>
> Is it just a simple addition of PCI_EXP_TYPE_RC_EC to above two places? Is there any difference to recover a root complex integrated endpoint? (compared with devices connected to root port).
It may well be that the extension would be as simple as you suggest.
If you want to test and post a patch to that effect, I'll review it
more carefully.
Bjorn
> > -----Original Message-----
> > From: linux-pci-owner@vger.kernel.org [mailto:linux-pci-
> > owner@vger.kernel.org] On Behalf Of Bjorn Helgaas
> > Sent: Wednesday, September 14, 2016 1:30 PM
> > To: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: linux-pci@vger.kernel.org; Lance Ortiz <lance.ortiz@hpe.com>; Betty Dall
> > <betty.dall@hpe.com>; Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>;
> > Chen Gong <gong.chen@linux.intel.com>; KeithBuschkeith.busch@intel.com;
> > Rafael J. Wysocki <rjw@rjwysocki.net>; linux-kernel@vger.kernel.org; Shuah
> > Khan <shuahkh@osg.samsung.com>; Huang, Ying <ying.huang@intel.com>;
> > Vijay Mohan Pandarathil <vijaymohan.pandarathil@hpe.com>; Derrick,
> > Jonathan <jonathan.derrick@intel.com>
> > Subject: Re: [PATCH 0/2] PCI/AER: Remove module parameters
> >
> > On Tue, Sep 06, 2016 at 05:19:22PM -0500, Bjorn Helgaas wrote:
> > > I'd like to remove the aerdriver.nosourceid and aerdriver.forceload
> > > kernel parameters. They seem like basically debugging things that are
> > > not really supportable upstream.
> > >
> > > "nosourceid" may have some legitimate uses, but I'd rather it were
> > > done via quirks with the flag Jon Derrick recently added so it happens
> > > automatically, without users having to specify the parameter.
> > >
> > > I don't *know* of any actual users of either of these, but obviously
> > > that doesn't mean there are none. Comments welcome.
> > >
> > > ---
> > >
> > > Bjorn Helgaas (2):
> > > PCI/AER: Remove aerdriver.nosourceid kernel parameter
> > > PCI/AER: Remove aerdriver.forceload kernel parameter
> > >
> > >
> > > Documentation/PCI/pcieaer-howto.txt | 26 +++++++++-----------------
> > > drivers/pci/pcie/aer/aerdrv.c | 7 +------
> > > drivers/pci/pcie/aer/aerdrv.h | 8 --------
> > > drivers/pci/pcie/aer/aerdrv_core.c | 31 ++++---------------------------
> > > 4 files changed, 14 insertions(+), 58 deletions(-)
> >
> > I'm going to go out on a limb a little bit and apply these to pci/aer for v4.9.
> > Please let me know if you trip over any issues.
> > --
> > 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
^ permalink raw reply [flat|nested] 6+ messages in thread