On 7/20/2025 7:19 PM, Michael Kelley wrote:
From: Roman Kisel <rom...@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
