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

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

1) Yes, I highly recommend remove Q35 keyword.
2) Got it. I think we had better add such info in the code as comment as well.

Thank you
Yao, Jiewen

> -----Original Message-----
> From: kraxel@redhat.com <kraxel@redhat.com>
> Sent: Thursday, April 18, 2024 7:45 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: devel@edk2.groups.io; Ard Biesheuvel <ardb@kernel.org>; Oliver Steffen
> <osteffen@redhat.com>
> Subject: Re: [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver
> 
> On Wed, Apr 17, 2024 at 01:20:57PM +0000, Yao, Jiewen wrote:
> > That is good start. The SMRAM lock and Flash lock seem good to me.
> >
> > Comment:
> > 1) Do we really need to add "Q35" for the policy?
> > #define VIRT_HSTI_BYTE0_Q35_SMM_SMRAM_LOCK         BIT0
> > #define VIRT_HSTI_BYTE0_Q35_SMM_SECURE_VARS_FLASH  BIT1
> >
> > I feel we had better remove it, since SMM_SMRAM_LOCK and
> SMM_SECURE_VARS_FLASH are common features for almost all X86 platforms.
> 
> Well, SMM mode is supported for the qemu 'q35' machine type only, the
> 'pc' machine type doesn't provide enough memory for SMM.  Which why I've
> added 'Q35' to the name.
> 
> The SMM_SMRAM_LOCK test actually is q35-specific because the control
> registers are chipset specific.  But, yes, the concept is not q35
> specific.
> 
> I can drop 'Q35' if you prefer it that way.
> 
> > 2) Would you please let me know what "READONLY_CODE_FLASH" really
> means?
> >
> > #define VIRT_HSTI_BYTE0_Q35_SMM_SECURE_VARS_FLASH  BIT1
> > #define VIRT_HSTI_BYTE0_READONLY_CODE_FLASH        BIT2
> >
> > Does READONLY_CODE_FLASH mean NO write to flash even in SMM mode?
> > Or does it just mean NO write in normal operation mode, but still writable in
> SMM mode?
> 
> With qemu being configured properly flash behavior should be this:
> 
>                                |  OVMF_CODE.fd  |  OVMF_VARS.fd
> -------------------------------+----------------+----------------
> SMM_REQUIRE=TRUE, SMM mode     |  read-only     |  writable
> SMM_REQUIRE=TRUE, normal mode  |  read-only (1) |  read-only (2)
> SMM_REQUIRE=FALSE              |  read-only (3) |  writable
> 
> VIRT_HSTI_BYTE0_READONLY_CODE_FLASH will verify (1) + (3).
> VIRT_HSTI_BYTE0_Q35_SMM_SECURE_VARS_FLASH will verify (2).
> 
> (probably a good idea to add that as comment to the patches).
> 
> take care,
>   Gerd



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