RE: [XEN RFC PATCH 05/40] xen/arm: Fix lowmem_bitsize when arch_get_dma_bitsize return 0

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Julien,

> -----Original Message-----
> From: Julien Grall <julien@xxxxxxx>
> Sent: 2021年8月19日 21:32
> To: Wei Chen <Wei.Chen@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx;
> sstabellini@xxxxxxxxxx; jbeulich@xxxxxxxx
> Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
> Subject: Re: [XEN RFC PATCH 05/40] xen/arm: Fix lowmem_bitsize when
> arch_get_dma_bitsize return 0
> 
> Hi,
> 
> I guess this patch may be dropped after my comment on patch #4. I will
> comment just on the process.
> 

Ok

> On 11/08/2021 11:23, Wei Chen wrote:
> > From: Hongda Deng <Hongda.Deng@xxxxxxx>
> >
> > In previous patch, we make arch_get_dma_bitsize return 0 when
> > dma_bitsize and platform->dma_bitsize are not set. But this
> > will affect lowmem_bitsize in allocate_memory_11 for domain0.
> > Because this function depends lowmem_bitsize to allocate memory
> > below 4GB.
> >
> > In current code, when arch_get_dma_bitsize return 0, lowmem_bitsize
> > will be set to 0. In this case, we will get "No bank has been
> > allocated below 0-bit." message while allocating domain0 memory.
> > And the lowmem will be set to false.
> >
> > This behavior is inconsistent with what allocate_memory_11 done
> > before, and doesn't meet this functions requirements. So we
> > check arch_get_dma_bitsize's return value before set lowmem_bitsize.
> > Avoid setting lowmem_bitsize to 0 by mistake.
> 
> In general, we want to avoid breaking bisection within a series. This
> means that this patch should be before patch #4.
> 
> >
> > Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> > Signed-off-by: Hongda Deng <Hongda.Deng@xxxxxxx>
> > ---
> >   xen/arch/arm/domain_build.c | 11 ++++++++++-
> >   1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index 6c86d52781..cf341f349f 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -265,9 +265,18 @@ static void __init allocate_memory_11(struct domain
> *d,
> >       int i;
> >
> >       bool lowmem = true;
> > -    unsigned int lowmem_bitsize = min(32U, arch_get_dma_bitsize());
> > +    unsigned int lowmem_bitsize = arch_get_dma_bitsize();
> >       unsigned int bits;
> >
> > +    /*
> > +       When dma_bitsize and platform->dma_bitsize are not set,
> > +       arch_get_dma_bitsize will return 0. That means this system
> > +       doesn't need to reserve memory for DMA. But in order to
> > +       meet above requirements, we still need to try to allocate
> > +       memory below 4GB for Dom0.
> > +    */
> 
> The coding style for comments is:
> 
> /*
>   * A
>   * B
>   */
> 

I will fix it.

> > +    lowmem_bitsize = lowmem_bitsize ? min(32U, lowmem_bitsize) : 32U;
> > +
> >       /*
> >        * TODO: Implement memory bank allocation when DOM0 is not direct
> >        * mapped
> >
> 
> Cheers,
> 
> --
> Julien Grall