public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 1/2] OvmfPkg: allow setting Firmware Version from build command line
       [not found] <20221012073504.511854-1-osteffen@redhat.com>
@ 2022-10-12  7:35 ` Oliver Steffen
  2022-10-12  7:35 ` [PATCH 2/2] ArmVirtPkg: " Oliver Steffen
  1 sibling, 0 replies; 6+ messages in thread
From: Oliver Steffen @ 2022-10-12  7:35 UTC (permalink / raw)
  To: devel
  Cc: Anthony Perard, Ard Biesheuvel, Gerd Hoffmann, Jian J Wang,
	Jiewen Yao, Jordan Justen, Julien Grall, Leif Lindholm,
	Liming Gao, Ray Ni, Sami Mujawar, Zhichao Gao, Pawel Polawski,
	Oliver Steffen

Initialize
gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
with with the value of the variable "FIRMWARE_VER"
in 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=...


Signed-off-by: Oliver Steffen <osteffen@redhat.com>
---
 OvmfPkg/OvmfPkgIa32.dsc    | 1 +
 OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
 OvmfPkg/OvmfPkgX64.dsc     | 1 +
 OvmfPkg/OvmfXen.dsc        | 1 +
 4 files changed, 4 insertions(+)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index e9ba491237ae..0a027577a673 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -488,6 +488,7 @@ [PcdsFeatureFlag]
 !endif

 [PcdsFixedAtBuild]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(FIRMWARE_VER)"
   gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
 !if $(SMM_REQUIRE) == FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index af566b953f36..70a310365100 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -494,6 +494,7 @@ [PcdsFeatureFlag]
 !endif

 [PcdsFixedAtBuild]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(FIRMWARE_VER)"
   gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
 !if $(SMM_REQUIRE) == FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index f39d9cd117e6..9e231278811f 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -514,6 +514,7 @@ [PcdsFeatureFlag]
 !endif

 [PcdsFixedAtBuild]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(FIRMWARE_VER)"
   gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
 !if $(SMM_REQUIRE) == FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 58a7c97cddf7..2a2987e637d5 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -367,6 +367,7 @@ [PcdsFeatureFlag]
 !endif

 [PcdsFixedAtBuild]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(FIRMWARE_VER)"
   gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration|TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
   gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] ArmVirtPkg: allow setting Firmware Version from build command line
       [not found] <20221012073504.511854-1-osteffen@redhat.com>
  2022-10-12  7:35 ` [PATCH 1/2] OvmfPkg: allow setting Firmware Version from build command line Oliver Steffen
@ 2022-10-12  7:35 ` Oliver Steffen
  2022-10-12 10:01   ` Sami Mujawar
  2022-10-12 10:39   ` Gerd Hoffmann
  1 sibling, 2 replies; 6+ messages in thread
From: Oliver Steffen @ 2022-10-12  7:35 UTC (permalink / raw)
  To: devel
  Cc: Anthony Perard, Ard Biesheuvel, Gerd Hoffmann, Jian J Wang,
	Jiewen Yao, Jordan Justen, Julien Grall, Leif Lindholm,
	Liming Gao, Ray Ni, Sami Mujawar, Zhichao Gao, Pawel Polawski,
	Oliver Steffen

Initialize
gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
with with the value of the variable "FIRMWARE_VER"
in all flavors of ArmVirtPkg.

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


Signed-off-by: Oliver Steffen <osteffen@redhat.com>
---
 ArmVirtPkg/ArmVirtCloudHv.dsc    | 1 +
 ArmVirtPkg/ArmVirtKvmTool.dsc    | 1 +
 ArmVirtPkg/ArmVirtQemu.dsc       | 1 +
 ArmVirtPkg/ArmVirtQemuKernel.dsc | 1 +
 4 files changed, 4 insertions(+)

diff --git a/ArmVirtPkg/ArmVirtCloudHv.dsc b/ArmVirtPkg/ArmVirtCloudHv.dsc
index 7ca7a391d9cf..7e3aa84cd321 100644
--- a/ArmVirtPkg/ArmVirtCloudHv.dsc
+++ b/ArmVirtPkg/ArmVirtCloudHv.dsc
@@ -86,6 +86,7 @@ [PcdsFeatureFlag.common]
   gEfiMdeModulePkgTokenSpaceGuid.PcdTurnOffUsbLegacySupport|TRUE

 [PcdsFixedAtBuild.common]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(FIRMWARE_VER)"
 !if $(ARCH) == AARCH64
   gArmTokenSpaceGuid.PcdVFPEnabled|1
 !endif
diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
index 3bd3ebd6e0b3..e07ed6fc4ca8 100644
--- a/ArmVirtPkg/ArmVirtKvmTool.dsc
+++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
@@ -113,6 +113,7 @@ [PcdsFeatureFlag.common]
   gPcAtChipsetPkgTokenSpaceGuid.PcdRtcUseMmio|TRUE

 [PcdsFixedAtBuild.common]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(FIRMWARE_VER)"
   gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8000000F

   gArmPlatformTokenSpaceGuid.PcdCoreCount|1
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 9369a88858fd..4963b2006165 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -135,6 +135,7 @@ [PcdsFeatureFlag.common]
   gArmVirtTokenSpaceGuid.PcdTpm2SupportEnabled|$(TPM2_ENABLE)

 [PcdsFixedAtBuild.common]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(FIRMWARE_VER)"
 !if $(ARCH) == AARCH64
   gArmTokenSpaceGuid.PcdVFPEnabled|1
 !endif
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index 7f7d15d6eee3..8454be6c2bfd 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -115,6 +115,7 @@ [PcdsFeatureFlag.common]
   gEfiMdeModulePkgTokenSpaceGuid.PcdTurnOffUsbLegacySupport|TRUE

 [PcdsFixedAtBuild.common]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(FIRMWARE_VER)"
 !if $(ARCH) == AARCH64
   gArmTokenSpaceGuid.PcdVFPEnabled|1
 !endif
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] ArmVirtPkg: allow setting Firmware Version from build command line
  2022-10-12  7:35 ` [PATCH 2/2] ArmVirtPkg: " Oliver Steffen
@ 2022-10-12 10:01   ` Sami Mujawar
  2022-10-12 10:39   ` Gerd Hoffmann
  1 sibling, 0 replies; 6+ messages in thread
From: Sami Mujawar @ 2022-10-12 10:01 UTC (permalink / raw)
  To: Oliver Steffen, devel
  Cc: Anthony Perard, Ard Biesheuvel, Gerd Hoffmann, Jian J Wang,
	Jiewen Yao, Jordan Justen, Julien Grall, Leif Lindholm,
	Liming Gao, Ray Ni, Zhichao Gao, Pawel Polawski, nd@arm.com

Hi Oliver,

Thank you for this patch.

These changes look good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 12/10/2022 08:35 am, Oliver Steffen wrote:
> Initialize
> gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
> with with the value of the variable "FIRMWARE_VER"
> in all flavors of ArmVirtPkg.
>
> This behavior is already implemented in ArmVirtXen.dsc.
> It allows specifying the firmware version string on the
> build command line with -D FIRMARE_VER=...
>
>
> Signed-off-by: Oliver Steffen <osteffen@redhat.com>
> ---
>   ArmVirtPkg/ArmVirtCloudHv.dsc    | 1 +
>   ArmVirtPkg/ArmVirtKvmTool.dsc    | 1 +
>   ArmVirtPkg/ArmVirtQemu.dsc       | 1 +
>   ArmVirtPkg/ArmVirtQemuKernel.dsc | 1 +
>   4 files changed, 4 insertions(+)
>
> diff --git a/ArmVirtPkg/ArmVirtCloudHv.dsc b/ArmVirtPkg/ArmVirtCloudHv.dsc
> index 7ca7a391d9cf..7e3aa84cd321 100644
> --- a/ArmVirtPkg/ArmVirtCloudHv.dsc
> +++ b/ArmVirtPkg/ArmVirtCloudHv.dsc
> @@ -86,6 +86,7 @@ [PcdsFeatureFlag.common]
>     gEfiMdeModulePkgTokenSpaceGuid.PcdTurnOffUsbLegacySupport|TRUE
>
>   [PcdsFixedAtBuild.common]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(FIRMWARE_VER)"
>   !if $(ARCH) == AARCH64
>     gArmTokenSpaceGuid.PcdVFPEnabled|1
>   !endif
> diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
> index 3bd3ebd6e0b3..e07ed6fc4ca8 100644
> --- a/ArmVirtPkg/ArmVirtKvmTool.dsc
> +++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
> @@ -113,6 +113,7 @@ [PcdsFeatureFlag.common]
>     gPcAtChipsetPkgTokenSpaceGuid.PcdRtcUseMmio|TRUE
>
>   [PcdsFixedAtBuild.common]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(FIRMWARE_VER)"
>     gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8000000F
>
>     gArmPlatformTokenSpaceGuid.PcdCoreCount|1
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index 9369a88858fd..4963b2006165 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -135,6 +135,7 @@ [PcdsFeatureFlag.common]
>     gArmVirtTokenSpaceGuid.PcdTpm2SupportEnabled|$(TPM2_ENABLE)
>
>   [PcdsFixedAtBuild.common]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(FIRMWARE_VER)"
>   !if $(ARCH) == AARCH64
>     gArmTokenSpaceGuid.PcdVFPEnabled|1
>   !endif
> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> index 7f7d15d6eee3..8454be6c2bfd 100644
> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> @@ -115,6 +115,7 @@ [PcdsFeatureFlag.common]
>     gEfiMdeModulePkgTokenSpaceGuid.PcdTurnOffUsbLegacySupport|TRUE
>
>   [PcdsFixedAtBuild.common]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(FIRMWARE_VER)"
>   !if $(ARCH) == AARCH64
>     gArmTokenSpaceGuid.PcdVFPEnabled|1
>   !endif

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] ArmVirtPkg: allow setting Firmware Version from build command line
  2022-10-12  7:35 ` [PATCH 2/2] ArmVirtPkg: " Oliver Steffen
  2022-10-12 10:01   ` Sami Mujawar
@ 2022-10-12 10:39   ` Gerd Hoffmann
  2022-10-12 11:06     ` Sami Mujawar
  1 sibling, 1 reply; 6+ messages in thread
From: Gerd Hoffmann @ 2022-10-12 10:39 UTC (permalink / raw)
  To: Oliver Steffen
  Cc: devel, Anthony Perard, Ard Biesheuvel, Jian J Wang, Jiewen Yao,
	Jordan Justen, Julien Grall, Leif Lindholm, Liming Gao, Ray Ni,
	Sami Mujawar, Zhichao Gao, Pawel Polawski

On Wed, Oct 12, 2022 at 07:35:23AM +0000, Oliver Steffen wrote:
> Initialize
> gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
> with with the value of the variable "FIRMWARE_VER"
> in all flavors of ArmVirtPkg.
> 
> This behavior is already implemented in ArmVirtXen.dsc.
> It allows specifying the firmware version string on the
> build command line with -D FIRMARE_VER=...

I think we need to decide which approach we want support for
setting PcdFirmwareVersionString (and write it down in armvirt/ovmf
readme).

The options we have are:

  (1) -D "FIRMARE_VER=${version}" (needs this patch), or
  (2) --pcd "PcdFirmwareVersionString=L'${version}\\0'"

Advantage of (1) is that the build command line is a bit simpler.

Disadvantage of (1) is that it overrides PcdFirmwareVersionString
even in case FIRMARE_VER is not set on the command line.  Which doesn't
make much of a difference today because the default value defined in
MdeModulePkg is just the empty string.  In case we set the default
to something more useful (https://edk2.groups.io/g/devel/message/94985)
overriding it is not so nice though ...

take care,
  Gerd


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] ArmVirtPkg: allow setting Firmware Version from build command line
  2022-10-12 10:39   ` Gerd Hoffmann
@ 2022-10-12 11:06     ` Sami Mujawar
  2022-10-12 11:18       ` Gerd Hoffmann
  0 siblings, 1 reply; 6+ messages in thread
From: Sami Mujawar @ 2022-10-12 11:06 UTC (permalink / raw)
  To: Gerd Hoffmann, Oliver Steffen
  Cc: devel, Anthony Perard, Ard Biesheuvel, Jian J Wang, Jiewen Yao,
	Jordan Justen, Julien Grall, Leif Lindholm, Liming Gao, Ray Ni,
	Zhichao Gao, Pawel Polawski, nd@arm.com

Hi Gerd,

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar

On 12/10/2022 11:39 am, Gerd Hoffmann wrote:
> On Wed, Oct 12, 2022 at 07:35:23AM +0000, Oliver Steffen wrote:
>> Initialize
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
>> with with the value of the variable "FIRMWARE_VER"
>> in all flavors of ArmVirtPkg.
>>
>> This behavior is already implemented in ArmVirtXen.dsc.
>> It allows specifying the firmware version string on the
>> build command line with -D FIRMARE_VER=...
> I think we need to decide which approach we want support for
> setting PcdFirmwareVersionString (and write it down in armvirt/ovmf
> readme).
>
> The options we have are:
>
>    (1) -D "FIRMARE_VER=${version}" (needs this patch), or
>    (2) --pcd "PcdFirmwareVersionString=L'${version}\\0'"
>
> Advantage of (1) is that the build command line is a bit simpler.
>
> Disadvantage of (1) is that it overrides PcdFirmwareVersionString
> even in case FIRMARE_VER is not set on the command line.  Which doesn't
> make much of a difference today because the default value defined in
> MdeModulePkg is just the empty string.  In case we set the default
> to something more useful (https://edk2.groups.io/g/devel/message/94985)
> overriding it is not so nice though ...

[SAMI] Thank you for pointing me to the MdeModulePkg patch.

I think we could do the following in ArmVirtPkg:

-----

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index c39e2506a3ea..49e96c9fb91c 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -289,6 +289,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

----

With this if -D "FIRMARE_VER=${version}" is not provided, the default 
string definition from MdeModulePkg would be used.

Also, since ArmVirt.dsc.inc is included all the platforms in ArmVirtPkg, 
this change is required only in one place (ArmVirtXen.dsc would need 
updating though).

[/SAMI]

>
> take care,
>    Gerd
>

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] ArmVirtPkg: allow setting Firmware Version from build command line
  2022-10-12 11:06     ` Sami Mujawar
@ 2022-10-12 11:18       ` Gerd Hoffmann
  0 siblings, 0 replies; 6+ messages in thread
