On 21-11-22 16:46:21, Jonathan Cameron wrote:
> On Fri, 19 Nov 2021 16:02:43 -0800
> Ben Widawsky <ben.widawsky@xxxxxxxxx> wrote:
>
> > The PCIe device DVSEC, defined in the CXL 2.0 spec, 8.1.3 is required to
> > be implemented by CXL 2.0 endpoint devices. Since the information
> > contained within this DVSEC will be critically important for region
> > configuration, it makes sense to find the value early.
> >
> > Since this DVSEC is not strictly required for mailbox functionality,
> > failure to find this information does not result in the driver failing
> > to bind.
>
> That feels like a path we are going to forget to test sometime in the
> future. Given it's a specification requirement, I'd treat it as
> an error and make our lives easier going forwards!
>
> Otherwise looks good to me.
>
Agreed. This is silly. Basically nothing will work if the device dvsec can't be
found. I don't remember what I was thinking...
> >
> > Signed-off-by: Ben Widawsky <ben.widawsky@xxxxxxxxx>
> > ---
> > drivers/cxl/cxlmem.h | 2 ++
> > drivers/cxl/pci.c | 7 +++++++
> > 2 files changed, 9 insertions(+)
> >
> > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> > index 8d96d009ad90..3ef3c652599e 100644
> > --- a/drivers/cxl/cxlmem.h
> > +++ b/drivers/cxl/cxlmem.h
> > @@ -98,6 +98,7 @@ struct cxl_mbox_cmd {
> > *
> > * @dev: The device associated with this CXL state
> > * @regs: Parsed register blocks
> > + * @device_dvsec: Offset to the PCIe device DVSEC
> > * @payload_size: Size of space for payload
> > * (CXL 2.0 8.2.8.4.3 Mailbox Capabilities Register)
> > * @lsa_size: Size of Label Storage Area
> > @@ -125,6 +126,7 @@ struct cxl_dev_state {
> > struct device *dev;
> >
> > struct cxl_regs regs;
> > + int device_dvsec;
> >
> > size_t payload_size;
> > size_t lsa_size;
> > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> > index d2c743a31b0c..f3872cbee7f8 100644
> > --- a/drivers/cxl/pci.c
> > +++ b/drivers/cxl/pci.c
> > @@ -474,6 +474,13 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> > if (IS_ERR(cxlds))
> > return PTR_ERR(cxlds);
> >
> > + cxlds->device_dvsec = pci_find_dvsec_capability(pdev,
> > + PCI_DVSEC_VENDOR_ID_CXL,
> > + CXL_DVSEC_PCIE_DEVICE);
> > + if (!cxlds->device_dvsec)
> > + dev_warn(&pdev->dev,
> > + "Device DVSEC not present. Expect limited functionality.\n");
> > +
> > rc = cxl_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map);
> > if (rc)
> > return rc;
>
