Re: [PATCH v2 6/8] xen: arm: add some helpers for assessing p2m pte

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

Hi Ian,

On 11/06/14 17:40, Ian Campbell wrote:
> Not terribly helpful right now, since they aren't widely used, but makes future
> patches easier to read.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
> v2: clarify common on p2m_{table,entry}
> ---
>   xen/arch/arm/p2m.c |   21 ++++++++++++++-------
>   1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 8a6d295..2a93ff9 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -14,6 +14,13 @@
>   #define P2M_FIRST_ORDER 1
>   #define P2M_FIRST_ENTRIES (LPAE_ENTRIES<<P2M_FIRST_ORDER)
>
> +#define p2m_valid(pte) ((pte).p2m.valid)
> +/* These two can only be used on L0..L2 ptes because L3 mappings set
> + * the table bit and therefore these would return the opposite to what
> + * you would expect. */
> +#define p2m_table(pte) (p2m_valid(pte) && (pte).p2m.table)
> +#define p2m_entry(pte) (p2m_valid(pte) && !(pte).p2m.table)

Sorry, I didn't spot it on the previous version. You are using twice pte 
here. Depending on how complex pte we may duplicate the operation 
(masking the address + dereference the table). I'm wondering if we need 
a temporary variable in both p2m_table and p2m_entry.

It seems that in your patch #7, you always use these 2 macros with 
non-complex variable. So I'm fine with one or the other way:

Acked-by: Julien Grall <julien.grall@linaro.org>

Regards,

-- 
Julien Grall