Re: [Xen-devel] [PATCH v13 05/11] x86/mm: add HYPERVISOR_memory_op to acquire guest resources

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.11.17 at 10:36, <Paul.Durrant@xxxxxxxxxx> wrote:
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> Sent: 23 November 2017 16:42
>> >>> On 30.10.17 at 18:48, <paul.durrant@xxxxxxxxxx> wrote:
>> > +    if ( !paging_mode_translate(currd) )
>> > +    {
>> > +        if ( copy_to_guest(xmar.frame_list, mfn_list, xmar.nr_frames) )
>> > +            rc = -EFAULT;
>> > +    }
>> > +    else
>> > +    {
>> > +        xen_pfn_t gfn_list[ARRAY_SIZE(mfn_list)];
>> > +        unsigned int i;
>> > +
>> > +        rc = -EFAULT;
>> > +        if ( copy_from_guest(gfn_list, xmar.frame_list, xmar.nr_frames) )
>> > +            goto out;
>> 
>> This will result in requests with nr_frames being zero to fail with
>> -EFAULT afaict. Let's please have such no-op requests succeed.
> 
> Oh, I was labouring under the assumption that a copy_from_guest() with a 
> zero count would succeed... I should have checked.

I don't think it would fail. The problem is that you won't enter ...

>> > +        for ( i = 0; i < xmar.nr_frames; i++ )
>> > +        {

... the body of this loop then, and hence rc will remain at -EFAULT.

>> > +            rc = set_foreign_p2m_entry(currd, gfn_list[i],
>> > +                                       _mfn(mfn_list[i]));
>> > +            if ( rc )
>> > +            {
>> > +                /*
>> > +                 * Make sure rc is -EIO for any iteration other than
>> > +                 * the first.
>> > +                 */
>> > +                rc = (i != 0) ? -EIO : rc;
>> 
>> Along the lines of what I've said above, "!=0" could be dropped
>> here, too.
> 
> The reason for the != 0 here is make the failure explicitly EIO for the case 
> where at least one iteration has successfully set_foreign_p2m_entry() but a 
> subsequent one has failed, which matches what is stated in the header. 
> (Specifically it means that, on ARM, the -EOPNOTSUPP from 
> set_foreign_p2m_entry() will get back to the caller).

I'm not asking for the condition to be removed, just for it to be
simplified to

                rc = i ? -EIO : rc;

Jan


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