[edk2-devel] [PATCH 00/11] OvmfPkg: tweak shell builds

[edk2-devel] [PATCH 00/11] OvmfPkg: tweak shell builds

[Gerd Hoffmann]

- Create include files to reduce duplication.
- Fix varpolicy command.
- Little CI tweak.

Gerd Hoffmann (11):
  OvmfPkg: add ShellComponents.dsc.inc
  OvmfPkg: add ShellLibs.dsc.inc
  OvmfPkg: add ShellDxe.fdf.inc
  OvmfPkg: Shell*.inc: allow building without network support
  OvmfPkg: ShellDxe.fdf.inc: add VariablePolicyDynamicCommand to FV
  OvmfPkg: switch OvmfPkgIa32 to new shell include files
  OvmfPkg: switch OvmfPkgIa32X64 to new shell include files
  OvmfPkg: switch AmdSevX64 to new shell include files
  OvmfPkg: switch IntelTdxX64 to new shell include files
  OvmfPkg: switch MicrovmX64 to new shell include files
  OvmfPkg/CI: copy shell to virtual drive

 OvmfPkg/Include/Dsc/ShellComponents.dsc.inc | 53 ++++++++++++++++++++
 OvmfPkg/Include/Dsc/ShellLibs.dsc.inc       | 11 +++++
 OvmfPkg/AmdSev/AmdSevX64.dsc                | 32 +-----------
 OvmfPkg/IntelTdx/IntelTdxX64.dsc            | 33 ++-----------
 OvmfPkg/Microvm/MicrovmX64.dsc              | 55 +++++----------------
 OvmfPkg/OvmfPkgIa32.dsc                     | 49 +-----------------
 OvmfPkg/OvmfPkgIa32X64.dsc                  | 54 +++-----------------
 OvmfPkg/OvmfPkgX64.dsc                      | 54 +++-----------------
 OvmfPkg/AmdSev/AmdSevX64.fdf                |  8 +--
 OvmfPkg/IntelTdx/IntelTdxX64.fdf            |  9 +---
 OvmfPkg/Microvm/MicrovmX64.fdf              |  9 +---
 OvmfPkg/OvmfPkgIa32.fdf                     | 11 +----
 OvmfPkg/OvmfPkgIa32X64.fdf                  | 11 +----
 OvmfPkg/OvmfPkgX64.fdf                      | 11 +----
 OvmfPkg/Include/Fdf/ShellDxe.fdf.inc        | 17 +++++++
 OvmfPkg/PlatformCI/PlatformBuildLib.py      | 12 ++++-
 16 files changed, 135 insertions(+), 294 deletions(-)
 create mode 100644 OvmfPkg/Include/Dsc/ShellComponents.dsc.inc
 create mode 100644 OvmfPkg/Include/Dsc/ShellLibs.dsc.inc
 create mode 100644 OvmfPkg/Include/Fdf/ShellDxe.fdf.inc

-- 
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114320): https://edk2.groups.io/g/devel/message/114320
Mute This Topic: https://groups.io/mt/103935343/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 01/11] OvmfPkg: add ShellComponents.dsc.inc

[Gerd Hoffmann]

Move EFI Shell components from OvmfPkgX64.dsc to
the new ShellComponents.dsc.inc include file.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/Include/Dsc/ShellComponents.dsc.inc | 49 +++++++++++++++++++++
 OvmfPkg/OvmfPkgX64.dsc                      | 43 +-----------------
 2 files changed, 50 insertions(+), 42 deletions(-)
 create mode 100644 OvmfPkg/Include/Dsc/ShellComponents.dsc.inc

diff --git a/OvmfPkg/Include/Dsc/ShellComponents.dsc.inc b/OvmfPkg/Include/Dsc/ShellComponents.dsc.inc
new file mode 100644
index 000000000000..1a3a349a9de5
--- /dev/null
+++ b/OvmfPkg/Include/Dsc/ShellComponents.dsc.inc
@@ -0,0 +1,49 @@
+##
+#    SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+!if $(BUILD_SHELL) == TRUE
+
+!if $(TOOL_CHAIN_TAG) != "XCODE5"
+  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf {
+    <PcdsFixedAtBuild>
+      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
+  }
+  ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf {
+    <PcdsFixedAtBuild>
+      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
+  }
+  ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicyDynamicCommand.inf {
+    <PcdsFixedAtBuild>
+      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
+  }
+  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf {
+    <PcdsFixedAtBuild>
+      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
+  }
+!endif
+
+  ShellPkg/Application/Shell/Shell.inf {
+    <LibraryClasses>
+      ShellCommandLib|ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.inf
+      NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
+      NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
+      NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
+      NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
+      NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
+      NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
+      NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
+!if $(NETWORK_IP6_ENABLE) == TRUE
+      NULL|ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.inf
+!endif
+      HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
+      PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
+      BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.inf
+
+    <PcdsFixedAtBuild>
+      gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xFF
+      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
+      gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
+  }
+
+!endif
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index bf4c7906c460..043b0a7a67e0 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -956,48 +956,7 @@ [Components]
   MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf
   MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
 
-!if $(TOOL_CHAIN_TAG) != "XCODE5" && $(BUILD_SHELL) == TRUE
-  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf {
-    <PcdsFixedAtBuild>
-      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
-  }
-  ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf {
-    <PcdsFixedAtBuild>
-      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
-  }
-  ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicyDynamicCommand.inf {
-    <PcdsFixedAtBuild>
-      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
-  }
-  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf {
-    <PcdsFixedAtBuild>
-      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
-  }
-!endif
-!if $(BUILD_SHELL) == TRUE
-  ShellPkg/Application/Shell/Shell.inf {
-    <LibraryClasses>
-      ShellCommandLib|ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.inf
-      NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
-!if $(NETWORK_IP6_ENABLE) == TRUE
-      NULL|ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.inf
-!endif
-      HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
-      PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
-      BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.inf
-
-    <PcdsFixedAtBuild>
-      gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xFF
-      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
-      gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
-  }
-!endif
+!include OvmfPkg/Include/Dsc/ShellComponents.dsc.inc
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
   SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
-- 
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114323): https://edk2.groups.io/g/devel/message/114323
Mute This Topic: https://groups.io/mt/103935346/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 01/11] OvmfPkg: add ShellComponents.dsc.inc

[Laszlo Ersek]

On 1/24/24 17:37, Gerd Hoffmann wrote:
> Move EFI Shell components from OvmfPkgX64.dsc to
> the new ShellComponents.dsc.inc include file.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/Include/Dsc/ShellComponents.dsc.inc | 49 +++++++++++++++++++++
>  OvmfPkg/OvmfPkgX64.dsc                      | 43 +-----------------
>  2 files changed, 50 insertions(+), 42 deletions(-)
>  create mode 100644 OvmfPkg/Include/Dsc/ShellComponents.dsc.inc

"git show --color --color-moved=zebra" works wonders on this patch.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114391): https://edk2.groups.io/g/devel/message/114391
Mute This Topic: https://groups.io/mt/103935346/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 02/11] OvmfPkg: add ShellLibs.dsc.inc

[Gerd Hoffmann]

Move EFI Shell libraries from OvmfPkgX64.dsc to
the new ShellComponents.dsc.inc include file.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/Include/Dsc/ShellLibs.dsc.inc | 11 +++++++++++
 OvmfPkg/OvmfPkgX64.dsc                | 11 +++++------
 2 files changed, 16 insertions(+), 6 deletions(-)
 create mode 100644 OvmfPkg/Include/Dsc/ShellLibs.dsc.inc

diff --git a/OvmfPkg/Include/Dsc/ShellLibs.dsc.inc b/OvmfPkg/Include/Dsc/ShellLibs.dsc.inc
new file mode 100644
index 000000000000..2e0bac74a261
--- /dev/null
+++ b/OvmfPkg/Include/Dsc/ShellLibs.dsc.inc
@@ -0,0 +1,11 @@
+##
+#    SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+!if $(BUILD_SHELL) == TRUE
+
+[LibraryClasses]
+  ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
+  ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf
+
+!endif
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 043b0a7a67e0..eae025bd0163 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -258,16 +258,12 @@ [LibraryClasses]
   TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
 !endif
 
-!if $(BUILD_SHELL) == TRUE
-  ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
-!endif
-  ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf
-
   S3BootScriptLib|MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
   SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
   OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
 
 !include OvmfPkg/Include/Dsc/OvmfTpmLibs.dsc.inc
+!include OvmfPkg/Include/Dsc/ShellLibs.dsc.inc
 
 [LibraryClasses.common]
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
@@ -960,7 +956,10 @@ [Components]
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
   SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
-  OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf
+  OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf {
+    <LibraryClasses>
+    ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf
+  }
 !endif
 
   OvmfPkg/PlatformDxe/Platform.inf
-- 
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114321): https://edk2.groups.io/g/devel/message/114321
Mute This Topic: https://groups.io/mt/103935344/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 02/11] OvmfPkg: add ShellLibs.dsc.inc

[Laszlo Ersek]

I have several comments on this one:

On 1/24/24 17:37, Gerd Hoffmann wrote:
> Move EFI Shell libraries from OvmfPkgX64.dsc to
> the new ShellComponents.dsc.inc include file.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/Include/Dsc/ShellLibs.dsc.inc | 11 +++++++++++
>  OvmfPkg/OvmfPkgX64.dsc                | 11 +++++------
>  2 files changed, 16 insertions(+), 6 deletions(-)
>  create mode 100644 OvmfPkg/Include/Dsc/ShellLibs.dsc.inc
>
> diff --git a/OvmfPkg/Include/Dsc/ShellLibs.dsc.inc b/OvmfPkg/Include/Dsc/ShellLibs.dsc.inc
> new file mode 100644
> index 000000000000..2e0bac74a261
> --- /dev/null
> +++ b/OvmfPkg/Include/Dsc/ShellLibs.dsc.inc
> @@ -0,0 +1,11 @@
> +##
> +#    SPDX-License-Identifier: BSD-2-Clause-Patent
> +##
> +
> +!if $(BUILD_SHELL) == TRUE
> +
> +[LibraryClasses]
> +  ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
> +  ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf
> +
> +!endif

(1) First, in general, I don't agree with including section headers such
as [LibraryClasses] in include files. As far as I can remember, my
opinion has always been that the include files should not attempt to
specify context; the context spec (i.e., the section header, and the
placement of the !include directive) is up to the top-level DSC file
itself.

