> -----Original Message-----
> From: Matt Ochs <[email protected]>
> Sent: 14 September 2026 16:56
> To: Peter Maydell <[email protected]>
> Cc: [email protected]; [email protected]; Eric Auger
> <[email protected]>; Nicolin Chen <[email protected]>; Shameer
> Kolothum Thodi <[email protected]>; [email protected]
> Subject: Re: [PATCH v2] hw/arm/tegra241-cmdqv: Keep VINTF page0 region
> alive
>
>
> > On Sep 14, 2026, at 07:29, Peter Maydell <[email protected]>
> wrote:
> > On Fri, 11 Sept 2026 at 20:04, Matthew R. Ochs <[email protected]>
> wrote:
> >>
> >>
> >> name = g_strdup_printf("%s vintf-page0",
> >> memory_region_name(&cmdqv->mmio_cmdqv));
> >> - cmdqv->mr_vintf_page0 = g_malloc0(sizeof(*cmdqv->mr_vintf_page0));
> >> - memory_region_init_ram_device_ptr(cmdqv->mr_vintf_page0,
> >> + memory_region_init_ram_device_ptr(&cmdqv->mr_vintf_page0,
> >>
> >> memory_region_owner(&cmdqv->mmio_cmdqv),
> >> name, VINTF_PAGE_SIZE,
> >> cmdqv->vintf_page0);
> >
> > ...but we don't actually initialize the MemoryRegion until here.
> >
> > I don't think you should assume that memory_region_* functions
> > will do anything sensible on a zeroed-out lump of memory.
>
> You're right. Although the current implementation of
> memory_region_is_mapped() only examines fields that are zero before
> initialization, using it here relies on the internal representation of
> an uninitialized MemoryRegion.
>
> memory_region_init_ram_device_ptr() has not yet called
> object_initialize() on the embedded region at that point. I'll restore
> the explicit initialization flag from v1 in v3 so that no MemoryRegion
> API is called before the region has been initialized.
Or we could avoid the check altogether by tying the MemoryRegion lifetime
to the vintf_page0 mmap.
- Move the initialization of mr_vintf_page0 to
tegra241_cmdqv_alloc_viommu(), just before the successful return.
Add it as a subregion, but disabled with memory_region_set_enabled(mr,
false).
- tegra241_cmdqv_guest_map_vintf_page0() and
tegra241_cmdqv_guest_unmap_vintf_page0() then become just
memory_region_set_enabled(&cmdqv->mr_vintf_page0, true/false).
- In tegra241_cmdqv_free_viommu(), remove the subregion and unparent
the region before the munmap.
This way we tie the MemoryRegion lifetime to the vintf_page0
mmap/munmap, and on VINTF0_CONFIG.ENABLE set/clear we just toggle
memory_region_set_enabled().
Thoughts?
Thanks,
Shameer
