Re: [PATCH v4 0/7] hyperv: Introduce new way to manage hypercall args

[Roman Kisel <romank@linux.microsoft.com>]

On 7/20/2025 7:19 PM, Michael Kelley wrote:
> From: Roman Kisel <romank@linux.microsoft.com> Sent: Friday, July 18, 2025 6:16 PM

[...]

> 
> Thanks for any testing you can do on standalone test machines without
> needing test clusters in Azure. It will be hard to get test coverage on
> *every* hypercall call site that is modified by the patch set, but doing
> basic smoke testing of running in the root partition and in VTL2 will
> cover more than I can cover running in a VTL0 guest on my laptop or
> in Azure. Fortunately, the changes overall in this patch set are pretty
> straightforward, and my testing of VTL0 guests didn’t turn up any bugs.
> I'm hoping that additional smoke testing is more about gaining
> confidence than finding actual bugs.  (Famous last words ....)
> 

Thank you a million times for pushing the bar higher and supporting the
code :)

>> VTL2 currently uses a limited number hypercalls that are set as enabled
>> in the OpenVMM code (`set_allowed_hypercalls`). You could take a look
>> and conclude if these hypercalls require any adjustments in the patches.
> 
> My patch set already covers all the hypercall call sites that originate in
> VTL2 code. Again, a basic smoke test should help gain confidence, or
> show that any confidence is misplaced :-)
> 

Very nice, should be smooth sailing then :)

>> My opinion has been to have two pages (input and output ones). As the
>> new code introduces just one page I do feel a bit apprehensive, got no
>> hard evidence that this is a bad approach though. If we tweak the code
>> to have 2 pages, perhaps there would be no need to run a full-blown
>> validation, and even smoke tests will suffice?
> 
> My view is that the 1 page vs. 2 pages is much less of a risk than just
> some coding error in introducing the new interfaces. The 1 page vs.
> 2 pages should only affect the batch size for rep hypercalls, and the
> existing code already handles different batch sizes. So I'm not as
> concerned about that risk. Wei Liu in the maintainer here, so I'll
> certainly follow his judgment and guidance on what is needed to
> be confident in this patch set.
> 

I agree with your risk assessment. Perhaps I am playing too much of
a spec lawyer yet it states

1) Input and output area may not intersect,
2) Either can be up to 4KiB of size.

Hence, one (be that for feature development or one-off debugging) would
be within their right to implement a hypercall that accepts 4KiB of
data and returns 4KiB of data. My understanding that after this patch,
that won't work out-of-the-box, and would need some fixing in the
kernel.

Perhaps, we could have a KConfig option to let the user choose if they
need 2 pages instead of making the user figure out what needs to be
fixed in the kernel?

> Michael

-- 
Thank you,
Roman