[PATCH v5 0/9] PCI passthru on Hyper-V

[PATCH v5 0/9] PCI passthru on Hyper-V

[Mukesh R]

Implement passthru of PCI devices to unprivileged virtual machines
(VMs) when Linux is running as a privileged VM on Microsoft Hyper-V
hypervisor. This support is made to fit within the workings of VFIO
framework. This supports both full device passthru and SR-IOV based VFs.

At a high level, the hypervisor supports traditional paging iommu domains
that use explicit map and unmap hypercalls for mapping and unmapping
guest RAM into the iommu subsystem. Hyper-V also has a concept of direct
attach devices (DDA) whereby the iommu subsystem simply uses the guest
HW page table (ept/npt/..). This series adds support for the former.

This series rebased to: a4ffc59238be (origin/hyperv-next)

Testing:
 o Most testing done on hyperv-next:a4ffc59238be using Cloud Hypervisor (51).
 o All tests involved passthru of devices using MSIx.
 o Tested on Baremetal dom0/root: 
       test 1: Mellanox ConnectX-6 Lx passthru
       test 2: NVIDIA Tesla Tesla T4 GPU.
       test 3: Both of above simultaneous passthru
 o VF: Mellanox ConnectX-6 Lx passthru 

This series strives to establish a base line. Some pending work items:
 o arm64 : some delta to make this work on arm64 (in progress).
 o Qemu and OpenVMM support (in progress).
 o device sleep/wakeup.
 o More stress testing with high end GPUs

Changes in V5:
 o enforce no multiple vms per vmm instance
 o make struct mshv_vfio_device_ops const
 o add WARN to iommu call. delete mappings in domain free
 o mmio_mfn++ in large page check in hv_map_mmio_pages()
 o no need to pass gfn to hv_map_mmio_pages()
 o use region access to make mmio mappings ro or rw

Changes in V4:
 o Completely remove direct device attach (DDA) support 
 o Remove first two patches that were submitted separately and are
   already merged.
 o Change hv_iommu_unmap_pages() to unmap in batches of HV_REP_COUNT_MAX,
   otherwise if a physical range happens to be larger than that, there would 
   be errors from the hypercall.
 o Use large page (2M) flag for mmio mappings as internal testing found
   that to speed things up greatly.
 o rename hv_no_movbl_pgs to hv_memreg in last patch.

Changes in V3:
 o patch #8: fix compiler issues incase of !CONFIG_HYPERV. Also, do forward
   declaration of struct pci_dev instead of including pci.h.
 o patch #9: minor changes to comments. Pass hv_domain instead of 
   iommu_domain to hv_iommu_detach_dev() since that's what it needs. Set
   device private to null if attach fails. Clam down number of PFNs passed
   to hv_iommu_map_pgs().

Changes in V2:
 o rebase to 5170a82e8921
 o minor fixes for arm64 build
 o drop patch 03: "x86/hyperv: add insufficient memory support in irqdomain.c" 
     as it that path is no longer used
 o drop patch 08: "PCI: hv: rename hv_compose_msi_msg .. " and do it separately
   outside this series.
 o minor updates to commit messages

Changes in V1:
 o patch 1: Don't tie hyperv-irq.c to CONFIG_HYPERV_IOMMU.
 o patch 4: Redesigned to address security vulnerability found by copilot 
            with passing tgid as a parameter.  Also, do tgid setting right 
            after setting pt_id.
 o patch 5: Remove unused type parameter from mshv_device_ops.device_create
 o patch 7: mshv_partition_ioctl_create_device cleanup on copy_to_user.
 o patch 10: Add export of hv_build_devid_type_pci here to get rid of 
             patch 11.
 o patch 12: Move functions to build device ids from patch 11 here for
             the benefit of arm64. Rename file to: hyperv-iommu-root.c.
 o patch 13: removed to be made part of interrupt part II of this support.
 o patch 14: get rid of fast path to reduce review noise.
 o New (last) patch to pin ram regions if device passthru to a VM.

Thanks,
-Mukesh

Mukesh R (9):
  mshv: Provide a way to get partition ID if running in a VMM process
  mshv: Add declarations and definitions for VFIO-MSHV bridge device
  mshv: Introduce basic mshv bridge device for VFIO to build upon
  mshv: Add ioctl support for MSHV-VFIO bridge device
  mshv: Import data structs around device passthru from hyperv headers
  PCI: hv: Export hv_build_devid_type_pci() and change return type
  x86/hyperv: Implement Hyper-V virtual IOMMU
  mshv: Populate mmio mappings for PCI passthru
  mshv: Disable movable regions upfront if device passthru

 arch/x86/hyperv/irqdomain.c          |   9 +-
 arch/x86/include/asm/mshyperv.h      |   6 +
 arch/x86/kernel/pci-dma.c            |   2 +
 drivers/hv/Makefile                  |   3 +-
 drivers/hv/mshv_root.h               |  24 +-
 drivers/hv/mshv_root_hv_call.c       | 104 +++-
 drivers/hv/mshv_root_main.c          | 287 ++++++++++-
 drivers/hv/mshv_vfio.c               | 211 ++++++++
 drivers/iommu/Kconfig                |   1 +
 drivers/iommu/hyperv/Kconfig         |  15 +
 drivers/iommu/hyperv/Makefile        |   1 +
 drivers/iommu/hyperv/hv-iommu-root.c | 706 +++++++++++++++++++++++++++
 include/asm-generic/mshyperv.h       |   8 +
 include/hyperv/hvgdk_mini.h          |   8 +
 include/hyperv/hvhdk_mini.h          |  67 +++
 include/linux/hyperv.h               |   6 +
 include/uapi/linux/mshv.h            |  31 ++
 17 files changed, 1440 insertions(+), 49 deletions(-)
 create mode 100644 drivers/hv/mshv_vfio.c
 create mode 100644 drivers/iommu/hyperv/Kconfig
 create mode 100644 drivers/iommu/hyperv/hv-iommu-root.c

-- 
2.51.2.vfs.0.1

[PATCH v5 1/9] mshv: Provide a way to get partition ID if running in a VMM process

[Mukesh R]

Many PCI passthru related hypercalls require partition ID of the target
guest. Guests are actually managed by MSHV driver and the partition ID
is only maintained there. Add a field in the partition struct in MSHV
driver to save the tgid of the VMM process creating the partition, and
add a function there to retrieve partition ID if the current process is
a VMM process. Furthermore, enforce one partition per VMM instance.

Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
Reviewed-by: Anirudh Rayabharam (Microsoft) <anirudh@anirudhrb.com>
---
 drivers/hv/mshv_root.h         |  1 +
 drivers/hv/mshv_root_main.c    | 33 ++++++++++++++++++++++++++++++++-
 include/asm-generic/mshyperv.h |  5 +++++
 3 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/drivers/hv/mshv_root.h b/drivers/hv/mshv_root.h
index 1f086dcb7aa1..a85c24dcc701 100644
--- a/drivers/hv/mshv_root.h
+++ b/drivers/hv/mshv_root.h
@@ -138,6 +138,7 @@ struct mshv_partition {
 
 	struct mshv_girq_routing_table __rcu *pt_girq_tbl;
 	u64 isolation_type;
+	pid_t pt_vmm_tgid;
 	bool import_completed;
 	bool pt_initialized;
 #if IS_ENABLED(CONFIG_DEBUG_FS)
diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
index bd1359eb58dd..9b53c476bd83 100644
--- a/drivers/hv/mshv_root_main.c
+++ b/drivers/hv/mshv_root_main.c
@@ -1908,17 +1908,48 @@ mshv_partition_release(struct inode *inode, struct file *filp)
 	return 0;
 }
 
+/* Given a process tgid, return partition id if it is a VMM process */
+u64 mshv_current_partid(void)
+{
+	struct mshv_partition *pt;
+	int i;
+	u64 ret_ptid = HV_PARTITION_ID_INVALID;
+
+	rcu_read_lock();
+
+	hash_for_each_rcu(mshv_root.pt_htable, i, pt, pt_hnode) {
+		if (pt->pt_vmm_tgid == current->tgid) {
+			ret_ptid = pt->pt_id;
+			break;
+		}
+	}
+
+	rcu_read_unlock();
+	return ret_ptid;
+}
+EXPORT_SYMBOL_GPL(mshv_current_partid);
+
+/* At present, we only allow one partition per VMM instance */
 static int
 add_partition(struct mshv_partition *partition)
 {
+	int rc = 0;
+
 	spin_lock(&mshv_root.pt_ht_lock);
 
+	if (mshv_current_partid() != HV_PARTITION_ID_INVALID) {
+		rc = -EEXIST;
+		goto out;
+	}
+
+	partition->pt_vmm_tgid = current->tgid;
 	hash_add_rcu(mshv_root.pt_htable, &partition->pt_hnode,
 		     partition->pt_id);
 
+out:
 	spin_unlock(&mshv_root.pt_ht_lock);
 
-	return 0;
+	return rc;
 }
 
 static_assert(MSHV_NUM_CPU_FEATURES_BANKS ==
diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
index bf601d67cecb..e8cbc4e3f7ad 100644
--- a/include/asm-generic/mshyperv.h
+++ b/include/asm-generic/mshyperv.h
@@ -350,6 +350,7 @@ int hv_call_add_logical_proc(int node, u32 lp_index, u32 acpi_id);
 int hv_call_notify_all_processors_started(void);
 bool hv_lp_exists(u32 lp_index);
 int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u32 flags);
+u64 mshv_current_partid(void);
 
 #else /* CONFIG_MSHV_ROOT */
 static inline bool hv_root_partition(void) { return false; }
@@ -380,6 +381,10 @@ static inline int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u3
 {
 	return -EOPNOTSUPP;
 }
+static inline u64 mshv_current_partid(void)
+{
+	return HV_PARTITION_ID_INVALID;
+}
 #endif /* CONFIG_MSHV_ROOT */
 
 static inline int hv_deposit_memory(u64 partition_id, u64 status)
-- 
2.51.2.vfs.0.1

Re: [PATCH v5 1/9] mshv: Provide a way to get partition ID if running in a VMM process

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Using `current->tgid` for implicit VM partition binding restricts multi-process architectures, introduces PID reuse vulnerabilities (Privilege Escalation and DoS), and causes spurious creation failures.

Pre-existing issues:
- [Critical] Unchecked virtual processor (VP) access in `mshv_try_assert_irq_fast` can lead to a kernel panic.
--

--- Patch [1]: [PATCH v5 1/9] mshv: Provide a way to get partition ID if running in a VMM process ---
commit c09a2034eaac3e66996c48aebdd226e834f0611e
Author: Mukesh R <mrathor@linux.microsoft.com>

mshv: Provide a way to get partition ID if running in a VMM process

This commit adds a field to the MSHV partition structure to save the
tgid of the VMM process that created it. It also introduces a helper to
retrieve the partition ID by matching the current tgid and restricts
creation to one partition per VMM instance.

> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> --- a/drivers/hv/mshv_root_main.c
> +++ b/drivers/hv/mshv_root_main.c
[ ... ]
> +u64 mshv_current_partid(void)
> +{
> +	struct mshv_partition *pt;
> +	int i;
> +	u64 ret_ptid = HV_PARTITION_ID_INVALID;
> +
> +	rcu_read_lock();
> +
> +	hash_for_each_rcu(mshv_root.pt_htable, i, pt, pt_hnode) {
> +		if (pt->pt_vmm_tgid == current->tgid) {
> +			ret_ptid = pt->pt_id;
> +			break;
> +		}
> +	}
> +
> +	rcu_read_unlock();
> +	return ret_ptid;
> +}
> +EXPORT_SYMBOL_GPL(mshv_current_partid);
> +
> +/* At present, we only allow one partition per VMM instance */
>  static int
>  add_partition(struct mshv_partition *partition)
>  {
> +	int rc = 0;
> +
>  	spin_lock(&mshv_root.pt_ht_lock);
>  
> +	if (mshv_current_partid() != HV_PARTITION_ID_INVALID) {
> +		rc = -EEXIST;
> +		goto out;
> +	}
> +
> +	partition->pt_vmm_tgid = current->tgid;

[Severity: High]
Does using current->tgid for implicit VM partition binding introduce risks
with process lifecycle and PID reuse?

If a VMM process creates a partition, passes the file descriptor to another
process (e.g., via a UNIX socket) and exits, the original tgid could be
recycled for a new, unrelated process.

If that new process performs operations that invoke mshv_current_partid(),
could it incorrectly inherit the partition context of the previous process?

Alternatively, if the new process attempts to create its own partition in
add_partition(), would it incorrectly fail with -EEXIST?

[Severity: Critical]
This is a pre-existing issue, but I noticed a potential unchecked access
during the review of this subsystem.

In drivers/hv/mshv_eventfd.c:mshv_try_assert_irq_fast():

    vp = partition->pt_vp_array[irq->lapic_apic_id];

    if (!vp->vp_register_page) {
        ...

If userspace sets up an irqfd routing entry with an APIC ID that has no
corresponding virtual processor created yet, will vp be NULL when fetched
from pt_vp_array?

If so, would the immediate dereference of vp->vp_register_page result in a
host kernel panic when the eventfd is signaled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731223427.2554388-1-mrathor@linux.microsoft.com?part=1

[PATCH v5 2/9] mshv: Add declarations and definitions for VFIO-MSHV bridge device

[Mukesh R]

Add data structs needed by the subsequent patch that introduces a new
module to implement VFIO-MSHV pseudo device.

Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
Reviewed-by: Souradeep Chakrabarti <schakrabarti@linux.microsoft.com>
---
 drivers/hv/mshv_root.h    | 19 +++++++++++++++++++
 include/uapi/linux/mshv.h | 30 ++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/drivers/hv/mshv_root.h b/drivers/hv/mshv_root.h
index a85c24dcc701..43114df3cbf0 100644
--- a/drivers/hv/mshv_root.h
+++ b/drivers/hv/mshv_root.h
@@ -227,6 +227,25 @@ struct port_table_info {
 	};
 };
 
+struct mshv_device {
+	const struct mshv_device_ops *device_ops;
+	struct mshv_partition *device_pt;
+	void *device_private;
+	struct hlist_node device_ptnode;
+};
+
+struct mshv_device_ops {
+	const char *device_name;
+	long (*device_create)(struct mshv_device *dev);
+	void (*device_release)(struct mshv_device *dev);
+	long (*device_set_attr)(struct mshv_device *dev,
+				struct mshv_device_attr *attr);
+	long (*device_has_attr)(struct mshv_device *dev,
+				struct mshv_device_attr *attr);
+};
+
+extern const struct mshv_device_ops mshv_vfio_device_ops;
+
 int mshv_update_routing_table(struct mshv_partition *partition,
 			      const struct mshv_user_irq_entry *entries,
 			      unsigned int numents);
diff --git a/include/uapi/linux/mshv.h b/include/uapi/linux/mshv.h
index 32ff92b6342b..be6fe3ee8707 100644
--- a/include/uapi/linux/mshv.h
+++ b/include/uapi/linux/mshv.h
@@ -404,4 +404,34 @@ struct mshv_sint_mask {
 /* hv_hvcall device */
 #define MSHV_HVCALL_SETUP        _IOW(MSHV_IOCTL, 0x1E, struct mshv_vtl_hvcall_setup)
 #define MSHV_HVCALL              _IOWR(MSHV_IOCTL, 0x1F, struct mshv_vtl_hvcall)
+
+/* Device passhthru */
+#define MSHV_CREATE_DEVICE_TEST		1
+
+enum {
+	MSHV_DEV_TYPE_VFIO,
+	MSHV_DEV_TYPE_MAX,
+};
+
+struct mshv_create_device {
+	__u32	type;	     /* in: MSHV_DEV_TYPE_xxx */
+	__u32	fd;	     /* out: device handle */
+	__u32	flags;	     /* in: MSHV_CREATE_DEVICE_xxx */
+};
+
+#define MSHV_DEV_VFIO_FILE      1
+#define MSHV_DEV_VFIO_FILE_ADD	1
+#define MSHV_DEV_VFIO_FILE_DEL	2
+
+struct mshv_device_attr {
+	__u32	flags;		/* no flags currently defined */
+	__u32	group;		/* device-defined */
+	__u64	attr;		/* group-defined */
+	__u64	addr;		/* userspace address of attr data */
+};
+
+/* Device fds created with MSHV_CREATE_DEVICE */
+#define MSHV_SET_DEVICE_ATTR	_IOW(MSHV_IOCTL, 0x00, struct mshv_device_attr)
+#define MSHV_HAS_DEVICE_ATTR	_IOW(MSHV_IOCTL, 0x01, struct mshv_device_attr)
+
 #endif
-- 
2.51.2.vfs.0.1

Re: [PATCH v5 2/9] mshv: Add declarations and definitions for VFIO-MSHV bridge device

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `MSHV_CREATE_DEVICE` ioctl command definition is completely missing from the UAPI header.
--

--- Patch [2]: [PATCH v5 2/9] mshv: Add declarations and definitions for VFIO-MSHV bridge device ---
commit e1e53f07045678558c5d600cce2f941f492fd69b
Author: Mukesh R <mrathor@linux.microsoft.com>

mshv: Add declarations and definitions for VFIO-MSHV bridge device

Add data structs needed by the subsequent patch that introduces a new
module to implement VFIO-MSHV pseudo device.

> diff --git a/include/uapi/linux/mshv.h b/include/uapi/linux/mshv.h
> index 32ff92b6342b2..be6fe3ee87078 100644
> --- a/include/uapi/linux/mshv.h
> +++ b/include/uapi/linux/mshv.h
> @@ -404,4 +404,34 @@ struct mshv_sint_mask {

[ ... ]

> +/* Device passhthru */
> +#define MSHV_CREATE_DEVICE_TEST		1
> +
> +enum {
> +	MSHV_DEV_TYPE_VFIO,
> +	MSHV_DEV_TYPE_MAX,
> +};
> +
> +struct mshv_create_device {
> +	__u32	type;	     /* in: MSHV_DEV_TYPE_xxx */
> +	__u32	fd;	     /* out: device handle */
> +	__u32	flags;	     /* in: MSHV_CREATE_DEVICE_xxx */
> +};
> +
> +#define MSHV_DEV_VFIO_FILE      1
> +#define MSHV_DEV_VFIO_FILE_ADD	1
> +#define MSHV_DEV_VFIO_FILE_DEL	2
> +
> +struct mshv_device_attr {
> +	__u32	flags;		/* no flags currently defined */
> +	__u32	group;		/* device-defined */
> +	__u64	attr;		/* group-defined */
> +	__u64	addr;		/* userspace address of attr data */
> +};
> +
> +/* Device fds created with MSHV_CREATE_DEVICE */
> +#define MSHV_SET_DEVICE_ATTR	_IOW(MSHV_IOCTL, 0x00, struct mshv_device_attr)
> +#define MSHV_HAS_DEVICE_ATTR	_IOW(MSHV_IOCTL, 0x01, struct mshv_device_attr)

[Severity: Medium]
Is the definition for the MSHV_CREATE_DEVICE ioctl missing here?

The comment right above mentions device fds created with MSHV_CREATE_DEVICE,
and the patch adds struct mshv_create_device, but the actual ioctl macro
is never defined. Would this prevent userspace from compiling against the
header to use the device passthrough feature?

> +
>  #endif

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731223427.2554388-1-mrathor@linux.microsoft.com?part=2

[PATCH v5 3/9] mshv: Introduce basic mshv bridge device for VFIO to build upon

[Mukesh R]

Add a new file to implement basic VFIO-MSHV bridge pseudo device. These
functions are called in the VFIO framework, and credits to kvm/vfio.c
as this file was adapted from it. This is a basic version to build upon.

Co-developed-by: Wei Liu <wei.liu@kernel.org>
Signed-off-by: Wei Liu <wei.liu@kernel.org>
Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
---
 drivers/hv/Makefile       |   3 +-
 drivers/hv/mshv_vfio.c    | 211 ++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/mshv.h |   1 +
 3 files changed, 214 insertions(+), 1 deletion(-)
 create mode 100644 drivers/hv/mshv_vfio.c

diff --git a/drivers/hv/Makefile b/drivers/hv/Makefile
index 888a748cc7cb..9ab6fc254c38 100644
--- a/drivers/hv/Makefile
+++ b/drivers/hv/Makefile
@@ -14,7 +14,8 @@ hv_vmbus-y := vmbus_drv.o \
 hv_vmbus-$(CONFIG_HYPERV_TESTING)	+= hv_debugfs.o
 hv_utils-y := hv_util.o hv_kvp.o hv_snapshot.o hv_utils_transport.o
 mshv_root-y := mshv_root_main.o mshv_synic.o mshv_eventfd.o mshv_irq.o \
-	       mshv_root_hv_call.o mshv_portid_table.o mshv_regions.o
+	       mshv_root_hv_call.o mshv_portid_table.o mshv_regions.o  \
+               mshv_vfio.o
 mshv_root-$(CONFIG_DEBUG_FS) += mshv_debugfs.o
 mshv_root-$(CONFIG_TRACEPOINTS) += mshv_trace.o
 mshv_vtl-y := mshv_vtl_main.o
diff --git a/drivers/hv/mshv_vfio.c b/drivers/hv/mshv_vfio.c
new file mode 100644
index 000000000000..92cfbaef0328
--- /dev/null
+++ b/drivers/hv/mshv_vfio.c
@@ -0,0 +1,211 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * VFIO-MSHV bridge pseudo device
+ *
+ * Heavily inspired by the VFIO-KVM bridge pseudo device.
+ */
+#include <linux/errno.h>
+#include <linux/file.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/vfio.h>
+#include <asm/mshyperv.h>
+
+#include "mshv.h"
+#include "mshv_root.h"
+
+struct mshv_vfio_file {
+	struct list_head node;
+	struct file *file;	/* list of struct mshv_vfio_file */
+};
+
+struct mshv_vfio {
+	struct list_head file_list;
+	struct mutex lock;
+};
+
+static bool mshv_vfio_file_is_valid(struct file *file)
+{
+	bool (*fn)(struct file *file);
+	bool ret;
+
+	fn = symbol_get(vfio_file_is_valid);
+	if (!fn)
+		return false;
+
+	ret = fn(file);
+
+	symbol_put(vfio_file_is_valid);
+
+	return ret;
+}
+
+static long mshv_vfio_file_add(struct mshv_device *mshvdev, unsigned int fd)
+{
+	struct mshv_vfio *mshv_vfio = mshvdev->device_private;
+	struct mshv_vfio_file *mvf;
+	struct file *filp;
+	long ret = 0;
+
+	filp = fget(fd);
+	if (!filp)
+		return -EBADF;
+
+	/* Ensure the FD is a vfio FD. */
+	if (!mshv_vfio_file_is_valid(filp)) {
+		ret = -EINVAL;
+		goto out_fput;
+	}
+
+	mutex_lock(&mshv_vfio->lock);
+
+	list_for_each_entry(mvf, &mshv_vfio->file_list, node) {
+		if (mvf->file == filp) {
+			ret = -EEXIST;
+			goto out_unlock;
+		}
+	}
+
+	mvf = kzalloc(sizeof(*mvf), GFP_KERNEL_ACCOUNT);
+	if (!mvf) {
+		ret = -ENOMEM;
+		goto out_unlock;
+	}
+
+	mvf->file = get_file(filp);
+	list_add_tail(&mvf->node, &mshv_vfio->file_list);
+
+out_unlock:
+	mutex_unlock(&mshv_vfio->lock);
+out_fput:
+	fput(filp);
+	return ret;
+}
+
+static long mshv_vfio_file_del(struct mshv_device *mshvdev, unsigned int fd)
+{
+	struct mshv_vfio *mshv_vfio = mshvdev->device_private;
+	struct mshv_vfio_file *mvf;
+	long ret;
+
+	CLASS(fd, f)(fd);
+
+	if (fd_empty(f))
+		return -EBADF;
+
+	ret = -ENOENT;
+	mutex_lock(&mshv_vfio->lock);
+
+	list_for_each_entry(mvf, &mshv_vfio->file_list, node) {
+		if (mvf->file != fd_file(f))
+			continue;
+
+		list_del(&mvf->node);
+		fput(mvf->file);
+		kfree(mvf);
+		ret = 0;
+		break;
+	}
+
+	mutex_unlock(&mshv_vfio->lock);
+	return ret;
+}
+
+static long mshv_vfio_set_file(struct mshv_device *mshvdev, long attr,
+			      void __user *arg)
+{
+	int32_t __user *argp = arg;
+	int32_t fd;
+
+	switch (attr) {
+	case MSHV_DEV_VFIO_FILE_ADD:
+		if (get_user(fd, argp))
+			return -EFAULT;
+		return mshv_vfio_file_add(mshvdev, fd);
+
+	case MSHV_DEV_VFIO_FILE_DEL:
+		if (get_user(fd, argp))
+			return -EFAULT;
+		return mshv_vfio_file_del(mshvdev, fd);
+	}
+
+	return -ENXIO;
+}
+
+static long mshv_vfio_set_attr(struct mshv_device *mshvdev,
+			      struct mshv_device_attr *attr)
+{
+	switch (attr->group) {
+	case MSHV_DEV_VFIO_FILE:
+		return mshv_vfio_set_file(mshvdev, attr->attr,
+					  u64_to_user_ptr(attr->addr));
+	}
+
+	return -ENXIO;
+}
+
+static long mshv_vfio_has_attr(struct mshv_device *mshvdev,
+			      struct mshv_device_attr *attr)
+{
+	switch (attr->group) {
+	case MSHV_DEV_VFIO_FILE:
+		switch (attr->attr) {
+		case MSHV_DEV_VFIO_FILE_ADD:
+		case MSHV_DEV_VFIO_FILE_DEL:
+			return 0;
+		}
+
+		break;
+	}
+
+	return -ENXIO;
+}
+
+static long mshv_vfio_create_device(struct mshv_device *mshvdev)
+{
+	struct mshv_device *tmp;
+	struct mshv_vfio *mshv_vfio;
+
+	/* Only one VFIO "device" per VM */
+	hlist_for_each_entry(tmp, &mshvdev->device_pt->pt_devices,
+			     device_ptnode)
+		if (tmp->device_ops == &mshv_vfio_device_ops)
+			return -EBUSY;
+
+	mshv_vfio = kzalloc_obj(*mshv_vfio);
+	if (mshv_vfio == NULL)
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&mshv_vfio->file_list);
+	mutex_init(&mshv_vfio->lock);
+
+	mshvdev->device_private = mshv_vfio;
+
+	return 0;
+}
+
+/* This is called from mshv_device_fop_release() */
+static void mshv_vfio_release_device(struct mshv_device *mshvdev)
+{
+	struct mshv_vfio *mv = mshvdev->device_private;
+	struct mshv_vfio_file *mvf, *tmp;
+
+	list_for_each_entry_safe(mvf, tmp, &mv->file_list, node) {
+		fput(mvf->file);
+		list_del(&mvf->node);
+		kfree(mvf);
+	}
+
+	kfree(mv);
+	kfree(mshvdev);
+}
+
+const struct mshv_device_ops mshv_vfio_device_ops = {
+	.device_name = "mshv-vfio",
+	.device_create = mshv_vfio_create_device,
+	.device_release = mshv_vfio_release_device,
+	.device_set_attr = mshv_vfio_set_attr,
+	.device_has_attr = mshv_vfio_has_attr,
+};
diff --git a/include/uapi/linux/mshv.h b/include/uapi/linux/mshv.h
index be6fe3ee8707..b038a79786d2 100644
--- a/include/uapi/linux/mshv.h
+++ b/include/uapi/linux/mshv.h
@@ -254,6 +254,7 @@ struct mshv_root_hvcall {
 #define MSHV_GET_GPAP_ACCESS_BITMAP	_IOWR(MSHV_IOCTL, 0x06, struct mshv_gpap_access_bitmap)
 /* Generic hypercall */
 #define MSHV_ROOT_HVCALL		_IOWR(MSHV_IOCTL, 0x07, struct mshv_root_hvcall)
+#define MSHV_CREATE_DEVICE             _IOWR(MSHV_IOCTL, 0x08, struct mshv_create_device)
 
 /*
  ********************************
-- 
2.51.2.vfs.0.1

Re: [PATCH v5 3/9] mshv: Introduce basic mshv bridge device for VFIO to build upon

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `mshv_vfio` structure is allocated without `__GFP_ACCOUNT`, allowing a container to allocate kernel memory without being charged to its memory cgroup limit.
--

--- Patch [3]: [PATCH v5 3/9] mshv: Introduce basic mshv bridge device for VFIO to build upon ---
commit c0b4902f22a48f131a914dbd4da4237e2be4826b
Author: Mukesh R <mrathor@linux.microsoft.com>

mshv: Introduce basic mshv bridge device for VFIO to build upon

This commit adds a new file to implement a basic VFIO-MSHV bridge pseudo
device. It provides the foundation for VFIO integration in the MSHV
framework, adapting concepts from the KVM-VFIO bridge.

> diff --git a/drivers/hv/mshv_vfio.c b/drivers/hv/mshv_vfio.c
> new file mode 100644
> index 0000000000000..92cfbaef0328d
> --- /dev/null
> +++ b/drivers/hv/mshv_vfio.c

[ ... ]

> +static long mshv_vfio_create_device(struct mshv_device *mshvdev)
> +{
> +	struct mshv_device *tmp;
> +	struct mshv_vfio *mshv_vfio;
> +
> +	/* Only one VFIO "device" per VM */
> +	hlist_for_each_entry(tmp, &mshvdev->device_pt->pt_devices,
> +			     device_ptnode)
> +		if (tmp->device_ops == &mshv_vfio_device_ops)
> +			return -EBUSY;
> +
> +	mshv_vfio = kzalloc_obj(*mshv_vfio);

[Severity: Medium]
Should this allocation include __GFP_ACCOUNT?

When a user space process creates a new MSHV VFIO device, it triggers
mshv_vfio_create_device(). The kzalloc_obj() macro defaults to GFP_KERNEL,
which omits the necessary accounting flags.

This allows a container to allocate kernel memory without being charged to
its memory cgroup limit.

> +	if (mshv_vfio == NULL)
> +		return -ENOMEM;
> +
> +	INIT_LIST_HEAD(&mshv_vfio->file_list);
> +	mutex_init(&mshv_vfio->lock);
> +
> +	mshvdev->device_private = mshv_vfio;
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731223427.2554388-1-mrathor@linux.microsoft.com?part=3

[PATCH v5 4/9] mshv: Add ioctl support for MSHV-VFIO bridge device

[Mukesh R]

Add ioctl support for MSHV devices for a partition. At present only VFIO
device types are supported, but more could be added. At a high level, a
partition ioctl to create device verifies it is of type VFIO and does some
setup for bridge code in mshv_vfio.c.  Adapted from KVM device ioctls.

Co-developed-by: Wei Liu <wei.liu@kernel.org>
Signed-off-by: Wei Liu <wei.liu@kernel.org>
Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
---
 drivers/hv/mshv_root_main.c | 119 ++++++++++++++++++++++++++++++++++++
 1 file changed, 119 insertions(+)

diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
index 9b53c476bd83..79456388bba9 100644
--- a/drivers/hv/mshv_root_main.c
+++ b/drivers/hv/mshv_root_main.c
@@ -1625,6 +1625,122 @@ mshv_partition_ioctl_initialize(struct mshv_partition *partition)
 	return ret;
 }
 
+static long mshv_device_attr_ioctl(struct mshv_device *mshv_dev, int cmd,
+				   ulong uarg)
+{
+	struct mshv_device_attr attr;
+	const struct mshv_device_ops *devops = mshv_dev->device_ops;
+
+	if (copy_from_user(&attr, (void __user *)uarg, sizeof(attr)))
+		return -EFAULT;
+
+	switch (cmd) {
+	case MSHV_SET_DEVICE_ATTR:
+		if (devops->device_set_attr)
+			return devops->device_set_attr(mshv_dev, &attr);
+		break;
+	case MSHV_HAS_DEVICE_ATTR:
+		if (devops->device_has_attr)
+			return devops->device_has_attr(mshv_dev, &attr);
+		break;
+	}
+
+	return -EPERM;
+}
+
+static long mshv_device_fop_ioctl(struct file *filp, unsigned int cmd,
+				  ulong uarg)
+{
+	struct mshv_device *mshv_dev = filp->private_data;
+
+	switch (cmd) {
+	case MSHV_SET_DEVICE_ATTR:
+	case MSHV_HAS_DEVICE_ATTR:
+		return mshv_device_attr_ioctl(mshv_dev, cmd, uarg);
+	}
+
+	return -ENOTTY;
+}
+
+static int mshv_device_fop_release(struct inode *inode, struct file *filp)
+{
+	struct mshv_device *mshv_dev = filp->private_data;
+	struct mshv_partition *partition = mshv_dev->device_pt;
+
+	if (mshv_dev->device_ops->device_release) {
+		mutex_lock(&partition->pt_mutex);
+		hlist_del(&mshv_dev->device_ptnode);
+		mshv_dev->device_ops->device_release(mshv_dev);
+		mutex_unlock(&partition->pt_mutex);
+	}
+
+	mshv_partition_put(partition);
+	return 0;
+}
+
+static const struct file_operations mshv_device_fops = {
+	.owner = THIS_MODULE,
+	.unlocked_ioctl = mshv_device_fop_ioctl,
+	.release = mshv_device_fop_release,
+};
+
+static long mshv_partition_ioctl_create_device(struct mshv_partition *partition,
+					       void __user *uarg)
+{
+	long rc;
+	struct mshv_create_device devargk;
+	struct mshv_device *mshv_dev;
+	const struct mshv_device_ops *vfio_ops;
+
+	if (copy_from_user(&devargk, uarg, sizeof(devargk)))
+		return -EFAULT;
+
+	/* At present, only VFIO is supported */
+	if (devargk.type != MSHV_DEV_TYPE_VFIO)
+		return -ENODEV;
+
+	if (devargk.flags & MSHV_CREATE_DEVICE_TEST)
+		return 0;
+
+	if (devargk.flags)	/* currently, only MSHV_CREATE_DEVICE_TEST */
+		return -EINVAL;
+
+	/* This is freed later by mshv_vfio_release_device() */
+	mshv_dev = kzalloc_obj(*mshv_dev);
+	if (mshv_dev == NULL)
+		return -ENOMEM;
+
+	vfio_ops = &mshv_vfio_device_ops;
+	mshv_dev->device_ops = vfio_ops;
+	mshv_dev->device_pt = partition;
+
+	rc = vfio_ops->device_create(mshv_dev);
+	if (rc < 0) {
+		kfree(mshv_dev);
+		return rc;
+	}
+
+	hlist_add_head(&mshv_dev->device_ptnode, &partition->pt_devices);
+
+	mshv_partition_get(partition);
+	rc = anon_inode_getfd(vfio_ops->device_name, &mshv_device_fops,
+			      mshv_dev, O_RDWR | O_CLOEXEC);
+	if (rc < 0)
+		goto undo_out;
+
+	devargk.fd = rc;
+	if (copy_to_user(uarg, &devargk, sizeof(devargk)))
+		return -EFAULT;    /* cleanup in mshv_device_fop_release() */
+
+	return 0;
+
+undo_out:
+	hlist_del(&mshv_dev->device_ptnode);
+	vfio_ops->device_release(mshv_dev);    /* will kfree(mshv_dev) */
+	mshv_partition_put(partition);
+	return rc;
+}
+
 static long
 mshv_partition_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
 {
@@ -1661,6 +1777,9 @@ mshv_partition_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
 	case MSHV_ROOT_HVCALL:
 		ret = mshv_ioctl_passthru_hvcall(partition, true, uarg);
 		break;
+	case MSHV_CREATE_DEVICE:
+		ret = mshv_partition_ioctl_create_device(partition, uarg);
+		break;
 	default:
 		ret = -ENOTTY;
 	}
-- 
2.51.2.vfs.0.1

Re: [PATCH v5 4/9] mshv: Add ioctl support for MSHV-VFIO bridge device

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Failing copy_to_user after anon_inode_getfd causes a file descriptor leak and leaves partition state inconsistent.
- [Medium] The MSHV_CREATE_DEVICE_TEST flag ignores all other flags, rendering capability checks broken.
--

--- Patch [4]: [PATCH v5 4/9] mshv: Add ioctl support for MSHV-VFIO bridge device ---
commit a96aa99c275e6a51436ce19dce86ac7d1a20b10d
Author: Mukesh R <mrathor@linux.microsoft.com>

mshv: Add ioctl support for MSHV-VFIO bridge device

This commit adds ioctl support for creating MSHV VFIO bridge devices in a
partition. It introduces a partition ioctl to verify the VFIO type and sets
up the bridge code.

> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> index 39e9ef3674f29..fc08e994ecdf5 100644
> --- a/drivers/hv/mshv_root_main.c
> +++ b/drivers/hv/mshv_root_main.c
[ ... ]
> @@ -1625,6 +1625,122 @@ mshv_partition_ioctl_initialize(struct mshv_partition *partition)
[ ... ]
> +static long mshv_partition_ioctl_create_device(struct mshv_partition *partition,
> +					       void __user *uarg)
> +{
> +	long rc;
> +	struct mshv_create_device devargk;
> +	struct mshv_device *mshv_dev;
> +	const struct mshv_device_ops *vfio_ops;
> +
> +	if (copy_from_user(&devargk, uarg, sizeof(devargk)))
> +		return -EFAULT;
> +
> +	/* At present, only VFIO is supported */
> +	if (devargk.type != MSHV_DEV_TYPE_VFIO)
> +		return -ENODEV;
> +
> +	if (devargk.flags & MSHV_CREATE_DEVICE_TEST)
> +		return 0;
> +
> +	if (devargk.flags)	/* currently, only MSHV_CREATE_DEVICE_TEST */
> +		return -EINVAL;

[Severity: Medium]
If userspace calls this with MSHV_CREATE_DEVICE_TEST combined with an
unknown flag, won't this unconditionally return 0 and bypass the check for
unsupported flags?

Could this mislead userspace into thinking a new flag is supported when
probing capabilities on older kernels?

[ ... ]
> +	mshv_partition_get(partition);
> +	rc = anon_inode_getfd(vfio_ops->device_name, &mshv_device_fops,
> +			      mshv_dev, O_RDWR | O_CLOEXEC);
> +	if (rc < 0)
> +		goto undo_out;
> +
> +	devargk.fd = rc;
> +	if (copy_to_user(uarg, &devargk, sizeof(devargk)))
> +		return -EFAULT;    /* cleanup in mshv_device_fop_release() */

[Severity: High]
Does this leak the file descriptor and the associated mshv_dev kernel
structures if copy_to_user() fails?

Since anon_inode_getfd() immediately installs the new file descriptor into
the task's fdtable, returning -EFAULT here leaves the fd permanently open
until the process exits.

Also, because the fd is installed before copy_to_user() is called, could
another thread guess the fd and start using it while the calling thread
receives -EFAULT and assumes device creation failed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731223427.2554388-1-mrathor@linux.microsoft.com?part=4

[PATCH v5 5/9] mshv: Import data structs around device passthru from hyperv headers

[Mukesh R]

Copy/import from Hyper-V public headers, definitions and declarations that
are related to creating domains in the hypervisor, attaching devices to
them, etc.

Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
---
 include/hyperv/hvgdk_mini.h |  8 +++++
 include/hyperv/hvhdk_mini.h | 67 +++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+)

diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
index 6a4e8b9d570f..e62f8d708e8e 100644
--- a/include/hyperv/hvgdk_mini.h
+++ b/include/hyperv/hvgdk_mini.h
@@ -326,6 +326,8 @@ union hv_hypervisor_version_info {
 /* stimer Direct Mode is available */
 #define HV_STIMER_DIRECT_MODE_AVAILABLE			BIT(19)
 
+#define HV_DEVICE_DOMAIN_AVAILABLE			BIT(24)
+
 /*
  * Implementation recommendations. Indicates which behaviors the hypervisor
  * recommends the OS implement for optimal performance.
@@ -486,9 +488,15 @@ union hv_vp_assist_msr_contents {	 /* HV_REGISTER_VP_ASSIST_PAGE */
 #define HVCALL_GET_VP_INDEX_FROM_APIC_ID		0x009a
 #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_SPACE	0x00af
 #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_LIST	0x00b0
+#define HVCALL_CREATE_DEVICE_DOMAIN                     0x00b1
+#define HVCALL_ATTACH_DEVICE_DOMAIN                     0x00b2
+#define HVCALL_MAP_DEVICE_GPA_PAGES                     0x00b3
+#define HVCALL_UNMAP_DEVICE_GPA_PAGES                   0x00b4
 #define HVCALL_SIGNAL_EVENT_DIRECT			0x00c0
 #define HVCALL_POST_MESSAGE_DIRECT			0x00c1
 #define HVCALL_DISPATCH_VP				0x00c2
+#define HVCALL_DETACH_DEVICE_DOMAIN                     0x00c4
+#define HVCALL_DELETE_DEVICE_DOMAIN                     0x00c5
 #define HVCALL_GET_GPA_PAGES_ACCESS_STATES		0x00c9
 #define HVCALL_ACQUIRE_SPARSE_SPA_PAGE_HOST_ACCESS	0x00d7
 #define HVCALL_RELEASE_SPARSE_SPA_PAGE_HOST_ACCESS	0x00d8
diff --git a/include/hyperv/hvhdk_mini.h b/include/hyperv/hvhdk_mini.h
index b4cb2fa26e9b..90497635c3cb 100644
--- a/include/hyperv/hvhdk_mini.h
+++ b/include/hyperv/hvhdk_mini.h
@@ -547,4 +547,71 @@ union hv_device_id {		/* HV_DEVICE_ID */
 	} acpi;
 } __packed;
 
+/* 3 domain types: stage 1, stage 2, and SOC */
+#define HV_DEVICE_DOMAIN_TYPE_S2  0 /* HV_DEVICE_DOMAIN_ID_TYPE_S2 */
+#define HV_DEVICE_DOMAIN_TYPE_S1  1 /* HV_DEVICE_DOMAIN_ID_TYPE_S1 */
+#define HV_DEVICE_DOMAIN_TYPE_SOC 2 /* HV_DEVICE_DOMAIN_ID_TYPE_SOC */
+
+/* ID for stage 2 default domain and NULL domain */
+#define HV_DEVICE_DOMAIN_ID_S2_DEFAULT 0
+#define HV_DEVICE_DOMAIN_ID_S2_NULL    0xFFFFFFFFULL
+
+union hv_device_domain_id {
+	u64 as_uint64;
+	struct {
+		u32 type : 4;
+		u32 reserved : 28;
+		u32 id;
+	};
+} __packed;
+
+struct hv_input_device_domain { /* HV_INPUT_DEVICE_DOMAIN */
+	u64 partition_id;
+	union hv_input_vtl owner_vtl;
+	u8 padding[7];
+	union hv_device_domain_id domain_id;
+} __packed;
+
+union hv_create_device_domain_flags {	/* HV_CREATE_DEVICE_DOMAIN_FLAGS */
+	u32 as_uint32;
+	struct {
+		u32 forward_progress_required : 1;
+		u32 inherit_owning_vtl : 1;
+		u32 reserved : 30;
+	} __packed;
+} __packed;
+
+struct hv_input_create_device_domain {	/* HV_INPUT_CREATE_DEVICE_DOMAIN */
+	struct hv_input_device_domain device_domain;
+	union hv_create_device_domain_flags create_device_domain_flags;
+} __packed;
+
+struct hv_input_delete_device_domain {	/* HV_INPUT_DELETE_DEVICE_DOMAIN */
+	struct hv_input_device_domain device_domain;
+} __packed;
+
+struct hv_input_attach_device_domain {	/* HV_INPUT_ATTACH_DEVICE_DOMAIN */
+	struct hv_input_device_domain device_domain;
+	union hv_device_id device_id;
+} __packed;
+
+struct hv_input_detach_device_domain {	/* HV_INPUT_DETACH_DEVICE_DOMAIN */
+	u64 partition_id;
+	union hv_device_id device_id;
+} __packed;
+
+struct hv_input_map_device_gpa_pages {	/* HV_INPUT_MAP_DEVICE_GPA_PAGES */
+	struct hv_input_device_domain device_domain;
+	union hv_input_vtl target_vtl;
+	u8 padding[3];
+	u32 map_flags;
+	u64 target_device_va_base;
+	u64 gpa_page_list[];
+} __packed;
+
+struct hv_input_unmap_device_gpa_pages {  /* HV_INPUT_UNMAP_DEVICE_GPA_PAGES */
+	struct hv_input_device_domain device_domain;
+	u64 target_device_va_base;
+} __packed;
+
 #endif /* _HV_HVHDK_MINI_H */
-- 
2.51.2.vfs.0.1

Re: [PATCH v5 5/9] mshv: Import data structs around device passthru from hyperv headers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The fields in `union hv_device_domain_id` are ordered incorrectly, resulting in a flawed bitfield layout that contradicts the domain ID constants.
- [High] `struct hv_input_create_device_domain` is exactly 28 bytes in size, violating the MS C implicit padding expectations for 8-byte aligned structures.
--

--- Patch [5]: [PATCH v5 5/9] mshv: Import data structs around device passthru from hyperv headers ---
commit 9932d93213b193c633772c7ddbee866c39282647
Author: Mukesh R <mrathor@linux.microsoft.com>

mshv: Import data structs around device passthru from hyperv headers

Copy/import from Hyper-V public headers, definitions and declarations that
are related to creating domains in the hypervisor, attaching devices to
them, etc.

> diff --git a/include/hyperv/hvhdk_mini.h b/include/hyperv/hvhdk_mini.h
> index b4cb2fa26e9bc..90497635c3cbd 100644
> --- a/include/hyperv/hvhdk_mini.h
> +++ b/include/hyperv/hvhdk_mini.h
[ ... ]
> +/* ID for stage 2 default domain and NULL domain */
> +#define HV_DEVICE_DOMAIN_ID_S2_DEFAULT 0
> +#define HV_DEVICE_DOMAIN_ID_S2_NULL    0xFFFFFFFFULL
> +
> +union hv_device_domain_id {
> +	u64 as_uint64;
> +	struct {
> +		u32 type : 4;
> +		u32 reserved : 28;
> +		u32 id;
> +	};
> +} __packed;

[Severity: High]
Are the fields in this struct ordered correctly for the ABI?

In little-endian C bitfields, the first member occupies the lowest bits. By
placing type and reserved before id, they will occupy the lower 32 bits
while id occupies the upper 32 bits.

This seems to contradict the constant HV_DEVICE_DOMAIN_ID_S2_NULL defined just
above this union, which expects the id (0xFFFFFFFF) in the lower 32 bits and
type (0) in the upper 32 bits.

Will this cause the hypervisor to receive swapped values and fail domain
attachment?

[ ... ]
> +struct hv_input_create_device_domain {	/* HV_INPUT_CREATE_DEVICE_DOMAIN */
> +	struct hv_input_device_domain device_domain;
> +	union hv_create_device_domain_flags create_device_domain_flags;
> +} __packed;

[Severity: High]
Does this struct need explicit padding at the end to match the hypervisor's
32-byte size expectation?

Because of the __packed attribute, the compiler won't add the 4-byte implicit
trailing padding required to bring the struct to its natural 32-byte size
(due to the 8-byte alignment of device_domain).

If standard hypercall initialization clears the input using:
memset(in, 0, sizeof(*in));

the sizeof will be 28, leaving the last 4 bytes of the 32-byte block
uninitialized on the shared hypercall page. 

If the hypervisor validates these padding bytes to be zero, could this fail
with HV_STATUS_INVALID_PARAMETER if there is leftover garbage from a
previous hypercall?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731223427.2554388-1-mrathor@linux.microsoft.com?part=5

[PATCH v5 6/9] PCI: hv: Export hv_build_devid_type_pci() and change return type

[Mukesh R]

On Hyper-V, most hypercalls related to PCI passthru to map/unmap regions,
interrupts, etc need a device ID as a parameter. This device ID refers
to that specific device during the lifetime of passthru. So,
make hv_build_devid_type_pci() public and change return type to u64 to
enforce it's size.

Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
Reviewed-by: Souradeep Chakrabarti <schakrabarti@linux.microsoft.com>
---
 arch/x86/hyperv/irqdomain.c     | 9 +++++----
 arch/x86/include/asm/mshyperv.h | 6 ++++++
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/arch/x86/hyperv/irqdomain.c b/arch/x86/hyperv/irqdomain.c
index b3ad50a874dc..8780573a4332 100644
--- a/arch/x86/hyperv/irqdomain.c
+++ b/arch/x86/hyperv/irqdomain.c
@@ -112,7 +112,7 @@ static int get_rid_cb(struct pci_dev *pdev, u16 alias, void *data)
 	return 0;
 }
 
-static union hv_device_id hv_build_devid_type_pci(struct pci_dev *pdev)
+u64 hv_build_devid_type_pci(struct pci_dev *pdev)
 {
 	int pos;
 	union hv_device_id hv_devid;
@@ -172,8 +172,9 @@ static union hv_device_id hv_build_devid_type_pci(struct pci_dev *pdev)
 	}
 
 out:
-	return hv_devid;
+	return hv_devid.as_uint64;
 }
+EXPORT_SYMBOL_GPL(hv_build_devid_type_pci);
 
 /*
  * hv_map_msi_interrupt() - Map the MSI IRQ in the hypervisor.
@@ -196,7 +197,7 @@ int hv_map_msi_interrupt(struct irq_data *data,
 
 	msidesc = irq_data_get_msi_desc(data);
 	pdev = msi_desc_to_pci_dev(msidesc);
-	hv_devid = hv_build_devid_type_pci(pdev);
+	hv_devid.as_uint64 = hv_build_devid_type_pci(pdev);
 	cpu = cpumask_first(irq_data_get_effective_affinity_mask(data));
 
 	return hv_map_interrupt(hv_devid, false, cpu, cfg->vector,
@@ -271,7 +272,7 @@ static int hv_unmap_msi_interrupt(struct pci_dev *pdev,
 {
 	union hv_device_id hv_devid;
 
-	hv_devid = hv_build_devid_type_pci(pdev);
+	hv_devid.as_uint64 = hv_build_devid_type_pci(pdev);
 	return hv_unmap_interrupt(hv_devid.as_uint64, irq_entry);
 }
 
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index f64393e853ee..8ebbd1cb7c8c 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -248,6 +248,12 @@ void hv_crash_asm_end(void);
 static inline void hv_root_crash_init(void) {}
 #endif  /* CONFIG_MSHV_ROOT && CONFIG_CRASH_DUMP */
 
