Re: [Xen-devel] [for-4.8][PATCH v2 18/23] xen/arm: p2m: Re-implement relinquish_p2m_mapping using p2m_{get, set}_entry

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 15 Sep 2016, Julien Grall wrote:
> The function relinquish_p2m_mapping can be re-implemented using
> p2m_{get,set}_entry by iterating over the range mapped and using the
> mapping order given by the callee.
> 
> Given that the preemption was chosen arbitrarily, it is now done on every
> 512 iterations. Meaning that Xen may check more often if the function is
> preempted when there are no mappings.
> 
> Finally drop the operation RELINQUISH in apply_* as nobody is using it
> anymore. Note that the functions could have been dropped in one go at
> the end, however I find easier to drop the operations one by one
> avoiding a big deletion in the patch that remove the last operation.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
>     Changes in v2:
>         - Erase entry one by one as I have not time so far to check
>         whether it is possible to avoid removing entry in the p2m.
>         - Use gfn_next_boundary
> ---
>  xen/arch/arm/p2m.c | 77 
> +++++++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 59 insertions(+), 18 deletions(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 5f7aef0..ce09c4c 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -754,7 +754,6 @@ static int p2m_mem_access_radix_set(struct p2m_domain 
> *p2m, gfn_t gfn,
>  enum p2m_operation {
>      INSERT,
>      REMOVE,
> -    RELINQUISH,
>      MEMACCESS,
>  };
>  
> @@ -1318,7 +1317,6 @@ static int apply_one_level(struct domain *d,
>  
>          break;
>  
> -    case RELINQUISH:
>      case REMOVE:
>          if ( !p2m_valid(orig_pte) )
>          {
> @@ -1502,17 +1500,6 @@ static int apply_p2m_changes(struct domain *d,
>          {
>              switch ( op )
>              {
> -            case RELINQUISH:
> -                /*
> -                 * Arbitrarily, preempt every 512 operations or 8192 nops.
> -                 * 512*P2M_ONE_PROGRESS == 8192*P2M_ONE_PROGRESS_NOP == 
> 0x2000
> -                 * This is set in preempt_count_limit.
> -                 *
> -                 */
> -                p2m->lowest_mapped_gfn = _gfn(addr >> PAGE_SHIFT);
> -                rc = -ERESTART;
> -                goto out;
> -
>              case MEMACCESS:
>              {
>                  /*
> @@ -1919,16 +1906,70 @@ int p2m_init(struct domain *d)
>      return rc;
>  }
>  
> +/*
> + * The function will go through the p2m and remove page reference when it
> + * is required. The mapping will be removed from the p2m.
> + *
> + * XXX: See whether the mapping can be left intact in the p2m.
> + */
>  int relinquish_p2m_mapping(struct domain *d)
>  {
>      struct p2m_domain *p2m = &d->arch.p2m;
> -    unsigned long nr;
> +    unsigned long count = 0;
> +    p2m_type_t t;
> +    int rc = 0;
> +    unsigned int order;
> +
> +    /* Convenience alias */
> +    gfn_t start = p2m->lowest_mapped_gfn;
> +    gfn_t end = p2m->max_mapped_gfn;
> +
> +    p2m_write_lock(p2m);
> +
> +    for ( ; gfn_x(start) < gfn_x(end);
> +          start = gfn_next_boundary(start, order) )
> +    {
> +        mfn_t mfn = p2m_get_entry(p2m, start, &t, NULL, &order);
> +
> +        count++;
> +        /*
> +         * Arbitrarily preempt every 512 iterations.
> +         */
> +        if ( !(count % 512) && hypercall_preempt_check() )
> +        {
> +            rc = -ERESTART;
> +            break;
> +        }
>  
> -    nr = gfn_x(p2m->max_mapped_gfn) - gfn_x(p2m->lowest_mapped_gfn);
> +        /*
> +         * p2m_set_entry will take care of removing reference on page
> +         * when it is necessary and removing the mapping in the p2m.
> +         */
> +        if ( !mfn_eq(mfn, INVALID_MFN) )
> +        {
> +            /*
> +             * For valid mapping, the start will always be aligned as
> +             * entry will be removed whilst relinquishing.
> +             */
> +            rc = __p2m_set_entry(p2m, start, order, INVALID_MFN,
> +                                 p2m_invalid, p2m_access_rwx);
> +            if ( unlikely(rc) )
> +            {
> +                printk(XENLOG_G_ERR "Unable to remove mapping 
> gfn=%#"PRI_gfn" order=%u from the p2m of domain %d\n", gfn_x(start), order, 
> d->domain_id);
> +                break;
> +            }
> +        }
> +    }
>  
> -    return apply_p2m_changes(d, RELINQUISH, p2m->lowest_mapped_gfn, nr,
> -                             INVALID_MFN, 0, p2m_invalid,
> -                             d->arch.p2m.default_access);
> +    /*
> +     * Update lowest_mapped_gfn so on the next call we still start where
> +     * we stopped.
> +     */
> +    p2m->lowest_mapped_gfn = start;
> +
> +    p2m_write_unlock(p2m);
> +
> +    return rc;
>  }
>  
>  int p2m_cache_flush(struct domain *d, gfn_t start, unsigned long nr)
> -- 
> 1.9.1
> 

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