Re: [PATCH 2/4] PCI: endpoint: pci-epf-test: Sanity-check doorbell offset within BAR

Koichiro Den
2026-02-16T14:17:40Z

On Mon, Feb 16, 2026 at 02:14:14PM +0100, Niklas Cassel wrote:
> 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?

Yes I see your point.

>
> 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...

Yes, that's my impression as well. This looks more like a theoritical safeguard
in case pci_epf_align_inbound_addr() or nearby code paths gets accidentally
broken by future changes..

>
> 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 ?

Hm, ok let's drop this patch at least from this series for now. I'll reconsider
it as part of the feature series instead.

Thanks,
Koichiro

>
>
> Kind regards,
> Niklas