Re: [Xen-devel] [V9 PATCH 6/8] pvh dom0: Add and remove foreign pages

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.04.14 at 02:19, <mukesh.rathor@xxxxxxxxxx> wrote:
> On Wed, 16 Apr 2014 18:37:42 -0700
> Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> wrote:
> 
>> On Wed, 16 Apr 2014 17:00:35 +0100
>> "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
>> 
> ............
> 
>> 
>> > > +        goto out;
>> > > +
>> > > +    rc = xsm_map_gmfn_foreign(XSM_TARGET, tdom, fdom);
>> > > +    if ( rc )
>> > > +        goto out;
>> > > +
>> > > +    /* following will take a refcnt on the mfn */
>> > > +    page = get_page_from_gfn(fdom, fgfn, &p2mt, P2M_ALLOC);
>> > > +    if ( !page || !p2m_is_valid(p2mt) )
>> > 
>> > Is this really p2m_is_valid() (i.e. including various types apart
>> > from p2m_ram_rw)?
>> 
>> Hmm.. let me check again in case of HVM guest coming up on PVH dom0,
>> if there are use cases of non-ram types. Otherwise, it could be just
>> p2m_is_ram.
> 
> Ok, following is the change I'm going to make in the next version
> for above comment:
> 
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index 67aa5f6..8d3eb95 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -1799,7 +1799,7 @@ int p2m_add_foreign(struct domain *tdom, unsigned long 
> fgf
>  
>      /* following will take a refcnt on the mfn */
>      page = get_page_from_gfn(fdom, fgfn, &p2mt, P2M_ALLOC);
> -    if ( !page || !p2m_is_valid(p2mt) )
> +    if ( !page || !p2m_is_strict_ram(p2mt) )
>      {
>          if ( page )
>              put_page(page);
> diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
> index 47604da..e0bf6dc 100644
> --- a/xen/include/asm-x86/p2m.h
> +++ b/xen/include/asm-x86/p2m.h
> @@ -165,6 +165,7 @@ typedef unsigned int p2m_query_t;
>  
>  /* Useful predicates */
> +#define p2m_is_strict_ram(_t) (p2m_to_mask(_t) & p2m_to_mask(p2m_ram_rw))

Not sure - this now precludes mapping log-dirty or r/o pages, i.e. I
think you went from too relaxed to too strict. I think the question
needs answering in a more abstract fashion first, i.e. without code
written immediately: Which pages absolutely need to be foreign-
mappable and which ones can safely get foreign-mapped. My first
thought would be P2M_RAM_TYPES & ~P2M_HOLE_TYPES, with
P2M_HOLE_TYPES contained in P2M_RAM_TYPES generating a kind
of -EAGAIN notification while triggering the re-creation of the page
(which already is an indication that instead of P2M_HOLE_TYPES it
might well need to be P2M_PAGING_TYPES).

Jan


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