linux-kernel - Re: [PATCH v2 1/2] dma: mmp_dma: add support for legacy transition

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 19 Feb 2015 11:29:26 +0100
From:	Daniel Mack <daniel@...que.org>
To:	Robert Jarzmik <robert.jarzmik@...e.fr>,
	Haojian Zhuang <haojian.zhuang@...il.com>,
	Vinod Koul <vinod.koul@...el.com>,
	Arnd Bergmann <arnd@...db.de>,
	Vasily Khoruzhick <anarsoul@...il.com>
CC:	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	dmaengine@...r.kernel.org
Subject: Re: [PATCH v2 1/2] dma: mmp_dma: add support for legacy transition

Hi Robert,

Thanks for pushing this topic :)

On 02/17/2015 09:39 PM, Robert Jarzmik wrote:
> In order to achieve smooth transition of pxa drivers from old legacy dma
> handling to new dmaengine, introduce a function to "hide" dma physical
> channels from dmaengine.
> 
> This is temporary situation where pxa dma will be handled in 2 places :
>  - arch/arm/plat-pxa/dma.c
>  - drivers/dma/mmp_pdma.c
> 
> The resources, ie. dma channels, will be controlled by mmp_dma. The
> legacy code will request or release a channel with
> mmp_pdma_toggle_reserved_channel().
> 
> This is not very pretty, but it ensures both legacy and dmaengine
> consumers can live in the same kernel until the conversion is done.

I like the approach. I actually thought the legacy DMA driver would
claim the io port range, which would have made the two drivers mutually
exclusive. But it doesn't, so this can in fact work, even though it's
really not pretty. Also, it's limited to one single instance of the
mmp-pdma driver, but that reflects reality, so I'm fine.

One minor nit:

> +int mmp_pdma_toggle_reserved_channel(int legacy_channel)
> +{
> +	if (legacy_unavailable & (1 << legacy_channel))
> +		return -EBUSY;
> +	legacy_reserved ^= 1 << legacy_channel;
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(mmp_pdma_toggle_reserved_channel);

My concern is that if pxa_request_dma() is called more than once for
whatever reason by a legacy implementation, the toggled bit mask might
get out of sync. As we know exactly on the caller site what we want to
achieve, let's make the API explicit with something like:

int mmp_pdma_set_reserved_channel(int legacy_channel, bool reserved)


Thanks,
Daniel

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/