Re: [Xen-devel] [PATCH] altp2m: Allow the hostp2m to be shared

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

Sadly I only have little nitpicks. Feel free to ignore them.

> diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
> index a522423..d5b4b2d 100644
> --- a/xen/arch/x86/mm/mem_sharing.c
> +++ b/xen/arch/x86/mm/mem_sharing.c
> @@ -35,6 +35,7 @@
>  #include <asm/p2m.h>
>  #include <asm/atomic.h>
>  #include <asm/event.h>
> +#include <asm/altp2m.h>
>  #include <xsm/xsm.h>
>  
>  #include "mm-locks.h"
> @@ -1026,6 +1027,16 @@ int mem_sharing_share_pages(struct domain *sd, 
> unsigned long sgfn, shr_handle_t
>      /* We managed to free a domain page. */
>      atomic_dec(&nr_shared_mfns);
>      atomic_inc(&nr_saved_mfns);
> +
> +    if( altp2m_active(cd) )

Missing space.
> +    {
> +        p2m_access_t a;

Newline.
> +        struct p2m_domain *p2m = p2m_get_hostp2m(cd);
> +        p2m->get_entry(p2m, cgfn, NULL, &a, 0, NULL, NULL);
> +        p2m_altp2m_propagate_change(cd, _gfn(cgfn), smfn, PAGE_ORDER_4K,
> +                                    p2m_ram_shared, a);
> +    }
> +
>      ret = 0;
>      
>  err_out:
> diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
> index 3cb6868..1ac3018 100644
> --- a/xen/arch/x86/mm/p2m-ept.c
> +++ b/xen/arch/x86/mm/p2m-ept.c
> @@ -846,7 +846,7 @@ out:
>      if ( is_epte_present(&old_entry) )
>          ept_free_entry(p2m, &old_entry, target);
>  
> -    if ( rc == 0 && p2m_is_hostp2m(p2m) )
> +    if ( rc == 0 && p2m_is_hostp2m(p2m) && p2mt != p2m_ram_shared )
>          p2m_altp2m_propagate_change(d, _gfn(gfn), mfn, order, p2mt, p2ma);
>  
>      return rc;
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index b3fce1b..d2aebf7 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -1739,11 +1739,10 @@ int p2m_set_altp2m_mem_access(struct domain *d, 
> struct p2m_domain *hp2m,
>      /* Check host p2m if no valid entry in alternate */
>      if ( !mfn_valid(mfn) )
>      {
> -        mfn = hp2m->get_entry(hp2m, gfn_l, &t, &old_a,
> -                              P2M_ALLOC | P2M_UNSHARE, &page_order, NULL);
> +        mfn = hp2m->get_entry(hp2m, gfn_l, &t, &old_a, 0, &page_order, NULL);
>  
>          rc = -ESRCH;
> -        if ( !mfn_valid(mfn) || t != p2m_ram_rw )
> +        if ( !mfn_valid(mfn) || (t != p2m_ram_rw && t != p2m_ram_shared) )

Would it be easier to read if there were more of ()?

>              return rc;
>  
>          /* If this is a superpage, copy that first */
> @@ -1760,7 +1759,7 @@ int p2m_set_altp2m_mem_access(struct domain *d, struct 
> p2m_domain *hp2m,
>      }
>  
>      return ap2m->set_entry(ap2m, gfn_l, mfn, PAGE_ORDER_4K, t, a,
> -                         (current->domain != d));
> +                           (current->domain != d));

That looks like a cleanup? Perhaps a different patch?

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