Received-SPF: pass (google.com: domain of linux-kernel+bounces-185072-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Tue, 21 May 2024 09:14:31 -0500
From: Bjorn Helgaas <helgaas@kernel.org>
To: Niklas Cassel <cassel@kernel.org>
Cc: Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= <kw@linux.com>,
	bhelgaas@google.com, mani@kernel.org, Frank Li <Frank.Li@nxp.com>,
	imx@lists.linux.dev, jdmason@kudzu.us, jingoohan1@gmail.com,
	linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
	lpieralisi@kernel.org, robh@kernel.org, dlemoal@kernel.org
Subject: Re: [PATCH v4 1/1] PCI: dwc: Fix index 0 incorrectly being
 interpreted as a free ATU slot
Message-ID: <20240521141431.GA25673@bhelgaas>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <Zkx0lzP76bmp4K0r@ryzen.lan>

On Tue, May 21, 2024 at 12:16:55PM +0200, Niklas Cassel wrote:
> On Sat, May 18, 2024 at 02:06:50AM +0900, Krzysztof Wilczyński wrote:
> > Hello,
> > 
> > > When PERST# assert and deassert happens on the PERST# supported platforms,
> > > the both iATU0 and iATU6 will map inbound window to BAR0. DMA will access
> > > to the area that was previously allocated (iATU0) for BAR0, instead of the
> > > new area (iATU6) for BAR0.
> > > 
> > > Right now, we dodge the bullet because both iATU0 and iATU6 should
> > > currently translate inbound accesses to BAR0 to the same allocated memory
> > > area. However, having two separate inbound mappings for the same BAR is a
> > > disaster waiting to happen.
> > > 
> > > The mapping between PCI BAR and iATU inbound window are maintained in the
> > > dw_pcie_ep::bar_to_atu[] array. While allocating a new inbound iATU map for
> > > a BAR, dw_pcie_ep_inbound_atu() API will first check for the availability
> > > of the existing mapping in the array and if it is not found (i.e., value in
> > > the array indexed by the BAR is found to be 0), then it will allocate a new
> > > map value using find_first_zero_bit().
> > > 
> > > The issue here is, the existing logic failed to consider the fact that the
> > > map value '0' is a valid value for BAR0. Because, find_first_zero_bit()
> > > will return '0' as the map value for BAR0 (note that it returns the first
> > > zero bit position).
> > > 
> > > Due to this, when PERST# assert + deassert happens on the PERST# supported
> > > platforms, the inbound window allocation restarts from BAR0 and the
> > > existing logic to find the BAR mapping will return '6' for BAR0 instead of
> > > '0' due to the fact that it considers '0' as an invalid map value.
> > > 
> > > So fix this issue by always incrementing the map value before assigning to
> > > bar_to_atu[] array and then decrementing it while fetching. This will make
> > > sure that the map value '0' always represents the invalid mapping."
> > 
> > Applied to controller/dwc, thank you!
> > 
> > [1/1] PCI: dwc: Fix index 0 incorrectly being interpreted as a free ATU slot
> >       https://git.kernel.org/pci/pci/c/cd3c2f0fff46
> > 
> > 	Krzysztof
> 
> Hello PCI maintainers,
> 
> There was a message sent out that this patch was applied, yet the patch does
> not appear to be part of the pull request that was sent out yesterday:
> https://lore.kernel.org/linux-pci/20240520222943.GA7973@bhelgaas/T/#u
> 
> In fact, there seems to be many PCI patches that have been reviewed and ready
> to be included (some of them for months) that is not part of the pull request.
> 
> Looking at pci/next, these patches do not appear there either, so I assume
> that these patches will also not be included in a follow-up pull request.
> 
> Some of these patches are actual fixes, like the patch in $subject, and do not
> appear to depend on any other patches, so what is the reason for not including
> them in the PCI pull request?

The problem was that we didn't get these applied soon enough for them
to get any time in linux-next before the merge window opened.  I don't
like to add non-trivial things during the merge window, so I deferred
most of these.  I plan to get them in linux-next as soon as v6.10-rc1
is tagged.

If we can make a case for post-merge window fixes, e.g., to fix a
regression in the pull request or other serious issue, that's always a
possibility.

Bjorn