[PATCH v4 0/2] Set Firmware Version from build command line

[PATCH v4 0/2] Set Firmware Version from build command line

[Oliver Steffen]

ArmVirtXen.dsc allows setting the firmware version
string in gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
via FIRMWARE_VER variable conveniently on the build command line.

These two patches add this behavior to all .dsc files in ArmVirtPkg
and OvmfPkg.

PR: https://github.com/tianocore/edk2/pull/4268

v4:
- Actually use the include file

v3:
- Introduce common include file for OVMF .dsc files
https://edk2.groups.io/g/devel/message/102952

v2:
Address comments by Sami and Gerd:
- Only set PCD if FIRMWARE_VER is defined
- ArmVirt: move logic to ArmVirt.dsc.inc
https://edk2.groups.io/g/devel/message/95208

v1:
https://edk2.groups.io/g/devel/message/95057

Oliver Steffen (2):
  OvmfPkg: allow setting Firmware Version from build command line
  ArmVirtPkg: allow setting Firmware Version from build command line

 ArmVirtPkg/ArmVirt.dsc.inc          | 4 ++++
 OvmfPkg/Include/Dsc/OvmfPkg.dsc.inc | 9 +++++++++
 ArmVirtPkg/ArmVirtXen.dsc           | 1 -
 OvmfPkg/OvmfPkgIa32.dsc             | 2 ++
 OvmfPkg/OvmfPkgIa32X64.dsc          | 2 ++
 OvmfPkg/OvmfPkgX64.dsc              | 3 +++
 OvmfPkg/OvmfXen.dsc                 | 2 ++
 7 files changed, 22 insertions(+), 1 deletion(-)
 create mode 100644 OvmfPkg/Include/Dsc/OvmfPkg.dsc.inc

-- 
2.40.0

[PATCH v4 1/2] OvmfPkg: allow setting Firmware Version from build command line

[Oliver Steffen]

Initialize gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString with
with the value of the variable "FIRMWARE_VER", if is is defined. Applies
to all flavors of OvmfPkg.

This behavior is already implemented in ArmVirtXen.dsc. It allows
specifying the firmware version string on the build command line with
-D FIRMARE_VER=...

Introduce a common include file to be used in the .dsc files for the
different OVMF flavors, and add the changes there.  (ArmVirtPkg already
has such a file).

Signed-off-by: Oliver Steffen <osteffen@redhat.com>
---
 OvmfPkg/Include/Dsc/OvmfPkg.dsc.inc | 9 +++++++++
 OvmfPkg/OvmfPkgIa32.dsc             | 2 ++
 OvmfPkg/OvmfPkgIa32X64.dsc          | 2 ++
 OvmfPkg/OvmfPkgX64.dsc              | 3 +++
 OvmfPkg/OvmfXen.dsc                 | 2 ++
 5 files changed, 18 insertions(+)
 create mode 100644 OvmfPkg/Include/Dsc/OvmfPkg.dsc.inc

diff --git a/OvmfPkg/Include/Dsc/OvmfPkg.dsc.inc b/OvmfPkg/Include/Dsc/OvmfPkg.dsc.inc
new file mode 100644
index 000000000000..f0bc73ede3df
--- /dev/null
+++ b/OvmfPkg/Include/Dsc/OvmfPkg.dsc.inc
@@ -0,0 +1,9 @@
+##
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[PcdsFixedAtBuild.common]
+!ifdef $(FIRMWARE_VER)
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(FIRMWARE_VER)"
+!endif
+
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 86177bb94899..0ee97c35b05c 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -78,6 +78,8 @@ [Defines]
 !endif
 !endif
 
+!include OvmfPkg/Include/Dsc/OvmfPkg.dsc.inc
+
 [BuildOptions]
   GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
   INTEL:RELEASE_*_*_CC_FLAGS           = /D MDEPKG_NDEBUG
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 065b54450647..19c197a4075c 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -77,6 +77,8 @@ [Defines]
 !endif
 !endif
 
+!include OvmfPkg/Include/Dsc/OvmfPkg.dsc.inc
+
 [BuildOptions]
   GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
   INTEL:RELEASE_*_*_CC_FLAGS           = /D MDEPKG_NDEBUG
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 3d405cd4ade0..0d8fb54a88d1 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -84,6 +84,8 @@ [Defines]
   DEFINE UP_CPU_PEI_GUID  = 280251c4-1d09-4035-9062-839acb5f18c1
   DEFINE UP_CPU_DXE_GUID  = 6490f1c5-ebcc-4665-8892-0075b9bb49b7
 
+!include OvmfPkg/Include/Dsc/OvmfPkg.dsc.inc
+
 [BuildOptions]
   GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
   INTEL:RELEASE_*_*_CC_FLAGS           = /D MDEPKG_NDEBUG
@@ -514,6 +516,7 @@ [PcdsFeatureFlag]
   gEfiMdeModulePkgTokenSpaceGuid.PcdRequireSelfSignedPk|TRUE
 !endif
 
