On 08/04/2019 19:13, Razvan Cojocaru wrote:
> 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.coop...@citrix.com>
>> ---
>> CC: Jan Beulich <jbeul...@suse.com>
>> CC: Wei Liu <wei.l...@citrix.com>
>> CC: Roger Pau Monné <roger....@citrix.com>
>> CC: Razvan Cojocaru <rcojoc...@bitdefender.com>
>> CC: Petre Pircalabu <ppircal...@bitdefender.com>
>> ---
>> 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.
do_memory_op() is the other way around. The compat wrapper is entered
first, and calls into the native path, and any set-up continuations are
then adjusted by hypercall_xlat_continuation() on the way back out.
All of this parameter handling is chronically complicated, and in
serious need of a more simple way to be found.
> The changes certainly look correct. I'll run some tests tomorrow as well.
Thanks,
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