(

The idea being that, if an include file provides its own section header,
then the !include directive *changes* the context for the subsequent
parts of the including DSC file. That can be very annoying / surprising,
and I'd only really want to accept it *if* we established a hard rule
that *any* !include directive in any DSC or FDF etc file should
unconditionally be followed by an explicit section header -- even if
that section header were the same as the one just before the !include
directive. Like:

[LibraryClasses]
  FooLib|FooPkg/Library/BaseFooLib/FooLib.inf

!include OtherLibs.inc

[LibraryClasses]
  BarLib|BarPkg/Library/BaseBarLib/BarLib.inf

)

My preferred style is observed for example in:

- NetworkPkg/NetworkLibs.dsc.inc
- RedfishPkg/RedfishLibs.dsc.inc

Alas, it is not observed in:

- MdePkg/MdeLibs.dsc.inc
- OvmfPkg/Include/Dsc/OvmfTpmLibs.dsc.inc

At least, from the last two, OvmfTpmLibs.dsc.inc has an excuse: it
provides library class resolutions for different module types.

So here I'd kind of prefer if we *didn't* include the [LibraryClasses]
header. There's no reason to.


(2) This change makes the ShellCEntryLib resolution dependent on
BUILD_SHELL, which is a functional change, and it is not justified,
AFAICT.

(That's why you have to compensate for it in a module scope lib class
resolution, under "EnrollDefaultKeys.inf".)

There are three "levels" of applications in edk2:
- bare bones
- shell
- libc


(2.1) "Bare bones" is for example
"MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.inf".

Any module qualifies that has (a) MODULE_TYPE=UEFI_APPLICATION and (b)
ENTRY_POINT=FunctionName and (c) UefiApplicationEntryPoint listed under
[LibraryClasses].

These applications have entry point functions like

  EFI_STATUS
  EFIAPI
  FunctionName (
    IN EFI_HANDLE        ImageHandle,
    IN EFI_SYSTEM_TABLE  *SystemTable
    )

If they want to parse their "command line parameters", they have to open
EFI_LOADED_IMAGE_PROTOCOL on their ImageHandle, and parse
LoadOptionsSize and LoadOptions manually.


(2.2) shell app is for example
"OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf".

Any module qualifies that has (a) MODULE_TYPE=UEFI_APPLICATION, (b)
ENTRY_POINT=ShellCEntryLib, and (c) ShellCEntryLib listed under
[LibraryClasses].

These apps are launched like this:

- the shell loads the image with gBS->LoadImage()

- the shell installs an EFI_SHELL_PARAMETERS_PROTOCOL instance on the
resultant image handle. This protocol interface contains broken-out Argv
and Argc members, prepared by the shell, from the command line that
invokes the app.

- the shell launches the image with gBS->StartImage()

- the image is entered in the ShellCEntryLib() function (from the sole
"ShellCEntryLib" library instance), per ENTRY_POINT define:

  EFI_STATUS
  EFIAPI
  ShellCEntryLib (
    IN EFI_HANDLE        ImageHandle,
    IN EFI_SYSTEM_TABLE  *SystemTable
    )

- ShellCEntryLib() opens EFI_SHELL_PARAMETERS_PROTOCOL on the image
handle, and calls ShellAppMain():

  INTN
  EFIAPI
  ShellAppMain (
    IN UINTN   Argc,
    IN CHAR16  **Argv
    );

with

    ReturnFromMain = ShellAppMain (
                       EfiShellParametersProtocol->Argc,
                       EfiShellParametersProtocol->Argv
                       );

- That is where the actual application code starts running. The actual
application source includes "ShellPkg/Include/Library/ShellCEntryLib.h"
for declaring ShellAppMain().

If such an application is launched outside of the shell, then the
EFI_SHELL_PARAMETERS_PROTOCOL lookup on the image handle fails in
ShellCEntryLib(), and ShellAppMain() can not be called.

The point is that the "ShellPkg/Application/Shell/Shell.inf" binary
itself is of type (2.1), not type (2.2). The shell itself cannot depend
on EFI_SHELL_PARAMETERS_PROTOCOL.

Our BUILD_SHELL macro controls whether we build the shell itself, but
that is independent of whether we build applications that require the
shell to launch them. Making the ShellCEntryLib class resolution
dependent on BUILD_SHELL would only be valid if we also built all the
shell applications dependent on the BUILD_SHELL macro -- which is not
the case.


(2.3) "libc apps", for completeness: these live at
<https://github.com/tianocore/edk2-libc.git>; for example
"AppPkg/Applications/Main/Main.inf".

Any module qualifies that has MODULE_TYPE=UEFI_APPLICATION,
ENTRY_POINT=ShellCEntryLib, and lists LibC under [LibraryClasses].

The sole LibC instance (StdLib/LibC/Main/Main.c) provides a
ShellAppMain() function, for ShellCEntryLib() to call, that mangles Argc
and Argv so they can be passed to the actual application entry point
function main().


> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 043b0a7a67e0..eae025bd0163 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -258,16 +258,12 @@ [LibraryClasses]
>    TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
>  !endif
>
> -!if $(BUILD_SHELL) == TRUE
> -  ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
> -!endif
> -  ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf
> -
>    S3BootScriptLib|MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
>    SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
>    OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
>
>  !include OvmfPkg/Include/Dsc/OvmfTpmLibs.dsc.inc
> +!include OvmfPkg/Include/Dsc/ShellLibs.dsc.inc
>
>  [LibraryClasses.common]
>    BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> @@ -960,7 +956,10 @@ [Components]
>
>  !if $(SECURE_BOOT_ENABLE) == TRUE
>    SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
> -  OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf
> +  OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf {
> +    <LibraryClasses>
> +    ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf
> +  }
>  !endif
>
>    OvmfPkg/PlatformDxe/Platform.inf

(3) Side comment (because this part should go away anyway): the
"ShellCEntryLib|..." line should be indented by two more space
characters.

Summary:

- please consider dropping the [LibraryClasses] header from the include
file

- the ShellCEntryLib class should be resolved regardless of BUILD_SHELL
(and so that resolution may not even belong in the ShellLibs.dsc.inc
file?)

- EnrollDefaultKeys.inf needs no module-scope lib class resolution

Thanks,
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114402): https://edk2.groups.io/g/devel/message/114402
Mute This Topic: https://groups.io/mt/103935344/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 02/11] OvmfPkg: add ShellLibs.dsc.inc

[Gerd Hoffmann]

  Hi,

> My preferred style is observed for example in:
> 
> - NetworkPkg/NetworkLibs.dsc.inc
> - RedfishPkg/RedfishLibs.dsc.inc
> 
> Alas, it is not observed in:
> 
> - MdePkg/MdeLibs.dsc.inc
> - OvmfPkg/Include/Dsc/OvmfTpmLibs.dsc.inc
> 
> At least, from the last two, OvmfTpmLibs.dsc.inc has an excuse: it
> provides library class resolutions for different module types.

Which is why I did it that way for TPM.  A single include file is enough
instead of having multiple include files for multiple module types.  The
OvmfPkg/Include/Dsc/OvmfTpmLibs.dsc.inc is placed in a location where
this works without problems, and I've placed the new shell include at
the same location.

I also have a WIP patch series (need to undust and rebase it ...) doing
the same for the crypto stuff, and that has the same problem the TPM
include has:  We have different configurations for PEI (stripped down
config, with only the hash functions for measurements) and DXE (full
TLS support).  And it is a single file with multiple sections too ...

tl;dr: I prefer the with-sections style.

> (2) This change makes the ShellCEntryLib resolution dependent on
> BUILD_SHELL, which is a functional change, and it is not justified,
> AFAICT.

> (2.1) "Bare bones" is for example
> "MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.inf".
> [ ... ]

> (2.2) shell app is for example
> "OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf".
> [ ... ]

Thanks for all this background info, much appreciated.

> Our BUILD_SHELL macro controls whether we build the shell itself, but
> that is independent of whether we build applications that require the
> shell to launch them. Making the ShellCEntryLib class resolution
> dependent on BUILD_SHELL would only be valid if we also built all the
> shell applications dependent on the BUILD_SHELL macro -- which is not
> the case.

Given that EnrollDefaultKeys depends on the shell to launch
I'm wondering whenever we should just change that and make the
EnrollDefaultKeys build depend on BUILD_SHELL (and also move it
into Shell*.inc) ?

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114613): https://edk2.groups.io/g/devel/message/114613
Mute This Topic: https://groups.io/mt/103935344/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 02/11] OvmfPkg: add ShellLibs.dsc.inc

[Laszlo Ersek]

On 1/26/24 14:59, Gerd Hoffmann wrote:
>   Hi,
> 
>> My preferred style is observed for example in:
>>
>> - NetworkPkg/NetworkLibs.dsc.inc
>> - RedfishPkg/RedfishLibs.dsc.inc
>>
>> Alas, it is not observed in:
>>
>> - MdePkg/MdeLibs.dsc.inc
>> - OvmfPkg/Include/Dsc/OvmfTpmLibs.dsc.inc
>>
>> At least, from the last two, OvmfTpmLibs.dsc.inc has an excuse: it
>> provides library class resolutions for different module types.
> 
> Which is why I did it that way for TPM.  A single include file is enough
> instead of having multiple include files for multiple module types.  The
> OvmfPkg/Include/Dsc/OvmfTpmLibs.dsc.inc is placed in a location where
> this works without problems, and I've placed the new shell include at
> the same location.
> 
> I also have a WIP patch series (need to undust and rebase it ...) doing
> the same for the crypto stuff, and that has the same problem the TPM
> include has:  We have different configurations for PEI (stripped down
> config, with only the hash functions for measurements) and DXE (full
> TLS support).  And it is a single file with multiple sections too ...
> 
> tl;dr: I prefer the with-sections style.

OK!

