Linux CXL
 help / color / mirror / Atom feed
From: Shuai Xue <xueshuai@linux.alibaba.com>
To: mhonap@nvidia.com, alex@shazbot.org, jgg@ziepe.ca,
	ankita@nvidia.com, jic23@kernel.org, dave.jiang@intel.com,
	alejandro.lucero-palau@amd.com, smadhavan@nvidia.com,
	corbet@lwn.net, skhan@linuxfoundation.org, dave@stgolabs.net,
	alison.schofield@intel.com, vishal.l.verma@intel.com,
	iweiny@kernel.org, ming.li@zohomail.com, yishaih@nvidia.com,
	skolothumtho@nvidia.com, kevin.tian@intel.com,
	bhelgaas@google.com, dmatlack@google.com, kees@kernel.org,
	gustavoars@kernel.org
Cc: cjia@nvidia.com, kjaju@nvidia.com, vsethi@nvidia.com,
	zhiw@nvidia.com, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	linux-cxl@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-hardening@vger.kernel.org
Subject: Re: [PATCH v4 27/27] selftests/vfio: Add CXL Type-2 passthrough corner-case tests
Date: Wed, 26 Aug 2026 15:28:58 +0800	[thread overview]
Message-ID: <8b6a3086-c491-4eff-938f-aecaac53b813@linux.alibaba.com> (raw)
In-Reply-To: <20260813093631.2288172-28-mhonap@nvidia.com>



