[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.11 at 16:44, Jean Guyader <jean.guyader@xxxxxxxxxxxxx> wrote: > 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? Indeed, and it's fortunately much simpler than I would have thought. > 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. Adding the support for that would seem to be strait forward too. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |