Re: [PATCH v3 1/5] xentrace: Common Support for get_pg_owner/put_pg_owner on ARM and x86

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 04/04/2016 19:48, Benjamin Sanda wrote:
> Moved get_pg_owner() and put_pg_owner() from the arch specific x86
> mm.c source files into the common page_alloc.c source. This was done
> as theses functions are now needed by both architectures to support
> xentrace on the ARM platform. Forward declarations were added to mm.h.
>
> One conditional compilation check was added in get_pg_owner() for the
> is_pvh_domain(curr) check which is only valid to perform on x86
> platforms.
>
> Signed-off-by: Benjamin Sanda <ben.sanda@dornerworks.com>
>
> ---
> Changed since v2:
>   * Combined patches 3-5 from v2 into one patch for v3. No code change.
> ---
>  xen/arch/x86/mm.c       | 48 ----------------------------------------------
>  xen/common/page_alloc.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++
>  xen/include/xen/mm.h    |  2 ++
>  3 files changed, 53 insertions(+), 48 deletions(-)
>
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index c997b53..0d695dd 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -2998,54 +2998,6 @@ int new_guest_cr3(unsigned long mfn)
>      return rc;
>  }
>  
> -static struct domain *get_pg_owner(domid_t domid)
> -{
> -    struct domain *pg_owner = NULL, *curr = current->domain;
> -
> -    if ( likely(domid == DOMID_SELF) )
> -    {
> -        pg_owner = rcu_lock_current_domain();
> -        goto out;
> -    }
> -
> -    if ( unlikely(domid == curr->domain_id) )
> -    {
> -        MEM_LOG("Cannot specify itself as foreign domain");
> -        goto out;
> -    }
> -
> -    if ( !is_pvh_domain(curr) && unlikely(paging_mode_translate(curr)) )
> -    {
> -        MEM_LOG("Cannot mix foreign mappings with translated domains");
> -        goto out;
> -    }
> -
> -    switch ( domid )
> -    {
> -    case DOMID_IO:
> -        pg_owner = rcu_lock_domain(dom_io);
> -        break;
> -    case DOMID_XEN:
> -        pg_owner = rcu_lock_domain(dom_xen);
> -        break;
> -    default:
> -        if ( (pg_owner = rcu_lock_domain_by_id(domid)) == NULL )
> -        {
> -            MEM_LOG("Unknown domain '%u'", domid);
> -            break;
> -        }
> -        break;
> -    }
> -
> - out:
> -    return pg_owner;
> -}
> -
> -static void put_pg_owner(struct domain *pg_owner)
> -{
> -    rcu_unlock_domain(pg_owner);
> -}
> -
>  static inline int vcpumask_to_pcpumask(
>      struct domain *d, XEN_GUEST_HANDLE_PARAM(const_void) bmap, cpumask_t *pmask)
>  {
> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
> index 98e30e5..8fe9c03 100644
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -145,6 +145,7 @@
>  #ifdef CONFIG_X86
>  #include <asm/p2m.h>
>  #include <asm/setup.h> /* for highmem_start only */
> +#include <asm/paging.h>
>  #else
>  #define p2m_pod_offline_or_broken_hit(pg) 0
>  #define p2m_pod_offline_or_broken_replace(pg) BUG_ON(pg != NULL)
> @@ -1996,6 +1997,56 @@ static __init int register_heap_trigger(void)
>  }
>  __initcall(register_heap_trigger);
>  
> +struct domain *get_pg_owner(domid_t domid)
> +{
> +    struct domain *pg_owner = NULL, *curr = current->domain;
> +
> +    if ( likely(domid == DOMID_SELF) )
> +    {
> +        pg_owner = rcu_lock_current_domain();
> +        goto out;
> +    }
> +
> +    if ( unlikely(domid == curr->domain_id) )
> +    {
> +        gdprintk(XENLOG_WARNING,"Cannot specify itself as foreign domain");
> +        goto out;
> +    }
> +
> +#ifdef CONFIG_X86
> +    if ( !is_pvh_domain(curr) && unlikely(paging_mode_translate(curr)) )
> +    {
> +        gdprintk(XENLOG_WARNING,"Cannot mix foreign mappings with translated domains");

Space after comma.

> +        goto out;
> +    }
> +#endif
> +
> +    switch ( domid )
> +    {
> +    case DOMID_IO:
> +        pg_owner = rcu_lock_domain(dom_io);
> +        break;
> +    case DOMID_XEN:
> +        pg_owner = rcu_lock_domain(dom_xen);
> +        break;
> +    default:
> +        if ( (pg_owner = rcu_lock_domain_by_id(domid)) == NULL )
> +        {
> +            gdprintk(XENLOG_WARNING,"Unknown domain '%u'", domid);

While changing this code, please use d%d for the domain id, to be
consistent with the newer style.

With these two minor issues fixed, Reviewed-by: Andrew Cooper
<andrew.cooper3@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel