Re: [PATCH v3 2/3] i386: Add support for loading from android bootimg

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 12.02.2016 20:34, Shea Levy wrote:
> OK. Do you have any thoughts on how best to extract the "load the kernel
> and set the command line to a specific string" functionality out of the
> linux command, then?
> 
At first I wanted to write a short skeleton patch to show what I meant
but once skeleton is done, there is little actual code involved. Please
try attached patch
> On 2016-02-12 14:22, Vladimir 'phcoder' Serbinenko wrote:
>> Separate command is better as it keeps interface tidy and unpoluted,
>> decreasing maintenance cost. Correct me if I'm wrong but it should be
>> clear from context of file is Android image or usual linux one?
>>
>> Le ven. 12 févr. 2016 20:19, Shea Levy <address@hidden> a écrit :
>>
>>> On 2016-02-12 12:50, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>>> > On 08.02.2016 21:47, Shea Levy wrote:
>>> >> ---
>>> >>  grub-core/loader/i386/linux.c | 27 +++++++++++++++++++++------
>>> >>  1 file changed, 21 insertions(+), 6 deletions(-)
>>> >>
>>> >> diff --git a/grub-core/loader/i386/linux.c
>>> >> b/grub-core/loader/i386/linux.c
>>> >> index fddcc46..6ab8d3c 100644
>>> >> --- a/grub-core/loader/i386/linux.c
>>> >> +++ b/grub-core/loader/i386/linux.c
>>> >> @@ -35,6 +35,7 @@
>>> >>  #include <grub/i18n.h>
>>> >>  #include <grub/lib/cmdline.h>
>>> >>  #include <grub/linux.h>
>>> >> +#include <grub/android_bootimg.h>
>>> >>
>>> >>  GRUB_MOD_LICENSE ("GPLv3+");
>>> >>
>>> >> @@ -695,7 +696,13 @@ grub_cmd_linux (grub_command_t cmd
>>> >> __attribute__ ((unused)),
>>> >>        goto fail;
>>> >>      }
>>> >>
>>> >> -  file = grub_file_open (argv[0]);
>>> >> +  char android_cmdline[BOOT_ARGS_SIZE];
>>> >> +  android_cmdline[0] = '';
>>> >> +  if (grub_memcmp (argv[0], "android_bootimg:", sizeof
>>> >> "android_bootimg:" - 1) == 0)
>>> >> +    grub_android_bootimg_load_kernel (argv[0] + sizeof
>>> >> "android_bootimg:" - 1,
>>> >> +                                      &file, android_cmdline);
>>> >> +  else
>>> >> +      file = grub_file_open (argv[0]);
>>> > I hoped more for autodetection. This gets a bit hairy and proper
>>> > separation is better. Sorry for confusion. I think it's simpler with
>>> > commands like
>>> > android_bootimg [--no-cmdline] [--no-initrd] IMAGE [EXTRA_ARGUMENTS]
>>> > by default it will load both IMAGE, with cmdline and initrd. With
>>> > --no-initrd you can use initrd for custom initrd.
>>>
>>> Autodetection would be possible actually, I didn't think of that. If
>>> grub_file_open fails, we can try grub_android_bootimg_load_kernel on the
>>> same file. Would that be preferable or do we still want a separate
>>> command?
>>>
>>> >
>>> >>    if (! file)
>>> >>      goto fail;
>>> >>
>>> >> @@ -1008,12 +1015,20 @@ grub_cmd_linux (grub_command_t cmd
>>> >> __attribute__ ((unused)),
>>> >>    linux_cmdline = grub_zalloc (maximal_cmdline_size + 1);
>>> >>    if (!linux_cmdline)
>>> >>      goto fail;
>>> >> -  grub_memcpy (linux_cmdline, LINUX_IMAGE, sizeof (LINUX_IMAGE));
>>> >> +  grub_size_t cmdline_offset = 0;
>>> >> +  if (android_cmdline[0])
>>> >> +    {
>>> >> +      cmdline_offset = grub_strlen (android_cmdline) + 1;
>>> >> +      grub_memcpy (linux_cmdline, android_cmdline, cmdline_offset -
>>> >> 1);
>>> >> +      linux_cmdline[cmdline_offset - 1] = ' ';
>>> >> +    }
>>> >> +  grub_memcpy (linux_cmdline + cmdline_offset, LINUX_IMAGE, sizeof
>>> >> (LINUX_IMAGE));
>>> >> +  cmdline_offset += sizeof LINUX_IMAGE - 1;
>>> > LINUX_IMAGE must be at the beginning. don't forget brackets around
>>> > sizeof.
>>> >>    grub_create_loader_cmdline (argc, argv,
>>> >> -                          linux_cmdline
>>> >> -                          + sizeof (LINUX_IMAGE) - 1,
>>> >> -                          maximal_cmdline_size
>>> >> -                          - (sizeof (LINUX_IMAGE) - 1));
>>> >> +                              linux_cmdline
>>> >> +                              + cmdline_offset,
>>> >> +                              maximal_cmdline_size
>>> >> +                              - cmdline_offset);
>>> >>
>>> >>    len = prot_file_size;
>>> >>    if (grub_file_read (file, prot_mode_mem, len) != len &&
>>> >> !grub_errno)
>>> >>
>>> >
>>> >
>>> >
>>> > _______________________________________________
>>> > Grub-devel mailing list
>>> > address@hidden
>>> > https://lists.gnu.org/mailman/listinfo/grub-devel [1]
>>>
>>> _______________________________________________
>>> Grub-devel mailing list
>>> address@hidden
>>> https://lists.gnu.org/mailman/listinfo/grub-devel [1]
>>
>>
>> Links:
>> ------
>> [1] https://lists.gnu.org/mailman/listinfo/grub-devel
>>
>> _______________________________________________
>> Grub-devel mailing list
>> address@hidden
>> https://lists.gnu.org/mailman/listinfo/grub-devel
> 
> 
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel

android.diff
Description: Text Data

signature.asc
Description: OpenPGP digital signature