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

["Jan Beulich" <JBeulich@suse.com>]

>>> On 24.04.14 at 04:21, <mukesh.rathor@oracle.com> wrote:
> On Thu, 17 Apr 2014 14:58:42 +0100
> "Jan Beulich" <JBeulich@suse.com> wrote:
>> Right - keeping the macro as is and introducing a derived function to
>> handle the extra requirements on leaf entries would seem quite okay,
>> so long as error propagation can be done properly.
> 
> Ok, how about something like the following?

Looks reasonable, especially if this is indeed the only place where leaf
entries get modified. I suppose there's no path allowing 2M or 1G pages
to be created with foreign type?

> In case of get_page failure,
> not sure EINVAL is the best one to return, EBUSY?

Indeed, -EBUSY seems like the more applicable one here.

> @@ -545,8 +569,14 @@ ept_set_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn,
>          ept_p2m_type_to_flags(&new_entry, p2mt, p2ma);
>      }
>  
> +    if ( unlikely(p2m_is_foreign(ept_entry->sa_p2mt)) )
> +        prev_foreign_mfn = ept_entry->mfn;
> +
>      atomic_write_ept_entry(ept_entry, new_entry);
>  
> +    if ( unlikely(prev_foreign_mfn != INVALID_MFN) )
> +        put_page(mfn_to_page(prev_foreign_mfn));
> +
>      /* Track the highest gfn for which we have ever had a valid mapping */
>      if ( p2mt != p2m_invalid &&
>           (gfn + (1UL << order) - 1 > p2m->max_mapped_pfn) )
> @@ -576,6 +606,10 @@ out:
>          }
>      }
>  
> +    /* do cleanup for foreign types in case of error */
> +    if ( unlikely(rc && ept_entry && p2m_is_foreign(p2mt)) )
> +        put_page(mfn_to_page(mfn_x(mfn)));

Comparing this with the above suggests that type safety rules
would want prev_foreign_mfn to be mfn_t, and this as well as the
code in ept_get_foreign_refcnt() suggest that you may want/
need to add *_to_* overrides similar to the ones near the top of
p2m-pt.c.

Jan