On 3/12/24 12:30 AM, Lukas Wunner wrote:
> On Mon, Mar 11, 2024 at 01:39:53PM -0700, Dave Jiang wrote:
>> +static bool is_cxl_device(struct pci_dev *dev)
>> +{
>> + return pci_find_dvsec_capability(dev, PCI_DVSEC_VENDOR_ID_CXL,
>> + CXL_DVSEC_PCIE_DEVICE);
>> +}
>
> If this was my bikeshed, I'd call it pci_is_cxl() to match pci_is_pcie().
Ok will change.
>
>
>> +static bool is_cxl_port_sbr_masked(struct pci_dev *dev)
>> +{
>> + int dvsec;
>> + int rc;
>> + u16 reg;
>
> Nit: Inverse Christmas tree?
Will change.
>
>
>> static int pci_reset_bus_function(struct pci_dev *dev, bool probe)
>> {
>> int rc;
>>
>> + /* If it's a CXL port and the SBR control is masked, fail the SBR */
>> + if (is_cxl_device(dev) && dev->bus->self &&
>> + is_cxl_port_sbr_masked(dev->bus->self)) {
>> + if (probe)
>> + return 0;
>> +
>> + return -EPERM;
>> + }
>> +
>
> Is this also necessary if CONFIG_CXL_PCI=n?
Yes. As the kernel only loads type3 mem class CXL device driver. This is attempt to cover all CXL devices and not dependent on a loaded driver.
>
> Return code on non-availability of a reset method is generally -ENOTTY.
Ok I can change it to that.
> Or is the choice deliberate to expose this reset method despite the bit
> being set and thus allow user space to unmask it in the first place?
Yes the idea is if user intentionally unmasks the bit via a user tool then reset can go through.
>
> Also, we mostly use pci_upstream_bridge(dev) in lieu of dev->bus->self.
> Is the choice to use the latter deliberate because maybe is_virtfn is
> never set and the device can never be on the root bus? (What about
> RCiEP CXL devices?)
I'll change to pci_upstream_bridge() call. I didn't realize that existed.
>
> Thanks,
>
> Lukas
