Re: [PATCH] arm64: dts: qcom: sc8280xp: fix PCIe DMA coherency

[Konrad Dybcio]

On 24.11.2022 15:25, Johan Hovold wrote:
> The devices on the SC8280XP PCIe buses are cache coherent and must be
> marked as such to avoid data corruption.
> 
> A coherent device can, for example, end up snooping stale data from the
> caches instead of using data written by the CPU through the
> non-cacheable mapping which is used for consistent DMA buffers for
> non-coherent devices.
> 
> Note that this is much more likely to happen since commit c44094eee32f
> ("arm64: dma: Drop cache invalidation from arch_dma_prep_coherent()")
> that was added in 6.1 and which removed the cache invalidation when
> setting up the non-cacheable mapping.
> 
> Marking the PCIe devices as coherent specifically fixes the intermittent
> NVMe probe failures observed on the Thinkpad X13s, which was due to
> corruption of the submission and completion queues. This was typically
> observed as corruption of the admin submission queue (with well-formed
> completion):
> 
> 	could not locate request for tag 0x0
> 	nvme nvme0: invalid id 0 completed on queue 0
> 
> or corruption of the admin or I/O completion queues (malformed
> completion):
> 
> 	could not locate request for tag 0x45f
> 	nvme nvme0: invalid id 25695 completed on queue 25965
> 
> presumably as these queues are small enough to not be allocated using
> CMA which in turn make them more likely to be cached (e.g. due to
> accesses to nearby pages through the cacheable linear map). Increasing
> the buffer sizes to two pages to force CMA allocation also appears to
> make the problem go away.
> 
> Fixes: 813e83157001 ("arm64: dts: qcom: sc8280xp/sa8540p: add PCIe2-4 nodes")
> Signed-off-by: Johan Hovold <johan+linaro@xxxxxxxxxx>
> ---
Looks like 8450 should also be like this, good catch!

Reviewed-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>

Konrad
>  arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> index 27f5c2f82338..7748cd29276d 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> @@ -854,6 +854,8 @@ pcie4: pcie@1c00000 {
>  				 <0x02000000 0x0 0x30300000 0x0 0x30300000 0x0 0x1d00000>;
>  			bus-range = <0x00 0xff>;
>  
> +			dma-coherent;
> +
>  			linux,pci-domain = <6>;
>  			num-lanes = <1>;
>  
> @@ -951,6 +953,8 @@ pcie3b: pcie@1c08000 {
>  				 <0x02000000 0x0 0x32300000 0x0 0x32300000 0x0 0x1d00000>;
>  			bus-range = <0x00 0xff>;
>  
> +			dma-coherent;
> +
>  			linux,pci-domain = <5>;
>  			num-lanes = <2>;
>  
> @@ -1046,6 +1050,8 @@ pcie3a: pcie@1c10000 {
>  				 <0x02000000 0x0 0x34300000 0x0 0x34300000 0x0 0x1d00000>;
>  			bus-range = <0x00 0xff>;
>  
> +			dma-coherent;
> +
>  			linux,pci-domain = <4>;
>  			num-lanes = <4>;
>  
> @@ -1144,6 +1150,8 @@ pcie2b: pcie@1c18000 {
>  				 <0x02000000 0x0 0x38300000 0x0 0x38300000 0x0 0x1d00000>;
>  			bus-range = <0x00 0xff>;
>  
> +			dma-coherent;
> +
>  			linux,pci-domain = <3>;
>  			num-lanes = <2>;
>  
> @@ -1239,6 +1247,8 @@ pcie2a: pcie@1c20000 {
>  				 <0x02000000 0x0 0x3c300000 0x0 0x3c300000 0x0 0x1d00000>;
>  			bus-range = <0x00 0xff>;
>  
> +			dma-coherent;
> +
>  			linux,pci-domain = <2>;
>  			num-lanes = <4>;
>