Re: [Xen-devel] [V12 PATCH 3/4] pvh dom0: Add and remove foreign pages

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.05.14 at 02:50, <mukesh.rathor@xxxxxxxxxx> wrote:
> +static int atomic_write_ept_entry(ept_entry_t *entryptr, ept_entry_t new,
> +                                  int level)
> +{
> +    unsigned long oldmfn = INVALID_MFN;
> +    bool_t skip_foreign = (new.mfn == entryptr->mfn &&
> +                           new.sa_p2mt == entryptr->sa_p2mt);

This still seems too weak to me: Shouldn't you also consider whether
the old and new entries respectively are present (also further down)?

> +
> +    if ( level )
> +    {
> +        ASSERT(!new.sp || !p2m_is_foreign(new.sa_p2mt));

Please use is_epte_superpage() here.

> @@ -292,7 +332,7 @@ static bool_t ept_invalidate_emt(mfn_t mfn, bool_t recalc)
>          e.emt = MTRR_NUM_TYPES;
>          if ( recalc )
>              e.recalc = 1;
> -        atomic_write_ept_entry(&epte[i], e);
> +        atomic_write_ept_entry(&epte[i], e, level);

I'm afraid you mustn't ever ignore this function failing (i.e. unless
you're in places where you know the non-leaf shortcut is always
going to be taken, but even there I think you'd be better off
documenting this via ASSERT()), for security reasons. And yes, I
realize that this isn't going to be trivial in some cases, especially if
you want to do better than domain_crash().

And a more general question: How is the insertion of p2m_foreign
entries working together with the controlled domain (i.e. the one
owning the page) being subject to paging/sharing? I only recall
fixme-s having got added for the two features presently not being
supported for PVH domains...

Jan


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