All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Ard Biesheuvel <ardb@kernel.org>,
	linux-efi <linux-efi@vger.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	Leif Lindholm <leif@nuviainc.com>,
	Peter Jones <pjones@redhat.com>,
	Matthew Garrett <mjg59@google.com>,
	Alexander Graf <agraf@csgraf.de>,
	Ilias Apalodimas <ilias.apalodimas@linaro.org>,
	Heinrich Schuchardt <xypron.glpk@gmx.de>,
	Daniel Kiper <daniel.kiper@oracle.com>
Subject: Re: [PATCH 1/2] efi/libstub: add support for loading the initrd from a device path
Date: Mon, 10 Feb 2020 15:26:02 +0100	[thread overview]
Message-ID: <edddcef9-6ed6-9b16-cce1-8e1c08b781a1@redhat.com> (raw)
In-Reply-To: <CAKv+Gu89o4oJEJvJMuE68HM5NDgokQ0W-D7YQB6xOX2EbhYYBw@mail.gmail.com>

On 02/07/20 13:36, Ard Biesheuvel wrote:
> On Fri, 7 Feb 2020 at 09:48, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 02/06/20 15:03, Ard Biesheuvel wrote:

>>> +efi_status_t efi_load_initrd_devpath(unsigned long *load_addr,
>>> +                                  unsigned long *load_size,
>>> +                                  unsigned long max)
>>> +{
>>> +     efi_guid_t lf2_proto_guid = EFI_LOAD_FILE2_PROTOCOL_GUID;
>>> +     efi_device_path_protocol_t *dp;
>>> +     efi_load_file2_protocol_t *lf2;
>>> +     unsigned long initrd_addr;
>>> +     unsigned long initrd_size;
>>> +     efi_handle_t handle;
>>> +     efi_status_t status;
>>> +
>>> +     if (!load_addr || !load_size)
>>> +             return EFI_INVALID_PARAMETER;
>>> +
>>> +     dp = (efi_device_path_protocol_t *)&initrd_devpath;
>>> +     status = efi_bs_call(locate_device_path, &lf2_proto_guid, &dp, &handle);
>>> +     if (status != EFI_SUCCESS)
>>> +             return status;
>>> +
>>> +     status = efi_bs_call(handle_protocol, handle, &lf2_proto_guid,
>>> +                          (void **)&lf2);
>>> +     if (status != EFI_SUCCESS)
>>> +             return status;
>>> +
>>> +     initrd_size = 0;
>>> +     status = efi_call_proto(lf2, load_file,
>>> +                             (efi_device_path_protocol_t *)&initrd_devpath,
>>> +                             false, &initrd_size, NULL);
>>
>> The second argument to EFI_LOAD_FILE2_PROTOCOL.LoadFile() is "FilePath",
>> specified as "The device specific path of the file to load". This means
>> it is supposed to be a (possibly empty) sequence of FILEPATH_DEVICE_PATH
>> nodes, terminated by and "End Entire Device Path" node. See
>>
>> - 10.3.1 Generic Device Path Structures
>> - 10.3.5.4 File Path Media Device Path
>>
>> in UEFI-2.8.
>>
>> And "initrd_devpath" is not a device path like that; instead it's the
>> VenMedia device path that's installed on the handle that also carries
>> our LoadFile2 instance.
>>
> 
> OK, so you are saying this could be used to disambiguate which of
> several files you may want to load from the initrd GUIDed device path?

Yes, exactly.

>> Now, I do see that this all theoretical here, as we don't expect the
>> LoadFile2 instance that we've found via our special
>> LINUX_EFI_INITRD_MEDIA_GUID VenMedia devpath to do *any* device-specific
>> filename / pathname parsing.
>>
>> But in that case (i.e., given that the FilePath argument is totally
>> irrelevant), I think it's much clearer if we simply pass an empty device
>> path -- one that consists of a single "End Entire Device Path" node.
>>
>> I've checked, and your ArmVirtQemu patch ignores the FilePath argument
>> too -- justifiedly so. I just think it's better to pass in a well-formed
>> device path, rather than NULL. Because, the FilePath parameter is not
>> marked OPTIONAL in the spec.
>>
> 
> One thing that occurred to me is that we have to decide whether we
> want to support the '10.3.5.8 Relative Offset Range' device path node
> for this file, so that you could potentially load subranges of the
> file. I don't see a use case for it right now, though.

Agreed, it doesn't seem necessary / justified.

> But for my understanding, would the FilePath passed to LoadFile2 be
> 'Offset(...)+EndEntire' in that case? Or should it include the GUID
> device path node as well?

I see the only specified, concrete use case for the Offset() devpath
node in "14.4.2.1 PCI Bus Driver Responsibilities". I think it doesn't
apply at all to our use case.

Also, according to "10.3.5.8 Relative Offset Range",

    This device path node specifies a range of offsets relative to the
    first byte available on the device.

In that sense, it seems like a (mutually exclusive) alternative to
FilePath. Given a device, one would specify *either* an offset range
(which is relative to the start of the device, when the device is viewed
as a range of bytes), *or* a FilePath (which is "relative" to the device
when viewed as a store of named files, but still not as a full-blown
random-access filesystem).

In brief, Offset() doesn't seem to apply in connection with LoadFile2,
at all. Certainly not in our particular use case, I'd suggest.

[...]

Thanks!
Laszlo


WARNING: multiple messages have this Message-ID
From: Laszlo Ersek <lersek@redhat.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: linux-efi <linux-efi@vger.kernel.org>,
	Alexander Graf <agraf@csgraf.de>,
	Heinrich Schuchardt <xypron.glpk@gmx.de>,
	Daniel Kiper <daniel.kiper@oracle.com>,
	Ilias Apalodimas <ilias.apalodimas@linaro.org>,
	Matthew Garrett <mjg59@google.com>,
	Peter Jones <pjones@redhat.com>,
	Leif Lindholm <leif@nuviainc.com>,
	Ard Biesheuvel <ardb@kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 1/2] efi/libstub: add support for loading the initrd from a device path
Date: Mon, 10 Feb 2020 15:26:02 +0100	[thread overview]
Message-ID: <edddcef9-6ed6-9b16-cce1-8e1c08b781a1@redhat.com> (raw)
In-Reply-To: <CAKv+Gu89o4oJEJvJMuE68HM5NDgokQ0W-D7YQB6xOX2EbhYYBw@mail.gmail.com>

On 02/07/20 13:36, Ard Biesheuvel wrote:
> On Fri, 7 Feb 2020 at 09:48, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 02/06/20 15:03, Ard Biesheuvel wrote:

>>> +efi_status_t efi_load_initrd_devpath(unsigned long *load_addr,
>>> +                                  unsigned long *load_size,
>>> +                                  unsigned long max)
>>> +{
>>> +     efi_guid_t lf2_proto_guid = EFI_LOAD_FILE2_PROTOCOL_GUID;
>>> +     efi_device_path_protocol_t *dp;
>>> +     efi_load_file2_protocol_t *lf2;
>>> +     unsigned long initrd_addr;
>>> +     unsigned long initrd_size;
>>> +     efi_handle_t handle;
>>> +     efi_status_t status;
>>> +
>>> +     if (!load_addr || !load_size)
>>> +             return EFI_INVALID_PARAMETER;
>>> +
>>> +     dp = (efi_device_path_protocol_t *)&initrd_devpath;
>>> +     status = efi_bs_call(locate_device_path, &lf2_proto_guid, &dp, &handle);
>>> +     if (status != EFI_SUCCESS)
>>> +             return status;
>>> +
>>> +     status = efi_bs_call(handle_protocol, handle, &lf2_proto_guid,
>>> +                          (void **)&lf2);
>>> +     if (status != EFI_SUCCESS)
>>> +             return status;
>>> +
>>> +     initrd_size = 0;
>>> +     status = efi_call_proto(lf2, load_file,
>>> +                             (efi_device_path_protocol_t *)&initrd_devpath,
>>> +                             false, &initrd_size, NULL);
>>
>> The second argument to EFI_LOAD_FILE2_PROTOCOL.LoadFile() is "FilePath",
>> specified as "The device specific path of the file to load". This means
>> it is supposed to be a (possibly empty) sequence of FILEPATH_DEVICE_PATH
>> nodes, terminated by and "End Entire Device Path" node. See
>>
>> - 10.3.1 Generic Device Path Structures
>> - 10.3.5.4 File Path Media Device Path
>>
>> in UEFI-2.8.
>>
>> And "initrd_devpath" is not a device path like that; instead it's the
>> VenMedia device path that's installed on the handle that also carries
>> our LoadFile2 instance.
>>
> 
> OK, so you are saying this could be used to disambiguate which of
> several files you may want to load from the initrd GUIDed device path?

Yes, exactly.

>> Now, I do see that this all theoretical here, as we don't expect the
>> LoadFile2 instance that we've found via our special
>> LINUX_EFI_INITRD_MEDIA_GUID VenMedia devpath to do *any* device-specific
>> filename / pathname parsing.
>>
>> But in that case (i.e., given that the FilePath argument is totally
>> irrelevant), I think it's much clearer if we simply pass an empty device
>> path -- one that consists of a single "End Entire Device Path" node.
>>
>> I've checked, and your ArmVirtQemu patch ignores the FilePath argument
>> too -- justifiedly so. I just think it's better to pass in a well-formed
>> device path, rather than NULL. Because, the FilePath parameter is not
>> marked OPTIONAL in the spec.
>>
> 
> One thing that occurred to me is that we have to decide whether we
> want to support the '10.3.5.8 Relative Offset Range' device path node
> for this file, so that you could potentially load subranges of the
> file. I don't see a use case for it right now, though.

Agreed, it doesn't seem necessary / justified.

> But for my understanding, would the FilePath passed to LoadFile2 be
> 'Offset(...)+EndEntire' in that case? Or should it include the GUID
> device path node as well?

I see the only specified, concrete use case for the Offset() devpath
node in "14.4.2.1 PCI Bus Driver Responsibilities". I think it doesn't
apply at all to our use case.

Also, according to "10.3.5.8 Relative Offset Range",

    This device path node specifies a range of offsets relative to the
    first byte available on the device.

In that sense, it seems like a (mutually exclusive) alternative to
FilePath. Given a device, one would specify *either* an offset range
(which is relative to the start of the device, when the device is viewed
as a range of bytes), *or* a FilePath (which is "relative" to the device
when viewed as a store of named files, but still not as a full-blown
random-access filesystem).

In brief, Offset() doesn't seem to apply in connection with LoadFile2,
at all. Certainly not in our particular use case, I'd suggest.

[...]

Thanks!
Laszlo


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-02-10 14:26 UTC|newest]

