On 4/9/24 2:56 PM, Dave Jiang wrote: > > 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> >>> --- Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx> >>> 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. May be that patch would be the right place to introduce a helper function. But I think it fine either way. >>> + >>> +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. Ok. >>> + >>> + 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 */ -- Sathyanarayanan Kuppuswamy Linux Kernel Developer
