On 4/9/24 2:39 PM, Kuppuswamy Sathyanarayanan wrote: > > On 4/9/24 9:01 AM, Dave Jiang wrote: >> Per CXL spec r3.1 8.1.5.2, Secondary Bus Reset (SBR) is masked unless the >> "Unmask SBR" bit is set. Add a check to the PCI secondary bus reset >> path to fail the CXL SBR request if the "Unmask SBR" bit is clear in >> the CXL Port Control Extensions register by returning -ENOTTY. >> >> When the "Unmask SBR" bit is set to 0 (default), the bus_reset would >> appear to have executed successfully. However the operation is actually >> masked. The intention is to inform the user that SBR for the CXL device >> is masked and will not go through. >> >> If the "Unmask SBR" bit is set to 1, then the bus reset will execute >> successfully. >> >> Link: https://lore.kernel.org/linux-cxl/20240220203956.GA1502351@bhelgaas/ >> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> >> Signed-off-by: Dave Jiang <dave.jiang@xxxxxxxxx> >> --- >> v4: >> - cxl_port_dvsec() should return u16. (Lukas) >> --- >> drivers/pci/pci.c | 45 +++++++++++++++++++++++++++++++++++ >> include/uapi/linux/pci_regs.h | 5 ++++ >> 2 files changed, 50 insertions(+) >> >> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c >> index e5f243dd4288..570b00fe10f7 100644 >> --- a/drivers/pci/pci.c >> +++ b/drivers/pci/pci.c >> @@ -4927,10 +4927,55 @@ static int pci_dev_reset_slot_function(struct pci_dev *dev, bool probe) >> return pci_reset_hotplug_slot(dev->slot->hotplug, probe); >> } >> >> +static u16 cxl_port_dvsec(struct pci_dev *dev) >> +{ >> + return pci_find_dvsec_capability(dev, PCI_VENDOR_ID_CXL, >> + PCI_DVSEC_CXL_PORT); >> +} > > Since cxl_sbr_masked() is the only user of this function, why not directly > check for this capability there. It's used by another function in the 3rd patch. I previously had it open coded. But Dan said to reduce churn, just create the function to begin with instead of moving that code to a function later on. > >> + >> +static bool cxl_sbr_masked(struct pci_dev *dev) >> +{ >> + u16 dvsec, reg; >> + int rc; >> + >> + /* >> + * No DVSEC found, either is not a CXL port, or not connected in which >> + * case mask state is a nop (CXL r3.1 sec 9.12.3 "Enumerating CXL RPs >> + * and DSPs" >> + */ >> + dvsec = cxl_port_dvsec(dev); >> + if (!dvsec) >> + return false; >> + >> + rc = pci_read_config_word(dev, dvsec + PCI_DVSEC_CXL_PORT_CTL, ®); >> + if (rc || PCI_POSSIBLE_ERROR(reg)) >> + return false; >> + >> + /* >> + * CXL spec r3.1 8.1.5.2 >> + * When 0, SBR bit in Bridge Control register of this Port has no effect. >> + * When 1, the Port shall generate hot reset when SBR bit in Bridge >> + * Control gets set to 1. >> + */ >> + if (reg & PCI_DVSEC_CXL_PORT_CTL_UNMASK_SBR) >> + return false; >> + >> + return true; >> +} >> + >> static int pci_reset_bus_function(struct pci_dev *dev, bool probe) >> { >> + struct pci_dev *bridge = pci_upstream_bridge(dev); >> int rc; >> >> + /* If it's a CXL port and the SBR control is masked, fail the SBR */ >> + if (bridge && cxl_sbr_masked(bridge)) { >> + if (probe) >> + return 0; > > Why return success during the probe? Otherwise the reset_method will disappear as available after initial probe. We want to leave the reset method available. If the register bit gets unmasked we can perform a bus reset. We don't want to take it away the option entirely if it's masked. > >> + >> + return -ENOTTY; >> + } >> + >> rc = pci_dev_reset_slot_function(dev, probe); >> if (rc != -ENOTTY) >> return rc; >> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h >> index a39193213ff2..d61fa43662e3 100644 >> --- a/include/uapi/linux/pci_regs.h >> +++ b/include/uapi/linux/pci_regs.h >> @@ -1148,4 +1148,9 @@ >> #define PCI_DOE_DATA_OBJECT_DISC_RSP_3_PROTOCOL 0x00ff0000 >> #define PCI_DOE_DATA_OBJECT_DISC_RSP_3_NEXT_INDEX 0xff000000 >> >> +/* Compute Express Link (CXL) */ >> +#define PCI_DVSEC_CXL_PORT 3 >> +#define PCI_DVSEC_CXL_PORT_CTL 0x0c >> +#define PCI_DVSEC_CXL_PORT_CTL_UNMASK_SBR 0x00000001 >> + >> #endif /* LINUX_PCI_REGS_H */ >
