Re: [edk2-devel] [PATCH v2 0/5] OvmfPkg: Add VirtHstiDxe driver

["Yao, Jiewen" <jiewen.yao@intel.com>]

Thanks.

I notice one more thing: CodeFlash check is using PcdBfvBase, while VariableFlash check is using PcdOvmfFdBaseAddress.

I feel the VariableFlash check may bring confusing and potential risk, because PcdOvmfFdBaseAddress means the address of the full binary. There is no guarantee that it must be variable region. (Although the current implementation is.)
After check the FDF file, I highly recommend we use PcdOvmfFlashNvStorageVariableBase or PcdCfvBase which should guarantee it is variable region.

With this naming change in "VirtHstiQemuFirmwareFlashCheck (PcdGet32 (PcdOvmfFdBaseAddress))", reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>

Thank you
Yao, Jiewen



> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Friday, April 19, 2024 8:31 PM
> To: devel@edk2.groups.io
> Cc: Oliver Steffen <osteffen@redhat.com>; Konstantin Kostiuk
> <kkostiuk@redhat.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Yao,
> Jiewen <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: [PATCH v2 0/5] OvmfPkg: Add VirtHstiDxe driver
> 
> v2:
>  - remove 'Q35' from test bits
>  - add patch with a README.md
> 
> Gerd Hoffmann (3):
>   OvmfPkg/VirtHstiDxe: add varstore flash check
>   OvmfPkg/VirtHstiDxe: add code flash check
>   OvmfPkg/VirtHstiDxe: add README.md
> 
> Konstantin Kostiuk (2):
>   OvmfPkg: Add VirtHstiDxe driver
>   OvmfPkg: Add VirtHstiDxe to OVMF firmware build
> 
>  OvmfPkg/OvmfPkgIa32.dsc             |   2 +
>  OvmfPkg/OvmfPkgIa32X64.dsc          |   2 +
>  OvmfPkg/OvmfPkgX64.dsc              |   2 +
>  OvmfPkg/OvmfPkgIa32.fdf             |   1 +
>  OvmfPkg/OvmfPkgIa32X64.fdf          |   1 +
>  OvmfPkg/OvmfPkgX64.fdf              |   1 +
>  OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf |  56 +++++++++
>  OvmfPkg/VirtHstiDxe/VirtHstiDxe.h   |  94 +++++++++++++++
>  OvmfPkg/VirtHstiDxe/Flash.c         |  90 +++++++++++++++
>  OvmfPkg/VirtHstiDxe/QemuCommon.c    |  36 ++++++
>  OvmfPkg/VirtHstiDxe/QemuPC.c        |  38 ++++++
>  OvmfPkg/VirtHstiDxe/QemuQ35.c       |  71 ++++++++++++
>  OvmfPkg/VirtHstiDxe/VirtHstiDxe.c   | 173 ++++++++++++++++++++++++++++
>  OvmfPkg/VirtHstiDxe/README.md       |  48 ++++++++
>  14 files changed, 615 insertions(+)
>  create mode 100644 OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf
>  create mode 100644 OvmfPkg/VirtHstiDxe/VirtHstiDxe.h
>  create mode 100644 OvmfPkg/VirtHstiDxe/Flash.c
>  create mode 100644 OvmfPkg/VirtHstiDxe/QemuCommon.c
>  create mode 100644 OvmfPkg/VirtHstiDxe/QemuPC.c
>  create mode 100644 OvmfPkg/VirtHstiDxe/QemuQ35.c
>  create mode 100644 OvmfPkg/VirtHstiDxe/VirtHstiDxe.c
>  create mode 100644 OvmfPkg/VirtHstiDxe/README.md
> 
> --
> 2.44.0



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