Re: [PATCH] Adding support for hyper-v enlightments hv-extcalls and hv-memory-hint (hypercall HvExtCallMemoryHeatHint)

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] qemu: hv_madvise_walk_step drops overlapping memory hints
--

commit e5fd145c0d62c5737370ada9533f9341dfc23d5a
Author: Shmuel Fomberg <shmuelfomberg@gmail.com>

Adding support for hyper-v enlightments hv-extcalls and hv-memory-hint (hypercall HvExtCallMemoryHeatHint)

This commit introduces support for Hyper-V extended memory heat-hint GPA 
range helpers. It includes the API for decoding and walking memory ranges, 
along with corresponding unit and functional tests.

> diff --git a/hw/hyperv/hv-gpa-range.c b/hw/hyperv/hv-gpa-range.c
> new file mode 100644
> index 0000000000000..e6d3d3eb42f7b
> --- /dev/null
> +++ b/hw/hyperv/hv-gpa-range.c
[ ... ]
> +bool hv_madvise_walk_step(hwaddr *gpa, hwaddr *len,
> +                          hwaddr section_start, hwaddr section_size,
> +                          hwaddr *out_gpa, hwaddr *out_len)
> +{
> +    hwaddr skip;
> +
> +    /*
> +     * A hole with no memory region makes the section start past the
> +     * cursor; a zero-size section means there is nothing left to find.
> +     * Either way, stop -- do not advise or advance past the gap.
> +     */
> +    if (!section_size || section_start < *gpa) {
> +        return false;
> +    }
> +
> +    skip = section_start - *gpa;
> +    if (skip >= *len || section_size > *len - skip) {
> +        return false;
> +    }

[Severity: Medium]
Does this incorrectly drop overlapping memory hints?

If memory_region_find() returns a section that extends beyond the originally
requested range, section_size > *len - skip evaluates to true and the walk
is aborted entirely by returning false. 

Should this instead clamp the section_size to the remaining length (*len - skip)
so that the valid overlapping portion is processed rather than discarded?

[ ... ]
> diff --git a/tests/unit/test-hv-gpa-range.c b/tests/unit/test-hv-gpa-range.c
> new file mode 100644
> index 0000000000000..70196f271b886
> --- /dev/null
> +++ b/tests/unit/test-hv-gpa-range.c
[ ... ]
> +static void test_walk_section_past_requested_range(void)
> +{
> +    /* Section is larger than what's left to process -> stop, don't act */
> +    hwaddr gpa = 0x1000, len = 0x1000;
> +    hwaddr out_gpa = 0xdead, out_len = 0xdead;
> +
> +    g_assert_false(hv_madvise_walk_step(&gpa, &len, 0x1000, 0x2000,
> +                                        &out_gpa, &out_len));

[Severity: Medium]
This unit test appears to strictly enforce the behavior of failing when a
section extends past the requested range. 

If hv_madvise_walk_step() is updated to clamp the returned size instead of
failing, will this test also need to be adjusted to expect the successful
clamped values?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014123954.95181-1-shmuelfomberg@gmail.com?part=1