>-----Original Message-----
>From: Cédric Le Goater <c...@redhat.com>
>Subject: [PATCH for-10.1 v2 11/37] vfio: Introduce new files for VFIORegion
>definitions and declarations
>
>Gather all VFIORegion related declarations and definitions into their
>own files to reduce exposure of VFIO internals in "hw/vfio/vfio-common.h".
>They were introduced for 'vfio-platform' support in commits
>db0da029a185 ("vfio: Generalize region support") and a664477db8da
>("hw/vfio/pci: Introduce VFIORegion").
>
>To be noted that the 'vfio-platform' devices have been deprecated and
>will be removed in QEMU 10.2. Until then, make the declarations
>available externally for 'sysbus-fdt.c'.
>
>Cc: Eric Auger <eric.au...@redhat.com>
>Signed-off-by: Cédric Le Goater <c...@redhat.com>
>---
> hw/vfio/vfio-display.h | 1 +
> include/hw/vfio/vfio-common.h | 32 +--
> include/hw/vfio/vfio-platform.h | 2 +
> include/hw/vfio/vfio-region.h | 47 ++++
> hw/core/sysbus-fdt.c | 1 +
> hw/vfio/helpers.c | 363 -----------------------------
> hw/vfio/pci-quirks.c | 1 +
> hw/vfio/pci.c | 1 +
> hw/vfio/platform.c | 1 +
> hw/vfio/region.c | 394 ++++++++++++++++++++++++++++++++
> hw/vfio/meson.build | 1 +
> hw/vfio/trace-events | 16 +-
> 12 files changed, 459 insertions(+), 401 deletions(-)
> create mode 100644 include/hw/vfio/vfio-region.h
> create mode 100644 hw/vfio/region.c
>
>diff --git a/hw/vfio/vfio-display.h b/hw/vfio/vfio-display.h
>index
>99b8cb67ef7558d3eefe3105a831e3fcb30afc4d..2606c34b396a88cec3e8f884adb
>158e48e8105f1 100644
>--- a/hw/vfio/vfio-display.h
>+++ b/hw/vfio/vfio-display.h
>@@ -11,6 +11,7 @@
>
> #include "ui/console.h"
> #include "hw/display/ramfb.h"
>+#include "hw/vfio/vfio-region.h"
>
> typedef struct VFIODMABuf {
> QemuDmaBuf *buf;
>diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
>index
>8d48f5300a791d8858fe29d1bb905f814ef11990..3d470d79325526e0508683c445
>a7635c78a57e34 100644
>--- a/include/hw/vfio/vfio-common.h
>+++ b/include/hw/vfio/vfio-common.h
>@@ -39,25 +39,6 @@ enum {
> VFIO_DEVICE_TYPE_CCW = 2,
> VFIO_DEVICE_TYPE_AP = 3,
> };
>-
>-typedef struct VFIOMmap {
>- MemoryRegion mem;
>- void *mmap;
>- off_t offset;
>- size_t size;
>-} VFIOMmap;
>-
>-typedef struct VFIORegion {
>- struct VFIODevice *vbasedev;
>- off_t fd_offset; /* offset of region within device fd */
>- MemoryRegion *mem; /* slow, read/write access */
>- size_t size;
>- uint32_t flags; /* VFIO region flags (rd/wr/mmap) */
>- uint32_t nr_mmaps;
>- VFIOMmap *mmaps;
>- uint8_t nr; /* cache the region number for debug */
>-} VFIORegion;
>-
> struct VFIOGroup;
>
> typedef struct VFIOContainer {
>@@ -168,17 +149,7 @@ void vfio_unmask_single_irqindex(VFIODevice
>*vbasedev, int index);
> void vfio_mask_single_irqindex(VFIODevice *vbasedev, int index);
> bool vfio_set_irq_signaling(VFIODevice *vbasedev, int index, int subindex,
> int action, int fd, Error **errp);
>-void vfio_region_write(void *opaque, hwaddr addr,
>- uint64_t data, unsigned size);
>-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);
>-int vfio_region_mmap(VFIORegion *region);
>-void vfio_region_mmaps_set_enabled(VFIORegion *region, bool enabled);
>-void vfio_region_unmap(VFIORegion *region);
>-void vfio_region_exit(VFIORegion *region);
>-void vfio_region_finalize(VFIORegion *region);
>+
> void vfio_reset_handler(void *opaque);
> struct vfio_device_info *vfio_get_device_info(int fd);
> bool vfio_device_is_mdev(VFIODevice *vbasedev);
>@@ -194,7 +165,6 @@ int vfio_kvm_device_del_fd(int fd, Error **errp);
> bool vfio_cpr_register_container(VFIOContainerBase *bcontainer, Error **errp);
> void vfio_cpr_unregister_container(VFIOContainerBase *bcontainer);
>
>-extern const MemoryRegionOps vfio_region_ops;
> typedef QLIST_HEAD(VFIOGroupList, VFIOGroup) VFIOGroupList;
> typedef QLIST_HEAD(VFIODeviceList, VFIODevice) VFIODeviceList;
> extern VFIOGroupList vfio_group_list;
>diff --git a/include/hw/vfio/vfio-platform.h b/include/hw/vfio/vfio-platform.h
>index
>c414c3dffcc840a2f40a1b252d0f7b4e309c78d4..3191545717da51abc41d10cd364
>6cd047b4a676c 100644
>--- a/include/hw/vfio/vfio-platform.h
>+++ b/include/hw/vfio/vfio-platform.h
>@@ -47,6 +47,8 @@ typedef struct VFIOINTp {
> /* function type for user side eventfd handler */
> typedef void (*eventfd_user_side_handler_t)(VFIOINTp *intp);
>
>+typedef struct VFIORegion VFIORegion;
>+
> struct VFIOPlatformDevice {
> SysBusDevice sbdev;
> VFIODevice vbasedev; /* not a QOM object */
>diff --git a/include/hw/vfio/vfio-region.h b/include/hw/vfio/vfio-region.h
>new file mode 100644
>index
>0000000000000000000000000000000000000000..9dc0535e7ce46fbb671e70918
>b93cb115857efe6
>--- /dev/null
>+++ b/include/hw/vfio/vfio-region.h
>@@ -0,0 +1,47 @@
>+/*
>+ * VFIO region
>+ *
>+ * Copyright Red Hat, Inc. 2025
>+ *
>+ * SPDX-License-Identifier: GPL-2.0-or-later
>+ */
>+
>+#ifndef HW_VFIO_REGION_H
>+#define HW_VFIO_REGION_H
>+
>+#include "exec/memory.h"
>+
>+typedef struct VFIOMmap {
>+ MemoryRegion mem;
>+ void *mmap;
>+ off_t offset;
>+ size_t size;
>+} VFIOMmap;
>+
>+typedef struct VFIODevice VFIODevice;
>+
>+typedef struct VFIORegion {
>+ struct VFIODevice *vbasedev;
>+ off_t fd_offset; /* offset of region within device fd */
>+ MemoryRegion *mem; /* slow, read/write access */
>+ size_t size;
>+ uint32_t flags; /* VFIO region flags (rd/wr/mmap) */
>+ uint32_t nr_mmaps;
>+ VFIOMmap *mmaps;
>+ uint8_t nr; /* cache the region number for debug */
>+} VFIORegion;
>+
>+
>+void vfio_region_write(void *opaque, hwaddr addr,
>+ uint64_t data, unsigned size);
>+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);
>+int vfio_region_mmap(VFIORegion *region);
>+void vfio_region_mmaps_set_enabled(VFIORegion *region, bool enabled);
>+void vfio_region_unmap(VFIORegion *region);
>+void vfio_region_exit(VFIORegion *region);
>+void vfio_region_finalize(VFIORegion *region);
>+
>+#endif /* HW_VFIO_REGION_H */
>diff --git a/hw/core/sysbus-fdt.c b/hw/core/sysbus-fdt.c
>index
>e85066b905637b1ca34b5965f383df341f3a4eb7..c339a27875cbee8131b064674a
>a09adf4b9efa25 100644
>--- a/hw/core/sysbus-fdt.c
>+++ b/hw/core/sysbus-fdt.c
>@@ -35,6 +35,7 @@
> #include "hw/vfio/vfio-platform.h"
> #include "hw/vfio/vfio-calxeda-xgmac.h"
> #include "hw/vfio/vfio-amd-xgbe.h"
>+#include "hw/vfio/vfio-region.h"
> #include "hw/display/ramfb.h"
> #include "hw/uefi/var-service-api.h"
> #include "hw/arm/fdt.h"
>diff --git a/hw/vfio/helpers.c b/hw/vfio/helpers.c
>index
>4b255d4f3a9e81f55df00c68fc71da769fd5bd04..89403943a7a219e491b6812d50b
>27b7f1fd7b4a4 100644
>--- a/hw/vfio/helpers.c
>+++ b/hw/vfio/helpers.c
>@@ -147,118 +147,6 @@ bool vfio_set_irq_signaling(VFIODevice *vbasedev, int
>index, int subindex,
> return false;
> }
>
>-/*
>- * IO Port/MMIO - Beware of the endians, VFIO is always little endian
>- */
>-void vfio_region_write(void *opaque, hwaddr addr,
>- uint64_t data, unsigned size)
>-{
>- VFIORegion *region = opaque;
>- VFIODevice *vbasedev = region->vbasedev;
>- union {
>- uint8_t byte;
>- uint16_t word;
>- uint32_t dword;
>- uint64_t qword;
>- } buf;
>-
>- switch (size) {
>- case 1:
>- buf.byte = data;
>- break;
>- case 2:
>- buf.word = cpu_to_le16(data);
>- break;
>- case 4:
>- buf.dword = cpu_to_le32(data);
>- break;
>- case 8:
>- buf.qword = cpu_to_le64(data);
>- break;
>- default:
>- hw_error("vfio: unsupported write size, %u bytes", size);
>- break;
>- }
>-
>- if (pwrite(vbasedev->fd, &buf, size, region->fd_offset + addr) != size) {
>- error_report("%s(%s:region%d+0x%"HWADDR_PRIx", 0x%"PRIx64
>- ",%d) failed: %m",
>- __func__, vbasedev->name, region->nr,
>- addr, data, size);
>- }
>-
>- trace_vfio_region_write(vbasedev->name, region->nr, addr, data, size);
>-
>- /*
>- * A read or write to a BAR always signals an INTx EOI. This will
>- * do nothing if not pending (including not in INTx mode). We assume
>- * that a BAR access is in response to an interrupt and that BAR
>- * accesses will service the interrupt. Unfortunately, we don't know
>- * which access will service the interrupt, so we're potentially
>- * getting quite a few host interrupts per guest interrupt.
>- */
>- vbasedev->ops->vfio_eoi(vbasedev);
>-}
>-
>-uint64_t vfio_region_read(void *opaque,
>- hwaddr addr, unsigned size)
>-{
>- VFIORegion *region = opaque;
>- VFIODevice *vbasedev = region->vbasedev;
>- union {
>- uint8_t byte;
>- uint16_t word;
>- uint32_t dword;
>- uint64_t qword;
>- } buf;
>- uint64_t data = 0;
>-
>- if (pread(vbasedev->fd, &buf, size, region->fd_offset + addr) != size) {
>- error_report("%s(%s:region%d+0x%"HWADDR_PRIx", %d) failed: %m",
>- __func__, vbasedev->name, region->nr,
>- addr, size);
>- return (uint64_t)-1;
>- }
>- switch (size) {
>- case 1:
>- data = buf.byte;
>- break;
>- case 2:
>- data = le16_to_cpu(buf.word);
>- break;
>- case 4:
>- data = le32_to_cpu(buf.dword);
>- break;
>- case 8:
>- data = le64_to_cpu(buf.qword);
>- break;
>- default:
>- hw_error("vfio: unsupported read size, %u bytes", size);
>- break;
>- }
>-
>- trace_vfio_region_read(vbasedev->name, region->nr, addr, size, data);
>-
>- /* Same as write above */
>- vbasedev->ops->vfio_eoi(vbasedev);
>-
>- return data;
>-}
>-
>-const MemoryRegionOps vfio_region_ops = {
>- .read = vfio_region_read,
>- .write = vfio_region_write,
>- .endianness = DEVICE_LITTLE_ENDIAN,
>- .valid = {
>- .min_access_size = 1,
>- .max_access_size = 8,
>- },
>- .impl = {
>- .min_access_size = 1,
>- .max_access_size = 8,
>- },
>-};
>-
> int vfio_bitmap_alloc(VFIOBitmap *vbmap, hwaddr size)
> {
> vbmap->pages = REAL_HOST_PAGE_ALIGN(size) / qemu_real_host_page_size();
>@@ -306,257 +194,6 @@ vfio_get_device_info_cap(struct vfio_device_info
>*info, uint16_t id)
> return vfio_get_cap((void *)info, info->cap_offset, id);
> }
>
>-static int vfio_setup_region_sparse_mmaps(VFIORegion *region,
>- struct vfio_region_info *info)
>-{
>- struct vfio_info_cap_header *hdr;
>- struct vfio_region_info_cap_sparse_mmap *sparse;
>- int i, j;
>-
>- hdr = vfio_get_region_info_cap(info,
>VFIO_REGION_INFO_CAP_SPARSE_MMAP);
>- if (!hdr) {
>- return -ENODEV;
>- }
>-
>- sparse = container_of(hdr, struct vfio_region_info_cap_sparse_mmap,
>header);
>-
>- trace_vfio_region_sparse_mmap_header(region->vbasedev->name,
>- region->nr, sparse->nr_areas);
>-
>- region->mmaps = g_new0(VFIOMmap, sparse->nr_areas);
>-
>- for (i = 0, j = 0; i < sparse->nr_areas; i++) {
>- if (sparse->areas[i].size) {
>- trace_vfio_region_sparse_mmap_entry(i, sparse->areas[i].offset,
>- sparse->areas[i].offset +
>- sparse->areas[i].size - 1);
>- region->mmaps[j].offset = sparse->areas[i].offset;
>- region->mmaps[j].size = sparse->areas[i].size;
>- j++;
>- }
>- }
>-
>- region->nr_mmaps = j;
>- region->mmaps = g_realloc(region->mmaps, j * sizeof(VFIOMmap));
>-
>- return 0;
>-}
>-
>-int vfio_region_setup(Object *obj, VFIODevice *vbasedev, VFIORegion *region,
>- int index, const char *name)
>-{
>- g_autofree struct vfio_region_info *info = NULL;
>- int ret;
>-
>- ret = vfio_get_region_info(vbasedev, index, &info);
>- if (ret) {
>- return ret;
>- }
>-
>- region->vbasedev = vbasedev;
>- region->flags = info->flags;
>- region->size = info->size;
>- region->fd_offset = info->offset;
>- region->nr = index;
>-
>- if (region->size) {
>- region->mem = g_new0(MemoryRegion, 1);
>- memory_region_init_io(region->mem, obj, &vfio_region_ops,
>- region, name, region->size);
>-
>- if (!vbasedev->no_mmap &&
>- region->flags & VFIO_REGION_INFO_FLAG_MMAP) {
>-
>- ret = vfio_setup_region_sparse_mmaps(region, info);
>-
>- if (ret) {
>- region->nr_mmaps = 1;
>- region->mmaps = g_new0(VFIOMmap, region->nr_mmaps);
>- region->mmaps[0].offset = 0;
>- region->mmaps[0].size = region->size;
>- }
>- }
>- }
>-
>- trace_vfio_region_setup(vbasedev->name, index, name,
>- region->flags, region->fd_offset, region->size);
>- return 0;
>-}
>-
>-static void vfio_subregion_unmap(VFIORegion *region, int index)
>-{
>- trace_vfio_region_unmap(memory_region_name(®ion-
>>mmaps[index].mem),
>- region->mmaps[index].offset,
>- region->mmaps[index].offset +
>- region->mmaps[index].size - 1);
>- memory_region_del_subregion(region->mem, ®ion->mmaps[index].mem);
>- munmap(region->mmaps[index].mmap, region->mmaps[index].size);
>- object_unparent(OBJECT(®ion->mmaps[index].mem));
>- region->mmaps[index].mmap = NULL;
>-}
>-
>-int vfio_region_mmap(VFIORegion *region)
>-{
>- int i, ret, prot = 0;
>- char *name;
>-
>- if (!region->mem) {
>- return 0;
>- }
>-
>- prot |= region->flags & VFIO_REGION_INFO_FLAG_READ ? PROT_READ : 0;
>- prot |= region->flags & VFIO_REGION_INFO_FLAG_WRITE ? PROT_WRITE : 0;
>-
>- for (i = 0; i < region->nr_mmaps; i++) {
>- size_t align = MIN(1ULL << ctz64(region->mmaps[i].size), 1 * GiB);
>- void *map_base, *map_align;
>-
>- /*
>- * Align the mmap for more efficient mapping in the kernel. Ideally
>- * we'd know the PMD and PUD mapping sizes to use as discrete
>alignment
>- * intervals, but we don't. As of Linux v6.12, the largest PUD size
>- * supporting huge pfnmap is 1GiB (ARCH_SUPPORTS_PUD_PFNMAP is only
>set
>- * on x86_64). Align by power-of-two size, capped at 1GiB.
>- *
>- * NB. qemu_memalign() and friends actually allocate memory, whereas
>- * the region size here can exceed host memory, therefore we manually
>- * create an oversized anonymous mapping and clean it up for
>alignment.
>- */
>- map_base = mmap(0, region->mmaps[i].size + align, PROT_NONE,
>- MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>- if (map_base == MAP_FAILED) {
>- ret = -errno;
>- goto no_mmap;
>- }
>-
>- map_align = (void *)ROUND_UP((uintptr_t)map_base, (uintptr_t)align);
>- munmap(map_base, map_align - map_base);
>- munmap(map_align + region->mmaps[i].size,
>- align - (map_align - map_base));
>-
>- region->mmaps[i].mmap = mmap(map_align, region->mmaps[i].size, prot,
>- MAP_SHARED | MAP_FIXED,
>- region->vbasedev->fd,
>- region->fd_offset +
>- region->mmaps[i].offset);
>- if (region->mmaps[i].mmap == MAP_FAILED) {
>- ret = -errno;
>- goto no_mmap;
>- }
>-
>- name = g_strdup_printf("%s mmaps[%d]",
>- memory_region_name(region->mem), i);
>- memory_region_init_ram_device_ptr(®ion->mmaps[i].mem,
>- memory_region_owner(region->mem),
>- name, region->mmaps[i].size,
>- region->mmaps[i].mmap);
>- g_free(name);
>- memory_region_add_subregion(region->mem, region->mmaps[i].offset,
>- ®ion->mmaps[i].mem);
>-
>- trace_vfio_region_mmap(memory_region_name(®ion->mmaps[i].mem),
>- region->mmaps[i].offset,
>- region->mmaps[i].offset +
>- region->mmaps[i].size - 1);
>- }
>-
>- return 0;
>-
>-no_mmap:
>- trace_vfio_region_mmap_fault(memory_region_name(region->mem), i,
>- region->fd_offset + region->mmaps[i].offset,
>- region->fd_offset + region->mmaps[i].offset +
>- region->mmaps[i].size - 1, ret);
>-
>- region->mmaps[i].mmap = NULL;
>-
>- for (i--; i >= 0; i--) {
>- vfio_subregion_unmap(region, i);
>- }
>-
>- return ret;
>-}
>-
>-void vfio_region_unmap(VFIORegion *region)
>-{
>- int i;
>-
>- if (!region->mem) {
>- return;
>- }
>-
>- for (i = 0; i < region->nr_mmaps; i++) {
>- if (region->mmaps[i].mmap) {
>- vfio_subregion_unmap(region, i);
>- }
>- }
>-}
>-
>-void vfio_region_exit(VFIORegion *region)
>-{
>- int i;
>-
>- if (!region->mem) {
>- return;
>- }
>-
>- for (i = 0; i < region->nr_mmaps; i++) {
>- if (region->mmaps[i].mmap) {
>- memory_region_del_subregion(region->mem, ®ion->mmaps[i].mem);
>- }
>- }
>-
>- trace_vfio_region_exit(region->vbasedev->name, region->nr);
>-}
>-
>-void vfio_region_finalize(VFIORegion *region)
>-{
>- int i;
>-
>- if (!region->mem) {
>- return;
>- }
>-
>- for (i = 0; i < region->nr_mmaps; i++) {
>- if (region->mmaps[i].mmap) {
>- munmap(region->mmaps[i].mmap, region->mmaps[i].size);
>- object_unparent(OBJECT(®ion->mmaps[i].mem));
>- }
>- }
>-
>- object_unparent(OBJECT(region->mem));
>-
>- g_free(region->mem);
>- g_free(region->mmaps);
>-
>- trace_vfio_region_finalize(region->vbasedev->name, region->nr);
>-
>- region->mem = NULL;
>- region->mmaps = NULL;
>- region->nr_mmaps = 0;
>- region->size = 0;
>- region->flags = 0;
>- region->nr = 0;
>-}
>-
>-void vfio_region_mmaps_set_enabled(VFIORegion *region, bool enabled)
>-{
>- int i;
>-
>- if (!region->mem) {
>- return;
>- }
>-
>- for (i = 0; i < region->nr_mmaps; i++) {
>- if (region->mmaps[i].mmap) {
>- memory_region_set_enabled(®ion->mmaps[i].mem, enabled);
>- }
>- }
>-
>- trace_vfio_region_mmaps_set_enabled(memory_region_name(region->mem),
>- enabled);
>-}
>-
> int vfio_get_region_info(VFIODevice *vbasedev, int index,
> struct vfio_region_info **info)
> {
>diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
>index
>3f002252acfb7ac809107c99bdbdbaf66d56a50d..4591ec68da877b307f43ea1a83
>0c315721b57e9e 100644
>--- a/hw/vfio/pci-quirks.c
>+++ b/hw/vfio/pci-quirks.c
>@@ -26,6 +26,7 @@
> #include "hw/qdev-properties.h"
> #include "pci.h"
> #include "pci-quirks.h"
>+#include "hw/vfio/vfio-region.h"
This looks unnecessary as pci.h already include it,
> #include "trace.h"
>
> /*
>diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>index
>158deca06cb240622a254f5059c47873e5fcc7de..7457e81cdb07bc6657bb151434
>9c172a152cb540 100644
>--- a/hw/vfio/pci.c
>+++ b/hw/vfio/pci.c
>@@ -45,6 +45,7 @@
> #include "migration/qemu-file.h"
> #include "system/iommufd.h"
> #include "vfio-migration-internal.h"
>+#include "hw/vfio/vfio-region.h"
Same here, otherwise
Reviewed-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
Thanks
Zhenzhong
