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: kra...@redhat.com <kra...@redhat.com> > Sent: Thursday, April 18, 2024 7:45 PM > To: Yao, Jiewen <jiewen....@intel.com> > Cc: devel@edk2.groups.io; Ard Biesheuvel <a...@kernel.org>; Oliver Steffen > <ostef...@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/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
