From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mx.groups.io with SMTP id smtpd.web10.4730.1665475198277151365 for ; Tue, 11 Oct 2022 00:59:58 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=YORBS10M; spf=pass (domain: redhat.com, ip: 170.10.129.124, mailfrom: kraxel@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1665475196; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=2+eHVO10ZqZAMyiK860p+qcKaDEtykFIVJA+IRI/MOM=; b=YORBS10MFtO+oPWeegsd1kVukniGxXq+dN83YmVepHebop23Ka6ZLLohEg2svthD+rE31K Z2YDxHop+iK8gcWVhHZlNAc/X4Xo4JNLU1nWlXX0GXHrKBUc6/9gX+rYQm91JlsWjC7CqZ +K/9jcAZdOWaPiaJqvtQwrOKIWnp7kY= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-563-48ihQst9Nu-O_nnRlJ4x7Q-1; Tue, 11 Oct 2022 03:59:55 -0400 X-MC-Unique: 48ihQst9Nu-O_nnRlJ4x7Q-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id D4968185A79C; Tue, 11 Oct 2022 07:59:54 +0000 (UTC) Received: from sirius.home.kraxel.org (unknown [10.39.195.183]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 89F38477F68; Tue, 11 Oct 2022 07:59:54 +0000 (UTC) Received: by sirius.home.kraxel.org (Postfix, from userid 1000) id B24931800082; Tue, 11 Oct 2022 09:59:52 +0200 (CEST) Date: Tue, 11 Oct 2022 09:59:52 +0200 From: "Gerd Hoffmann" To: Rebecca Cran Cc: devel@edk2.groups.io, Jordan Justen , Julien Grall , Oliver Steffen , Pawel Polawski , Ard Biesheuvel , Anthony Perard , Jiewen Yao Subject: Re: [edk2-devel] [PATCH 1/1] OvmfPkg/SmbiosPlatformDxe: use PcdFirmwareVersionString Message-ID: <20221011075952.r5yxm6s2hukzapnz@sirius.home.kraxel.org> References: <20221010144213.1470478-1-kraxel@redhat.com> <8fde917f-c932-0e0c-499f-bbe9daa95ac9@quicinc.com> MIME-Version: 1.0 In-Reply-To: <8fde917f-c932-0e0c-499f-bbe9daa95ac9@quicinc.com> X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 > > --- > > > > #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