WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH 4/6] mm: New XENMEM, XENMEM_add_to_physmap_gmfn_r

At 13:32 +0000 on 08 Nov (1320759148), Tim Deegan wrote:
> At 02:20 -0700 on 08 Nov (1320718835), Jan Beulich wrote:
> > >>> On 07.11.11 at 19:25, Jean Guyader <jean.guyader@xxxxxxxxxxxxx> wrote:
> > 
> > Sorry, noticed this only now, but neither title nor description of this
> > are in sync with the actual patch.
> 
> 
> Indeed; they should be updated.  But otherwise:
> Acked-by: Tim Deegan <tim@xxxxxxx>

No, wait, that was the other patch, which I already acked!
ENOCOFFEE. :(

I have a few other comments on this patch...

> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index f75011e..cca64b8 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -4714,9 +4714,29 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
>              return -EPERM;
>          }
>  
> -        xenmem_add_to_physmap(d, xatp);
> +        if ( xatp.space != XENMAPSPACE_gmfn_range )
> +            xatp.size = 1;
> +
> +        for ( ; xatp.size > 0; xatp.size-- )
> +        {
> +            if ( hypercall_preempt_check() )
> +            {
> +                rc = -EAGAIN;
> +                break;
> +            }
> +            xenmem_add_to_physmap(d, xatp);
> +            xatp.idx++;
> +            xatp.gpfn++;
> +        }

Having moved XATP into its own function, this loop probably belongs in
that function. 

While I'm looking at it, updating two loop vars explicitly and one in
the header is a bit ugly - why not just use a while() and update all
three together?


>          rcu_unlock_domain(d);
> +        if ( rc == -EAGAIN )
> +        {
> +            if ( copy_to_guest(arg, &xatp, 1) )
> +                return -EFAULT;
> +            rc = hypercall_create_continuation(
> +                    __HYPERVISOR_memory_op, "ih", op, arg);
> +        }

I think this might need some compat glue in
arch/x86/x86_64/compat/mm.c for it to work with 32-bit callers.


Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>