Re: [PATCH V1 3/3] x86/hyperv: Implement root VM IOMMU kernel only driver

[Mukesh R <mrathor@linux.microsoft.com>]

On 9/25/26 00:13, J?rg R?del wrote:
> On Thu, Sep 24, 2026 at 05:14:12PM -0700, Mukesh R wrote:
>> So, I'm at a bit of loss. I could go back to V0 which did what what you
>> suggested above, but it can also leave stale mappings when map hypercall
>> succeeds partially and we delete the entire tree node in cleanup (but
>> we could add to the tree back whatever succeeded, but that could fail
>> also, but then we could unmap whatever was mapped, but that could fail
>> also.. see :)...).
>>
> 
> The root-problem seems to be that every operation in the step can fail,
> inluding the unmap operation. This makes it really hard to maintain a
> consistent state. Are there any guarantees Hyper-V gives on the unmap
> operation, e.g under which circumstances it can fail? It would really help if
> the code could assume that it will never fail, or treat a failure as a hard
> error.

I talked to the hyp dev, and unmap is guaranteed to succeed unless
the input is bad, even suggesting i could put a bug there. But i know
there is aversion to putting lot of BUG()s, so i'll just leave the WARN
there.

> If there are no guarantees HV can give, a solution might be to add a
> mapping-resize operation to the internal tree (which can not fail) to correctly
> update the tree structure on partially successful unmaps.

Yeah, perhaps we could do that in future to benefit all drivers.

> I still believe that the internal tree structure should be updated before the
> HV map operation takes place.

Yes, i'll move that before map.

Thanks again,
-Mukesh

>>>> +static int __init hv_iommu_init(void)
>>>> +{
>>>> +	int rc;
>>>> +	struct iommu_device *iommup = &hv_virt_iommu;
>>>> +	struct hv_output_get_iommu_capabilities caps;
>>>> +
>>>> +	if (!hv_is_hyperv_initialized())
>>>> +		return -ENODEV;
>>>> +
>>>> +	rc = hv_iommu_get_caps(&caps);
>>>> +	if (rc)
>>>> +		return rc;
>>>> +
>>>
>>> The capabilities returned need more checking. I think at least it needs a check
>>> for HV_IOMMU_CAP_PRESENT and that PAGE_SIZE is set in the pgsize_bitmap.
>>
>> In general this caps hypercall is not available to root partitions,
>> our only agreement is for hyp to provide max_iova_width. But, we check
>> HV_DEVICE_DOMAIN_AVAILABLE in hv_iommu_detect() which sorta supersedes
>> HV_IOMMU_CAP_PRESENT. As for page size, hyp owns the iommu and can chose
>> the page size, meaning it can automatically use larger page size when
>> possible. But the hypercall HVCALL_MAP_DEVICE_GPA_PAGES will only take
>> 4k pfns as input, hence:
>>
>> #define HV_IOMMU_PGSIZES SZ_4K        /* for now, to be enhanced */
> 
> Okay, so this deserves a comment in the code to make it clear why there is no
> additional checking and what the hyper-v to guest contract for feature
> detection is.
> 
>> Ok, I can change it back to DMA_BIT_MASK. (I removed after comment
>> on V0 that this is not dma address).
> 
> An IOVA is a DMA address by definition :)
> 
> -Joerg