Re: [PATCH] nestedhvm: do not translate INVALID_GFN

[Christoph Egger <Christoph.Egger@amd.com>]

On 08/02/12 13:19, Tim Deegan wrote:

> 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);


The caller will see the return value of pfec and not from pfec_21.
If this is what the caller expects then this is fine with me.

Christoph

>      }
>  
>      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
> 
> 



-- 
---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