Re: [Xen-devel] [PATCH] x86/hvm: Fix altp2m_op hypercall continuations

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 4/8/19 8:39 PM, Andrew Cooper wrote:
> c/s 9383de210 "x86/altp2m: support for setting restrictions for an array of
> pages" introduced this logic, but do_hvm_op() was already capable of handling
> -ERESTART correctly.
> 
> More problematic however is a continuation from compat_altp2m_op().  The arg
> written back into register state points into the hypercall XLAT area, not at
> the original parameter passed by the guest.  It may be truncated by the
> vmentry, but definitely won't be correct on the next invocation.
> 
> Delete the hypercall_create_continuation() call, and return -ERESTART, which
> will cause the compat case to start working correctly.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
> CC: Petre Pircalabu <ppircalabu@xxxxxxxxxxxxxxx>
> ---
>  xen/arch/x86/hvm/hvm.c | 12 ++----------
>  1 file changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index e798b49..cc89ee7 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -4729,12 +4729,10 @@ static int do_altp2m_op(
>          if ( rc > 0 )
>          {
>              a.u.set_mem_access_multi.opaque = rc;
> +            rc = -ERESTART;
>              if ( __copy_field_to_guest(guest_handle_cast(arg, 
> xen_hvm_altp2m_op_t),
>                                         &a, u.set_mem_access_multi.opaque) )
>                  rc = -EFAULT;
> -            else
> -                rc = hypercall_create_continuation(__HYPERVISOR_hvm_op, "lh",
> -                                                   HVMOP_altp2m, arg);
>          }
>          break;

Right, that part was taken from the XENMEM_access_op_set_access_multi
code and plopped in. Didn't follow the call chain all the way through
and so missed the simpler way of doing this.

The changes certainly look correct. I'll run some tests tomorrow as well.


Thanks,
Razvan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel