* [PATCH 1/1] OvmfPkg/SmbiosPlatformDxe: use PcdFirmwareVersionString
@ 2022-10-10 14:42 Gerd Hoffmann
2022-10-10 16:27 ` [edk2-devel] " Rebecca Cran
0 siblings, 1 reply; 6+ messages in thread
From: Gerd Hoffmann @ 2022-10-10 14:42 UTC (permalink / raw)
To: devel
Cc: Gerd Hoffmann, Jordan Justen, Julien Grall, Oliver Steffen,
Pawel Polawski, Ard Biesheuvel, Anthony Perard, Jiewen Yao
Instead of using hard-coded string "0.0.0" for BiosVersion (which is
quite useless) read PcdFirmwareVersionString and append that to the
type0 entry string table.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
.../SmbiosPlatformDxe/SmbiosPlatformDxe.inf | 4 +++
.../XenSmbiosPlatformDxe.inf | 7 +++-
OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 32 ++++++++++++++++---
3 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
index 0066bbc9229c..1a0a032342f9 100644
--- a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
+++ b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
@@ -32,9 +32,12 @@ [Sources]
[Packages]
MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
OvmfPkg/OvmfPkg.dec
[LibraryClasses]
+ BaseLib
+ BaseMemoryLib
DebugLib
MemoryAllocationLib
PcdLib
@@ -45,6 +48,7 @@ [LibraryClasses]
[Pcd]
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
[Protocols]
gEfiSmbiosProtocolGuid # PROTOCOL ALWAYS_CONSUMED
diff --git a/OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.inf b/OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.inf
index 7f4588e33d1e..73c78ec7cc73 100644
--- a/OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.inf
+++ b/OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.inf
@@ -38,19 +38,24 @@ [Sources.ARM, Sources.AARCH64]
[Packages]
MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
[Packages.IA32, Packages.X64]
OvmfPkg/OvmfPkg.dec
[LibraryClasses]
+ BaseLib
+ BaseMemoryLib
DebugLib
UefiBootServicesTableLib
UefiDriverEntryPoint
[LibraryClasses.IA32, LibraryClasses.X64]
- BaseLib
HobLib
+[Pcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
+
[Protocols]
gEfiSmbiosProtocolGuid # PROTOCOL ALWAYS_CONSUMED
diff --git a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
index 94249d3ff1b0..f2eb568b6964 100644
--- a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
+++ b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
@@ -9,7 +9,11 @@
**/
#include <IndustryStandard/SmBios.h> // SMBIOS_TABLE_TYPE0
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
#include <Library/DebugLib.h> // ASSERT_EFI_ERROR()
+#include <Library/MemoryAllocationLib.h>
+#include <Library/PcdLib.h>
#include <Library/UefiBootServicesTableLib.h> // gBS
#include <Protocol/Smbios.h> // EFI_SMBIOS_PROTOCOL
@@ -17,8 +21,7 @@
#define TYPE0_STRINGS \
"EFI Development Kit II / OVMF\0" /* Vendor */ \
- "0.0.0\0" /* BiosVersion */ \
- "02/06/2015\0" /* BiosReleaseDate */
+ "02/06/2015" /* BiosReleaseDate */
//
// Type definition and contents of the default Type 0 SMBIOS table.
//
@@ -37,9 +40,9 @@ STATIC CONST OVMF_TYPE0 mOvmfDefaultType0 = {
sizeof (SMBIOS_TABLE_TYPE0), // UINT8 Length
},
1, // SMBIOS_TABLE_STRING Vendor
- 2, // SMBIOS_TABLE_STRING BiosVersion
+ 3, // SMBIOS_TABLE_STRING BiosVersion
0xE800, // UINT16 BiosSegment
- 3, // SMBIOS_TABLE_STRING BiosReleaseDate
+ 2, // SMBIOS_TABLE_STRING BiosReleaseDate
0, // UINT8 BiosSize
{ // MISC_BIOS_CHARACTERISTICS BiosCharacteristics
0, // Reserved :2
@@ -153,14 +156,33 @@ InstallAllStructures (
//
// Add OVMF default Type 0 (BIOS Information) table
//
+ CHAR16 *Str16;
+ UINTN Chars;
+ CHAR8 *Type0;
+
+ Str16 = (CHAR16 *)FixedPcdGetPtr (PcdFirmwareVersionString);
+ Chars = StrLen (Str16);
+ if (Chars == 0) {
+ Str16 = L"unknown";
+ Chars = StrLen (Str16);
+ }
+
+ DEBUG ((DEBUG_INFO, "FirmwareVersionString: \"%s\" (%d chars)\n", Str16, Chars));
+
+ Type0 = AllocateZeroPool (sizeof (mOvmfDefaultType0) + Chars + 2);
+ CopyMem (Type0, &mOvmfDefaultType0, sizeof (mOvmfDefaultType0));
+ UnicodeStrToAsciiStrS (Str16, Type0 + sizeof (mOvmfDefaultType0), Chars + 1);
+
SmbiosHandle = SMBIOS_HANDLE_PI_RESERVED;
Status = Smbios->Add (
Smbios,
NULL,
&SmbiosHandle,
- (EFI_SMBIOS_TABLE_HEADER *)&mOvmfDefaultType0
+ (EFI_SMBIOS_TABLE_HEADER *)Type0
);
ASSERT_EFI_ERROR (Status);
+
+ FreePool (Type0);
}
return EFI_SUCCESS;
--
2.37.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] OvmfPkg/SmbiosPlatformDxe: use PcdFirmwareVersionString
2022-10-10 14:42 [PATCH 1/1] OvmfPkg/SmbiosPlatformDxe: use PcdFirmwareVersionString Gerd Hoffmann
@ 2022-10-10 16:27 ` Rebecca Cran
2022-10-11 7:59 ` Gerd Hoffmann
0 siblings, 1 reply; 6+ messages in thread
From: Rebecca Cran @ 2022-10-10 16:27 UTC (permalink / raw)
To: devel, kraxel
Cc: Jordan Justen, Julien Grall, Oliver Steffen, Pawel Polawski,
Ard Biesheuvel, Anthony Perard, Jiewen Yao
[-- Attachment #1: Type: text/plain, Size: 883 bytes --]
On 10/10/2022 8:42 AM, Gerd Hoffmann wrote:
> Instead of using hard-coded string "0.0.0" for BiosVersion (which is
> quite useless) read PcdFirmwareVersionString and append that to the
> type0 entry string table.
>
> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
> ---
>
> #define TYPE0_STRINGS \
> "EFI Development Kit II / OVMF\0" /* Vendor */ \
> - "0.0.0\0" /* BiosVersion */ \
> - "02/06/2015\0" /* BiosReleaseDate */
> + "02/06/2015" /* BiosReleaseDate */
I know this is unrelated to this patch, but should we update the release date at some point?
> + DEBUG ((DEBUG_INFO, "FirmwareVersionString: \"%s\" (%d chars)\n", Str16, Chars));
> +
> + Type0 = AllocateZeroPool (sizeof (mOvmfDefaultType0) + Chars + 2);
Should we check for an allocation failure here?
--
Rebecca Cran
[-- Attachment #2: Type: text/html, Size: 1469 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] OvmfPkg/SmbiosPlatformDxe: use PcdFirmwareVersionString
2022-10-10 16:27 ` [edk2-devel] " Rebecca Cran
@ 2022-10-11 7:59 ` Gerd Hoffmann
2022-10-11 15:34 ` Rebecca Cran
0 siblings, 1 reply; 6+ messages in thread
From: Gerd Hoffmann @ 2022-10-11 7:59 UTC (permalink / raw)
To: Rebecca Cran
Cc: devel, Jordan Justen, Julien Grall, Oliver Steffen,
Pawel Polawski, Ard Biesheuvel, Anthony Perard, Jiewen Yao
On Mon, Oct 10, 2022 at 10:27:39AM -0600, Rebecca Cran wrote:
> On 10/10/2022 8:42 AM, Gerd Hoffmann wrote:
>
> > Instead of using hard-coded string "0.0.0" for BiosVersion (which is
> > quite useless) read PcdFirmwareVersionString and append that to the
> > type0 entry string table.
> >
> > Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
> > ---
> >
> > #define TYPE0_STRINGS \
> > "EFI Development Kit II / OVMF\0" /* Vendor */ \
> > - "0.0.0\0" /* BiosVersion */ \
> > - "02/06/2015\0" /* BiosReleaseDate */
> > + "02/06/2015" /* BiosReleaseDate */
>
> I know this is unrelated to this patch, but should we update the release date at some point?
I saw we also have PcdFirmwareVendor + PcdFirmwareReleaseDateString.
So I guess it makes sense to just generate the whole string table
dynamically.
Next question is how to set them. I think it makes sense to have some
sensible defaults, but still allow to override them. MdeModulePkg
defines them to empty strings (except vendor). Should we set them to
the most recent stable tag instead, i.e. something like this?
- gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L""|VOID*|0x00010052
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"edk2-stable202208"|VOID*|0x00010052
- gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareReleaseDateString|L""|VOID*|0x00010053
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareReleaseDateString|L"26/08/2022"|VOID*|0x00010053
When doing that: Can this be overridden on the command line? Trying to
do so using 'build --pcd PcdFirmwareVersionString=Test' didn't work for
me, the string wasn't translated to unicode ...
I've noticed ArmVirtPkg/ArmVirtXen.dsc has this line ...
gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(FIRMWARE_VER)"
... which allows to override using 'build -D FIRMWARE_VER=Test'.
Unicode encoding works that way, but it would also override the
MdeModulePkg default (if we add one).
> > + DEBUG ((DEBUG_INFO, "FirmwareVersionString: \"%s\" (%d chars)\n", Str16, Chars));
> > +
> > + Type0 = AllocateZeroPool (sizeof (mOvmfDefaultType0) + Chars + 2);
>
> Should we check for an allocation failure here?
Yes, fixed.
take care,
Gerd
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] OvmfPkg/SmbiosPlatformDxe: use PcdFirmwareVersionString
2022-10-11 7:59 ` Gerd Hoffmann
@ 2022-10-11 15:34 ` Rebecca Cran
2022-10-12 8:52 ` Gerd Hoffmann
0 siblings, 1 reply; 6+ messages in thread
From: Rebecca Cran @ 2022-10-11 15:34 UTC (permalink / raw)
To: devel, kraxel, Rebecca Cran
Cc: Jordan Justen, Julien Grall, Oliver Steffen, Pawel Polawski,
Ard Biesheuvel, Anthony Perard, Jiewen Yao
On 10/11/22 01:59, Gerd Hoffmann wrote:
>
> Next question is how to set them. I think it makes sense to have some
> sensible defaults, but still allow to override them. MdeModulePkg
> defines them to empty strings (except vendor). Should we set them to
> the most recent stable tag instead, i.e. something like this?
>
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L""|VOID*|0x00010052
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"edk2-stable202208"|VOID*|0x00010052
>
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareReleaseDateString|L""|VOID*|0x00010053
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareReleaseDateString|L"26/08/2022"|VOID*|0x00010053
>
> When doing that: Can this be overridden on the command line? Trying to
> do so using 'build --pcd PcdFirmwareVersionString=Test' didn't work for
> me, the string wasn't translated to unicode ...
>
> I've noticed ArmVirtPkg/ArmVirtXen.dsc has this line ...
>
> gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(FIRMWARE_VER)"
>
> ... which allows to override using 'build -D FIRMWARE_VER=Test'.
> Unicode encoding works that way, but it would also override the
> MdeModulePkg default (if we add one).
The method I've used in the past is to override strings on the command
line, just like ArmVirtXen.dsc does, and I like that approach.
I like the idea of defaulting to the stable tag, though we could perhaps
shorten it to "202208" instead? I'm fairly sure the release date should
be in MM/DD/YYYY format for e.g. SMBIOS compatibility (sigh) - though
I've just realized that for SMBIOS we ignore that PCD and just use
__DATE__ and __TIME__.
--
Rebecca Cran
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] OvmfPkg/SmbiosPlatformDxe: use PcdFirmwareVersionString
2022-10-11 15:34 ` Rebecca Cran
@ 2022-10-12 8:52 ` Gerd Hoffmann
2022-10-24 14:03 ` Ard Biesheuvel
0 siblings, 1 reply; 6+ messages in thread
From: Gerd Hoffmann @ 2022-10-12 8:52 UTC (permalink / raw)
To: Rebecca Cran
Cc: devel, Rebecca Cran, Jordan Justen, Julien Grall, Oliver Steffen,
Pawel Polawski, Ard Biesheuvel, Anthony Perard, Jiewen Yao
On Tue, Oct 11, 2022 at 09:34:17AM -0600, Rebecca Cran wrote:
> On 10/11/22 01:59, Gerd Hoffmann wrote:
> >
> > Next question is how to set them. I think it makes sense to have some
> > sensible defaults, but still allow to override them. MdeModulePkg
> > defines them to empty strings (except vendor). Should we set them to
> > the most recent stable tag instead, i.e. something like this?
> >
> > - gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L""|VOID*|0x00010052
> > + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"edk2-stable202208"|VOID*|0x00010052
> >
> > - gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareReleaseDateString|L""|VOID*|0x00010053
> > + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareReleaseDateString|L"26/08/2022"|VOID*|0x00010053
> >
> > When doing that: Can this be overridden on the command line? Trying to
> > do so using 'build --pcd PcdFirmwareVersionString=Test' didn't work for
> > me, the string wasn't translated to unicode ...
> >
> > I've noticed ArmVirtPkg/ArmVirtXen.dsc has this line ...
> >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(FIRMWARE_VER)"
> >
> > ... which allows to override using 'build -D FIRMWARE_VER=Test'.
> > Unicode encoding works that way, but it would also override the
> > MdeModulePkg default (if we add one).
>
> The method I've used in the past is to override strings on the command line,
> just like ArmVirtXen.dsc does, and I like that approach.
After digging around in the source code and experimenting a bit
I figured how to do it without the FIRMWARE_VER indirection:
build --pcd="PcdFirmwareVersionString=L'${version}\\0'"
> I like the idea of defaulting to the stable tag, though we could perhaps
> shorten it to "202208" instead?
I'd prefer to keep it identical to the stable tag name.
Makes it easier to figure where this comes from.
> I'm fairly sure the release date should be
> in MM/DD/YYYY format for e.g. SMBIOS compatibility (sigh)
Hmm. At least we tag stable releases close to the end of the month
(after 12th), so it should be clear what is DD and what is MM ...
take care,
Gerd
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] OvmfPkg/SmbiosPlatformDxe: use PcdFirmwareVersionString
2022-10-12 8:52 ` Gerd Hoffmann
@ 2022-10-24 14:03 ` Ard Biesheuvel
0 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2022-10-24 14:03 UTC (permalink / raw)
To: devel, kraxel
Cc: Rebecca Cran, Rebecca Cran, Jordan Justen, Julien Grall,
Oliver Steffen, Pawel Polawski, Anthony Perard, Jiewen Yao
On Wed, 12 Oct 2022 at 10:52, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> On Tue, Oct 11, 2022 at 09:34:17AM -0600, Rebecca Cran wrote:
> > On 10/11/22 01:59, Gerd Hoffmann wrote:
> > >
> > > Next question is how to set them. I think it makes sense to have some
> > > sensible defaults, but still allow to override them. MdeModulePkg
> > > defines them to empty strings (except vendor). Should we set them to
> > > the most recent stable tag instead, i.e. something like this?
> > >
> > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L""|VOID*|0x00010052
> > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"edk2-stable202208"|VOID*|0x00010052
> > >
> > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareReleaseDateString|L""|VOID*|0x00010053
> > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareReleaseDateString|L"26/08/2022"|VOID*|0x00010053
> > >
> > > When doing that: Can this be overridden on the command line? Trying to
> > > do so using 'build --pcd PcdFirmwareVersionString=Test' didn't work for
> > > me, the string wasn't translated to unicode ...
> > >
> > > I've noticed ArmVirtPkg/ArmVirtXen.dsc has this line ...
> > >
> > > gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(FIRMWARE_VER)"
> > >
> > > ... which allows to override using 'build -D FIRMWARE_VER=Test'.
> > > Unicode encoding works that way, but it would also override the
> > > MdeModulePkg default (if we add one).
> >
> > The method I've used in the past is to override strings on the command line,
> > just like ArmVirtXen.dsc does, and I like that approach.
>
> After digging around in the source code and experimenting a bit
> I figured how to do it without the FIRMWARE_VER indirection:
>
> build --pcd="PcdFirmwareVersionString=L'${version}\\0'"
>
> > I like the idea of defaulting to the stable tag, though we could perhaps
> > shorten it to "202208" instead?
>
> I'd prefer to keep it identical to the stable tag name.
> Makes it easier to figure where this comes from.
>
> > I'm fairly sure the release date should be
> > in MM/DD/YYYY format for e.g. SMBIOS compatibility (sigh)
>
> Hmm. At least we tag stable releases close to the end of the month
> (after 12th), so it should be clear what is DD and what is MM ...
>
Is there a conclusion here?
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-10-24 14:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-10 14:42 [PATCH 1/1] OvmfPkg/SmbiosPlatformDxe: use PcdFirmwareVersionString Gerd Hoffmann
2022-10-10 16:27 ` [edk2-devel] " Rebecca Cran
2022-10-11 7:59 ` Gerd Hoffmann
2022-10-11 15:34 ` Rebecca Cran
2022-10-12 8:52 ` Gerd Hoffmann
2022-10-24 14:03 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox