On 4/15/25 11:07, mhkelley58@xxxxxxxxx wrote:
From: Michael Kelley <mhklinux@xxxxxxxxxxx>
Current code allocates the "hyperv_pcpu_input_arg", and in
some configurations, the "hyperv_pcpu_output_arg". Each is a 4 KiB
page of memory allocated per-vCPU. A hypercall call site disables
interrupts, then uses this memory to set up the input parameters for
the hypercall, read the output results after hypercall execution, and
re-enable interrupts. The open coding of these steps leads to
inconsistencies, and in some cases, violation of the generic
requirements for the hypercall input and output as described in the
Hyper-V Top Level Functional Spec (TLFS)[1].
<snip>
The new functions are realized as a single inline function that
handles the most complex case, which is a hypercall with input
and output, both of which contain arrays. Simpler cases are mapped to
this most complex case with #define wrappers that provide zero or NULL
for some arguments. Several of the arguments to this new function
must be compile-time constants generated by "sizeof()"
expressions. As such, most of the code in the new function can be
evaluated by the compiler, with the result that the code paths are
no longer than with the current open coding. The one exception is
new code generated to zero the fixed-size portion of the input area
in cases where it is not currently done.
IMHO, this is unnecessary change that just obfuscates code. With status quo
one has the advantage of seeing what exactly is going on, one can use the
args any which way, change batch size any which way, and is thus flexible.
With time these functions only get more complicated and error prone. The
saving of ram is very minimal, this makes analyzing crash dumps harder,
and in some cases like in your patch 3/7 disables unnecessarily in error case:
- if (count > HV_MAX_MODIFY_GPA_REP_COUNT) {
- pr_err("Hyper-V: GPA count:%d exceeds supported:%lu\n", count,
- HV_MAX_MODIFY_GPA_REP_COUNT);
+ local_irq_save(flags); <<<<<<<
...
So, this is a nak from me. sorry.
<snip>
+/*
+ * Allocate one page that is shared between input and output args, which is
+ * sufficient for all current hypercalls. If a future hypercall requires
That is incorrect. We've iommu map hypercalls that will use up entire page
for input. More coming as we implement ram withdrawl from the hypervisor.
Thanks,
-Mukesh
