Thank you for your contribution! Sashiko AI review found 3 potential issue(s) 
to consider:
- [Critical] hv_iommu_fill_iova_list() incorrectly uses guest PAGE_SHIFT 
instead of hypervisor HV_HYP_PAGE_SHIFT, causing incomplete and incorrect IOTLB 
flushes on architectures with non-4KB pages.
- [High] Unconditional use of the new HVCALL_FLUSH_DEVICE_DOMAIN_LIST hypercall 
without a capability check or fallback.
- [Medium] Bitfield assignment on uninitialized flexible array member causes 
KMSAN use-of-uninitialized-value warnings.
--

commit 5fd02fd07f9fbc4f1734f312b8a33dfd79a38961
Author: Yu Zhang <[email protected]>

iommu/hyperv: Add page-selective IOTLB flush support

This patch adds page-selective IOTLB flush support using the
HVCALL_FLUSH_DEVICE_DOMAIN_LIST hypercall. It decomposes IOVA ranges into
aligned power-of-two regions to fit in a single hypercall page, falling
back to a full domain flush if the range exceeds page capacity.

> diff --git a/drivers/iommu/hyperv/iommu.c b/drivers/iommu/hyperv/iommu.c
> index e5fc625314b55..3bca362b78150 100644
> --- a/drivers/iommu/hyperv/iommu.c
> +++ b/drivers/iommu/hyperv/iommu.c

[ ... ]

> +static inline u16 hv_iommu_fill_iova_list(union hv_iommu_flush_va *iova_list,
> +                                       unsigned long start,
> +                                       unsigned long end)
> +{
> +     unsigned long start_pfn = start >> PAGE_SHIFT;
> +     unsigned long end_pfn = PAGE_ALIGN(end) >> PAGE_SHIFT;

Should this use HV_HYP_PAGE_SHIFT instead of PAGE_SHIFT?

If the kernel is compiled with a larger page size (like 64KB on ARM64),
start >> PAGE_SHIFT produces a 64KB guest PFN.

Since Hyper-V hypercalls expect 4KB PFNs (HV_HYP_PAGE_SHIFT = 12), this could
result in flushing the wrong pages and leaving stale DMA mappings active.

> +     unsigned long nr_pages = end_pfn - start_pfn;
> +     u16 count = 0;
> +
> +     while (nr_pages > 0) {
> +             unsigned long flush_pages;
> +             int order;
> +             unsigned long pfn_align;
> +             unsigned long size_align;
> +
> +             if (count >= HV_IOMMU_MAX_FLUSH_VA_COUNT) {
> +                     count = HV_IOMMU_FLUSH_VA_OVERFLOW;
> +                     break;
> +             }
> +
> +             if (start_pfn)
> +                     pfn_align = __ffs(start_pfn);
> +             else
> +                     pfn_align = BITS_PER_LONG - 1;
> +
> +             size_align = __fls(nr_pages);
> +             order = min(pfn_align, size_align);
> +             iova_list[count].page_mask_shift = order;
> +             iova_list[count].page_number = start_pfn;

Could this assignment trigger a KMSAN use-of-uninitialized-value warning?

The input buffer is from a per-CPU page and memset in
hv_flush_device_domain_list() only zeroes the fixed-size header, leaving the
flexible array uninitialized.

Assigning to bitfields forces a read-modify-write on the uninitialized 64-bit
union hv_iommu_flush_va.

> +
> +             flush_pages = 1UL << order;
> +             start_pfn += flush_pages;
> +             nr_pages -= flush_pages;
> +             count++;
> +     }
> +
> +     return count;
> +}
> +
> +static void hv_flush_device_domain_list(struct hv_iommu_domain *hv_domain,
> +                                     struct iommu_iotlb_gather *iotlb_gather)
> +{
> +     u64 status;
> +     u16 count;
> +     unsigned long flags;
> +     struct hv_input_flush_device_domain_list *input;
> +
> +     local_irq_save(flags);
> +
> +     input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> +     memset(input, 0, sizeof(*input));
> +
> +     input->device_domain = hv_domain->device_domain;
> +     input->flags |= HV_FLUSH_DEVICE_DOMAIN_LIST_IOMMU_FORMAT;
> +     count = hv_iommu_fill_iova_list(input->iova_list,
> +                                     iotlb_gather->start,
> +                                     iotlb_gather->end);
> +     if (count == HV_IOMMU_FLUSH_VA_OVERFLOW) {
> +             /*
> +              * Range exceeds hypercall page capacity. Fall back to a full
> +              * domain flush.
> +              */
> +             struct hv_input_flush_device_domain *flush_all = (void *)input;
> +
> +             memset(flush_all, 0, sizeof(*flush_all));
> +             flush_all->device_domain = hv_domain->device_domain;
> +             status = hv_do_hypercall(HVCALL_FLUSH_DEVICE_DOMAIN,
> +                                     flush_all, NULL);
> +     } else {
> +             status = hv_do_rep_hypercall(
> +                             HVCALL_FLUSH_DEVICE_DOMAIN_LIST,
> +                             count, 0, input, NULL);
> +     }
> +
> +     local_irq_restore(flags);
> +
> +     if (!hv_result_success(status))
> +             pr_err("HVCALL_FLUSH_DEVICE_DOMAIN_LIST failed, status %lld\n", 
> status);

Does this code need a capability check or fallback for older hosts?

If the hypervisor doesn't support the new HVCALL_FLUSH_DEVICE_DOMAIN_LIST
hypercall, hv_do_rep_hypercall() might return an error status.

The code prints an error but doesn't fall back to HVCALL_FLUSH_DEVICE_DOMAIN,
potentially leaving the IOTLB unflushed.

> +}

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=4

Reply via email to