Re: [PATCH v1 2/2] mkimage: adding sbat data into sbat ELF Note on powerpc

[Daniel Kiper]

On Thu, Jun 06, 2024 at 09:44:10PM +0530, Sudhakar Kuppusamy wrote:
> it reads the SBAT data from sbat.csv and create the ELF Note for it then
> store the SBAT data on it while generate image with -s option

May I ask you to use proper English sentences in the commit messages?

> Signed-off-by: Sudhakar Kuppusamy <sudhakar@linux.ibm.com>
> Co-authored-by: Daniel Axtens <dja@axtens.net>
> ---
>  util/mkimage.c | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/util/mkimage.c b/util/mkimage.c
> index 5b7e977ee..ca1718f4d 100644
> --- a/util/mkimage.c
> +++ b/util/mkimage.c
> @@ -954,8 +954,9 @@ grub_install_generate_image (const char *dir, const char 
> *prefix,
>        total_module_size += dtb_size + sizeof (struct grub_module_header);
>      }
>
> -  if (sbat_path != NULL && image_target->id != IMAGE_EFI)
> -    grub_util_error (_(".sbat section can be embedded into EFI images 
> only"));
> +  if (sbat_path != NULL && (image_target->id != IMAGE_EFI && 
> image_target->id != IMAGE_PPC))
> +    grub_util_error (_(".sbat section can be embedded into EFI images/"
> +                       "sbat ELF Note cab be added into powerpc-ieee1275 
> images only"));

s/cab/can/

>
>    if (disable_shim_lock)
>      total_module_size += sizeof (struct grub_module_header);
> @@ -1828,6 +1829,16 @@ grub_install_generate_image (const char *dir, const 
> char *prefix,
>      case IMAGE_I386_IEEE1275:
>        {
>       grub_uint64_t target_addr;
> +       char *sbat = NULL;
> +
> +     if (sbat_path != NULL)
> +       {
> +         sbat_size = grub_util_get_image_size (sbat_path);
> +            sbat = xmalloc (sbat_size);

Please use tabs and spaces to properly format the code. E.g. one line above is 
correct.

> +         grub_util_load_image (sbat_path, sbat);
> +            layout.sbat_size = sbat_size;

Ditto.

> +       }
> +
>       if (image_target->id == IMAGE_LOONGSON_ELF)
>         {
>           if (comp == GRUB_COMPRESSION_NONE)
> @@ -1839,10 +1850,10 @@ grub_install_generate_image (const char *dir, const 
> char *prefix,
>       else
>         target_addr = image_target->link_addr;
>       if (image_target->voidp_sizeof == 4)
> -       grub_mkimage_generate_elf32 (image_target, note, appsig_size, 
> &core_img, &core_size,
> +       grub_mkimage_generate_elf32 (image_target, note, appsig_size, sbat, 
> &core_img, &core_size,
>                                      target_addr, &layout);
>       else
> -       grub_mkimage_generate_elf64 (image_target, note, appsig_size, 
> &core_img, &core_size,
> +       grub_mkimage_generate_elf64 (image_target, note, appsig_size, sbat, 
> &core_img, &core_size,
>                                      target_addr, &layout);
>        }
>        break;

I have a feeling this hunk should go to the first patch.

Daniel