> 
>> (2) This change makes the ShellCEntryLib resolution dependent on
>> BUILD_SHELL, which is a functional change, and it is not justified,
>> AFAICT.
> 
>> (2.1) "Bare bones" is for example
>> "MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.inf".
>> [ ... ]
> 
>> (2.2) shell app is for example
>> "OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf".
>> [ ... ]
> 
> Thanks for all this background info, much appreciated.
> 
>> Our BUILD_SHELL macro controls whether we build the shell itself, but
>> that is independent of whether we build applications that require the
>> shell to launch them. Making the ShellCEntryLib class resolution
>> dependent on BUILD_SHELL would only be valid if we also built all the
>> shell applications dependent on the BUILD_SHELL macro -- which is not
>> the case.
> 
> Given that EnrollDefaultKeys depends on the shell to launch
> I'm wondering whenever we should just change that and make the
> EnrollDefaultKeys build depend on BUILD_SHELL

This certainly sounds justified. It's hard to imagine a use case where
someone wanted an EnrollDefaultKeys.efi binary, but not a fresh shell.efi.

> (and also move it into Shell*.inc) ?

Seems to make sense, technically speaking; it's again the naming that
annoys me a bit. :( It's a utility that's supposed to be run from under
the shell, but not related to the shell itself. Hmm. :/

I guess we can live with it.

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114685): https://edk2.groups.io/g/devel/message/114685
Mute This Topic: https://groups.io/mt/103935344/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 02/11] OvmfPkg: add ShellLibs.dsc.inc

[Gerd Hoffmann]

> > Given that EnrollDefaultKeys depends on the shell to launch
> > I'm wondering whenever we should just change that and make the
> > EnrollDefaultKeys build depend on BUILD_SHELL
> 
> This certainly sounds justified. It's hard to imagine a use case where
> someone wanted an EnrollDefaultKeys.efi binary, but not a fresh shell.efi.
> 
> > (and also move it into Shell*.inc) ?
> 
> Seems to make sense, technically speaking; it's again the naming that
> annoys me a bit. :( It's a utility that's supposed to be run from under
> the shell, but not related to the shell itself. Hmm. :/
> 
> I guess we can live with it.

We can certainly try to find a better name.
How about Applications*.inc?

In that case it'll probably make sense to also move UiApp.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114697): https://edk2.groups.io/g/devel/message/114697
Mute This Topic: https://groups.io/mt/103935344/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 02/11] OvmfPkg: add ShellLibs.dsc.inc

[Laszlo Ersek]

On 1/29/24 13:25, Gerd Hoffmann wrote:
>>> Given that EnrollDefaultKeys depends on the shell to launch
>>> I'm wondering whenever we should just change that and make the
>>> EnrollDefaultKeys build depend on BUILD_SHELL
>>
>> This certainly sounds justified. It's hard to imagine a use case where
>> someone wanted an EnrollDefaultKeys.efi binary, but not a fresh shell.efi.
>>
>>> (and also move it into Shell*.inc) ?
>>
>> Seems to make sense, technically speaking; it's again the naming that
>> annoys me a bit. :( It's a utility that's supposed to be run from under
>> the shell, but not related to the shell itself. Hmm. :/
>>
>> I guess we can live with it.
> 
> We can certainly try to find a better name.
> How about Applications*.inc?
> 
> In that case it'll probably make sense to also move UiApp.

Moving UiApp seems counter-productive. UiApp needs to be included in the
firmware image. Its FILE_GUID (from the INF file) is referenced by
PcdBootManagerMenuFile. The same does not apply to
EnrollDefaultKeys.efi, which we don't build into the firmware image.

It's like UiApp and the shell might go in one kind of include file
(set), and EnrollDefaultKeys should be separate.

But... what's the original purpose here? Including the shell in a bunch
of DSC and FDF files is a chore, because the DSC snippet is large (and
so it can easily become inconsistent between DSCs). That's what we want
to solve, right?

Meaning we could leave EnrollDefaultKeys alone, for now (regardless of
whether we decide to make it dependent on BUILD_SHELL as well,
separately). It means means we wouldn't extract / centralize as much
code as technically possible perhaps, but at least we don't increase
semantic confusion.

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114699): https://edk2.groups.io/g/devel/message/114699
Mute This Topic: https://groups.io/mt/103935344/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 03/11] OvmfPkg: add ShellDxe.fdf.inc

[Gerd Hoffmann]

Move EFI Shell firmware volume files to
the new ShellDxe.fdf.inc file.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/OvmfPkgX64.fdf               | 11 ++---------
 OvmfPkg/Include/Fdf/ShellDxe.fdf.inc | 14 ++++++++++++++
 2 files changed, 16 insertions(+), 9 deletions(-)
 create mode 100644 OvmfPkg/Include/Fdf/ShellDxe.fdf.inc

diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index f47ab1727e4c..eb3fb90cb8b6 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -319,15 +319,6 @@ [FV.DXEFV]
 INF  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
 INF  OvmfPkg/VirtioFsDxe/VirtioFsDxe.inf
 
-!if $(BUILD_SHELL) == TRUE && $(TOOL_CHAIN_TAG) != "XCODE5"
-INF  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
-INF  ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
-INF  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf
-!endif
-!if $(BUILD_SHELL) == TRUE
-INF  ShellPkg/Application/Shell/Shell.inf
-!endif
-
 INF MdeModulePkg/Logo/LogoDxe.inf
 
 INF OvmfPkg/TdxDxe/TdxDxe.inf
@@ -402,6 +393,8 @@ [FV.DXEFV]
 #
 !include OvmfPkg/Include/Fdf/OvmfTpmDxe.fdf.inc
 
+!include OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
+
 ################################################################################
 
 [FV.FVMAIN_COMPACT]
diff --git a/OvmfPkg/Include/Fdf/ShellDxe.fdf.inc b/OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
new file mode 100644
index 000000000000..0935f06fa368
--- /dev/null
+++ b/OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
@@ -0,0 +1,14 @@
+##
+#    SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+!if $(BUILD_SHELL) == TRUE
+
+!if $(TOOL_CHAIN_TAG) != "XCODE5"
+INF  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
+INF  ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
+INF  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf
+!endif
+
+INF  ShellPkg/Application/Shell/Shell.inf
+!endif
-- 
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114319): https://edk2.groups.io/g/devel/message/114319
Mute This Topic: https://groups.io/mt/103935342/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 03/11] OvmfPkg: add ShellDxe.fdf.inc

[Laszlo Ersek]

On 1/24/24 17:37, Gerd Hoffmann wrote:
> Move EFI Shell firmware volume files to
> the new ShellDxe.fdf.inc file.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/OvmfPkgX64.fdf               | 11 ++---------
>  OvmfPkg/Include/Fdf/ShellDxe.fdf.inc | 14 ++++++++++++++
>  2 files changed, 16 insertions(+), 9 deletions(-)
>  create mode 100644 OvmfPkg/Include/Fdf/ShellDxe.fdf.inc

(1) I think that the filename "ShellDxe.fdf.inc" (also seen in the
subject) is a misnomer. The affected binaries are not all DXE drivers,
the shell itself is a UEFI application.

The naming probably alludes to these modules being included in
[FV.DXEFV]. Therefore "ShellDxeFv.fdf.inc" would be more correct.

(And indeed that applies to existent files too: OvmfTpmDxe should be
OvmfTpmDxeFv, OvmfTpmPei should be OvmfTpmPeiFv.)

For consistency, adding the Fv part to just this new file name, is not
good, of course. So if you don't feel like renaming the existing FDF
include files, then ignore this point.

> 
> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
> index f47ab1727e4c..eb3fb90cb8b6 100644
> --- a/OvmfPkg/OvmfPkgX64.fdf
> +++ b/OvmfPkg/OvmfPkgX64.fdf
> @@ -319,15 +319,6 @@ [FV.DXEFV]
>  INF  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
>  INF  OvmfPkg/VirtioFsDxe/VirtioFsDxe.inf
>  
> -!if $(BUILD_SHELL) == TRUE && $(TOOL_CHAIN_TAG) != "XCODE5"
> -INF  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
> -INF  ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
> -INF  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf
> -!endif
> -!if $(BUILD_SHELL) == TRUE
> -INF  ShellPkg/Application/Shell/Shell.inf
> -!endif
> -
>  INF MdeModulePkg/Logo/LogoDxe.inf
>  
>  INF OvmfPkg/TdxDxe/TdxDxe.inf
> @@ -402,6 +393,8 @@ [FV.DXEFV]
>  #
>  !include OvmfPkg/Include/Fdf/OvmfTpmDxe.fdf.inc
>  
> +!include OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
> +
>  ################################################################################
>  
>  [FV.FVMAIN_COMPACT]
> diff --git a/OvmfPkg/Include/Fdf/ShellDxe.fdf.inc b/OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
> new file mode 100644
> index 000000000000..0935f06fa368
> --- /dev/null
> +++ b/OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
> @@ -0,0 +1,14 @@
> +##
> +#    SPDX-License-Identifier: BSD-2-Clause-Patent
> +##
> +
> +!if $(BUILD_SHELL) == TRUE
> +
> +!if $(TOOL_CHAIN_TAG) != "XCODE5"
> +INF  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
> +INF  ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
> +INF  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf
> +!endif
> +
> +INF  ShellPkg/Application/Shell/Shell.inf
> +!endif

... and this indeed shows, in comparison to patch#1, that
VariablePolicyDynamicCommand is only built, but not included in the
firmware volume / flash device.

With (1) fixed or not:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114403): https://edk2.groups.io/g/devel/message/114403
Mute This Topic: https://groups.io/mt/103935342/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 04/11] OvmfPkg: Shell*.inc: allow building without network support

[Gerd Hoffmann]

Add NETWORK_ENABLE conditionals for the components
which need network support.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/Include/Dsc/ShellComponents.dsc.inc | 4 ++++
 OvmfPkg/Include/Fdf/ShellDxe.fdf.inc        | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/OvmfPkg/Include/Dsc/ShellComponents.dsc.inc b/OvmfPkg/Include/Dsc/ShellComponents.dsc.inc
index 1a3a349a9de5..248b4b454b70 100644
--- a/OvmfPkg/Include/Dsc/ShellComponents.dsc.inc
+++ b/OvmfPkg/Include/Dsc/ShellComponents.dsc.inc
@@ -5,6 +5,7 @@
 !if $(BUILD_SHELL) == TRUE
 
 !if $(TOOL_CHAIN_TAG) != "XCODE5"
+!if $(NETWORK_ENABLE) == TRUE
   ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf {
     <PcdsFixedAtBuild>
       gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
@@ -13,6 +14,7 @@
     <PcdsFixedAtBuild>
       gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
   }
+!endif
   ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicyDynamicCommand.inf {
     <PcdsFixedAtBuild>
       gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
@@ -32,7 +34,9 @@
       NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
       NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
       NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
+!if $(NETWORK_ENABLE) == TRUE
       NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
+!endif
 !if $(NETWORK_IP6_ENABLE) == TRUE
       NULL|ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.inf
 !endif
diff --git a/OvmfPkg/Include/Fdf/ShellDxe.fdf.inc b/OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
index 0935f06fa368..6536c30c5413 100644
--- a/OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
+++ b/OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
@@ -5,8 +5,10 @@
 !if $(BUILD_SHELL) == TRUE
 
 !if $(TOOL_CHAIN_TAG) != "XCODE5"
+!if $(NETWORK_ENABLE) == TRUE
 INF  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
 INF  ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
+!endif
 INF  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf
 !endif
 
-- 
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114322): https://edk2.groups.io/g/devel/message/114322
Mute This Topic: https://groups.io/mt/103935345/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 04/11] OvmfPkg: Shell*.inc: allow building without network support

[Laszlo Ersek]

On 1/24/24 17:37, Gerd Hoffmann wrote:
> Add NETWORK_ENABLE conditionals for the components
> which need network support.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/Include/Dsc/ShellComponents.dsc.inc | 4 ++++
>  OvmfPkg/Include/Fdf/ShellDxe.fdf.inc        | 2 ++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/OvmfPkg/Include/Dsc/ShellComponents.dsc.inc b/OvmfPkg/Include/Dsc/ShellComponents.dsc.inc
> index 1a3a349a9de5..248b4b454b70 100644
> --- a/OvmfPkg/Include/Dsc/ShellComponents.dsc.inc
> +++ b/OvmfPkg/Include/Dsc/ShellComponents.dsc.inc
> @@ -5,6 +5,7 @@
>  !if $(BUILD_SHELL) == TRUE
>  
>  !if $(TOOL_CHAIN_TAG) != "XCODE5"
> +!if $(NETWORK_ENABLE) == TRUE
>    ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf {
>      <PcdsFixedAtBuild>
>        gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> @@ -13,6 +14,7 @@
>      <PcdsFixedAtBuild>
>        gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
>    }
> +!endif
>    ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicyDynamicCommand.inf {
>      <PcdsFixedAtBuild>
>        gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE

(1) This will prevent testing those code paths in these utilities that deal with *not finding* the underlying networking protocols. That's an argument against this hunk.

However, we already have something similar in place, for UefiShellNetwork2CommandsLib and NETWORK_IP6_ENABLE. So, for consistency's sake, this hunk does seem justified. OK.

(2) Further verification:

- if NETWORK_ENABLE is false, then none of the networking stack is built (see NetworkPkg/NetworkComponents.dsc.inc), and then we don't need these utilities.

- if NETWORK_ENABLE is true, then *at least one* of the IPv4 and IPv6 stacks will be available (see NetworkPkg/NetworkDefines.dsc.inc), and then these utilities should be functional.

OK.

> @@ -32,7 +34,9 @@
>        NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
>        NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
>        NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
> +!if $(NETWORK_ENABLE) == TRUE
>        NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
> +!endif
>  !if $(NETWORK_IP6_ENABLE) == TRUE
>        NULL|ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.inf
>  !endif

(3) This looks incorrect. The Network1 command set is related to IPv4, so we should check NETWORK_IP4_ENABLE.

More precisely, the conditionals should look like

!if $(NETWORK_ENABLE) == TRUE
!if $(NETWORK_IP4_ENABLE) == TRUE
      NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
!endif
!if $(NETWORK_IP6_ENABLE) == TRUE
      NULL|ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.inf
!endif
!endif

(Meaning that the extant NETWORK_IP6_ENABLE check isn't fully correct either.)


> diff --git a/OvmfPkg/Include/Fdf/ShellDxe.fdf.inc b/OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
> index 0935f06fa368..6536c30c5413 100644
> --- a/OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
> +++ b/OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
> @@ -5,8 +5,10 @@
>  !if $(BUILD_SHELL) == TRUE
>  
>  !if $(TOOL_CHAIN_TAG) != "XCODE5"
> +!if $(NETWORK_ENABLE) == TRUE
>  INF  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
>  INF  ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
> +!endif
>  INF  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf
>  !endif
>  

Seems OK.

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114404): https://edk2.groups.io/g/devel/message/114404
Mute This Topic: https://groups.io/mt/103935345/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 05/11] OvmfPkg: ShellDxe.fdf.inc: add VariablePolicyDynamicCommand to FV

[Gerd Hoffmann]

Needed to make the new 'varpolicy' EFI shell command
actually available in the OVMF firmware builds.

Fixes: fe6cd1c18721 ("OvmfPkg: Add varpolicy shell command")
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/Include/Fdf/ShellDxe.fdf.inc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/OvmfPkg/Include/Fdf/ShellDxe.fdf.inc b/OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
index 6536c30c5413..3081ac41780a 100644
--- a/OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
+++ b/OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
@@ -9,6 +9,7 @@
 INF  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
 INF  ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
 !endif
+INF  ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicyDynamicCommand.inf
 INF  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf
 !endif
 
-- 
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114324): https://edk2.groups.io/g/devel/message/114324
Mute This Topic: https://groups.io/mt/103935347/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 05/11] OvmfPkg: ShellDxe.fdf.inc: add VariablePolicyDynamicCommand to FV

[Laszlo Ersek]

On 1/24/24 17:37, Gerd Hoffmann wrote:
> Needed to make the new 'varpolicy' EFI shell command
> actually available in the OVMF firmware builds.
> 
> Fixes: fe6cd1c18721 ("OvmfPkg: Add varpolicy shell command")
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/Include/Fdf/ShellDxe.fdf.inc | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/OvmfPkg/Include/Fdf/ShellDxe.fdf.inc b/OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
> index 6536c30c5413..3081ac41780a 100644
> --- a/OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
> +++ b/OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
> @@ -9,6 +9,7 @@
>  INF  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
>  INF  ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
>  !endif
> +INF  ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicyDynamicCommand.inf
>  INF  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf
>  !endif
>  

Reviewed-by: Laszlo Ersek <lersek@redhat.com>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114431): https://edk2.groups.io/g/devel/message/114431
Mute This Topic: https://groups.io/mt/103935347/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 06/11] OvmfPkg: switch OvmfPkgIa32 to new shell include files

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/OvmfPkgIa32.dsc | 49 ++---------------------------------------
 OvmfPkg/OvmfPkgIa32.fdf | 11 ++-------
 2 files changed, 4 insertions(+), 56 deletions(-)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 28379961a78e..23f949fd9845 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -237,16 +237,12 @@ [LibraryClasses]
   TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
 !endif
 
-!if $(BUILD_SHELL) == TRUE
-  ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
-!endif
-  ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf
-
   S3BootScriptLib|MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
   SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
   OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
 
 !include OvmfPkg/Include/Dsc/OvmfTpmLibs.dsc.inc
+!include OvmfPkg/Include/Dsc/ShellLibs.dsc.inc
 
 [LibraryClasses.common]
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
@@ -874,48 +870,7 @@ [Components]
   MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf
   MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
 
-!if $(TOOL_CHAIN_TAG) != "XCODE5" && $(BUILD_SHELL) == TRUE
-  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf {
-    <PcdsFixedAtBuild>
-      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
-  }
-  ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf {
-    <PcdsFixedAtBuild>
-      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
-  }
-  ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicyDynamicCommand.inf {
-    <PcdsFixedAtBuild>
-      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
-  }
-  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf {
-    <PcdsFixedAtBuild>
-      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
-  }
-!endif
-!if $(BUILD_SHELL) == TRUE
-  ShellPkg/Application/Shell/Shell.inf {
-    <LibraryClasses>
-      ShellCommandLib|ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.inf
-      NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
-!if $(NETWORK_IP6_ENABLE) == TRUE
-      NULL|ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.inf
-!endif
-      HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
-      PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
-      BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.inf
-
-    <PcdsFixedAtBuild>
-      gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xFF
-      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
-      gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
-  }
-!endif
+!include OvmfPkg/Include/Dsc/ShellComponents.dsc.inc
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
   SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
index 501b4de4695e..6c56c5e53f21 100644
--- a/OvmfPkg/OvmfPkgIa32.fdf
+++ b/OvmfPkg/OvmfPkgIa32.fdf
@@ -293,15 +293,6 @@ [FV.DXEFV]
 INF  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
 INF  OvmfPkg/VirtioFsDxe/VirtioFsDxe.inf
 
-!if $(BUILD_SHELL) == TRUE && $(TOOL_CHAIN_TAG) != "XCODE5"
-INF  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
-INF  ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
-INF  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf
-!endif
-!if $(BUILD_SHELL) == TRUE
-INF  ShellPkg/Application/Shell/Shell.inf
-!endif
-
 INF MdeModulePkg/Logo/LogoDxe.inf
 
 #
@@ -361,6 +352,8 @@ [FV.DXEFV]
 #
 !include OvmfPkg/Include/Fdf/OvmfTpmDxe.fdf.inc
 
+!include OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
+
 !if $(LOAD_X64_ON_IA32_ENABLE) == TRUE
 INF  OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf
 !endif
-- 
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114326): https://edk2.groups.io/g/devel/message/114326
Mute This Topic: https://groups.io/mt/103935349/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 06/11] OvmfPkg: switch OvmfPkgIa32 to new shell include files

[Laszlo Ersek]

