Message-ID: <c5d4d351-a7ff-4762-8bb3-61554d4f9731@linux.microsoft.com>
Date: Fri, 18 Jul 2025 09:33:13 -0700
From: Easwar Hariharan <eahariha@...ux.microsoft.com>
To: mhklinux@...look.com
Cc: kys@...rosoft.com, haiyangz@...rosoft.com, wei.liu@...nel.org,
 decui@...rosoft.com, tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
 dave.hansen@...ux.intel.com, hpa@...or.com, lpieralisi@...nel.org,
 kw@...ux.com, mani@...nel.org, robh@...nel.org, bhelgaas@...gle.com,
 arnd@...db.de, eahariha@...ux.microsoft.com, x86@...nel.org,
 linux-hyperv@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-pci@...r.kernel.org, linux-arch@...r.kernel.org
Subject: Re: [PATCH v4 0/7] hyperv: Introduce new way to manage hypercall args

On 7/17/2025 9:55 PM, mhkelley58@...il.com wrote:
> From: Michael Kelley <mhklinux@...look.com>
> 
> This patch set introduces a new way to manage the use of the per-vCPU
> memory that is usually the input and output arguments to Hyper-V
> hypercalls. 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 has led 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]. This patch set introduces
> a new family of inline functions to replace the open coding. The new
> functions encapsulate key aspects of the use of per-vCPU memory for
> hypercall input and output, and ensure that the TLFS requirements are
> met (max size of 1 page each for input and output, no overlap of input
> and output, aligned to 8 bytes, etc.).
> 
> With this change, hypercall call sites no longer directly access
> "hyperv_pcpu_input_arg" and "hyperv_pcpu_output_arg". Instead, one of
> a family of new functions provides the per-vCPU memory that a hypercall
> call site uses to set up hypercall input and output areas.
> Conceptually, there is no longer a difference between the "per-vCPU
> input page" and "per-vCPU output page". Only a single per-vCPU page is
> allocated, and it is used to provide both hypercall input and output.
> All current hypercalls can fit their input and output within that single
> page, though the new code allows easy changing to two pages should a
> future hypercall require a full page for each of the input and output.
> 
> The new functions always zero the fixed-size portion of the hypercall
> input area (but not any array portion -- see below) so that
> uninitialized memory isn't inadvertently passed to the hypercall.
> Current open-coded hypercall call sites are inconsistent on this point,
> and use of the new functions addresses that inconsistency. The output
> area is not zero'ed by the new code as it is Hyper-V's responsibility
> to provide legal output.
> 
> When the input or output (or both) contain an array, the new code
> calculates and returns how many array entries fit within the per-vCPU
> memory page, which is effectively the "batch size" for the hypercall
> processing multiple entries. This batch size can then be used in the
> hypercall control word to specify the repetition count. This
> calculation of the batch size replaces current open coding of the
> batch size, which is prone to errors. Note that the array portion of
> the input area is *not* zero'ed. The arrays are almost always 64-bit
> GPAs or something similar, and zero'ing that much memory seems
> wasteful at runtime when it will all be overwritten. The hypercall
> call site is responsible for ensuring that no part of the array is
> left uninitialized (just as with current code).
> 
> The new family of functions is 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 is evaluated by the
> compiler, with the result that the runtime code paths are no longer
> than with the current open coding. An exception is the new code
> generated to zero the fixed-size portion of the input area in cases
> where it was not previously done.
> 
> Use of the new function typically (but not always) saves a few lines
> of code at each hypercall call site. This is traded off against the
> lines of code added for the new functions. With code currently
> upstream, the net is an add of about 20 lines of code and comments.
> 
> A couple hypercall call sites have requirements that are not 100%
> handled by the new function. These still require some manual open-
> coded adjustment or open-coded batch size calculations -- see the
> individual patches in this series. Suggestions on how to do better
> are welcome.
> 
> The patches in the series do the following:
> 
> Patch 1: Introduce the new family of functions for assigning hypercall
>          input and output arguments.
> 
> Patch 2 to 6: Change existing hypercall call sites to use one of the new
>          functions. In some cases, tweaks to the hypercall argument data
>          structures are necessary, but these tweaks are making the data
>          structures more consistent with the overall pattern. These
>          5 patches are independent of each other, and can go in any
>          order. The breakup into 5 patches is for ease of review.
> 
> Patch 7: Update the name of the variable used to hold the per-vCPU memory
>          used for hypercall arguments. Remove code for managing the
>          per-vCPU output page.
> 
> The new code compiles and runs successfully on x86 and arm64. However,
> basic smoke tests cover only a limited number of hypercall call sites
> that have been modified. I don't have the hardware or Hyper-V
> configurations needed to test running in the Hyper-V root partition
> or running in a VTL other than VTL 0. The related hypercall call sites
> still need to be tested to make sure I didn't break anything. Hopefully
> someone with the necessary configurations and Hyper-V versions can
> help with that testing.
> 
> For gcc 9.4.0, I've looked at the generated code for a couple of
> hypercall call sites on both x86 and arm64 to ensure that it boils
> down to the equivalent of the current open coding. I have not looked
> at the generated code for later gcc versions or for Clang/LLVM, but
> there's no reason to expect something worse as the code isn't doing
> anything tricky.
> 
> This patch set is built against linux-next20250716.
> 
> [1] https://learn.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/tlfs
> 
> Michael Kelley (7):
>   Drivers: hv: Introduce hv_setup_*() functions for hypercall arguments
>   x86/hyperv: Use hv_setup_*() to set up hypercall arguments -- part 1
>   x86/hyperv: Use hv_setup_*() to set up hypercall arguments -- part 2
>   Drivers: hv: Use hv_setup_*() to set up hypercall arguments
>   PCI: hv: Use hv_setup_*() to set up hypercall arguments
>   Drivers: hv: Use hv_setup_*() to set up hypercall arguments for mshv
>     code
>   Drivers: hv: Replace hyperv_pcpu_input/output_arg with hyperv_pcpu_arg
> 
>  arch/x86/hyperv/hv_apic.c           |  10 +--
>  arch/x86/hyperv/hv_init.c           |  12 ++-
>  arch/x86/hyperv/hv_vtl.c            |   3 +-
>  arch/x86/hyperv/irqdomain.c         |  17 ++--
>  arch/x86/hyperv/ivm.c               |  18 ++---
>  arch/x86/hyperv/mmu.c               |  19 ++---
>  arch/x86/hyperv/nested.c            |  14 ++--
>  drivers/hv/hv.c                     |   6 +-
>  drivers/hv/hv_balloon.c             |   8 +-
>  drivers/hv/hv_common.c              |  64 +++++----------
>  drivers/hv/hv_proc.c                |  23 +++---
>  drivers/hv/hyperv_vmbus.h           |   2 +-
>  drivers/hv/mshv_common.c            |  31 +++----
>  drivers/hv/mshv_root_hv_call.c      | 121 +++++++++++-----------------
>  drivers/hv/mshv_root_main.c         |   5 +-
>  drivers/pci/controller/pci-hyperv.c |  18 ++---
>  include/asm-generic/mshyperv.h      | 106 +++++++++++++++++++++++-
>  include/hyperv/hvgdk_mini.h         |   4 +-
>  18 files changed, 251 insertions(+), 230 deletions(-)
> 

Thank you for spinning this version!

For the series,

Reviewed-by: Easwar Hariharan <eahariha@...ux.microsoft.com>

Powered by blists - more mailing lists