Re: [PATCH 1/2] efi/libstub: add support for loading the initrd from a device path

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On Sun, 9 Feb 2020 at 07:50, Lukas Wunner <lukas@wunner.de> wrote:
>
> On Thu, Feb 06, 2020 at 02:03:51PM +0000, Ard Biesheuvel wrote:
> > --- a/drivers/firmware/efi/libstub/efistub.h
> > +++ b/drivers/firmware/efi/libstub/efistub.h
> > @@ -566,6 +566,14 @@ union efi_load_file_protocol {
> >       } mixed_mode;
> >  };
> >
> > +struct efi_vendor_dev_path {
> > +     u8              type;
> > +     u8              sub_type;
> > +     u16             length;
> > +     efi_guid_t      vendorguid;
> > +     u8              vendordata[];
> > +} __packed;
>
> In commit 46cd4b75cd0e ("efi: Add device path parser") I introduced a
> struct efi_dev_path with a union for the type-specific data.  Maybe
> you can make use of it instead of introducing a new type?
>

That would make sense, but how do you take the size then? Perhaps it
would be better to have a generic struct definition for the shared
fields, struct definitions for the different types that use the
generic type as the first fields, and a separate union that ties them
altogether for the parser.

>
> > --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> > +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> > @@ -323,3 +323,68 @@ void efi_char16_printk(efi_char16_t *str)
> >       efi_call_proto(efi_table_attr(efi_system_table(), con_out),
> >                      output_string, str);
> >  }
> > +
> > +static const struct {
> > +     struct efi_vendor_dev_path      vendor;
> > +     struct efi_generic_dev_path     end;
> > +} __packed initrd_devpath = {
>
> Nit:  Can we consistently stick to "dev_path" in lieu of "devpath"?
>

Sure.