RE: [PATCH] Drivers: hv: Use nested hypercall for post message and signal event

From: Michael Kelley (LINUX)
Date: Fri Apr 07 2023 - 18:28:51 EST


From: Nuno Das Neves <nunodasneves@xxxxxxxxxxxxxxxxxxx> Sent: Tuesday, April 4, 2023 9:57 AM
>
> On 4/3/2023 11:45 PM, Olaf Hering wrote:
> > Mon, 3 Apr 2023 16:22:58 -0700 Nuno Das Neves
> <nunodasneves@xxxxxxxxxxxxxxxxxxx>:
> >
> >> Only relevant for x86; nested functionality is not available in ARM64.
> >
> >> +#if defined(CONFIG_X86_64)
> >> + else if (hv_nested)
> >
> > Should there be a hv_nested in the ARM64 code path?
> > Looks like c4bdf94f97c86 provided such thing, so the Kconfig conditional could be
> removed.
> >
> > Olaf
>
> This will not compile on ARM64 without the guard, because hv_do_nested_hypercall
> and hv_do_fast_nested_hypercall8 are not defined.
> These are inline functions only defined in the x86 mshyperv.h header.
>
> The alternative to these guards would be defining dummy inline functions for the
> nested versions of hv_do_hypercall in the ARM64 mshyperv.h.
> I could take that approach if it is preferable.

Having to do "if (hv_nested)" and "#ifdef CONFIG_X86_64" at multiple call
sites is indeed rather messy. I wonder if it is feasible to fold all this logic into
the x86 version of hv_do_hypercall() and friends, so that the call sites are not
affected? That's what was done with hv_get/set_register(). And it would
allow the ARM64 side to be unchanged.

Here's an approach:
1) Create a global bitmap with one bit for each hypercall code that Hyper-V
accepts. The max is something less than 512, so this bitmap is less than 64
bytes. Initialize the bitmap to all zeros.
2) During early initialization, if hv_nested is set to "true", set the bit in
the bitmap corresponding to hypercalls that need the HV_HYPERCALL_NESTED
flag added.
3) In hv_do_hypercall(), use the hypercall code to index into the bitmap and
retrieve the bit. Use that bit to decide whether to set HV_HYPERCALL_NESTED.
Note that hv_nested doesn't even need to be tested because the bitmap will
be all zeros when hv_nested is "false".

The one snag is extended hypercalls, for which the hypercall code is a much
bigger value, and we might not want to make the bitmap that big. Maybe
they have to be special-cased to be non-nested.

This approach assumes that a hypercall is always either nested or non-nested
from all call sites.

If the bitmap approach is too obscure, testing hv_nested and doing a switch
statement to handle the cases that need HV_HYPERCALL_NESTED would
also work. The performance probably wouldn't be as good, but it probably
doesn't matter. Again, this would be like hv_get/set_register().

With this approach, hv_do_nested_hypercall() and friends are not needed,
but I don't know what other usage patterns for hv_do_nested_hypercall()
might be planned. These other usage patterns might make this idea not so
workable.

Michael