Re: [PATCH 2/4] PCI: endpoint: pci-epf-test: Sanity-check doorbell offset within BAR
Niklas Cassel <[email protected]>
| Newsgroups | dev.linux.lists.ntb,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pci |
|---|---|
| Message-ID | <aZMYJsjpqPR9uNqp@ryzen> |
On Mon, Feb 16, 2026 at 12:09:12AM +0900, Koichiro Den wrote: > pci-epf-test advertises the doorbell target to the RC as a BAR number > and an offset. The RC rings the doorbell with a single DWORD MMIO write > to BAR + offset. > > For MSI/MSI-X-based doorbells, the message address is required to be > DWORD-aligned, so the computed offset should not straddle a BAR boundary > in normal operation. > > However, with support for doorbells based on mechanisms other than > MSI/MSI-X (via pci_epf_alloc_doorbell()), the returned message address > may not necessarily be DWORD-aligned. In such a case, offset plus the > 32-bit write width could cross the end of the BAR aperture. The offset > returned by pci_epf_align_inbound_addr() is guaranteed to be within the > BAR size, but this alone does not ensure that a 32-bit write starting at > that offset stays within the BAR. > > Add a bounds check to ensure that the 32-bit doorbell write always stays > within the BAR aperture. While this should not trigger for > spec-compliant MSI/MSI-X addresses, it provides a defensive guard > against unexpected offsets from future doorbell implementations. I think everything you write is true, and I know that I suggested this... But for the MMIO address, will it ever not be 32-bit aligned? Even in the eDMA case, the eDMA registers are 32-bit aligned. Did I perhaps have a brain fart and overthink this? I guess theoretically, some future pci_epf_alloc_doorbell() implementation could return something that is not 32-bit aligned... But if we really want to add a safety check for that... perhaps a 32-bit alignment check would be better suited to have in pci_epf_alloc_doorbell() ? Perhaps this check is better added in pci_epf_alloc_doorbell() or pci_epf_alloc_doorbell_embedded(), in the series that adds support for embedded doorbells ? Kind regards, Niklas