Re: [PATCH V4 5/5] x86/hyperv: Implement root partition IOMMU kernel only driver

[Mukesh R]

On 11/6/26 13:54, Easwar Hariharan wrote:
> On 11/4/2026 18:01, Mukesh R wrote:
> > Add a new file to implement a kernel only virtual IOMMU that works
> > on Microsoft Hyper-V hypervisor (aka MSHV) in privileged VMs aka
> > root VMs. The hypervisor claims the IOMMU upon boot, and this driver
> > communicates with it for creating and deleting paging domains, attaching
> > of devices, mapping and unmapping of pages, etc. During boot, hypervisor
> > automatically creates identity and blocked domains, so there is no need
> > to do hypercalls to create them. This is a kernel only driver and only
> > supported on baremetal root (and not L1VH root) without any guest passthru
> > support. Support for guest device passthru will be added incrementally.
> >
> > Signed-off-by: Mukesh R <mrathor@xxxxxxxxxxxxxxxxxxx>
> > ---
> >   arch/x86/kernel/pci-dma.c            |   2 +
> >   drivers/iommu/hyperv/Kconfig         |  13 +
> >   drivers/iommu/hyperv/Makefile        |   1 +
> >   drivers/iommu/hyperv/hv-iommu-root.c | 613 +++++++++++++++++++++++++++
> >   include/asm-generic/mshyperv.h       |   3 +
> >   include/linux/hyperv.h               |   6 +
> >   6 files changed, 638 insertions(+)
> >   create mode 100644 drivers/iommu/hyperv/hv-iommu-root.c
>
> <snip>
>
> >   obj-$(CONFIG_HYPERV_PVIOMMU) += hv-iommu-guest.o
> > +obj-$(CONFIG_HYPERV_ROOT_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..e0e3e9d4385e
> > --- /dev/null
> > +++ b/drivers/iommu/hyperv/hv-iommu-root.c
> > @@ -0,0 +1,613 @@
> > +// 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"
> > +
> > +static dma_addr_t hv_iommu_max_iova;
> > +static struct iommu_domain_ops hv_paging_domain_ops;
> > +
> > +/* IOMMU device that we export to the world. MSHV supports max of one */
> > +static struct iommu_device hv_virt_iommu;
> > +
> > +struct hv_iommu_domain {
> > +	struct iommu_domain iommu_dom;
> > +	u32 domid_num;			      /* as opposed to domain_id.type */
> > +	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_iommu_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. It has two types:
> > + *   S2 default: access to entire root partition memory. This for the host
> > + *		 maps to IOMMU_DOMAIN_IDENTITY in the iommu subsystem, and
> > + *		 is called HV_DEVICE_DOMAIN_ID_S2_DEFAULT in the hypervisor.
> > + *		 (Linux renamed to : HV_DEVICE_DOMAIN_ID_DEFAULT)
> > + *   S2 NULL: Blocks everything, ie, IOMMU_DOMAIN_BLOCKED in linux.
> > + */
> > +
> > +/*
> > + * 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_iommu_domain hv_def_identity_dom;
> > +static struct hv_iommu_domain hv_def_blocked_dom;
> > +
> > +/*
> > + * The hypervisor owns the IOMMU. As such it will choose the best page size
> > + * and whenever possible use larger page sizes automatically. The map gpa
> > + * hypercall to map the iova however currently uses only 4K pfns as input.
> > + * So we specify 4K as the IOMMU PAGE_SIZE.
> > + */
> > +#define HV_IOMMU_PGSIZES SZ_4K
> > +
> > +static DEFINE_IDA(hv_iommu_domids);   /* unique numeric ids for new domains */
> > +
> > +static bool hv_iommu_capable(struct device *dev, enum iommu_cap cap)
> > +{
> > +	switch (cap) {
> > +	case IOMMU_CAP_CACHE_COHERENCY:
> > +		return true;
> > +	default:
> > +		return false;
> > +	}
> > +}
>
> Seems like this could match the guest driver and just be: return cap == IOMMU_CAP_CACHE_COHERENCY.
>
> <snip>
>
> > +
> > +static struct iommu_domain_ops hv_paging_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,
> > +};
> > +
> > +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,
> > +	.device_group	    = pci_device_group,
> > +	.get_resv_regions   = hv_iommu_get_resv_regions,
> > +	.owner		    = THIS_MODULE,
> > +	.identity_domain    = &hv_def_identity_dom.iommu_dom,
> > +	.blocked_domain     = &hv_def_blocked_dom.iommu_dom,
> > +	.release_domain     = &hv_def_blocked_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.domid_num = HV_DEVICE_DOMAIN_ID_DEFAULT; /* 0 */
> > +
> > +	hv_def_blocked_dom.iommu_dom.type = IOMMU_DOMAIN_BLOCKED;
> > +	hv_def_blocked_dom.iommu_dom.ops = &hv_special_domain_ops;
> > +	hv_def_blocked_dom.iommu_dom.owner = &hv_iommu_ops;
> > +	hv_def_blocked_dom.domid_num = HV_DEVICE_DOMAIN_ID_NULL;    /* INTMAX */
> > +}
> > +
>
> A net negative 9 lines to initialize these with .field notation, as suggested in v0: https://lore.kernel.org/all/4f7cadc5-817c-4163-bac8-1c02a7c6a862@xxxxxxxxxxxxxxxxxxx/

Huh! which are the 9 lines you are claiming to have removed? are you saying
the definition part is not a "line"?

> Wasn't super hard to do the 1 forward declaration needed...

