The current look and feel of setup_mm() leaves a lot to be desired. The
scope of variables is not the best, many variables are not really needed
while some others are set but not used. The first iteration of membanks
is split from the loop for no reason. Tidy up this function for better
readability.
No functional change.
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
---
xen/arch/arm/arm32/mmu/mm.c | 28 +++++++++-------------------
1 file changed, 9 insertions(+), 19 deletions(-)
diff --git a/xen/arch/arm/arm32/mmu/mm.c b/xen/arch/arm/arm32/mmu/mm.c
index e6d9b49acd3c..5e4766ddcf65 100644
--- a/xen/arch/arm/arm32/mmu/mm.c
+++ b/xen/arch/arm/arm32/mmu/mm.c
@@ -74,8 +74,7 @@ static paddr_t __init fit_xenheap_in_static_heap(uint32_t size, paddr_t align)
void __init setup_mm(void)
{
const struct membanks *mem = bootinfo_get_mem();
- paddr_t ram_start, ram_end, ram_size, e, bank_start, bank_end, bank_size;
- paddr_t static_heap_end = 0, static_heap_size = 0;
+ paddr_t ram_start = INVALID_PADDR, ram_end = 0, ram_size = 0, e;
unsigned long heap_pages, xenheap_pages, domheap_pages;
unsigned int i;
const uint32_t ctr = READ_CP32(CTR);
@@ -89,19 +88,14 @@ void __init setup_mm(void)
init_pdx();
- ram_start = mem->bank[0].start;
- ram_size = mem->bank[0].size;
- ram_end = ram_start + ram_size;
-
- for ( i = 1; i < mem->nr_banks; i++ )
+ for ( i = 0; i < mem->nr_banks; i++ )
{
- bank_start = mem->bank[i].start;
- bank_size = mem->bank[i].size;
- bank_end = bank_start + bank_size;
+ const struct membank *bank = &mem->bank[i];
+ paddr_t bank_end = bank->start + bank->size;
- ram_size = ram_size + bank_size;
- ram_start = min(ram_start,bank_start);
- ram_end = max(ram_end,bank_end);
+ ram_size = ram_size + bank->size;
+ ram_start = min(ram_start, bank->start);
+ ram_end = max(ram_end, bank_end);
}
total_pages = ram_size >> PAGE_SHIFT;
@@ -109,18 +103,14 @@ void __init setup_mm(void)
if ( using_static_heap )
{
const struct membanks *reserved_mem = bootinfo_get_reserved_mem();
+ paddr_t static_heap_size = 0;
for ( i = 0 ; i < reserved_mem->nr_banks; i++ )
{
if ( reserved_mem->bank[i].type != MEMBANK_STATIC_HEAP )
continue;
- bank_start = reserved_mem->bank[i].start;
- bank_size = reserved_mem->bank[i].size;
- bank_end = bank_start + bank_size;
-
- static_heap_size += bank_size;
- static_heap_end = max(static_heap_end, bank_end);
+ static_heap_size += reserved_mem->bank[i].size;
}
heap_pages = static_heap_size >> PAGE_SHIFT;
--
2.25.1On Fri, 4 Jul 2025, Michal Orzel wrote:
> The current look and feel of setup_mm() leaves a lot to be desired. The
> scope of variables is not the best, many variables are not really needed
> while some others are set but not used. The first iteration of membanks
> is split from the loop for no reason. Tidy up this function for better
> readability.
>
> No functional change.
>
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
> xen/arch/arm/arm32/mmu/mm.c | 28 +++++++++-------------------
> 1 file changed, 9 insertions(+), 19 deletions(-)
>
> diff --git a/xen/arch/arm/arm32/mmu/mm.c b/xen/arch/arm/arm32/mmu/mm.c
> index e6d9b49acd3c..5e4766ddcf65 100644
> --- a/xen/arch/arm/arm32/mmu/mm.c
> +++ b/xen/arch/arm/arm32/mmu/mm.c
> @@ -74,8 +74,7 @@ static paddr_t __init fit_xenheap_in_static_heap(uint32_t size, paddr_t align)
> void __init setup_mm(void)
> {
> const struct membanks *mem = bootinfo_get_mem();
> - paddr_t ram_start, ram_end, ram_size, e, bank_start, bank_end, bank_size;
> - paddr_t static_heap_end = 0, static_heap_size = 0;
> + paddr_t ram_start = INVALID_PADDR, ram_end = 0, ram_size = 0, e;
> unsigned long heap_pages, xenheap_pages, domheap_pages;
> unsigned int i;
> const uint32_t ctr = READ_CP32(CTR);
> @@ -89,19 +88,14 @@ void __init setup_mm(void)
>
> init_pdx();
>
> - ram_start = mem->bank[0].start;
> - ram_size = mem->bank[0].size;
> - ram_end = ram_start + ram_size;
> -
> - for ( i = 1; i < mem->nr_banks; i++ )
> + for ( i = 0; i < mem->nr_banks; i++ )
> {
> - bank_start = mem->bank[i].start;
> - bank_size = mem->bank[i].size;
> - bank_end = bank_start + bank_size;
> + const struct membank *bank = &mem->bank[i];
> + paddr_t bank_end = bank->start + bank->size;
>
> - ram_size = ram_size + bank_size;
> - ram_start = min(ram_start,bank_start);
> - ram_end = max(ram_end,bank_end);
> + ram_size = ram_size + bank->size;
> + ram_start = min(ram_start, bank->start);
> + ram_end = max(ram_end, bank_end);
> }
>
> total_pages = ram_size >> PAGE_SHIFT;
> @@ -109,18 +103,14 @@ void __init setup_mm(void)
> if ( using_static_heap )
> {
> const struct membanks *reserved_mem = bootinfo_get_reserved_mem();
> + paddr_t static_heap_size = 0;
>
> for ( i = 0 ; i < reserved_mem->nr_banks; i++ )
> {
> if ( reserved_mem->bank[i].type != MEMBANK_STATIC_HEAP )
> continue;
>
> - bank_start = reserved_mem->bank[i].start;
> - bank_size = reserved_mem->bank[i].size;
> - bank_end = bank_start + bank_size;
> -
> - static_heap_size += bank_size;
> - static_heap_end = max(static_heap_end, bank_end);
> + static_heap_size += reserved_mem->bank[i].size;
> }
>
> heap_pages = static_heap_size >> PAGE_SHIFT;
> --
> 2.25.1
>
Hi Michal, > On Fri, Jul 04, 2025 at 11:08:31AM +0000, Michal Orzel wrote: > The current look and feel of setup_mm() leaves a lot to be desired. The > scope of variables is not the best, many variables are not really needed > while some others are set but not used. The first iteration of membanks > is split from the loop for no reason. Tidy up this function for better > readability. > > No functional change. > > Signed-off-by: Michal Orzel <michal.orzel@amd.com> Reviewed-by: Hari Limaye <hari.limaye@arm.com> Tested-by: Hari Limaye <hari.limaye@arm.com> LGTM! Tested (compilation) via Arm AArch32 build. Many thanks, Hari
© 2016 - 2025 Red Hat, Inc.