lkml.org 
[lkml]   [2022]   [Dec]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRE: [PATCH V2 01/11] cxl/pci: Add generic MSI-X/MSI irq support
    ira.weiny@ wrote:
    > From: Davidlohr Bueso <dave@stgolabs.net>
    >
    > Currently the only CXL features targeted for irq support require their
    > message numbers to be within the first 16 entries. The device may
    > however support less than 16 entries depending on the support it
    > provides.
    >
    > Attempt to allocate these 16 irq vectors. If the device supports less
    > then the PCI infrastructure will allocate that number.

    What happens if the device supports 16, but irq-core allocates less? I
    believe the answer is with the first user, but this patch does not
    include a user.

    > Store the number of vectors actually allocated in the device state for
    > later use by individual functions.

    The patch does not do that.

    I know this patch has gone through a lot of discussion, but this
    mismatch shows it should really be squashed with the first user because
    it does not stand on its own anymore.

    > Upon successful allocation, users can plug in their respective isr at
    > any point thereafter, for example, if the irq setup is not done in the
    > PCI driver, such as the case of the CXL-PMU.
    >
    > Cc: Bjorn Helgaas <helgaas@kernel.org>
    > Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
    > Co-developed-by: Ira Weiny <ira.weiny@intel.com>
    > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
    > Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
    >
    > ---
    > Changes from V1:
    > Jonathan
    > pci_alloc_irq_vectors() cleans up the vectors automatically
    > use msi_enabled rather than nr_irq_vecs
    >
    > Changes from Ira
    > Remove reviews
    > Allocate up to a static 16 vectors.
    > Change cover letter
    > ---
    > drivers/cxl/cxlmem.h | 3 +++
    > drivers/cxl/cxlpci.h | 6 ++++++
    > drivers/cxl/pci.c | 23 +++++++++++++++++++++++
    > 3 files changed, 32 insertions(+)
    >
    > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
    > index 88e3a8e54b6a..cd35f43fedd4 100644
    > --- a/drivers/cxl/cxlmem.h
    > +++ b/drivers/cxl/cxlmem.h
    > @@ -211,6 +211,7 @@ struct cxl_endpoint_dvsec_info {
    > * @info: Cached DVSEC information about the device.
    > * @serial: PCIe Device Serial Number
    > * @doe_mbs: PCI DOE mailbox array
    > + * @msi_enabled: MSI-X/MSI has been enabled
    > * @mbox_send: @dev specific transport for transmitting mailbox commands
    > *
    > * See section 8.2.9.5.2 Capacity Configuration and Label Storage for
    > @@ -247,6 +248,8 @@ struct cxl_dev_state {
    >
    > struct xarray doe_mbs;
    >
    > + bool msi_enabled;
    > +

    This goes unused in this patch and it also duplicates what the core
    offers with pdev->{msi,msix}_enabled.

    > int (*mbox_send)(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd);
    > };
    >
    > diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
    > index eec597dbe763..b7f4e2f417d3 100644
    > --- a/drivers/cxl/cxlpci.h
    > +++ b/drivers/cxl/cxlpci.h
    > @@ -53,6 +53,12 @@
    > #define CXL_DVSEC_REG_LOCATOR_BLOCK_ID_MASK GENMASK(15, 8)
    > #define CXL_DVSEC_REG_LOCATOR_BLOCK_OFF_LOW_MASK GENMASK(31, 16)
    >
    > +/*
    > + * NOTE: Currently all the functions which are enabled for CXL require their
    > + * vectors to be in the first 16. Use this as the max.
    > + */
    > +#define CXL_PCI_REQUIRED_VECTORS 16
    > +
    > /* Register Block Identifier (RBI) */
    > enum cxl_regloc_type {
    > CXL_REGLOC_RBI_EMPTY = 0,
    > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
    > index faeb5d9d7a7a..8f86f85d89c7 100644
    > --- a/drivers/cxl/pci.c
    > +++ b/drivers/cxl/pci.c
    > @@ -428,6 +428,27 @@ static void devm_cxl_pci_create_doe(struct cxl_dev_state *cxlds)
    > }
    > }
    >
    > +static void cxl_pci_alloc_irq_vectors(struct cxl_dev_state *cxlds)
    > +{
    > + struct device *dev = cxlds->dev;
    > + struct pci_dev *pdev = to_pci_dev(dev);
    > + int nvecs;
    > +
    > + /*
    > + * NOTE: pci_alloc_irq_vectors() handles calling pci_free_irq_vectors()
    > + * automatically despite not being called pcim_*. See
    > + * pci_setup_msi_context().
    > + */
    > + nvecs = pci_alloc_irq_vectors(pdev, 1, CXL_PCI_REQUIRED_VECTORS,
    > + PCI_IRQ_MSIX | PCI_IRQ_MSI);

    clang-format would scooch that second line in for you.

    Might also be worth a comment for the next person that goes looking for
    why this isn't PCI_IRQ_ALL_TYPES.

    From CXL 3.0 3.1.1 CXL.io Endpoint:
    A Function on a CXL device must not generate INTx messages if that
    Function participates in CXL.cache protocol or CXL.mem protocols.


    > + if (nvecs < 0) {
    > + dev_dbg(dev, "Failed to alloc irq vectors; use polling instead.\n");
    > + return;
    > + }
    > +
    > + cxlds->msi_enabled = true;
    > +}
    > +
    > static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
    > {
    > struct cxl_register_map map;
    > @@ -494,6 +515,8 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
    > if (rc)
    > return rc;
    >
    > + cxl_pci_alloc_irq_vectors(cxlds);
    > +
    > cxlmd = devm_cxl_add_memdev(cxlds);
    > if (IS_ERR(cxlmd))
    > return PTR_ERR(cxlmd);
    > --
    > 2.37.2
    >


    \
     
     \ /
      Last update: 2022-12-02 01:29    [W:5.221 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site