RE: [PATCH v4 27/27] selftests/vfio: Add CXL Type-2 passthrough corner-case tests
From: Manish Honap
Date: Wed Aug 26 2026 - 12:20:10 EST
> -----Original Message-----
> From: Shuai Xue <xueshuai@xxxxxxxxxxxxxxxxx>
> Sent: Wednesday, August 26, 2026 12:59 PM
> To: Manish Honap <mhonap@xxxxxxxxxx>; alex@xxxxxxxxxxx; jgg@xxxxxxxx;
> Ankit Agrawal <ankita@xxxxxxxxxx>; jic23@xxxxxxxxxx; dave.jiang@xxxxxxxxx;
> alejandro.lucero-palau@xxxxxxx; Srirangan Madhavan
> <smadhavan@xxxxxxxxxx>; corbet@xxxxxxx; skhan@xxxxxxxxxxxxxxxxxxx;
> dave@xxxxxxxxxxxx; alison.schofield@xxxxxxxxx; vishal.l.verma@xxxxxxxxx;
> iweiny@xxxxxxxxxx; ming.li@xxxxxxxxxxxx; Yishai Hadas
> <yishaih@xxxxxxxxxx>; Shameer Kolothum Thodi
> <skolothumtho@xxxxxxxxxx>; kevin.tian@xxxxxxxxx; bhelgaas@xxxxxxxxxx;
> dmatlack@xxxxxxxxxx; kees@xxxxxxxxxx; gustavoars@xxxxxxxxxx
> Cc: Neo Jia <cjia@xxxxxxxxxx>; Krishnakant Jaju <kjaju@xxxxxxxxxx>; Vikram
> Sethi <vsethi@xxxxxxxxxx>; Zhi Wang <zhiw@xxxxxxxxxx>; linux-
> doc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx;
> linux-cxl@xxxxxxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; linux-
> kselftest@xxxxxxxxxxxxxxx; linux-hardening@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v4 27/27] selftests/vfio: Add CXL Type-2 passthrough
> corner-case tests
>
> External email: Use caution opening links or attachments
>
>
> On 8/13/26 5:36 PM, mhonap@xxxxxxxxxx wrote:
> > From: Manish Honap <mhonap@xxxxxxxxxx>
> >
> > 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@xxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxxx
> > 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@xxxxxxxxxxx> 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, ®ion);
>
> 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, ®ion);
>
> 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
>
Hello Shuai,
Thanks for applying the patch series with dependencies and running the tests.
My test tree carried an out-of-tree iommufd patch that adds a fallback to the
exact function you point at, pfn_reader_user_pin(). When the gup pin fails on
a VM_PFNMAP VMA, it follows the PFNs with follow_pfnmap_start()/fixup_user_fault()
instead of pinning pages, and only for struct-page-less PFNs.
With that in place the IOAS map succeeds; on a clean upstream tree like yours
it correctly returns -EFAULT. I did mention this in the cover letter, but I should have
provide more concrete details.
For v5 I will do both things you suggest:
- Make hdm_mem_ioas_map tolerant: call the non-asserting __iommu_map()
and SKIP/xfail on -EFAULT, with a comment that the vaddr path needs
iommufd support for struct-page-less device memory.
- Note the dependency in the cover letter instead of leaving it implicit.
- Include the dma-buf import implementation for CXL.mem and not run selftests
With any out-of-tree patches.
I intend the final path for vfio-cxl series to be a dma-buf export of the CXL_MEM region,
imported by iommufd, rather than an iommufd pin fallback.
--
One request: could you please share which QEMU patches you used for the emulated
Type-2 device? I would like to reproduce your exact setup.
Specifically:
- which QEMU tree/branch and version
- the device model you instantiated (for example cxl-type3 adapted, or an
accelerator/Type-2 model, and how it advertises the CACHE + MEM DVSEC so
vfio-cxl treats it as Type-2)
- how the HDM decoder is firmware-committed at boot, and the full pxb-cxl /
cxl-rp / CFMWS command line.
Thanks,
Manish