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

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Jan,

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: 2022年1月18日 16:16
> To: Wei Chen <Wei.Chen@xxxxxxx>
> Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>; xen-
> devel@xxxxxxxxxxxxxxxxxxxx; sstabellini@xxxxxxxxxx; julien@xxxxxxx
> Subject: Re: [PATCH 04/37] xen: introduce an arch helper for default dma
> zone status
> 
> On 18.01.2022 08:51, Wei Chen wrote:
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@xxxxxxxx>
> >> Sent: 2022年1月18日 0:11
> >> 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.)
> >
> > On Arm, we still can have some devices with limited DMA capability.
> > And we also don't force all such devices to use IOMMU. This devices
> > will affect the dma_bitsize. Like RPi platform, it sets its dma_bitsize
> > to 30. But in multiple NUMA nodes system, Arm doesn't have a default
> > DMA zone. Multiple nodes is not a constraint on dma_bitsize. And some
> > previous discussions are placed here [1].
> 
> I'm afraid that doesn't give me more clues. For example, in the mail
> being replied to there I find "That means, only first 4GB memory can
> be used for DMA." Yet that's not an implication from setting
> dma_bitsize. DMA is fine to occur to any address. The special address
> range is being held back in case in particular Dom0 is in need of such
> a range to perform I/O to _some_ devices.

I am sorry that my last reply hasn't given you more clues. On Arm, only
Dom0 can have DMA without IOMMU. So when we allocate memory for Dom0,
we're trying to allocate memory under 4GB or in the range of dma_bitsize
indicated. I think these operations meet your above Dom0 special address
range description. As we have already allocated memory for DMA, so I
think we don't need a DMA zone in page allocation. I am not sure is that
answers your earlier question?

> 
> >>> --- 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 ...
> >>
> >
> > I will fix them in next version. But I am not very clear about
> > this comment "of that you don't need a conditional expression above",
> > The "above" indicates this line:
> > "return ( num_online_nodes() > 1 ) ? 1 : 0;"?
> 
> Yes. Even without the use of bool such an expression is a more
> complicated form of
> 
>     return num_online_nodes() > 1;
> 
> where we'd prefer to use the simpler variant for being easier to
> read / follow.
> 

Thanks for clarification, I will fix it. 

> Jan
> 
> > [1] https://lists.xenproject.org/archives/html/xen-devel/2021-
> 08/msg00772.html
> >
> >