Re: [Xen-devel] [PATCH 01/18] xen: reinstate previously unused XENMEM_remove_from_physmap hypercall

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.01.12 at 00:35, Daniel De Graaf <dgdegra@xxxxxxxxxxxxx> wrote:
> --- a/xen/arch/ia64/xen/mm.c
> +++ b/xen/arch/ia64/xen/mm.c
> @@ -3448,6 +3448,40 @@ arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
>          break;
>      }
>  
> +    case XENMEM_remove_from_physmap:
> +    {
> +        struct xen_remove_from_physmap xrfp;
> +        unsigned long mfn;
> +        struct domain *d;
> +
> +        if ( copy_from_guest(&xrfp, arg, 1) )
> +            return -EFAULT;
> +
> +        rc = rcu_lock_target_domain_by_id(xrfp.domid, &d);
> +        if ( rc != 0 )
> +            return rc;
> +
> +        if ( xsm_remove_from_physmap(current->domain, d) )
> +        {
> +            rcu_unlock_domain(d);
> +            return -EPERM;
> +        }
> +
> +        domain_lock(d);
> +
> +        mfn = gmfn_to_mfn(d, xrfp.gpfn);
> +
> +        if ( mfn_valid(mfn) )
> +            guest_physmap_remove_page(d, xrfp.gpfn, mfn, 0);

Here and in the other similar paths - is it really to be considered
'success' if !mfn_valid(mfn)?

Further, given that the code looks all the same between ia64 and x86
- is this really arch-specific (the get_gfn_untyped() and put_gfn() used
in x86 should really be stubbed out for ia64 anyway)?

> +
> +        domain_unlock(d);
> +
> +        rcu_unlock_domain(d);
> +
> +        break;
> +    }
> +
> +
>      case XENMEM_machine_memory_map:
>      {
>          struct xen_memory_map memmap;
> --- a/xen/include/xlat.lst
> +++ b/xen/include/xlat.lst
> @@ -81,6 +81,7 @@
>  !    processor_power                 platform.h
>  ?    processor_px                    platform.h
>  !    psd_package                     platform.h
> +!    remove_from_physmap             memory.h

Sorry, but this is not how I meant my comment on v1 to be
interpreted - I thought that by looking at the rest of the file it would
go without saying that the sorting is by header name (alphabetically),
then by type name (again alphabetically). (I realize there are other
cases where sorting is not fully correct, but as I'm striving to adjust
this via secondary changes, I'd like to avoid getting the situation
worse with new additions.)

>  ?    xenpf_pcpuinfo                  platform.h
>  ?    xenpf_pcpu_version              platform.h
>  !    sched_poll                      sched.h

Jan


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