Re: [PATCH v2] PCI: dwc: endpoint: Fix dw_pcie_ep_raise_msix_irq() alignment support

[Niklas Cassel <Niklas.Cassel@wdc.com>]

On Wed, Jan 03, 2024 at 11:33:35AM +0530, Kishon Vijay Abraham I wrote:
> Hi Niklas,

Hello Kishon!


> > > I assume dw_pcie_ep_map_addr(), writel(), dw_pcie_ep_unmap_addr() have
> > > to happen atomically so nobody else uses that piece of the ATU while
> > > we're doing this?  There's no mutex here, so I guess we must know this
> > > is atomic already because of something else?
> > 
> > Most devices have multiple iATUs (so multiple iATU indexes).
> > 
> > pcie-designware-ep.c:dw_pcie_ep_outbound_atu()
> > uses find_first_zero_bit() to make sure that a specific iATU (index)
> > is not reused for something else:
> > https://github.com/torvalds/linux/blob/v6.7-rc7/drivers/pci/controller/dwc/pcie-designware-ep.c#L208
> > 
> > A specific iATU (index) is then freed by dw_pcie_ep_unmap_addr(),
> > which does a clear_bit() for that iATU (index).
> > 
> > It is a bit scary that there is no mutex or anything, since
> > find_first_zero_bit() is _not_ atomic, so if we have concurrent calls
> > to dw_pcie_ep_map_addr(), things might break, but that is a separate
> > issue.
> 
> There cannot be concurrent calls to dw_pcie_ep_map_addr() in the current
> code path as pci_epc_raise_irq(), pci_epc_map_addr() and
> pci_epc_unmap_addr() which invokes dw_pcie_ep_map_addr() takes EPC lock in
> pci-epc-core.

I must have overlooked the mutex in pci-epc-core.
Thank you for clearing that up.


Kind regards,
Niklas