Re: [XEN RFC PATCH 04/40] xen/arm: return default DMA bit width when platform is not set

[Julien Grall <julien@xxxxxxx>]

Hi,

On 11/08/2021 11:23, Wei Chen wrote:
> From: Hongda Deng <Hongda.Deng@xxxxxxx>
>
> In current code, arch_get_dma_bitsize will return 32 when platorm
> or platform->dma_bitsize is not set. It's not resonable, for Arm,
s/resonable/reasonable/

> we don't require to reserve DMA memory. So we set dma_bitsize always
> be 0. In NO-NUMA system, arch_get_dma_bitsize will not be invoked,
> so dma_bitsize will not be overrided by this function. 
arch_get_dma_bitsize() is also used to allocate dom0 memory. We need to 
be able to allocate some DMA-able memory that can be used by every devices.
> But in NUMA
> system, once the online nodes are greater than 1, this function will
> be invoked. The dma_bitsize will be limited to 32. That means, only
> first 4GB memory can be used for DMA. But that's against our hardware
> design. We don't have that kind of restriction on hardware.
What do you mean by "hardware design"? Are you referring to the server 
you boot Xen on?
Anyway, there are plenty of platform out that have devices which can't 
DMA into memory above 32-bit. On RPI, this is even lower (30-bit).
So I would be cautious to change the default limit.

At the moment, the only place on Arm where we need DMA-able memory is 
for dom0. This is allocated at boot and can't change afterwards (for now).
So I would explore to remove the NUMA check for drop the DMA zone. FAOD, 
both suggestion are for Arm only. For x86, they need to be kept.
> Only
> platform setting can override dma_bitsize. So in this patch, we
> return default dma_bitsize, when platform and platorm->dma_bitsize
> are not set.
>
> Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> Signed-off-by: Hongda Deng <Hongda.Deng@xxxxxxx>
> ---
>   xen/arch/arm/platform.c | 4 +++-
>   xen/common/page_alloc.c | 2 +-
>   2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/platform.c b/xen/arch/arm/platform.c
> index 4db5bbb4c5..0a27fef9a4 100644
> --- a/xen/arch/arm/platform.c
> +++ b/xen/arch/arm/platform.c
> @@ -27,6 +27,7 @@ extern const struct platform_desc _splatform[], _eplatform[];
>   /* Pointer to the current platform description */
>   static const struct platform_desc *platform;
>   
> +extern unsigned int dma_bitsize;
>   
>   static bool __init platform_is_compatible(const struct platform_desc *plat)
>   {
> @@ -157,7 +158,8 @@ bool platform_device_is_blacklisted(const struct 
> dt_device_node *node)
>   
>   unsigned int arch_get_dma_bitsize(void)
>   {
> -    return ( platform && platform->dma_bitsize ) ? platform->dma_bitsize : 32;
> +    return ( platform && platform->dma_bitsize ) ? platform->dma_bitsize
> +                                                 : dma_bitsize;
>   }
>   
>   /*
> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
> index 958ba0cd92..0f0cae5a4e 100644
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -227,7 +227,7 @@ static bool __read_mostly scrub_debug;
>    * Bit width of the DMA heap -- used to override NUMA-node-first.
>    * allocation strategy, which can otherwise exhaust low memory.
>    */
> -static unsigned int dma_bitsize;
> +unsigned int dma_bitsize;
>   integer_param("dma_bits", dma_bitsize);
>   
>   /* Offlined page list, protected by heap_lock. */
Cheers,

--
Julien Grall