+
 [PcdsFixedAtBuild]
   gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
 !if $(SMM_REQUIRE) == FALSE
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 8bfc16c2d3d6..1f44ec86c9c7 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -62,6 +62,8 @@ [Defines]
 !endif
 !endif
 
+!include OvmfPkg/Include/Dsc/OvmfPkg.dsc.inc
+
 [BuildOptions]
   GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
   INTEL:RELEASE_*_*_CC_FLAGS           = /D MDEPKG_NDEBUG
-- 
2.40.0

[PATCH v4 2/2] ArmVirtPkg: allow setting Firmware Version from build command line

[Oliver Steffen]

ArmVirtXen.dsc initializes
gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString with with the
value of the variable "FIRMWARE_VER".

Move that functionality to ArmVirt.dsc.inc to make it available to all
ArmVirt packages, and make it conditional: only set the PCD string if
FIRMWARE_VER is actually defined.

This allows specifying the firmware version string on the build command
line with -D FIRMARE_VER=...

Signed-off-by: Oliver Steffen <osteffen@redhat.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
---
 ArmVirtPkg/ArmVirt.dsc.inc | 4 ++++
 ArmVirtPkg/ArmVirtXen.dsc  | 1 -
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 5b18184be263..2443e8351c99 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -282,6 +282,10 @@ [PcdsFeatureFlag.AARCH64]
   gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
 
 [PcdsFixedAtBuild.common]
+!ifdef $(FIRMWARE_VER)
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(FIRMWARE_VER)"
+!endif
+
   gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength|1000000
   gEfiMdePkgTokenSpaceGuid.PcdMaximumAsciiStringLength|1000000
   gEfiMdePkgTokenSpaceGuid.PcdMaximumLinkedListLength|0
diff --git a/ArmVirtPkg/ArmVirtXen.dsc b/ArmVirtPkg/ArmVirtXen.dsc
index 58088af6ea3a..a280ae203bae 100644
--- a/ArmVirtPkg/ArmVirtXen.dsc
+++ b/ArmVirtPkg/ArmVirtXen.dsc
@@ -69,7 +69,6 @@ [BuildOptions]
 ################################################################################
 
 [PcdsFixedAtBuild.common]
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(FIRMWARE_VER)"
 !if $(ARCH) == AARCH64
   gArmTokenSpaceGuid.PcdVFPEnabled|1
 !endif
-- 
2.40.0

Re: [edk2-devel] [PATCH v4 0/2] Set Firmware Version from build command line

[Rebecca Cran]

Sorry for not commenting on this earlier.

Would it be better to have people use the build `--pcd` option instead? 
e.g. build 
--pcd="gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString=edk2-stable202302".


-- 

Rebecca Cran


On 4/14/23 02:33, Oliver Steffen wrote:
> ArmVirtXen.dsc allows setting the firmware version
> string in gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
> via FIRMWARE_VER variable conveniently on the build command line.
>
> These two patches add this behavior to all .dsc files in ArmVirtPkg
> and OvmfPkg.
>
> PR: https://github.com/tianocore/edk2/pull/4268
>
> v4:
> - Actually use the include file
>
> v3:
> - Introduce common include file for OVMF .dsc files
> https://edk2.groups.io/g/devel/message/102952
>
> v2:
> Address comments by Sami and Gerd:
> - Only set PCD if FIRMWARE_VER is defined
> - ArmVirt: move logic to ArmVirt.dsc.inc
> https://edk2.groups.io/g/devel/message/95208
>
> v1:
> https://edk2.groups.io/g/devel/message/95057
>
> Oliver Steffen (2):
>    OvmfPkg: allow setting Firmware Version from build command line
>    ArmVirtPkg: allow setting Firmware Version from build command line
>
>   ArmVirtPkg/ArmVirt.dsc.inc          | 4 ++++
>   OvmfPkg/Include/Dsc/OvmfPkg.dsc.inc | 9 +++++++++
>   ArmVirtPkg/ArmVirtXen.dsc           | 1 -
>   OvmfPkg/OvmfPkgIa32.dsc             | 2 ++
>   OvmfPkg/OvmfPkgIa32X64.dsc          | 2 ++
>   OvmfPkg/OvmfPkgX64.dsc              | 3 +++
>   OvmfPkg/OvmfXen.dsc                 | 2 ++
>   7 files changed, 22 insertions(+), 1 deletion(-)
>   create mode 100644 OvmfPkg/Include/Dsc/OvmfPkg.dsc.inc
>

Re: [edk2-devel] [PATCH v4 0/2] Set Firmware Version from build command line

[Gerd Hoffmann]

On Fri, Apr 14, 2023 at 06:13:03AM -0600, Rebecca Cran wrote:
> Sorry for not commenting on this earlier.
> 
> Would it be better to have people use the build `--pcd` option instead? e.g.
> build --pcd="gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString=edk2-stable202302".

Possible, but the syntax is not that simple, you need 'L' prefix (so
CHAR16 not CHAR8 is used) and '\0' postfix (so the string is properly
terminated), i.e. --pcd=PcdFirmwareVersionString="L${version}\\0".

Providing a shorter and less error prone way to specify the version
looks sensible to me.

take care,
  Gerd

Re: [edk2-devel] [PATCH v4 0/2] Set Firmware Version from build command line

[Rebecca Cran]

On 4/17/23 4:17 AM, Gerd Hoffmann wrote:
> On Fri, Apr 14, 2023 at 06:13:03AM -0600, Rebecca Cran wrote:
>> Sorry for not commenting on this earlier.
>>
>> Would it be better to have people use the build `--pcd` option instead? e.g.
>> build --pcd="gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString=edk2-stable202302".
> Possible, but the syntax is not that simple, you need 'L' prefix (so
> CHAR16 not CHAR8 is used) and '\0' postfix (so the string is properly
> terminated), i.e. --pcd=PcdFirmwareVersionString="L${version}\\0".
>
> Providing a shorter and less error prone way to specify the version
> looks sensible to me.

Agreed. Thanks.


-- 
Rebecca Cran

Re: [PATCH v4 0/2] Set Firmware Version from build command line

[Gerd Hoffmann]

Ping.

On Fri, Apr 14, 2023 at 10:33:21AM +0200, Oliver Steffen wrote:
> ArmVirtXen.dsc allows setting the firmware version
> string in gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
> via FIRMWARE_VER variable conveniently on the build command line.
> 
> These two patches add this behavior to all .dsc files in ArmVirtPkg
> and OvmfPkg.
> 
> PR: https://github.com/tianocore/edk2/pull/4268
> 
> v4:
> - Actually use the include file
> 
> v3:
> - Introduce common include file for OVMF .dsc files
> https://edk2.groups.io/g/devel/message/102952
> 
> v2:
> Address comments by Sami and Gerd:
> - Only set PCD if FIRMWARE_VER is defined
> - ArmVirt: move logic to ArmVirt.dsc.inc
> https://edk2.groups.io/g/devel/message/95208
> 
> v1:
> https://edk2.groups.io/g/devel/message/95057
> 
> Oliver Steffen (2):
>   OvmfPkg: allow setting Firmware Version from build command line
>   ArmVirtPkg: allow setting Firmware Version from build command line
> 
>  ArmVirtPkg/ArmVirt.dsc.inc          | 4 ++++
>  OvmfPkg/Include/Dsc/OvmfPkg.dsc.inc | 9 +++++++++
>  ArmVirtPkg/ArmVirtXen.dsc           | 1 -
>  OvmfPkg/OvmfPkgIa32.dsc             | 2 ++
>  OvmfPkg/OvmfPkgIa32X64.dsc          | 2 ++
>  OvmfPkg/OvmfPkgX64.dsc              | 3 +++
>  OvmfPkg/OvmfXen.dsc                 | 2 ++
>  7 files changed, 22 insertions(+), 1 deletion(-)
>  create mode 100644 OvmfPkg/Include/Dsc/OvmfPkg.dsc.inc
> 
> -- 
> 2.40.0
> 

-- 

Re: [edk2-devel] [PATCH v4 0/2] Set Firmware Version from build command line

[Ard Biesheuvel]

On Thu, 4 May 2023 at 12:13, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> Ping.
>

I'll pick these up - thanks for the reminder.


> On Fri, Apr 14, 2023 at 10:33:21AM +0200, Oliver Steffen wrote:
> > ArmVirtXen.dsc allows setting the firmware version
> > string in gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
> > via FIRMWARE_VER variable conveniently on the build command line.
> >
> > These two patches add this behavior to all .dsc files in ArmVirtPkg
> > and OvmfPkg.
> >
> > PR: https://github.com/tianocore/edk2/pull/4268
> >
> > v4:
> > - Actually use the include file
> >
> > v3:
> > - Introduce common include file for OVMF .dsc files
> > https://edk2.groups.io/g/devel/message/102952
> >
> > v2:
> > Address comments by Sami and Gerd:
> > - Only set PCD if FIRMWARE_VER is defined
> > - ArmVirt: move logic to ArmVirt.dsc.inc
> > https://edk2.groups.io/g/devel/message/95208
> >
> > v1:
> > https://edk2.groups.io/g/devel/message/95057
> >
> > Oliver Steffen (2):
> >   OvmfPkg: allow setting Firmware Version from build command line
> >   ArmVirtPkg: allow setting Firmware Version from build command line
> >
> >  ArmVirtPkg/ArmVirt.dsc.inc          | 4 ++++
> >  OvmfPkg/Include/Dsc/OvmfPkg.dsc.inc | 9 +++++++++
> >  ArmVirtPkg/ArmVirtXen.dsc           | 1 -
> >  OvmfPkg/OvmfPkgIa32.dsc             | 2 ++
> >  OvmfPkg/OvmfPkgIa32X64.dsc          | 2 ++
> >  OvmfPkg/OvmfPkgX64.dsc              | 3 +++
> >  OvmfPkg/OvmfXen.dsc                 | 2 ++
> >  7 files changed, 22 insertions(+), 1 deletion(-)
> >  create mode 100644 OvmfPkg/Include/Dsc/OvmfPkg.dsc.inc
> >
> > --
> > 2.40.0
> >
>
> --
>
>
>
> 
>
>