+#ifdef CONFIG_PCI_MSI
+u64 hv_build_devid_type_pci(struct pci_dev *pdev);
+#else
+static inline u64 hv_build_devid_type_pci(struct pci_dev *pdev) { return 0; }
+#endif
+
 #else /* CONFIG_HYPERV */
 static inline void hyperv_init(void) {}
 static inline void hyperv_setup_mmu_ops(void) {}
-- 
2.51.2.vfs.0.1

Re: [PATCH v5 6/9] PCI: hv: Export hv_build_devid_type_pci() and change return type

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Incomplete API export for cross-architecture code

Pre-existing issues:
- [High] Resource leak of multi-vector MSI IRQs during teardown
--

--- Patch [6]: [PATCH v5 6/9] PCI: hv: Export hv_build_devid_type_pci() and change return type ---
commit 2b1777d94c87d493032cb98e1c21a4774a30a00a
Author: Mukesh R <mrathor@linux.microsoft.com>

PCI: hv: Export hv_build_devid_type_pci() and change return type

On Hyper-V, most hypercalls related to PCI passthru to map/unmap regions,
interrupts, etc need a device ID as a parameter. This device ID refers
to that specific device during the lifetime of passthru. So,
make hv_build_devid_type_pci() public and change return type to u64 to
enforce it's size.

> diff --git a/arch/x86/hyperv/irqdomain.c b/arch/x86/hyperv/irqdomain.c
> index b3ad50a874dca..8780573a43326 100644
> --- a/arch/x86/hyperv/irqdomain.c
> +++ b/arch/x86/hyperv/irqdomain.c
[ ... ]
> @@ -172,8 +172,9 @@ static union hv_device_id hv_build_devid_type_pci(struct pci_dev *pdev)
>  	}
>  
>  out:
> -	return hv_devid;
> +	return hv_devid.as_uint64;
>  }
> +EXPORT_SYMBOL_GPL(hv_build_devid_type_pci);

[Severity: Medium]
Will exporting and implementing this API solely in an x86-specific file cause
build failures for ARM64?

The commit message mentions making this public for PCI passthrough hypercalls,
but the mshv driver supports both x86 and ARM64. It appears cross-architecture
builds will lack this implementation when they try to utilize the newly
exported API.

[ ... ]
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index f64393e853ee3..8ebbd1cb7c8ce 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -248,6 +248,12 @@ void hv_crash_asm_end(void);
>  static inline void hv_root_crash_init(void) {}
>  #endif  /* CONFIG_MSHV_ROOT && CONFIG_CRASH_DUMP */
>  
> +#ifdef CONFIG_PCI_MSI
> +u64 hv_build_devid_type_pci(struct pci_dev *pdev);
> +#else
> +static inline u64 hv_build_devid_type_pci(struct pci_dev *pdev) { return 0; }
> +#endif
> +

[Severity: Medium]
Similarly, should this declaration be placed in a shared or cross-architecture
header so that ARM64 drivers can safely include it without breaking?

While reviewing this file, I also noticed an issue in hv_msi_domain_free().

[Severity: High]
This is a pre-existing issue, but does this loop fail to tear down the
remaining vectors for multi-vector MSI IRQs?

arch/x86/hyperv/irqdomain.c:hv_msi_domain_free() {
    ...
	for (int i = 0; i < nr_irqs; ++i) {
		struct irq_data *irqd = irq_domain_get_irq_data(d, virq);
    ...
}

Because it looks up the base virq on every iteration instead of virq + i,
does this continuously leak hypervisor-level interrupt resources and chip_data
memory allocations during a device unbind?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731223427.2554388-1-mrathor@linux.microsoft.com?part=6

[PATCH v5 7/9] x86/hyperv: Implement Hyper-V virtual IOMMU

[Mukesh R]

Add a new file to implement a virtual IOMMU that works with Microsoft
Hyper-V hypervisor. The hypervisor claims the IOMMU upon boot, and this
driver communicates with it for creating and deleting paging domains,
mapping and unmapping of pages, etc. The default domain type is IDENTITY.
Lastly, this is only supported on baremetal or nested root/dom0, and not
L1VH root/dom0.

Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
---
 arch/x86/kernel/pci-dma.c            |   2 +
 drivers/iommu/Kconfig                |   1 +
 drivers/iommu/hyperv/Kconfig         |  15 +
 drivers/iommu/hyperv/Makefile        |   1 +
 drivers/iommu/hyperv/hv-iommu-root.c | 706 +++++++++++++++++++++++++++
 include/asm-generic/mshyperv.h       |   3 +
 include/linux/hyperv.h               |   6 +
 7 files changed, 734 insertions(+)
 create mode 100644 drivers/iommu/hyperv/Kconfig
 create mode 100644 drivers/iommu/hyperv/hv-iommu-root.c

diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index 6267363e0189..37fed8c7a8c2 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -8,6 +8,7 @@
 #include <linux/gfp.h>
 #include <linux/pci.h>
 #include <linux/amd-iommu.h>
+#include <linux/hyperv.h>
 
 #include <asm/proto.h>
 #include <asm/dma.h>
@@ -103,6 +104,7 @@ void __init pci_iommu_alloc(void)
 	}
 	pci_swiotlb_detect();
 	gart_iommu_hole_init();
+	hv_iommu_detect();
 	amd_iommu_detect();
 	detect_intel_iommu();
 	swiotlb_init(x86_swiotlb_enable, x86_swiotlb_flags);
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 1becc0f20222..e95a56a6e075 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -197,6 +197,7 @@ source "drivers/iommu/arm/Kconfig"
 source "drivers/iommu/intel/Kconfig"
 source "drivers/iommu/iommufd/Kconfig"
 source "drivers/iommu/riscv/Kconfig"
+source "drivers/iommu/hyperv/Kconfig"
 
 config IRQ_REMAP
 	bool "Support for Interrupt Remapping"
