Re: [Xen-devel] [PATCH v1 1/2] x86: add p2m_ram_wp

["Ye, Wei" <wei.ye@xxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Monday, August 4, 2014 3:38 PM
> To: Ye, Wei
> Cc: ian.campbell@xxxxxxxxxx; Paul.Durrant@xxxxxxxxxx;
> ian.jackson@xxxxxxxxxxxxx; stefano.stabellini@xxxxxxxxxxxxx; Tian, Kevin; Lv,
> Zhiyuan; xen-devel@xxxxxxxxxxxxx; keir@xxxxxxx; Tim Deegan
> Subject: RE: [PATCH v1 1/2] x86: add p2m_ram_wp
> 
> >>> On 04.08.14 at 07:10, <wei.ye@xxxxxxxxx> wrote:
> >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> >> >>> On 28.07.14 at 19:55, <wei.ye@xxxxxxxxx> wrote:
> >> > @@ -167,6 +169,9 @@ typedef unsigned int p2m_query_t;
> >> >   * and must not be touched. */
> >> >  #define P2M_BROKEN_TYPES (p2m_to_mask(p2m_ram_broken))
> >> >
> >> > +/* Write protection types */
> >> > +#define P2M_WP_TYPES (p2m_to_mask(p2m_ram_wp))
> >> > +
> >> >  /* Useful predicates */
> >> >  #define p2m_is_ram(_t) (p2m_to_mask(_t) & P2M_RAM_TYPES)
> #define
> >> > p2m_is_hole(_t) (p2m_to_mask(_t) & P2M_HOLE_TYPES) @@ -191,6
> +196,7
> >> > @@ typedef unsigned int p2m_query_t;
> >> >  #define p2m_is_any_ram(_t)  (p2m_to_mask(_t) &                   \
> >> >                               (P2M_RAM_TYPES | P2M_GRANT_TYPES |  \
> >> >                                p2m_to_mask(p2m_map_foreign)))
> >> > +#define p2m_is_wp_ram(_t)   (p2m_to_mask(_t) & P2M_WP_TYPES)
> >>
> >> To me such single-type classes don't seem very useful. Agreed there
> >> are a number of pre-existing ones, but for classes where future
> >> extensions are rather hard to imagine I wouldn't needlessly add
> >> classes right away. But Tim (whom you failed to Cc anyway) will have
> >> the final say here.
> >
> > How about using the existing p2m_mmio_dm, in that way, both read &
> > write access will go to device model for emulation.
> 
> Perhaps you didn't read what I wrote the way I intended it: I'm not opposed
> to the new P2M type. What I dislike is the single type class that you 
> introduce.
> To check for that in the code, you could as well use direct comparisons with
> p2m_ram_wp.

Sorry for my misunderstanding. Agree that this single class may be unnecessary. 
So I'll
remove this class type and compare it directly with p2m_ram_wp. But another 
option
maybe we can remove this new introduced p2m type, since it doesn't work well 
when page
table sharing with IOMMU case. And just using the existing p2m type 
"p2m_mmio_dm".

> 
> Jan


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