| Message ID | 20090402221751.11757.36392.stgit@bob.kio (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
Andrew Patterson wrote: > Add support for turning PCIe ECRC on or off > > Adds support for PCI Express transaction layer end-to-end CRC checking > (ECRC). This patch will enable/disable ECRC checking by setting/clearing > the ECRC Check Enable and/or ECRC Generation Enable bits for devices that > support ECRC. > > The ECRC setting is controlled by the "pcie_ecrc=" command-line option. If > this option is not set or is set to 'default", the enable and generation > bits are left in whatever state that firmware/BIOS sets them to. The > "off" setting turns them off, and the "on" option turns them on (if the > device supports it). > > Signed-off-by: Andrew Patterson <andrew.patterson@hp.com> > --- > > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt > index 1754fed..c346b55 100644 > --- a/Documentation/kernel-parameters.txt > +++ b/Documentation/kernel-parameters.txt > @@ -1776,6 +1776,12 @@ and is between 256 and 4096 characters. It is defined in the file > force Enable ASPM even on devices that claim not to support it. > WARNING: Forcing ASPM on may cause system lockups. > > + pcie_ecrc= [PCI] Enable/disable PCIe ECRC (transaction layer > + end-to-end CRC checking). > + pcie_ecrc=default: Use BIOS/firmware settings. > + pcie_ecrc=off: Turn ECRC off > + pcie_ecrc=on: Turn ECRC on. > + > pcmv= [HW,PCMCIA] BadgePAD 4 > > pd. [PARIDE] > diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig > index 5a0c6ad..d90f831 100644 > --- a/drivers/pci/pcie/Kconfig > +++ b/drivers/pci/pcie/Kconfig > @@ -46,3 +46,15 @@ config PCIEASPM_DEBUG > help > This enables PCI Express ASPM debug support. It will add per-device > interface to control ASPM. > + > +# > +# PCI Express ECRC > +# > +config PCIEECRC > + bool "PCI Express ECRC support" > + depends on PCI > + help > + Enables PCI Express ECRC (transaction layer end-to-end CRC > + checking) > + > + When in doubt, say N. > diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile > index 11f6bb1..acef212 100644 > --- a/drivers/pci/pcie/Makefile > +++ b/drivers/pci/pcie/Makefile > @@ -5,6 +5,9 @@ > # Build PCI Express ASPM if needed > obj-$(CONFIG_PCIEASPM) += aspm.o > > +# Build PCI Express ECRC if needed > +obj-$(CONFIG_PCIEECRC) += ecrc.o > + > pcieportdrv-y := portdrv_core.o portdrv_pci.o portdrv_bus.o > > obj-$(CONFIG_PCIEPORTBUS) += pcieportdrv.o > diff --git a/drivers/pci/pcie/ecrc.c b/drivers/pci/pcie/ecrc.c > new file mode 100644 > index 0000000..eeb93df > --- /dev/null > +++ b/drivers/pci/pcie/ecrc.c > @@ -0,0 +1,94 @@ > +/* > + * Enables/disables PCIe ECRC checking. > + * > + * (C) Copyright 20009 Hewlett-Packard Development Company, L.P. > + * Andrew Patterson <andrew.patterson@hp.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; version 2 of the License. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA > + * 02111-1307, USA. > + * > + */ > + > + > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/moduleparam.h> > +#include <linux/pci.h> > +#include <linux/pci_regs.h> > +#include <linux/errno.h> > +#include "../pci.h" > + > +#define ECRC_POLICY_DEFAULT 0 /* ECRC set by BIOS */ > +#define ECRC_POLICY_OFF 1 /* ECRC off */ > +#define ECRC_POLICY_ON 2 /* ECRC on */ > + > +static int ecrc_policy = ECRC_POLICY_DEFAULT; > + > +static const char *ecrc_policy_str[] = { > + [ECRC_POLICY_DEFAULT] = "default", > + [ECRC_POLICY_OFF] = "off", > + [ECRC_POLICY_ON] = "on" > +}; > + > +/** > + * pcie_set_ercr_checking - enable/disable PCIe ECRC checking > + * @dev: the PCI device > + * > + * Returns 0 on success, or negative on failure. > + */ > +int pcie_set_ecrc_checking(struct pci_dev *dev) > +{ > + int pos; > + u32 reg32; > + > + if (!dev->is_pcie) > + return -ENODEV; > + > + /* Use firmware/BIOS setting if default */ > + if (ecrc_policy == ECRC_POLICY_DEFAULT) > + return 0; > + > + pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR); > + if (!pos) > + return -ENODEV; > + > + pci_read_config_dword(dev, pos + PCI_ERR_CAP, ®32); > + if (ecrc_policy == ECRC_POLICY_OFF) { > + reg32 &= ~(PCI_ERR_CAP_ECRC_GENE | PCI_ERR_CAP_ECRC_CHKE); > + } else if (ecrc_policy == ECRC_POLICY_ON) { > + if (reg32 & PCI_ERR_CAP_ECRC_GENC) > + reg32 |= PCI_ERR_CAP_ECRC_GENE; > + if (reg32 & PCI_ERR_CAP_ECRC_CHKC) > + reg32 |= PCI_ERR_CAP_ECRC_CHKE; > + } > + pci_write_config_dword(dev, pos + PCI_ERR_CAP, reg32); > + return 0; > +} > +EXPORT_SYMBOL_GPL(pcie_set_ecrc_checking); I think this EXPORT_SYMBOL_GPL() is redundant. By the way, I have the following question and comment, though I'm not familiar with ECRC at all. - This patch seems to change PCI Express Advanced Error capabilities and Control Register. Can we do it without requesting control over PCI Express Advanced Error reporting? - What about implementing ECRC on/off feature as one of the feature of existing AER driver. Thanks, Kenji Kaneshige > + > +static int __init pcie_ecrc_get_policy(char *str) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(ecrc_policy_str); i++) > + if (!strncmp(str, ecrc_policy_str[i], > + strlen(ecrc_policy_str[i]))) > + break; > + if (i >= ARRAY_SIZE(ecrc_policy_str)) > + return 0; > + > + ecrc_policy = i; > + return 1; > +} > +__setup("pcie_ecrc=", pcie_ecrc_get_policy); > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index e2f3dd0..e2b0ab5 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -984,6 +984,9 @@ static void pci_init_capabilities(struct pci_dev *dev) > > /* Single Root I/O Virtualization */ > pci_iov_init(dev); > + > + /* PCIe ECRC */ > + pcie_set_ecrc_checking(dev); > } > > void pci_device_add(struct pci_dev *dev, struct pci_bus *bus) > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 507552d..080afd8 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -879,6 +879,15 @@ static inline int pcie_aspm_enabled(void) > extern int pcie_aspm_enabled(void); > #endif > > +#ifndef CONFIG_PCIEECRC > +static inline int pcie_set_ecrc_checking(struct pci_dev *dev) > +{ > + return 0; > +} > +#else > +extern int pcie_set_ecrc_checking(struct pci_dev *dev); > +#endif > + > #define pci_enable_msi(pdev) pci_enable_msi_block(pdev, 1) > > #ifdef CONFIG_HT_IRQ > > -- > 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 > > -- 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
Andrew Patterson <andrew.patterson@hp.com> writes: > Add support for turning PCIe ECRC on or off > > Adds support for PCI Express transaction layer end-to-end CRC checking > (ECRC). This patch will enable/disable ECRC checking by setting/clearing > the ECRC Check Enable and/or ECRC Generation Enable bits for devices that > support ECRC. > > The ECRC setting is controlled by the "pcie_ecrc=" command-line option. If > this option is not set or is set to 'default", the enable and generation > bits are left in whatever state that firmware/BIOS sets them to. The > "off" setting turns them off, and the "on" option turns them on (if the > device supports it). Can you please expand a little bit on your motvation? Why does the kernel need to set that over the firmware? And why does it need to be a boot parameter vs some sysfs file? -Andi
On Fri, 2009-04-03 at 11:08 +0900, Kenji Kaneshige wrote: > Andrew Patterson wrote: > > Add support for turning PCIe ECRC on or off > > > > Adds support for PCI Express transaction layer end-to-end CRC checking > > (ECRC). This patch will enable/disable ECRC checking by setting/clearing > > the ECRC Check Enable and/or ECRC Generation Enable bits for devices that > > support ECRC. > > > > The ECRC setting is controlled by the "pcie_ecrc=" command-line option. If > > this option is not set or is set to 'default", the enable and generation > > bits are left in whatever state that firmware/BIOS sets them to. The > > "off" setting turns them off, and the "on" option turns them on (if the > > device supports it). > > > > Signed-off-by: Andrew Patterson <andrew.patterson@hp.com> > > --- > > > > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt > > index 1754fed..c346b55 100644 > > --- a/Documentation/kernel-parameters.txt > > +++ b/Documentation/kernel-parameters.txt > > @@ -1776,6 +1776,12 @@ and is between 256 and 4096 characters. It is defined in the file > > force Enable ASPM even on devices that claim not to support it. > > WARNING: Forcing ASPM on may cause system lockups. > > > > + pcie_ecrc= [PCI] Enable/disable PCIe ECRC (transaction layer > > + end-to-end CRC checking). > > + pcie_ecrc=default: Use BIOS/firmware settings. > > + pcie_ecrc=off: Turn ECRC off > > + pcie_ecrc=on: Turn ECRC on. > > + > > pcmv= [HW,PCMCIA] BadgePAD 4 > > > > pd. [PARIDE] > > diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig > > index 5a0c6ad..d90f831 100644 > > --- a/drivers/pci/pcie/Kconfig > > +++ b/drivers/pci/pcie/Kconfig > > @@ -46,3 +46,15 @@ config PCIEASPM_DEBUG > > help > > This enables PCI Express ASPM debug support. It will add per-device > > interface to control ASPM. > > + > > +# > > +# PCI Express ECRC > > +# > > +config PCIEECRC > > + bool "PCI Express ECRC support" > > + depends on PCI > > + help > > + Enables PCI Express ECRC (transaction layer end-to-end CRC > > + checking) > > + > > + When in doubt, say N. > > diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile > > index 11f6bb1..acef212 100644 > > --- a/drivers/pci/pcie/Makefile > > +++ b/drivers/pci/pcie/Makefile > > @@ -5,6 +5,9 @@ > > # Build PCI Express ASPM if needed > > obj-$(CONFIG_PCIEASPM) += aspm.o > > > > +# Build PCI Express ECRC if needed > > +obj-$(CONFIG_PCIEECRC) += ecrc.o > > + > > pcieportdrv-y := portdrv_core.o portdrv_pci.o portdrv_bus.o > > > > obj-$(CONFIG_PCIEPORTBUS) += pcieportdrv.o > > diff --git a/drivers/pci/pcie/ecrc.c b/drivers/pci/pcie/ecrc.c > > new file mode 100644 > > index 0000000..eeb93df > > --- /dev/null > > +++ b/drivers/pci/pcie/ecrc.c > > @@ -0,0 +1,94 @@ > > +/* > > + * Enables/disables PCIe ECRC checking. > > + * > > + * (C) Copyright 20009 Hewlett-Packard Development Company, L.P. > > + * Andrew Patterson <andrew.patterson@hp.com> > > + * > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License as published by > > + * the Free Software Foundation; version 2 of the License. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE. See the GNU > > + * General Public License for more details. > > + * > > + * You should have received a copy of the GNU General Public License > > + * along with this program; if not, write to the Free Software > > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA > > + * 02111-1307, USA. > > + * > > + */ > > + > > + > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/moduleparam.h> > > +#include <linux/pci.h> > > +#include <linux/pci_regs.h> > > +#include <linux/errno.h> > > +#include "../pci.h" > > + > > +#define ECRC_POLICY_DEFAULT 0 /* ECRC set by BIOS */ > > +#define ECRC_POLICY_OFF 1 /* ECRC off */ > > +#define ECRC_POLICY_ON 2 /* ECRC on */ > > + > > +static int ecrc_policy = ECRC_POLICY_DEFAULT; > > + > > +static const char *ecrc_policy_str[] = { > > + [ECRC_POLICY_DEFAULT] = "default", > > + [ECRC_POLICY_OFF] = "off", > > + [ECRC_POLICY_ON] = "on" > > +}; > > + > > +/** > > + * pcie_set_ercr_checking - enable/disable PCIe ECRC checking > > + * @dev: the PCI device > > + * > > + * Returns 0 on success, or negative on failure. > > + */ > > +int pcie_set_ecrc_checking(struct pci_dev *dev) > > +{ > > + int pos; > > + u32 reg32; > > + > > + if (!dev->is_pcie) > > + return -ENODEV; > > + > > + /* Use firmware/BIOS setting if default */ > > + if (ecrc_policy == ECRC_POLICY_DEFAULT) > > + return 0; > > + > > + pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR); > > + if (!pos) > > + return -ENODEV; > > + > > + pci_read_config_dword(dev, pos + PCI_ERR_CAP, ®32); > > + if (ecrc_policy == ECRC_POLICY_OFF) { > > + reg32 &= ~(PCI_ERR_CAP_ECRC_GENE | PCI_ERR_CAP_ECRC_CHKE); > > + } else if (ecrc_policy == ECRC_POLICY_ON) { > > + if (reg32 & PCI_ERR_CAP_ECRC_GENC) > > + reg32 |= PCI_ERR_CAP_ECRC_GENE; > > + if (reg32 & PCI_ERR_CAP_ECRC_CHKC) > > + reg32 |= PCI_ERR_CAP_ECRC_CHKE; > > + } > > + pci_write_config_dword(dev, pos + PCI_ERR_CAP, reg32); > > + return 0; > > +} > > +EXPORT_SYMBOL_GPL(pcie_set_ecrc_checking); > > I think this EXPORT_SYMBOL_GPL() is redundant. I'll remove this. > > By the way, I have the following question and comment, though I'm not > familiar with ECRC at all. > > - This patch seems to change PCI Express Advanced Error capabilities > and Control Register. Can we do it without requesting control over > PCI Express Advanced Error reporting? I believe so. The spec says: The capability to generate and check ECRC is reported to software, and the ability to do so is enabled by software (see Section 7.10.7) and: If a device supports ECRC generation/checking, at least one of its Functions must support Advanced Error Reporting (see Section 6.2) Does "supporting AER" mean that we must request software control for before using it? I would think that the ECRC Check Capable and ECRC Generation Capable bits are letting us know if we can use this feature. > > - What about implementing ECRC on/off feature as one of the feature > of existing AER driver. > I originally started down this path, but came to the conclusion that AER reporting is not required to use this feature. We have systems that generate NMIs/MCAs for an ECRC violation even if AER is not enabled. Andrew > Thanks, > Kenji Kaneshige > > > > > + > > +static int __init pcie_ecrc_get_policy(char *str) > > +{ > > + int i; > > + > > + for (i = 0; i < ARRAY_SIZE(ecrc_policy_str); i++) > > + if (!strncmp(str, ecrc_policy_str[i], > > + strlen(ecrc_policy_str[i]))) > > + break; > > + if (i >= ARRAY_SIZE(ecrc_policy_str)) > > + return 0; > > + > > + ecrc_policy = i; > > + return 1; > > +} > > +__setup("pcie_ecrc=", pcie_ecrc_get_policy); > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > > index e2f3dd0..e2b0ab5 100644 > > --- a/drivers/pci/probe.c > > +++ b/drivers/pci/probe.c > > @@ -984,6 +984,9 @@ static void pci_init_capabilities(struct pci_dev *dev) > > > > /* Single Root I/O Virtualization */ > > pci_iov_init(dev); > > + > > + /* PCIe ECRC */ > > + pcie_set_ecrc_checking(dev); > > } > > > > void pci_device_add(struct pci_dev *dev, struct pci_bus *bus) > > diff --git a/include/linux/pci.h b/include/linux/pci.h > > index 507552d..080afd8 100644 > > --- a/include/linux/pci.h > > +++ b/include/linux/pci.h > > @@ -879,6 +879,15 @@ static inline int pcie_aspm_enabled(void) > > extern int pcie_aspm_enabled(void); > > #endif > > > > +#ifndef CONFIG_PCIEECRC > > +static inline int pcie_set_ecrc_checking(struct pci_dev *dev) > > +{ > > + return 0; > > +} > > +#else > > +extern int pcie_set_ecrc_checking(struct pci_dev *dev); > > +#endif > > + > > #define pci_enable_msi(pdev) pci_enable_msi_block(pdev, 1) > > > > #ifdef CONFIG_HT_IRQ > > > > -- > > 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 > > > > > -- 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
On Fri, 2009-04-03 at 08:54 +0200, Andi Kleen wrote: > Andrew Patterson <andrew.patterson@hp.com> writes: > > > Add support for turning PCIe ECRC on or off > > > > Adds support for PCI Express transaction layer end-to-end CRC checking > > (ECRC). This patch will enable/disable ECRC checking by setting/clearing > > the ECRC Check Enable and/or ECRC Generation Enable bits for devices that > > support ECRC. > > > > The ECRC setting is controlled by the "pcie_ecrc=" command-line option. If > > this option is not set or is set to 'default", the enable and generation > > bits are left in whatever state that firmware/BIOS sets them to. The > > "off" setting turns them off, and the "on" option turns them on (if the > > device supports it). > > Can you please expand a little bit on your motvation? Why does the kernel > need to set that over the firmware? My main motivation is to turn this on for systems that support ECRC but don't currently have it turned on. I think that turning this on or off provides a possible tradeoff of increased data integrity over some sort of performance penalty (hardware no longer has to calculate ECRC data, and packet length is smaller due to lack of ECRC fields). > And why does it need to be a boot > parameter vs some sysfs file? It could be either. I thought about doing both actually, but decided it probably wasn't worth it. One possible advantage of doing sysfs is that you could turn it on or off selectively for each root bridge. The advantage of doing it at boot is that we could turn it off/on very early for balky hardware. > > -Andi >
Andrew Patterson wrote: > On Fri, 2009-04-03 at 11:08 +0900, Kenji Kaneshige wrote: >> Andrew Patterson wrote: >>> Add support for turning PCIe ECRC on or off >>> >>> Adds support for PCI Express transaction layer end-to-end CRC checking >>> (ECRC). This patch will enable/disable ECRC checking by setting/clearing >>> the ECRC Check Enable and/or ECRC Generation Enable bits for devices that >>> support ECRC. >>> >>> The ECRC setting is controlled by the "pcie_ecrc=" command-line option. If >>> this option is not set or is set to 'default", the enable and generation >>> bits are left in whatever state that firmware/BIOS sets them to. The >>> "off" setting turns them off, and the "on" option turns them on (if the >>> device supports it). >>> >>> Signed-off-by: Andrew Patterson <andrew.patterson@hp.com> >>> --- >>> >>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt >>> index 1754fed..c346b55 100644 >>> --- a/Documentation/kernel-parameters.txt >>> +++ b/Documentation/kernel-parameters.txt >>> @@ -1776,6 +1776,12 @@ and is between 256 and 4096 characters. It is defined in the file >>> force Enable ASPM even on devices that claim not to support it. >>> WARNING: Forcing ASPM on may cause system lockups. >>> >>> + pcie_ecrc= [PCI] Enable/disable PCIe ECRC (transaction layer >>> + end-to-end CRC checking). >>> + pcie_ecrc=default: Use BIOS/firmware settings. >>> + pcie_ecrc=off: Turn ECRC off >>> + pcie_ecrc=on: Turn ECRC on. >>> + >>> pcmv= [HW,PCMCIA] BadgePAD 4 >>> >>> pd. [PARIDE] >>> diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig >>> index 5a0c6ad..d90f831 100644 >>> --- a/drivers/pci/pcie/Kconfig >>> +++ b/drivers/pci/pcie/Kconfig >>> @@ -46,3 +46,15 @@ config PCIEASPM_DEBUG >>> help >>> This enables PCI Express ASPM debug support. It will add per-device >>> interface to control ASPM. >>> + >>> +# >>> +# PCI Express ECRC >>> +# >>> +config PCIEECRC >>> + bool "PCI Express ECRC support" >>> + depends on PCI >>> + help >>> + Enables PCI Express ECRC (transaction layer end-to-end CRC >>> + checking) >>> + >>> + When in doubt, say N. >>> diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile >>> index 11f6bb1..acef212 100644 >>> --- a/drivers/pci/pcie/Makefile >>> +++ b/drivers/pci/pcie/Makefile >>> @@ -5,6 +5,9 @@ >>> # Build PCI Express ASPM if needed >>> obj-$(CONFIG_PCIEASPM) += aspm.o >>> >>> +# Build PCI Express ECRC if needed >>> +obj-$(CONFIG_PCIEECRC) += ecrc.o >>> + >>> pcieportdrv-y := portdrv_core.o portdrv_pci.o portdrv_bus.o >>> >>> obj-$(CONFIG_PCIEPORTBUS) += pcieportdrv.o >>> diff --git a/drivers/pci/pcie/ecrc.c b/drivers/pci/pcie/ecrc.c >>> new file mode 100644 >>> index 0000000..eeb93df >>> --- /dev/null >>> +++ b/drivers/pci/pcie/ecrc.c >>> @@ -0,0 +1,94 @@ >>> +/* >>> + * Enables/disables PCIe ECRC checking. >>> + * >>> + * (C) Copyright 20009 Hewlett-Packard Development Company, L.P. >>> + * Andrew Patterson <andrew.patterson@hp.com> >>> + * >>> + * This program is free software; you can redistribute it and/or modify >>> + * it under the terms of the GNU General Public License as published by >>> + * the Free Software Foundation; version 2 of the License. >>> + * >>> + * This program is distributed in the hope that it will be useful, >>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >>> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE. See the GNU >>> + * General Public License for more details. >>> + * >>> + * You should have received a copy of the GNU General Public License >>> + * along with this program; if not, write to the Free Software >>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA >>> + * 02111-1307, USA. >>> + * >>> + */ >>> + >>> + >>> +#include <linux/kernel.h> >>> +#include <linux/module.h> >>> +#include <linux/moduleparam.h> >>> +#include <linux/pci.h> >>> +#include <linux/pci_regs.h> >>> +#include <linux/errno.h> >>> +#include "../pci.h" >>> + >>> +#define ECRC_POLICY_DEFAULT 0 /* ECRC set by BIOS */ >>> +#define ECRC_POLICY_OFF 1 /* ECRC off */ >>> +#define ECRC_POLICY_ON 2 /* ECRC on */ >>> + >>> +static int ecrc_policy = ECRC_POLICY_DEFAULT; >>> + >>> +static const char *ecrc_policy_str[] = { >>> + [ECRC_POLICY_DEFAULT] = "default", >>> + [ECRC_POLICY_OFF] = "off", >>> + [ECRC_POLICY_ON] = "on" >>> +}; >>> + >>> +/** >>> + * pcie_set_ercr_checking - enable/disable PCIe ECRC checking >>> + * @dev: the PCI device >>> + * >>> + * Returns 0 on success, or negative on failure. >>> + */ >>> +int pcie_set_ecrc_checking(struct pci_dev *dev) >>> +{ >>> + int pos; >>> + u32 reg32; >>> + >>> + if (!dev->is_pcie) >>> + return -ENODEV; >>> + >>> + /* Use firmware/BIOS setting if default */ >>> + if (ecrc_policy == ECRC_POLICY_DEFAULT) >>> + return 0; >>> + >>> + pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR); >>> + if (!pos) >>> + return -ENODEV; >>> + >>> + pci_read_config_dword(dev, pos + PCI_ERR_CAP, ®32); >>> + if (ecrc_policy == ECRC_POLICY_OFF) { >>> + reg32 &= ~(PCI_ERR_CAP_ECRC_GENE | PCI_ERR_CAP_ECRC_CHKE); >>> + } else if (ecrc_policy == ECRC_POLICY_ON) { >>> + if (reg32 & PCI_ERR_CAP_ECRC_GENC) >>> + reg32 |= PCI_ERR_CAP_ECRC_GENE; >>> + if (reg32 & PCI_ERR_CAP_ECRC_CHKC) >>> + reg32 |= PCI_ERR_CAP_ECRC_CHKE; >>> + } >>> + pci_write_config_dword(dev, pos + PCI_ERR_CAP, reg32); >>> + return 0; >>> +} >>> +EXPORT_SYMBOL_GPL(pcie_set_ecrc_checking); >> I think this EXPORT_SYMBOL_GPL() is redundant. > > I'll remove this. > >> By the way, I have the following question and comment, though I'm not >> familiar with ECRC at all. >> >> - This patch seems to change PCI Express Advanced Error capabilities >> and Control Register. Can we do it without requesting control over >> PCI Express Advanced Error reporting? > > I believe so. The spec says: > I checked the spec again. Unfortunately my interpretation is different from yours. > The capability to generate and check ECRC is reported to software, and > the ability to do so is enabled by software (see Section 7.10.7) > I think the "software" means not only OS, but also firmware. > and: > > If a device supports ECRC generation/checking, at least one of its > Functions must support Advanced Error Reporting (see Section 6.2) > I don't think this description in the spec explains the relationship between OS and firmware. > Does "supporting AER" mean that we must request software control for > before using it? I would think that the ECRC Check Capable and ECRC > Generation Capable bits are letting us know if we can use this feature. > I think that those bits are not only for OS, but also for firmware. To prevent the conflict between OS and firmware, I think OS must take control from firmware before using AER registers. The reason why I think we need to request control over AER from firmware is based on the following descriptions in "6.2.9 _OSC (Operating System Capabilities) of ACPI3.0b spec. For example, "... If any bits in the Control Field are returned cleared (masked to zero) by the _OSC control method, the respective feature is designated unsupported by the platform and must not be enabled by the OS. Some of these features may be controlled by platform firmware prior to OS boot or during runtime for a legacy OS, while others may be disabled/inoperative until native OS support is available. ..." (in "6.2.9.1 _OSC Implementation Example for PCI Host Bridge Devices") "... The OS must evaluate _OSC under the following conditions: During initialization of any driver that provides native support for features described in the section above. ..." (in "6.2.9.1.1.2 Evaluation Conditions") Please also see "Table 6-10 Interpretation of _OSC Control Field, Passed in via Arg3" and "Table 6-11 Interpretation of _OSC Control Field, Returned Value". And my interpretation is also based on the following spec in "6.2.7.3 PCI Express Setting Record (Type 2)" in ACPI3.0b. "... The Type 2 Setting Record allows a PCI Express-aware OS that supports native hot plug to configure the specified registers of the hot plugged PCI Express device. A PCI Express-aware OS that has assumed ownership of native hot plug (via _OSC) but does not support or does not have ownership of the AER register set must use the data values returned by the _HPX object‘s Type 2 record to program the AER registers of a hot-added PCI Express device. ..." I believe "PCI Express Advanced Error capabilities and Control Register" is one of the AER registers. Thanks, Kenji Kaneshige >> - What about implementing ECRC on/off feature as one of the feature >> of existing AER driver. >> > > I originally started down this path, but came to the conclusion that AER > reporting is not required to use this feature. We have systems that > generate NMIs/MCAs for an ECRC violation even if AER is not enabled. > > Andrew > >> Thanks, >> Kenji Kaneshige >> >> >> >>> + >>> +static int __init pcie_ecrc_get_policy(char *str) >>> +{ >>> + int i; >>> + >>> + for (i = 0; i < ARRAY_SIZE(ecrc_policy_str); i++) >>> + if (!strncmp(str, ecrc_policy_str[i], >>> + strlen(ecrc_policy_str[i]))) >>> + break; >>> + if (i >= ARRAY_SIZE(ecrc_policy_str)) >>> + return 0; >>> + >>> + ecrc_policy = i; >>> + return 1; >>> +} >>> +__setup("pcie_ecrc=", pcie_ecrc_get_policy); >>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c >>> index e2f3dd0..e2b0ab5 100644 >>> --- a/drivers/pci/probe.c >>> +++ b/drivers/pci/probe.c >>> @@ -984,6 +984,9 @@ static void pci_init_capabilities(struct pci_dev *dev) >>> >>> /* Single Root I/O Virtualization */ >>> pci_iov_init(dev); >>> + >>> + /* PCIe ECRC */ >>> + pcie_set_ecrc_checking(dev); >>> } >>> >>> void pci_device_add(struct pci_dev *dev, struct pci_bus *bus) >>> diff --git a/include/linux/pci.h b/include/linux/pci.h >>> index 507552d..080afd8 100644 >>> --- a/include/linux/pci.h >>> +++ b/include/linux/pci.h >>> @@ -879,6 +879,15 @@ static inline int pcie_aspm_enabled(void) >>> extern int pcie_aspm_enabled(void); >>> #endif >>> >>> +#ifndef CONFIG_PCIEECRC >>> +static inline int pcie_set_ecrc_checking(struct pci_dev *dev) >>> +{ >>> + return 0; >>> +} >>> +#else >>> +extern int pcie_set_ecrc_checking(struct pci_dev *dev); >>> +#endif >>> + >>> #define pci_enable_msi(pdev) pci_enable_msi_block(pdev, 1) >>> >>> #ifdef CONFIG_HT_IRQ >>> >>> -- >>> 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 >>> >>> > > > -- > 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 > > -- 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
On Tue, 2009-04-07 at 10:43 +0900, Kenji Kaneshige wrote: > Andrew Patterson wrote: > > On Fri, 2009-04-03 at 11:08 +0900, Kenji Kaneshige wrote: > >> Andrew Patterson wrote: > >>> Add support for turning PCIe ECRC on or off > >>> > >>> Adds support for PCI Express transaction layer end-to-end CRC checking > >>> (ECRC). This patch will enable/disable ECRC checking by setting/clearing > >>> the ECRC Check Enable and/or ECRC Generation Enable bits for devices that > >>> support ECRC. > >>> > >>> The ECRC setting is controlled by the "pcie_ecrc=" command-line option. If > >>> this option is not set or is set to 'default", the enable and generation > >>> bits are left in whatever state that firmware/BIOS sets them to. The > >>> "off" setting turns them off, and the "on" option turns them on (if the > >>> device supports it). > >>> > >>> Signed-off-by: Andrew Patterson <andrew.patterson@hp.com> > >>> --- > >>> > >>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt > >>> index 1754fed..c346b55 100644 > >>> --- a/Documentation/kernel-parameters.txt > >>> +++ b/Documentation/kernel-parameters.txt > >>> @@ -1776,6 +1776,12 @@ and is between 256 and 4096 characters. It is defined in the file > >>> force Enable ASPM even on devices that claim not to support it. > >>> WARNING: Forcing ASPM on may cause system lockups. > >>> > >>> + pcie_ecrc= [PCI] Enable/disable PCIe ECRC (transaction layer > >>> + end-to-end CRC checking). > >>> + pcie_ecrc=default: Use BIOS/firmware settings. > >>> + pcie_ecrc=off: Turn ECRC off > >>> + pcie_ecrc=on: Turn ECRC on. > >>> + > >>> pcmv= [HW,PCMCIA] BadgePAD 4 > >>> > >>> pd. [PARIDE] > >>> diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig > >>> index 5a0c6ad..d90f831 100644 > >>> --- a/drivers/pci/pcie/Kconfig > >>> +++ b/drivers/pci/pcie/Kconfig > >>> @@ -46,3 +46,15 @@ config PCIEASPM_DEBUG > >>> help > >>> This enables PCI Express ASPM debug support. It will add per-device > >>> interface to control ASPM. > >>> + > >>> +# > >>> +# PCI Express ECRC > >>> +# > >>> +config PCIEECRC > >>> + bool "PCI Express ECRC support" > >>> + depends on PCI > >>> + help > >>> + Enables PCI Express ECRC (transaction layer end-to-end CRC > >>> + checking) > >>> + > >>> + When in doubt, say N. > >>> diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile > >>> index 11f6bb1..acef212 100644 > >>> --- a/drivers/pci/pcie/Makefile > >>> +++ b/drivers/pci/pcie/Makefile > >>> @@ -5,6 +5,9 @@ > >>> # Build PCI Express ASPM if needed > >>> obj-$(CONFIG_PCIEASPM) += aspm.o > >>> > >>> +# Build PCI Express ECRC if needed > >>> +obj-$(CONFIG_PCIEECRC) += ecrc.o > >>> + > >>> pcieportdrv-y := portdrv_core.o portdrv_pci.o portdrv_bus.o > >>> > >>> obj-$(CONFIG_PCIEPORTBUS) += pcieportdrv.o > >>> diff --git a/drivers/pci/pcie/ecrc.c b/drivers/pci/pcie/ecrc.c > >>> new file mode 100644 > >>> index 0000000..eeb93df > >>> --- /dev/null > >>> +++ b/drivers/pci/pcie/ecrc.c > >>> @@ -0,0 +1,94 @@ > >>> +/* > >>> + * Enables/disables PCIe ECRC checking. > >>> + * > >>> + * (C) Copyright 20009 Hewlett-Packard Development Company, L.P. > >>> + * Andrew Patterson <andrew.patterson@hp.com> > >>> + * > >>> + * This program is free software; you can redistribute it and/or modify > >>> + * it under the terms of the GNU General Public License as published by > >>> + * the Free Software Foundation; version 2 of the License. > >>> + * > >>> + * This program is distributed in the hope that it will be useful, > >>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of > >>> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE. See the GNU > >>> + * General Public License for more details. > >>> + * > >>> + * You should have received a copy of the GNU General Public License > >>> + * along with this program; if not, write to the Free Software > >>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA > >>> + * 02111-1307, USA. > >>> + * > >>> + */ > >>> + > >>> + > >>> +#include <linux/kernel.h> > >>> +#include <linux/module.h> > >>> +#include <linux/moduleparam.h> > >>> +#include <linux/pci.h> > >>> +#include <linux/pci_regs.h> > >>> +#include <linux/errno.h> > >>> +#include "../pci.h" > >>> + > >>> +#define ECRC_POLICY_DEFAULT 0 /* ECRC set by BIOS */ > >>> +#define ECRC_POLICY_OFF 1 /* ECRC off */ > >>> +#define ECRC_POLICY_ON 2 /* ECRC on */ > >>> + > >>> +static int ecrc_policy = ECRC_POLICY_DEFAULT; > >>> + > >>> +static const char *ecrc_policy_str[] = { > >>> + [ECRC_POLICY_DEFAULT] = "default", > >>> + [ECRC_POLICY_OFF] = "off", > >>> + [ECRC_POLICY_ON] = "on" > >>> +}; > >>> + > >>> +/** > >>> + * pcie_set_ercr_checking - enable/disable PCIe ECRC checking > >>> + * @dev: the PCI device > >>> + * > >>> + * Returns 0 on success, or negative on failure. > >>> + */ > >>> +int pcie_set_ecrc_checking(struct pci_dev *dev) > >>> +{ > >>> + int pos; > >>> + u32 reg32; > >>> + > >>> + if (!dev->is_pcie) > >>> + return -ENODEV; > >>> + > >>> + /* Use firmware/BIOS setting if default */ > >>> + if (ecrc_policy == ECRC_POLICY_DEFAULT) > >>> + return 0; > >>> + > >>> + pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR); > >>> + if (!pos) > >>> + return -ENODEV; > >>> + > >>> + pci_read_config_dword(dev, pos + PCI_ERR_CAP, ®32); > >>> + if (ecrc_policy == ECRC_POLICY_OFF) { > >>> + reg32 &= ~(PCI_ERR_CAP_ECRC_GENE | PCI_ERR_CAP_ECRC_CHKE); > >>> + } else if (ecrc_policy == ECRC_POLICY_ON) { > >>> + if (reg32 & PCI_ERR_CAP_ECRC_GENC) > >>> + reg32 |= PCI_ERR_CAP_ECRC_GENE; > >>> + if (reg32 & PCI_ERR_CAP_ECRC_CHKC) > >>> + reg32 |= PCI_ERR_CAP_ECRC_CHKE; > >>> + } > >>> + pci_write_config_dword(dev, pos + PCI_ERR_CAP, reg32); > >>> + return 0; > >>> +} > >>> +EXPORT_SYMBOL_GPL(pcie_set_ecrc_checking); > >> I think this EXPORT_SYMBOL_GPL() is redundant. > > > > I'll remove this. > > > >> By the way, I have the following question and comment, though I'm not > >> familiar with ECRC at all. > >> > >> - This patch seems to change PCI Express Advanced Error capabilities > >> and Control Register. Can we do it without requesting control over > >> PCI Express Advanced Error reporting? > > > > I believe so. The spec says: > > > > I checked the spec again. > Unfortunately my interpretation is different from yours. > > > The capability to generate and check ECRC is reported to software, and > > the ability to do so is enabled by software (see Section 7.10.7) > > > > I think the "software" means not only OS, but also firmware. > OK. > > and: > > > > If a device supports ECRC generation/checking, at least one of its > > Functions must support Advanced Error Reporting (see Section 6.2) > > > > I don't think this description in the spec explains the relationship > between OS and firmware. > > > Does "supporting AER" mean that we must request software control for > > before using it? I would think that the ECRC Check Capable and ECRC > > Generation Capable bits are letting us know if we can use this feature. > > > > I think that those bits are not only for OS, but also for firmware. To > prevent the conflict between OS and firmware, I think OS must take > control from firmware before using AER registers. > I am not sure here. > The reason why I think we need to request control over AER from firmware > is based on the following descriptions in "6.2.9 _OSC (Operating System > Capabilities) of ACPI3.0b spec. For example, > > "... If any bits in the Control Field are returned cleared (masked > to zero) by the _OSC control method, the respective feature is > designated unsupported by the platform and must not be enabled by the > OS. Some of these features may be controlled by platform firmware > prior to OS boot or during runtime for a legacy OS, while others may > be disabled/inoperative until native OS support is available. ..." > (in "6.2.9.1 _OSC Implementation Example for PCI Host Bridge Devices") > > "... The OS must evaluate _OSC under the following conditions: > During initialization of any driver that provides native support for > features described in the section above. ..." (in "6.2.9.1.1.2 > Evaluation Conditions") > > Please also see "Table 6-10 Interpretation of _OSC Control Field, Passed > in via Arg3" and "Table 6-11 Interpretation of _OSC Control Field, > Returned Value". > Here is the AER entry in table 6-11: "The firmware sets this bit to 1 to grant control over PCI Express Advanced Error Reporting. If firmware allows the OS control of this feature, then in the context of the _OSC method it must ensure that error messages are routed to device interrupts as described in the PCI Express Base Specification. Additionally, after control is transferred to the OS, firmware must not modify the Advanced Error Reporting Capability. If control of this feature was requested and denied or was not requested, firmware returns this bit set to 0." Does this mean that you can't access any of the bits in any AER registers unless you take control via _OSC? On the other hand, given that firmware cannot touch AER registers after _OSC grants control, it makes some sense that software cannot do so without permission. > And my interpretation is also based on the following spec in "6.2.7.3 > PCI Express Setting Record (Type 2)" in ACPI3.0b. > > "... The Type 2 Setting Record allows a PCI Express-aware OS that > supports native hot plug to configure the specified registers of the > hot plugged PCI Express device. A PCI Express-aware OS that has > assumed ownership of native hot plug (via _OSC) but does not support > or does not have ownership of the AER register set must use the data > values returned by the _HPX object‘s Type 2 record to program the > AER registers of a hot-added PCI Express device. ..." > > I believe "PCI Express Advanced Error capabilities and Control Register" > is one of the AER registers. Yes. I am mostly convinced. I will rework this. > > Thanks, > Kenji Kaneshige > > > > >> - What about implementing ECRC on/off feature as one of the feature > >> of existing AER driver. > >> > > > > I originally started down this path, but came to the conclusion that AER > > reporting is not required to use this feature. We have systems that > > generate NMIs/MCAs for an ECRC violation even if AER is not enabled. > > > > Andrew > > > >> Thanks, > >> Kenji Kaneshige > >> > >> > >> > >>> + > >>> +static int __init pcie_ecrc_get_policy(char *str) > >>> +{ > >>> + int i; > >>> + > >>> + for (i = 0; i < ARRAY_SIZE(ecrc_policy_str); i++) > >>> + if (!strncmp(str, ecrc_policy_str[i], > >>> + strlen(ecrc_policy_str[i]))) > >>> + break; > >>> + if (i >= ARRAY_SIZE(ecrc_policy_str)) > >>> + return 0; > >>> + > >>> + ecrc_policy = i; > >>> + return 1; > >>> +} > >>> +__setup("pcie_ecrc=", pcie_ecrc_get_policy); > >>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > >>> index e2f3dd0..e2b0ab5 100644 > >>> --- a/drivers/pci/probe.c > >>> +++ b/drivers/pci/probe.c > >>> @@ -984,6 +984,9 @@ static void pci_init_capabilities(struct pci_dev *dev) > >>> > >>> /* Single Root I/O Virtualization */ > >>> pci_iov_init(dev); > >>> + > >>> + /* PCIe ECRC */ > >>> + pcie_set_ecrc_checking(dev); > >>> } > >>> > >>> void pci_device_add(struct pci_dev *dev, struct pci_bus *bus) > >>> diff --git a/include/linux/pci.h b/include/linux/pci.h > >>> index 507552d..080afd8 100644 > >>> --- a/include/linux/pci.h > >>> +++ b/include/linux/pci.h > >>> @@ -879,6 +879,15 @@ static inline int pcie_aspm_enabled(void) > >>> extern int pcie_aspm_enabled(void); > >>> #endif > >>> > >>> +#ifndef CONFIG_PCIEECRC > >>> +static inline int pcie_set_ecrc_checking(struct pci_dev *dev) > >>> +{ > >>> + return 0; > >>> +} > >>> +#else > >>> +extern int pcie_set_ecrc_checking(struct pci_dev *dev); > >>> +#endif > >>> + > >>> #define pci_enable_msi(pdev) pci_enable_msi_block(pdev, 1) > >>> > >>> #ifdef CONFIG_HT_IRQ > >>> > >>> -- > >>> 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 > >>> > >>> > > > > > > -- > > 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 > > > > > -- 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
Andrew Patterson wrote: > On Tue, 2009-04-07 at 10:43 +0900, Kenji Kaneshige wrote: >> Andrew Patterson wrote: >>> On Fri, 2009-04-03 at 11:08 +0900, Kenji Kaneshige wrote: >>>> Andrew Patterson wrote: >>>>> Add support for turning PCIe ECRC on or off >>>>> (snip.) > >> The reason why I think we need to request control over AER from firmware >> is based on the following descriptions in "6.2.9 _OSC (Operating System >> Capabilities) of ACPI3.0b spec. For example, >> >> "... If any bits in the Control Field are returned cleared (masked >> to zero) by the _OSC control method, the respective feature is >> designated unsupported by the platform and must not be enabled by the >> OS. Some of these features may be controlled by platform firmware >> prior to OS boot or during runtime for a legacy OS, while others may >> be disabled/inoperative until native OS support is available. ..." >> (in "6.2.9.1 _OSC Implementation Example for PCI Host Bridge Devices") >> >> "... The OS must evaluate _OSC under the following conditions: >> During initialization of any driver that provides native support for >> features described in the section above. ..." (in "6.2.9.1.1.2 >> Evaluation Conditions") >> >> Please also see "Table 6-10 Interpretation of _OSC Control Field, Passed >> in via Arg3" and "Table 6-11 Interpretation of _OSC Control Field, >> Returned Value". >> > > Here is the AER entry in table 6-11: > > "The firmware sets this bit to 1 to grant control over PCI Express > Advanced Error Reporting. If firmware allows the OS control of this > feature, then in the context of > the _OSC method it must ensure that error messages are routed to device > interrupts as described in the PCI Express Base Specification. > Additionally, after control is transferred to the OS, firmware must not > modify the Advanced Error Reporting Capability. If control of this > feature was requested and denied or was not requested, firmware returns > this bit set to 0." > > Does this mean that you can't access any of the bits in any AER > registers unless you take control via _OSC? On the other hand, given > that firmware cannot touch AER registers after _OSC grants control, it > makes some sense that software cannot do so without permission. > Yes, I think so. (I think we cannot program AER registers). > >> And my interpretation is also based on the following spec in "6.2.7.3 >> PCI Express Setting Record (Type 2)" in ACPI3.0b. >> >> "... The Type 2 Setting Record allows a PCI Express-aware OS that >> supports native hot plug to configure the specified registers of the >> hot plugged PCI Express device. A PCI Express-aware OS that has >> assumed ownership of native hot plug (via _OSC) but does not support >> or does not have ownership of the AER register set must use the data >> values returned by the _HPX object‘s Type 2 record to program the >> AER registers of a hot-added PCI Express device. ..." >> >> I believe "PCI Express Advanced Error capabilities and Control Register" >> is one of the AER registers. > > Yes. > > I am mostly convinced. I will rework this. > Just in case, what I thought from the description of _HPX is that the OS must not program AER registers by its own decision if the OS does not have ownership of the AER registers. Thanks, Kenji Kaneshige -- 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
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt index 1754fed..c346b55 100644 --- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -1776,6 +1776,12 @@ and is between 256 and 4096 characters. It is defined in the file force Enable ASPM even on devices that claim not to support it. WARNING: Forcing ASPM on may cause system lockups. + pcie_ecrc= [PCI] Enable/disable PCIe ECRC (transaction layer + end-to-end CRC checking). + pcie_ecrc=default: Use BIOS/firmware settings. + pcie_ecrc=off: Turn ECRC off + pcie_ecrc=on: Turn ECRC on. + pcmv= [HW,PCMCIA] BadgePAD 4 pd. [PARIDE] diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig index 5a0c6ad..d90f831 100644 --- a/drivers/pci/pcie/Kconfig +++ b/drivers/pci/pcie/Kconfig @@ -46,3 +46,15 @@ config PCIEASPM_DEBUG help This enables PCI Express ASPM debug support. It will add per-device interface to control ASPM. + +# +# PCI Express ECRC +# +config PCIEECRC + bool "PCI Express ECRC support" + depends on PCI + help + Enables PCI Express ECRC (transaction layer end-to-end CRC + checking) + + When in doubt, say N. diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile index 11f6bb1..acef212 100644 --- a/drivers/pci/pcie/Makefile +++ b/drivers/pci/pcie/Makefile @@ -5,6 +5,9 @@ # Build PCI Express ASPM if needed obj-$(CONFIG_PCIEASPM) += aspm.o +# Build PCI Express ECRC if needed +obj-$(CONFIG_PCIEECRC) += ecrc.o + pcieportdrv-y := portdrv_core.o portdrv_pci.o portdrv_bus.o obj-$(CONFIG_PCIEPORTBUS) += pcieportdrv.o diff --git a/drivers/pci/pcie/ecrc.c b/drivers/pci/pcie/ecrc.c new file mode 100644 index 0000000..eeb93df --- /dev/null +++ b/drivers/pci/pcie/ecrc.c @@ -0,0 +1,94 @@ +/* + * Enables/disables PCIe ECRC checking. + * + * (C) Copyright 20009 Hewlett-Packard Development Company, L.P. + * Andrew Patterson <andrew.patterson@hp.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA + * 02111-1307, USA. + * + */ + + +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/moduleparam.h> +#include <linux/pci.h> +#include <linux/pci_regs.h> +#include <linux/errno.h> +#include "../pci.h" + +#define ECRC_POLICY_DEFAULT 0 /* ECRC set by BIOS */ +#define ECRC_POLICY_OFF 1 /* ECRC off */ +#define ECRC_POLICY_ON 2 /* ECRC on */ + +static int ecrc_policy = ECRC_POLICY_DEFAULT; + +static const char *ecrc_policy_str[] = { + [ECRC_POLICY_DEFAULT] = "default", + [ECRC_POLICY_OFF] = "off", + [ECRC_POLICY_ON] = "on" +}; + +/** + * pcie_set_ercr_checking - enable/disable PCIe ECRC checking + * @dev: the PCI device + * + * Returns 0 on success, or negative on failure. + */ +int pcie_set_ecrc_checking(struct pci_dev *dev) +{ + int pos; + u32 reg32; + + if (!dev->is_pcie) + return -ENODEV; + + /* Use firmware/BIOS setting if default */ + if (ecrc_policy == ECRC_POLICY_DEFAULT) + return 0; + + pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR); + if (!pos) + return -ENODEV; + + pci_read_config_dword(dev, pos + PCI_ERR_CAP, ®32); + if (ecrc_policy == ECRC_POLICY_OFF) { + reg32 &= ~(PCI_ERR_CAP_ECRC_GENE | PCI_ERR_CAP_ECRC_CHKE); + } else if (ecrc_policy == ECRC_POLICY_ON) { + if (reg32 & PCI_ERR_CAP_ECRC_GENC) + reg32 |= PCI_ERR_CAP_ECRC_GENE; + if (reg32 & PCI_ERR_CAP_ECRC_CHKC) + reg32 |= PCI_ERR_CAP_ECRC_CHKE; + } + pci_write_config_dword(dev, pos + PCI_ERR_CAP, reg32); + return 0; +} +EXPORT_SYMBOL_GPL(pcie_set_ecrc_checking); + +static int __init pcie_ecrc_get_policy(char *str) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(ecrc_policy_str); i++) + if (!strncmp(str, ecrc_policy_str[i], + strlen(ecrc_policy_str[i]))) + break; + if (i >= ARRAY_SIZE(ecrc_policy_str)) + return 0; + + ecrc_policy = i; + return 1; +} +__setup("pcie_ecrc=", pcie_ecrc_get_policy); diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index e2f3dd0..e2b0ab5 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -984,6 +984,9 @@ static void pci_init_capabilities(struct pci_dev *dev) /* Single Root I/O Virtualization */ pci_iov_init(dev); + + /* PCIe ECRC */ + pcie_set_ecrc_checking(dev); } void pci_device_add(struct pci_dev *dev, struct pci_bus *bus) diff --git a/include/linux/pci.h b/include/linux/pci.h index 507552d..080afd8 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -879,6 +879,15 @@ static inline int pcie_aspm_enabled(void) extern int pcie_aspm_enabled(void); #endif +#ifndef CONFIG_PCIEECRC +static inline int pcie_set_ecrc_checking(struct pci_dev *dev) +{ + return 0; +} +#else +extern int pcie_set_ecrc_checking(struct pci_dev *dev); +#endif + #define pci_enable_msi(pdev) pci_enable_msi_block(pdev, 1) #ifdef CONFIG_HT_IRQ
Add support for turning PCIe ECRC on or off Adds support for PCI Express transaction layer end-to-end CRC checking (ECRC). This patch will enable/disable ECRC checking by setting/clearing the ECRC Check Enable and/or ECRC Generation Enable bits for devices that support ECRC. The ECRC setting is controlled by the "pcie_ecrc=" command-line option. If this option is not set or is set to 'default", the enable and generation bits are left in whatever state that firmware/BIOS sets them to. The "off" setting turns them off, and the "on" option turns them on (if the device supports it). Signed-off-by: Andrew Patterson <andrew.patterson@hp.com> --- -- 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