From: Gerd Hoffmann @ 2022-10-12 11:18 UTC (permalink / raw)
  To: Sami Mujawar
  Cc: Oliver Steffen, devel, Anthony Perard, Ard Biesheuvel,
	Jian J Wang, Jiewen Yao, Jordan Justen, Julien Grall,
	Leif Lindholm, Liming Gao, Ray Ni, Zhichao Gao, Pawel Polawski,
	nd@arm.com

  Hi,

> I think we could do the following in ArmVirtPkg:
> 
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc

>  [PcdsFixedAtBuild.common]
> +!ifdef $(FIRMWARE_VER)
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(FIRMWARE_VER)"
> +!endif

Yes, that idea looks good to me.

take care,
  Gerd


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-10-12 11:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20221012073504.511854-1-osteffen@redhat.com>
2022-10-12  7:35 ` [PATCH 1/2] OvmfPkg: allow setting Firmware Version from build command line Oliver Steffen
2022-10-12  7:35 ` [PATCH 2/2] ArmVirtPkg: " Oliver Steffen
2022-10-12 10:01   ` Sami Mujawar
2022-10-12 10:39   ` Gerd Hoffmann
2022-10-12 11:06     ` Sami Mujawar
2022-10-12 11:18       ` Gerd Hoffmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox