diff mbox series

xen/arm: setup: nr_banks should be unsigned int

Message ID 20220523194631.66262-1-julien@xen.org (mailing list archive)
State New
Headers show
Series xen/arm: setup: nr_banks should be unsigned int | expand

Commit Message

Julien Grall May 23, 2022, 7:46 p.m. UTC
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(-)

Comments

Wei Chen May 24, 2022, 1:07 a.m. UTC | #1
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
>
Stefano Stabellini May 24, 2022, 11:41 p.m. UTC | #2
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 mbox series

Patch

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 )