diff --git a/drivers/iommu/hyperv/Kconfig b/drivers/iommu/hyperv/Kconfig
new file mode 100644
index 000000000000..512b154b2f59
--- /dev/null
+++ b/drivers/iommu/hyperv/Kconfig
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0-only
+# Hyper-V IOMMU support
+
+config HYPERV_IOMMU
+	bool "Hyper-V IOMMU Device"
+	depends on HYPERV && X86
+	select IOMMU_API
+	default HYPERV
+	help
+	  This enables Hyper-V pseudo IOMMU device. When running as privileged
+          VM aka root on Microsoft Hyper-V hypervisor, this must be enabled
+          for doing any PCI passthru of devices to guest VMs. This applies to
+          both PFs and VFs. When enabling this, it is best to disable amd/intel
+          iommus via: intel_iommu=off amd_iommu=off as the hypervisor really
+          owns the iommu.
diff --git a/drivers/iommu/hyperv/Makefile b/drivers/iommu/hyperv/Makefile
index 6ef0ef97f3dd..aaced370a94f 100644
--- a/drivers/iommu/hyperv/Makefile
+++ b/drivers/iommu/hyperv/Makefile
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_IRQ_REMAP) += hv-irq-remap-x86.o
+obj-$(CONFIG_HYPERV_IOMMU) += hv-iommu-root.o
diff --git a/drivers/iommu/hyperv/hv-iommu-root.c b/drivers/iommu/hyperv/hv-iommu-root.c
new file mode 100644
index 000000000000..97ddd6956031
--- /dev/null
+++ b/drivers/iommu/hyperv/hv-iommu-root.c
@@ -0,0 +1,706 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Hyper-V root vIOMMU driver.
+ * Copyright (C) 2026, Microsoft, Inc.
+ */
+#include <linux/pci.h>
+#include <linux/dma-map-ops.h>
+#include <linux/interval_tree.h>
+#include <linux/hyperv.h>
+#include <asm/iommu.h>
+#include <asm/mshyperv.h>
+#include "../dma-iommu.h"
+
+/* We will not claim these PCI devices, eg hypervisor needs it for debugger */
+static char *pci_devs_to_skip;
+static int __init hv_iommu_setup_skip(char *str)
+{
+	pci_devs_to_skip = str;
+
+	return 0;
+}
+/* Eg: hv_iommu_skip=(SSSS:BB:DD.F)(SSSS:BB:DD.F) */
+__setup("hv_iommu_skip=", hv_iommu_setup_skip);
+
+/* IOMMU device that we export to the world. HyperV supports max of one */
+static struct iommu_device hv_virt_iommu;
+
+struct hv_domain {
+	struct iommu_domain iommu_dom;
+	u32 domid_num;			      /* as opposed to domain_id.type */
+	u64 partid;			      /* partition id */
+	spinlock_t mappings_lock;	      /* protects mappings_tree */
+	struct rb_root_cached mappings_tree;  /* iova to pa lookup tree */
+};
+
+#define to_hv_domain(d) container_of(d, struct hv_domain, iommu_dom)
+
+struct hv_iommu_mapping {
+	phys_addr_t paddr;
+	struct interval_tree_node iova;
+	u32 flags;
+};
+
+/*
+ * By default, during boot the hypervisor creates one Stage 2 (S2) default
+ * domain. Stage 2 means that the page table is controlled by the hypervisor.
+ * It has two types:
+ *   S2 default: access to entire root partition memory. This for us easily
+ *		 maps to IOMMU_DOMAIN_IDENTITY in the iommu subsystem, and
+ *		 is called HV_DEVICE_DOMAIN_ID_S2_DEFAULT in the hypervisor.
+ *   S2 NULL: Blocks everything except RMRR
+ */
+
+/*
+ * Create dummy domains to correspond to hypervisor prebuilt default identity
+ * and null domains (dummy because we do not make hypercalls to create them).
+ */
+static struct hv_domain hv_def_identity_dom;
+static struct hv_domain hv_null_dom;
+
+static bool hv_special_domain(struct hv_domain *hvdom)
+{
+	return hvdom == &hv_def_identity_dom || hvdom == &hv_null_dom;
+}
+
+struct iommu_domain_geometry default_geometry = (struct iommu_domain_geometry) {
+	.aperture_start = 0,
+	.aperture_end = -1UL,
+	.force_aperture = true,
+};
+
+#define HV_IOMMU_PGSIZES SZ_4K	/* for now, to be enhanced */
+
+static atomic_t hv_unique_id;		/* unique numeric id for a new domain */
+
+/*
+ * If the current thread is a VMM thread, return the partition id of the VM it
+ * is managing, else return HV_PARTITION_ID_INVALID.
+ */
+static u64 hv_get_current_partid(void)
+{
+	u64 (*fn)(void);
+	u64 ptid;
+
+	fn = symbol_get(mshv_current_partid);
+	if (!fn)
+		return HV_PARTITION_ID_INVALID;
+
+	ptid = fn();
+	symbol_put(mshv_current_partid);
+
+	return ptid;
+}
+
+static bool hv_iommu_capable(struct device *dev, enum iommu_cap cap)
+{
+	switch (cap) {
+	case IOMMU_CAP_CACHE_COHERENCY:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static int hv_iommu_add_tree_mapping(struct hv_domain *hvdom,
+				     unsigned long iova, phys_addr_t paddr,
+				     size_t size, u32 flags)
+{
+	unsigned long irqflags;
+	struct hv_iommu_mapping *mapping;
+
+	mapping = kzalloc_obj(struct hv_iommu_mapping, GFP_ATOMIC);
+	if (!mapping)
+		return -ENOMEM;
+
+	mapping->paddr = paddr;
+	mapping->iova.start = iova;
+	mapping->iova.last = iova + size - 1;
+	mapping->flags = flags;
+
+	spin_lock_irqsave(&hvdom->mappings_lock, irqflags);
+	interval_tree_insert(&mapping->iova, &hvdom->mappings_tree);
+	spin_unlock_irqrestore(&hvdom->mappings_lock, irqflags);
+
+	return 0;
+}
+
+/* If size == 0, then last = ULONG_MAX to remove everything with iova 0 */
+static size_t hv_iommu_del_tree_mappings(struct hv_domain *hvdom,
+					 unsigned long iova, size_t size)
+{
+	unsigned long flags;
+	size_t unmapped = 0;
+	unsigned long last = iova + size - 1;
+	struct hv_iommu_mapping *mapping = NULL;
+	struct interval_tree_node *node, *next;
+
+	spin_lock_irqsave(&hvdom->mappings_lock, flags);
+	next = interval_tree_iter_first(&hvdom->mappings_tree, iova, last);
+	while (next) {
+		node = next;
+		mapping = container_of(node, struct hv_iommu_mapping, iova);
+		next = interval_tree_iter_next(node, iova, last);
+
+		/* Splitting of a mapping is not supported at present */
+		if (mapping->iova.start < iova)
+			break;
+
+		unmapped += mapping->iova.last - mapping->iova.start + 1;
+
+		interval_tree_remove(node, &hvdom->mappings_tree);
+		kfree(mapping);
+	}
+	spin_unlock_irqrestore(&hvdom->mappings_lock, flags);
+
+	return unmapped;
+}
+
+/* Create a new device domain in the hypervisor */
+static int hv_iommu_create_hyp_devdom(struct hv_domain *hvdom)
+{
+	u64 status;
+	struct hv_input_device_domain *ddp;
+	struct hv_input_create_device_domain *input;
+	unsigned long flags;
+
+	local_irq_save(flags);
+	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
+	memset(input, 0, sizeof(*input));
+
+	ddp = &input->device_domain;
+	ddp->partition_id = HV_PARTITION_ID_SELF;
+	ddp->domain_id.type = HV_DEVICE_DOMAIN_TYPE_S2;
+	ddp->domain_id.id = hvdom->domid_num;
+
+	input->create_device_domain_flags.forward_progress_required = 1;
+	input->create_device_domain_flags.inherit_owning_vtl = 0;
+
+	status = hv_do_hypercall(HVCALL_CREATE_DEVICE_DOMAIN, input, NULL);
+
+	local_irq_restore(flags);
+
+	if (!hv_result_success(status))
+		hv_status_err(status, "\n");
+
+	return hv_result_to_errno(status);
+}
+
+static struct iommu_domain *hv_iommu_domain_alloc_paging(struct device *dev)
+{
+	struct hv_domain *hvdom;
+	int rc;
+	u32 unique_id;
+	u64 ptid = hv_get_current_partid();
+
+	if (ptid == HV_PARTITION_ID_INVALID)
+		return NULL;
+
+	hvdom = kzalloc_obj(struct hv_domain);
+	if (hvdom == NULL)
+		return NULL;
+
+	spin_lock_init(&hvdom->mappings_lock);
+	hvdom->mappings_tree = RB_ROOT_CACHED;
+
+	unique_id = (u32)atomic_inc_return(&hv_unique_id);
+	if (unique_id == HV_DEVICE_DOMAIN_ID_S2_NULL)	/* ie, UINTMAX */
+		goto out_err;
+
+	hvdom->domid_num = unique_id;
+	hvdom->partid = ptid;
+	hvdom->iommu_dom.geometry = default_geometry;
+	hvdom->iommu_dom.pgsize_bitmap = HV_IOMMU_PGSIZES;
+
+	rc = hv_iommu_create_hyp_devdom(hvdom);
+	if (rc)
+		goto out_err;
+
+	return &hvdom->iommu_dom;
+
+out_err:
+	kfree(hvdom);
+	return NULL;
+}
+
+static void hv_iommu_domain_free(struct iommu_domain *immdom)
+{
+	unsigned long flags;
+	u64 status;
+	struct hv_input_delete_device_domain *input;
+	struct hv_input_device_domain *ddp;
+	struct hv_domain *hvdom = to_hv_domain(immdom);
+
+	if (hv_special_domain(hvdom))
+		return;
+
+	/* Cleanup any remaining. 0 for size results in ULONG_MAX as the last */
+	hv_iommu_del_tree_mappings(hvdom, 0, 0);
+
+	local_irq_save(flags);
+	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
+	ddp = &input->device_domain;
+	memset(input, 0, sizeof(*input));
+
+	ddp->partition_id = HV_PARTITION_ID_SELF;
+	ddp->domain_id.type = HV_DEVICE_DOMAIN_TYPE_S2;
+	ddp->domain_id.id = hvdom->domid_num;
+
+	status = hv_do_hypercall(HVCALL_DELETE_DEVICE_DOMAIN, input,
+				 NULL);
+	local_irq_restore(flags);
+
+	if (!hv_result_success(status))
+		hv_status_err(status, "\n");
+
+	kfree(hvdom);
+}
+
+static void hv_iommu_det_dev_from_dom(struct pci_dev *pdev)
+{
+	u64 status;
+	unsigned long flags;
+	struct hv_input_detach_device_domain *input;
+
+	local_irq_save(flags);
+	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
+	memset(input, 0, sizeof(*input));
+
+	input->partition_id = HV_PARTITION_ID_SELF;
+	input->device_id.as_uint64 = hv_build_devid_type_pci(pdev);
+	status = hv_do_hypercall(HVCALL_DETACH_DEVICE_DOMAIN, input, NULL);
+	local_irq_restore(flags);
+
+	if (!hv_result_success(status))
+		hv_status_err(status, "\n");
+}
+
+static void hv_iommu_detach_dev(struct hv_domain *hvdom, struct device *dev)
+{
+	struct pci_dev *pdev;
+
+	/* See the attach function, only PCI devices for now */
+	if (!dev_is_pci(dev))
+		return;
+
+	pdev = to_pci_dev(dev);
+	hv_iommu_det_dev_from_dom(pdev);
+}
+
+/*
+ * Attach a device to the default domain, or the null domain, or to a domain
+ * previously created in the hypervisor.
+ */
+static int hv_iommu_att_dev2dom(struct hv_domain *hvdom, struct pci_dev *pdev)
+{
+	unsigned long flags;
+	u64 status;
+	struct hv_input_attach_device_domain *input;
+
+	local_irq_save(flags);
+	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
+	memset(input, 0, sizeof(*input));
+
+	/* For null domain, hvdom->domid_num == HV_DEVICE_DOMAIN_ID_S2_NULL */
+	input->device_domain.partition_id = HV_PARTITION_ID_SELF;
+	input->device_domain.domain_id.type = HV_DEVICE_DOMAIN_TYPE_S2;
+	input->device_domain.domain_id.id = hvdom->domid_num;
+
+	/* NB: Upon guest shutdown, device is re-attached to the default domain
+	 *     without explicit detach.
+	 */
+	input->device_id.as_uint64 = hv_build_devid_type_pci(pdev);
+
+	status = hv_do_hypercall(HVCALL_ATTACH_DEVICE_DOMAIN, input, NULL);
+	local_irq_restore(flags);
+
+	if (!hv_result_success(status))
+		hv_status_err(status, "\n");
+
+	return hv_result_to_errno(status);
+}
+
+static int hv_iommu_attach_dev(struct iommu_domain *immdom, struct device *dev,
+			       struct iommu_domain *old)
+{
+	struct pci_dev *pdev;
+	int rc;
+	struct hv_domain *hvdom_new = to_hv_domain(immdom);
+	struct hv_domain *hvdom_prev = to_hv_domain(old);
+
+	/* Only allow PCI devices for now */
+	if (!dev_is_pci(dev))
+		return -EINVAL;
+
+	pdev = to_pci_dev(dev);
+
+	/* There are no explicit detach calls, hence check if we need to detach
+	 * first. Also, in case of guest shutdown, it's the VMM thread that
+	 * attaches it back to the hv_def_identity_dom, and hvdom_prev will not
+	 * be null then. It is null during boot.
+	 */
+	if (hvdom_prev && !hv_special_domain(hvdom_prev))
+		hv_iommu_detach_dev(hvdom_prev, dev);
+
+	rc = hv_iommu_att_dev2dom(hvdom_new, pdev);
+
+	if (rc == 0)
+		dev_iommu_priv_set(dev, hvdom_new);  /* sets "private" field */
+	else {
+		if (hvdom_prev && !hv_special_domain(hvdom_prev))
+			WARN(1, "Failed to attach dev:%s\n", pci_name(pdev));
+		dev_iommu_priv_set(dev, NULL);
+	}
+
+	return rc;
+}
+
+static u64 hv_iommu_unmap_batch(u32 domid_num, ulong iova, u16 count)
+{
+	ulong flags;
+	struct hv_input_unmap_device_gpa_pages *input;
+	u64 status;
+
+	local_irq_save(flags);
+	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
+	memset(input, 0, sizeof(*input));
+
+	input->device_domain.partition_id = HV_PARTITION_ID_SELF;
+	input->device_domain.domain_id.type = HV_DEVICE_DOMAIN_TYPE_S2;
+	input->device_domain.domain_id.id = domid_num;
+	input->target_device_va_base = iova;
+
+	status = hv_do_rep_hypercall(HVCALL_UNMAP_DEVICE_GPA_PAGES, count,
+				     0, input, NULL);
+	local_irq_restore(flags);
+
+	if (!hv_result_success(status))
+		hv_status_err(status, "iova:0x%lx count:0x%x\n", iova, count);
+
+	return status;
+}
+
+static size_t hv_iommu_unmap_pages(struct iommu_domain *immdom, ulong iova,
+				   size_t pgsize, size_t pgcount,
+				   struct iommu_iotlb_gather *gather)
+{
+	unsigned long npages;
+	u64 status;
+	struct hv_domain *hvdom = to_hv_domain(immdom);
+	size_t unmapped, tot_done = 0, size = pgsize * pgcount;
+
+	unmapped = hv_iommu_del_tree_mappings(hvdom, iova, size);
+	if (unmapped < size)
+		pr_err("%s: could not delete all mappings (%lx:%lx/%lx)\n",
+		       __func__, iova, unmapped, size);
+
+	npages = unmapped >> HV_HYP_PAGE_SHIFT;
+
+	while (npages) {
+		int done, count = min(npages, HV_REP_COUNT_MAX);
+
+		status = hv_iommu_unmap_batch(hvdom->domid_num, iova, count);
+
+		done = hv_repcomp(status);
+		tot_done += done;
+		npages -= done;
+		iova += done << HV_HYP_PAGE_SHIFT;
+
+		if (!hv_result_success(status))
+			break;
+	}
+
+	return tot_done << HV_HYP_PAGE_SHIFT;
+}
+
+/* Return: must return exact status from the hypercall without changes */
+static u64 hv_iommu_map_pgs(struct hv_domain *hvdom,
+			    unsigned long iova, phys_addr_t paddr,
+			    unsigned long npages, u32 map_flags)
+{
+	u64 status;
+	int i;
+	struct hv_input_map_device_gpa_pages *input;
+	unsigned long flags, pfn;
+
+	local_irq_save(flags);
+	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
+	memset(input, 0, sizeof(*input));
+
+	input->device_domain.partition_id = HV_PARTITION_ID_SELF;
+	input->device_domain.domain_id.type = HV_DEVICE_DOMAIN_TYPE_S2;
+	input->device_domain.domain_id.id = hvdom->domid_num;
+	input->map_flags = map_flags;
+	input->target_device_va_base = iova;
+
+	pfn = paddr >> HV_HYP_PAGE_SHIFT;
+	for (i = 0; i < npages; i++, pfn++)
+		input->gpa_page_list[i] = pfn;
+
+	status = hv_do_rep_hypercall(HVCALL_MAP_DEVICE_GPA_PAGES, npages, 0,
+				     input, NULL);
+	local_irq_restore(flags);
+
+	return status;
+}
+
+#define HV_MAP_DEVICE_GPA_BATCH_SIZE   \
+	((HV_HYP_PAGE_SIZE - sizeof(struct hv_input_map_device_gpa_pages)) \
+			/ sizeof(u64))
+
+/*
+ * The core VFIO code loops over memory ranges calling this function with the
+ * largest pgsize from HV_IOMMU_PGSIZES. cond_resched() is in vfio_iommu_map.
+ */
+static int hv_iommu_map_pages(struct iommu_domain *immdom, ulong iova,
+			      phys_addr_t paddr, size_t pgsize, size_t pgcount,
+			      int prot, gfp_t gfp, size_t *mapped)
+{
+	u32 map_flags;
+	int ret;
+	u64 status;
+	unsigned long npages, done = 0;
+	struct hv_domain *hvdom = to_hv_domain(immdom);
+	size_t size = pgsize * pgcount;
+
+	map_flags = HV_MAP_GPA_READABLE;	/* required */
+	map_flags |= prot & IOMMU_WRITE ? HV_MAP_GPA_WRITABLE : 0;
+
+	ret = hv_iommu_add_tree_mapping(hvdom, iova, paddr, size, map_flags);
+	if (ret)
+		return ret;
+
+	npages = size >> HV_HYP_PAGE_SHIFT;
+	while (done < npages) {
+		ulong completed, remain = npages - done;
+
+		remain = min(remain, HV_MAP_DEVICE_GPA_BATCH_SIZE);
+
+		status = hv_iommu_map_pgs(hvdom, iova, paddr, remain,
+					  map_flags);
+
+		completed = hv_repcomp(status);
+		done = done + completed;
+		iova = iova + (completed << HV_HYP_PAGE_SHIFT);
+		paddr = paddr + (completed << HV_HYP_PAGE_SHIFT);
+
+		if (hv_result(status) == HV_STATUS_INSUFFICIENT_MEMORY) {
+			ret = hv_call_deposit_pages(NUMA_NO_NODE,
+						    hv_current_partition_id,
+						    256);
+			if (ret)
+				break;
+			continue;
+		}
+		if (!hv_result_success(status))
+			break;
+	}
+
+	if (!hv_result_success(status)) {
+		size_t done_size = done << HV_HYP_PAGE_SHIFT;
+
+		hv_status_err(status, "pgs:%lx/%lx iova:%lx\n",
+			      done, npages, iova);
+		/*
+		 * lookup tree has all mappings [0 - size-1]. Below unmap will
+		 * only remove from [0 - done], we need to remove second chunk
+		 * [done+1 - size-1].
+		 */
+		hv_iommu_del_tree_mappings(hvdom, iova, size - done_size);
+		hv_iommu_unmap_pages(immdom, iova - done_size, HV_HYP_PAGE_SIZE,
+				     done, NULL);
+		if (mapped)
+			*mapped = 0;
+	} else
+		if (mapped)
+			*mapped = size;
+
+	return hv_result_to_errno(status);
+}
+
+static phys_addr_t hv_iommu_iova_to_phys(struct iommu_domain *immdom,
+					 dma_addr_t iova)
+{
+	unsigned long flags;
+	struct hv_iommu_mapping *mapping;
+	struct interval_tree_node *node;
+	u64 paddr = 0;
+	struct hv_domain *hvdom = to_hv_domain(immdom);
+
+	spin_lock_irqsave(&hvdom->mappings_lock, flags);
+	node = interval_tree_iter_first(&hvdom->mappings_tree, iova, iova);
+	if (node) {
+		mapping = container_of(node, struct hv_iommu_mapping, iova);
+		paddr = mapping->paddr + (iova - mapping->iova.start);
+	}
+	spin_unlock_irqrestore(&hvdom->mappings_lock, flags);
+
+	return paddr;
+}
+
+/*
+ * Currently, hypervisor does not provide list of devices it is using
+ * dynamically. So use this to allow users to manually specify devices that
+ * should be skipped. (eg. hypervisor debugger using some network device).
+ */
+static struct iommu_device *hv_iommu_probe_device(struct device *dev)
+{
+	if (!dev_is_pci(dev))
+		return ERR_PTR(-ENODEV);
+
+	if (pci_devs_to_skip && *pci_devs_to_skip) {
+		int rc, parsed, segment, bus, slot, func;
+		int pos = 0;
+		struct pci_dev *pdev = to_pci_dev(dev);
+
+		do {
+			parsed = 0;
+
+			rc = sscanf(pci_devs_to_skip + pos, " (%x:%x:%x.%x) %n",
+				    &segment, &bus, &slot, &func, &parsed);
+
+			if (rc != 4 || parsed <= 0)
+				break;
+
+			if (pci_domain_nr(pdev->bus) == segment &&
+			    pdev->bus->number == bus &&
+			    PCI_SLOT(pdev->devfn) == slot &&
+			    PCI_FUNC(pdev->devfn) == func) {
+
+				dev_info(dev, "skipped by Hyper-V IOMMU\n");
+				return ERR_PTR(-ENODEV);
+			}
+			pos += parsed;
+
+		} while (pci_devs_to_skip[pos]);
+	}
+
+	/* Device will be explicitly attached to the default domain, so no need
+	 * to do dev_iommu_priv_set() here.
+	 */
+
+	return &hv_virt_iommu;
+}
+
+static void hv_iommu_probe_finalize(struct device *dev)
+{
+	struct iommu_domain *immdom = iommu_get_domain_for_dev(dev);
+
+	if (immdom && immdom->type == IOMMU_DOMAIN_DMA)
+		iommu_setup_dma_ops(dev, immdom);
+	else
+		set_dma_ops(dev, NULL);
+}
+
+static void hv_iommu_release_device(struct device *dev)
+{
+	struct hv_domain *hvdom = dev_iommu_priv_get(dev);
+
+	/* Need to detach device from device domain if necessary. */
+	if (hvdom)
+		hv_iommu_detach_dev(hvdom, dev);
+
+	dev_iommu_priv_set(dev, NULL);
+	set_dma_ops(dev, NULL);
+}
+
+static struct iommu_group *hv_iommu_device_group(struct device *dev)
+{
+	if (dev_is_pci(dev))
+		return pci_device_group(dev);
+	else
+		return generic_device_group(dev);
+}
+
+static int hv_iommu_def_domain_type(struct device *dev)
+{
+	/* The hypervisor always creates this by default during boot */
+	return IOMMU_DOMAIN_IDENTITY;
+}
+
+static struct iommu_ops hv_iommu_ops = {
+	.capable	    = hv_iommu_capable,
+	.domain_alloc_paging	= hv_iommu_domain_alloc_paging,
+	.probe_device	    = hv_iommu_probe_device,
+	.probe_finalize     = hv_iommu_probe_finalize,
+	.release_device     = hv_iommu_release_device,
+	.def_domain_type    = hv_iommu_def_domain_type,
+	.device_group	    = hv_iommu_device_group,
+	.default_domain_ops = &(const struct iommu_domain_ops) {
+		.attach_dev   = hv_iommu_attach_dev,
+		.map_pages    = hv_iommu_map_pages,
+		.unmap_pages  = hv_iommu_unmap_pages,
+		.iova_to_phys = hv_iommu_iova_to_phys,
+		.free	      = hv_iommu_domain_free,
+	},
+	.owner		    = THIS_MODULE,
+	.identity_domain = &hv_def_identity_dom.iommu_dom,
+	.blocked_domain  = &hv_null_dom.iommu_dom,
+};
+
+static const struct iommu_domain_ops hv_special_domain_ops = {
+	.attach_dev = hv_iommu_attach_dev,
+};
+
+static void __init hv_initialize_special_domains(void)
+{
+	hv_def_identity_dom.iommu_dom.type = IOMMU_DOMAIN_IDENTITY;
+	hv_def_identity_dom.iommu_dom.ops = &hv_special_domain_ops;
+	hv_def_identity_dom.iommu_dom.owner = &hv_iommu_ops;
+	hv_def_identity_dom.iommu_dom.geometry = default_geometry;
+	hv_def_identity_dom.domid_num = HV_DEVICE_DOMAIN_ID_S2_DEFAULT; /* 0 */
+
+	hv_null_dom.iommu_dom.type = IOMMU_DOMAIN_BLOCKED;
+	hv_null_dom.iommu_dom.ops = &hv_special_domain_ops;
+	hv_null_dom.iommu_dom.owner = &hv_iommu_ops;
+	hv_null_dom.iommu_dom.geometry = default_geometry;
+	hv_null_dom.domid_num = HV_DEVICE_DOMAIN_ID_S2_NULL;  /* INTMAX */
+}
+
+static int __init hv_iommu_init(void)
+{
+	int ret;
+	struct iommu_device *iommup = &hv_virt_iommu;
+
+	if (!hv_is_hyperv_initialized())
+		return -ENODEV;
+
+	ret = iommu_device_sysfs_add(iommup, NULL, NULL, "%s", "hyperv-iommu");
+	if (ret) {
+		pr_err("Hyper-V: iommu_device_sysfs_add failed: %d\n", ret);
+		return ret;
+	}
+
+	/* This must come before iommu_device_register() because the latter
+	 * calls into the hooks.
+	 */
+	hv_initialize_special_domains();
+
+	ret = iommu_device_register(iommup, &hv_iommu_ops, NULL);
+	if (ret) {
+		pr_err("Hyper-V: iommu_device_register failed: %d\n", ret);
+		goto err_sysfs_remove;
+	}
+
+	pr_info("Hyper-V IOMMU initialized\n");
+
+	return 0;
+
+err_sysfs_remove:
+	iommu_device_sysfs_remove(iommup);
+	return ret;
+}
+
+void __init hv_iommu_detect(void)
+{
+	if (no_iommu || iommu_detected || hv_l1vh_partition())
+		return;
+
+	if (!(ms_hyperv.misc_features & HV_DEVICE_DOMAIN_AVAILABLE))
+		return;
+
+	iommu_detected = 1;
+	x86_init.iommu.iommu_init = hv_iommu_init;
+
+	pci_request_acs();
+}
diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
index e8cbc4e3f7ad..2c5d0812f19d 100644
--- a/include/asm-generic/mshyperv.h
+++ b/include/asm-generic/mshyperv.h
@@ -28,6 +28,9 @@
 
 #define VTPM_BASE_ADDRESS 0xfed40000
 
+#define HV_REP_COUNT_MAX        \
+	(HV_HYPERCALL_REP_COMP_MASK >> HV_HYPERCALL_REP_COMP_OFFSET)
+
 enum hv_partition_type {
 	HV_PARTITION_TYPE_GUEST,
 	HV_PARTITION_TYPE_ROOT,
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 964f1be8150c..f78645e8d776 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1769,4 +1769,10 @@ static inline unsigned long virt_to_hvpfn(void *addr)
 #define HVPFN_DOWN(x)	((x) >> HV_HYP_PAGE_SHIFT)
 #define page_to_hvpfn(page)	(page_to_pfn(page) * NR_HV_HYP_PAGES_IN_PAGE)
 
+#ifdef CONFIG_HYPERV_IOMMU
+void __init hv_iommu_detect(void);
+#else
+static inline void hv_iommu_detect(void) { }
+#endif /* CONFIG_HYPERV_IOMMU */
+
 #endif /* _HYPERV_H */
-- 
2.51.2.vfs.0.1

Re: [PATCH v5 7/9] x86/hyperv: Implement Hyper-V virtual IOMMU

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Broken partial unmapping logic incorrectly removes entire mapping ranges, leaks mappings, and causes catastrophic error rollback failures.
- [High] Calling dev_iommu_priv_set() in attach_dev callback without holding iommu_probe_device_lock causes a lockdep violation.
- [High] hv_iommu_map_pages ignores the caller's gfp_t argument and calls a potentially sleeping function.
- [Medium] Atomic counter wrap-around in domain ID allocation risks colliding with reserved hypervisor domain IDs.
- [Low] The __setup callback hv_iommu_setup_skip incorrectly returns 0, causing the kernel to treat the command-line argument as an unknown parameter.
--

--- Patch [7]: [PATCH v5 7/9] x86/hyperv: Implement Hyper-V virtual IOMMU ---
commit cdb89379b7cc5351b8dae8a1be7ede22f440c196
Author: Mukesh R <mrathor@linux.microsoft.com>

x86/hyperv: Implement Hyper-V virtual IOMMU

Add a new file to implement a virtual IOMMU that works with Microsoft Hyper-V
hypervisor. The driver communicates with the hypervisor for creating and 
deleting paging domains and mapping pages. The default domain type is 
IDENTITY.

> diff --git a/drivers/iommu/hyperv/hv-iommu-root.c b/drivers/iommu/hyperv/hv-iommu-root.c
> new file mode 100644
> index 0000000000000..97ddd69560318
> --- /dev/null
> +++ b/drivers/iommu/hyperv/hv-iommu-root.c
> @@ -0,0 +1,706 @@
[ ... ]
> +static char *pci_devs_to_skip;
> +static int __init hv_iommu_setup_skip(char *str)
> +{
> +	pci_devs_to_skip = str;
> +
> +	return 0;
> +}

[Severity: Low]
Should this __setup callback return 1? Returning 0 causes the kernel to
treat the command-line argument as an unknown parameter and pass it to
the init process as an environment variable.

[ ... ]
> +static size_t hv_iommu_del_tree_mappings(struct hv_domain *hvdom,
> +					 unsigned long iova, size_t size)
> +{
> +	unsigned long flags;
> +	size_t unmapped = 0;
> +	unsigned long last = iova + size - 1;
> +	struct hv_iommu_mapping *mapping = NULL;
> +	struct interval_tree_node *node, *next;
> +
> +	spin_lock_irqsave(&hvdom->mappings_lock, flags);
> +	next = interval_tree_iter_first(&hvdom->mappings_tree, iova, last);
> +	while (next) {
> +		node = next;
> +		mapping = container_of(node, struct hv_iommu_mapping, iova);
> +		next = interval_tree_iter_next(node, iova, last);
> +
> +		/* Splitting of a mapping is not supported at present */
> +		if (mapping->iova.start < iova)
> +			break;

[Severity: High]
How does this code handle partial unmaps? If an unmap request starts in the
middle of a mapping, doesn't the break statement here leave the memory
mapped? Furthermore, if the request starts at the beginning but is smaller
than the full mapping, this removes the entire node and returns its full size,
prompting the caller to erroneously unmap the full region from hardware.

[ ... ]
> +static struct iommu_domain *hv_iommu_domain_alloc_paging(struct device *dev)
> +{
> +	struct hv_domain *hvdom;
> +	int rc;
> +	u32 unique_id;
> +	u64 ptid = hv_get_current_partid();
> +
> +	if (ptid == HV_PARTITION_ID_INVALID)
> +		return NULL;
> +
> +	hvdom = kzalloc_obj(struct hv_domain);
> +	if (hvdom == NULL)
> +		return NULL;
> +
> +	spin_lock_init(&hvdom->mappings_lock);
> +	hvdom->mappings_tree = RB_ROOT_CACHED;
> +
> +	unique_id = (u32)atomic_inc_return(&hv_unique_id);
> +	if (unique_id == HV_DEVICE_DOMAIN_ID_S2_NULL)	/* ie, UINTMAX */
> +		goto out_err;

[Severity: Medium]
What happens when this 32-bit atomic counter wraps around? Since it will wrap
to 0 (which is HV_DEVICE_DOMAIN_ID_S2_DEFAULT), could this new domain
improperly map to the hypervisor's default domain and cause hypercall
failures? Should an IDA allocator be used instead?

[ ... ]
> +static int hv_iommu_attach_dev(struct iommu_domain *immdom, struct device *dev,
> +			       struct iommu_domain *old)
> +{
> +	struct pci_dev *pdev;
> +	int rc;
> +	struct hv_domain *hvdom_new = to_hv_domain(immdom);
> +	struct hv_domain *hvdom_prev = to_hv_domain(old);
[ ... ]
> +	rc = hv_iommu_att_dev2dom(hvdom_new, pdev);
> +
> +	if (rc == 0)
> +		dev_iommu_priv_set(dev, hvdom_new);  /* sets "private" field */

[Severity: High]
Can this trigger a lockdep splat? The core function dev_iommu_priv_set()
requires iommu_probe_device_lock to be held, but this attach_dev callback
executes with group->mutex held. Is it safe to use this private field for
dynamic domain state?

> +	else {
> +		if (hvdom_prev && !hv_special_domain(hvdom_prev))
> +			WARN(1, "Failed to attach dev:%s\n", pci_name(pdev));
> +		dev_iommu_priv_set(dev, NULL);
> +	}
> +
> +	return rc;
> +}
[ ... ]
> +static int hv_iommu_map_pages(struct iommu_domain *immdom, ulong iova,
> +			      phys_addr_t paddr, size_t pgsize, size_t pgcount,
> +			      int prot, gfp_t gfp, size_t *mapped)
> +{
[ ... ]
> +		if (hv_result(status) == HV_STATUS_INSUFFICIENT_MEMORY) {
> +			ret = hv_call_deposit_pages(NUMA_NO_NODE,
> +						    hv_current_partition_id,
> +						    256);

[Severity: High]
Does this sequence sleep in atomic context? The IOMMU map_pages callback can
be invoked in atomic contexts (such as inside interrupt handlers) and is
passed a gfp_t argument. However, this ignores the gfp argument and calls
hv_call_deposit_pages(), which allocates pages using GFP_KERNEL.

[ ... ]
> +	if (!hv_result_success(status)) {
> +		size_t done_size = done << HV_HYP_PAGE_SHIFT;
> +
> +		hv_status_err(status, "pgs:%lx/%lx iova:%lx\n",
> +			      done, npages, iova);
> +		/*
> +		 * lookup tree has all mappings [0 - size-1]. Below unmap will
> +		 * only remove from [0 - done], we need to remove second chunk
> +		 * [done+1 - size-1].
> +		 */
> +		hv_iommu_del_tree_mappings(hvdom, iova, size - done_size);

[Severity: High]
Is the error cleanup path unmapping memory that was never mapped? If a
multi-page map fails halfway, it tries to unmap the unmapped portion, which
hits the break condition in hv_iommu_del_tree_mappings() discussed earlier.
This fails to remove the node and then erroneously issues a hypercall to
unmap the entire size.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731223427.2554388-1-mrathor@linux.microsoft.com?part=7

[PATCH v5 8/9] mshv: Populate mmio mappings for PCI passthru

[Mukesh R]

Upon guest accesses, in case of missing mmio mappings, the hypervisor
generates unmapped gpa intercepts. In this path, lookup the PCI
resource pfn for the guest gpa, and ask the hypervisor to map it
via hypercall. The PCI resource pfn is maintained by the VFIO driver,
and obtained via fixup_user_fault() call (similar to KVM). Try to use
2M page size as much as possible for significant performance gains.

Also, remove existing code that is using vma->vm_pgoff to map mmio
space, it is broken and will cause panics.

Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
---
 drivers/hv/mshv_root.h         |   3 +-
 drivers/hv/mshv_root_hv_call.c | 104 +++++++++++++++++++++-------
 drivers/hv/mshv_root_main.c    | 120 ++++++++++++++++++++++++++++-----
 3 files changed, 185 insertions(+), 42 deletions(-)

diff --git a/drivers/hv/mshv_root.h b/drivers/hv/mshv_root.h
index 43114df3cbf0..2ef781cf8e83 100644
--- a/drivers/hv/mshv_root.h
+++ b/drivers/hv/mshv_root.h
@@ -301,7 +301,8 @@ int hv_call_create_partition(u64 flags,
 int hv_call_initialize_partition(u64 partition_id);
 int hv_call_finalize_partition(u64 partition_id);
 int hv_call_delete_partition(u64 partition_id);
-int hv_call_map_mmio_pages(u64 partition_id, u64 gfn, u64 mmio_spa, u64 numpgs);
+int hv_map_mmio_pages(u64 partition_id, struct mshv_mem_region *reg,
+		      u64 mmio_mfn);
 int hv_call_map_gpa_pages(u64 partition_id, u64 gpa_target, u64 page_count,
 			  u32 flags, struct page **pages);
 int hv_call_unmap_gpa_pages(u64 partition_id, u64 gpa_target, u64 page_count,
diff --git a/drivers/hv/mshv_root_hv_call.c b/drivers/hv/mshv_root_hv_call.c
index cb55d4d4be2e..4f6eb3a2aed0 100644
--- a/drivers/hv/mshv_root_hv_call.c
+++ b/drivers/hv/mshv_root_hv_call.c
@@ -189,31 +189,33 @@ int hv_call_delete_partition(u64 partition_id)
 }
 
 /* Ask the hypervisor to map guest ram pages or the guest mmio space */
-static int hv_do_map_gpa_hcall(u64 partition_id, u64 gfn, u64 page_struct_count,
-			       u32 flags, struct page **pages, u64 mmio_spa)
+static int hv_do_map_gpa_hcall(u64 partition_id, u64 gfn, u64 page_count,
+			       u32 flags, struct page **pages, u64 mmio_mfn)
 {
 	struct hv_input_map_gpa_pages *input_page;
 	u64 status, *pfnlist;
 	unsigned long irq_flags, large_shift = 0;
-	int ret = 0, done = 0;
-	u64 page_count = page_struct_count;
-
-	if (page_count == 0 || (pages && mmio_spa))
-		return -EINVAL;
+	int i, ret = 0, done = 0;
+	u64 adj_page_count = page_count;
 
-	if (flags & HV_MAP_GPA_LARGE_PAGE) {
-		if (mmio_spa)
+	if (mmio_mfn) {
+		if (pages)
 			return -EINVAL;
+		for (i = 0; i < page_count; i++)
+			if (page_is_ram(mmio_mfn + i))
+				return -EINVAL;
+	}
 
+	if (flags & HV_MAP_GPA_LARGE_PAGE) {
 		if (!HV_PAGE_COUNT_2M_ALIGNED(page_count))
 			return -EINVAL;
 
 		large_shift = HV_HYP_LARGE_PAGE_SHIFT - HV_HYP_PAGE_SHIFT;
-		page_count >>= large_shift;
+		adj_page_count >>= large_shift;
 	}
 
-	while (done < page_count) {
-		ulong i, completed, remain = page_count - done;
+	while (done < adj_page_count) {
+		ulong i, completed, remain = adj_page_count - done;
 		int rep_count = min(remain, HV_MAP_GPA_BATCH_SIZE);
 
 		local_irq_save(irq_flags);
@@ -230,13 +232,14 @@ static int hv_do_map_gpa_hcall(u64 partition_id, u64 gfn, u64 page_struct_count,
 			} else if (pages) {
 				u64 index = (done + i) << large_shift;
 
-				if (index >= page_struct_count) {
+				if (index >= page_count) {
 					ret = -EINVAL;
 					break;
 				}
 				pfnlist[i] = page_to_pfn(pages[index]);
 			} else {
-				pfnlist[i] = mmio_spa + done + i;
+				pfnlist[i] = mmio_mfn +
+						((done + i) << large_shift);
 			}
 		if (ret)
 			break;
@@ -254,6 +257,9 @@ static int hv_do_map_gpa_hcall(u64 partition_id, u64 gfn, u64 page_struct_count,
 				break;
 
 		} else if (!hv_result_success(status)) {
+			pr_err("%s: failed to map pages at gfn %#llx: completed %u/%llu, flags=%#x, status=%#llx (%s)\n",
+			       __func__, gfn, done, page_count, flags, status,
+			       hv_result_to_string(hv_result(status)));
 			ret = hv_result_to_errno(status);
 			break;
 		}
@@ -280,19 +286,69 @@ int hv_call_map_gpa_pages(u64 partition_id, u64 gpa_target, u64 page_count,
 				   flags, pages, 0);
 }
 
-/* Ask the hypervisor to map guest mmio space */
-int hv_call_map_mmio_pages(u64 partition_id, u64 gfn, u64 mmio_spa, u64 numpgs)
+/*
+ * Ask the hypervisor to map guest mmio space. Don't set HV_MAP_GPA_NOT_CACHED
+ * in hcall flags for slightly better performance, and in that case the guest
+ * stage 1 page table will control caching.
+ */
+int hv_map_mmio_pages(u64 partition_id, struct mshv_mem_region *reg,
+		      u64 mmio_mfn)
 {
-	int i;
-	u32 flags = HV_MAP_GPA_READABLE | HV_MAP_GPA_WRITABLE |
-		    HV_MAP_GPA_NOT_CACHED;
+	int rc;
+	u64 gfn, start_mmio_mfn, hpages;
+	u32 flags = HV_MAP_GPA_READABLE;
+	u64 numpgs = 0, numpgs_in_hpage = HPAGE_SIZE / PAGE_SIZE;
 
-	for (i = 0; i < numpgs; i++)
-		if (page_is_ram(mmio_spa + i))
-			return -EINVAL;
+	if (reg->hv_map_flags & HV_MAP_GPA_WRITABLE)
+		flags |= HV_MAP_GPA_WRITABLE;
+	if (reg->hv_map_flags & HV_MAP_GPA_EXECUTABLE)
+		flags |= HV_MAP_GPA_EXECUTABLE;
+
+	/*
+	 * Any significantly large range is expected to be properly aligned,
+	 * so keep it simple.
+	 */
+	gfn = reg->start_gfn;
+	start_mmio_mfn = mmio_mfn;
+	while (!HV_PAGE_COUNT_2M_ALIGNED(gfn) &&
+	       !HV_PAGE_COUNT_2M_ALIGNED(mmio_mfn) &&
+	       numpgs < reg->nr_pages) {
+		numpgs++;
+		gfn++;
+		mmio_mfn++;
+	}
+
+	if (numpgs) {
+		rc = hv_do_map_gpa_hcall(partition_id, reg->start_gfn, numpgs,
+					 flags, NULL, start_mmio_mfn);
+		if (rc || numpgs == reg->nr_pages)
+			return rc;
+	}
+
+	numpgs = reg->nr_pages - numpgs;
+
+	if (numpgs < numpgs_in_hpage)
+		return hv_do_map_gpa_hcall(partition_id, gfn, numpgs, flags,
+					   NULL, mmio_mfn);
+
+	for (hpages = 0; numpgs >= numpgs_in_hpage;) {
+		hpages++;
+		numpgs = numpgs - numpgs_in_hpage;
+	}
+	rc = hv_do_map_gpa_hcall(partition_id, gfn, hpages * numpgs_in_hpage,
+				 flags | HV_MAP_GPA_LARGE_PAGE, NULL, mmio_mfn);
+	if (rc)
+		return rc;
+
+	if (numpgs) {
+		gfn = gfn + hpages * numpgs_in_hpage;
+		mmio_mfn = mmio_mfn + hpages * numpgs_in_hpage;
+
+		rc = hv_do_map_gpa_hcall(partition_id, gfn, numpgs, flags, NULL,
+					 mmio_mfn);
+	}
 
-	return hv_do_map_gpa_hcall(partition_id, gfn, numpgs, flags, NULL,
-				   mmio_spa);
+	return rc;
 }
 
 int hv_call_unmap_gpa_pages(u64 partition_id, u64 gfn, u64 page_count_4k,
diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
index 79456388bba9..14a640750ccf 100644
--- a/drivers/hv/mshv_root_main.c
+++ b/drivers/hv/mshv_root_main.c
@@ -641,6 +641,96 @@ mshv_partition_region_by_gfn_get(struct mshv_partition *p, u64 gfn)
 	return region;
 }
 
+/*
+ * Check if uaddr is for mmio range. If yes, return 0 with mmio_pfn filled in
+ * else just return -errno.
+ */
+static int mshv_chk_get_mmio_start_pfn(u64 uaddr, u64 *mmio_pfnp, bool wr)
+{
+	struct vm_area_struct *vma;
+	bool is_mmio;
+	struct follow_pfnmap_args pfnmap_args;
+	int rc = -EINVAL;
+	enum fault_flag prot = wr ? FAULT_FLAG_WRITE : 0;
+
+	mmap_read_lock(current->mm);
+	vma = vma_lookup(current->mm, uaddr);
+	is_mmio = vma ? !!(vma->vm_flags & (VM_IO | VM_PFNMAP)) : 0;
+	if (!is_mmio)
+		goto unlock_mmap_out;
+
+	pfnmap_args.vma = vma;
+	pfnmap_args.address = uaddr;
+
+	rc = follow_pfnmap_start(&pfnmap_args);
+	if (rc) {
+		rc = fixup_user_fault(current->mm, uaddr, prot, NULL);
+		if (rc)
+			goto unlock_mmap_out;
+
+		rc = follow_pfnmap_start(&pfnmap_args);
+		if (rc)
+			goto unlock_mmap_out;
+	}
+
+	*mmio_pfnp = pfnmap_args.pfn;
+	follow_pfnmap_end(&pfnmap_args);
+
+unlock_mmap_out:
+	mmap_read_unlock(current->mm);
+	return rc;
+}
+
+/*
+ * Check if the unmapped gpa belongs to mmio space. If yes, resolve it.
+ *
+ * Returns: True if valid mmio intercept and handled, else false.
+ */
+static bool mshv_handle_unmapped_gpa(struct mshv_vp *vp)
+{
+	struct hv_message *hvmsg = vp->vp_intercept_msg_page;
+	u64 gfn, uaddr, mmio_mfn;
+	struct mshv_mem_region *rg;
+	bool writable;
+	int rc = -EINVAL;
+	struct mshv_partition *pt = vp->vp_partition;
+#if defined(CONFIG_X86_64)
+	struct hv_x64_memory_intercept_message *msg =
+		(struct hv_x64_memory_intercept_message *)hvmsg->u.payload;
+#elif defined(CONFIG_ARM64)
+	struct hv_arm64_memory_intercept_message *msg =
+		(struct hv_arm64_memory_intercept_message *)hvmsg->u.payload;
+#endif
+
+	gfn = msg->guest_physical_address >> HV_HYP_PAGE_SHIFT;
+
+	rg = mshv_partition_region_by_gfn_get(pt, gfn);
+	if (rg == NULL)
+		return false;
+	if (rg->mreg_type != MSHV_REGION_TYPE_MMIO)
+		goto put_rg_out;
+
+	uaddr = rg->start_uaddr + ((gfn - rg->start_gfn) << HV_HYP_PAGE_SHIFT);
+
+	writable = rg->hv_map_flags & HV_MAP_GPA_WRITABLE;
+	rc = mshv_chk_get_mmio_start_pfn(uaddr, &mmio_mfn, writable);
+	if (rc)
+		goto put_rg_out;
+
+	mmio_mfn = mmio_mfn - (gfn - rg->start_gfn);   /* start of the range */
+
+	/* Map the entire mmio region now. mmap lock not needed because we
+	 * are just mapping GPA to static device addresses, and the above
+	 * mshv_partition_region_by_gfn_get() gets a hold on the region and
+	 * hence its GPA. Unmap will occur when region ref count goes to 0.
+	 */
+	rc = hv_map_mmio_pages(pt->pt_id, rg, mmio_mfn);
+
+put_rg_out:
+	mshv_region_put(rg);
+	return rc == 0;
+}
+
 /**
  * mshv_handle_gpa_intercept - Handle GPA (Guest Physical Address) intercepts.
  * @vp: Pointer to the virtual processor structure.
@@ -699,6 +789,8 @@ static bool mshv_handle_gpa_intercept(struct mshv_vp *vp)
 static bool mshv_vp_handle_intercept(struct mshv_vp *vp)
 {
 	switch (vp->vp_intercept_msg_page->header.message_type) {
+	case HVMSG_UNMAPPED_GPA:
+		return mshv_handle_unmapped_gpa(vp);
 	case HVMSG_GPA_INTERCEPT:
 		return mshv_handle_gpa_intercept(vp);
 	}
@@ -1322,16 +1414,8 @@ static int mshv_prepare_pinned_region(struct mshv_mem_region *region)
 }
 
 /*
- * This maps two things: guest RAM and for pci passthru mmio space.
- *
- * mmio:
- *  - vfio overloads vm_pgoff to store the mmio start pfn/spa.
- *  - Two things need to happen for mapping mmio range:
- *	1. mapped in the uaddr so VMM can access it.
- *	2. mapped in the hwpt (gfn <-> mmio phys addr) so guest can access it.
- *
- *   This function takes care of the second. The first one is managed by vfio,
- *   and hence is taken care of via vfio_pci_mmap_fault().
+ * This is called for both user ram and mmio space. The mmio space is not
+ * mapped here, but later during intercept on demand.
  */
 static long
 mshv_map_user_memory(struct mshv_partition *partition,
@@ -1340,7 +1424,6 @@ mshv_map_user_memory(struct mshv_partition *partition,
 	struct mshv_mem_region *region;
 	struct vm_area_struct *vma;
 	bool is_mmio;
-	ulong mmio_pfn;
 	long ret;
 
 	if (mem->flags & BIT(MSHV_SET_MEM_BIT_UNMAP) ||
@@ -1350,12 +1433,19 @@ mshv_map_user_memory(struct mshv_partition *partition,
 	mmap_read_lock(current->mm);
 	vma = vma_lookup(current->mm, mem->userspace_addr);
 	is_mmio = vma ? !!(vma->vm_flags & (VM_IO | VM_PFNMAP)) : 0;
-	mmio_pfn = is_mmio ? vma->vm_pgoff : 0;
 	mmap_read_unlock(current->mm);
 
 	if (!vma)
 		return -EINVAL;
 
+	if (is_mmio) {
+		size_t vma_sz = vma->vm_end - vma->vm_start;
+
+		/* Upon mmio intercept, entire region is mapped */
+		if (mem->size > vma_sz)
+			return -EPERM;
+	}
+
 	ret = mshv_partition_create_region(partition, mem, &region,
 					   is_mmio);
 	if (ret)
@@ -1376,11 +1466,7 @@ mshv_map_user_memory(struct mshv_partition *partition,
 					    region->nr_pages,
 					    HV_MAP_GPA_NO_ACCESS, NULL);
 		break;
-	case MSHV_REGION_TYPE_MMIO:
-		ret = hv_call_map_mmio_pages(partition->pt_id,
-					     region->start_gfn,
-					     mmio_pfn,
-					     region->nr_pages);
+	default:
 		break;
 	}
 
-- 
2.51.2.vfs.0.1

Re: [PATCH v5 8/9] mshv: Populate mmio mappings for PCI passthru

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:

New issues:
- [Critical] Insecure MMIO region bounds check and PFN extrapolation allows mapping arbitrary host RAM into the guest.
- [Critical] Conflation of Host PFNs with Hyper-V 4KB PFNs causes arbitrary memory mapping on architectures with >4KB pages (ARM64).
- [Critical] Missing MMU interval notifier allows stale physical mappings (TOCTOU and Use-After-Unmap).
- [High] Use-After-Free of VMA pointer after dropping mmap_read_lock.
- [High] Alignment check logic error causes hypercall protocol violation for large pages.
- [High] Overloading `mmio_mfn` with `0` bypasses `page_is_ram()` checks for PFN 0.

Pre-existing issues:
- [High] Incorrect unit passed to unmap function on error path causes silent resource leak.
- [High] Unbounded O(N) `page_is_ram()` checks in process context cause CPU soft lockups.
--

--- Patch [8]: [PATCH v5 8/9] mshv: Populate mmio mappings for PCI passthru ---
commit ef68862e98379b2ba894e7946e9748f29e46d61f
Author: Mukesh R <mrathor@linux.microsoft.com>
Subject: mshv: Populate mmio mappings for PCI passthru

This commit handles hypervisor unmapped GPA intercepts by resolving the PCI
resource PFN and mapping it via hypercall. It attempts to map 2M pages where
possible to improve performance and removes broken VMA offset code.

> diff --git a/drivers/hv/mshv_root_hv_call.c b/drivers/hv/mshv_root_hv_call.c
> index cb55d4d4be2e5..4f6eb3a2aed03 100644
> --- a/drivers/hv/mshv_root_hv_call.c
> +++ b/drivers/hv/mshv_root_hv_call.c
[ ... ]
> -static int hv_do_map_gpa_hcall(u64 partition_id, u64 gfn, u64 page_struct_count,
> -			       u32 flags, struct page **pages, u64 mmio_spa)
> +static int hv_do_map_gpa_hcall(u64 partition_id, u64 gfn, u64 page_count,
> +			       u32 flags, struct page **pages, u64 mmio_mfn)
>  {
[ ... ]
> -	if (page_count == 0 || (pages && mmio_spa))
> -		return -EINVAL;
> +	if (mmio_mfn) {
> +		if (pages)
>  			return -EINVAL;
> +		for (i = 0; i < page_count; i++)
> +			if (page_is_ram(mmio_mfn + i))
> +				return -EINVAL;
> +	}

[Severity: High]
Does this check evaluate to false when a legitimate MMIO region points to
physical PFN 0?

If mmio_mfn is 0, it appears to bypass the page_is_ram() loop entirely. Could
this allow a VMM process to map the host's physical RAM page 0 into the guest
as MMIO?

[Severity: High]
This is a pre-existing issue, but does this loop iterate over a potentially
massive number of pages without yielding?

Since page_count is derived from the user-provided region size, a maliciously
large region might force the host CPU into a soft lockup. Should we add a
cond_resched() here?

[ ... ]
>  		} else if (!hv_result_success(status)) {
> +			pr_err("%s: failed to map pages at gfn %#llx: completed %u/%llu, flags=%#x, status=%#llx (%s)\n",
> +			       __func__, gfn, done, page_count, flags, status,
> +			       hv_result_to_string(hv_result(status)));
>  			ret = hv_result_to_errno(status);
>  			break;
>  		}

[Severity: High]
This is a pre-existing issue, but does the error rollback path near the end
of this function pass the wrong units to hv_call_unmap_gpa_pages()?

The variable done tracks successfully mapped large pages, but
hv_call_unmap_gpa_pages() expects its page_count argument in 4KB units. If we
pass the large page count instead, it seems like it will fail the alignment
check and silently leak dangling large page mappings in the hypervisor's
stage 2 tables.

[ ... ]
> +int hv_map_mmio_pages(u64 partition_id, struct mshv_mem_region *reg,
> +		      u64 mmio_mfn)
>  {
[ ... ]
> +	while (!HV_PAGE_COUNT_2M_ALIGNED(gfn) &&
> +	       !HV_PAGE_COUNT_2M_ALIGNED(mmio_mfn) &&
> +	       numpgs < reg->nr_pages) {
> +		numpgs++;
> +		gfn++;
> +		mmio_mfn++;
> +	}

[Severity: High]
Does the logical AND operator here cause the alignment loop to exit
prematurely?

If the offsets of gfn and mmio_mfn relative to a 2MB boundary differ, the loop
will break as soon as only one of them becomes aligned, leaving the other
unaligned.

Will this cause us to pass unaligned addresses to a large-page hypercall later
in this function, violating the Hyper-V protocol?

> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> index fc08e994ecdf5..b45c6b275f999 100644
> --- a/drivers/hv/mshv_root_main.c
> +++ b/drivers/hv/mshv_root_main.c
[ ... ]
> +static bool mshv_handle_unmapped_gpa(struct mshv_vp *vp)
> +{
[ ... ]
> +	mmio_mfn = mmio_mfn - (gfn - rg->start_gfn);   /* start of the range */

[Severity: Critical]
Could this backwards extrapolation yield arbitrary physical addresses if a
region spans outside its intended VMA? By extrapolating the base PFN for the
entire region backwards, it looks like it could map adjacent, unauthorized host
physical memory into the guest.

Also, on architectures with page sizes larger than 4KB (like ARM64), does this
mix incompatible units?

The function mshv_chk_get_mmio_start_pfn() returns a host PFN (e.g. 64KB
scaled), but this subtracts a 4KB-based Hyper-V GFN offset. Passing this
corrupted value to the hypervisor could cause incorrect physical addresses to
be mapped.

> +
> +	/* Map the entire mmio region now. mmap lock not needed because we
> +	 * are just mapping GPA to static device addresses, and the above
> +	 * mshv_partition_region_by_gfn_get() gets a hold on the region and
> +	 * hence its GPA. Unmap will occur when region ref count goes to 0.
> +	 */
> +	rc = hv_map_mmio_pages(pt->pt_id, rg, mmio_mfn);

[Severity: Critical]
Are these physical memory mappings protected against stale access?

Because MMIO regions do not register an mmu_interval_notifier, there doesn't
appear to be a mechanism to invalidate the hypervisor's translation tables if
the device is unbound or the VMA is unmapped.

Furthermore, dropping the mmap lock earlier seems to introduce a TOCTOU race
where a concurrent munmap could invalidate the PFN before the hypercall is
made. Will this allow stale mappings to persist?

> @@ -1340,7 +1424,6 @@ mshv_map_user_memory(struct mshv_partition *partition,
[ ... ]
>  	mmap_read_lock(current->mm);
>  	vma = vma_lookup(current->mm, mem->userspace_addr);
>  	is_mmio = vma ? !!(vma->vm_flags & (VM_IO | VM_PFNMAP)) : 0;
> -	mmio_pfn = is_mmio ? vma->vm_pgoff : 0;
>  	mmap_read_unlock(current->mm);
>  
>  	if (!vma)
>  		return -EINVAL;
>  
> +	if (is_mmio) {
> +		size_t vma_sz = vma->vm_end - vma->vm_start;

[Severity: High]
Can this result in a use-after-free?

The vma pointer is unconditionally dereferenced here, but the lock is dropped
just above it with mmap_read_unlock(current->mm).

Could a concurrent thread in the VMM process calling munmap() unlink and
free the VMA before this calculation occurs, leading to memory corruption?

> +
> +		/* Upon mmio intercept, entire region is mapped */
> +		if (mem->size > vma_sz)
> +			return -EPERM;
> +	}

[Severity: Critical]
Does this bounds check properly validate the starting offset?

It only compares the total size against vma_sz, neglecting to check if the
region's start offset fits within the VMA. For example, it doesn't verify
that userspace_addr + size <= vm_end.

Would this allow a malicious userspace VMM to create a region that spans
outside its intended VMA?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731223427.2554388-1-mrathor@linux.microsoft.com?part=8

[PATCH v5 9/9] mshv: Disable movable regions upfront if device passthru

[Mukesh R]

If a VM is started with device attached, the mem regions must be marked
non-movable as the device attach hypercall right away allows the use of
SLAT for IOMMU. Marking them non-movable forces mapping of the entire
guest RAM in the SLAT at the time of region creation along with the region
pinned. Also, because a device could be dynamically attached much later
in a VM, create a module parameter to allow a user to pin them upfront
if they anticipate such an action. It is understood that VFIO pins the
regions also, so there will be double pinning. This will be enhanced in
the near future with a third region type that is not movable nor pinned.

Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
---
 drivers/hv/mshv_root.h      |  1 +
 drivers/hv/mshv_root_main.c | 15 ++++++++++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/hv/mshv_root.h b/drivers/hv/mshv_root.h
index 2ef781cf8e83..f5747c43ad82 100644
--- a/drivers/hv/mshv_root.h
+++ b/drivers/hv/mshv_root.h
@@ -141,6 +141,7 @@ struct mshv_partition {
 	pid_t pt_vmm_tgid;
 	bool import_completed;
 	bool pt_initialized;
+	bool pt_regions_pinned;
 #if IS_ENABLED(CONFIG_DEBUG_FS)
 	struct dentry *pt_stats_dentry;
 	struct dentry *pt_vp_dentry;
diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
index 14a640750ccf..e24b44051456 100644
--- a/drivers/hv/mshv_root_main.c
+++ b/drivers/hv/mshv_root_main.c
@@ -46,6 +46,10 @@ MODULE_DESCRIPTION("Microsoft Hyper-V root partition VMM interface /dev/mshv");
 #define HV_VP_COUNTER_ROOT_DISPATCH_THREAD_BLOCKED 95
 #endif
 
+static uint hv_memreg;  /* mshv_mem_region allocations type */
+module_param(hv_memreg, uint, 0644);
+MODULE_PARM_DESC(hv_memreg, "0: movable (default), 1: pinned");
+
 struct mshv_root mshv_root;
 
 enum hv_scheduler_type hv_scheduler_type;
@@ -1302,6 +1306,12 @@ static void mshv_async_hvcall_handler(void *data, u64 *status)
 	*status = partition->async_hypercall_status;
 }
 
+static bool mshv_do_pt_regions_pinned(struct mshv_partition *pt)
+{
+	return pt->pt_regions_pinned || mshv_partition_encrypted(pt) ||
+	       hv_memreg == 1;
+}
+
 /*
  * NB: caller checks and makes sure mem->size is page aligned
  * Returns: 0 with regionpp updated on success, or -errno
@@ -1332,7 +1342,7 @@ static int mshv_partition_create_region(struct mshv_partition *partition,
 
 	if (is_mmio)
 		rg->mreg_type = MSHV_REGION_TYPE_MMIO;
-	else if (mshv_partition_encrypted(partition) ||
+	else if (mshv_do_pt_regions_pinned(partition) ||
 		 !mshv_region_movable_init(rg))
 		rg->mreg_type = MSHV_REGION_TYPE_MEM_PINNED;
 	else
@@ -1818,6 +1828,9 @@ static long mshv_partition_ioctl_create_device(struct mshv_partition *partition,
 	if (copy_to_user(uarg, &devargk, sizeof(devargk)))
 		return -EFAULT;    /* cleanup in mshv_device_fop_release() */
 
+	/* For now, just mark regions pinned if there is device passthru. */
+	partition->pt_regions_pinned = true;
+
 	return 0;
 
 undo_out:
-- 
2.51.2.vfs.0.1

Re: [PATCH v5 9/9] mshv: Disable movable regions upfront if device passthru

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Bypass of memory pinning enforcement on device creation via `copy_to_user` failure.

Pre-existing issues:
- [High] Error paths in `mshv_map_user_memory` and `mshv_prepare_pinned_region` cause a NULL pointer dereference in `unpin_user_pages()`.
--

--- Patch [9]: [PATCH v5 9/9] mshv: Disable movable regions upfront if device passthru ---
commit 43489ca84b127042602836c2214d379f535126bf
Author: Mukesh R <mrathor@linux.microsoft.com>

mshv: Disable movable regions upfront if device passthru

If a VM is started with a device attached, the memory regions must be marked
non-movable to allow the use of SLAT for IOMMU. This commit introduces a
module parameter to pin regions upfront and updates the device creation ioctl
to mark regions as pinned when a device is passed through.

> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> --- a/drivers/hv/mshv_root_main.c
> +++ b/drivers/hv/mshv_root_main.c
[ ... ]
> @@ -1332,7 +1342,7 @@ static int mshv_partition_create_region(struct mshv_partition *partition,
>  
>  	if (is_mmio)
>  		rg->mreg_type = MSHV_REGION_TYPE_MMIO;
> -	else if (mshv_partition_encrypted(partition) ||
> +	else if (mshv_do_pt_regions_pinned(partition) ||
>  		 !mshv_region_movable_init(rg))
>  		rg->mreg_type = MSHV_REGION_TYPE_MEM_PINNED;

[Severity: High]
This is a pre-existing issue, but I noticed an error handling problem with
pinned regions that might be triggered more frequently now.

If pinning fails in mshv_map_user_memory(), we drop the reference:

mshv_map_user_memory() {
    ...
    errout:
        mshv_region_put(region);
        return ret;
}

This triggers mshv_region_destroy() which unconditionally invalidates the
region:

mshv_region_destroy() {
    ...
    mshv_region_invalidate(region);
    vfree(region);
}

If the region pages were already partially unpinned and NULLed out during
the pinning failure, mshv_region_invalidate_pages() will pass those NULL
pointers directly to unpin_user_pages():

mshv_region_invalidate_pages() {
    ...
    if (region->mreg_type == MSHV_REGION_TYPE_MEM_PINNED)
        unpin_user_pages(region->mreg_pages + page_offset, page_count);
}

Since unpin_user_pages() does not check for NULL pointers, does this
sequence result in a NULL pointer dereference when cleaning up after a
pinning failure?

[ ... ]
> @@ -1818,6 +1828,9 @@ static long mshv_partition_ioctl_create_device(struct mshv_partition *partition,
>  	if (copy_to_user(uarg, &devargk, sizeof(devargk)))
>  		return -EFAULT;    /* cleanup in mshv_device_fop_release() */
>  
> +	/* For now, just mark regions pinned if there is device passthru. */
> +	partition->pt_regions_pinned = true;
> +
>  	return 0;

[Severity: High]
If copy_to_user() fails here, we return -EFAULT, but the file descriptor
has already been installed into the process's file descriptor table via
anon_inode_getfd() and remains active. 

Could this allow userspace to bypass setting pt_regions_pinned to true
while successfully obtaining and using the device file descriptor? If a
process intentionally passes an invalid pointer, it seems it might be able
to map movable memory while still attaching a device.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731223427.2554388-1-mrathor@linux.microsoft.com?part=9