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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95098): https://edk2.groups.io/g/devel/message/95098
Mute This Topic: https://groups.io/mt/94276443/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
