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'm not certain in which standard they should go. TCG would seem
appropriate, or IETF is also used for protocols related to confidential
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"
* 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.apalodi...@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
