[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 04/37] xen: introduce an arch helper for default dma zone status


  • To: Wei Chen <wei.chen@xxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Mon, 17 Jan 2022 17:10:43 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=u1amZc2Q3D4e8l4yc7etdJ/t6PaP+XFOxFFp4mWKjl4=; b=ZxmJxo/k+JowZ0GXGl/8MKvORKJM05iez8wzS0FxD337+B8CbeFl6sljx/cFHmb9MM/2V8Jik4SK+X8rIwFEePOChLhU0d0vUT1a82YkdXyA0S0MJO/brvugTDZfVAOzrCpl4RuBgK3AkET2BLIB5C2RbABl2B0AmzoNSxh7NJChDMlhB+fM0o6vzD3tsgDAfYFPPGpJZ8gUkhWLqYstrVARVfPeBPpFoVJGCbFVTNJRUYL6HtIpKHt+xxKkTYXQeYz5IfM1U5XP4E+AqG0Y0YjrPZaFVM7nF0Bys+lu+FYXzgiM5S81EtMpkPDScaH9qmPrdG8UzuPvdVd4Mtofkg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Yabpt0KQoleTKRjd1VOMCfpXctx0v2SNIF+6ENtw1P56DaTpewsgtvhoQNk7Gq1ZnnDhQ744mnyhlSVJITUlHf8cIJQHVRGxcn+LH5ypfEShPcSUJJSLr4UB3SBI0tUBbW6uB05U3ay1Xv4cfMoujFM5WasKdogH3QdkQz4RF4C5QGoW25th8jew0MFWkMFJFTBy1xqtCeF0jHo4MmyQBVCeUVX+RqdkQ4vBoTCAZetGuAqmeoyEG7ao7/Kh665ngmgOZLlSxgioVEFShHKirk517f7FAWZaSYLoTLu1tMkzBcsj39MB7W7q4Anw/NEHsXU5aI7ZK+zlRup7nAz4TA==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Bertrand.Marquis@xxxxxxx, xen-devel@xxxxxxxxxxxxxxxxxxxx, sstabellini@xxxxxxxxxx, julien@xxxxxxx
  • Delivery-date: Mon, 17 Jan 2022 16:11:12 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

I realize this series has been pending for a long time, but I don't
recall any indication that it would have been dropped. Hence as a
first try, a few comments on this relatively simple change. I'm
sorry it to have taken so long to get to it.

On 23.09.2021 14:02, Wei Chen wrote:
> In current code, when Xen is running in a multiple nodes NUMA
> system, it will set dma_bitsize in end_boot_allocator to reserve
> some low address memory for DMA.
> 
> There are some x86 implications in current implementation. Becuase
> on x86, memory starts from 0. On a multiple nodes NUMA system, if
> a single node contains the majority or all of the DMA memory. x86
> prefer to give out memory from non-local allocations rather than
> exhausting the DMA memory ranges. Hence x86 use dma_bitsize to set
> aside some largely arbitrary amount memory for DMA memory ranges.
> The allocations from these memory ranges would happen only after
> exhausting all other nodes' memory.
> 
> But the implications are not shared across all architectures. For
> example, Arm doesn't have these implications. So in this patch, we
> introduce an arch_have_default_dmazone helper for arch to determine
> that it need to set dma_bitsize for reserve DMA allocations or not.

How would Arm guarantee availability of memory below a certain
boundary for limited-capability devices? Or is there no need
because there's an assumption that I/O for such devices would
always pass through an IOMMU, lifting address size restrictions?
(I guess in a !PV build on x86 we could also get rid of such a
reservation.)

> --- a/xen/arch/x86/numa.c
> +++ b/xen/arch/x86/numa.c
> @@ -371,6 +371,11 @@ unsigned int __init arch_get_dma_bitsize(void)
>                   + PAGE_SHIFT, 32);
>  }
>  
> +unsigned int arch_have_default_dmazone(void)
> +{
> +    return ( num_online_nodes() > 1 ) ? 1 : 0;
> +}

According to the expression and ...

> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -1889,7 +1889,7 @@ void __init end_boot_allocator(void)
>      }
>      nr_bootmem_regions = 0;
>  
> -    if ( !dma_bitsize && (num_online_nodes() > 1) )
> +    if ( !dma_bitsize && arch_have_default_dmazone() )
>          dma_bitsize = arch_get_dma_bitsize();

... the use site, you mean the function to return boolean. Please
indicate so by making it have a return type of "bool". Independent
of that you don't need a conditional expression above, nor
(malformed) use of parentheses. I further wonder whether ...

> --- a/xen/include/asm-arm/numa.h
> +++ b/xen/include/asm-arm/numa.h
> @@ -25,6 +25,11 @@ extern mfn_t first_valid_mfn;
>  #define node_start_pfn(nid) (mfn_x(first_valid_mfn))
>  #define __node_distance(a, b) (20)
>  
> +static inline unsigned int arch_have_default_dmazone(void)
> +{
> +    return 0;
> +}

... like this one, x86'es couldn't be inline as well. If indeed
it can't be, making it a macro may still be better (and avoid a
further comment regarding the lack of __init).

Jan




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.