Re: [PATCH RFC 7/9] xen: arm: handle variable p2m levels in apply_p2m_changes

[Julien Grall <julien.grall@linaro.org>]

Hi Ian,

On 07/30/2014 02:47 PM, Ian Campbell wrote:
> As with prervious changes this involves conversion from a linear series of

previous

[..]

> @@ -726,9 +699,27 @@ static int apply_p2m_changes(struct domain *d,
>  
>      spin_lock(&p2m->lock);
>  
> +    if ( P2M_ROOT_PAGES == 1 )
> +    {
> +        /* Static mapping. P2M_ROOT_PAGES > 1 are handled below */
> +        mappings[P2M_ROOT_LEVEL] = __map_domain_page(p2m->root);
> +        {
> +            rc = -EINVAL;
> +            goto out;
> +        }
> +    }
> +
>      addr = start_gpaddr;
>      while ( addr < end_gpaddr )
>      {
> +        int root_table;
> +        const unsigned int offsets[4] = {
> +            zeroeth_table_offset(addr),
> +            first_table_offset(addr),
> +            second_table_offset(addr),
> +            third_table_offset(addr)
> +        };
> +
>          /*
>           * Arbitrarily, preempt every 512 operations or 8192 nops.
>           * 512*P2M_ONE_PROGRESS == 8192*P2M_ONE_PROGRESS_NOP == 0x2000
> @@ -748,73 +739,71 @@ static int apply_p2m_changes(struct domain *d,
>              count = 0;
>          }
>  
> -        if ( cur_first_page != p2m_first_level_index(addr) )
> +        if ( P2M_ROOT_PAGES > 1 )
>          {
> -            if ( first ) unmap_domain_page(first);
> -            first = p2m_map_first(p2m, addr);
> -            if ( !first )
> +            int i;
> +            /*
> +             * Concatenated root-level tables. The table number will be the
> +             * offset at the previous level. It is not possible to concetenate

concatenate

> +             * a level-0 root.
> +             */
> +            BUG_ON(P2M_ROOT_LEVEL == 0);

My remark on #6 can be applied here. You are checking this every time
you loop rather that it's only set once at Xen boot.

If a developer decides to change those value after boot we will be in
trouble in other places.

Regards,

-- 
Julien Grall