From: Alex Ionescu <aionescu@xxxxxxxxx> Sent: Monday, January 6, 2025 6:54 AM
>
> For another 2c worth, I had previously requested #1 (always allocate
> output page) as this would simplify some further work I was interested
> in at some point to provide VSM-like functionality to a VTL 0 Linux
> guest, which would, at that point, not make it wasteful for VTL 0 any
> longer (since some required hypercalls for VSM support need an output
> page).
>
> I realize this is "future-proofing" something that doesn't exist yet
> but it would avoid a further change down the road.
>
Alex --
I'm prototyping a new approach to managing per-cpu memory for
Hyper-V hypercall inputs and outputs. This new approach doesn’t have
explicit "input" and "output" pages, but may share a single page for
both input and output (without overlapping, as required by the TLFS).
The goal is to abstract and "regularize" the way hypercall input and
output space is set up, so that we avoid the current ad hoc approach
that is open coded for each hypercall invocation and is subject to errors.
I want to make sure my approach would handle your case. Could you
elaborate on the nature of the input and output from the VSM-related
hypercalls? Is it some fixed size structure? Or perhaps a fixed size
structure plus an array? Or something else?
Thanks,
Michael
> >
> > On Fri, Jan 3, 2025 at 5:08 PM Michael Kelley <mhklinux@xxxxxxxxxxx> wrote:
> >>
> >> From: Stanislav Kinsburskii <skinsburskii@xxxxxxxxxxxxxxxxxxx> Sent: Friday, January 3, 2025 11:20 AM
> >> >
> >> > On Mon, Dec 30, 2024 at 10:09:39AM -0800, Roman Kisel wrote:
> >> > > Due to the hypercall page not being allocated in the VTL mode,
> >> > > the code resorts to using a part of the input page.
> >> > >
> >> > > Allocate the hypercall output page in the VTL mode thus enabling
> >> > > it to use it for output and share code with dom0.
> >> > >
> >> > > Signed-off-by: Roman Kisel <romank@xxxxxxxxxxxxxxxxxxx>
> >> > > ---
> >> > > drivers/hv/hv_common.c | 6 +++---
> >> > > 1 file changed, 3 insertions(+), 3 deletions(-)
> >> > >
> >> > > diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> >> > > index c6ed3ba4bf61..c983cfd4d6c0 100644
> >> > > --- a/drivers/hv/hv_common.c
> >> > > +++ b/drivers/hv/hv_common.c
> >> > > @@ -340,7 +340,7 @@ int __init hv_common_init(void)
> >> > > BUG_ON(!hyperv_pcpu_input_arg);
> >> > >
> >> > > /* Allocate the per-CPU state for output arg for root */
> >> > > - if (hv_root_partition) {
> >> > > + if (hv_root_partition || IS_ENABLED(CONFIG_HYPERV_VTL_MODE)) {
> >> >
> >> > This check doesn't look nice.
> >> > First of all, IS_ENABLED(CONFIG_HYPERV_VTL_MODE) doesn't mean that this
> >> > particular kernel is being booted in VTL other that VTL0.
> >>
> >> Actually, it does mean that. Kernels built with CONFIG_HYPERV_VTL_MODE=y
> >> will not run as a normal guest in VTL 0. See the third paragraph of the
> >> "help" section for HYPERV_VTL_MODE in drivers/hv/Kconfig.
> >>
> >> Michael
> >>
> >> >
> >> > Second, current approach is that a VTL1+ kernel is a different build from VTL0
> >> > kernel and thus relying on the config option looks reasonable. However,
> >> > this is not true in general case.
> >> >
> >> > I'd suggest one of the following three options:
> >> >
> >> > 1. Always allocate per-cpu output pages. This is wasteful for the VTL0
> >> > guest case, but it might worth it for overall simplification.
> >> >
> >> > 2. Don't touch this code and keep the cnage in the Underhill tree.
> >> >
> >> > 3. Introduce a configuration option (command line or device tree or
> >> > both) and use it instead of the kernel config option.
> >> >
> >> > Thanks,
> >> > Stas
> >> >
> >> > > hyperv_pcpu_output_arg = alloc_percpu(void *);
> >> > > BUG_ON(!hyperv_pcpu_output_arg);
> >> > > }
> >> > > @@ -435,7 +435,7 @@ int hv_common_cpu_init(unsigned int cpu)
> >> > > void **inputarg, **outputarg;
> >> > > u64 msr_vp_index;
> >> > > gfp_t flags;
> >> > > - int pgcount = hv_root_partition ? 2 : 1;
> >> > > + const int pgcount = (hv_root_partition ||
> >> > IS_ENABLED(CONFIG_HYPERV_VTL_MODE)) ? 2 : 1;
> >> > > void *mem;
> >> > > int ret;
> >> > >
> >> > > @@ -453,7 +453,7 @@ int hv_common_cpu_init(unsigned int cpu)
> >> > > if (!mem)
> >> > > return -ENOMEM;
> >> > >
> >> > > - if (hv_root_partition) {
> >> > > + if (hv_root_partition || IS_ENABLED(CONFIG_HYPERV_VTL_MODE)) {
> >> > > outputarg = (void **)this_cpu_ptr(hyperv_pcpu_output_arg);
> >> > > *outputarg = (char *)mem + HV_HYP_PAGE_SIZE;
> >> > > }
> >> > > --
> >> > > 2.34.1
> >> > >
> >>
