> -----Original Message-----
> From: Cédric Le Goater <[email protected]>
> Sent: Thursday, September 17, 2026 6:57 PM
> To: Manish Honap <[email protected]>; [email protected]; Ankit Agrawal
> <[email protected]>; [email protected]; [email protected];
> [email protected]; Srirangan Madhavan
> <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> Cc: Krishnakant Jaju <[email protected]>; Vikram Sethi <[email protected]>;
> Zhi Wang <[email protected]>; [email protected]; qemu-
> [email protected]; [email protected]
> Subject: Re: [PATCH v2 02/10] hw/vfio/region: Add
> vfio_region_setup_with_ops()
>
> External email: Use caution opening links or attachments
>
>
> On 9/16/26 20:44, [email protected] wrote:
> > From: Manish Honap <[email protected]>
> >
> > A CXL Type-2 device traps its HDM decoder block, so that region needs
> > custom MemoryRegionOps rather than the pass-through default. Split the
> > setup body out and let a caller supply the ops; NULL keeps the
> > existing behaviour.
> >
> > AI-used-for: code (prototype)
> > Signed-off-by: Manish Honap <[email protected]>
> > ---
> > hw/vfio/region.c | 27 ++++++++++++++++++++++++---
> > hw/vfio/vfio-region.h | 3 +++
> > 2 files changed, 27 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/vfio/region.c b/hw/vfio/region.c index
> > 54ad11a6c8..21b54978b5 100644
> > --- a/hw/vfio/region.c
> > +++ b/hw/vfio/region.c
> > @@ -228,8 +228,9 @@ static int
> vfio_setup_region_sparse_mmaps(VFIORegion *region,
> > return 0;
> > }
> >
> > -int vfio_region_setup(Object *obj, VFIODevice *vbasedev, VFIORegion
> *region,
> > - int index, const char *name, Error **errp)
> > +static int vfio_region_do_setup(Object *obj, VFIODevice *vbasedev,
> > + VFIORegion *region, int index, const char
> > *name,
> > + const MemoryRegionOps *ops, Error
> > +**errp)
> > {
> > struct vfio_region_info *info = NULL;
> > int ret;
> > @@ -249,7 +250,7 @@ int vfio_region_setup(Object *obj, VFIODevice
> > *vbasedev, VFIORegion *region,
> >
> > if (region->size) {
> > region->mem = g_new0(MemoryRegion, 1);
> > - memory_region_init_io(region->mem, obj, &vfio_region_ops,
> > + memory_region_init_io(region->mem, obj, ops,
> > region, name, region->size);
> >
> > if (!vbasedev->no_mmap &&
> > @@ -273,6 +274,26 @@ int vfio_region_setup(Object *obj, VFIODevice
> *vbasedev, VFIORegion *region,
> > return 0;
> > }
> >
> > +int vfio_region_setup(Object *obj, VFIODevice *vbasedev, VFIORegion
> *region,
> > + int index, const char *name, Error **errp) {
> > + return vfio_region_do_setup(obj, vbasedev, region, index, name,
> > + &vfio_region_ops, errp); }
> > +
> > +/*
> > + * Like vfio_region_setup() but traps the region through @ops instead of
> the
> > + * default pass-through, so a caller can intercept accesses (the CXL HDM
> > + * decoder block). A NULL @ops keeps the default.
> > + */
> > +int vfio_region_setup_with_ops(Object *obj, VFIODevice *vbasedev,
> > + VFIORegion *region, int index, const char
> > *name,
> > + const MemoryRegionOps *ops, Error **errp)
> > +{
> > + return vfio_region_do_setup(obj, vbasedev, region, index, name,
> > + ops ? ops : &vfio_region_ops, errp);
>
> if you're calling _with_ops(), you have custom ops, that's whole point.
> Passing NULL is a caller bug, no need to have a fall back. Or assert().
>
Okay, understood. A NULL ops here is a caller bug. I will add an assert on ops
and drop the fallback.
> Thanks,
>
> C.
>
>
>
> > +}
> > +
> > static void vfio_subregion_unmap(VFIORegion *region, int index)
> > {
> > trace_vfio_region_unmap(memory_region_name(®ion-
> >mmaps[index].mem),
> > diff --git a/hw/vfio/vfio-region.h b/hw/vfio/vfio-region.h
> > index 58b236f113..8d5699013a 100644
> > --- a/hw/vfio/vfio-region.h
> > +++ b/hw/vfio/vfio-region.h
> > @@ -39,6 +39,9 @@ uint64_t vfio_region_read(void *opaque,
> > hwaddr addr, unsigned size);
> > int vfio_region_setup(Object *obj, VFIODevice *vbasedev, VFIORegion
> *region,
> > int index, const char *name, Error **errp);
> > +int vfio_region_setup_with_ops(Object *obj, VFIODevice *vbasedev,
> > + VFIORegion *region, int index, const char
> > *name,
> > + const MemoryRegionOps *ops, Error **errp);
> > int vfio_region_mmap(VFIORegion *region);
> > void vfio_region_mmaps_set_enabled(VFIORegion *region, bool enabled);
> > void vfio_region_exit(VFIORegion *region);
