From mboxrd@z Thu Jan  1 00:00:00 1970
From: Julien Grall <julien.grall@linaro.org>
Subject: Re: [PATCH RFC 7/9] xen: arm: handle variable p2m
	levels in apply_p2m_changes
Date: Thu, 31 Jul 2014 16:38:56 +0100
Message-ID: <53DA6310.1080300@linaro.org>
References: <3ef2b68c511f3e31de409b76757b95c78b99d750.1406728037.git.ian.campbell@citrix.com>
	<de76f5314d72cfb7e8aa5827c3b8fe24c6193806.1406728037.git.ian.campbell@citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <de76f5314d72cfb7e8aa5827c3b8fe24c6193806.1406728037.git.ian.campbell@citrix.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Ian Campbell <ian.campbell@citrix.com>, xen-devel@lists.xen.org
Cc: tim@xen.org, vijay.kilari@gmail.com, stefano.stabellini@eu.citrix.com
List-Id: xen-devel@lists.xenproject.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