On 24/05/2019 08:38, Jan Beulich wrote: >>>> On 24.05.19 at 07:41, <jgr...@suse.com> wrote: >> On 22/05/2019 12:10, Jan Beulich wrote: >>>>>> On 22.05.19 at 11:45, <jgr...@suse.com> 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.
Would you be okay with replacing the removed stuff above with: /* * If the required guest memory is paged out this function may sleep. * So in theory we should bail out if called in atomic context. * Unfortunately this is true for PVH dom0 doing domctl calls which * holds the domctl lock when accessing dom0 memory. OTOH dom0 memory * should never be paged out, so we are fine without testing for * atomic context. */ Juergen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
