Re: [PATCH] mshv: Do pre-commit before mapping a guest VM GPA

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] mshv: GFP_KERNEL allocation in hv_pre_commit_gpa() during MMU
  invalidation
--

commit 326d99247cfb889e00e7b24f8217bcbc574b15e8
Author: Mukesh R <mrathor@linux.microsoft.com>

mshv: Do pre-commit before mapping a guest VM GPA

This commit adds an HVCALL_PRECOMMIT_GPA_PAGES step before mapping guest
GPAs in hv_do_map_gpa_hcall() to pre-allocate hypervisor data structures
and reduce TLB flushes.

> diff --git a/drivers/hv/mshv_root_hv_call.c b/drivers/hv/mshv_root_hv_call.c
> index e761addda50e1..595d044fb9264 100644
> --- a/drivers/hv/mshv_root_hv_call.c
> +++ b/drivers/hv/mshv_root_hv_call.c
> @@ -181,6 +181,54 @@ int hv_call_delete_partition(u64 partition_id)
>  	return hv_result_to_errno(status);
>  }
>  
> +/*
> + * Pre commit to reduce TLB flush overheads in the hypervisor. On systems with
> + * large CPUs, there is signficant advantage to doing this. Pre-commit allows
> + * the hypervisor to build all data structs needed in anticipation of the map
> + * gpa hypercall, thus reducing or eliminating deposit page returns from the
> + * map gpa hypercall, and as a result reducing TLB flushes.
> + */
> +static int hv_pre_commit_gpa(u64 pt_id, u64 gfn, u64 num_pages)
> +{
> +	ulong irq_flags;
> +	struct hv_input_precommit_gpa_pages *input_page;
> +	int rc = 0;
> +	u64 status, done = 0;
> +
> +	while (done < num_pages) {
> +		u16 rep_count = min(num_pages - done, HV_REP_COUNT_MAX);
> +
> +		local_irq_save(irq_flags);
> +		input_page = *this_cpu_ptr(hyperv_pcpu_input_arg);
> +		memset(input_page, 0, sizeof(*input_page));
> +
> +		input_page->partition_id = pt_id;
> +		input_page->target_gpa_base = gfn;
> +
> +		status = hv_do_rep_hypercall(HVCALL_PRECOMMIT_GPA_PAGES,
> +					     rep_count, 0, input_page, NULL);
> +		local_irq_restore(irq_flags);
> +
> +		done += hv_repcomp(status);
> +		gfn += hv_repcomp(status);
> +
> +		if (hv_result_needs_memory(status)) {
> +			rc = hv_deposit_memory(pt_id, status);
> +			if (rc)
> +				break;
> +
> +		} else if (!hv_result_success(status)) {
> +			pr_err("%s: failed to pre commit for gfn %#llx done %llu/%llu, status=%#llx (%s)\n",
> +			       __func__, gfn, done, num_pages, status,
> +			       hv_result_to_string(hv_result(status)));
> +			rc = hv_result_to_errno(status);
> +			break;
> +		}
> +	}
> +
> +	return rc;
> +}
> +
>  /* 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)
> @@ -205,6 +253,10 @@ static int hv_do_map_gpa_hcall(u64 partition_id, u64 gfn, u64 page_struct_count,
>  		page_count >>= large_shift;
>  	}
>  
> +	ret = hv_pre_commit_gpa(partition_id, gfn, page_struct_count);
> +	if (ret)
> +		return ret;

[Severity: High]
Can calling hv_pre_commit_gpa() unconditionally here sleep and enter
direct reclaim from an MMU notifier invalidation callback?

mshv_region_interval_invalidate() calls mshv_region_remap_pages() with
HV_MAP_GPA_NO_ACCESS while holding region->mreg_mutex inside
mmu_notifier_invalidate_range_start(), including when
!mmu_notifier_range_blockable(range):

drivers/hv/mshv_regions.c:mshv_region_interval_invalidate() {
	...
	if (mmu_notifier_range_blockable(range))
		mutex_lock(&region->mreg_mutex);
	else if (!mutex_trylock(&region->mreg_mutex))
		goto out_fail;

	mmu_interval_set_seq(mni, cur_seq);

	ret = mshv_region_remap_pages(region, HV_MAP_GPA_NO_ACCESS,
				      page_offset, page_count);
	...
}

That reaches hv_do_map_gpa_hcall(), which now calls hv_pre_commit_gpa()
with page_struct_count (4K pages, with input_page->flags = 0 even when the
region was mapped with HV_MAP_GPA_LARGE_PAGE).  If the hypervisor asks for
more memory (for example to shatter a 2M SLAT entry into 4K entries),
hv_deposit_memory() -> hv_call_deposit_pages() allocates pages with
GFP_KERNEL.  This sleeps in non-blockable MMU notifier context, triggers a
lockdep splat on __mmu_notifier_invalidate_range_start_map in
fs_reclaim_acquire(), and can deadlock if direct reclaim re-enters
mshv_region_interval_invalidate() on region->mreg_mutex.

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