Re: [PATCH v2 16/20] vfio/cxl: Register regions with VFIO layer

[Dan Williams]

mhonap@ wrote:
> From: Manish Honap <mhonap@xxxxxxxxxx>
> 
> Register the DPA and component register region with VFIO layer.
> Region indices for both these regions are cached for quick lookup.
> 
> vfio_cxl_register_cxl_region()
> - memremap(WB) the region HPA (treat CXL.mem as RAM, not MMIO)
> - Register VFIO_REGION_SUBTYPE_CXL
> - Records dpa_region_idx.
> 
> vfio_cxl_register_comp_regs_region()
> - Registers VFIO_REGION_SUBTYPE_CXL_COMP_REGS with size
>   hdm_reg_offset + hdm_reg_size
> - Records comp_reg_region_idx.
> 
> Signed-off-by: Manish Honap <mhonap@xxxxxxxxxx>
> ---
>  drivers/vfio/pci/cxl/vfio_cxl_core.c | 98 +++++++++++++++++++++++++++-
>  drivers/vfio/pci/cxl/vfio_cxl_emu.c  | 34 ++++++++++
>  drivers/vfio/pci/cxl/vfio_cxl_priv.h |  2 +
>  drivers/vfio/pci/vfio_pci.c          | 23 +++++++
>  drivers/vfio/pci/vfio_pci_priv.h     | 11 ++++
>  5 files changed, 167 insertions(+), 1 deletion(-)
[..]
> @@ -622,4 +640,82 @@ static const struct vfio_pci_regops vfio_cxl_regops = {
>  	.release	= vfio_cxl_region_release,
>  };
>  
> +int vfio_cxl_register_cxl_region(struct vfio_pci_core_device *vdev)
> +{
> +	struct vfio_pci_cxl_state *cxl = vdev->cxl;
> +	u32 flags;
> +	int ret;
> +
> +	if (!cxl)
> +		return -ENODEV;
> +
> +	if (!cxl->region || cxl->region_vaddr)
> +		return -ENODEV;
> +
> +	/*
> +	 * CXL device memory is RAM, not MMIO.  Use memremap() rather than

Right, CXL.mem is RAM, not MMIO, so I question why is this not being
mapped via a RAM mechanism? Can you explain a bit more about why reusing
vfio MMIO mapping mechanisms is suitable here and what happens when we
get to questions like guest_memfd integration, page conversions, and
large page mapping support? I am looking at proposals like Gregory's as
a way to get full MM semantics for CXL.mem but without the memory being
consumed for other purposes [1].

Maybe this is the start of the conversation for something simple, but I
would actually prefer uncached ioremap() if only to avoid all the
coherence management and suitability of MMIO mechanisms questions.
ioremap() being a poor fit for CXL is a "feature" to force the "find the
right, something better" direction.

[1]: https://lore.kernel.org/all/20260222084842.1824063-1-gourry@xxxxxxxxxx