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

["Jan Beulich" <JBeulich@xxxxxxxx>]

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

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;

Attachment: EPT-ignore-IDENT_PT
Description: Binary data

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