linux-kernel - Re: [PATCH v2 3/3] PCI: endpoint: pci-epf-vntb: Add MSI doorbell support

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ex3lip5baeop6pj4bjq5z6icq5eif3t4ss57kllplgzbaoubjv@dsn57mraytfs>
Date: Thu, 25 Sep 2025 21:39:48 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Frank Li <Frank.li@....com>
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 Thu, Sep 25, 2025 at 11:25:41AM -0400, Frank Li wrote:

[...]

> > > > > +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.
> > >
> >
> > The memory and size are allocated during pci_epf_test_bind(), I agree. But
> > that's for memory allocated through pci_epf_alloc_space(). So if the MSI region
> > has size restrictions (as taken care by this API), it should be handled in the
> > pci-epf-test driver as well.
> 
> pci_epf_alloc_space() is not only allocate memory, but also set bar's size.
> 
> But we can't change bar's size, for example allocated 64K at
> pci_epf_alloc_space()
> 
> but MSI map only require 4K. Current framework can't change bar's size.
> still have to use 64K for doorbell even MSI just require 4k.
> 
> >
> > Otherwise, we will end up with different implementations between EPF drivers.
> 
> pci_epf_test is special use case for doorbell. No one switch a bar to MSI
> as doorbell dynamtically. Generally, allocated a dedicated bar for
> doorbell.
> 

Ok! I think we would ideally need to expose doorbell registers in any used BAR,
not just in a dedicated BAR. But I believe it is difficult to have multiple
inbound mappings for the same BAR with ATUs.

- Mani

-- 
மணிவண்ணன் சதாசிவம்