Re: [Xen-devel] [PATCH] Don't track all memory when enabling log dirty to track vram

["Xu, Dongxiao" <dongxiao.xu@xxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Tuesday, February 18, 2014 11:29 PM
> To: Xu, Dongxiao; Zhang, Xiantao; Zhang, Yang Z
> Cc: andrew.cooper3@xxxxxxxxxx; George Dunlap; Dugger, Donald D;
> xen-devel@xxxxxxxxxxxxx; Tim Deegan
> Subject: Re: [Xen-devel] [PATCH] Don't track all memory when enabling log 
> dirty
> to track vram
> 
> >>> On 18.02.14 at 12:46, "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
> > Nothing at all, as it turns out. The regression is due to Dongxiao's
> >
> > http://lists.xenproject.org/archives/html/xen-devel/2013-12/msg00367.html
> >
> > which I have in my tree as part of various things pending for 4.5.
> > And which at the first, second, and third glance looks pretty
> > innocent (IOW I still have to find out _why_ it is wrong).
> 
> And here's a fixed version of the patch - we simply can't drop
> the bogus HVM_PARAM_IDENT_PT check entirely yet.
> 
> In the course of fixing this I also found two other shortcomings:
> - EPT EMT field should be updated upon guest MTRR writes (the
>   lack thereof is the reason for needing to retain the bogus check)
> - epte_get_entry_emt() either needs "order" passed, or its callers
>   must call it more than once for big/huge pages
> 

The below patch looks okay to me. Thanks!

Thanks,
Dongxiao

> Jan
> 
> x86/hvm: refine the judgment on IDENT_PT for EMT
> 
> When trying to get the EPT EMT type, the judgment on
> HVM_PARAM_IDENT_PT is not correct which always returns WB type if
> the parameter is not set. Remove the related code.
> 
> Signed-off-by: Dongxiao Xu <dongxiao.xu@xxxxxxxxx>
> 
> We can't fully drop the dependency yet, but we should certainly avoid
> overriding cases already properly handled. The reason for this is that
> the guest setting up its MTRRs happens _after_ the EPT tables got
> already constructed, and no code is in place to propagate this to the
> EPT code. Without this check we're forcing the guest to run with all of
> its memory uncachable until something happens to re-write every single
> EPT entry. But of course this has to be just a temporary solution.
> 
> In the same spirit we should defer the "very early" (when the guest is
> still being constructed and has no vCPU yet) override to the last
> possible point.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- a/xen/arch/x86/hvm/mtrr.c
> +++ b/xen/arch/x86/hvm/mtrr.c
> @@ -689,13 +689,8 @@ uint8_t epte_get_entry_emt(struct domain
> 
>      *ipat = 0;
> 
> -    if ( (current->domain != d) &&
> -         ((d->vcpu == NULL) || ((v = d->vcpu[0]) == NULL)) )
> -        return MTRR_TYPE_WRBACK;
> -
> -    if ( !is_pvh_vcpu(v) &&
> -         !v->domain->arch.hvm_domain.params[HVM_PARAM_IDENT_PT] )
> -        return MTRR_TYPE_WRBACK;
> +    if ( v->domain != d )
> +        v = d->vcpu ? d->vcpu[0] : NULL;
> 
>      if ( !mfn_valid(mfn_x(mfn)) )
>          return MTRR_TYPE_UNCACHABLE;
> @@ -718,7 +713,8 @@ uint8_t epte_get_entry_emt(struct domain
>          return MTRR_TYPE_WRBACK;
>      }
> 
> -    gmtrr_mtype = is_hvm_vcpu(v) ?
> +    gmtrr_mtype = is_hvm_domain(d) && v &&
> +                  d->arch.hvm_domain.params[HVM_PARAM_IDENT_PT] ?
>                    get_mtrr_type(&v->arch.hvm_vcpu.mtrr, (gfn <<
> PAGE_SHIFT)) :
>                    MTRR_TYPE_WRBACK;
> 
> 
> 
> 


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