Re: [PATCH 1/5] x86/HVM: wire up multicalls

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 18/06/2021 11:23, Jan Beulich wrote:
> To be able to use them from, in particular, the tool stack, they need to
> be supported for all guest types. Note that xc_resource_op() already
> does, so would not work without this on PVH Dom0.

I'm not a fan of multicalls as a concept - they're mostly a layering
violation adding substantial complexity - and frankly, working around a
Linux kernel/user ABI error is a terrible reason to make this change.

But I won't object if it happens to be the least terrible option going. 
I accept that there are no good options here.

> @@ -334,6 +336,39 @@ int hvm_hypercall(struct cpu_user_regs *
>      return curr->hcall_preempted ? HVM_HCALL_preempted : HVM_HCALL_completed;
>  }
>  
> +enum mc_disposition hvm_do_multicall_call(struct mc_state *state)
> +{
> +    struct vcpu *curr = current;
> +    hypercall_fn_t *func = NULL;
> +
> +    if ( hvm_guest_x86_mode(curr) == 8 )
> +    {
> +        struct multicall_entry *call = &state->call;
> +
> +        if ( call->op < ARRAY_SIZE(hvm_hypercall_table) )
> +            func = array_access_nospec(hvm_hypercall_table, call->op).native;
> +        if ( func )
> +            call->result = func(call->args[0], call->args[1], call->args[2],
> +                                call->args[3], call->args[4], call->args[5]);
> +        else
> +            call->result = -ENOSYS;
> +    }
> +    else
> +    {
> +        struct compat_multicall_entry *call = &state->compat_call;
> +
> +        if ( call->op < ARRAY_SIZE(hvm_hypercall_table) )
> +            func = array_access_nospec(hvm_hypercall_table, call->op).compat;
> +        if ( func )
> +            call->result = func(call->args[0], call->args[1], call->args[2],
> +                                call->args[3], call->args[4], call->args[5]);
> +        else
> +            call->result = -ENOSYS;
> +    }
> +
> +    return !hvm_get_cpl(curr) ? mc_continue : mc_preempt;

This is ported across from XSA-213, but even for PV guests, it was just
defence in depth IIRC for any cases we hadn't spotted, changing privilege.

There is no pagetable accounting in HVM guests to become confused by a
privilege change, and hvm_get_cpl() isn't totally free.  Any kernel
which puts VCPUOP_initialise in a multicall gets to keep all resulting
pieces.

I think this wants to be just "return mc_continue;"

If so, Begrudingly acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

~Andrew