Re: [Xen-devel] [PATCH 09/22] xen/arm: p2m: Use a whitelist rather than blacklist in get_page_from_gfn

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 27/07/16 18:56, Stefano Stabellini wrote:
> On Wed, 27 Jul 2016, Julien Grall wrote:
> > On 26/07/16 23:44, Stefano Stabellini wrote:
> > > On Wed, 20 Jul 2016, Julien Grall wrote:
> > > > Currently, the check in get_page_from_gfn is using a blacklist. This is
> > > > very fragile because we may forgot to update the check when a new p2m
> > > > type is added.
> > > >
> > > > To avoid any possible issue, use a whitelist. All type backed by a RAM
> > > > page can could potential be valid. The check is borrowed from x86.
> > > >
> > > > Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> > > > ---
> > > >  xen/include/asm-arm/p2m.h | 9 ++++++++-
> > > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
> > > > index 3091c04..78d37ab 100644
> > > > --- a/xen/include/asm-arm/p2m.h
> > > > +++ b/xen/include/asm-arm/p2m.h
> > > > @@ -104,9 +104,16 @@ typedef enum {
> > > >  #define P2M_RAM_TYPES (p2m_to_mask(p2m_ram_rw) |        \
> > > >                         p2m_to_mask(p2m_ram_ro))
> > > >
> > > > +/* Grant mapping types, which map to a real frame in another VM */
> > > > +#define P2M_GRANT_TYPES (p2m_to_mask(p2m_grant_map_rw) |  \
> > > > +                         p2m_to_mask(p2m_grant_map_ro))
> > > > +
> > > >  /* Useful predicates */
> > > >  #define p2m_is_ram(_t) (p2m_to_mask(_t) & P2M_RAM_TYPES)
> > > >  #define p2m_is_foreign(_t) (p2m_to_mask(_t) &
> > > > p2m_to_mask(p2m_map_foreign))
> > > > +#define p2m_is_any_ram(_t) (p2m_to_mask(_t) &                   \
> > > > +                            (P2M_RAM_TYPES | P2M_GRANT_TYPES |  \
> > > > +                             p2m_to_mask(p2m_map_foreign)))
> > > >
> > > >  static inline
> > > >  void p2m_mem_access_emulate_check(struct vcpu *v,
> > > > @@ -224,7 +231,7 @@ static inline struct page_info *get_page_from_gfn(
> > > >      if (t)
> > > >          *t = p2mt;
> > > >
> > > > -    if ( p2mt == p2m_invalid || p2mt == p2m_mmio_direct )
> > > > +    if ( !p2m_is_any_ram(p2mt) )
> > > >          return NULL;
> > > What about the iommu mappings?
> > iommu mappings (p2m_iommu_map_*) are special mappings for the direct mapping
> > workaround. They should not be used in get_page_from_gfn.
> Make sense. I think they deserve to be mentioned in the commit message,
> as this patch changes behavior for them.
Good point. I will update the commit message.

Regards,

--
Julien Grall

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