Re: [RFC PATCH v3 25/26] target/arm/kvm-rme: Add measurement log

[Stefan Berger]

On 12/13/24 9:21 AM, Jean-Philippe Brucker wrote:
> On Mon, Dec 09, 2024 at 05:08:37PM -0500, Stefan Berger wrote:
> > >    typedef struct {
> > >        hwaddr base;
> > >        hwaddr size;
> > > +    uint8_t *data;
> > > +    RmeLogFiletype *filetype;
> > >    } RmeRamRegion;
> > > +typedef struct {
> > > +    char        signature[16];
> > > +    char        name[32];
> > > +    char        version[40];
> > > +    uint64_t    ram_size;
> > > +    uint32_t    num_cpus;
> > > +    uint64_t    flags;
> > > +} EventLogVmmVersion;
> > > +
> > > +typedef struct {
> > > +    uint32_t    id;
> > > +    uint32_t    data_size;
> > > +    uint8_t     data[];
> > > +} EventLogTagged;
> > > +
> >
> >
> > > +#define EVENT_LOG_TAG_REALM_CREATE  1
> > > +#define EVENT_LOG_TAG_INIT_RIPAS    2
> > > +#define EVENT_LOG_TAG_REC_CREATE    3
> > > +
> > If these are ARM-related structures and constants from a document you may
> > want to mention the document you got them from.
>
> Agreed. At the moment they're just numbers and structures I made up [1].

I looked through old TCG specs (likely there are newer ones that mention 
this as well, but don't currently know) that have some definitions for 
EV_EVENT_TAG with a specific structure for logging the event. You seem 
to use the same structure:
https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClientImplementation_1-21_1_00.pdf


11.3.2.1:

typedef struct tdTCG_PCClientTaggedEventStruct {
UINT32 EventID;
UINT32 EventDataSize;
BYTE[EventDataSize] EventData;
} TCG_PCClientTaggedEventStruct;

This seems to be your EventLogTagged.

EV_EVENT_TAG has tags defined for x86_64 defined in 11.3.2.3.{1 .. 13} 
in that spec with numbers 0 .. 0xe. You would be clashing with those 
numbers but otoh the are platform-specific.

> I'm not certain in which standard they should go. TCG would seem
> appropriate, or IETF is also used for protocols related to confidential

The log is from TCG, so it would probably have to go into some sort of 
TCG registry.

> computing attestation. Or maybe it could live in the reference verifier
> documentation. QEMU docs wouldn't be the best place because VMMs might
> been reluctant to adopt it because they don't consider it a standard (like
> cloud-hv and fw_cfg)
>
> When researching this I found TCG event types and payloads that only seem
> to be documented in their respective project:
> * efistub [2] with
>    * EV_EVENT_TAG, id=0x8F3B22EC, data="Linux initrd",
>    * EV_EVENT_TAG, id=0x8F3B22ED, data="LOADED_IMAGE::LoadOptions"

Maybe the first id is just a random number or in a vendor-specific space 
that I don't know about. My guess is they likely try to avoid clashes 
with existing numbers.


> * grub [3] with a few EV_IPL
> * systemd [4] with various EV_EVENT_TAG and EV_IPL
>
> I'm wondering if we could create a common registry somewhere for these,
> like IANA or somewhere informal.
> >
> [1] 
> https://github.com/veraison/cca-realm-measurements/blob/main/docs/measurement-log.md#rim-log
> [2] 
> https://lore.kernel.org/all/20211119114745.1560453-1-ilias.apalodimas@linaro.org/
> [3] https://www.gnu.org/software/grub/manual/grub/html_node/Measured-Boot.html
> [4] https://systemd.io/TPM2_PCR_MEASUREMENTS/
>
>
> > > +/* Log VM type and Realm Descriptor create */
> > > +static int rme_log_realm_create(Error **errp)
> > > +{
> > > +    int ret;
> > > +    ARMCPU *cpu;
> > > +    EventLogVmmVersion vmm_version = {
> > > +        .signature = "VM VERSION",
> > > +        .name = "QEMU",
> > > +        .version = "9.1", /* TODO: dynamic */
> >
> > $ grep -r QEMU_VERSION_M build/
> > build/config-host.h:#define QEMU_VERSION_MAJOR 9
> > build/config-host.h:#define QEMU_VERSION_MICRO 93
> > build/config-host.h:#define QEMU_VERSION_MINOR 1
> >
> > $ cat VERSION
> > 9.1.93
>
> Ah yes that would work, thank you
>
> > > +static int rme_log_rec(uint64_t flags, uint64_t pc, uint64_t gprs[8], Error 
> > > **errp)
> > > +{
> >
> > $ ./scripts/checkpatch.pl ./tmp/*.patch
> > [...]
> > Checking ./tmp/0002-target-arm-kvm-rme-Add-measurement-log.patch...
> > WARNING: line over 80 characters
> > #353: FILE: target/arm/kvm-rme.c:303:
> > +static int rme_log_rec(uint64_t flags, uint64_t pc, uint64_t gprs[8], Error
> > **errp)
> >
> > May want to run this on all patches.
> >
> > Rest LGTM.
>
> Thank you!
>
> Jean