[PATCH 1/3] lmb: avoid superfluous value check in lmb_map_update_notify()

[Ilias Apalodimas]

On Sun, 16 Feb 2025 at 13:12, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
>
> Instead of testing the value of parameter op at runtime use an enum to
> ensure that only valid values are used.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> ---
>  include/lmb.h | 12 ++++++++++++
>  lib/lmb.c     | 23 +++++++----------------
>  2 files changed, 19 insertions(+), 16 deletions(-)
>
> diff --git a/include/lmb.h b/include/lmb.h
> index d9d7435a431..09297a4f530 100644
> --- a/include/lmb.h
> +++ b/include/lmb.h
> @@ -31,6 +31,18 @@
>  #define LMB_NOOVERWRITE BIT(1)
>  #define LMB_NONOTIFY BIT(2)
>
> +/**
> + * enum lmb_map_op - memory map operation
> + */
> +enum lmb_map_op {
> +       /** @LMB_MAP_OP_RESERVE:        reserve memory */
> +       LMB_MAP_OP_RESERVE = 1,
> +       /** @LMB_MAP_OP_FREE:           free memory */
> +       LMB_MAP_OP_FREE,
> +       /** @LMB_MAP_OP_ADD:            add memory */
> +       LMB_MAP_OP_ADD,
> +};
> +
>  /**
>   * struct lmb_region - Description of one region
>   * @base: Base address of the region
> diff --git a/lib/lmb.c b/lib/lmb.c
> index 7ca44591e1d..7534a231c99 100644
> --- a/lib/lmb.c
> +++ b/lib/lmb.c
> @@ -23,10 +23,6 @@
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> -#define MAP_OP_RESERVE         (u8)0x1
> -#define MAP_OP_FREE            (u8)0x2
> -#define MAP_OP_ADD             (u8)0x3
> -
>  /*
>   * The following low level LMB functions must not access the global LMB memory
>   * map since they are also used to manage IOVA memory maps in iommu drivers like
> @@ -436,18 +432,13 @@ static bool lmb_should_notify(u32 flags)
>                 CONFIG_IS_ENABLED(EFI_LOADER);
>  }
>
> -static int lmb_map_update_notify(phys_addr_t addr, phys_size_t size, u8 op,
> -                                u32 flags)
> +static int lmb_map_update_notify(phys_addr_t addr, phys_size_t size,
> +                                enum lmb_map_op op, u32 flags)
>  {
>         u64 efi_addr;
>         u64 pages;
>         efi_status_t status;
>
> -       if (op != MAP_OP_RESERVE && op != MAP_OP_FREE && op != MAP_OP_ADD) {
> -               log_err("Invalid map update op received (%d)\n", op);
> -               return -1;
> -       }
> -
>         if (!lmb_should_notify(flags))
>                 return 0;
>
> @@ -456,7 +447,7 @@ static int lmb_map_update_notify(phys_addr_t addr, phys_size_t size, u8 op,
>         efi_addr &= ~EFI_PAGE_MASK;
>
>         status = efi_add_memory_map_pg(efi_addr, pages,
> -                                      op == MAP_OP_RESERVE ?
> +                                      op == LMB_MAP_OP_RESERVE ?
>                                        EFI_BOOT_SERVICES_DATA :
>                                        EFI_CONVENTIONAL_MEMORY,
>                                        false);
> @@ -642,7 +633,7 @@ long lmb_add(phys_addr_t base, phys_size_t size)
>         if (ret)
>                 return ret;
>
> -       return lmb_map_update_notify(base, size, MAP_OP_ADD, LMB_NONE);
> +       return lmb_map_update_notify(base, size, LMB_MAP_OP_ADD, LMB_NONE);
>  }
>
>  long lmb_free_flags(phys_addr_t base, phys_size_t size,
> @@ -654,7 +645,7 @@ long lmb_free_flags(phys_addr_t base, phys_size_t size,
>         if (ret < 0)
>                 return ret;
>
> -       return lmb_map_update_notify(base, size, MAP_OP_FREE, flags);
> +       return lmb_map_update_notify(base, size, LMB_MAP_OP_FREE, flags);
>  }
>
>  long lmb_free(phys_addr_t base, phys_size_t size)
> @@ -671,7 +662,7 @@ long lmb_reserve(phys_addr_t base, phys_size_t size, u32 flags)
>         if (ret)
>                 return ret;
>
> -       return lmb_map_update_notify(base, size, MAP_OP_RESERVE, flags);
> +       return lmb_map_update_notify(base, size, LMB_MAP_OP_RESERVE, flags);
>  }
>
>  static phys_addr_t _lmb_alloc_base(phys_size_t size, ulong align,
> @@ -712,7 +703,7 @@ static phys_addr_t _lmb_alloc_base(phys_size_t size, ulong align,
>                                         return 0;
>
>                                 ret = lmb_map_update_notify(base, size,
> -                                                           MAP_OP_RESERVE,
> +                                                           LMB_MAP_OP_RESERVE,
>                                                             flags);
>                                 if (ret)
>                                         return ret;
> --
> 2.47.1
>

Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>