Not a question of hard or not. Having that upfront in the module provides
good introduction to a reader, specially since hyperv is all new around
here and internals are not readily available. Moreover, imo, generally
just embedding definitions in code abrupts the flow often, and historically
hasn't been a common practice.

Thanks,
-Mukesh


> diff --git a/drivers/iommu/hyperv/hv-iommu-root.c b/drivers/iommu/hyperv/hv-iommu-root.c
> index e0e3e9d4385e2..4e6bf94413458 100644
> --- a/drivers/iommu/hyperv/hv-iommu-root.c
> +++ b/drivers/iommu/hyperv/hv-iommu-root.c
> @@ -12,6 +12,7 @@
>   #include "../dma-iommu.h"
>   
>   static dma_addr_t hv_iommu_max_iova;
> +static struct iommu_ops hv_iommu_ops;
>   static struct iommu_domain_ops hv_paging_domain_ops;
>   
>   /* IOMMU device that we export to the world. MSHV supports max of one */
> @@ -32,23 +33,6 @@ struct hv_iommu_mapping {
>   	u32 flags;
>   };
>   
> -/*
> - * By default, during boot the hypervisor creates one Stage 2 (S2) default
> - * domain. It has two types:
> - *   S2 default: access to entire root partition memory. This for the host
> - *		 maps to IOMMU_DOMAIN_IDENTITY in the iommu subsystem, and
> - *		 is called HV_DEVICE_DOMAIN_ID_S2_DEFAULT in the hypervisor.
> - *		 (Linux renamed to : HV_DEVICE_DOMAIN_ID_DEFAULT)
> - *   S2 NULL: Blocks everything, ie, IOMMU_DOMAIN_BLOCKED in linux.
> - */
> -
> -/*
> - * 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_iommu_domain hv_def_identity_dom;
> -static struct hv_iommu_domain hv_def_blocked_dom;
> -
>   /*
>    * The hypervisor owns the IOMMU. As such it will choose the best page size
>    * and whenever possible use larger page sizes automatically. The map gpa
> @@ -488,6 +472,35 @@ static void hv_iommu_get_resv_regions(struct device *dev,
>   	iommu_add_resv_x86_msi_region(head);
>   }
>   
> +static const struct iommu_domain_ops hv_special_domain_ops = {
> +	.attach_dev = hv_iommu_attach_dev,
> +};
> +
> +/*
> + * By default, during boot the hypervisor creates one Stage 2 (S2) default
> + * domain. It has two types:
> + *   S2 default: access to entire root partition memory. This for the host
> + *		 maps to IOMMU_DOMAIN_IDENTITY in the iommu subsystem, and
> + *		 is called HV_DEVICE_DOMAIN_ID_S2_DEFAULT in the hypervisor.
> + *		 (Linux renamed to : HV_DEVICE_DOMAIN_ID_DEFAULT)
> + *   S2 NULL: Blocks everything, ie, IOMMU_DOMAIN_BLOCKED in linux.
> + * 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_iommu_domain hv_def_identity_dom = {
> +	.iommu_dom.type = IOMMU_DOMAIN_IDENTITY,
> +	.iommu_dom.ops = &hv_special_domain_ops,
> +	.iommu_dom.owner = &hv_iommu_ops,
> +	.domid_num = HV_DEVICE_DOMAIN_ID_DEFAULT, /* 0 */
> +};
> +
> +static struct hv_iommu_domain hv_def_blocked_dom = {
> +	.iommu_dom.type = IOMMU_DOMAIN_BLOCKED,
> +	.iommu_dom.ops = &hv_special_domain_ops,
> +	.iommu_dom.owner = &hv_iommu_ops,
> +	.domid_num = HV_DEVICE_DOMAIN_ID_NULL,    /* INTMAX */
> +};
> +
>   static struct iommu_domain_ops hv_paging_domain_ops = {
>   	.attach_dev = hv_iommu_attach_dev,
>   	.map_pages = hv_iommu_map_pages,
> @@ -508,23 +521,6 @@ static struct iommu_ops hv_iommu_ops = {
>   	.release_domain     = &hv_def_blocked_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.domid_num = HV_DEVICE_DOMAIN_ID_DEFAULT; /* 0 */
> -
> -	hv_def_blocked_dom.iommu_dom.type = IOMMU_DOMAIN_BLOCKED;
> -	hv_def_blocked_dom.iommu_dom.ops = &hv_special_domain_ops;
> -	hv_def_blocked_dom.iommu_dom.owner = &hv_iommu_ops;
> -	hv_def_blocked_dom.domid_num = HV_DEVICE_DOMAIN_ID_NULL;    /* INTMAX */
> -}
> -
>   /*
>    * In general, the hypervisor does not support the capabilities hypercall for
>    * root partitions, an exception was made for max_iova_width. For IOMMU cap,
> @@ -571,11 +567,6 @@ static int __init hv_root_iommu_init(void)
>   
>   	hv_iommu_max_iova = DMA_BIT_MASK(caps.max_iova_width);
>   
> -	/* This must come before iommu_device_register() because the latter
> -	 * calls into the hooks.
> -	 */
> -	hv_initialize_special_domains();
> -
>   	rc = iommu_device_sysfs_add(iommup, NULL, NULL, "%s", "hyperv-iommu");
>   	if (rc) {
>   		pr_err("Hyper-V: iommu_device_sysfs_add failed: %d\n", rc);
>
>
> <snip>
>
> Thanks,
> Easwar (he/him)