| Message ID | 1470146119-18187-1-git-send-email-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 02.08.16 at 15:55, <roger.pau@citrix.com> wrote: > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -2283,7 +2283,11 @@ int hvm_set_cr0(unsigned long value, bool_t may_defer) > if ( !nestedhvm_vmswitch_in_progress(v) && nestedhvm_vcpu_in_guestmode(v) ) > paging_update_nestedmode(v); > else > + { > paging_update_paging_modes(v); > + /* Force an update of the memory cache attributes. */ > + memory_type_changed(d); > + } Why only in the "else" case, rather than after the if/else block? > --- a/xen/arch/x86/hvm/mtrr.c > +++ b/xen/arch/x86/hvm/mtrr.c > @@ -814,10 +814,19 @@ int epte_get_entry_emt(struct domain *d, unsigned long gfn, mfn_t mfn, > if ( gmtrr_mtype == -EADDRNOTAVAIL ) > return -1; > > - gmtrr_mtype = is_hvm_domain(d) && v ? > - get_mtrr_type(&v->arch.hvm_vcpu.mtrr, > - gfn << PAGE_SHIFT, order) : > - MTRR_TYPE_WRBACK; > + if ( !v ) > + gmtrr_mtype = MTRR_TYPE_WRBACK; > + else if ( v->arch.hvm_vcpu.mtrr.enabled & 0x2 ) > + /* MTRR is enabled, use MTRR. */ > + gmtrr_mtype = get_mtrr_type(&v->arch.hvm_vcpu.mtrr, gfn << PAGE_SHIFT, > + order); > + else if ( !hvm_paging_enabled(v) ) > + /* MTRR is not enabled and paging is disabled, force UC. */ > + gmtrr_mtype = MTRR_TYPE_UNCACHABLE; > + else > + /* MTRR is not enabled and paging is enabled, use PAT. */ > + gmtrr_mtype = MTRR_TYPE_WRBACK; I'm afraid this needs to be UC, or else you provide non-architectural behavior to the guest (see the SDM's description of the enable flag being clear). Which means the original code was correct. I'm sorry for not having checked the SDM upon looking at v1 already. Jan
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index daaee1d..db4b2d6 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -2283,7 +2283,11 @@ int hvm_set_cr0(unsigned long value, bool_t may_defer) if ( !nestedhvm_vmswitch_in_progress(v) && nestedhvm_vcpu_in_guestmode(v) ) paging_update_nestedmode(v); else + { paging_update_paging_modes(v); + /* Force an update of the memory cache attributes. */ + memory_type_changed(d); + } } return X86EMUL_OKAY; diff --git a/xen/arch/x86/hvm/mtrr.c b/xen/arch/x86/hvm/mtrr.c index f7831ff..bf3ab72 100644 --- a/xen/arch/x86/hvm/mtrr.c +++ b/xen/arch/x86/hvm/mtrr.c @@ -814,10 +814,19 @@ int epte_get_entry_emt(struct domain *d, unsigned long gfn, mfn_t mfn, if ( gmtrr_mtype == -EADDRNOTAVAIL ) return -1; - gmtrr_mtype = is_hvm_domain(d) && v ? - get_mtrr_type(&v->arch.hvm_vcpu.mtrr, - gfn << PAGE_SHIFT, order) : - MTRR_TYPE_WRBACK; + if ( !v ) + gmtrr_mtype = MTRR_TYPE_WRBACK; + else if ( v->arch.hvm_vcpu.mtrr.enabled & 0x2 ) + /* MTRR is enabled, use MTRR. */ + gmtrr_mtype = get_mtrr_type(&v->arch.hvm_vcpu.mtrr, gfn << PAGE_SHIFT, + order); + else if ( !hvm_paging_enabled(v) ) + /* MTRR is not enabled and paging is disabled, force UC. */ + gmtrr_mtype = MTRR_TYPE_UNCACHABLE; + else + /* MTRR is not enabled and paging is enabled, use PAT. */ + gmtrr_mtype = MTRR_TYPE_WRBACK; + hmtrr_mtype = get_mtrr_type(&mtrr_state, mfn_x(mfn) << PAGE_SHIFT, order); if ( gmtrr_mtype < 0 || hmtrr_mtype < 0 ) return -1;
Currently the code that calculates the cache attributes of the HAP page tables assume that if MTRR are disabled the memory type is UC, this can cause issues if MTRR are never enabled because the guest only plans to use PAT. In order to solve this modify epte_get_entry_emt so that is takes into account that MTRR can be disabled and that PAT should be used instead, this also implies that when page tables are enabled inside the guest a recalculation of the HAP page table cache attributes must be performed. The special casing that was previously done for PVHv1 guests can now be removed, since PVHv1 guest cannot have MTRR enabled or paging disabled, so the memory type is always going to be MTRR_TYPE_WRBACK in that case. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> --- Changes since v1: - Fix comments. - Slightly change the if ... else logic. - Correctly check if MTRR are enabled. - Expand description. --- xen/arch/x86/hvm/hvm.c | 4 ++++ xen/arch/x86/hvm/mtrr.c | 17 +++++++++++++---- 2 files changed, 17 insertions(+), 4 deletions(-)