On 1/24/24 17:37, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/OvmfPkgIa32.dsc | 49 ++---------------------------------------
>  OvmfPkg/OvmfPkgIa32.fdf | 11 ++-------
>  2 files changed, 4 insertions(+), 56 deletions(-)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 28379961a78e..23f949fd9845 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -237,16 +237,12 @@ [LibraryClasses]
>    TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
>  !endif
>  
> -!if $(BUILD_SHELL) == TRUE
> -  ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
> -!endif
> -  ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf
> -
>    S3BootScriptLib|MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
>    SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
>    OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
>  
>  !include OvmfPkg/Include/Dsc/OvmfTpmLibs.dsc.inc
> +!include OvmfPkg/Include/Dsc/ShellLibs.dsc.inc
>  
>  [LibraryClasses.common]
>    BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> @@ -874,48 +870,7 @@ [Components]
>    MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf
>    MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
>  
> -!if $(TOOL_CHAIN_TAG) != "XCODE5" && $(BUILD_SHELL) == TRUE
> -  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf {
> -    <PcdsFixedAtBuild>
> -      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> -  }
> -  ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf {
> -    <PcdsFixedAtBuild>
> -      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> -  }
> -  ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicyDynamicCommand.inf {
> -    <PcdsFixedAtBuild>
> -      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> -  }
> -  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf {
> -    <PcdsFixedAtBuild>
> -      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> -  }
> -!endif
> -!if $(BUILD_SHELL) == TRUE
> -  ShellPkg/Application/Shell/Shell.inf {
> -    <LibraryClasses>
> -      ShellCommandLib|ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.inf
> -      NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
> -!if $(NETWORK_IP6_ENABLE) == TRUE
> -      NULL|ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.inf
> -!endif
> -      HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
> -      PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
> -      BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.inf
> -
> -    <PcdsFixedAtBuild>
> -      gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xFF
> -      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> -      gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
> -  }
> -!endif
> +!include OvmfPkg/Include/Dsc/ShellComponents.dsc.inc
>  
>  !if $(SECURE_BOOT_ENABLE) == TRUE
>    SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
> diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
> index 501b4de4695e..6c56c5e53f21 100644
> --- a/OvmfPkg/OvmfPkgIa32.fdf
> +++ b/OvmfPkg/OvmfPkgIa32.fdf
> @@ -293,15 +293,6 @@ [FV.DXEFV]
>  INF  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
>  INF  OvmfPkg/VirtioFsDxe/VirtioFsDxe.inf
>  
> -!if $(BUILD_SHELL) == TRUE && $(TOOL_CHAIN_TAG) != "XCODE5"
> -INF  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
> -INF  ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
> -INF  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf
> -!endif
> -!if $(BUILD_SHELL) == TRUE
> -INF  ShellPkg/Application/Shell/Shell.inf
> -!endif
> -
>  INF MdeModulePkg/Logo/LogoDxe.inf
>  
>  #
> @@ -361,6 +352,8 @@ [FV.DXEFV]
>  #
>  !include OvmfPkg/Include/Fdf/OvmfTpmDxe.fdf.inc
>  
> +!include OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
> +
>  !if $(LOAD_X64_ON_IA32_ENABLE) == TRUE
>  INF  OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf
>  !endif

Note that this patch is inconsistent with patch#2 (OvmfPkg: add
ShellLibs.dsc.inc); in that patch, "OvmfPkgX64.dsc" gets a module-scope
ShellCEntryLib class resolution for "EnrollDefaultKeys.inf", but here,
"OvmfPkgIa32.dsc".

*However*, that inconsistency is actually good (for this patch), because
-- as I argue under patch#2 -- EnrollDefaultKeys.inf should be left alone.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114432): https://edk2.groups.io/g/devel/message/114432
Mute This Topic: https://groups.io/mt/103935349/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 07/11] OvmfPkg: switch OvmfPkgIa32X64 to new shell include files

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/OvmfPkgIa32X64.dsc | 54 +++++---------------------------------
 OvmfPkg/OvmfPkgIa32X64.fdf | 11 ++------
 2 files changed, 8 insertions(+), 57 deletions(-)

diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 5e9eee628aea..4d57026bb1ed 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -242,16 +242,12 @@ [LibraryClasses]
   TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
 !endif
 
-!if $(BUILD_SHELL) == TRUE
-  ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
-!endif
-  ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf
-
   S3BootScriptLib|MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
   SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
   OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
 
 !include OvmfPkg/Include/Dsc/OvmfTpmLibs.dsc.inc
+!include OvmfPkg/Include/Dsc/ShellLibs.dsc.inc
 
 [LibraryClasses.common]
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
@@ -888,52 +884,14 @@ [Components.X64]
   MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf
   MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
 
-!if $(TOOL_CHAIN_TAG) != "XCODE5" && $(BUILD_SHELL) == TRUE
-  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf {
-    <PcdsFixedAtBuild>
-      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
-  }
-  ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf {
-    <PcdsFixedAtBuild>
-      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
-  }
-  ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicyDynamicCommand.inf {
-    <PcdsFixedAtBuild>
-      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
-  }
-  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf {
-    <PcdsFixedAtBuild>
-      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
-  }
-!endif
-!if $(BUILD_SHELL) == TRUE
-  ShellPkg/Application/Shell/Shell.inf {
-    <LibraryClasses>
-      ShellCommandLib|ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.inf
-      NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
-!if $(NETWORK_IP6_ENABLE) == TRUE
-      NULL|ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.inf
-!endif
-      HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
-      PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
-      BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.inf
-
-    <PcdsFixedAtBuild>
-      gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xFF
-      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
-      gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
-  }
-!endif
+!include OvmfPkg/Include/Dsc/ShellComponents.dsc.inc
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
   SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
-  OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf
+  OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf {
+    <LibraryClasses>
+    ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf
+  }
 !endif
 
   OvmfPkg/PlatformDxe/Platform.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
index 74cfb58f06cd..ee8068ad55dc 100644
--- a/OvmfPkg/OvmfPkgIa32X64.fdf
+++ b/OvmfPkg/OvmfPkgIa32X64.fdf
@@ -294,15 +294,6 @@ [FV.DXEFV]
 INF  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
 INF  OvmfPkg/VirtioFsDxe/VirtioFsDxe.inf
 
-!if $(BUILD_SHELL) == TRUE && $(TOOL_CHAIN_TAG) != "XCODE5"
-INF  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
-INF  ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
-INF  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf
-!endif
-!if $(BUILD_SHELL) == TRUE
-INF  ShellPkg/Application/Shell/Shell.inf
-!endif
-
 INF MdeModulePkg/Logo/LogoDxe.inf
 
 #
@@ -368,6 +359,8 @@ [FV.DXEFV]
 #
 !include OvmfPkg/Include/Fdf/OvmfTpmDxe.fdf.inc
 
+!include OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
+
 ################################################################################
 
 [FV.FVMAIN_COMPACT]
-- 
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114325): https://edk2.groups.io/g/devel/message/114325
Mute This Topic: https://groups.io/mt/103935348/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 07/11] OvmfPkg: switch OvmfPkgIa32X64 to new shell include files

[Laszlo Ersek]

On 1/24/24 17:37, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/OvmfPkgIa32X64.dsc | 54 +++++---------------------------------
>  OvmfPkg/OvmfPkgIa32X64.fdf | 11 ++------
>  2 files changed, 8 insertions(+), 57 deletions(-)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 5e9eee628aea..4d57026bb1ed 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -242,16 +242,12 @@ [LibraryClasses]
>    TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
>  !endif
>  
> -!if $(BUILD_SHELL) == TRUE
> -  ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
> -!endif
> -  ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf
> -
>    S3BootScriptLib|MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
>    SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
>    OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
>  
>  !include OvmfPkg/Include/Dsc/OvmfTpmLibs.dsc.inc
> +!include OvmfPkg/Include/Dsc/ShellLibs.dsc.inc
>  
>  [LibraryClasses.common]
>    BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> @@ -888,52 +884,14 @@ [Components.X64]
>    MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf
>    MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
>  
> -!if $(TOOL_CHAIN_TAG) != "XCODE5" && $(BUILD_SHELL) == TRUE
> -  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf {
> -    <PcdsFixedAtBuild>
> -      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> -  }
> -  ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf {
> -    <PcdsFixedAtBuild>
> -      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> -  }
> -  ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicyDynamicCommand.inf {
> -    <PcdsFixedAtBuild>
> -      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> -  }
> -  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf {
> -    <PcdsFixedAtBuild>
> -      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> -  }
> -!endif
> -!if $(BUILD_SHELL) == TRUE
> -  ShellPkg/Application/Shell/Shell.inf {
> -    <LibraryClasses>
> -      ShellCommandLib|ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.inf
> -      NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
> -!if $(NETWORK_IP6_ENABLE) == TRUE
> -      NULL|ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.inf
> -!endif
> -      HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
> -      PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
> -      BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.inf
> -
> -    <PcdsFixedAtBuild>
> -      gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xFF
> -      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> -      gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
> -  }
> -!endif
> +!include OvmfPkg/Include/Dsc/ShellComponents.dsc.inc
>  
>  !if $(SECURE_BOOT_ENABLE) == TRUE
>    SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
> -  OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf
> +  OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf {
> +    <LibraryClasses>
> +    ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf
> +  }
>  !endif
>  
>    OvmfPkg/PlatformDxe/Platform.inf

(1) OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf should not be changed.

With that undone:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>


> diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
> index 74cfb58f06cd..ee8068ad55dc 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.fdf
> +++ b/OvmfPkg/OvmfPkgIa32X64.fdf
> @@ -294,15 +294,6 @@ [FV.DXEFV]
>  INF  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
>  INF  OvmfPkg/VirtioFsDxe/VirtioFsDxe.inf
>  
> -!if $(BUILD_SHELL) == TRUE && $(TOOL_CHAIN_TAG) != "XCODE5"
> -INF  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
> -INF  ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
> -INF  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf
> -!endif
> -!if $(BUILD_SHELL) == TRUE
> -INF  ShellPkg/Application/Shell/Shell.inf
> -!endif
> -
>  INF MdeModulePkg/Logo/LogoDxe.inf
>  
>  #
> @@ -368,6 +359,8 @@ [FV.DXEFV]
>  #
>  !include OvmfPkg/Include/Fdf/OvmfTpmDxe.fdf.inc
>  
> +!include OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
> +
>  ################################################################################
>  
>  [FV.FVMAIN_COMPACT]



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114433): https://edk2.groups.io/g/devel/message/114433
Mute This Topic: https://groups.io/mt/103935348/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 08/11] OvmfPkg: switch AmdSevX64 to new shell include files

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/AmdSev/AmdSevX64.dsc | 32 ++------------------------------
 OvmfPkg/AmdSev/AmdSevX64.fdf |  8 ++------
 2 files changed, 4 insertions(+), 36 deletions(-)

diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index a31a89344a60..41c61467253f 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -194,16 +194,12 @@ [LibraryClasses]
   VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
   VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
 
-!if $(BUILD_SHELL) == TRUE
-  ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
-  ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf
-!endif
-
   SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
   OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
   S3BootScriptLib|MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
 
 !include OvmfPkg/Include/Dsc/OvmfTpmLibs.dsc.inc
+!include OvmfPkg/Include/Dsc/ShellLibs.dsc.inc
 
 [LibraryClasses.common]
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
@@ -729,34 +725,10 @@ [Components]
   MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf
   MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
 
-!if $(TOOL_CHAIN_TAG) != "XCODE5" && $(BUILD_SHELL) == TRUE
-  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf {
-    <PcdsFixedAtBuild>
-      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
-  }
-!endif
   OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf
   OvmfPkg/AmdSev/Grub/Grub.inf
-!if $(BUILD_SHELL) == TRUE
-  ShellPkg/Application/Shell/Shell.inf {
-    <LibraryClasses>
-      ShellCommandLib|ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.inf
-      NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
-      HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
-      PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
-      BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.inf
 
-    <PcdsFixedAtBuild>
-      gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xFF
-      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
-      gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
-  }
-!endif
+!include OvmfPkg/Include/Dsc/ShellComponents.dsc.inc
 
   OvmfPkg/PlatformDxe/Platform.inf
   OvmfPkg/AmdSevDxe/AmdSevDxe.inf {
diff --git a/OvmfPkg/AmdSev/AmdSevX64.fdf b/OvmfPkg/AmdSev/AmdSevX64.fdf
index 9dd409596780..b84981e7ba04 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.fdf
+++ b/OvmfPkg/AmdSev/AmdSevX64.fdf
@@ -277,14 +277,8 @@ [FV.DXEFV]
 INF  FatPkg/EnhancedFatDxe/Fat.inf
 INF  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
 
-!if $(TOOL_CHAIN_TAG) != "XCODE5" && $(BUILD_SHELL) == TRUE
-INF  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf
-!endif
 INF OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf
 INF  OvmfPkg/AmdSev/Grub/Grub.inf
-!if $(BUILD_SHELL) == TRUE
-INF  ShellPkg/Application/Shell/Shell.inf
-!endif
 
 INF MdeModulePkg/Logo/LogoDxe.inf
 
@@ -320,6 +314,8 @@ [FV.DXEFV]
 #
 !include OvmfPkg/Include/Fdf/OvmfTpmDxe.fdf.inc
 
+!include OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
+
 ################################################################################
 
 [FV.FVMAIN_COMPACT]
-- 
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114329): https://edk2.groups.io/g/devel/message/114329
Mute This Topic: https://groups.io/mt/103935353/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 08/11] OvmfPkg: switch AmdSevX64 to new shell include files

[Laszlo Ersek]

On 1/24/24 17:37, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/AmdSev/AmdSevX64.dsc | 32 ++------------------------------
>  OvmfPkg/AmdSev/AmdSevX64.fdf |  8 ++------
>  2 files changed, 4 insertions(+), 36 deletions(-)
> 
> diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
> index a31a89344a60..41c61467253f 100644
> --- a/OvmfPkg/AmdSev/AmdSevX64.dsc
> +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
> @@ -194,16 +194,12 @@ [LibraryClasses]
>    VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
>    VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
>  
> -!if $(BUILD_SHELL) == TRUE
> -  ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
> -  ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf
> -!endif
> -

The prepatch code seems incorrect to begin with (per earlier
ShellCEntryLib discussion); from commit 30d277ed7a82 ("OvmfPkg/Amdsev:
Base commit to build encrypted boot specific OVMF", 2020-12-14) -- which
I had apparently reviewed. :/ So, good riddance (once ShellLibs.dsc.inc
is fixed).


>    SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
>    OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
>    S3BootScriptLib|MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
>  
>  !include OvmfPkg/Include/Dsc/OvmfTpmLibs.dsc.inc
> +!include OvmfPkg/Include/Dsc/ShellLibs.dsc.inc
>  
>  [LibraryClasses.common]
>    BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> @@ -729,34 +725,10 @@ [Components]
>    MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf
>    MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
>  
> -!if $(TOOL_CHAIN_TAG) != "XCODE5" && $(BUILD_SHELL) == TRUE
> -  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf {
> -    <PcdsFixedAtBuild>
> -      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> -  }
> -!endif
>    OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf
>    OvmfPkg/AmdSev/Grub/Grub.inf
> -!if $(BUILD_SHELL) == TRUE
> -  ShellPkg/Application/Shell/Shell.inf {
> -    <LibraryClasses>
> -      ShellCommandLib|ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.inf
> -      NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
> -      HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
> -      PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
> -      BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.inf
>  
> -    <PcdsFixedAtBuild>
> -      gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xFF
> -      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> -      gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
> -  }
> -!endif
> +!include OvmfPkg/Include/Dsc/ShellComponents.dsc.inc
>  
>    OvmfPkg/PlatformDxe/Platform.inf
>    OvmfPkg/AmdSevDxe/AmdSevDxe.inf {

(1) This is not an identical change. That's alright (we could say this
is an intended benefit from factoring out the shell stuff -- it is now
bringing forward this platform to the common state!), but it should be
documented in the commit message.

With that:

Acked-by: Laszlo Ersek <lersek@redhat.com>


> diff --git a/OvmfPkg/AmdSev/AmdSevX64.fdf b/OvmfPkg/AmdSev/AmdSevX64.fdf
> index 9dd409596780..b84981e7ba04 100644
> --- a/OvmfPkg/AmdSev/AmdSevX64.fdf
> +++ b/OvmfPkg/AmdSev/AmdSevX64.fdf
> @@ -277,14 +277,8 @@ [FV.DXEFV]
>  INF  FatPkg/EnhancedFatDxe/Fat.inf
>  INF  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
>  
> -!if $(TOOL_CHAIN_TAG) != "XCODE5" && $(BUILD_SHELL) == TRUE
> -INF  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf
> -!endif
>  INF OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf
>  INF  OvmfPkg/AmdSev/Grub/Grub.inf
> -!if $(BUILD_SHELL) == TRUE
> -INF  ShellPkg/Application/Shell/Shell.inf
> -!endif
>  
>  INF MdeModulePkg/Logo/LogoDxe.inf
>  
> @@ -320,6 +314,8 @@ [FV.DXEFV]
>  #
>  !include OvmfPkg/Include/Fdf/OvmfTpmDxe.fdf.inc
>  
> +!include OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
> +
>  ################################################################################
>  
>  [FV.FVMAIN_COMPACT]



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114434): https://edk2.groups.io/g/devel/message/114434
Mute This Topic: https://groups.io/mt/103935353/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 08/11] OvmfPkg: switch AmdSevX64 to new shell include files

[Gerd Hoffmann]

  Hi,

> > -!if $(TOOL_CHAIN_TAG) != "XCODE5" && $(BUILD_SHELL) == TRUE
> > -  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf {
> > -    <PcdsFixedAtBuild>
> > -      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> > -  }
> > -!endif
> >    OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf
> >    OvmfPkg/AmdSev/Grub/Grub.inf
> > -!if $(BUILD_SHELL) == TRUE
> > -  ShellPkg/Application/Shell/Shell.inf {
> > -    <LibraryClasses>
> > -      ShellCommandLib|ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.inf
> > -      NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
> > -      NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
> > -      NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
> > -      NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
> > -      NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
> > -      NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
> > -      HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
> > -      PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
> > -      BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.inf
> >  
> > -    <PcdsFixedAtBuild>
> > -      gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xFF
> > -      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> > -      gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
> > -  }
> > -!endif
> > +!include OvmfPkg/Include/Dsc/ShellComponents.dsc.inc
> >  
> >    OvmfPkg/PlatformDxe/Platform.inf
> >    OvmfPkg/AmdSevDxe/AmdSevDxe.inf {
> 
> (1) This is not an identical change. That's alright (we could say this
> is an intended benefit from factoring out the shell stuff -- it is now
> bringing forward this platform to the common state!), but it should be
> documented in the commit message.

Well, AmdSev is compiled without network support, so thanks to the
networking conditionals added easier in this patch series the it
should actually be identical.

I'll note that in the commit message.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115456): https://edk2.groups.io/g/devel/message/115456
Mute This Topic: https://groups.io/mt/103935353/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 09/11] OvmfPkg: switch IntelTdxX64 to new shell include files

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/IntelTdx/IntelTdxX64.dsc | 33 +++-----------------------------
 OvmfPkg/IntelTdx/IntelTdxX64.fdf |  9 ++-------
 2 files changed, 5 insertions(+), 37 deletions(-)

diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
index 82e3e41cfc57..57072116e43e 100644
--- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
+++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
@@ -201,10 +201,6 @@ [LibraryClasses]
   VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
   VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
 
-!if $(BUILD_SHELL) == TRUE
-  ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
-!endif
-  ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf
   S3BootScriptLib|MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
   SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
   OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
@@ -212,6 +208,8 @@ [LibraryClasses]
   Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
   TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
 
+!include OvmfPkg/Include/Dsc/ShellLibs.dsc.inc
+
 [LibraryClasses.common]
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
   CcExitLib|OvmfPkg/Library/CcExitLib/CcExitLib.inf
@@ -717,32 +715,7 @@ [Components]
   MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf
   MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
 
-!if $(TOOL_CHAIN_TAG) != "XCODE5" && $(BUILD_SHELL) == TRUE
-  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf {
-    <PcdsFixedAtBuild>
-      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
-  }
-!endif
-!if $(BUILD_SHELL) == TRUE
-  ShellPkg/Application/Shell/Shell.inf {
-    <LibraryClasses>
-      ShellCommandLib|ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.inf
-      NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
-      HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
-      PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
-      BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.inf
-
-    <PcdsFixedAtBuild>
-      gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xFF
-      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
-      gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
-  }
-!endif
+!include OvmfPkg/Include/Dsc/ShellComponents.dsc.inc
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
   SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.fdf b/OvmfPkg/IntelTdx/IntelTdxX64.fdf
index e844a0988d68..f3b5126254c6 100644
--- a/OvmfPkg/IntelTdx/IntelTdxX64.fdf
+++ b/OvmfPkg/IntelTdx/IntelTdxX64.fdf
@@ -307,13 +307,6 @@ [FV.NCCFV]
 INF  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
 INF  OvmfPkg/VirtioFsDxe/VirtioFsDxe.inf
 
-!if $(BUILD_SHELL) == TRUE && $(TOOL_CHAIN_TAG) != "XCODE5"
-INF  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf
-!endif
-!if $(BUILD_SHELL) == TRUE
-INF  ShellPkg/Application/Shell/Shell.inf
-!endif
-
 INF MdeModulePkg/Logo/LogoDxe.inf
 
 #
@@ -332,6 +325,8 @@ [FV.NCCFV]
 INF  OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
 INF  OvmfPkg/PlatformDxe/Platform.inf
 
+!include OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
+
 ################################################################################
 
 [FV.FVMAIN_COMPACT]
-- 
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114327): https://edk2.groups.io/g/devel/message/114327
Mute This Topic: https://groups.io/mt/103935351/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 09/11] OvmfPkg: switch IntelTdxX64 to new shell include files

[Laszlo Ersek]

On 1/24/24 17:38, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/IntelTdx/IntelTdxX64.dsc | 33 +++-----------------------------
>  OvmfPkg/IntelTdx/IntelTdxX64.fdf |  9 ++-------
>  2 files changed, 5 insertions(+), 37 deletions(-)
> 
> diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
> index 82e3e41cfc57..57072116e43e 100644
> --- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
> +++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
> @@ -201,10 +201,6 @@ [LibraryClasses]
>    VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
>    VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
>  
> -!if $(BUILD_SHELL) == TRUE
> -  ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
> -!endif
> -  ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf
>    S3BootScriptLib|MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
>    SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
>    OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
> @@ -212,6 +208,8 @@ [LibraryClasses]
>    Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
>    TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
>  
> +!include OvmfPkg/Include/Dsc/ShellLibs.dsc.inc
> +
>  [LibraryClasses.common]
>    BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
>    CcExitLib|OvmfPkg/Library/CcExitLib/CcExitLib.inf
> @@ -717,32 +715,7 @@ [Components]
>    MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf
>    MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
>  
> -!if $(TOOL_CHAIN_TAG) != "XCODE5" && $(BUILD_SHELL) == TRUE
> -  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf {
> -    <PcdsFixedAtBuild>
> -      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> -  }
> -!endif
> -!if $(BUILD_SHELL) == TRUE
> -  ShellPkg/Application/Shell/Shell.inf {
> -    <LibraryClasses>
> -      ShellCommandLib|ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.inf
> -      NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
> -      HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
> -      PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
> -      BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.inf
> -
> -    <PcdsFixedAtBuild>
> -      gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xFF
> -      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> -      gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
> -  }
> -!endif
> +!include OvmfPkg/Include/Dsc/ShellComponents.dsc.inc
>  
>  !if $(SECURE_BOOT_ENABLE) == TRUE
>    SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
> diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.fdf b/OvmfPkg/IntelTdx/IntelTdxX64.fdf
> index e844a0988d68..f3b5126254c6 100644
> --- a/OvmfPkg/IntelTdx/IntelTdxX64.fdf
> +++ b/OvmfPkg/IntelTdx/IntelTdxX64.fdf
> @@ -307,13 +307,6 @@ [FV.NCCFV]
>  INF  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
>  INF  OvmfPkg/VirtioFsDxe/VirtioFsDxe.inf
>  
> -!if $(BUILD_SHELL) == TRUE && $(TOOL_CHAIN_TAG) != "XCODE5"
> -INF  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf
> -!endif
> -!if $(BUILD_SHELL) == TRUE
> -INF  ShellPkg/Application/Shell/Shell.inf
> -!endif
> -
>  INF MdeModulePkg/Logo/LogoDxe.inf
>  
>  #
> @@ -332,6 +325,8 @@ [FV.NCCFV]
>  INF  OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
>  INF  OvmfPkg/PlatformDxe/Platform.inf
>  
> +!include OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
> +
>  ################################################################################
>  
>  [FV.FVMAIN_COMPACT]

Again, please document in the commit message that this brings some new
stuff (shell commands) to the platform.

Acked-by: Laszlo Ersek <lersek@redhat.com>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114435): https://edk2.groups.io/g/devel/message/114435
Mute This Topic: https://groups.io/mt/103935351/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 10/11] OvmfPkg: switch MicrovmX64 to new shell include files

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/Microvm/MicrovmX64.dsc | 55 ++++++++--------------------------
 OvmfPkg/Microvm/MicrovmX64.fdf |  9 ++----
 2 files changed, 14 insertions(+), 50 deletions(-)

diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc
index 063324cd0572..2f2b95c9f0fa 100644
--- a/OvmfPkg/Microvm/MicrovmX64.dsc
+++ b/OvmfPkg/Microvm/MicrovmX64.dsc
@@ -33,6 +33,11 @@ [Defines]
   DEFINE SMM_REQUIRE             = FALSE
   DEFINE SOURCE_DEBUG_ENABLE     = FALSE
 
+  #
+  # Shell can be useful for debugging but should not be enabled for production
+  #
+  DEFINE BUILD_SHELL             = TRUE
+
   #
   # Network definition
   #
@@ -230,8 +235,6 @@ [LibraryClasses]
   TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
 !endif
 
-  ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
-  ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf
   S3BootScriptLib|MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
   SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
   OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
@@ -239,6 +242,8 @@ [LibraryClasses]
   Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
   TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
 
+!include OvmfPkg/Include/Dsc/ShellLibs.dsc.inc
+
 [LibraryClasses.common]
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
   CcExitLib|OvmfPkg/Library/CcExitLib/CcExitLib.inf
@@ -837,50 +842,14 @@ [Components]
   MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf
   MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
 
-!if $(TOOL_CHAIN_TAG) != "XCODE5"
-  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf {
-    <PcdsFixedAtBuild>
-      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
-  }
-  ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf {
-    <PcdsFixedAtBuild>
-      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
-  }
-  ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicyDynamicCommand.inf {
-    <PcdsFixedAtBuild>
-      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
-  }
-  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf {
-    <PcdsFixedAtBuild>
-      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
-  }
-!endif
-  ShellPkg/Application/Shell/Shell.inf {
-    <LibraryClasses>
-      ShellCommandLib|ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.inf
-      NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
-      NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
-!if $(NETWORK_IP6_ENABLE) == TRUE
-      NULL|ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.inf
-!endif
-      HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
-      PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
-      BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.inf
-
-    <PcdsFixedAtBuild>
-      gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xFF
-      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
-      gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
-  }
+!include OvmfPkg/Include/Dsc/ShellComponents.dsc.inc
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
   SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
-  OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf
+  OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf {
+    <LibraryClasses>
+    ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf
+  }
 !endif
 
   OvmfPkg/PlatformDxe/Platform.inf
diff --git a/OvmfPkg/Microvm/MicrovmX64.fdf b/OvmfPkg/Microvm/MicrovmX64.fdf
index c877b3f088f9..825bf9f5e41f 100644
--- a/OvmfPkg/Microvm/MicrovmX64.fdf
+++ b/OvmfPkg/Microvm/MicrovmX64.fdf
@@ -257,13 +257,6 @@ [FV.DXEFV]
 INF  EmbeddedPkg/Drivers/FdtClientDxe/FdtClientDxe.inf
 INF  OvmfPkg/Fdt/VirtioFdtDxe/VirtioFdtDxe.inf
 
-!if $(TOOL_CHAIN_TAG) != "XCODE5"
-INF  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
-INF  ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
-INF  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf
-!endif
-INF  ShellPkg/Application/Shell/Shell.inf
-
 INF MdeModulePkg/Logo/LogoDxe.inf
 
 #
@@ -302,6 +295,8 @@ [FV.DXEFV]
 INF  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
 INF  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
 
+!include OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
+
 ################################################################################
 
 [FV.FVMAIN_COMPACT]
-- 
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114330): https://edk2.groups.io/g/devel/message/114330
Mute This Topic: https://groups.io/mt/103935354/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 10/11] OvmfPkg: switch MicrovmX64 to new shell include files

[Laszlo Ersek]

On 1/24/24 17:38, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/Microvm/MicrovmX64.dsc | 55 ++++++++--------------------------
>  OvmfPkg/Microvm/MicrovmX64.fdf |  9 ++----
>  2 files changed, 14 insertions(+), 50 deletions(-)
> 
> diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc
> index 063324cd0572..2f2b95c9f0fa 100644
> --- a/OvmfPkg/Microvm/MicrovmX64.dsc
> +++ b/OvmfPkg/Microvm/MicrovmX64.dsc
> @@ -33,6 +33,11 @@ [Defines]
>    DEFINE SMM_REQUIRE             = FALSE
>    DEFINE SOURCE_DEBUG_ENABLE     = FALSE
>  
> +  #
> +  # Shell can be useful for debugging but should not be enabled for production
> +  #
> +  DEFINE BUILD_SHELL             = TRUE
> +
>    #
>    # Network definition
>    #
> @@ -230,8 +235,6 @@ [LibraryClasses]
>    TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
>  !endif
>  
> -  ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
> -  ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf
>    S3BootScriptLib|MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
>    SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
>    OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
> @@ -239,6 +242,8 @@ [LibraryClasses]
>    Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
>    TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
>  
> +!include OvmfPkg/Include/Dsc/ShellLibs.dsc.inc
> +
>  [LibraryClasses.common]
>    BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
>    CcExitLib|OvmfPkg/Library/CcExitLib/CcExitLib.inf
> @@ -837,50 +842,14 @@ [Components]
>    MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf
>    MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
>  
> -!if $(TOOL_CHAIN_TAG) != "XCODE5"
> -  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf {
> -    <PcdsFixedAtBuild>
> -      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> -  }
> -  ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf {
> -    <PcdsFixedAtBuild>
> -      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> -  }
> -  ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicyDynamicCommand.inf {
> -    <PcdsFixedAtBuild>
> -      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> -  }
> -  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf {
> -    <PcdsFixedAtBuild>
> -      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> -  }
> -!endif
> -  ShellPkg/Application/Shell/Shell.inf {
> -    <LibraryClasses>
> -      ShellCommandLib|ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.inf
> -      NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
> -      NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
> -!if $(NETWORK_IP6_ENABLE) == TRUE
> -      NULL|ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.inf
> -!endif
> -      HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
> -      PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
> -      BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.inf
> -
> -    <PcdsFixedAtBuild>
> -      gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xFF
> -      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> -      gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
> -  }
> +!include OvmfPkg/Include/Dsc/ShellComponents.dsc.inc
>  
>  !if $(SECURE_BOOT_ENABLE) == TRUE
>    SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
> -  OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf
> +  OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf {
> +    <LibraryClasses>
> +    ShellCEntryLib|ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf
> +  }
>  !endif
>  
>    OvmfPkg/PlatformDxe/Platform.inf

"OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf" should be left alone.

Acked-by: Laszlo Ersek <lersek@redhat.com>


> diff --git a/OvmfPkg/Microvm/MicrovmX64.fdf b/OvmfPkg/Microvm/MicrovmX64.fdf
> index c877b3f088f9..825bf9f5e41f 100644
> --- a/OvmfPkg/Microvm/MicrovmX64.fdf
> +++ b/OvmfPkg/Microvm/MicrovmX64.fdf
> @@ -257,13 +257,6 @@ [FV.DXEFV]
>  INF  EmbeddedPkg/Drivers/FdtClientDxe/FdtClientDxe.inf
>  INF  OvmfPkg/Fdt/VirtioFdtDxe/VirtioFdtDxe.inf
>  
> -!if $(TOOL_CHAIN_TAG) != "XCODE5"
> -INF  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
> -INF  ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
> -INF  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf
> -!endif
> -INF  ShellPkg/Application/Shell/Shell.inf
> -
>  INF MdeModulePkg/Logo/LogoDxe.inf
>  
>  #
> @@ -302,6 +295,8 @@ [FV.DXEFV]
>  INF  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
>  INF  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
>  
> +!include OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
> +
>  ################################################################################
>  
>  [FV.FVMAIN_COMPACT]



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114436): https://edk2.groups.io/g/devel/message/114436
Mute This Topic: https://groups.io/mt/103935354/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 11/11] OvmfPkg/CI: copy shell to virtual drive

[Gerd Hoffmann]

Place the EFI shell as EFI/BOOT/BOOT{ARCH}.EFI on the virtual drive.
This allows the "run to shell" CI test case to work even in case the
shell is not included in the firmware image.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/PlatformCI/PlatformBuildLib.py | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/PlatformCI/PlatformBuildLib.py b/OvmfPkg/PlatformCI/PlatformBuildLib.py
index f829738cdda4..00d454954bff 100644
--- a/OvmfPkg/PlatformCI/PlatformBuildLib.py
+++ b/OvmfPkg/PlatformCI/PlatformBuildLib.py
@@ -5,6 +5,7 @@
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 ##
 import os
+import shutil
 import logging
 import io
 
@@ -181,7 +182,8 @@ class PlatformBuilder( UefiBuilder, BuildSettingsManager):
 
     def FlashRomImage(self):
         VirtualDrive = os.path.join(self.env.GetValue("BUILD_OUTPUT_BASE"), "VirtualDrive")
-        os.makedirs(VirtualDrive, exist_ok=True)
+        VirtualDriveBoot = os.path.join(VirtualDrive, "EFI", "BOOT")
+        os.makedirs(VirtualDriveBoot, exist_ok=True)
         OutputPath_FV = os.path.join(self.env.GetValue("BUILD_OUTPUT_BASE"), "FV")
 
         if (self.env.GetValue("QEMU_SKIP") and
@@ -189,6 +191,14 @@ class PlatformBuilder( UefiBuilder, BuildSettingsManager):
             logging.info("skipping qemu boot test")
             return 0
 
+        # copy shell to VirtualDrive
+        for arch in self.env.GetValue("TARGET_ARCH").split():
+            src = os.path.join(self.env.GetValue("BUILD_OUTPUT_BASE"), arch, "Shell.efi")
+            dst = os.path.join(VirtualDriveBoot, f'BOOT{arch}.EFI')
+            if os.path.exists(src):
+                logging.info("copy %s -> %s", src, dst)
+                shutil.copyfile(src, dst)
+
         #
         # QEMU must be on the path
         #
-- 
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114328): https://edk2.groups.io/g/devel/message/114328
Mute This Topic: https://groups.io/mt/103935352/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 11/11] OvmfPkg/CI: copy shell to virtual drive

[Laszlo Ersek]

On 1/24/24 17:38, Gerd Hoffmann wrote:
> Place the EFI shell as EFI/BOOT/BOOT{ARCH}.EFI on the virtual drive.
> This allows the "run to shell" CI test case to work even in case the
> shell is not included in the firmware image.

In the commit message, can you list the platforms explicitly that
benefit from this change?

Thanks,
Laszlo

> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/PlatformCI/PlatformBuildLib.py | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/PlatformCI/PlatformBuildLib.py b/OvmfPkg/PlatformCI/PlatformBuildLib.py
> index f829738cdda4..00d454954bff 100644
> --- a/OvmfPkg/PlatformCI/PlatformBuildLib.py
> +++ b/OvmfPkg/PlatformCI/PlatformBuildLib.py
> @@ -5,6 +5,7 @@
>  # SPDX-License-Identifier: BSD-2-Clause-Patent
>  ##
>  import os
> +import shutil
>  import logging
>  import io
>  
> @@ -181,7 +182,8 @@ class PlatformBuilder( UefiBuilder, BuildSettingsManager):
>  
>      def FlashRomImage(self):
>          VirtualDrive = os.path.join(self.env.GetValue("BUILD_OUTPUT_BASE"), "VirtualDrive")
> -        os.makedirs(VirtualDrive, exist_ok=True)
> +        VirtualDriveBoot = os.path.join(VirtualDrive, "EFI", "BOOT")
> +        os.makedirs(VirtualDriveBoot, exist_ok=True)
>          OutputPath_FV = os.path.join(self.env.GetValue("BUILD_OUTPUT_BASE"), "FV")
>  
>          if (self.env.GetValue("QEMU_SKIP") and
> @@ -189,6 +191,14 @@ class PlatformBuilder( UefiBuilder, BuildSettingsManager):
>              logging.info("skipping qemu boot test")
>              return 0
>  
> +        # copy shell to VirtualDrive
> +        for arch in self.env.GetValue("TARGET_ARCH").split():
> +            src = os.path.join(self.env.GetValue("BUILD_OUTPUT_BASE"), arch, "Shell.efi")
> +            dst = os.path.join(VirtualDriveBoot, f'BOOT{arch}.EFI')
> +            if os.path.exists(src):
> +                logging.info("copy %s -> %s", src, dst)
> +                shutil.copyfile(src, dst)
> +
>          #
>          # QEMU must be on the path
>          #



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114437): https://edk2.groups.io/g/devel/message/114437
Mute This Topic: https://groups.io/mt/103935352/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 00/11] OvmfPkg: tweak shell builds

[Yao, Jiewen]

Always good to reduce duplication!
Thanks for doing that.

Acked-by: Jiewen Yao <Jiewen.yao@intel.com>

> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Thursday, January 25, 2024 12:38 AM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Ard Biesheuvel
> <ardb+tianocore@kernel.org>; Michael Roth <michael.roth@amd.com>; Gerd
> Hoffmann <kraxel@redhat.com>; Laszlo Ersek <lersek@redhat.com>; Aktas,
> Erdem <erdemaktas@google.com>; Xu, Min M <min.m.xu@intel.com>; Tom
> Lendacky <thomas.lendacky@amd.com>; Oliver Steffen <osteffen@redhat.com>
> Subject: [PATCH 00/11] OvmfPkg: tweak shell builds
> 
> - Create include files to reduce duplication.
> - Fix varpolicy command.
> - Little CI tweak.
> 
> Gerd Hoffmann (11):
>   OvmfPkg: add ShellComponents.dsc.inc
>   OvmfPkg: add ShellLibs.dsc.inc
>   OvmfPkg: add ShellDxe.fdf.inc
>   OvmfPkg: Shell*.inc: allow building without network support
>   OvmfPkg: ShellDxe.fdf.inc: add VariablePolicyDynamicCommand to FV
>   OvmfPkg: switch OvmfPkgIa32 to new shell include files
>   OvmfPkg: switch OvmfPkgIa32X64 to new shell include files
>   OvmfPkg: switch AmdSevX64 to new shell include files
>   OvmfPkg: switch IntelTdxX64 to new shell include files
>   OvmfPkg: switch MicrovmX64 to new shell include files
>   OvmfPkg/CI: copy shell to virtual drive
> 
>  OvmfPkg/Include/Dsc/ShellComponents.dsc.inc | 53 ++++++++++++++++++++
>  OvmfPkg/Include/Dsc/ShellLibs.dsc.inc       | 11 +++++
>  OvmfPkg/AmdSev/AmdSevX64.dsc                | 32 +-----------
>  OvmfPkg/IntelTdx/IntelTdxX64.dsc            | 33 ++-----------
>  OvmfPkg/Microvm/MicrovmX64.dsc              | 55 +++++----------------
>  OvmfPkg/OvmfPkgIa32.dsc                     | 49 +-----------------
>  OvmfPkg/OvmfPkgIa32X64.dsc                  | 54 +++-----------------
>  OvmfPkg/OvmfPkgX64.dsc                      | 54 +++-----------------
>  OvmfPkg/AmdSev/AmdSevX64.fdf                |  8 +--
>  OvmfPkg/IntelTdx/IntelTdxX64.fdf            |  9 +---
>  OvmfPkg/Microvm/MicrovmX64.fdf              |  9 +---
>  OvmfPkg/OvmfPkgIa32.fdf                     | 11 +----
>  OvmfPkg/OvmfPkgIa32X64.fdf                  | 11 +----
>  OvmfPkg/OvmfPkgX64.fdf                      | 11 +----
>  OvmfPkg/Include/Fdf/ShellDxe.fdf.inc        | 17 +++++++
>  OvmfPkg/PlatformCI/PlatformBuildLib.py      | 12 ++++-
>  16 files changed, 135 insertions(+), 294 deletions(-)
>  create mode 100644 OvmfPkg/Include/Dsc/ShellComponents.dsc.inc
>  create mode 100644 OvmfPkg/Include/Dsc/ShellLibs.dsc.inc
>  create mode 100644 OvmfPkg/Include/Fdf/ShellDxe.fdf.inc
> 
> --
> 2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114362): https://edk2.groups.io/g/devel/message/114362
Mute This Topic: https://groups.io/mt/103935343/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-