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

["Laszlo Ersek" <lersek@redhat.com>]

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]
-=-=-=-=-=-=-=-=-=-=-=-