Re: [PATCH] nestedhvm: do not translate INVALID_GFN

[Tim Deegan <tim@xen.org>]

Hi,

At 17:37 +0200 on 31 Jul (1343756240), Christoph Egger wrote:
> Do not translate INVALID_GFN as l2 guest gfn into l1 guest gfn.

Why not?  l2 gfns don't have any special meaning that we can
dictate from inside Xen.

> Pass correct pfec for translation into l1 guest gfn.

This seems like a good idea, but probably should happen for all
entries, not just INVALID_GFN ones -- we shouldn't be returning a PFEC
to the guest that comes from translations outside his control.

How about this:

diff -r fdd4b7b36959 xen/arch/x86/mm/p2m.c
--- a/xen/arch/x86/mm/p2m.c	Thu Aug 02 12:04:31 2012 +0100
+++ b/xen/arch/x86/mm/p2m.c	Thu Aug 02 12:17:48 2012 +0100
@@ -1581,6 +1581,7 @@ unsigned long paging_gva_to_gfn(struct v
         unsigned long gfn;
         struct p2m_domain *p2m;
         const struct paging_mode *mode;
+        uint32_t pfec_21 = *pfec;
         uint64_t ncr3 = nhvm_vcpu_hostcr3(v);
 
         /* translate l2 guest va into l2 guest gfn */
@@ -1590,7 +1591,7 @@ unsigned long paging_gva_to_gfn(struct v
 
         /* translate l2 guest gfn into l1 guest gfn */
         return hostmode->p2m_ga_to_gfn(v, hostp2m, ncr3,
-                                       gfn << PAGE_SHIFT, pfec, NULL);
+                                       gfn << PAGE_SHIFT, &pfec_21, NULL);
     }
 
     return hostmode->gva_to_gfn(v, hostp2m, va, pfec);

Cheers,

Tim.

> Found with Hyper-V.
> 
> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>
> CC: Tim Deegan <tim@xen.org>
> 
> -- 
> ---to satisfy European Law for business letters:
> Advanced Micro Devices GmbH
> Einsteinring 24, 85689 Dornach b. Muenchen
> Geschaeftsfuehrer: Alberto Bozzo
> Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
> Registergericht Muenchen, HRB Nr. 43632

Content-Description: xen_p2m.diff
> diff -r 8330198c3240 xen/arch/x86/mm/p2m.c
> --- a/xen/arch/x86/mm/p2m.c	Fri Jul 27 12:24:03 2012 +0200
> +++ b/xen/arch/x86/mm/p2m.c	Tue Jul 31 16:49:54 2012 +0200
> @@ -1582,12 +1582,19 @@ unsigned long paging_gva_to_gfn(struct v
>          struct p2m_domain *p2m;
>          const struct paging_mode *mode;
>          uint64_t ncr3 = nhvm_vcpu_hostcr3(v);
> +        uint32_t pfec1 = *pfec;
>  
>          /* translate l2 guest va into l2 guest gfn */
>          p2m = p2m_get_nestedp2m(v, ncr3);
>          mode = paging_get_nestedmode(v);
>          gfn = mode->gva_to_gfn(v, p2m, va, pfec);
>  
> +        /* if l1 guest maps its mmio pages into the
> +         * l2 guest then we see this case here. */
> +        if (gfn == INVALID_GFN)
> +            return INVALID_GFN;
> +        *pfec = pfec1;
> +
>          /* translate l2 guest gfn into l1 guest gfn */
>          return hostmode->p2m_ga_to_gfn(v, hostp2m, ncr3,
>                                         gfn << PAGE_SHIFT, pfec, NULL);

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