| Message ID | 20210811102423.28908-6-wei.chen@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add device tree based NUMA support to Arm64 | expand |
Hi, I guess this patch may be dropped after my comment on patch #4. I will comment just on the process. On 11/08/2021 11:23, Wei Chen wrote: > From: Hongda Deng <Hongda.Deng@arm.com> > > 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@arm.com> > Signed-off-by: Hongda Deng <Hongda.Deng@arm.com> > --- > 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 */ > + lowmem_bitsize = lowmem_bitsize ? min(32U, lowmem_bitsize) : 32U; > + > /* > * TODO: Implement memory bank allocation when DOM0 is not direct > * mapped > Cheers,
Hi Julien, > -----Original Message----- > From: Julien Grall <julien@xen.org> > Sent: 2021年8月19日 21:32 > To: Wei Chen <Wei.Chen@arm.com>; xen-devel@lists.xenproject.org; > sstabellini@kernel.org; jbeulich@suse.com > Cc: Bertrand Marquis <Bertrand.Marquis@arm.com> > 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@arm.com> > > > > 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@arm.com> > > Signed-off-by: Hongda Deng <Hongda.Deng@arm.com> > > --- > > 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
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. + */ + lowmem_bitsize = lowmem_bitsize ? min(32U, lowmem_bitsize) : 32U; + /* * TODO: Implement memory bank allocation when DOM0 is not direct * mapped