| Message ID | 20240531213841.3246055-4-wei.huang2@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | PCIe TPH and cache direct injection support | expand |
On Fri, 31 May 2024 16:38:35 -0500 Wei Huang <wei.huang2@amd.com> wrote: > Provide a kernel option, with related helper functions, to completely > disable TPH so that no TPH headers are generated. Why would someone use this option? > > Co-developed-by: Eric Van Tassell <Eric.VanTassell@amd.com> > Signed-off-by: Eric Van Tassell <Eric.VanTassell@amd.com> > Signed-off-by: Wei Huang <wei.huang2@amd.com> > Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com> > Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com> > Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com> > --- > .../admin-guide/kernel-parameters.txt | 1 + > drivers/pci/pci-driver.c | 7 ++++- > drivers/pci/pci.c | 12 ++++++++ > drivers/pci/pcie/tph.c | 30 +++++++++++++++++++ > include/linux/pci-tph.h | 19 ++++++++++++ > include/linux/pci.h | 1 + > 6 files changed, 69 insertions(+), 1 deletion(-) > create mode 100644 include/linux/pci-tph.h > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt > index 500cfa776225..fedcc69e35c1 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -4623,6 +4623,7 @@ > nomio [S390] Do not use MIO instructions. > norid [S390] ignore the RID field and force use of > one PCI domain per PCI function > + notph [PCIE] Do not use PCIe TPH > > pcie_aspm= [PCIE] Forcibly enable or ignore PCIe Active State Power > Management. > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c > index af2996d0d17f..9722d070c0ca 100644 > --- a/drivers/pci/pci-driver.c > +++ b/drivers/pci/pci-driver.c > @@ -21,6 +21,7 @@ > #include <linux/acpi.h> > #include <linux/dma-map-ops.h> > #include <linux/iommu.h> > +#include <linux/pci-tph.h> > #include "pci.h" > #include "pcie/portdrv.h" > > @@ -322,8 +323,12 @@ static long local_pci_probe(void *_ddi) > pm_runtime_get_sync(dev); > pci_dev->driver = pci_drv; > rc = pci_drv->probe(pci_dev, ddi->id); > - if (!rc) > + if (!rc) { > + if (pci_tph_disabled()) > + pcie_tph_disable(pci_dev); > + > return rc; > + } > if (rc < 0) { > pci_dev->driver = NULL; > pm_runtime_put_sync(dev); > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 59e0949fb079..31c443504ce9 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -157,6 +157,9 @@ static bool pcie_ari_disabled; > /* If set, the PCIe ATS capability will not be used. */ > static bool pcie_ats_disabled; > > +/* If set, the PCIe TPH capability will not be used. */ > +static bool pcie_tph_disabled; > + > /* If set, the PCI config space of each device is printed during boot. */ > bool pci_early_dump; > > @@ -166,6 +169,12 @@ bool pci_ats_disabled(void) > } > EXPORT_SYMBOL_GPL(pci_ats_disabled); > > +bool pci_tph_disabled(void) > +{ > + return pcie_tph_disabled; > +} > +EXPORT_SYMBOL_GPL(pci_tph_disabled); > + > /* Disable bridge_d3 for all PCIe ports */ > static bool pci_bridge_d3_disable; > /* Force bridge_d3 for all PCIe ports */ > @@ -6806,6 +6815,9 @@ static int __init pci_setup(char *str) > pci_no_domains(); > } else if (!strncmp(str, "noari", 5)) { > pcie_ari_disabled = true; > + } else if (!strcmp(str, "notph")) { > + pr_info("PCIe: TPH is disabled\n"); > + pcie_tph_disabled = true; > } else if (!strncmp(str, "cbiosize=", 9)) { > pci_cardbus_io_size = memparse(str + 9, &str); > } else if (!strncmp(str, "cbmemsize=", 10)) { > diff --git a/drivers/pci/pcie/tph.c b/drivers/pci/pcie/tph.c > index 5f0cc06b74bb..5dc533b89a33 100644 > --- a/drivers/pci/pcie/tph.c > +++ b/drivers/pci/pcie/tph.c > @@ -16,11 +16,41 @@ > #include <linux/errno.h> > #include <linux/msi.h> > #include <linux/pci.h> > +#include <linux/pci-tph.h> > #include <linux/msi.h> > #include <linux/pci-acpi.h> > > #include "../pci.h" > > +static int tph_set_reg_field_u32(struct pci_dev *dev, u8 offset, u32 mask, > + u8 shift, u32 field) I'm unconvinced this helper makes sense. Do we do similar for other PCI capabilities? If it does make sense to have a field update, then provide one alongside pci_read_config_dword() etc. > +{ > + u32 reg_val; > + int ret; > + > + if (!dev->tph_cap) > + return -EINVAL; > + > + ret = pci_read_config_dword(dev, dev->tph_cap + offset, ®_val); > + if (ret) > + return ret; > + > + reg_val &= ~mask; > + reg_val |= (field << shift) & mask; > + > + ret = pci_write_config_dword(dev, dev->tph_cap + offset, reg_val); > + > + return ret; return pci_write_config_dword(); > +} > + > +int pcie_tph_disable(struct pci_dev *dev) > +{ > + return tph_set_reg_field_u32(dev, PCI_TPH_CTRL, extra space after return > + PCI_TPH_CTRL_REQ_EN_MASK, > + PCI_TPH_CTRL_REQ_EN_SHIFT, > + PCI_TPH_REQ_DISABLE); > +} > + > void pcie_tph_init(struct pci_dev *dev) > { > dev->tph_cap = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_TPH);
On Fri, May 31, 2024 at 04:38:35PM -0500, Wei Huang wrote: > Provide a kernel option, with related helper functions, to completely > disable TPH so that no TPH headers are generated. If we need this option, say what the option is, why we need it, and include an example of how to use it in the commit log. Include the option in the subject line, too, e.g., "Add pci=notph option to disable TPH". Or maybe "prevent use of TPH"? "Disable TPH" hints that BIOS might have enabled it, and this option would turn that off. I haven't looked hard enough to know exactly what you intend or whether there's a difference.
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 500cfa776225..fedcc69e35c1 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -4623,6 +4623,7 @@ nomio [S390] Do not use MIO instructions. norid [S390] ignore the RID field and force use of one PCI domain per PCI function + notph [PCIE] Do not use PCIe TPH pcie_aspm= [PCIE] Forcibly enable or ignore PCIe Active State Power Management. diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index af2996d0d17f..9722d070c0ca 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -21,6 +21,7 @@ #include <linux/acpi.h> #include <linux/dma-map-ops.h> #include <linux/iommu.h> +#include <linux/pci-tph.h> #include "pci.h" #include "pcie/portdrv.h" @@ -322,8 +323,12 @@ static long local_pci_probe(void *_ddi) pm_runtime_get_sync(dev); pci_dev->driver = pci_drv; rc = pci_drv->probe(pci_dev, ddi->id); - if (!rc) + if (!rc) { + if (pci_tph_disabled()) + pcie_tph_disable(pci_dev); + return rc; + } if (rc < 0) { pci_dev->driver = NULL; pm_runtime_put_sync(dev); diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 59e0949fb079..31c443504ce9 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -157,6 +157,9 @@ static bool pcie_ari_disabled; /* If set, the PCIe ATS capability will not be used. */ static bool pcie_ats_disabled; +/* If set, the PCIe TPH capability will not be used. */ +static bool pcie_tph_disabled; + /* If set, the PCI config space of each device is printed during boot. */ bool pci_early_dump; @@ -166,6 +169,12 @@ bool pci_ats_disabled(void) } EXPORT_SYMBOL_GPL(pci_ats_disabled); +bool pci_tph_disabled(void) +{ + return pcie_tph_disabled; +} +EXPORT_SYMBOL_GPL(pci_tph_disabled); + /* Disable bridge_d3 for all PCIe ports */ static bool pci_bridge_d3_disable; /* Force bridge_d3 for all PCIe ports */ @@ -6806,6 +6815,9 @@ static int __init pci_setup(char *str) pci_no_domains(); } else if (!strncmp(str, "noari", 5)) { pcie_ari_disabled = true; + } else if (!strcmp(str, "notph")) { + pr_info("PCIe: TPH is disabled\n"); + pcie_tph_disabled = true; } else if (!strncmp(str, "cbiosize=", 9)) { pci_cardbus_io_size = memparse(str + 9, &str); } else if (!strncmp(str, "cbmemsize=", 10)) { diff --git a/drivers/pci/pcie/tph.c b/drivers/pci/pcie/tph.c index 5f0cc06b74bb..5dc533b89a33 100644 --- a/drivers/pci/pcie/tph.c +++ b/drivers/pci/pcie/tph.c @@ -16,11 +16,41 @@ #include <linux/errno.h> #include <linux/msi.h> #include <linux/pci.h> +#include <linux/pci-tph.h> #include <linux/msi.h> #include <linux/pci-acpi.h> #include "../pci.h" +static int tph_set_reg_field_u32(struct pci_dev *dev, u8 offset, u32 mask, + u8 shift, u32 field) +{ + u32 reg_val; + int ret; + + if (!dev->tph_cap) + return -EINVAL; + + ret = pci_read_config_dword(dev, dev->tph_cap + offset, ®_val); + if (ret) + return ret; + + reg_val &= ~mask; + reg_val |= (field << shift) & mask; + + ret = pci_write_config_dword(dev, dev->tph_cap + offset, reg_val); + + return ret; +} + +int pcie_tph_disable(struct pci_dev *dev) +{ + return tph_set_reg_field_u32(dev, PCI_TPH_CTRL, + PCI_TPH_CTRL_REQ_EN_MASK, + PCI_TPH_CTRL_REQ_EN_SHIFT, + PCI_TPH_REQ_DISABLE); +} + void pcie_tph_init(struct pci_dev *dev) { dev->tph_cap = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_TPH); diff --git a/include/linux/pci-tph.h b/include/linux/pci-tph.h new file mode 100644 index 000000000000..e187d7e89e8c --- /dev/null +++ b/include/linux/pci-tph.h @@ -0,0 +1,19 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * TPH (TLP Processing Hints) + * + * Copyright (C) 2024 Advanced Micro Devices, Inc. + * Eric Van Tassell <Eric.VanTassell@amd.com> + * Wei Huang <wei.huang2@amd.com> + */ +#ifndef LINUX_PCI_TPH_H +#define LINUX_PCI_TPH_H + +#ifdef CONFIG_PCIE_TPH +int pcie_tph_disable(struct pci_dev *dev); +#else +static inline int pcie_tph_disable(struct pci_dev *dev) +{ return -EOPNOTSUPP; } +#endif + +#endif /* LINUX_PCI_TPH_H */ diff --git a/include/linux/pci.h b/include/linux/pci.h index d75a88ec5136..d88ebe87815a 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1841,6 +1841,7 @@ static inline bool pci_aer_available(void) { return false; } #endif bool pci_ats_disabled(void); +bool pci_tph_disabled(void); #ifdef CONFIG_PCIE_PTM int pci_enable_ptm(struct pci_dev *dev, u8 *granularity);