| Message ID | 20260902072804.665639-11-smadhavan@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | PCI/CXL: Add CXL reset support for Type 2 devices | expand |
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Bypass of hardware-enforced CXL decoder lock via reset -- commit ac98b5cb3878ca89e0ba4a99f7bf56eb9d00c253 Author: Srirangan Madhavan <smadhavan@nvidia.com> PCI/CXL: Expose CXL Reset as a PCI reset method Add the CXL Reset helper to the PCI reset-method table so userspace can select it through the existing reset_method ABI. Advertise the method for Type 2 CXL devices that report CXL Reset support in the CXL Device DVSEC and pass the function-scope check. Reset execution requires cached HDM decoder state for the target so affected ranges can be validated and HDM programming can be restored. If that state is unavailable at reset time, return -ENOTTY so PCI can try the next reset method. > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index b2879a6be5f80..157f06c576e93 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c [ ... ] > @@ -5145,6 +5146,7 @@ const struct pci_reset_fn_method pci_reset_fn_methods[] = { > { }, > { pci_dev_specific_reset, .name = "device_specific" }, > { pci_dev_acpi_reset, .name = "acpi" }, > + { cxl_reset_function, .name = "cxl_reset" }, [Severity: High] Does exposing this reset method allow bypassing the hardware-enforced CXL decoder lock? When a privileged user triggers a PCI reset on the CXL device via sysfs, the hardware reset clears the decoder lock bit. During recovery, the lock bit is explicitly stripped from the saved state before restoration: drivers/cxl/core/resource.c:cxl_restore_hdm_decoder_state() { ... ctrl &= ~(CXL_HDM_DECODER0_CTRL_COMMIT | CXL_HDM_DECODER0_CTRL_COMMITTED | CXL_HDM_DECODER0_CTRL_COMMIT_ERROR | CXL_HDM_DECODER0_CTRL_LOCK); ... } Then in setup_hw_decoder(), which is called by cxl_commit_start(), the decoder is committed but fails to check settings->flags & CXL_DECODER_F_LOCK to re-apply the lock. The modified control register is written to hardware, leaving it permanently unlocked: drivers/cxl/core/resource.c:setup_hw_decoder() { ... writel(ctrl, hdm + CXL_HDM_DECODER0_CTRL_OFFSET(id)); ... } Since firmware/BIOS sets the CXL decoder lock bit to prevent system memory map modifications, does exposing this reset method expose the system to malicious reprogramming of CXL decoders by bypassing the hardware lock? > { pcie_reset_flr, .name = "flr" },
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index b2879a6be5f8..157f06c576e9 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -35,6 +35,7 @@ #include <linux/aer.h> #include <linux/bitfield.h> #include <linux/suspend.h> +#include <cxl/cxl.h> #include "pci.h" DEFINE_MUTEX(pci_slot_mutex); @@ -5145,6 +5146,7 @@ const struct pci_reset_fn_method pci_reset_fn_methods[] = { { }, { pci_dev_specific_reset, .name = "device_specific" }, { pci_dev_acpi_reset, .name = "acpi" }, + { cxl_reset_function, .name = "cxl_reset" }, { pcie_reset_flr, .name = "flr" }, { pci_af_flr, .name = "af_flr" }, { pci_pm_reset, .name = "pm" }, diff --git a/include/linux/pci.h b/include/linux/pci.h index 84d058b1b492..074767c7afc4 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -52,7 +52,7 @@ PCI_STATUS_PARITY) /* Number of reset methods used in pci_reset_fn_methods array in pci.c */ -#define PCI_NUM_RESET_METHODS 8 +#define PCI_NUM_RESET_METHODS 9 #define PCI_RESET_PROBE true #define PCI_RESET_DO_RESET false
Add the CXL Reset helper to the PCI reset-method table so userspace can select it through the existing reset_method ABI. Advertise the method for Type 2 CXL devices that report CXL Reset support in the CXL Device DVSEC and pass the function-scope check. Reset execution requires cached HDM decoder state for the target so affected ranges can be validated and HDM programming can be restored. If that state is unavailable at reset time, return -ENOTTY so PCI can try the next reset method. Signed-off-by: Srirangan Madhavan <smadhavan@nvidia.com> --- drivers/pci/pci.c | 2 ++ include/linux/pci.h | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-)