Hi Michael - thanks for taking the time to review this work.
On Tue, Sep 15, 2026 at 05:50:37PM -0500, Michael Roth wrote:
> On Thu, Sep 03, 2026 at 01:35:57PM -0600, Mathieu Poirier wrote:
> > From: Jean-Philippe Brucker <[email protected]>
> >
> > Add a Rom notifier to keep track of binary blobs loaded in Realm memory.
> > That way we can deterministically calculate the Realm Initial Measurement
> > (RIM).
> >
> > Signed-off-by: Jean-Philippe Brucker <[email protected]>
> > Signed-off-by: Lorenzo Pieralisi <[email protected]>
> > Signed-off-by: Mathieu Poirier <[email protected]>
> > ---
> > target/arm/kvm-rme.c | 52 ++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 52 insertions(+)
> >
> > diff --git a/target/arm/kvm-rme.c b/target/arm/kvm-rme.c
> > index f7c2cc786e2c..9985f5daba89 100644
> > --- a/target/arm/kvm-rme.c
> > +++ b/target/arm/kvm-rme.c
> > @@ -10,6 +10,7 @@
> >
> > #include "hw/core/boards.h"
> > #include "hw/core/cpu.h"
> > +#include "hw/core/loader.h"
> > #include "kvm_arm.h"
> > #include "migration/blocker.h"
> > #include "qapi/error.h"
> > @@ -22,8 +23,18 @@
> > #define TYPE_RME_GUEST "rme-guest"
> > OBJECT_DECLARE_SIMPLE_TYPE(RmeGuest, RME_GUEST)
> >
> > +#define RME_PAGE_SIZE qemu_real_host_page_size()
> > +
> > +typedef struct {
> > + hwaddr base;
> > + hwaddr size;
> > + uint8_t *data;
> > +} RmeRamRegion;
> > +
> > struct RmeGuest {
> > ConfidentialGuestSupport parent_obj;
> > + Notifier rom_load_notifier;
> > + GSList *ram_regions;
> > };
> >
> > OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(RmeGuest, rme_guest, RME_GUEST,
> > @@ -41,6 +52,44 @@ static void rme_vm_state_change(void *opaque, bool
> > running, RunState state)
> > kvm_mark_guest_state_protected();
> > }
> >
> > +static gint rme_compare_ram_regions(gconstpointer a, gconstpointer b)
> > +{
> > + const RmeRamRegion *ra = a;
> > + const RmeRamRegion *rb = b;
> > +
> > + g_assert(ra->base != rb->base);
> > + return ra->base < rb->base ? -1 : 1;
> > +}
> > +
> > +static void rme_rom_load_notify(Notifier *notifier, void *data)
> > +{
> > + RmeRamRegion *region;
> > + RomLoaderNotifyData *rom = data;
> > +
> > + if (rom->addr == -1) {
>
> Are these entries also added via the rom_reset() path in the previous
> patch? If so, wouldn't it be better to filter them out there?
Duly noted. I will also wrap the previous patch into this one.
>
> Also, if that's where these entries are added, then it looks like we
> might call address_space_flush_icache_range() with this -1 addr before
> adding it to the notifier list. Is that expected?
Function address_space_flush_icache_range() is indeed called with @rom->addr set
to -1 (0xffffffffffffffff). Since this address is not accessible, the inner
loop of the function skips over the entire Rom area without ever calling
flush_idcache_range().
Regards,
Mathieu
>
> -Mike
>
> > + /*
> > + * These blobs (ACPI tables) are not loaded into guest RAM at
> > reset.
> > + * Instead the firmware will load them via fw_cfg and measure them
> > + * itself.
> > + */
> > + return;
> > + }
> > +
> > + region = g_new0(RmeRamRegion, 1);
> > + region->base = rom->addr;
> > + region->size = rom->len;
> > + region->data = rom->data;
> > +
> > + /*
> > + * The Realm Initial Measurement (RIM) depends on the order in which we
> > + * initialize and populate the RAM regions. To help a verifier
> > + * independently calculate the RIM, sort regions by GPA.
> > + */
> > + rme_guest->ram_regions = g_slist_insert_sorted(rme_guest->ram_regions,
> > + region,
> > +
> > rme_compare_ram_regions);
> > +}
> > +
> > static int kvm_arm_rme_init(ConfidentialGuestSupport *cgs, Error **errp)
> > {
> > KVMState *s = KVM_STATE(current_accel());
> > @@ -58,6 +107,9 @@ static int kvm_arm_rme_init(ConfidentialGuestSupport
> > *cgs, Error **errp)
> > error_setg(&rme_mig_blocker, "RME: migration is not implemented");
> > migrate_add_blocker(&rme_mig_blocker, &error_fatal);
> >
> > + rme_guest->rom_load_notifier.notify = rme_rom_load_notify;
> > + rom_add_load_notifier(&rme_guest->rom_load_notifier);
> > +
> > /*
> > * The realm activation is done last, when the VM starts, after all
> > images
> > * have been loaded and all vcpus finalized.
> > --
> > 2.43.0
> >
> >
