From: Ira Weiny <ira.weiny@intel.com>
To: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Dan Williams <dan.j.williams@intel.com>,
Bjorn Helgaas <bhelgaas@google.com>,
Alison Schofield <alison.schofield@intel.com>,
Vishal Verma <vishal.l.verma@intel.com>,
Ben Widawsky <ben.widawsky@intel.com>,
linux-kernel@vger.kernel.org, linux-cxl@vger.kernel.org,
linux-pci@vger.kernel.org
Subject: Re: [PATCH V8 04/10] cxl/pci: Create auxiliary devices for each DOE mailbox
Date: Fri, 29 Apr 2022 10:20:54 -0700 [thread overview]
Message-ID: <YmwedkzLrsOmsQt/@iweiny-desk3> (raw)
In-Reply-To: <20220429165502.00000d01@huawei.com>
On Fri, Apr 29, 2022 at 04:55:02PM +0100, Jonathan Cameron wrote:
> On Thu, 14 Apr 2022 13:32:31 -0700
> ira.weiny@intel.com wrote:
>
> > From: Ira Weiny <ira.weiny@intel.com>
> >
[snip]
> > +
> > +/**
> > + * cxl_pci_create_doe_devices - Create auxiliary bus DOE devices for all DOE
> > + * mailboxes found
> > + *
> > + * @pci_dev: The PCI device to scan for DOE mailboxes
> > + *
> > + * There is no coresponding destroy of these devices. This function associates
> > + * the DOE auxiliary devices created with the pci_dev passed in. That
> > + * association is device managed (devm_*) such that the DOE auxiliary device
> > + * lifetime is always less than or equal to the lifetime of the pci_dev.
> > + *
> > + * RETURNS: 0 on success -ERRNO on failure.
> > + */
> > +static int cxl_pci_create_doe_devices(struct pci_dev *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + bool use_irq = true;
> > + int irqs = 0;
> > + u16 off = 0;
> > + int rc;
> > +
> > + pci_doe_for_each_off(pdev, off)
> > + irqs++;
> I believe this is insufficient because there may be other irqs in use
> on the device.
I did not think that was true for any current CXL device. From what I could
tell all CXL devices would be covered by this simple calculation. I left it to
the reader to determine if a new CXL device came along which needed other irq's
to lift this somewhere to cover those allocations. I probably should have made
some comment. Sorry.
> In a similar fashion to that done in pcie/portdrv_core.c
> I think we need to instead find the maximum msi/msix vector number
> by reading the config space.
I was not aware I could do that...
> Then we request one more vector
> than that max value to ensure the vector we need has been requested.
Yea at some point I figured this would be lifted out of here as part of a
larger 'allocate all the vectors for the device' function.
But for now this is the only place that needs irqs so I punted. I can lift
this into something like
cxl_pci_alloc_irq_vectors(...) and then pass use_irq here.
But to move this series forward I would propose that
cxl_pci_alloc_irq_vectors() do what I'm doing here for now.
Ira
>
> Jonathan
>
> > + pci_info(pdev, "Found %d DOE mailbox's\n", irqs);
> > +
> > + /*
> > + * Allocate enough vectors for the DOE's
> > + */
> > + rc = pci_alloc_irq_vectors(pdev, irqs, irqs, PCI_IRQ_MSI |
> > + PCI_IRQ_MSIX);
> > + if (rc != irqs) {
> > + pci_err(pdev,
> > + "Not enough interrupts for all the DOEs; use polling\n");
> > + use_irq = false;
> > + /* Some got allocated; clean them up */
> > + if (rc > 0)
> > + cxl_pci_free_irq_vectors(pdev);
> > + } else {
> > + /*
> > + * Enabling bus mastering is require for MSI/MSIx. It could be
> > + * done later within the DOE initialization, but as it
> > + * potentially has other impacts keep it here when setting up
> > + * the IRQ's.
> > + */
> > + pci_set_master(pdev);
> > + rc = devm_add_action_or_reset(dev,
> > + cxl_pci_free_irq_vectors,
> > + pdev);
> > + if (rc)
> > + return rc;
> > + }
> > +
> > + pci_doe_for_each_off(pdev, off) {
> > + struct auxiliary_device *adev;
> > + struct cxl_doe_dev *new_dev;
> > + int id;
> > +
> > + new_dev = kzalloc(sizeof(*new_dev), GFP_KERNEL);
> > + if (!new_dev)
> > + return -ENOMEM;
> > +
> > + new_dev->pdev = pdev;
> > + new_dev->cap_offset = off;
> > + new_dev->use_irq = use_irq;
> > +
> > + /* Set up struct auxiliary_device */
> > + adev = &new_dev->adev;
> > + id = ida_alloc(&pci_doe_adev_ida, GFP_KERNEL);
> > + if (id < 0) {
> > + kfree(new_dev);
> > + return -ENOMEM;
> > + }
> > +
> > + adev->id = id;
> > + adev->name = DOE_DEV_NAME;
> > + adev->dev.release = cxl_pci_doe_dev_release;
> > + adev->dev.parent = dev;
> > +
> > + if (auxiliary_device_init(adev)) {
> > + cxl_pci_doe_dev_release(&adev->dev);
> > + return -EIO;
> > + }
> > +
> > + if (auxiliary_device_add(adev)) {
> > + auxiliary_device_uninit(adev);
> > + return -EIO;
> > + }
> > +
> > + rc = devm_add_action_or_reset(dev, cxl_pci_doe_destroy_device,
> > + adev);
> > + if (rc)
> > + return rc;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> > {
> > struct cxl_register_map map;
> > @@ -630,6 +753,10 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> > if (rc)
> > return rc;
> >
> > + rc = cxl_pci_create_doe_devices(pdev);
> > + if (rc)
> > + return rc;
> > +
> > cxl_dvsec_ranges(cxlds);
> >
> > cxlmd = devm_cxl_add_memdev(cxlds);
>
next prev parent reply other threads:[~2022-04-29 17:21 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-14 20:32 [PATCH V8 00/10] CXL: Read CDAT and DSMAS data from the device ira.weiny
2022-04-14 20:32 ` [PATCH V8 01/10] PCI: Add vendor ID for the PCI SIG ira.weiny
2022-04-14 20:32 ` [PATCH V8 02/10] PCI: Replace magic constant for PCI Sig Vendor ID ira.weiny
2022-04-14 20:32 ` [PATCH V8 03/10] PCI: Create PCI library functions in support of DOE mailboxes ira.weiny
2022-04-28 21:27 ` Bjorn Helgaas
2022-05-02 5:36 ` ira.weiny
2022-05-30 19:06 ` Lukas Wunner
2022-05-31 10:33 ` Jonathan Cameron
2022-06-01 2:59 ` Ira Weiny
2022-06-01 7:18 ` Lukas Wunner
2022-06-01 14:23 ` Jonathan Cameron
2022-06-01 17:16 ` Ira Weiny
2022-06-01 17:56 ` Lukas Wunner
2022-06-01 20:17 ` Ira Weiny
2022-06-06 14:46 ` Jonathan Cameron
2022-06-06 19:56 ` Ira Weiny
2022-06-07 9:58 ` Jonathan Cameron
2022-05-31 23:43 ` Ira Weiny
2022-04-14 20:32 ` [PATCH V8 04/10] cxl/pci: Create auxiliary devices for each DOE mailbox ira.weiny
2022-04-27 17:19 ` Jonathan Cameron
2022-04-28 21:09 ` ira.weiny
2022-04-29 16:38 ` Jonathan Cameron
2022-04-29 17:01 ` Dan Williams
2022-05-03 16:14 ` Jonathan Cameron
2022-04-29 15:55 ` Jonathan Cameron
2022-04-29 17:20 ` Ira Weiny [this message]
2022-05-03 15:32 ` Jonathan Cameron
2022-04-14 20:32 ` [PATCH V8 05/10] cxl/pci: Create DOE auxiliary driver ira.weiny
2022-04-27 17:43 ` Jonathan Cameron
2022-04-28 14:48 ` ira.weiny
2022-04-28 15:17 ` Jonathan Cameron
2022-04-14 20:32 ` [PATCH V8 06/10] cxl/pci: Find the DOE mailbox which supports CDAT ira.weiny
2022-04-27 17:49 ` Jonathan Cameron
2022-05-09 21:25 ` Ira Weiny
2022-04-14 20:32 ` [PATCH V8 07/10] cxl/mem: Read CDAT table ira.weiny
2022-04-27 17:55 ` Jonathan Cameron
2022-04-14 20:32 ` [PATCH V8 08/10] cxl/cdat: Introduce cxl_cdat_valid() ira.weiny
2022-04-27 17:56 ` Jonathan Cameron
2022-04-14 20:32 ` [PATCH V8 09/10] cxl/mem: Retry reading CDAT on failure ira.weiny
2022-04-27 17:57 ` Jonathan Cameron
2022-04-14 20:32 ` [PATCH V8 10/10] cxl/port: Parse out DSMAS data from CDAT table ira.weiny
2022-04-27 18:01 ` Jonathan Cameron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YmwedkzLrsOmsQt/@iweiny-desk3 \
--to=ira.weiny@intel.com \
--cc=Jonathan.Cameron@huawei.com \
--cc=alison.schofield@intel.com \
--cc=ben.widawsky@intel.com \
--cc=bhelgaas@google.com \
--cc=dan.j.williams@intel.com \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=vishal.l.verma@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).