Re: [PATCH V2] arm: xen: mm: use __GPF_DMA32 for arm64

From: Julien Grall <Julien.Grall@arm•com>
To: Peng Fan <peng.fan@nxp•com>,
	"sstabellini@kernel•org" <sstabellini@kernel•org>,
	"linux@armlinux•org.uk" <linux@armlinux•org.uk>,
	Catalin Marinas <Catalin.Marinas@arm•com>,
	"will@kernel•org" <will@kernel•org>,
	Robin Murphy <Robin.Murphy@arm•com>
Cc: "xen-devel@lists•xenproject.org" <xen-devel@lists•xenproject.org>,
	nd <nd@arm•com>, dl-linux-imx <linux-imx@nxp•com>,
	"linux-arm-kernel@lists•infradead.org"
	<linux-arm-kernel@lists•infradead.org>
Subject: Re: [PATCH V2] arm: xen: mm: use __GPF_DMA32 for arm64
Date: Fri, 30 Aug 2019 08:39:40 +0000	[thread overview]
Message-ID: <d7477406-a8a0-5c3c-13dc-2c84e27b8afa@arm.com> (raw)
In-Reply-To: <1567175255-1798-1-git-send-email-peng.fan@nxp.com>

Hi Peng,

On 30/08/2019 04:28, Peng Fan wrote:
> From: Peng Fan <peng.fan@nxp•com>
> 
> arm64 shares some code under arch/arm/xen, including mm.c.
> However ZONE_DMA is removed by commit
> ad67f5a6545("arm64: replace ZONE_DMA with ZONE_DMA32").
> So introduce xen_set_gfp_dma for arm32/arm64 and using __GFP_DMA
> for the former and __GFP_DMA32 for the latter.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp•com>
> ---
> 
> V2:
>   Follow suggestion from Stefano,
>   introduce static inline void xen_set_gfp_dma(gfp_t *flags) for arm32/arm64, and
>   for arm64 using __GFP_DMA for the former and __GFP_DMA32 for the latter.
> 
>   arch/arm/include/asm/xen/page.h   | 5 +++++
>   arch/arm/xen/mm.c                 | 2 +-
>   arch/arm64/include/asm/xen/page.h | 5 +++++
>   3 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/include/asm/xen/page.h b/arch/arm/include/asm/xen/page.h
> index 31bbc803cecb..d08309c45e6c 100644
> --- a/arch/arm/include/asm/xen/page.h
> +++ b/arch/arm/include/asm/xen/page.h
> @@ -1 +1,6 @@
>   #include <xen/arm/page.h>
> +
> +static inline void xen_set_gfp_dma(gfp_t *flags)
> +{
> +	*flags |= __GFP_DMA;
> +}
> diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
> index d33b77e9add3..828f49dc95f9 100644
> --- a/arch/arm/xen/mm.c
> +++ b/arch/arm/xen/mm.c
> @@ -28,7 +28,7 @@ unsigned long xen_get_swiotlb_free_pages(unsigned int order)
>   
>   	for_each_memblock(memory, reg) {
>   		if (reg->base < (phys_addr_t)0xffffffff) {
> -			flags |= __GFP_DMA;
> +			xen_set_gfp_dma(&flags);

The name of the helper is quite misleading, this is specific to swiotlb 
yet it gives the impression it can be used everywhere. The helper will 
actually set the flags in order to allocate memory below 4GB.

Also, I saw an e-mail suggesting that __GFP_DMA may be used on Arm64. So 
a user may think using xen_set_gfp_dma() will set _GFP_DMA and not 
_GFP_DMA32.

I know duplication is not great but it feels that duplicating the full 
function (or only the allocation part) would be the best. This would 
require to introduce a new file mm{32,64}.c in the respective arch 
directory.

Cheers,

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