On 10/11 10:15, Tim Deegan wrote:
> At 09:54 +0000 on 10 Nov (1320918887), Jan Beulich wrote:
> > >>> On 10.11.11 at 09:44, Jean Guyader <jean.guyader@xxxxxxxxxxxxx> wrote:
> >
> > In the native implementation I neither see the XENMAPSPACE_gmfn_range
> > case getting actually handled in the main switch (did you mean to change
> > xatp.space to XENMAPSPACE_gmfn in that case?), nor do I see how you
> > communicate back how many of the pages were successfully processed in
> > the event of an error in the middle of the processing or when a
> > continuation is required.
> >
> > But with the patch being pretty hard to read, maybe I'm simply
> > overlooking something?
>
> The patch changes the (compat-translated) hypercall arguments in place to
> reflect what's been done. Agreed that it's particularly hard to read,
> though.
>
Actually no, because I'm not using the pointer in xenmem_add_to_physmap.
That will be part of the next series, and that will make the patch even
harder to read :(...
Jean
> Tim.
>
> > Further (I realize I should have commented on this earlier) I think that
> > in order to allow forward progress you should not check for preemption
> > on the very first iteration of each (re-)invocation. That would also
> > guarantee no behavioral change to the original single-page variants.
> >
> > >--- a/xen/arch/x86/x86_64/compat/mm.c
> > >+++ b/xen/arch/x86/x86_64/compat/mm.c
> > >@@ -63,6 +63,16 @@ 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 )
> > >+ return rc;
> > >+
> > >+ 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;
> >
> > Other than in the XENMEM_[gs]et_pod_target you (so far, subject to the
> > above comment resulting in a behavioral change) don't have any real
> > outputs here, and hence there's no need to always to the outbound
> > translation - i.e. all of this could be moved into the if ()'s body.
> >
> > Jan
> >
> > >
> > > break;
> > > }
> >
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home