Thread overview: 77+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-06 14:03 [PATCH 0/2] arch-agnostic initrd loading method for EFI systems Ard Biesheuvel
2020-02-06 14:03 ` Ard Biesheuvel
2020-02-06 14:03 ` [PATCH 1/2] efi/libstub: add support for loading the initrd from a device path Ard Biesheuvel
2020-02-06 14:03   ` Ard Biesheuvel
2020-02-06 18:26   ` Heinrich Schuchardt
2020-02-06 18:26     ` Heinrich Schuchardt
2020-02-06 18:46     ` Ilias Apalodimas
2020-02-06 18:46       ` Ilias Apalodimas
2020-02-06 19:15       ` Heinrich Schuchardt
2020-02-06 19:15         ` Heinrich Schuchardt
2020-02-06 20:09         ` Ilias Apalodimas
2020-02-06 20:09           ` Ilias Apalodimas
2020-02-06 22:49           ` Heinrich Schuchardt
2020-02-06 22:49             ` Heinrich Schuchardt
2020-02-07  7:35             ` Ilias Apalodimas
2020-02-07  7:35               ` Ilias Apalodimas
2020-02-06 22:35     ` Ard Biesheuvel
2020-02-06 22:35       ` Ard Biesheuvel
2020-02-07  0:01       ` Heinrich Schuchardt
2020-02-07  0:01         ` Heinrich Schuchardt
2020-02-07  0:21         ` Ard Biesheuvel
2020-02-07  0:21           ` Ard Biesheuvel
2020-02-07  0:57           ` Heinrich Schuchardt
2020-02-07  0:57             ` Heinrich Schuchardt
2020-02-07  8:12             ` Ard Biesheuvel
2020-02-07  8:12               ` Ard Biesheuvel
2020-02-07 13:30               ` Heinrich Schuchardt
2020-02-07 13:30                 ` Heinrich Schuchardt
2020-02-07 13:58                 ` Ard Biesheuvel
2020-02-07 13:58                   ` Ard Biesheuvel
2020-02-07 14:18                   ` Alexander Graf
2020-02-07 14:18                     ` Alexander Graf
2020-02-07 15:30                     ` Ard Biesheuvel
2020-02-07 15:30                       ` Ard Biesheuvel
2020-02-07 15:35                     ` Heinrich Schuchardt
2020-02-07 15:35                       ` Heinrich Schuchardt
2020-02-07 11:09       ` Laszlo Ersek
2020-02-07 11:09         ` Laszlo Ersek
2020-02-07 11:03     ` Laszlo Ersek
2020-02-07 11:03       ` Laszlo Ersek
2020-02-07  9:48   ` Laszlo Ersek
2020-02-07  9:48     ` Laszlo Ersek
2020-02-07 12:36     ` Ard Biesheuvel
2020-02-07 12:36       ` Ard Biesheuvel
2020-02-10 14:26       ` Laszlo Ersek [this message]
2020-02-10 14:26         ` Laszlo Ersek
2020-02-09  6:39   ` Lukas Wunner
2020-02-09 11:35     ` Ard Biesheuvel
2020-02-09 11:35       ` Ard Biesheuvel
2020-02-06 14:03 ` [PATCH 2/2] efi/libstub: take noinitrd cmdline argument into account for devpath initrd Ard Biesheuvel
2020-02-06 14:03   ` Ard Biesheuvel
2020-02-06 18:33   ` Heinrich Schuchardt
2020-02-06 18:33     ` Heinrich Schuchardt
2020-02-06 23:44     ` Ard Biesheuvel
2020-02-06 23:44       ` Ard Biesheuvel
2020-02-12 16:01   ` Peter Jones
2020-02-12 16:01     ` Peter Jones
2020-02-07  9:09 ` [PATCH 0/2] arch-agnostic initrd loading method for EFI systems Laszlo Ersek
2020-02-07  9:09   ` Laszlo Ersek
2020-02-07  9:22   ` Laszlo Ersek
2020-02-07  9:22     ` Laszlo Ersek
2020-02-07 12:23     ` Ard Biesheuvel
2020-02-07 12:23       ` Ard Biesheuvel
2020-02-07 16:20       ` James Bottomley
2020-02-07 16:20         ` James Bottomley
2020-02-07 18:31         ` Ard Biesheuvel
2020-02-07 18:31           ` Ard Biesheuvel
2020-02-07 19:54           ` James Bottomley
2020-02-07 19:54             ` James Bottomley
2020-02-07 20:03             ` Ard Biesheuvel
2020-02-07 20:03               ` Ard Biesheuvel
2020-02-07 18:45 ` Arvind Sankar
2020-02-07 18:45   ` Arvind Sankar
2020-02-07 19:47   ` Ard Biesheuvel
2020-02-07 19:47     ` Ard Biesheuvel
2020-02-07 20:26     ` Arvind Sankar
2020-02-07 20:26       ` Arvind Sankar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=edddcef9-6ed6-9b16-cce1-8e1c08b781a1@redhat.com \
    --to=lersek@redhat.com \
    --cc=agraf@csgraf.de \
    --cc=ard.biesheuvel@linaro.org \
    --cc=ardb@kernel.org \
    --cc=daniel.kiper@oracle.com \
    --cc=ilias.apalodimas@linaro.org \
    --cc=leif@nuviainc.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=mjg59@google.com \
    --cc=pjones@redhat.com \
    --cc=xypron.glpk@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.