On 8/13/26 5:36 PM, mhonap@nvidia.com wrote:
> From: Manish Honap <mhonap@nvidia.com>
> 
> Exercise the vfio-cxl contract on a bound CXL Type-2 device: the two VFIO
> regions and the geometry capability, the HDM memory mmap (including a 2 MB
> huge fault), the dword-aligned trapped decoder block, and the
> lock-on-commit FSM. The decoder writes land in the per-open shadow and
> each test reopens the device, so the FSM tests repeat cleanly.
> 
> Cover the HDM memory two ways: a host-CPU load/store of the mmap, and the
> path a VMM actually uses, mmap plus a stage-2 IOAS map for the device's
> ATS access. The mmap flag is required for the IOAS path, so assert it is
> advertised rather than skipping when it is absent.
> 
> Signed-off-by: Manish Honap <mhonap@nvidia.com>
> ---
>   MAINTAINERS                                   |   1 +
>   tools/testing/selftests/vfio/Makefile         |   1 +
>   .../selftests/vfio/lib/vfio_pci_device.c      |  57 +-
>   .../selftests/vfio/vfio_cxl_type2_test.c      | 799 ++++++++++++++++++
>   4 files changed, 855 insertions(+), 3 deletions(-)
>   create mode 100644 tools/testing/selftests/vfio/vfio_cxl_type2_test.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b9361a8d618e..192b1681b3bd 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -28319,6 +28319,7 @@ L:	linux-cxl@vger.kernel.org
>   S:	Supported
>   F:	Documentation/driver-api/vfio-pci-cxl.rst
>   F:	drivers/vfio/pci/cxl/
> +F:	tools/testing/selftests/vfio/vfio_cxl_type2_test.c
>   
>   VFIO DRIVER
>   M:	Alex Williamson <alex@shazbot.org>
> diff --git a/tools/testing/selftests/vfio/Makefile b/tools/testing/selftests/vfio/Makefile
> index 2c32c48db509..08f88e88cb4d 100644
> --- a/tools/testing/selftests/vfio/Makefile
> +++ b/tools/testing/selftests/vfio/Makefile
> @@ -13,6 +13,7 @@ TEST_GEN_PROGS += vfio_pci_device_test
>   TEST_GEN_PROGS += vfio_pci_device_init_perf_test
>   TEST_GEN_PROGS += vfio_pci_driver_test
>   TEST_GEN_PROGS += vfio_pci_sriov_uapi_test
> +TEST_GEN_PROGS += vfio_cxl_type2_test
>   
>   TEST_FILES += scripts/cleanup.sh
>   TEST_FILES += scripts/lib.sh
> diff --git a/tools/testing/selftests/vfio/lib/vfio_pci_device.c b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> index 94dc5fcecbeb..ab49b41653c4 100644
> --- a/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> +++ b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> @@ -160,9 +160,31 @@ static void vfio_pci_region_get(struct vfio_pci_device *device, int index,
>   	ioctl_assert(device->fd, VFIO_DEVICE_GET_REGION_INFO, info);
>   }
>   
> +/* Return the sparse-mmap capability in @info, or NULL if the region has none. */
> +static struct vfio_region_info_cap_sparse_mmap *
> +vfio_pci_sparse_mmap_cap(struct vfio_region_info *info)
> +{
> +	struct vfio_info_cap_header *hdr;
> +	u32 offset;
> +
> +	if (!(info->flags & VFIO_REGION_INFO_FLAG_CAPS))
> +		return NULL;
> +
> +	for (offset = info->cap_offset; offset; offset = hdr->next) {
> +		hdr = (void *)info + offset;
> +		if (hdr->id == VFIO_REGION_INFO_CAP_SPARSE_MMAP)
> +			return (struct vfio_region_info_cap_sparse_mmap *)hdr;
> +	}
> +
> +	return NULL;
> +}
> +
>   static void vfio_pci_bar_map(struct vfio_pci_device *device, int index)
>   {
>   	struct vfio_pci_bar *bar = &device->bars[index];
> +	struct vfio_region_info_cap_sparse_mmap *sparse;
> +	u8 infobuf[1024] = {};
> +	struct vfio_region_info *info = (void *)infobuf;
>   	size_t align, size;
>   	int prot = 0;
>   	void *vaddr;
> @@ -190,9 +212,38 @@ static void vfio_pci_bar_map(struct vfio_pci_device *device, int index)
>   	align = min_t(size_t, size, SZ_1G);
>   
>   	vaddr = mmap_reserve(size, align, 0);
> -	bar->vaddr = mmap(vaddr, size, prot, MAP_SHARED | MAP_FIXED,
> -			  device->fd, bar->info.offset);
> -	VFIO_ASSERT_NE(bar->vaddr, MAP_FAILED);
> +
> +	/*
> +	 * A BAR that is only partially mmappable, such as a CXL Type-2 component
> +	 * BAR with the HDM decoder block trapped, advertises the mmappable
> +	 * ranges through a sparse-mmap capability. Map each area within the
> +	 * reservation and leave the excluded ranges unmapped; mapping the whole
> +	 * BAR would be rejected.
> +	 */
> +	info->argsz = sizeof(infobuf);
> +	info->index = index;
> +	ioctl_assert(device->fd, VFIO_DEVICE_GET_REGION_INFO, info);
> +	sparse = vfio_pci_sparse_mmap_cap(info);
> +	if (sparse) {
> +		u32 i;
> +
> +		bar->vaddr = vaddr;
> +		for (i = 0; i < sparse->nr_areas; i++) {
> +			void *p;
> +
> +			if (!sparse->areas[i].size)
> +				continue;
> +			p = mmap(vaddr + sparse->areas[i].offset,
> +				 sparse->areas[i].size, prot,
> +				 MAP_SHARED | MAP_FIXED, device->fd,
> +				 bar->info.offset + sparse->areas[i].offset);
> +			VFIO_ASSERT_NE(p, MAP_FAILED);
> +		}
> +	} else {
> +		bar->vaddr = mmap(vaddr, size, prot, MAP_SHARED | MAP_FIXED,
> +				  device->fd, bar->info.offset);
> +		VFIO_ASSERT_NE(bar->vaddr, MAP_FAILED);
> +	}
>   
>   	madvise(bar->vaddr, size, MADV_HUGEPAGE);
>   }
> diff --git a/tools/testing/selftests/vfio/vfio_cxl_type2_test.c b/tools/testing/selftests/vfio/vfio_cxl_type2_test.c
> new file mode 100644
> index 000000000000..8c23ddd014ca
> --- /dev/null
> +++ b/tools/testing/selftests/vfio/vfio_cxl_type2_test.c
> @@ -0,0 +1,799 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * vfio_cxl_type2_test - corner-case tests for the vfio-cxl kernel contract.
> + *
> + * Exercises the user-visible surface the vfio-cxl module adds to a CXL Type-2
> + * device: the two VFIO regions (HDM memory and the trapped HDM decoder block),
> + * the component-register geometry capability, and the lock-on-commit decoder
> + * FSM the kernel runs on the trapped block.
> + *
> + * Unlike a plain vfio-pci device the guest programs its own endpoint decoder,
> + * so the trapped block enforces the commit handshake and freezes a locked
> + * decoder. These tests drive that FSM directly. Writes to the decoder block
> + * land in the per-open kernel shadow only, never on the physical decoder, and
> + * each test reopens the device (fresh shadow), so the FSM tests are safe to
> + * repeat and do not leak state between tests.
> + *
> + * Usage: ./vfio_cxl_type2_test <BDF>  (or export VFIO_SELFTESTS_BDF=<BDF>).
> + * The device must be bound to vfio-pci with the vfio-cxl module available.
> + *
> + * Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES.
> + */
> +
> +#include <fcntl.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +
> +#include <sys/ioctl.h>
> +#include <sys/mman.h>
> +
> +#include <linux/pci_regs.h>
> +#include <linux/sizes.h>
> +#include <linux/vfio.h>
> +
> +#include <cxl/cxl_regs.h>
> +
> +#include <libvfio.h>
> +
> +#include "kselftest_harness.h"
> +
> +#define PCI_DVSEC_VENDOR_ID_CXL		0x1e98
> +#define PCI_DVSEC_ID_CXL_DEVICE		0x0000
> +
> +/* CXL r3.1 8.1.9.1: Register Block Identifier for the component registers. */
> +#define CXL_REGLOC_RBI_COMPONENT	1
> +
> +/*
> + * Register Locator DVSEC block-1 field masks. The uapi pci_regs.h names expand
> + * to __GENMASK(), which is not a macro in this userspace include path, so use
> + * explicit values.
> + */
> +#define REG_LOCATOR_BIR_MASK		0x00000007
> +#define REG_LOCATOR_BLOCK_ID_MASK	0x0000ff00
> +#define REG_LOCATOR_BLOCK_OFF_LOW_MASK	0xffff0000
> +
> +/*
> + * vfio-pci's region-offset packing is kernel-internal (vfio_pci_core.h), not
> + * UAPI. Define it locally; the guards let a future kernel hoist it to UAPI.
> + */
> +#ifndef VFIO_PCI_OFFSET_SHIFT
> +#define VFIO_PCI_OFFSET_SHIFT		40
> +#endif
> +#ifndef VFIO_PCI_INDEX_TO_OFFSET
> +#define VFIO_PCI_INDEX_TO_OFFSET(i)	((uint64_t)(i) << VFIO_PCI_OFFSET_SHIFT)
> +#endif
> +
> +static const char *device_bdf;
> +
> +/* Locate a region-info capability by id inside a GET_REGION_INFO buffer. */
> +static const struct vfio_info_cap_header *
> +find_region_cap(const void *buf, size_t bufsz, uint16_t id)
> +{
> +	const struct vfio_region_info *ri = buf;
> +	const struct vfio_info_cap_header *cap;
> +	size_t off = ri->cap_offset;
> +
> +	while (off && off + sizeof(*cap) <= bufsz) {
> +		cap = (const void *)((const char *)buf + off);
> +		if (cap->id == id)
> +			return cap;
> +		off = cap->next;
> +	}
> +	return NULL;
> +}
> +
> +/*
> + * Find a CXL region by scanning every region's VFIO_REGION_INFO_CAP_TYPE for
> + * the CXL type and the requested subtype. Returns the region index or -1.
> + * @buf is a caller scratch buffer left holding the matched region's info
> + * (with caps).
> + */
> +static int find_cxl_region(int fd, uint32_t nregions, uint32_t subtype,
> +			   void *buf, size_t bufsz)
> +{
> +	uint32_t i;
> +
> +	for (i = 0; i < nregions; i++) {
> +		struct vfio_region_info *ri = buf;
> +		const struct vfio_region_info_cap_type *t;
> +		const struct vfio_info_cap_header *hdr;
> +
> +		memset(buf, 0, bufsz);
> +		ri->argsz = bufsz;
> +		ri->index = i;
> +		if (ioctl(fd, VFIO_DEVICE_GET_REGION_INFO, ri))
> +			continue;
> +		if (!(ri->flags & VFIO_REGION_INFO_FLAG_CAPS))
> +			continue;
> +
> +		hdr = find_region_cap(buf, bufsz, VFIO_REGION_INFO_CAP_TYPE);
> +		if (!hdr)
> +			continue;
> +		t = (const void *)hdr;
> +		if (t->type == VFIO_REGION_TYPE_CXL && t->subtype == subtype)
> +			return i;
> +	}
> +	return -1;
> +}
> +
> +/* Walk the PCI extended capability list for the CXL Device DVSEC. */
> +static uint16_t find_cxl_dvsec(struct vfio_pci_device *dev)
> +{
> +	uint16_t pos = PCI_CFG_SPACE_SIZE;
> +	int iter = 0;
> +
> +	while (pos && iter++ < 64) {
> +		uint32_t hdr = vfio_pci_config_readl(dev, pos);
> +		uint16_t cap_id = hdr & 0xffff;
> +		uint16_t next = (hdr >> 20) & 0xffc;
> +		uint32_t h1, h2;
> +
> +		if (cap_id == PCI_EXT_CAP_ID_DVSEC) {
> +			h1 = vfio_pci_config_readl(dev, pos + 4);
> +			h2 = vfio_pci_config_readl(dev, pos + 8);
> +			if ((h1 & 0xffff) == PCI_DVSEC_VENDOR_ID_CXL &&
> +			    (h2 & 0xffff) == PCI_DVSEC_ID_CXL_DEVICE)
> +				return pos;
> +		}
> +		pos = next;
> +	}
> +	return 0;
> +}
> +
> +FIXTURE(vfio_cxl) {
> +	struct iommu *iommu;
> +	struct vfio_pci_device *dev;
> +
> +	int mem_idx;
> +	uint64_t mem_size;
> +	uint32_t mem_flags;
> +	int comp_idx;
> +	uint64_t comp_size;
> +	uint32_t comp_bar;
> +	uint64_t comp_offset;	/* HDM block offset within comp_bar */
> +	uint64_t comp_off;	/* mmap/rw base offset of the comp region */
> +	uint16_t dvsec;
> +};
> +
> +FIXTURE_SETUP(vfio_cxl)
> +{
> +	uint8_t infobuf[512] = {};
> +	struct vfio_device_info *info = (void *)infobuf;
> +	const struct vfio_region_info_cap_cxl_comp_regs *geo;
> +	const struct vfio_info_cap_header *hdr;
> +	uint8_t rbuf[1024];
> +
> +	self->iommu = iommu_init(default_iommu_mode);
> +	self->dev = vfio_pci_device_init(device_bdf, self->iommu);
> +
> +	info->argsz = sizeof(infobuf);
> +	ASSERT_EQ(0, ioctl(self->dev->fd, VFIO_DEVICE_GET_INFO, info));
> +
> +	if (!(info->flags & VFIO_DEVICE_FLAGS_CXL))
> +		SKIP(return, "not a CXL Type-2 device");
> +
> +	self->mem_idx = find_cxl_region(self->dev->fd, info->num_regions,
> +					VFIO_REGION_SUBTYPE_CXL_MEM,
> +					rbuf, sizeof(rbuf));
> +	ASSERT_GE(self->mem_idx, 0);
> +	self->mem_size = ((struct vfio_region_info *)rbuf)->size;
> +	self->mem_flags = ((struct vfio_region_info *)rbuf)->flags;
> +
> +	self->comp_idx = find_cxl_region(self->dev->fd, info->num_regions,
> +					 VFIO_REGION_SUBTYPE_CXL_COMP_REGS,
> +					 rbuf, sizeof(rbuf));
> +	ASSERT_GE(self->comp_idx, 0);
> +	self->comp_size = ((struct vfio_region_info *)rbuf)->size;
> +
> +	/* The geometry cap rides on the component-register region. */
> +	hdr = find_region_cap(rbuf, sizeof(rbuf),
> +			      VFIO_REGION_INFO_CAP_CXL_COMP_REGS);
> +	ASSERT_NE(NULL, hdr);
> +	geo = (const void *)hdr;
> +	self->comp_bar = geo->bar;
> +	self->comp_offset = geo->offset;
> +
> +	self->comp_off = VFIO_PCI_INDEX_TO_OFFSET(self->comp_idx);
> +	self->dvsec = find_cxl_dvsec(self->dev);
> +}
> +
> +FIXTURE_TEARDOWN(vfio_cxl)
> +{
> +	vfio_pci_device_cleanup(self->dev);
> +	iommu_cleanup(self->iommu);
> +}
> +
> +/* GET_INFO advertises the flag and both CXL regions with a sane geometry cap. */
> +TEST_F(vfio_cxl, device_is_cxl)
> +{
> +	ASSERT_NE(self->mem_idx, self->comp_idx);
> +	ASSERT_GT(self->mem_size, 0);
> +	ASSERT_GT(self->comp_size, 0);
> +	ASSERT_LT(self->comp_bar, PCI_STD_NUM_BARS);
> +	/* The HDM memory must advertise mmap; a VMM needs it for stage-2. */
> +	ASSERT_NE(0, self->mem_flags & VFIO_REGION_INFO_FLAG_MMAP);
> +}
> +
> +/*
> + * The component BAR carries the physical HDM decoder block, which vfio traps
> + * and excludes from mmap so the guest cannot reprogram it directly. Mapping the
> + * whole BAR must fail; mapping the ranges around the excluded block, as the
> + * sparse-mmap capability advertises, must succeed.
> + */
> +TEST_F(vfio_cxl, comp_bar_sparse_mmap)
> +{
> +	size_t page_size = getpagesize();
> +	uint8_t rbuf[1024] = {};
> +	struct vfio_region_info *ri = (void *)rbuf;
> +	const struct vfio_region_info_cap_sparse_mmap *sm;
> +	const struct vfio_info_cap_header *hdr;
> +	uint64_t bar_off, decoder_page;
> +	void *map;
> +	uint32_t i;
> +
> +	/* Region info for the component BAR, with capabilities. */
> +	ri->argsz = sizeof(rbuf);
> +	ri->index = self->comp_bar;
> +	ASSERT_EQ(0, ioctl(self->dev->fd, VFIO_DEVICE_GET_REGION_INFO, ri));
> +	ASSERT_NE(0, ri->flags & VFIO_REGION_INFO_FLAG_MMAP);
> +	bar_off = ri->offset;
> +
> +	/* The trapped decoder block splits the BAR, so it must be sparse. */
> +	hdr = find_region_cap(rbuf, sizeof(rbuf),
> +			      VFIO_REGION_INFO_CAP_SPARSE_MMAP);
> +	ASSERT_NE(NULL, hdr);
> +	sm = (const void *)hdr;
> +	ASSERT_GT(sm->nr_areas, 0);
> +
> +	/* Mapping the whole BAR must fail: it covers the excluded block. */
> +	map = mmap(NULL, ri->size, PROT_READ | PROT_WRITE, MAP_SHARED,
> +		   self->dev->fd, bar_off);
> +	ASSERT_EQ(MAP_FAILED, map);
> +
> +	/* Every advertised area is page aligned and must map. */
> +	for (i = 0; i < sm->nr_areas; i++) {
> +		uint64_t ao = sm->areas[i].offset;
> +		uint64_t as = sm->areas[i].size;
> +
> +		if (!as)
> +			continue;
> +		ASSERT_EQ(0, ao & (page_size - 1));
> +		ASSERT_EQ(0, as & (page_size - 1));
> +
> +		map = mmap(NULL, as, PROT_READ | PROT_WRITE, MAP_SHARED,
> +			   self->dev->fd, bar_off + ao);
> +		ASSERT_NE(MAP_FAILED, map);
> +		ASSERT_EQ(0, munmap(map, as));
> +	}
> +
> +	/* The page holding the decoder block must never be mmappable. */
> +	decoder_page = self->comp_offset & ~(uint64_t)(page_size - 1);
> +	map = mmap(NULL, page_size, PROT_READ | PROT_WRITE, MAP_SHARED,
> +		   self->dev->fd, bar_off + decoder_page);
> +	ASSERT_EQ(MAP_FAILED, map);
> +}
> +
> +/* mmap one page of the HDM memory, write a pattern, read it back. */
> +TEST_F(vfio_cxl, hdm_mem_mmap_rw)
> +{
> +	uint64_t off = VFIO_PCI_INDEX_TO_OFFSET(self->mem_idx);
> +	uint32_t pattern = 0xdeadbeefU, readback = 0;
> +	void *map;
> +
> +	if (self->mem_size < SZ_4K)
> +		SKIP(return, "HDM memory < 4K");
> +
> +	map = mmap(NULL, SZ_4K, PROT_READ | PROT_WRITE, MAP_SHARED,
> +		   self->dev->fd, off);
> +	ASSERT_NE(MAP_FAILED, map);
> +
> +	memcpy(map, &pattern, sizeof(pattern));
> +	memcpy(&readback, map, sizeof(readback));
> +	ASSERT_EQ(pattern, readback);
> +
> +	ASSERT_EQ(0, munmap(map, SZ_4K));
> +}
> +
> +/*
> + * A 2 MB-aligned window should map as a huge (PMD) fault. The kernel falls back
> + * to base pages when it cannot, so only correctness (write/read) is asserted.
> + */
> +TEST_F(vfio_cxl, hdm_mem_huge_mmap)
> +{
> +	uint64_t off = VFIO_PCI_INDEX_TO_OFFSET(self->mem_idx);
> +	uint32_t pattern = 0x5a5a5a5aU, readback = 0;
> +	void *map, *last;
> +
> +	if (self->mem_size < SZ_2M)
> +		SKIP(return, "HDM memory < 2M");
> +
> +	map = mmap(NULL, SZ_2M, PROT_READ | PROT_WRITE, MAP_SHARED,
> +		   self->dev->fd, off);
> +	ASSERT_NE(MAP_FAILED, map);
> +
> +	/* Touch the last dword so a 2 MB PMD fault covers the whole window. */
> +	last = (char *)map + SZ_2M - sizeof(pattern);
> +	memcpy(last, &pattern, sizeof(pattern));
> +	memcpy(&readback, last, sizeof(readback));
> +	ASSERT_EQ(pattern, readback);
> +
> +	ASSERT_EQ(0, munmap(map, SZ_2M));
> +}
> +
> +/*
> + * A guest driver disables and re-enables PCI Memory-Space during init and
> + * reset. The committed HDM decoder stays valid across that toggle, so once
> + * Memory-Space is re-enabled the coherent HDM memory must be reachable again
> + * without a reset. This is the regression test for the hdm_valid access gate
> + * being cleared by a Memory-Space disable and never restored, which left a
> + * later valid mmap fault wrongly SIGBUS-ing.
> + *
> + * The region is exercised only through the mmap path (as a VMM does) and only
> + * while Memory-Space is enabled. An access with Memory-Space disabled aborts
> + * on the fabric as a fatal host error, so the test never attempts one: the
> + * toggle in between is pure config-space writes.
> + */
> +TEST_F(vfio_cxl, hdm_mem_survives_mem_space_toggle)
> +{
> +	uint64_t off = VFIO_PCI_INDEX_TO_OFFSET(self->mem_idx);
> +	uint32_t pattern = 0x12345678U, readback = 0;
> +	uint16_t cmd;
> +	void *map;
> +
> +	if (self->mem_size < SZ_4K)
> +		SKIP(return, "HDM memory < 4K");
> +
> +	/* Seed a known pattern through the mmap path with Memory-Space on. */
> +	cmd = vfio_pci_config_readw(self->dev, PCI_COMMAND);
> +	vfio_pci_config_writew(self->dev, PCI_COMMAND,
> +			       cmd | PCI_COMMAND_MEMORY);
> +	map = mmap(NULL, SZ_4K, PROT_READ | PROT_WRITE, MAP_SHARED,
> +		   self->dev->fd, off);
> +	ASSERT_NE(MAP_FAILED, map);
> +	memcpy(map, &pattern, sizeof(pattern));
> +	ASSERT_EQ(0, munmap(map, SZ_4K));
> +
> +	/*
> +	 * Toggle Memory-Space off and back on with no HDM access in between,
> +	 * as a guest driver does during init/reset.
> +	 */
> +	vfio_pci_config_writew(self->dev, PCI_COMMAND,
> +			       cmd & ~PCI_COMMAND_MEMORY);
> +	vfio_pci_config_writew(self->dev, PCI_COMMAND,
> +			       cmd | PCI_COMMAND_MEMORY);
> +
> +	/*
> +	 * The committed decoder stayed valid across the toggle, so a fresh mmap
> +	 * fault succeeds and the seeded pattern reads back, without a reset.
> +	 * Before the fix the gate was cleared by the disable and never restored,
> +	 * so the fault wrongly SIGBUS-ed.
> +	 */
> +	map = mmap(NULL, SZ_4K, PROT_READ | PROT_WRITE, MAP_SHARED,
> +		   self->dev->fd, off);
> +	ASSERT_NE(MAP_FAILED, map);
> +	memcpy(&readback, map, sizeof(readback));
> +	ASSERT_EQ(pattern, readback);
> +	ASSERT_EQ(0, munmap(map, SZ_4K));
> +
> +	/* Restore PCI_COMMAND. */
> +	vfio_pci_config_writew(self->dev, PCI_COMMAND, cmd);
> +}
> +
> +/*
> + * Mirror how a VMM uses the region: mmap the HDM memory and map it into the
> + * IOAS (stage-2) so the device can reach it over ATS. The mmap flag is required
> + * for that path, so its absence is a failure, not a skip. The host CPU does not
> + * dereference the mapping; the guest reaches it through stage-2.
> + */
> +TEST_F(vfio_cxl, hdm_mem_ioas_map)
> +{
> +	uint64_t off = VFIO_PCI_INDEX_TO_OFFSET(self->mem_idx);
> +	struct iova_allocator *iova_alloc;
> +	struct dma_region region;
> +	void *map;
> +
> +	ASSERT_NE(0, self->mem_flags & VFIO_REGION_INFO_FLAG_MMAP);
> +
> +	/* iova_allocator_alloc() requires a power-of-2 size. */
> +	if (self->mem_size < SZ_2M)
> +		SKIP(return, "HDM memory < 2M");
> +
> +	map = mmap(NULL, SZ_2M, PROT_READ | PROT_WRITE, MAP_SHARED,
> +		   self->dev->fd, off);
> +	ASSERT_NE(MAP_FAILED, map);
> +
> +	iova_alloc = iova_allocator_init(self->iommu);
> +	region.vaddr = map;
> +	region.size = SZ_2M;
> +	region.iova = iova_allocator_alloc(iova_alloc, SZ_2M);
> +
> +	iommu_map(self->iommu, &region);

Hi Manish,

We ran this series' selftests with a QEMU-emulated Type-2 device
(pxb-cxl host bridge, firmware-committed HDM decoder).  16 of the 17
tests pass; the one failure is hdm_mem_ioas_map:

 > 	iova_alloc = iova_allocator_init(self->iommu);
 > 	region.vaddr = map;
 > 	region.size = SZ_2M;
 > 	region.iova = iova_allocator_alloc(iova_alloc, SZ_2M);
 >
 > 	iommu_map(self->iommu, &region);

IOMMU_IOAS_MAP (the vaddr variant) on the mmap of the CXL_MEM region
fails with -EFAULT.  The path is:

	iommufd_ioas_map()
	  iopt_map_user_pages()
	    pfn_reader_user_pin()
	      pin_user_pages_fast()
	        check_vma_flags()		/* mm/gup.c */
	          if (vm_flags & (VM_IO | VM_PFNMAP))
	            return -EFAULT;

The CXL_MEM region is struct-page-less device memory, so vfio_cxl_core.c
creates the VMA with VM_IO | VM_PFNMAP.  pin_user_pages() refuses such
VMAs outright and pfn_reader_user_pin() has no fallback, so with the
current upstream iommufd the vaddr variant of IOMMU_IOAS_MAP cannot map
this region at all, and the test's unconditional assertion fails.

The test documents a real part of the contract (the mmap is what lets
the device reach its DPA through stage-2), so rather than have reviewers
read this as a series regression, maybe:

   - tolerate the current upstream behavior: skip (or xfail) when
     IOMMU_IOAS_MAP fails with -EFAULT on the PFNMAP VMA, with a comment
     that the vaddr path needs iommufd support for PFNMAP device memory;
     or
   - note the dependency in the cover letter.

FWIW, the dmabuf variant does not offer a way around this today either:
VFIO_DEVICE_FEATURE_DMA_BUF only exports BARs, while the CXL_MEM region
is a vendor region backed by the resolved HPA window, so there is
currently no upstream path at all to IOAS-map the HDM memory from
userspace.  If the vaddr path is meant to work eventually, it might be
worth saying which side owns that (iommufd pin fallback vs. a dmabuf
export for this region).

Everything else here works nicely, including the guest reset path and
the HDM shadow/commit FSM tests.

Test setup, in case it helps reproduction:

   - this series applied on an upstream-based tree
   - QEMU with pxb-cxl and an emulated Type-2 device whose decoder is
     firmware-committed at boot
   - result: 16/17 pass, hdm_mem_ioas_map fails with -EFAULT from
     pin_user_pages_fast()

Thanks,
Shuai Xue



  reply	other threads:[~2026-08-26  7:29 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-13  9:36 [PATCH v4 00/27] vfio/pci: Add CXL Type-2 device passthrough support mhonap
2026-08-13  9:36 ` [PATCH v4 01/27] cxl: Fix resource.c include path and export cxl_restore_hdm_after_pci_reset mhonap
2026-08-21 22:52   ` Jonathan Cameron
2026-08-22  1:22     ` Manish Honap
2026-08-13  9:36 ` [PATCH v4 02/27] cxl/regs: Skip sub-block region request for BAR-owning drivers mhonap
2026-08-25 21:26   ` Alex Williamson
2026-08-13  9:36 ` [PATCH v4 03/27] cxl: Move component register defines to uapi/cxl/cxl_regs.h mhonap
2026-08-13  9:36 ` [PATCH v4 04/27] cxl: Establish media readiness in cxl_mem_probe() mhonap
2026-08-25 22:18   ` Alex Williamson
2026-08-13  9:36 ` [PATCH v4 05/27] cxl: Add a function-scoped reset entry for vfio-pci mhonap
2026-08-25 23:11   ` Alex Williamson
2026-08-13  9:36 ` [PATCH v4 06/27] vfio/pci: Add CXL ops registration interface mhonap
2026-08-26 21:11   ` Alex Williamson
2026-08-13  9:36 ` [PATCH v4 07/27] vfio/pci: Detect CXL devices and load vfio-cxl on demand mhonap
2026-08-26 22:17   ` Alex Williamson
2026-08-13  9:36 ` [PATCH v4 08/27] vfio/cxl: Add the vfio-cxl module skeleton mhonap
2026-08-26 22:50   ` Alex Williamson
2026-08-13  9:36 ` [PATCH v4 09/27] vfio/cxl: Create the CXL memory device at bind mhonap
2026-08-13  9:36 ` [PATCH v4 10/27] vfio/cxl: Reject unsupported decoder topologies " mhonap
2026-08-13  9:36 ` [PATCH v4 11/27] vfio/cxl: Own the whole component register BAR mhonap
2026-08-13  9:36 ` [PATCH v4 12/27] vfio/pci: Let a provider exclude a BAR sub-range from mmap mhonap
2026-08-13  9:36 ` [PATCH v4 13/27] vfio/pci: Refuse read/write to an excluded BAR sub-range mhonap
2026-08-13  9:36 ` [PATCH v4 14/27] vfio: Add CXL region type for the HDM region mhonap
2026-08-13  9:36 ` [PATCH v4 15/27] vfio/pci: Call CXL open and close hooks around device use mhonap
2026-08-13  9:36 ` [PATCH v4 16/27] vfio/cxl: Shadow the CXL DVSEC body at open mhonap
2026-08-13  9:36 ` [PATCH v4 17/27] vfio/cxl: Virtualize the CXL DVSEC mhonap
2026-08-13  9:36 ` [PATCH v4 18/27] vfio/cxl: Expose the HDM memory and trap the decoder registers mhonap
2026-08-13  9:36 ` [PATCH v4 19/27] vfio/cxl: Keep the HDM decoder block off the direct BAR mapping mhonap
2026-08-13  9:36 ` [PATCH v4 20/27] vfio/cxl: Emulate the HDM decoder commit handshake mhonap
2026-08-13  9:36 ` [PATCH v4 21/27] vfio/cxl: Describe the CXL device and decoder geometry to userspace mhonap
2026-08-13  9:36 ` [PATCH v4 22/27] vfio/cxl: Revoke the HDM mapping on reset and power transitions mhonap
2026-08-13  9:36 ` [PATCH v4 23/27] vfio/cxl: Refresh the decoder snapshot after a device reset mhonap
2026-08-13  9:36 ` [PATCH v4 24/27] vfio/cxl: Service a guest-triggered CXL reset mhonap
2026-08-13  9:36 ` [PATCH v4 25/27] vfio/pci: Provide an opt-out for the CXL Type-2 extensions mhonap
2026-08-13  9:36 ` [PATCH v4 26/27] Documentation: vfio-pci: Document CXL Type-2 device passthrough mhonap
2026-08-13  9:36 ` [PATCH v4 27/27] selftests/vfio: Add CXL Type-2 passthrough corner-case tests mhonap
2026-08-26  7:28   ` Shuai Xue [this message]
2026-08-26 16:17     ` Manish Honap

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8b6a3086-c491-4eff-938f-aecaac53b813@linux.alibaba.com \
    --to=xueshuai@linux.alibaba.com \
    --cc=alejandro.lucero-palau@amd.com \
    --cc=alex@shazbot.org \
    --cc=alison.schofield@intel.com \
    --cc=ankita@nvidia.com \
    --cc=bhelgaas@google.com \
    --cc=cjia@nvidia.com \
    --cc=corbet@lwn.net \
    --cc=dave.jiang@intel.com \
    --cc=dave@stgolabs.net \
    --cc=dmatlack@google.com \
    --cc=gustavoars@kernel.org \
    --cc=iweiny@kernel.org \
    --cc=jgg@ziepe.ca \
    --cc=jic23@kernel.org \
    --cc=kees@kernel.org \
    --cc=kevin.tian@intel.com \
    --cc=kjaju@nvidia.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-cxl@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mhonap@nvidia.com \
    --cc=ming.li@zohomail.com \
    --cc=skhan@linuxfoundation.org \
    --cc=skolothumtho@nvidia.com \
    --cc=smadhavan@nvidia.com \
    --cc=vishal.l.verma@intel.com \
    --cc=vsethi@nvidia.com \
    --cc=yishaih@nvidia.com \
    --cc=zhiw@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox