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

[Easwar Hariharan <easwar.hariharan@linux.microsoft.com>]

On 11/6/2026 15:39, Mukesh R wrote:
> 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@linux.microsoft.com>
>>> ---
>>>   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@linux.microsoft.com/
> 
> Huh! which are the 9 lines you are claiming to have removed? are you saying
> the definition part is not a "line"?

It was the diffstat of the diff I pasted below.

 drivers/iommu/hyperv/hv-iommu-root.c | 69 ++++++++++++++++++++++++++++++---------------------------------------
 1 file changed, 30 insertions(+), 39 deletions(-)

> 
>> Wasn't super hard to do the 1 forward declaration needed...
> 
> Not a question of hard or not.

I was referencing your response to v0:

"it has dependencies on things declared above it, so not worth forward
declarations etc.."

 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.
 
I'm having difficulty parsing this, but I *think* you're saying that putting data structure
definitions in between functions hasn't been a common practice, and that may be true,
but I'd just ask you to look at the intel/, amd/, and arm-smmu-v3/ drivers, it's a
well-established pattern in this subsystem.

Thanks,
Easwar (he/him)

> 
> 
>> 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)
>