Re: REGRESSION in 6.0-rc7 caused by patch "ARM/dma-mapping: use dma-direct unconditionally"

From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Marcin Wojtas <mw@semihalf.com>
Cc: "Marek Behún" <kabel@kernel.org>,
	pali@kernel.org, "Christoph Hellwig" <hch@lst.de>,
	"Robin Murphy" <robin.murphy@arm.com>,
	"Arnd Bergmann" <arnd@kernel.org>,
	"Andre Przywara" <andre.przywara@arm.com>,
	"Marc Zyngier" <maz@kernel.org>,
	"Linus Torvalds" <torvalds@linux-foundation.org>,
	"Andrew Lunn" <andrew@lunn.ch>,
	"Gregory Clement" <gregory.clement@bootlin.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	iommu@lists.linux-foundation.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: REGRESSION in 6.0-rc7 caused by patch "ARM/dma-mapping: use dma-direct unconditionally"
Date: Tue, 4 Oct 2022 10:08:15 +0100	[thread overview]
Message-ID: <Yzv3/zM5vMaPSrMS@shell.armlinux.org.uk> (raw)
In-Reply-To: <CAPv3WKeWDWGaMs4VxcLb01tzEz+1pVs5do_MbMdOpvOC7J_DTw@mail.gmail.com>

On Tue, Oct 04, 2022 at 10:30:40AM +0200, Marcin Wojtas wrote:
> Thanks for the explanation and I agree with your reasoning. Therefore
> the below should be sufficient if we use HW BM and non-coherent
> setting:
> 
> --- a/drivers/net/ethernet/marvell/mvneta_bm.c
> +++ b/drivers/net/ethernet/marvell/mvneta_bm.c
> @@ -103,7 +103,7 @@ int mvneta_bm_construct(struct hwbm_pool
> *hwbm_pool, void *buf)
>          */
>         *(u32 *)buf = (u32)buf;
>         phys_addr = dma_map_single(&priv->pdev->dev, buf, bm_pool->buf_size,
> -                                  DMA_FROM_DEVICE);
> +                                  DMA_BIDIRECTIONAL);
>         if (unlikely(dma_mapping_error(&priv->pdev->dev, phys_addr)))
>                 return -ENOMEM;
> 
> Marek - can you please confirm that?

This is insufficient. Marek's patch is the correct version.

The DMA API requires that the direction argument is the same for
mapping, unmapping and syncing a region.

> > I did notice that there was no dma-coherent markings in DT, which means
> > that the DMA API will assume the device is non-coherent, and cache
> > maintenance will be performed. If it is dma-coherent, then cache
> > maintenance won't be performed, and DT needs to be updated to indicate
> > this.
> >
> > If firmware is making the devices DMA coherent, and it's under firmware
> > control, then shouldn't firmware also be updating the kernel's device
> > tree to indicate how it's configured the hardware coherency?
> >
> 
> Imo there are too many boxes out there and updating firmware in the
> field is rather no-go. We already handle this in kernel / DT in big
> extent, so I think we should stick to that path.

Seemingly, we don't handle it "well enough" in the kernel, since with
Christoph's change, we end up with the devices being non-coherent.

Yes, the kernel needs fixing to work with older firmware, but I think
firmware should also be fixed.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel