Re: [edk2-devel] [PATCH v3 4/5] OvmfPkg/VirtHstiDxe: add code flash check

["Aithal, Srikanth via groups.io" <srikanth.aithal=amd.com@groups.io>]

[-- Attachment #1: Type: text/plain, Size: 6811 bytes --]

Correcting.

On 4/23/2024 7:09 PM, Aithal, Srikanth wrote:
> Hello,
>
> Todays OVMF/edk2 master branch is breaking AMD SEV-ES guest boot with 
> OvmfX64 package, where as sev-es guest boots fine with AmdSev package.
>
> Git bisect pointed to below commit as bad, going back to previous 
> commit i.e ddc43e7a SEV-ES guest boots fine with OvmfX64 package:
Git bisect pointed to below commit as bad, going back to previous commit 
i.e ddc43e7a SEV-ES guest boots fine. With OVMF/edk2 master branch 
SEV-ES guest boots fine with *AmdSev *package:
>
> commit 506740982bba199f12e75f6cfda510c30aa4e7c6
> Author: Gerd Hoffmann <kraxel@redhat.com>
> Date:   Mon Apr 22 12:47:28 2024 +0200
>
>     OvmfPkg/VirtHstiDxe: add code flash check
>
>     Detects qemu config issue: code pflash is writable.
>     Checked for both PC and Q35.
>
>     Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>     Cc: Jiewen Yao <jiewen.yao@intel.com>
>     Cc: Konstantin Kostiuk <kkostiuk@redhat.com>
>     Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>     Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
>
> QEMU commandline used:
>
> qemu-system-x86_64 \
> -machine q35,confidential-guest-support=sev0,vmport=off \
> -object 
> sev-guest,id=sev0,cbitpos=51,policy=0x5,reduced-phys-bits=1,kernel-hashes=off 
> \
> -name guest=vm,debug-threads=on \
> -drive if=pflash,format=raw,unit=0,file=<path to OVMF_X64/OVMF_CODE.fd 
> or OVMF_X64/OVMF.fd>,readonly  \
> -cpu EPYC-Milan-v2 \
> -m 4096 \
> -smp 1,cores=1,threads=1,dies=1,sockets=1 \
> -drive file=22.04-serverfull.qcow2,index=0,media=disk,format=qcow2 \
> --enable-kvm \
> --nographic
>
>
> Component levels used in test:
> qemu: v8.2.2
> host_kernel and guest_kernel: v6.8.2
> ovmf: current master of https://github.com/tianocore/edk2, Head: 86c8d69
>
> Attaching guest serial log.
>
>
> Thanks,
>
> Aithal, Srikanth <Srikanth.Aithal@amd.com>
>
> On 4/22/2024 4:17 PM, Gerd Hoffmann via groups.io wrote:
>> Detects qemu config issue: code pflash is writable.
>> Checked for both PC and Q35.
>>
>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>> Cc: Konstantin Kostiuk <kkostiuk@redhat.com>
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
>> ---
>>   OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf |  2 ++
>>   OvmfPkg/VirtHstiDxe/VirtHstiDxe.h   | 13 +++++++++++
>>   OvmfPkg/VirtHstiDxe/QemuCommon.c    | 36 +++++++++++++++++++++++++++++
>>   OvmfPkg/VirtHstiDxe/VirtHstiDxe.c   |  4 ++++
>>   4 files changed, 55 insertions(+)
>>   create mode 100644 OvmfPkg/VirtHstiDxe/QemuCommon.c
>>
>> diff --git a/OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf 
>> b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf
>> index b6bdd1f22e83..9514933011e8 100644
>> --- a/OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf
>> +++ b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf
>> @@ -22,6 +22,7 @@ [Sources]
>>     VirtHstiDxe.c
>>     QemuPC.c
>>     QemuQ35.c
>> +  QemuCommon.c
>>     Flash.c
>>     [Packages]
>> @@ -48,6 +49,7 @@ [FeaturePcd]
>>     gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
>>     [Pcd]
>> +  gUefiOvmfPkgTokenSpaceGuid.PcdBfvBase
>>     gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase
>>     [Depex]
>> diff --git a/OvmfPkg/VirtHstiDxe/VirtHstiDxe.h 
>> b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.h
>> index ceff41c03711..f8bdcfe8f219 100644
>> --- a/OvmfPkg/VirtHstiDxe/VirtHstiDxe.h
>> +++ b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.h
>> @@ -8,6 +8,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>>     #define VIRT_HSTI_BYTE0_SMM_SMRAM_LOCK         BIT0
>>   #define VIRT_HSTI_BYTE0_SMM_SECURE_VARS_FLASH  BIT1
>> +#define VIRT_HSTI_BYTE0_READONLY_CODE_FLASH    BIT2
>>     typedef struct {
>>     // ADAPTER_INFO_PLATFORM_SECURITY
>> @@ -67,6 +68,18 @@ VirtHstiQemuPCVerify (
>>     VOID
>>     );
>>   +/* QemuCommon.c */
>> +
>> +VOID
>> +VirtHstiQemuCommonInit (
>> +  VIRT_ADAPTER_INFO_PLATFORM_SECURITY  *VirtHsti
>> +  );
>> +
>> +VOID
>> +VirtHstiQemuCommonVerify (
>> +  VOID
>> +  );
>> +
>>   /* Flash.c */
>>     #define QEMU_FIRMWARE_FLASH_UNKNOWN    0
>> diff --git a/OvmfPkg/VirtHstiDxe/QemuCommon.c 
>> b/OvmfPkg/VirtHstiDxe/QemuCommon.c
>> new file mode 100644
>> index 000000000000..4ab3fe2d6e63
>> --- /dev/null
>> +++ b/OvmfPkg/VirtHstiDxe/QemuCommon.c
>> @@ -0,0 +1,36 @@
>> +/** @file
>> +
>> +SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> +**/
>> +
>> +#include <Library/BaseLib.h>
>> +#include <Library/DebugLib.h>
>> +
>> +#include "VirtHstiDxe.h"
>> +
>> +VOID
>> +VirtHstiQemuCommonInit (
>> +  VIRT_ADAPTER_INFO_PLATFORM_SECURITY  *VirtHsti
>> +  )
>> +{
>> +  VirtHstiSetSupported (VirtHsti, 0, 
>> VIRT_HSTI_BYTE0_READONLY_CODE_FLASH);
>> +}
>> +
>> +VOID
>> +VirtHstiQemuCommonVerify (
>> +  VOID
>> +  )
>> +{
>> +  CHAR16  *ErrorMsg;
>> +
>> +  switch (VirtHstiQemuFirmwareFlashCheck (PcdGet32 (PcdBfvBase))) {
>> +    case QEMU_FIRMWARE_FLASH_WRITABLE:
>> +      ErrorMsg = L"qemu code pflash is writable";
>> +      break;
>> +    default:
>> +      ErrorMsg = NULL;
>> +  }
>> +
>> +  VirtHstiTestResult (ErrorMsg, 0, 
>> VIRT_HSTI_BYTE0_READONLY_CODE_FLASH);
>> +}
>> diff --git a/OvmfPkg/VirtHstiDxe/VirtHstiDxe.c 
>> b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.c
>> index 74e5e6bd9d4f..b6e53a1219d1 100644
>> --- a/OvmfPkg/VirtHstiDxe/VirtHstiDxe.c
>> +++ b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.c
>> @@ -104,9 +104,11 @@ VirtHstiOnReadyToBoot (
>>     switch (VirtHstiGetHostBridgeDevId ()) {
>>       case INTEL_82441_DEVICE_ID:
>>         VirtHstiQemuPCVerify ();
>> +      VirtHstiQemuCommonVerify ();
>>         break;
>>       case INTEL_Q35_MCH_DEVICE_ID:
>>         VirtHstiQemuQ35Verify ();
>> +      VirtHstiQemuCommonVerify ();
>>         break;
>>       default:
>>         ASSERT (FALSE);
>> @@ -142,9 +144,11 @@ VirtHstiDxeEntrypoint (
>>     switch (DevId) {
>>       case INTEL_82441_DEVICE_ID:
>>         VirtHsti = VirtHstiQemuPCInit ();
>> +      VirtHstiQemuCommonInit (VirtHsti);
>>         break;
>>       case INTEL_Q35_MCH_DEVICE_ID:
>>         VirtHsti = VirtHstiQemuQ35Init ();
>> +      VirtHstiQemuCommonInit (VirtHsti);
>>         break;
>>       default:
>>         DEBUG ((DEBUG_INFO, "%a: unknown platform (0x%x)\n", 
>> __func__, DevId));


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118172): https://edk2.groups.io/g/devel/message/118172
Mute This Topic: https://groups.io/mt/105667072/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 12814 bytes --]