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

[Julien Grall <julien.grall@xxxxxxxxxx>]

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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>

Regards,

--
Julien Grall

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