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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2014-06-11 at 22:39 +0100, Julien Grall wrote:
> 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.

A static function would be preferable to that I think.

> It seems that in your patch #7, you always use these 2 macros with 
> non-complex variable.

Yes, this was deliberate.

>  So I'm fine with one or the other way:
> 
> Acked-by: Julien Grall <julien.grall@xxxxxxxxxx>
> 
> Regards,
> 



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