Re: [Xen-devel] [PATCH] xen/physmap: Propagate error rc from xenmem_add_to_physmap_one

[Jiandi An <anjiandi@xxxxxxxxxxxxxx>]

On 12/07/16 05:43, Jan Beulich wrote:
>>>> On 07.12.16 at 02:07, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 07/12/2016 01:00, Jiandi An wrote:
>>> --- a/xen/common/memory.c
>>> +++ b/xen/common/memory.c
>>> @@ -762,6 +762,8 @@ static int xenmem_add_to_physmap_batch(struct domain *d,
>>>          rc = xenmem_add_to_physmap_one(d, xatpb->space,
>>>                                         xatpb->u,
>>>                                         idx, _gfn(gpfn));
>>> +        if ( rc < 0 )
>>> +            goto out;
>>>  
>>>          if ( unlikely(__copy_to_guest_offset(xatpb->errs, 0, &rc, 1)) )
>>>          {
>>
>> This can't be correct.  You now skip writing rc into the errs[] array on
>> a failure, which means that userspace will get an overall failure but an
>> errs[] array which said that nothing went wrong.
>>
>> This code addition looks like it wants to be an "else if" on the end of
>> this if() in context.
> 
> Why would this go elsewhere? It's unneeded - it's a property of the
> hypercall that when seeing overall success you still need to look at
> errs[].
> 
> Jan
> 

I realized the issue is on the guest side errs[] is not being checked
after the heypercall.  Will fix on the guest side.

-- 
Jiandi An
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux 
Foundation Collaborative Project.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel