Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/QemuBootOrderLib: Measure the etc/boot-menu-wait

["Gerd Hoffmann" <kraxel@redhat.com>]

On Wed, Mar 20, 2024 at 08:41:41AM +0000, Sun, CepingX wrote:
> 
> On Thursday, March 14, 2024 5:31 PM Gerd Hoffmann wrote:
> > Load, measure and cache all fw_cfg entries we care about early in the PEI phase
> > (or SEC phase for pei-less builds), so we can
> >  (a) easily have a fixed order, and
> >  (b) store them all in HOBs?
> > 
> > Which implies SEC/PEI must read all relevant fw_cfg entries, even in case they
> > are used only later in the DXE phase.
> > 
> > Advantage is we have a single cache which can be used in all firmware phases.
> > When using global variables in DXE we still can end up reading entries multiple
> > times, either because entries are needed by both PEI and DXE, or because
> > multiple DXE modules need them (global variables are per module).
> > 
> We have some concerns that in above solution the size of a single cache is large
> ( because all fw_cfg data need to be read from qemu). 

We don't need to read + cache all fw_cfg data.  We only need to cache
the entries which (a) must be measured, and (b) will not be measured in
some other way.

The big entries I'm aware of are:
 (1) kernel for direct boot (no fw_cfg measurement needed, binary
     will be measured before launch).
 (2) ACPI tables (no fw_cfg measurement needed, acpi tables are
     measured when installed).
 (3) smbios tables (to be decided, could be handled similar to ACPI).

Anything else where you have size concerns?

> Further more this is not a lazy mode which means some fw_cfg data 
> may be read but it is not to be consumed later. 

The problem with lazy mode is that the measurement order is not fixed.

> We propose below solution :
> 
> Add a new API in QemuFwCfgLib,
> RETURN_STATUS QemuFwCfgGetData(fw_cfg_name, *size, *value, FW_CFG_GET_DATA_FLAG flag).	

I'd suggest to *not* touch the existing interfaces for reading entries.
Instead change the existing functions to first check the cache, in case
there is no cache entry go read from fw_cfg.

Add a new interface for adding fw_cfg entries to the cache, with
optional measurement.

Populate the cache with all entries which need fw_cfg measurement.

Additionally cache entries which are used frequently
(QemuFwCfgItemSignature + QemuFwCfgItemInterfaceVersion +
QemuFwCfgItemFileDir are candidates).

Note that QemuFwCfgItemFileDir must be cached for security reasons (so
the hypervisor can't present different versions of the file list) even
though I don't think we need to measure it.

> Pros:
> Caller can decide how to get the fw_cfg data from QEMU (cache? Measurement?)

Cache and measurement are not independent options.  Measurement without
caching doesn't make sense.

>  And it can reduce the code complexity because it pack 3 fw_cfg call 
> (QemuFwCfgFindFile/QemuFwCfgSelectItem/QemuFwCfgReadBytes) into one call. 

There are a few places in the code which expect they can read fw_cfg
files in multiple chunks (i.e. call QemuFwCfgReadBytes multiple times).

Also note that not all fw_cfg entries are (pseudo-)files.

take care,
  Gerd



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