Re: [RFC v1 10/25] hw/core/loader: Add a ROM loader notifier

Mathieu Poirier <[email protected]>
Newsgroups org.nongnu.qemu-arm,org.kernel.vger.kvm,org.nongnu.qemu-devel
Message-ID <al59Y0S_DLMbAESK@p14s>
On Mon, Jul 20, 2026 at 04:47:53PM +1000, Gavin Shan wrote:
> On 7/17/26 7:10 AM, Mathieu Poirier wrote:
> > On Tue, Jul 14, 2026 at 08:52:43PM +1000, Gavin Shan wrote:
> > > On 7/8/26 8:42 AM, Mathieu Poirier wrote:
> > > > From: Jean-Philippe Brucker <[email protected]>
> > > > 
> > > > Add a function to register a notifier that is invoked when ROMs get
> > > > loaded into guest memory.
> > > > 
> > > > It will be used by Arm confidential guest support, in order to register
> > > > all blobs loaded into memory with KVM, so that their content is moved
> > > > into Realm state and measured into the initial VM state.
> > > > 
> > > > Signed-off-by: Jean-Philippe Brucker <[email protected]>
> > > > Signed-off-by: Mathieu Poirier <[email protected]>
> > > > ---
> > > >    hw/core/loader.c         | 15 +++++++++++++++
> > > >    include/hw/core/loader.h | 17 +++++++++++++++++
> > > >    2 files changed, 32 insertions(+)
> > > > 
> > > > diff --git a/hw/core/loader.c b/hw/core/loader.c
> > > > index 5cbfba0a86d2..b3d769bec5cb 100644
> > > > --- a/hw/core/loader.c
> > > > +++ b/hw/core/loader.c
> > > > @@ -74,6 +74,8 @@
> > > >    #endif
> > > 
> > > The (edk2) firmware for the realm guest can be specified by the legacy
> > > '-drive if=pflash' or '-bios'. The memory region corresponding to the
> > > firmware image specified by '-bios' is recorded by the newly added notifier
> > > and populated to TF-RMM properly.
> > > 
> > > I don't see how the memory region corresponding to (edk2) firmware image
> > > specified by '-drive if=pflash' is recorded and populated to TF-RMM
> > > accordingly in this series. So I guess '-drive if=pflash' isn't supported
> > > yet?
> > 
> > Please give '-drive if=pflash' a try and let me know how you fare.  Edk2 is not
> > part of the bootstack I'm currently testing with but I know of configurations
> > where it does work.
> > 
> > I'll add it to my list of things to do if it doesn't work, though I may not
> > address it in this patchset.  We'll see how busy things get.
> > 
> 
> Ok, I don't think '-driver if=pflash' or the 'pflash0' property of the virt machine
> aren't supported any more due to [RFC v1 21/25], where the 'pflash0' property has
> been removed.
> 
>   [RFC v1 21/25] hw/arm/virt: Use RAM instead of flash for confidential guest firmware
> 
> - Tried to boot the realm guest with '-driver if=pflash' and I was told it's not supported.
> 
>   qemu-system-aarch64: -drive if=pflash,format=raw,unit=0,file=/mnt/edk2/Build/ArmVirtQemu-AARCH64/RELEASE_GCC5/FV/QEMU_EFI.fd,readonly=on: machine type   does not support if=pflash,bus=0,unit=0
> 
> - Tried to use 'pflash0' property, and I was told it's not supported either.
> 
>   qemu-system-aarch64: Property 'virt-11.1-machine.pflash0' not found
> 
> So the only option to specify the firmware image is '-bios', which worked for me.

Thanks for the follow-up.

> 
> Thanks,
> Gavin
> 
> 
> > > 
> > > Note that I didn't give '-drive if=pflash' a try yet.
> > > 
> > > >    static int roms_loaded;
> > > > +static NotifierList rom_loader_notifier =
> > > > +    NOTIFIER_LIST_INITIALIZER(rom_loader_notifier);
> > > >    /* return the size or -1 if error */
> > > >    int64_t get_image_size(const char *filename, Error **errp)
> > > > @@ -1201,6 +1203,11 @@ MemoryRegion *rom_add_blob(const char *name, const void *blob, size_t len,
> > > >        return mr;
> > > >    }
> > > > +void rom_add_load_notifier(Notifier *notifier)
> > > > +{
> > > > +    notifier_list_add(&rom_loader_notifier, notifier);
> > > > +}
> > > > +
> > > >    /* This function is specific for elf program because we don't need to allocate
> > > >     * all the rom. We just allocate the first part and the rest is just zeros. This
> > > >     * is why romsize and datasize are different. Also, this function takes its own
> > > > @@ -1242,6 +1249,7 @@ ssize_t rom_add_option(const char *file, int32_t bootindex)
> > > >    static void rom_reset(void *unused)
> > > >    {
> > > >        Rom *rom;
> > > > +    RomLoaderNotifyData notify;
> > > >        QTAILQ_FOREACH(rom, &roms, next) {
> > > >            if (rom->fw_file) {
> > > > @@ -1290,6 +1298,13 @@ static void rom_reset(void *unused)
> > > >            address_space_flush_icache_range(rom->as, rom->addr, rom->datasize);
> > > >            trace_loader_write_rom(rom->name, rom->addr, rom->datasize, rom->isrom);
> > > > +
> > > > +        notify = (RomLoaderNotifyData) {
> > > > +            .addr = rom->addr,
> > > > +            .len = rom->datasize,
> > > > +            .as = rom->as,
> > > > +        };
> > > > +        notifier_list_notify(&rom_loader_notifier, &notify);
> > > >        }
> > > >    }
> > > > diff --git a/include/hw/core/loader.h b/include/hw/core/loader.h
> > > > index d9431e8a8d12..94cf6ad2e26b 100644
> > > > --- a/include/hw/core/loader.h
> > > > +++ b/include/hw/core/loader.h
> > > > @@ -342,6 +342,23 @@ void *rom_ptr_for_as(AddressSpace *as, hwaddr addr, size_t size);
> > > >    ssize_t rom_add_vga(const char *file);
> > > >    ssize_t rom_add_option(const char *file, int32_t bootindex);
> > > > +typedef struct RomLoaderNotifyData {
> > > > +    /* Address of the blob in guest memory */
> > > > +    hwaddr addr;
> > > > +    /* Length of the blob */
> > > > +    size_t len;
> > > > +    /* Address space of the blob */
> > > > +    AddressSpace *as;
> > > > +} RomLoaderNotifyData;
> > > > +
> > > > +/**
> > > > + * rom_add_load_notifier - Add a notifier for loaded images
> > > > + *
> > > > + * Add a notifier that will be invoked with a RomLoaderNotifyData structure for
> > > > + * each blob loaded into guest memory, after the blob is loaded.
> > > > + */
> > > > +void rom_add_load_notifier(Notifier *notifier);
> > > > +
> > > >    /* This is the usual maximum in uboot, so if a uImage overflows this, it would
> > > >     * overflow on real hardware too. */
> > > >    #define UBOOT_MAX_DECOMPRESSED_BYTES (64 << 20)
> > > 
> > > Thanks,
> > > Gavin
> > > 
> > 
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.