From: "Laszlo Ersek" <lersek@redhat.com>
To: devel@edk2.groups.io, ard.biesheuvel@linaro.org
Cc: leif@nuviainc.com, Liming Gao <liming.gao@intel.com>
Subject: Re: [edk2-devel] [PATCH v4 7/7] OvmfPkg/LinuxInitrdDynamicShellCommand: bail if initrd already exists
Date: Tue, 3 Mar 2020 22:08:33 +0100 [thread overview]
Message-ID: <04d4423d-b64b-9af3-b6e8-5b22731445da@redhat.com> (raw)
In-Reply-To: <3f96cac2-06fe-9c5c-52d1-8da5f7096fb8@redhat.com>
On 03/03/20 22:03, Laszlo Ersek wrote:
> On 03/03/20 15:01, Ard Biesheuvel wrote:
>> Before taking any actions, check if an instance of the LoadFile2 exists
>> already on the Linux initrd media GUID device path, and whether it was
>> provided by this command. If so, abort, since no duplicate instances of
>> the device path should exist.
>>
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2564
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.c | 31 ++++++++++++++++++++
>> OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.uni | 3 ++
>> 2 files changed, 34 insertions(+)
>>
>> diff --git a/OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.c b/OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.c
>> index 47ed26b50d3a..ed8fbaa77069 100644
>> --- a/OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.c
>> +++ b/OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.c
>> @@ -53,6 +53,33 @@ STATIC CONST SINGLE_NODE_VENDOR_MEDIA_DEVPATH mInitrdDevicePath = {
>> }
>> };
>>
>> +STATIC
>> +BOOLEAN
>> +IsOtherInitrdDevicePathAlreadyInstalled (
>> + VOID
>> + )
>> +{
>> + EFI_STATUS Status;
>> + EFI_DEVICE_PATH_PROTOCOL *DevicePath;
>> + EFI_HANDLE Handle;
>> +
>> + DevicePath = (EFI_DEVICE_PATH_PROTOCOL *)&mInitrdDevicePath;
>> + Status = gBS->LocateDevicePath (&gEfiLoadFile2ProtocolGuid, &DevicePath,
>> + &Handle);
>> + if (EFI_ERROR (Status)) {
>> + return FALSE;
>> + }
>> +
>> + //
>> + // Check whether the existing instance is one that we installed during
>> + // a previous invocation.
>> + //
>> + if (Handle == mInitrdLoadFile2Handle) {
>> + return FALSE;
>> + }
>> + return TRUE;
>> +}
>
> Looks good, the function will return TRUE when mInitrdLoadFile2Handle is
> NULL.
To clarify, what I mean is that the controlling expression
(Handle == mInitrdLoadFile2Handle)
assuming it is reached, will evaluate to 0, if mInitrdLoadFile2Handle is
NULL. That's because Handle cannot be NULL at that point. Hence the
function will return TRUE.
Thanks
Laszlo
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>
> Thanks
> Laszlo
>
>
>> +
>> STATIC
>> EFI_STATUS
>> EFIAPI
>> @@ -217,6 +244,10 @@ RunInitrd (
>> } else {
>> ASSERT(FALSE);
>> }
>> + } else if (IsOtherInitrdDevicePathAlreadyInstalled ()) {
>> + ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_ALREADY_INSTALLED),
>> + mLinuxInitrdShellCommandHiiHandle, L"initrd");
>> + ShellStatus = SHELL_UNSUPPORTED;
>> } else {
>> if (ShellCommandLineGetCount (Package) > 2) {
>> ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_TOO_MANY),
>> diff --git a/OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.uni b/OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.uni
>> index a88fa6e3641b..4b6b1285fffd 100644
>> --- a/OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.uni
>> +++ b/OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.uni
>> @@ -18,6 +18,7 @@
>> #langdef en-US "english"
>>
>> #string STR_GEN_PROBLEM #language en-US "%H%s%N: Unknown flag - '%H%s%N'\r\n"
>> +#string STR_GEN_ALREADY_INSTALLED #language en-US "%H%s%N: Linux initrd already provided by platform\r\n"
>> #string STR_GEN_TOO_MANY #language en-US "%H%s%N: Too many arguments.\r\n"
>> #string STR_GEN_TOO_FEW #language en-US "%H%s%N: Too few arguments.\r\n"
>> #string STR_GEN_FIND_FAIL #language en-US "%H%s%N: File not found - '%H%s%N'\r\n"
>> @@ -47,3 +48,5 @@
>> " Consumers of the LoadFile2 protocol on the LINUX_EFI_INITRD_MEDIA_GUID\r\n"
>> " device path that are started via means other than the shell will be able\r\n"
>> " to locate the protocol and invoke it.\r\n"
>> +" 3. Exposing an initrd using this command is only supported if no initrd is\r\n"
>> +" already being exposed by another driver on the platform.\r\n"
>>
>
next prev parent reply other threads:[~2020-03-03 21:08 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-03 14:01 [PATCH v4 0/7] OvmfPkg: implement initrd shell command and mixed mode loader Ard Biesheuvel
2020-03-03 14:01 ` [PATCH v4 1/7] OvmfPkg: add definition of LINUX_EFI_INITRD_MEDIA_GUID Ard Biesheuvel
2020-03-03 14:01 ` [PATCH v4 2/7] OvmfPkg: add 'initrd' shell command to expose Linux initrd via device path Ard Biesheuvel
2020-03-03 14:01 ` [PATCH v4 3/7] ArmVirtPkg: add the 'initrd' dynamic shell command Ard Biesheuvel
2020-03-03 14:01 ` [PATCH v4 4/7] OvmfPkg: " Ard Biesheuvel
2020-03-03 14:01 ` [PATCH v4 5/7] MdeModulePkg/DxeCore: defer PE/COFF emulator registration to StartImage Ard Biesheuvel
2020-03-03 14:01 ` [PATCH v4 6/7] OvmfPkg IA32: add support for loading X64 images Ard Biesheuvel
2020-03-03 20:54 ` [edk2-devel] " Laszlo Ersek
2020-03-03 21:53 ` Ard Biesheuvel
2020-03-03 14:01 ` [PATCH v4 7/7] OvmfPkg/LinuxInitrdDynamicShellCommand: bail if initrd already exists Ard Biesheuvel
2020-03-03 21:03 ` [edk2-devel] " Laszlo Ersek
2020-03-03 21:08 ` Laszlo Ersek [this message]
2020-03-03 21:53 ` Ard Biesheuvel
2020-03-04 9:29 ` [PATCH v4 0/7] OvmfPkg: implement initrd shell command and mixed mode loader Ard Biesheuvel
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-list 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=04d4423d-b64b-9af3-b6e8-5b22731445da@redhat.com \
--to=devel@edk2.groups.io \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox