From: Ira Weiny <ira.weiny@intel.com>
To: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Bjorn Helgaas <helgaas@kernel.org>,
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 V6 04/10] PCI/DOE: Introduce pci_doe_create_doe_devices
Date: Thu, 24 Mar 2022 16:44:33 -0700 [thread overview]
Message-ID: <Yj0CYdSrVMxlmXyJ@iweiny-desk3> (raw)
In-Reply-To: <20220324140539.00004be8@Huawei.com>
On Thu, Mar 24, 2022 at 02:05:39PM +0000, Jonathan Cameron wrote:
> Hi Ira,
>
> > Here is the code to be more clear...
> >
> >
> > drivers/cxl/pci.c:
> >
> > 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++;
> > 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) {
> > ...
> > /* Create each auxiliary device which internally calls */
> > pci_doe_create_mb(pdev, off, use_irq);
> > ...
> > }
> > ...
> > }
> >
> >
> > drivers/pci/pci-doe.c:
> >
> > static irqreturn_t pci_doe_irq_handler(int irq, void *data)
> > {
> > ...
> > }
> >
> > static int pci_doe_request_irq(struct pci_doe_mb *doe_mb)
> > {
> > struct pci_dev *pdev = doe_mb->pdev;
> > int offset = doe_mb->cap_offset;
> > int doe_irq, rc;
> > u32 val;
> >
> > pci_read_config_dword(pdev, offset + PCI_DOE_CAP, &val);
> >
> > if (!FIELD_GET(PCI_DOE_CAP_INT, val))
> > return -ENOTSUPP;
> >
> > doe_irq = FIELD_GET(PCI_DOE_CAP_IRQ, val);
> > rc = pci_request_irq(pdev, doe_irq, pci_doe_irq_handler,
> > NULL, doe_mb,
> > "DOE[%d:%s]", doe_irq, pci_name(pdev));
> > if (rc)
> > return rc;
> >
> > doe_mb->irq = doe_irq;
> > pci_write_config_dword(pdev, offset + PCI_DOE_CTRL,
> > PCI_DOE_CTRL_INT_EN);
> > return 0;
> > }
> >
> > struct pci_doe_mb *pci_doe_create_mb(struct pci_dev *pdev, u16 cap_offset,
> > bool use_irq)
> > {
> > ...
> > if (use_irq) {
> > rc = pci_doe_request_irq(doe_mb);
> > if (rc)
> > pci_err(pdev, "DOE request irq failed for mailbox @ %u : %d\n",
> > cap_offset, rc);
> > }
> > ...
> > }
> >
> >
> > Does this look reasonable?
>
> I'm a little nervous about how we are going to make DOEs on switches work.
> Guess I'll do an experiment once your next version is out and check we
> can do that reasonably cleanly. For switches we'll probably have to
> check for DOEs on all such ports and end up with infrastructure to
> map to all protocols we might see on a switch.
Are the switches not represented as PCI devices in linux?
If my vision of switches is correct I think that problem is independent of what
I'm solving here. In other words the relationship between a port on a switch
and a DOE capability on that switch will have to be established somehow and
nothing I'm doing precludes doing that, but at the same time nothing I'm doing
helps that either.
Ira
>
> Jonathan
>
> >
next prev parent reply other threads:[~2022-03-24 23:44 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-01 7:19 [PATCH V6 00/10] CXL: Read CDAT and DSMAS data from the device ira.weiny
2022-02-01 7:19 ` [PATCH V6 01/10] PCI: Add vendor ID for the PCI SIG ira.weiny
2022-02-03 17:11 ` Bjorn Helgaas
2022-02-03 20:28 ` Ira Weiny
2022-02-01 7:19 ` [PATCH V6 02/10] PCI: Replace magic constant for PCI Sig Vendor ID ira.weiny
2022-02-04 21:16 ` Dan Williams
2022-02-04 21:49 ` Bjorn Helgaas
2022-03-15 21:48 ` Ira Weiny
2022-02-01 7:19 ` [PATCH V6 03/10] PCI/DOE: Add Data Object Exchange Aux Driver ira.weiny
2022-02-03 22:40 ` Bjorn Helgaas
2022-03-15 21:48 ` Ira Weiny
2022-02-09 0:59 ` Dan Williams
2022-02-09 10:13 ` Jonathan Cameron
2022-02-09 16:26 ` Dan Williams
2022-02-09 16:57 ` Jonathan Cameron
2022-02-09 19:57 ` Dan Williams
2022-02-10 21:51 ` Ira Weiny
2022-03-16 22:50 ` Ira Weiny
2022-03-17 19:37 ` Ira Weiny
2022-02-01 7:19 ` [PATCH V6 04/10] PCI/DOE: Introduce pci_doe_create_doe_devices ira.weiny
2022-02-03 22:44 ` Bjorn Helgaas
2022-02-04 14:51 ` Jonathan Cameron
2022-02-04 16:27 ` Bjorn Helgaas
2022-02-11 2:54 ` Dan Williams
2022-03-24 0:26 ` Ira Weiny
2022-03-24 14:05 ` Jonathan Cameron
2022-03-24 23:44 ` Ira Weiny [this message]
2022-03-25 12:02 ` Jonathan Cameron
2022-02-01 7:19 ` [PATCH V6 05/10] cxl/pci: Create DOE auxiliary devices ira.weiny
2022-02-01 7:19 ` [PATCH V6 06/10] cxl/pci: Find the DOE mailbox which supports CDAT ira.weiny
2022-02-01 18:49 ` Ben Widawsky
2022-02-01 22:18 ` Ira Weiny
2022-02-04 14:04 ` Jonathan Cameron
2022-02-01 7:19 ` [PATCH V6 07/10] cxl/mem: Read CDAT table ira.weiny
2022-02-04 13:46 ` Jonathan Cameron
2022-02-01 7:19 ` [PATCH V6 08/10] cxl/cdat: Introduce cdat_hdr_valid() ira.weiny
2022-02-01 18:56 ` Ben Widawsky
2022-02-01 22:29 ` Ira Weiny
2022-02-04 13:17 ` Jonathan Cameron
2022-02-01 7:19 ` [PATCH V6 09/10] cxl/mem: Retry reading CDAT on failure ira.weiny
2022-02-01 18:59 ` Ben Widawsky
2022-02-01 22:31 ` Ira Weiny
2022-02-04 13:20 ` Jonathan Cameron
2022-02-01 7:19 ` [PATCH V6 10/10] cxl/cdat: Parse out DSMAS data from CDAT table ira.weiny
2022-02-01 19:05 ` Ben Widawsky
2022-02-01 22:37 ` Ira Weiny
2022-02-04 13:33 ` Jonathan Cameron
2022-02-04 13:41 ` Jonathan Cameron
2022-02-04 13:40 ` 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=Yj0CYdSrVMxlmXyJ@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=helgaas@kernel.org \
--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).