[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] Re: [PATCH 5/7] mm: New XENMEM space, XENMAPSPACE_gmfn_range
On 10/11 12:47, Jan Beulich wrote: > >>> On 10.11.11 at 12:35, Jean Guyader <jean.guyader@xxxxxxxxxxxxx> wrote: > >@@ -4716,6 +4748,17 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE(void) > >arg) > > } > > > > rc = xenmem_add_to_physmap(d, &xatp); > >+ if ( rc == -EAGAIN ) > > if ( rc ) > > >+ { > >+ if ( copy_to_guest(arg, &xatp, 1) ) > >+ { > >+ rcu_unlock_domain(d); > >+ return -EFAULT; > >+ } > > } > if ( rc == -EAGAIN ) > > (with some room for further simplification). Without that (or the minimal > alternative of copying back just .size or yet some other mechanism), as > pointed out before, the caller won't have a way to know how far into > the batch things succeeded. > > >+ > >+ rc = hypercall_create_continuation( > >+ __HYPERVISOR_memory_op, "ih", op, arg); > >+ } > > > > rcu_unlock_domain(d); > > Also, the whole block above can be move past this rcu_unlock_domain(), > eliminating the need to do it separately in the above error path(s). > > > > >--- a/xen/arch/x86/x86_64/compat/mm.c > >+++ b/xen/arch/x86/x86_64/compat/mm.c > >@@ -63,6 +63,18 @@ int compat_arch_memory_op(int op, XEN_GUEST_HANDLE(void) > >arg) > > > > XLAT_add_to_physmap(nat, &cmp); > > rc = arch_memory_op(op, guest_handle_from_ptr(nat, void)); > >+ if ( rc < 0 ) > >+ break; > >+ > > With the way the code below is currently this is superfluous. But just > as above you will need to provide some indication to the caller > *where* the failure occurred. > > >+ if ( rc == __HYPERVISOR_memory_op ) > >+ { > >+ hypercall_xlat_continuation(NULL, 0x2, nat, arg); > >+ > >+ XLAT_add_to_physmap(&cmp, nat); > >+ > >+ if ( copy_to_guest(arg, &cmp, 1) ) > >+ return -EFAULT; > > I realize that this is the same way in the code handling > XENMEM_[gs]et_pod_target, but unfortunately that's wrong (that's > why I'm copying you, George): Once a continuation was set up, you > mustn't change the return value anymore, since the continuation was > established by adjusting the guest's rIP. > > As for other memory ops, the continuation can be encoded in "op" (see > xen/common/memory.c and xen/common/compat/memory.c). However, > while suitable here I don't think that's usable for the PoD variant. The > alternative is to cancel the continuation (would require quite a bit of > new code I think) or to adjust the low level hypercall handler code (at > least the compat mode one) to special case rAX values in the negative > errno range, leaving rAX unchanged instead of returning -ENOSYS. > Keir? > Jan, did you have something like the attached patch in mind? We will return EFAULT to the hypercall without touching the guest memory because the copy_to_guest failed. For the example I ignored the multicalls. Jean Attachment:
hypercall-cancel-continuation.patch _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |