| Message ID | 20220523194631.66262-1-julien@xen.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/arm: setup: nr_banks should be unsigned int | expand |
Hi Julien, > -----Original Message----- > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of > Julien Grall > Sent: 2022年5月24日 3:47 > To: xen-devel@lists.xenproject.org > Cc: Michal Orzel <Michal.Orzel@arm.com>; Julien Grall <jgrall@amazon.com>; > Stefano Stabellini <sstabellini@kernel.org>; Julien Grall <julien@xen.org>; > Bertrand Marquis <Bertrand.Marquis@arm.com>; Volodymyr Babchuk > <Volodymyr_Babchuk@epam.com> > Subject: [PATCH] xen/arm: setup: nr_banks should be unsigned int > > From: Julien Grall <jgrall@amazon.com> > > It is not possible to have a negative number of banks. So switch > to unsigned int. > > The type change is also propagated to any users of nr_banks that > were using "int" (there are not that many). > > Note that fdt_num_mem_rsv() can actually returns a negative value > in case of an error. So the return should be checked before assigning > the result to an unsigned variable. > > Signed-off-by: Julien Grall <jgrall@amazon.com> > --- > xen/arch/arm/domain_build.c | 9 +++++---- > xen/arch/arm/efi/efi-dom0.c | 4 ++-- > xen/arch/arm/include/asm/setup.h | 6 +++--- > xen/arch/arm/setup.c | 17 +++++++++++++---- > 4 files changed, 23 insertions(+), 13 deletions(-) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index aa777741bdd0..6ecb6673a3cd 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -111,7 +111,8 @@ static bool __init insert_11_bank(struct domain *d, > struct page_info *pg, > unsigned int order) > { > - int res, i; > + unsigned int i; > + int res; > mfn_t smfn; > paddr_t start, size; > > @@ -264,7 +265,7 @@ static void __init allocate_memory_11(struct domain *d, > const unsigned int min_order = get_order_from_bytes(MB(4)); > struct page_info *pg; > unsigned int order = get_allocation_size(kinfo->unassigned_mem); > - int i; > + unsigned int i; > > bool lowmem = true; > unsigned int lowmem_bitsize = min(32U, arch_get_dma_bitsize()); > @@ -1022,8 +1023,8 @@ static int __init make_memory_node(const struct > domain *d, > int addrcells, int sizecells, > struct meminfo *mem) > { > - int res, i; > - int reg_size = addrcells + sizecells; > + unsigned int i; > + int res, reg_size = addrcells + sizecells; > int nr_cells = 0; > /* Placeholder for memory@ + a 64-bit number + \0 */ > char buf[24]; > diff --git a/xen/arch/arm/efi/efi-dom0.c b/xen/arch/arm/efi/efi-dom0.c > index 494420eaa23e..aae0f979112a 100644 > --- a/xen/arch/arm/efi/efi-dom0.c > +++ b/xen/arch/arm/efi/efi-dom0.c > @@ -34,14 +34,14 @@ > /* Constant to indicate "Xen" in unicode u16 format */ > static const CHAR16 xen_efi_fw_vendor[] = {0x0058, 0x0065, 0x006E, > 0x0000}; > > -size_t __init estimate_efi_size(int mem_nr_banks) > +size_t __init estimate_efi_size(unsigned int mem_nr_banks) > { > size_t size; > size_t est_size = sizeof(EFI_SYSTEM_TABLE); > size_t ect_size = sizeof(EFI_CONFIGURATION_TABLE); > size_t emd_size = sizeof(EFI_MEMORY_DESCRIPTOR); > size_t fw_vendor_size = sizeof(xen_efi_fw_vendor); > - int acpi_mem_nr_banks = 0; > + unsigned int acpi_mem_nr_banks = 0; > > if ( !acpi_disabled ) > acpi_mem_nr_banks = bootinfo.acpi.nr_banks; > diff --git a/xen/arch/arm/include/asm/setup.h > b/xen/arch/arm/include/asm/setup.h > index 7a1e1d67989c..2bb01ecfa88f 100644 > --- a/xen/arch/arm/include/asm/setup.h > +++ b/xen/arch/arm/include/asm/setup.h > @@ -30,7 +30,7 @@ struct membank { > }; > > struct meminfo { > - int nr_banks; > + unsigned int nr_banks; > struct membank bank[NR_MEM_BANKS]; > }; > > @@ -93,7 +93,7 @@ extern domid_t max_init_domid; > > void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len); > > -size_t estimate_efi_size(int mem_nr_banks); > +size_t estimate_efi_size(unsigned int mem_nr_banks); > > void acpi_create_efi_system_table(struct domain *d, > struct membank tbl_add[]); > @@ -109,7 +109,7 @@ void create_dom0(void); > > void discard_initial_modules(void); > void fw_unreserved_regions(paddr_t s, paddr_t e, > - void (*cb)(paddr_t, paddr_t), int first); > + void (*cb)(paddr_t, paddr_t), unsigned int > first); > > size_t boot_fdt_info(const void *fdt, paddr_t paddr); > const char *boot_fdt_cmdline(const void *fdt); > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > index db1768c03f03..b30bccbaa7df 100644 > --- a/xen/arch/arm/setup.c > +++ b/xen/arch/arm/setup.c > @@ -201,9 +201,17 @@ static void __init processor_id(void) > > static void __init dt_unreserved_regions(paddr_t s, paddr_t e, > void (*cb)(paddr_t, paddr_t), > - int first) > + unsigned int first) > { > - int i, nr = fdt_num_mem_rsv(device_tree_flattened); > + unsigned int i, nr; > + int rc; > + > + rc = fdt_num_mem_rsv(device_tree_flattened); > + if ( rc < 0 ) > + panic("Unable to retrieve the number of reserved regions > (rc=%d)\n", > + rc); > + > + nr = rc; > > for ( i = first; i < nr ; i++ ) > { > @@ -249,7 +257,8 @@ static void __init dt_unreserved_regions(paddr_t s, > paddr_t e, > } > > void __init fw_unreserved_regions(paddr_t s, paddr_t e, > - void (*cb)(paddr_t, paddr_t), int first) > + void (*cb)(paddr_t, paddr_t), > + unsigned int first) > { > if ( acpi_disabled ) > dt_unreserved_regions(s, e, cb, first); > @@ -693,7 +702,7 @@ static void __init setup_mm(void) > paddr_t ram_start, ram_end, ram_size, e; > unsigned long ram_pages; > unsigned long heap_pages, xenheap_pages, domheap_pages; > - int i; > + unsigned int i; > const uint32_t ctr = READ_CP32(CTR); > > if ( !bootinfo.mem.nr_banks ) Reviewed-by: Wei Chen <Wei.Chen@arm.com> > -- > 2.32.0 >
On Tue, 24 May 2022, Wei Chen wrote: > Hi Julien, > > > -----Original Message----- > > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of > > Julien Grall > > Sent: 2022年5月24日 3:47 > > To: xen-devel@lists.xenproject.org > > Cc: Michal Orzel <Michal.Orzel@arm.com>; Julien Grall <jgrall@amazon.com>; > > Stefano Stabellini <sstabellini@kernel.org>; Julien Grall <julien@xen.org>; > > Bertrand Marquis <Bertrand.Marquis@arm.com>; Volodymyr Babchuk > > <Volodymyr_Babchuk@epam.com> > > Subject: [PATCH] xen/arm: setup: nr_banks should be unsigned int > > > > From: Julien Grall <jgrall@amazon.com> > > > > It is not possible to have a negative number of banks. So switch > > to unsigned int. > > > > The type change is also propagated to any users of nr_banks that > > were using "int" (there are not that many). > > > > Note that fdt_num_mem_rsv() can actually returns a negative value > > in case of an error. So the return should be checked before assigning > > the result to an unsigned variable. > > > > Signed-off-by: Julien Grall <jgrall@amazon.com> > > --- > > xen/arch/arm/domain_build.c | 9 +++++---- > > xen/arch/arm/efi/efi-dom0.c | 4 ++-- > > xen/arch/arm/include/asm/setup.h | 6 +++--- > > xen/arch/arm/setup.c | 17 +++++++++++++---- > > 4 files changed, 23 insertions(+), 13 deletions(-) > > > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > > index aa777741bdd0..6ecb6673a3cd 100644 > > --- a/xen/arch/arm/domain_build.c > > +++ b/xen/arch/arm/domain_build.c > > @@ -111,7 +111,8 @@ static bool __init insert_11_bank(struct domain *d, > > struct page_info *pg, > > unsigned int order) > > { > > - int res, i; > > + unsigned int i; > > + int res; > > mfn_t smfn; > > paddr_t start, size; > > > > @@ -264,7 +265,7 @@ static void __init allocate_memory_11(struct domain *d, > > const unsigned int min_order = get_order_from_bytes(MB(4)); > > struct page_info *pg; > > unsigned int order = get_allocation_size(kinfo->unassigned_mem); > > - int i; > > + unsigned int i; > > > > bool lowmem = true; > > unsigned int lowmem_bitsize = min(32U, arch_get_dma_bitsize()); > > @@ -1022,8 +1023,8 @@ static int __init make_memory_node(const struct > > domain *d, > > int addrcells, int sizecells, > > struct meminfo *mem) > > { > > - int res, i; > > - int reg_size = addrcells + sizecells; > > + unsigned int i; > > + int res, reg_size = addrcells + sizecells; > > int nr_cells = 0; > > /* Placeholder for memory@ + a 64-bit number + \0 */ > > char buf[24]; > > diff --git a/xen/arch/arm/efi/efi-dom0.c b/xen/arch/arm/efi/efi-dom0.c > > index 494420eaa23e..aae0f979112a 100644 > > --- a/xen/arch/arm/efi/efi-dom0.c > > +++ b/xen/arch/arm/efi/efi-dom0.c > > @@ -34,14 +34,14 @@ > > /* Constant to indicate "Xen" in unicode u16 format */ > > static const CHAR16 xen_efi_fw_vendor[] = {0x0058, 0x0065, 0x006E, > > 0x0000}; > > > > -size_t __init estimate_efi_size(int mem_nr_banks) > > +size_t __init estimate_efi_size(unsigned int mem_nr_banks) > > { > > size_t size; > > size_t est_size = sizeof(EFI_SYSTEM_TABLE); > > size_t ect_size = sizeof(EFI_CONFIGURATION_TABLE); > > size_t emd_size = sizeof(EFI_MEMORY_DESCRIPTOR); > > size_t fw_vendor_size = sizeof(xen_efi_fw_vendor); > > - int acpi_mem_nr_banks = 0; > > + unsigned int acpi_mem_nr_banks = 0; > > > > if ( !acpi_disabled ) > > acpi_mem_nr_banks = bootinfo.acpi.nr_banks; > > diff --git a/xen/arch/arm/include/asm/setup.h > > b/xen/arch/arm/include/asm/setup.h > > index 7a1e1d67989c..2bb01ecfa88f 100644 > > --- a/xen/arch/arm/include/asm/setup.h > > +++ b/xen/arch/arm/include/asm/setup.h > > @@ -30,7 +30,7 @@ struct membank { > > }; > > > > struct meminfo { > > - int nr_banks; > > + unsigned int nr_banks; > > struct membank bank[NR_MEM_BANKS]; > > }; > > > > @@ -93,7 +93,7 @@ extern domid_t max_init_domid; > > > > void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len); > > > > -size_t estimate_efi_size(int mem_nr_banks); > > +size_t estimate_efi_size(unsigned int mem_nr_banks); > > > > void acpi_create_efi_system_table(struct domain *d, > > struct membank tbl_add[]); > > @@ -109,7 +109,7 @@ void create_dom0(void); > > > > void discard_initial_modules(void); > > void fw_unreserved_regions(paddr_t s, paddr_t e, > > - void (*cb)(paddr_t, paddr_t), int first); > > + void (*cb)(paddr_t, paddr_t), unsigned int > > first); > > > > size_t boot_fdt_info(const void *fdt, paddr_t paddr); > > const char *boot_fdt_cmdline(const void *fdt); > > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > > index db1768c03f03..b30bccbaa7df 100644 > > --- a/xen/arch/arm/setup.c > > +++ b/xen/arch/arm/setup.c > > @@ -201,9 +201,17 @@ static void __init processor_id(void) > > > > static void __init dt_unreserved_regions(paddr_t s, paddr_t e, > > void (*cb)(paddr_t, paddr_t), > > - int first) > > + unsigned int first) > > { > > - int i, nr = fdt_num_mem_rsv(device_tree_flattened); > > + unsigned int i, nr; > > + int rc; > > + > > + rc = fdt_num_mem_rsv(device_tree_flattened); > > + if ( rc < 0 ) > > + panic("Unable to retrieve the number of reserved regions > > (rc=%d)\n", > > + rc); > > + > > + nr = rc; > > > > for ( i = first; i < nr ; i++ ) > > { > > @@ -249,7 +257,8 @@ static void __init dt_unreserved_regions(paddr_t s, > > paddr_t e, > > } > > > > void __init fw_unreserved_regions(paddr_t s, paddr_t e, > > - void (*cb)(paddr_t, paddr_t), int first) > > + void (*cb)(paddr_t, paddr_t), > > + unsigned int first) > > { > > if ( acpi_disabled ) > > dt_unreserved_regions(s, e, cb, first); > > @@ -693,7 +702,7 @@ static void __init setup_mm(void) > > paddr_t ram_start, ram_end, ram_size, e; > > unsigned long ram_pages; > > unsigned long heap_pages, xenheap_pages, domheap_pages; > > - int i; > > + unsigned int i; > > const uint32_t ctr = READ_CP32(CTR); > > > > if ( !bootinfo.mem.nr_banks ) > > Reviewed-by: Wei Chen <Wei.Chen@arm.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org> and committed
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index aa777741bdd0..6ecb6673a3cd 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -111,7 +111,8 @@ static bool __init insert_11_bank(struct domain *d, struct page_info *pg, unsigned int order) { - int res, i; + unsigned int i; + int res; mfn_t smfn; paddr_t start, size; @@ -264,7 +265,7 @@ static void __init allocate_memory_11(struct domain *d, const unsigned int min_order = get_order_from_bytes(MB(4)); struct page_info *pg; unsigned int order = get_allocation_size(kinfo->unassigned_mem); - int i; + unsigned int i; bool lowmem = true; unsigned int lowmem_bitsize = min(32U, arch_get_dma_bitsize()); @@ -1022,8 +1023,8 @@ static int __init make_memory_node(const struct domain *d, int addrcells, int sizecells, struct meminfo *mem) { - int res, i; - int reg_size = addrcells + sizecells; + unsigned int i; + int res, reg_size = addrcells + sizecells; int nr_cells = 0; /* Placeholder for memory@ + a 64-bit number + \0 */ char buf[24]; diff --git a/xen/arch/arm/efi/efi-dom0.c b/xen/arch/arm/efi/efi-dom0.c index 494420eaa23e..aae0f979112a 100644 --- a/xen/arch/arm/efi/efi-dom0.c +++ b/xen/arch/arm/efi/efi-dom0.c @@ -34,14 +34,14 @@ /* Constant to indicate "Xen" in unicode u16 format */ static const CHAR16 xen_efi_fw_vendor[] = {0x0058, 0x0065, 0x006E, 0x0000}; -size_t __init estimate_efi_size(int mem_nr_banks) +size_t __init estimate_efi_size(unsigned int mem_nr_banks) { size_t size; size_t est_size = sizeof(EFI_SYSTEM_TABLE); size_t ect_size = sizeof(EFI_CONFIGURATION_TABLE); size_t emd_size = sizeof(EFI_MEMORY_DESCRIPTOR); size_t fw_vendor_size = sizeof(xen_efi_fw_vendor); - int acpi_mem_nr_banks = 0; + unsigned int acpi_mem_nr_banks = 0; if ( !acpi_disabled ) acpi_mem_nr_banks = bootinfo.acpi.nr_banks; diff --git a/xen/arch/arm/include/asm/setup.h b/xen/arch/arm/include/asm/setup.h index 7a1e1d67989c..2bb01ecfa88f 100644 --- a/xen/arch/arm/include/asm/setup.h +++ b/xen/arch/arm/include/asm/setup.h @@ -30,7 +30,7 @@ struct membank { }; struct meminfo { - int nr_banks; + unsigned int nr_banks; struct membank bank[NR_MEM_BANKS]; }; @@ -93,7 +93,7 @@ extern domid_t max_init_domid; void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len); -size_t estimate_efi_size(int mem_nr_banks); +size_t estimate_efi_size(unsigned int mem_nr_banks); void acpi_create_efi_system_table(struct domain *d, struct membank tbl_add[]); @@ -109,7 +109,7 @@ void create_dom0(void); void discard_initial_modules(void); void fw_unreserved_regions(paddr_t s, paddr_t e, - void (*cb)(paddr_t, paddr_t), int first); + void (*cb)(paddr_t, paddr_t), unsigned int first); size_t boot_fdt_info(const void *fdt, paddr_t paddr); const char *boot_fdt_cmdline(const void *fdt); diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index db1768c03f03..b30bccbaa7df 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -201,9 +201,17 @@ static void __init processor_id(void) static void __init dt_unreserved_regions(paddr_t s, paddr_t e, void (*cb)(paddr_t, paddr_t), - int first) + unsigned int first) { - int i, nr = fdt_num_mem_rsv(device_tree_flattened); + unsigned int i, nr; + int rc; + + rc = fdt_num_mem_rsv(device_tree_flattened); + if ( rc < 0 ) + panic("Unable to retrieve the number of reserved regions (rc=%d)\n", + rc); + + nr = rc; for ( i = first; i < nr ; i++ ) { @@ -249,7 +257,8 @@ static void __init dt_unreserved_regions(paddr_t s, paddr_t e, } void __init fw_unreserved_regions(paddr_t s, paddr_t e, - void (*cb)(paddr_t, paddr_t), int first) + void (*cb)(paddr_t, paddr_t), + unsigned int first) { if ( acpi_disabled ) dt_unreserved_regions(s, e, cb, first); @@ -693,7 +702,7 @@ static void __init setup_mm(void) paddr_t ram_start, ram_end, ram_size, e; unsigned long ram_pages; unsigned long heap_pages, xenheap_pages, domheap_pages; - int i; + unsigned int i; const uint32_t ctr = READ_CP32(CTR); if ( !bootinfo.mem.nr_banks )