| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aNRZxAU0n1hvYeEZ@lizhi-Precision-Tower-5810>
Date: Wed, 24 Sep 2025 16:51:16 -0400
From: Frank Li <Frank.li@....com>
To: Manivannan Sadhasivam <mani@...nel.org>
Cc: Krzysztof Wilczyński <kwilczynski@...nel.org>,
Kishon Vijay Abraham I <kishon@...nel.org>,
Bjorn Helgaas <bhelgaas@...gle.com>, Jon Mason <jdmason@...zu.us>,
Dave Jiang <dave.jiang@...el.com>, Allen Hubbe <allenbh@...il.com>,
linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
ntb@...ts.linux.dev, imx@...ts.linux.dev
Subject: Re: [PATCH v2 3/3] PCI: endpoint: pci-epf-vntb: Add MSI doorbell
support
On Wed, Sep 24, 2025 at 11:52:29PM +0530, Manivannan Sadhasivam wrote:
> On Mon, Sep 15, 2025 at 06:22:46PM -0400, Frank Li wrote:
> > Add MSI doorbell support to reduce latency between PCI host and EP.
> >
> > Before this change:
> > ping 169.254.172.137
> > 64 bytes from 169.254.172.137: icmp_seq=1 ttl=64 time=0.575 ms
> > 64 bytes from 169.254.172.137: icmp_seq=2 ttl=64 time=1.80 ms
> > 64 bytes from 169.254.172.137: icmp_seq=3 ttl=64 time=8.19 ms
> > 64 bytes from 169.254.172.137: icmp_seq=4 ttl=64 time=2.00 ms
> >
> > After this change:
> > ping 169.254.144.71
> > 64 bytes from 169.254.144.71: icmp_seq=1 ttl=64 time=0.215 ms
> > 64 bytes from 169.254.144.71: icmp_seq=2 ttl=64 time=0.456 ms
> > 64 bytes from 169.254.144.71: icmp_seq=3 ttl=64 time=0.448 ms
> >
> > Change u64 db to atomic_64 because difference doorbell may happen at the
> > same time.
> >
> > Signed-off-by: Frank Li <Frank.Li@....com>
> > ---
> > change in v2
> > - update api pci_epf_set_inbound_space
> > - atomic_64 should be enough, which just record doorbell events, which is
> > similar with W1C irq status register.
> > ---
> > drivers/pci/endpoint/functions/pci-epf-vntb.c | 153 +++++++++++++++++++++++---
> > 1 file changed, 136 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > index 83e9ab10f9c4fc2b485d5463faa2172500f12999..06c13f26db1c6e55d23769e98e2cfd80da693a20 100644
> > --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > @@ -36,11 +36,13 @@
> > * PCIe Root Port PCI EP
> > */
> >
> > +#include <linux/atomic.h>
> > #include <linux/delay.h>
> > #include <linux/io.h>
> > #include <linux/module.h>
> > #include <linux/slab.h>
> >
> > +#include <linux/pci-ep-msi.h>
> > #include <linux/pci-epc.h>
> > #include <linux/pci-epf.h>
> > #include <linux/ntb.h>
> > @@ -126,12 +128,13 @@ struct epf_ntb {
> > u32 db_count;
> > u32 spad_count;
> > u64 mws_size[MAX_MW];
> > - u64 db;
> > + atomic64_t db;
> > u32 vbus_number;
> > u16 vntb_pid;
> > u16 vntb_vid;
> >
> > bool linkup;
> > + bool msi_doorbell;
> > u32 spad_size;
> >
> > enum pci_barno epf_ntb_bar[VNTB_BAR_NUM];
> > @@ -258,9 +261,9 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
> >
> > ntb = container_of(work, struct epf_ntb, cmd_handler.work);
> >
> > - for (i = 1; i < ntb->db_count; i++) {
> > + for (i = 1; i < ntb->db_count && !ntb->msi_doorbell; i++) {
> > if (ntb->epf_db[i]) {
> > - ntb->db |= 1 << (i - 1);
> > + atomic64_or(1 << (i - 1), &ntb->db);
> > ntb_db_event(&ntb->ntb, i);
> > ntb->epf_db[i] = 0;
> > }
> > @@ -319,7 +322,24 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
> >
> > reset_handler:
> > queue_delayed_work(kpcintb_workqueue, &ntb->cmd_handler,
> > - msecs_to_jiffies(5));
> > + ntb->msi_doorbell ? msecs_to_jiffies(500) : msecs_to_jiffies(5));
> > +}
> > +
> > +static irqreturn_t epf_ntb_doorbell_handler(int irq, void *data)
> > +{
> > + struct epf_ntb *ntb = data;
> > + int i = 0;
> > +
> > + for (i = 1; i < ntb->db_count; i++)
> > + if (irq == ntb->epf->db_msg[i].virq) {
> > + atomic64_or(1 << (i - 1), &ntb->db);
> > + ntb_db_event(&ntb->ntb, i);
> > + }
> > +
> > + if (irq == ntb->epf->db_msg[0].virq)
>
> So doorbell 0 is supposed to trigger the command handler? Is it documented
> somewhere?
I missed my very old patch in drivers/ntb/hw/epf/ntb_hw_epf.c to use
doorbell 0 as trigger command handler, this path should never triggered.
db 0 is not used in ntb_hw_epf.c
>
> > + queue_delayed_work(kpcintb_workqueue, &ntb->cmd_handler, 0);
> > +
> > + return IRQ_HANDLED;
> > }
> >
> > /**
> > @@ -500,6 +520,90 @@ static int epf_ntb_configure_interrupt(struct epf_ntb *ntb)
> > return 0;
> > }
> >
> > +static int epf_ntb_db_bar_init_msi_doorbell(struct epf_ntb *ntb,
> > + struct pci_epf_bar *db_bar,
> > + const struct pci_epc_features *epc_features,
> > + enum pci_barno barno)
> > +{
> > + struct pci_epf *epf = ntb->epf;
> > + dma_addr_t low, high;
> > + struct msi_msg *msg;
> > + size_t sz;
> > + int ret;
> > + int i;
> > +
> > + ret = pci_epf_alloc_doorbell(epf, ntb->db_count);
> > + if (ret)
> > + return ret;
> > +
> > + for (i = 0; i < ntb->db_count; i++) {
> > + ret = request_irq(epf->db_msg[i].virq, epf_ntb_doorbell_handler,
> > + 0, "vntb_db", ntb);
> > +
> > + if (ret) {
> > + dev_err(&epf->dev,
> > + "Failed to request doorbell IRQ: %d\n",
> > + epf->db_msg[i].virq);
> > + goto err_request_irq;
> > + }
> > + }
> > +
> > + msg = &epf->db_msg[0].msg;
> > +
> > + high = 0;
> > + low = (u64)msg->address_hi << 32 | msg->address_lo;
> > +
> > + for (i = 0; i < ntb->db_count; i++) {
> > + struct msi_msg *msg = &epf->db_msg[i].msg;
> > + dma_addr_t addr = (u64)msg->address_hi << 32 | msg->address_lo;
> > +
> > + low = min(low, addr);
> > + high = max(high, addr);
> > + }
> > +
> > + sz = high - low + sizeof(u32);
> > +
> > + ret = pci_epf_set_inbound_space(epf, sz, barno,
> > + epc_features, 0, low);
>
> Should this new API be used in pci-epf-test also?
Needn't, because pcie-epf-test default set system memory as bar's space.
switch to MMIO when enable doorbell and switch back to system memory.
size alignment already consider at bar initilization, and we can't change
bar's size after bind now.
Ideally, msg MMIO space can append into BAR0 space to avoid polling status
change. But not easy to implement yet.
Frank
>
> - Mani
>
> --
> மணிவண்ணன் சதாசிவம்
Powered by blists - more mailing lists