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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Fri, 7 Apr 2023 22:28:42 +0000
From:   "Michael Kelley (LINUX)" <mikelley@...rosoft.com>
To:     Nuno Das Neves <nunodasneves@...ux.microsoft.com>,
        Olaf Hering <olaf@...fle.de>
CC:     "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        KY Srinivasan <kys@...rosoft.com>,
        "wei.liu@...nel.org" <wei.liu@...nel.org>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Dexuan Cui <decui@...rosoft.com>
Subject: RE: [PATCH] Drivers: hv: Use nested hypercall for post message and
 signal event

From: Nuno Das Neves <nunodasneves@...ux.microsoft.com> 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@...ux.microsoft.com>:
> >
> >> 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