Re: [PATCH] PCI: dwc: ep: Do not map more memory than needed to raise a MSI/MSI-X IRQ

[Niklas Cassel <cassel@kernel.org>]

On Tue, Nov 05, 2024 at 06:13:54AM +0900, Krzysztof Wilczyński wrote:
> Hello,
> 
> > In dw_pcie_ep_init() we allocate memory from the EPC address space that we
> > will later use to raise a MSI/MSI-X IRQ. This memory is only freed in
> > dw_pcie_ep_deinit().
> > 
> > Performing this allocation in dw_pcie_ep_init() is to ensure that we will
> > not fail to allocate memory from the EPC address space when trying to raise
> > a MSI/MSI-X IRQ.
> > 
> > We still map/unmap this memory every time we raise an IRQ, in order to not
> > constantly occupy an iATU, especially for controllers with few iATUs.
> > (So we can still fail to raise an MSI/MSI-X IRQ if all iATUs are occupied.)
> > 
> > When allocating this memory in dw_pcie_ep_init(), we allocate
> > epc->mem->window.page_size memory, which is the smallest unit that we can
> > allocate from the EPC address space.
> > 
> > However, when writing/sending the msg data, which is only 2 bytes for MSI,
> > 4 bytes for MSI-X, in either case a single writel() is sufficient. Thus,
> > the size that we need to map is a single PCI DWORD (4 bytes).
> > 
> > This is the size that we should send in to the pci_epc_ops::align_addr()
> > API. It is align_addr()'s responsibility to return a size that is aligned
> > to the EPC page size, for platforms that need a special alignment.
> > 
> > Modify the align_addr() call to send in the proper size that we need to
> > map.
> > 
> > Before this patch on a system with a EPC page size 64k, we would
> > incorrectly map 128k (which is larger than our allocation) instead of 64k.
> > 
> > After this patch, we will correctly map 64k (a single page). (We should
> > never need to map more than a page to write a single DWORD.)
> [...]
> > Feel free to squash this with the patch that it fixes, if you so prefer.
> 
> Squashed with the rest of the pending changes, per:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/commit/?h=endpoint&id=d2d9f84914e147d6ee399e0ed8d938fea7f0c35c
> 
> Let me know if anything needs to be updated there.

Thank you Krzysztof!


I do not see any point in you adding my Co-developed-by tag since I'm
the Author: of the original commit (the commit that you squashed with).

As far as I know, the Co-developed-by tag should only be added since
there can only be one person marked as Author:


And perhaps:
[kwilczynski: squashed patch that fixes memory map sizes on
    platforms with EPC page size of 64k]

should be:
[kwilczynski: squashed patch that fixes memory map sizes on
    all platforms]
or just:
[kwilczynski: squashed patch that fixes memory map sizes]

Since I (embarrassingly) always mapped twice as much memory as needed in
the original commit.


The squashed commit itself looks correct :)


Kind regards,
Niklas