Re: [Xen-devel] [PATCH 1/3] xen: drop in_atomic()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.05.19 at 07:41, <jgross@xxxxxxxx> wrote:
> On 22/05/2019 12:10, Jan Beulich wrote:
>>>>> On 22.05.19 at 11:45, <jgross@xxxxxxxx> wrote:
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -3185,22 +3185,6 @@ static enum hvm_translation_result __hvm_copy(
>>>  
>>>      ASSERT(is_hvm_vcpu(v));
>>>  
>>> -    /*
>>> -     * XXX Disable for 4.1.0: PV-on-HVM drivers will do grant-table ops
>>> -     * such as query_size. Grant-table code currently does 
>>> copy_to/from_guest
>>> -     * accesses under the big per-domain lock, which this test would 
>>> disallow.
>>> -     * The test is not needed until we implement sleeping-on-waitqueue when
>>> -     * we access a paged-out frame, and that's post 4.1.0 now.
>>> -     */
>>> -#if 0
>>> -    /*
>>> -     * If the required guest memory is paged out, this function may sleep.
>>> -     * Hence we bail immediately if called from atomic context.
>>> -     */
>>> -    if ( in_atomic() )
>>> -        return HVMTRANS_unhandleable;
>>> -#endif
>> 
>> Dealing with this TODO item is of course much appreciated, but
>> should it really be deleted altogether? The big-domain-lock issue
>> is gone afair, in which case dropping the #if 0 would seem
>> possible to me, even if it's not strictly needed without the sleep-
>> on-waitqueue behavior mentioned.
> 
> I just had a look and found the following path:
> 
> do_domctl() (takes domctl_lock and hypercall_deadlock_mutex)
>   arch_do_domctl()
>     raw_copy_from_guest()
>       copy_from_user_hvm()
>         hvm_copy_from_guest_linear()
>           __hvm_copy()
> 
> So no, we can't do the in_atomic() test IMO.

Oh, right - that's a PVH constraint that could probably not even
be thought of that the time the comment was written. I'm still
of the opinion though that at least the still applicable part of
the comment should be kept in place. Whether this means also
keeping in_atomic() itself is then an independent question, i.e.
I wouldn't consider it overly bad if there was no implementation
in the tree, but the above still served as documentation of what
would need to be re-added. Still my preference would be for it
to be kept.

Jan



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