* [PATCH 0/4] Shell/Quark/ArmPkg: promote shell app FILE_GUID to proper GUID
@ 2017-03-22 14:04 Ard Biesheuvel
2017-03-22 14:04 ` [PATCH 1/4] ShellPkg: add GUID declaration for FILE_GUID of UEFI Shell app to package Ard Biesheuvel
` (4 more replies)
0 siblings, 5 replies; 15+ messages in thread
From: Ard Biesheuvel @ 2017-03-22 14:04 UTC (permalink / raw)
To: edk2-devel, leif.lindholm, lersek, jaben.carsey, ruiyu.ni,
michael.d.kinney, kelly.steele
Cc: Ard Biesheuvel
To avoid the need for open coded GUID declarations that copy the FILE_GUID
of the Shell app, add it to ShellPkg.dec and replace existing references.
Ard Biesheuvel (4):
ShellPkg: add GUID declaration for FILE_GUID of UEFI Shell app to
package
QuarkPlatformPkg/PlatformBootManagerLib: use new UefiShellFileGuid
definition
ArmPkg/PlatformBootManagerLib: refer to Shell FILE_GUID directly
ArmVirtPkg/ArmVirtQemu: refer to Shell app via its declared GUID
ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 5 ++---
ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 3 ++-
ArmVirtPkg/ArmVirtQemu.dsc | 1 -
ArmVirtPkg/ArmVirtQemuKernel.dsc | 1 -
ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c | 2 +-
ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 6 +++---
QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.c | 4 +---
QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 2 ++
ShellPkg/ShellPkg.dec | 3 +++
9 files changed, 14 insertions(+), 13 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/4] ShellPkg: add GUID declaration for FILE_GUID of UEFI Shell app to package
2017-03-22 14:04 [PATCH 0/4] Shell/Quark/ArmPkg: promote shell app FILE_GUID to proper GUID Ard Biesheuvel
@ 2017-03-22 14:04 ` Ard Biesheuvel
2017-03-22 15:13 ` Carsey, Jaben
2017-03-22 14:04 ` [PATCH 2/4] QuarkPlatformPkg/PlatformBootManagerLib: use new UefiShellFileGuid definition Ard Biesheuvel
` (3 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Ard Biesheuvel @ 2017-03-22 14:04 UTC (permalink / raw)
To: edk2-devel, leif.lindholm, lersek, jaben.carsey, ruiyu.ni,
michael.d.kinney, kelly.steele
Cc: Ard Biesheuvel
In QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.c,
there is a definition of mUefiShellFileGuid which is a constant reference
to the FILE_GUID as defined in ShellPkg/Application/Shell/Shell.inf.
To prevent the need for duplicating it to other modules, promote it to
a proper global GUID, and add it to the ShellPkg.dec package declaration.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
ShellPkg/ShellPkg.dec | 3 +++
1 file changed, 3 insertions(+)
diff --git a/ShellPkg/ShellPkg.dec b/ShellPkg/ShellPkg.dec
index bb31c2df8cb3..3ad17a44b447 100644
--- a/ShellPkg/ShellPkg.dec
+++ b/ShellPkg/ShellPkg.dec
@@ -57,6 +57,9 @@ [Guids]
gShellTftpHiiGuid = {0x738a9314, 0x82c1, 0x4592, {0x8f, 0xf7, 0xc1, 0xbd, 0xf1, 0xb2, 0x0e, 0xd4}}
gShellBcfgHiiGuid = {0x5f5f605d, 0x1583, 0x4a2d, {0xa6, 0xb2, 0xeb, 0x12, 0xda, 0xb4, 0xa2, 0xb6}}
+ # FILE_GUID as defined in ShellPkg/Application/Shell/Shell.inf
+ gUefiShellFileGuid = {0x7c04a583, 0x9e3e, 0x4f1c, {0xad, 0x65, 0xe0, 0x52, 0x68, 0xd0, 0xb4, 0xd1}}
+
[Protocols]
gEfiShellEnvironment2Guid = {0x47c7b221, 0xc42a, 0x11d2, {0x8e, 0x57, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b}}
gEfiShellInterfaceGuid = {0x47c7b223, 0xc42a, 0x11d2, {0x8e, 0x57, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b}}
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/4] QuarkPlatformPkg/PlatformBootManagerLib: use new UefiShellFileGuid definition
2017-03-22 14:04 [PATCH 0/4] Shell/Quark/ArmPkg: promote shell app FILE_GUID to proper GUID Ard Biesheuvel
2017-03-22 14:04 ` [PATCH 1/4] ShellPkg: add GUID declaration for FILE_GUID of UEFI Shell app to package Ard Biesheuvel
@ 2017-03-22 14:04 ` Ard Biesheuvel
2017-03-22 15:43 ` Kinney, Michael D
2017-03-22 14:04 ` [PATCH 3/4] ArmPkg/PlatformBootManagerLib: refer to Shell FILE_GUID directly Ard Biesheuvel
` (2 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Ard Biesheuvel @ 2017-03-22 14:04 UTC (permalink / raw)
To: edk2-devel, leif.lindholm, lersek, jaben.carsey, ruiyu.ni,
michael.d.kinney, kelly.steele
Cc: Ard Biesheuvel
Move to the new definition of UefiShellFileGuid, which is defined in the
ShellPkg package declaration file rather than hardcoded in this module.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.c | 4 +---
QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 2 ++
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.c b/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
index b61eb03360a2..3c213180690d 100644
--- a/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
+++ b/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
@@ -15,8 +15,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include "PlatformBootManager.h"
-EFI_GUID mUefiShellFileGuid = {0x7C04A583, 0x9E3E, 0x4f1c, {0xAD, 0x65, 0xE0, 0x52, 0x68, 0xD0, 0xB4, 0xD1 }};
-
/**
Return the index of the load option in the load option array.
@@ -246,7 +244,7 @@ PlatformBootManagerBeforeConsole (
//
// Register UEFI Shell
//
- PlatformRegisterFvBootOption (&mUefiShellFileGuid, L"UEFI Shell", LOAD_OPTION_ACTIVE);
+ PlatformRegisterFvBootOption (&gUefiShellFileGuid, L"UEFI Shell", LOAD_OPTION_ACTIVE);
Status = gBS->LocateProtocol(&gEsrtManagementProtocolGuid, NULL, (VOID **)&EsrtManagement);
if (EFI_ERROR(Status)) {
diff --git a/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index af399e529a92..25394d8ca000 100644
--- a/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -40,6 +40,7 @@ [Packages]
SourceLevelDebugPkg/SourceLevelDebugPkg.dec
QuarkPlatformPkg/QuarkPlatformPkg.dec
SecurityPkg/SecurityPkg.dec
+ ShellPkg/ShellPkg.dec
SignedCapsulePkg/SignedCapsulePkg.dec
[LibraryClasses]
@@ -70,6 +71,7 @@ [Guids]
gEfiVTUTF8Guid
gEfiTtyTermGuid
gEfiEndOfDxeEventGroupGuid
+ gUefiShellFileGuid
[Pcd]
gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/4] ArmPkg/PlatformBootManagerLib: refer to Shell FILE_GUID directly
2017-03-22 14:04 [PATCH 0/4] Shell/Quark/ArmPkg: promote shell app FILE_GUID to proper GUID Ard Biesheuvel
2017-03-22 14:04 ` [PATCH 1/4] ShellPkg: add GUID declaration for FILE_GUID of UEFI Shell app to package Ard Biesheuvel
2017-03-22 14:04 ` [PATCH 2/4] QuarkPlatformPkg/PlatformBootManagerLib: use new UefiShellFileGuid definition Ard Biesheuvel
@ 2017-03-22 14:04 ` Ard Biesheuvel
2017-03-22 14:04 ` [PATCH 4/4] ArmVirtPkg/ArmVirtQemu: refer to Shell app via its declared GUID Ard Biesheuvel
2017-03-22 15:38 ` [PATCH 0/4] Shell/Quark/ArmPkg: promote shell app FILE_GUID to proper GUID Ard Biesheuvel
4 siblings, 0 replies; 15+ messages in thread
From: Ard Biesheuvel @ 2017-03-22 14:04 UTC (permalink / raw)
To: edk2-devel, leif.lindholm, lersek, jaben.carsey, ruiyu.ni,
michael.d.kinney, kelly.steele
Cc: Ard Biesheuvel
Instead of indirecting the reference to the Shell binary via a PCD
that is defined in IntelFrameworkModulePkg, and which invariably
gets set to the same value by all users of this library, refer to
the UEFI Shell application by its declared symbolic GUID.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
---
ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 5 ++---
ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 3 ++-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
index cc5a4d1ff9b3..3eaed3a4799b 100644
--- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
+++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
@@ -33,7 +33,6 @@
#define DP_NODE_LEN(Type) { (UINT8)sizeof (Type), (UINT8)(sizeof (Type) >> 8) }
-
#pragma pack (1)
typedef struct {
VENDOR_DEVICE_PATH SerialDxe;
@@ -327,7 +326,7 @@ AddOutput (
STATIC
VOID
PlatformRegisterFvBootOption (
- EFI_GUID *FileGuid,
+ CONST EFI_GUID *FileGuid,
CHAR16 *Description,
UINT32 Attributes
)
@@ -540,7 +539,7 @@ PlatformBootManagerAfterConsole (
// Register UEFI Shell
//
PlatformRegisterFvBootOption (
- PcdGetPtr (PcdShellFile), L"UEFI Shell", LOAD_OPTION_ACTIVE
+ &gUefiShellFileGuid, L"UEFI Shell", LOAD_OPTION_ACTIVE
);
}
diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index 8ec4f1dea6c4..beca5800d649 100644
--- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -39,6 +39,7 @@ [Packages]
IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec
MdeModulePkg/MdeModulePkg.dec
MdePkg/MdePkg.dec
+ ShellPkg/ShellPkg.dec
[LibraryClasses]
BaseLib
@@ -59,7 +60,6 @@ [FeaturePcd]
[FixedPcd]
gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdLogoFile
- gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile
gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate
gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits
gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity
@@ -75,6 +75,7 @@ [Guids]
gEfiFileSystemVolumeLabelInfoIdGuid
gEfiEndOfDxeEventGroupGuid
gEfiTtyTermGuid
+ gUefiShellFileGuid
[Protocols]
gEfiDevicePathProtocolGuid
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/4] ArmVirtPkg/ArmVirtQemu: refer to Shell app via its declared GUID
2017-03-22 14:04 [PATCH 0/4] Shell/Quark/ArmPkg: promote shell app FILE_GUID to proper GUID Ard Biesheuvel
` (2 preceding siblings ...)
2017-03-22 14:04 ` [PATCH 3/4] ArmPkg/PlatformBootManagerLib: refer to Shell FILE_GUID directly Ard Biesheuvel
@ 2017-03-22 14:04 ` Ard Biesheuvel
2017-03-22 14:39 ` Laszlo Ersek
2017-03-22 15:38 ` [PATCH 0/4] Shell/Quark/ArmPkg: promote shell app FILE_GUID to proper GUID Ard Biesheuvel
4 siblings, 1 reply; 15+ messages in thread
From: Ard Biesheuvel @ 2017-03-22 14:04 UTC (permalink / raw)
To: edk2-devel, leif.lindholm, lersek, jaben.carsey, ruiyu.ni,
michael.d.kinney, kelly.steele
Cc: Ard Biesheuvel
Currently, the file GUID reference of the UEFI Shell app is indirected
via the PCD gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile,
which is set to a fixed value for our platforms.
So instead, use the new symbolic GUID added for this purpose, and drop
the reference to this PCD, and to the IntelFrameworkModulePkg package
entirely.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
ArmVirtPkg/ArmVirtQemu.dsc | 1 -
ArmVirtPkg/ArmVirtQemuKernel.dsc | 1 -
ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c | 2 +-
ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 6 +++---
4 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 0bbbe4a7aa4a..4f686faa559c 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -139,7 +139,6 @@ [PcdsFixedAtBuild.common]
gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
- gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile|{ 0x83, 0xA5, 0x04, 0x7C, 0x3E, 0x9E, 0x1C, 0x4F, 0xAD, 0x65, 0xE0, 0x52, 0x68, 0xD0, 0xB4, 0xD1 }
#
# The maximum physical I/O addressability of the processor, set with
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index 71f16ed192de..6b6555c889a3 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -156,7 +156,6 @@ [PcdsFixedAtBuild.AARCH64]
gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
- gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile|{ 0x83, 0xA5, 0x04, 0x7C, 0x3E, 0x9E, 0x1C, 0x4F, 0xAD, 0x65, 0xE0, 0x52, 0x68, 0xD0, 0xB4, 0xD1 }
#
# The maximum physical I/O addressability of the processor, set with
diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
index 94da51ad49f1..f9591964d577 100644
--- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
+++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
@@ -691,7 +691,7 @@ PlatformBootManagerAfterConsole (
// Register UEFI Shell
//
PlatformRegisterFvBootOption (
- PcdGetPtr (PcdShellFile), L"EFI Internal Shell", LOAD_OPTION_ACTIVE
+ &gUefiShellFileGuid, L"EFI Internal Shell", LOAD_OPTION_ACTIVE
);
RemoveStaleFvFileOptions ();
diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index 1f162c663fc1..bac6c781b3c8 100644
--- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -35,11 +35,11 @@ [Sources]
QemuKernel.c
[Packages]
- IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec
+ ArmVirtPkg/ArmVirtPkg.dec
MdeModulePkg/MdeModulePkg.dec
MdePkg/MdePkg.dec
OvmfPkg/OvmfPkg.dec
- ArmVirtPkg/ArmVirtPkg.dec
+ ShellPkg/ShellPkg.dec
[LibraryClasses]
BaseLib
@@ -58,7 +58,6 @@ [LibraryClasses]
UefiRuntimeServicesTableLib
[FixedPcd]
- gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile
gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate
gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits
gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity
@@ -74,6 +73,7 @@ [Guids]
gEfiFileSystemVolumeLabelInfoIdGuid
gEfiEndOfDxeEventGroupGuid
gRootBridgesConnectedEventGroupGuid
+ gUefiShellFileGuid
[Protocols]
gEfiDevicePathProtocolGuid
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] ArmVirtPkg/ArmVirtQemu: refer to Shell app via its declared GUID
2017-03-22 14:04 ` [PATCH 4/4] ArmVirtPkg/ArmVirtQemu: refer to Shell app via its declared GUID Ard Biesheuvel
@ 2017-03-22 14:39 ` Laszlo Ersek
0 siblings, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2017-03-22 14:39 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel, leif.lindholm, jaben.carsey, ruiyu.ni,
michael.d.kinney, kelly.steele
On 03/22/17 15:04, Ard Biesheuvel wrote:
> Currently, the file GUID reference of the UEFI Shell app is indirected
> via the PCD gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile,
> which is set to a fixed value for our platforms.
>
> So instead, use the new symbolic GUID added for this purpose, and drop
> the reference to this PCD, and to the IntelFrameworkModulePkg package
> entirely.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> ArmVirtPkg/ArmVirtQemu.dsc | 1 -
> ArmVirtPkg/ArmVirtQemuKernel.dsc | 1 -
> ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c | 2 +-
> ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 6 +++---
> 4 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index 0bbbe4a7aa4a..4f686faa559c 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -139,7 +139,6 @@ [PcdsFixedAtBuild.common]
>
> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
> gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
> - gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile|{ 0x83, 0xA5, 0x04, 0x7C, 0x3E, 0x9E, 0x1C, 0x4F, 0xAD, 0x65, 0xE0, 0x52, 0x68, 0xD0, 0xB4, 0xD1 }
>
> #
> # The maximum physical I/O addressability of the processor, set with
> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> index 71f16ed192de..6b6555c889a3 100644
> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> @@ -156,7 +156,6 @@ [PcdsFixedAtBuild.AARCH64]
>
> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
> gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
> - gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile|{ 0x83, 0xA5, 0x04, 0x7C, 0x3E, 0x9E, 0x1C, 0x4F, 0xAD, 0x65, 0xE0, 0x52, 0x68, 0xD0, 0xB4, 0xD1 }
>
> #
> # The maximum physical I/O addressability of the processor, set with
> diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
> index 94da51ad49f1..f9591964d577 100644
> --- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
> +++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -691,7 +691,7 @@ PlatformBootManagerAfterConsole (
> // Register UEFI Shell
> //
> PlatformRegisterFvBootOption (
> - PcdGetPtr (PcdShellFile), L"EFI Internal Shell", LOAD_OPTION_ACTIVE
> + &gUefiShellFileGuid, L"EFI Internal Shell", LOAD_OPTION_ACTIVE
> );
>
> RemoveStaleFvFileOptions ();
> diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> index 1f162c663fc1..bac6c781b3c8 100644
> --- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> @@ -35,11 +35,11 @@ [Sources]
> QemuKernel.c
>
> [Packages]
> - IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec
> + ArmVirtPkg/ArmVirtPkg.dec
> MdeModulePkg/MdeModulePkg.dec
> MdePkg/MdePkg.dec
> OvmfPkg/OvmfPkg.dec
> - ArmVirtPkg/ArmVirtPkg.dec
> + ShellPkg/ShellPkg.dec
>
> [LibraryClasses]
> BaseLib
> @@ -58,7 +58,6 @@ [LibraryClasses]
> UefiRuntimeServicesTableLib
>
> [FixedPcd]
> - gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile
> gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate
> gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits
> gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity
> @@ -74,6 +73,7 @@ [Guids]
> gEfiFileSystemVolumeLabelInfoIdGuid
> gEfiEndOfDxeEventGroupGuid
> gRootBridgesConnectedEventGroupGuid
> + gUefiShellFileGuid
>
> [Protocols]
> gEfiDevicePathProtocolGuid
>
Works for me if the shellpkg maintainers are OK with the centralization
of the GUID.
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] ShellPkg: add GUID declaration for FILE_GUID of UEFI Shell app to package
2017-03-22 14:04 ` [PATCH 1/4] ShellPkg: add GUID declaration for FILE_GUID of UEFI Shell app to package Ard Biesheuvel
@ 2017-03-22 15:13 ` Carsey, Jaben
2017-03-22 15:19 ` Ard Biesheuvel
0 siblings, 1 reply; 15+ messages in thread
From: Carsey, Jaben @ 2017-03-22 15:13 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel@lists.01.org, leif.lindholm@linaro.org,
lersek@redhat.com, Ni, Ruiyu, Kinney, Michael D, Steele, Kelly
Cc: Carsey, Jaben
Ard,
I am good with this change.
What do you think about a comment to the INF file so that if someone makes a change to the GUI there, they are informed that they need to change this location? I worry as usually updating an INF GUID is permitted without any need to change another file...
-Jaben
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Wednesday, March 22, 2017 7:04 AM
> To: edk2-devel@lists.01.org; leif.lindholm@linaro.org; lersek@redhat.com;
> Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>; Steele, Kelly
> <kelly.steele@intel.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Subject: [edk2] [PATCH 1/4] ShellPkg: add GUID declaration for FILE_GUID of
> UEFI Shell app to package
> Importance: High
>
> In
> QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.
> c,
> there is a definition of mUefiShellFileGuid which is a constant reference
> to the FILE_GUID as defined in ShellPkg/Application/Shell/Shell.inf.
>
> To prevent the need for duplicating it to other modules, promote it to
> a proper global GUID, and add it to the ShellPkg.dec package declaration.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> ShellPkg/ShellPkg.dec | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/ShellPkg/ShellPkg.dec b/ShellPkg/ShellPkg.dec
> index bb31c2df8cb3..3ad17a44b447 100644
> --- a/ShellPkg/ShellPkg.dec
> +++ b/ShellPkg/ShellPkg.dec
> @@ -57,6 +57,9 @@ [Guids]
> gShellTftpHiiGuid = {0x738a9314, 0x82c1, 0x4592, {0x8f, 0xf7, 0xc1,
> 0xbd, 0xf1, 0xb2, 0x0e, 0xd4}}
> gShellBcfgHiiGuid = {0x5f5f605d, 0x1583, 0x4a2d, {0xa6, 0xb2, 0xeb,
> 0x12, 0xda, 0xb4, 0xa2, 0xb6}}
>
> + # FILE_GUID as defined in ShellPkg/Application/Shell/Shell.inf
> + gUefiShellFileGuid = {0x7c04a583, 0x9e3e, 0x4f1c, {0xad, 0x65, 0xe0,
> 0x52, 0x68, 0xd0, 0xb4, 0xd1}}
> +
> [Protocols]
> gEfiShellEnvironment2Guid = {0x47c7b221, 0xc42a, 0x11d2, {0x8e,
> 0x57, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b}}
> gEfiShellInterfaceGuid = {0x47c7b223, 0xc42a, 0x11d2, {0x8e, 0x57,
> 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b}}
> --
> 2.7.4
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] ShellPkg: add GUID declaration for FILE_GUID of UEFI Shell app to package
2017-03-22 15:13 ` Carsey, Jaben
@ 2017-03-22 15:19 ` Ard Biesheuvel
2017-03-22 15:21 ` Carsey, Jaben
0 siblings, 1 reply; 15+ messages in thread
From: Ard Biesheuvel @ 2017-03-22 15:19 UTC (permalink / raw)
To: Carsey, Jaben
Cc: edk2-devel@lists.01.org, leif.lindholm@linaro.org,
lersek@redhat.com, Ni, Ruiyu, Kinney, Michael D, Steele, Kelly
On 22 March 2017 at 15:13, Carsey, Jaben <jaben.carsey@intel.com> wrote:
> Ard,
>
> I am good with this change.
>
> What do you think about a comment to the INF file so that if someone makes a change to the GUI there, they are informed that they need to change this location? I worry as usually updating an INF GUID is permitted without any need to change another file...
>
Something like this perhaps?
--- a/ShellPkg/Application/Shell/Shell.inf
+++ b/ShellPkg/Application/Shell/Shell.inf
@@ -17,7 +17,7 @@
[Defines]
INF_VERSION = 0x00010006
BASE_NAME = Shell
- FILE_GUID = 7C04A583-9E3E-4f1c-AD65-E05268D0B4D1
+ FILE_GUID = 7C04A583-9E3E-4f1c-AD65-E05268D0B4D1 # gUefiShellFileGuid
MODULE_TYPE = UEFI_APPLICATION
VERSION_STRING = 1.0
ENTRY_POINT = UefiMain
(Note that the same FILE_GUID occurs in ShellBinPkg as well, but
people are unlikely that randomly change that one without regard to
the source build)
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] ShellPkg: add GUID declaration for FILE_GUID of UEFI Shell app to package
2017-03-22 15:19 ` Ard Biesheuvel
@ 2017-03-22 15:21 ` Carsey, Jaben
2017-03-22 15:39 ` Kinney, Michael D
0 siblings, 1 reply; 15+ messages in thread
From: Carsey, Jaben @ 2017-03-22 15:21 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Ni, Ruiyu, edk2-devel@lists.01.org, leif.lindholm@linaro.org,
Kinney, Michael D, lersek@redhat.com, Carsey, Jaben
Yes. that looks great.
For the changes to ShellPkg.
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Wednesday, March 22, 2017 8:20 AM
> To: Carsey, Jaben <jaben.carsey@intel.com>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org;
> leif.lindholm@linaro.org; Kinney, Michael D <michael.d.kinney@intel.com>;
> lersek@redhat.com
> Subject: Re: [edk2] [PATCH 1/4] ShellPkg: add GUID declaration for
> FILE_GUID of UEFI Shell app to package
> Importance: High
>
> On 22 March 2017 at 15:13, Carsey, Jaben <jaben.carsey@intel.com> wrote:
> > Ard,
> >
> > I am good with this change.
> >
> > What do you think about a comment to the INF file so that if someone
> makes a change to the GUI there, they are informed that they need to
> change this location? I worry as usually updating an INF GUID is permitted
> without any need to change another file...
> >
>
> Something like this perhaps?
>
> --- a/ShellPkg/Application/Shell/Shell.inf
> +++ b/ShellPkg/Application/Shell/Shell.inf
> @@ -17,7 +17,7 @@
> [Defines]
> INF_VERSION = 0x00010006
> BASE_NAME = Shell
> - FILE_GUID = 7C04A583-9E3E-4f1c-AD65-E05268D0B4D1
> + FILE_GUID = 7C04A583-9E3E-4f1c-AD65-E05268D0B4D1 #
> gUefiShellFileGuid
> MODULE_TYPE = UEFI_APPLICATION
> VERSION_STRING = 1.0
> ENTRY_POINT = UefiMain
>
> (Note that the same FILE_GUID occurs in ShellBinPkg as well, but
> people are unlikely that randomly change that one without regard to
> the source build)
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/4] Shell/Quark/ArmPkg: promote shell app FILE_GUID to proper GUID
2017-03-22 14:04 [PATCH 0/4] Shell/Quark/ArmPkg: promote shell app FILE_GUID to proper GUID Ard Biesheuvel
` (3 preceding siblings ...)
2017-03-22 14:04 ` [PATCH 4/4] ArmVirtPkg/ArmVirtQemu: refer to Shell app via its declared GUID Ard Biesheuvel
@ 2017-03-22 15:38 ` Ard Biesheuvel
2017-03-22 15:39 ` Carsey, Jaben
4 siblings, 1 reply; 15+ messages in thread
From: Ard Biesheuvel @ 2017-03-22 15:38 UTC (permalink / raw)
To: edk2-devel@lists.01.org, Leif Lindholm, Laszlo Ersek,
Carsey, Jaben, Ruiyu Ni, Kinney, Michael D, Kelly Steele
Cc: Ard Biesheuvel
On 22 March 2017 at 14:04, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> To avoid the need for open coded GUID declarations that copy the FILE_GUID
> of the Shell app, add it to ShellPkg.dec and replace existing references.
>
Thanks all
I went ahead and pushed
2edc20c46863 ShellPkg: add GUID declaration for FILE_GUID of UEFI
Shell app to package
07548e17c5ff ArmPkg/PlatformBootManagerLib: refer to Shell FILE_GUID directly
c81c2c0fc453 ArmVirtPkg/ArmVirtQemu: refer to Shell app via its declared GUID
because I have some stuff queued up that depends on it.
> Ard Biesheuvel (4):
> ShellPkg: add GUID declaration for FILE_GUID of UEFI Shell app to
> package
> QuarkPlatformPkg/PlatformBootManagerLib: use new UefiShellFileGuid
> definition
> ArmPkg/PlatformBootManagerLib: refer to Shell FILE_GUID directly
> ArmVirtPkg/ArmVirtQemu: refer to Shell app via its declared GUID
>
> ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 5 ++---
> ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 3 ++-
> ArmVirtPkg/ArmVirtQemu.dsc | 1 -
> ArmVirtPkg/ArmVirtQemuKernel.dsc | 1 -
> ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c | 2 +-
> ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 6 +++---
> QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.c | 4 +---
> QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 2 ++
> ShellPkg/ShellPkg.dec | 3 +++
> 9 files changed, 14 insertions(+), 13 deletions(-)
>
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] ShellPkg: add GUID declaration for FILE_GUID of UEFI Shell app to package
2017-03-22 15:21 ` Carsey, Jaben
@ 2017-03-22 15:39 ` Kinney, Michael D
2017-03-22 15:41 ` Carsey, Jaben
2017-03-22 15:54 ` Andrew Fish
0 siblings, 2 replies; 15+ messages in thread
From: Kinney, Michael D @ 2017-03-22 15:39 UTC (permalink / raw)
To: Carsey, Jaben, Ard Biesheuvel, Kinney, Michael D
Cc: Ni, Ruiyu, edk2-devel@lists.01.org, leif.lindholm@linaro.org,
lersek@redhat.com
Jaben,
I like the added comment.
Maybe we should consider an INF spec enhancement to support a GUID C Name
for the FILE_GUID define, and the GUID C Names can be any GUID declared
in a dependent packages from the [Packages] section of the INF. This
would eliminate the GUID value duplication for this use case.
Mike
> -----Original Message-----
> From: Carsey, Jaben
> Sent: Wednesday, March 22, 2017 8:21 AM
> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org; leif.lindholm@linaro.org;
> Kinney, Michael D <michael.d.kinney@intel.com>; lersek@redhat.com; Carsey, Jaben
> <jaben.carsey@intel.com>
> Subject: RE: [edk2] [PATCH 1/4] ShellPkg: add GUID declaration for FILE_GUID of UEFI
> Shell app to package
>
> Yes. that looks great.
>
> For the changes to ShellPkg.
> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
>
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Ard Biesheuvel
> > Sent: Wednesday, March 22, 2017 8:20 AM
> > To: Carsey, Jaben <jaben.carsey@intel.com>
> > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org;
> > leif.lindholm@linaro.org; Kinney, Michael D <michael.d.kinney@intel.com>;
> > lersek@redhat.com
> > Subject: Re: [edk2] [PATCH 1/4] ShellPkg: add GUID declaration for
> > FILE_GUID of UEFI Shell app to package
> > Importance: High
> >
> > On 22 March 2017 at 15:13, Carsey, Jaben <jaben.carsey@intel.com> wrote:
> > > Ard,
> > >
> > > I am good with this change.
> > >
> > > What do you think about a comment to the INF file so that if someone
> > makes a change to the GUI there, they are informed that they need to
> > change this location? I worry as usually updating an INF GUID is permitted
> > without any need to change another file...
> > >
> >
> > Something like this perhaps?
> >
> > --- a/ShellPkg/Application/Shell/Shell.inf
> > +++ b/ShellPkg/Application/Shell/Shell.inf
> > @@ -17,7 +17,7 @@
> > [Defines]
> > INF_VERSION = 0x00010006
> > BASE_NAME = Shell
> > - FILE_GUID = 7C04A583-9E3E-4f1c-AD65-E05268D0B4D1
> > + FILE_GUID = 7C04A583-9E3E-4f1c-AD65-E05268D0B4D1 #
> > gUefiShellFileGuid
> > MODULE_TYPE = UEFI_APPLICATION
> > VERSION_STRING = 1.0
> > ENTRY_POINT = UefiMain
> >
> > (Note that the same FILE_GUID occurs in ShellBinPkg as well, but
> > people are unlikely that randomly change that one without regard to
> > the source build)
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/4] Shell/Quark/ArmPkg: promote shell app FILE_GUID to proper GUID
2017-03-22 15:38 ` [PATCH 0/4] Shell/Quark/ArmPkg: promote shell app FILE_GUID to proper GUID Ard Biesheuvel
@ 2017-03-22 15:39 ` Carsey, Jaben
0 siblings, 0 replies; 15+ messages in thread
From: Carsey, Jaben @ 2017-03-22 15:39 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel@lists.01.org, Leif Lindholm,
Laszlo Ersek, Ni, Ruiyu, Kinney, Michael D, Steele, Kelly
Cc: Carsey, Jaben
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Wednesday, March 22, 2017 8:38 AM
> To: edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Laszlo
> Ersek <lersek@redhat.com>; Carsey, Jaben <jaben.carsey@intel.com>; Ni,
> Ruiyu <ruiyu.ni@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Steele, Kelly <kelly.steele@intel.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Subject: Re: [edk2] [PATCH 0/4] Shell/Quark/ArmPkg: promote shell app
> FILE_GUID to proper GUID
> Importance: High
>
> On 22 March 2017 at 14:04, Ard Biesheuvel <ard.biesheuvel@linaro.org>
> wrote:
> > To avoid the need for open coded GUID declarations that copy the
> FILE_GUID
> > of the Shell app, add it to ShellPkg.dec and replace existing references.
> >
>
> Thanks all
>
> I went ahead and pushed
I was hoping you would since it was involved in the series. :)
>
> 2edc20c46863 ShellPkg: add GUID declaration for FILE_GUID of UEFI
> Shell app to package
> 07548e17c5ff ArmPkg/PlatformBootManagerLib: refer to Shell FILE_GUID
> directly
> c81c2c0fc453 ArmVirtPkg/ArmVirtQemu: refer to Shell app via its declared
> GUID
>
> because I have some stuff queued up that depends on it.
>
>
> > Ard Biesheuvel (4):
> > ShellPkg: add GUID declaration for FILE_GUID of UEFI Shell app to
> > package
> > QuarkPlatformPkg/PlatformBootManagerLib: use new UefiShellFileGuid
> > definition
> > ArmPkg/PlatformBootManagerLib: refer to Shell FILE_GUID directly
> > ArmVirtPkg/ArmVirtQemu: refer to Shell app via its declared GUID
> >
> > ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 5
> ++---
> > ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> | 3 ++-
> > ArmVirtPkg/ArmVirtQemu.dsc | 1 -
> > ArmVirtPkg/ArmVirtQemuKernel.dsc | 1 -
> > ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c | 2
> +-
> >
> ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> | 6 +++---
> >
> QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.
> c | 4 +---
> >
> QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManagerL
> ib.inf | 2 ++
> > ShellPkg/ShellPkg.dec | 3 +++
> > 9 files changed, 14 insertions(+), 13 deletions(-)
> >
> > --
> > 2.7.4
> >
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] ShellPkg: add GUID declaration for FILE_GUID of UEFI Shell app to package
2017-03-22 15:39 ` Kinney, Michael D
@ 2017-03-22 15:41 ` Carsey, Jaben
2017-03-22 15:54 ` Andrew Fish
1 sibling, 0 replies; 15+ messages in thread
From: Carsey, Jaben @ 2017-03-22 15:41 UTC (permalink / raw)
To: Kinney, Michael D, Ard Biesheuvel
Cc: Ni, Ruiyu, edk2-devel@lists.01.org, leif.lindholm@linaro.org,
lersek@redhat.com, Carsey, Jaben
That would be quite nice. I know that spinning GUIDs due to a non-backwards compatible change can be a scary thing.
-Jaben
> -----Original Message-----
> From: Kinney, Michael D
> Sent: Wednesday, March 22, 2017 8:40 AM
> To: Carsey, Jaben <jaben.carsey@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org;
> leif.lindholm@linaro.org; lersek@redhat.com
> Subject: RE: [edk2] [PATCH 1/4] ShellPkg: add GUID declaration for
> FILE_GUID of UEFI Shell app to package
> Importance: High
>
> Jaben,
>
> I like the added comment.
>
> Maybe we should consider an INF spec enhancement to support a GUID C
> Name
> for the FILE_GUID define, and the GUID C Names can be any GUID declared
> in a dependent packages from the [Packages] section of the INF. This
> would eliminate the GUID value duplication for this use case.
>
> Mike
>
> > -----Original Message-----
> > From: Carsey, Jaben
> > Sent: Wednesday, March 22, 2017 8:21 AM
> > To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org;
> leif.lindholm@linaro.org;
> > Kinney, Michael D <michael.d.kinney@intel.com>; lersek@redhat.com;
> Carsey, Jaben
> > <jaben.carsey@intel.com>
> > Subject: RE: [edk2] [PATCH 1/4] ShellPkg: add GUID declaration for
> FILE_GUID of UEFI
> > Shell app to package
> >
> > Yes. that looks great.
> >
> > For the changes to ShellPkg.
> > Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
> >
> > > -----Original Message-----
> > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > > Ard Biesheuvel
> > > Sent: Wednesday, March 22, 2017 8:20 AM
> > > To: Carsey, Jaben <jaben.carsey@intel.com>
> > > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org;
> > > leif.lindholm@linaro.org; Kinney, Michael D
> <michael.d.kinney@intel.com>;
> > > lersek@redhat.com
> > > Subject: Re: [edk2] [PATCH 1/4] ShellPkg: add GUID declaration for
> > > FILE_GUID of UEFI Shell app to package
> > > Importance: High
> > >
> > > On 22 March 2017 at 15:13, Carsey, Jaben <jaben.carsey@intel.com>
> wrote:
> > > > Ard,
> > > >
> > > > I am good with this change.
> > > >
> > > > What do you think about a comment to the INF file so that if someone
> > > makes a change to the GUI there, they are informed that they need to
> > > change this location? I worry as usually updating an INF GUID is
> permitted
> > > without any need to change another file...
> > > >
> > >
> > > Something like this perhaps?
> > >
> > > --- a/ShellPkg/Application/Shell/Shell.inf
> > > +++ b/ShellPkg/Application/Shell/Shell.inf
> > > @@ -17,7 +17,7 @@
> > > [Defines]
> > > INF_VERSION = 0x00010006
> > > BASE_NAME = Shell
> > > - FILE_GUID = 7C04A583-9E3E-4f1c-AD65-E05268D0B4D1
> > > + FILE_GUID = 7C04A583-9E3E-4f1c-AD65-E05268D0B4D1 #
> > > gUefiShellFileGuid
> > > MODULE_TYPE = UEFI_APPLICATION
> > > VERSION_STRING = 1.0
> > > ENTRY_POINT = UefiMain
> > >
> > > (Note that the same FILE_GUID occurs in ShellBinPkg as well, but
> > > people are unlikely that randomly change that one without regard to
> > > the source build)
> > > _______________________________________________
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] QuarkPlatformPkg/PlatformBootManagerLib: use new UefiShellFileGuid definition
2017-03-22 14:04 ` [PATCH 2/4] QuarkPlatformPkg/PlatformBootManagerLib: use new UefiShellFileGuid definition Ard Biesheuvel
@ 2017-03-22 15:43 ` Kinney, Michael D
0 siblings, 0 replies; 15+ messages in thread
From: Kinney, Michael D @ 2017-03-22 15:43 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel@lists.01.org, leif.lindholm@linaro.org,
lersek@redhat.com, Carsey, Jaben, Ni, Ruiyu, Steele, Kelly,
Kinney, Michael D
Reviewed-by: Michael Kinney <michael.d.kinney@intel.com>
Mike
> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Wednesday, March 22, 2017 7:04 AM
> To: edk2-devel@lists.01.org; leif.lindholm@linaro.org; lersek@redhat.com; Carsey,
> Jaben <jaben.carsey@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Steele, Kelly <kelly.steele@intel.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Subject: [PATCH 2/4] QuarkPlatformPkg/PlatformBootManagerLib: use new
> UefiShellFileGuid definition
>
> Move to the new definition of UefiShellFileGuid, which is defined in the
> ShellPkg package declaration file rather than hardcoded in this module.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.c | 4 +---
> QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 2 ++
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
> b/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
> index b61eb03360a2..3c213180690d 100644
> --- a/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
> +++ b/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
> @@ -15,8 +15,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
> IMPLIED.
>
> #include "PlatformBootManager.h"
>
> -EFI_GUID mUefiShellFileGuid = {0x7C04A583, 0x9E3E, 0x4f1c, {0xAD, 0x65, 0xE0, 0x52,
> 0x68, 0xD0, 0xB4, 0xD1 }};
> -
> /**
> Return the index of the load option in the load option array.
>
> @@ -246,7 +244,7 @@ PlatformBootManagerBeforeConsole (
> //
> // Register UEFI Shell
> //
> - PlatformRegisterFvBootOption (&mUefiShellFileGuid, L"UEFI Shell",
> LOAD_OPTION_ACTIVE);
> + PlatformRegisterFvBootOption (&gUefiShellFileGuid, L"UEFI Shell",
> LOAD_OPTION_ACTIVE);
>
> Status = gBS->LocateProtocol(&gEsrtManagementProtocolGuid, NULL, (VOID
> **)&EsrtManagement);
> if (EFI_ERROR(Status)) {
> diff --git
> a/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> b/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> index af399e529a92..25394d8ca000 100644
> --- a/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +++ b/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> @@ -40,6 +40,7 @@ [Packages]
> SourceLevelDebugPkg/SourceLevelDebugPkg.dec
> QuarkPlatformPkg/QuarkPlatformPkg.dec
> SecurityPkg/SecurityPkg.dec
> + ShellPkg/ShellPkg.dec
> SignedCapsulePkg/SignedCapsulePkg.dec
>
> [LibraryClasses]
> @@ -70,6 +71,7 @@ [Guids]
> gEfiVTUTF8Guid
> gEfiTtyTermGuid
> gEfiEndOfDxeEventGroupGuid
> + gUefiShellFileGuid
>
> [Pcd]
> gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate
> --
> 2.7.4
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] ShellPkg: add GUID declaration for FILE_GUID of UEFI Shell app to package
2017-03-22 15:39 ` Kinney, Michael D
2017-03-22 15:41 ` Carsey, Jaben
@ 2017-03-22 15:54 ` Andrew Fish
1 sibling, 0 replies; 15+ messages in thread
From: Andrew Fish @ 2017-03-22 15:54 UTC (permalink / raw)
To: Mike Kinney
Cc: Carsey, Jaben, Ard Biesheuvel, Ni, Ruiyu, edk2-devel@lists.01.org,
lersek@redhat.com, leif.lindholm@linaro.org
> On Mar 22, 2017, at 8:39 AM, Kinney, Michael D <michael.d.kinney@intel.com> wrote:
>
> Jaben,
>
> I like the added comment.
>
> Maybe we should consider an INF spec enhancement to support a GUID C Name
> for the FILE_GUID define, and the GUID C Names can be any GUID declared
> in a dependent packages from the [Packages] section of the INF. This
> would eliminate the GUID value duplication for this use case.
>
+1
Thanks,
Andrew Fish
> Mike
>
>> -----Original Message-----
>> From: Carsey, Jaben
>> Sent: Wednesday, March 22, 2017 8:21 AM
>> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org; leif.lindholm@linaro.org;
>> Kinney, Michael D <michael.d.kinney@intel.com>; lersek@redhat.com; Carsey, Jaben
>> <jaben.carsey@intel.com>
>> Subject: RE: [edk2] [PATCH 1/4] ShellPkg: add GUID declaration for FILE_GUID of UEFI
>> Shell app to package
>>
>> Yes. that looks great.
>>
>> For the changes to ShellPkg.
>> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
>>
>>> -----Original Message-----
>>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>>> Ard Biesheuvel
>>> Sent: Wednesday, March 22, 2017 8:20 AM
>>> To: Carsey, Jaben <jaben.carsey@intel.com>
>>> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org;
>>> leif.lindholm@linaro.org; Kinney, Michael D <michael.d.kinney@intel.com>;
>>> lersek@redhat.com
>>> Subject: Re: [edk2] [PATCH 1/4] ShellPkg: add GUID declaration for
>>> FILE_GUID of UEFI Shell app to package
>>> Importance: High
>>>
>>> On 22 March 2017 at 15:13, Carsey, Jaben <jaben.carsey@intel.com> wrote:
>>>> Ard,
>>>>
>>>> I am good with this change.
>>>>
>>>> What do you think about a comment to the INF file so that if someone
>>> makes a change to the GUI there, they are informed that they need to
>>> change this location? I worry as usually updating an INF GUID is permitted
>>> without any need to change another file...
>>>>
>>>
>>> Something like this perhaps?
>>>
>>> --- a/ShellPkg/Application/Shell/Shell.inf
>>> +++ b/ShellPkg/Application/Shell/Shell.inf
>>> @@ -17,7 +17,7 @@
>>> [Defines]
>>> INF_VERSION = 0x00010006
>>> BASE_NAME = Shell
>>> - FILE_GUID = 7C04A583-9E3E-4f1c-AD65-E05268D0B4D1
>>> + FILE_GUID = 7C04A583-9E3E-4f1c-AD65-E05268D0B4D1 #
>>> gUefiShellFileGuid
>>> MODULE_TYPE = UEFI_APPLICATION
>>> VERSION_STRING = 1.0
>>> ENTRY_POINT = UefiMain
>>>
>>> (Note that the same FILE_GUID occurs in ShellBinPkg as well, but
>>> people are unlikely that randomly change that one without regard to
>>> the source build)
>>> _______________________________________________
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org
>>> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2017-03-22 15:54 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-22 14:04 [PATCH 0/4] Shell/Quark/ArmPkg: promote shell app FILE_GUID to proper GUID Ard Biesheuvel
2017-03-22 14:04 ` [PATCH 1/4] ShellPkg: add GUID declaration for FILE_GUID of UEFI Shell app to package Ard Biesheuvel
2017-03-22 15:13 ` Carsey, Jaben
2017-03-22 15:19 ` Ard Biesheuvel
2017-03-22 15:21 ` Carsey, Jaben
2017-03-22 15:39 ` Kinney, Michael D
2017-03-22 15:41 ` Carsey, Jaben
2017-03-22 15:54 ` Andrew Fish
2017-03-22 14:04 ` [PATCH 2/4] QuarkPlatformPkg/PlatformBootManagerLib: use new UefiShellFileGuid definition Ard Biesheuvel
2017-03-22 15:43 ` Kinney, Michael D
2017-03-22 14:04 ` [PATCH 3/4] ArmPkg/PlatformBootManagerLib: refer to Shell FILE_GUID directly Ard Biesheuvel
2017-03-22 14:04 ` [PATCH 4/4] ArmVirtPkg/ArmVirtQemu: refer to Shell app via its declared GUID Ard Biesheuvel
2017-03-22 14:39 ` Laszlo Ersek
2017-03-22 15:38 ` [PATCH 0/4] Shell/Quark/ArmPkg: promote shell app FILE_GUID to proper GUID Ard Biesheuvel
2017-03-22 15:39 ` Carsey, Jaben
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox