| Message ID | 1436988356-4565-1-git-send-email-guangrong.xiao@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 2015-07-16 at 03:25 +0800, Xiao Guangrong wrote: > From: Xiao Guangrong <guangrong.xiao@intel.com> > > Currently code uses default memory type if MTRR is fully disabled, > fix it by using UC instead > > Signed-off-by: Xiao Guangrong <guangrong.xiao@intel.com> > --- Seems to work for me. I don't see a 0th patch, but for the series: Tested-by: Alex Williamson <alex.williamson@redhat.com> Thanks! > arch/x86/kvm/mtrr.c | 21 ++++++++++++++++++++- > 1 file changed, 20 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/mtrr.c b/arch/x86/kvm/mtrr.c > index de1d2d8..e275013 100644 > --- a/arch/x86/kvm/mtrr.c > +++ b/arch/x86/kvm/mtrr.c > @@ -120,6 +120,16 @@ static u8 mtrr_default_type(struct kvm_mtrr *mtrr_state) > return mtrr_state->deftype & IA32_MTRR_DEF_TYPE_TYPE_MASK; > } > > +static u8 mtrr_disabled_type(void) > +{ > + /* > + * Intel SDM 11.11.2.2: all MTRRs are disabled when > + * IA32_MTRR_DEF_TYPE.E bit is cleared, and the UC > + * memory type is applied to all of physical memory. > + */ > + return MTRR_TYPE_UNCACHABLE; > +} > + > /* > * Three terms are used in the following code: > * - segment, it indicates the address segments covered by fixed MTRRs. > @@ -434,6 +444,8 @@ struct mtrr_iter { > > /* output fields. */ > int mem_type; > + /* mtrr is completely disabled? */ > + bool mtrr_disabled; > /* [start, end) is not fully covered in MTRRs? */ > bool partial_map; > > @@ -549,7 +561,7 @@ static void mtrr_lookup_var_next(struct mtrr_iter *iter) > static void mtrr_lookup_start(struct mtrr_iter *iter) > { > if (!mtrr_is_enabled(iter->mtrr_state)) { > - iter->partial_map = true; > + iter->mtrr_disabled = true; > return; > } > > @@ -563,6 +575,7 @@ static void mtrr_lookup_init(struct mtrr_iter *iter, > iter->mtrr_state = mtrr_state; > iter->start = start; > iter->end = end; > + iter->mtrr_disabled = false; > iter->partial_map = false; > iter->fixed = false; > iter->range = NULL; > @@ -656,6 +669,9 @@ u8 kvm_mtrr_get_guest_memory_type(struct kvm_vcpu *vcpu, gfn_t gfn) > return MTRR_TYPE_WRBACK; > } > > + if (iter.mtrr_disabled) > + return mtrr_disabled_type(); > + > /* It is not covered by MTRRs. */ > if (iter.partial_map) { > /* > @@ -689,6 +705,9 @@ bool kvm_mtrr_check_gfn_range_consistency(struct kvm_vcpu *vcpu, gfn_t gfn, > return false; > } > > + if (iter.mtrr_disabled) > + return true; > + > if (!iter.partial_map) > return true; > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 16/07/2015 06:10, Alex Williamson wrote: > On Thu, 2015-07-16 at 03:25 +0800, Xiao Guangrong wrote: >> > From: Xiao Guangrong <guangrong.xiao@intel.com> >> > >> > Currently code uses default memory type if MTRR is fully disabled, >> > fix it by using UC instead >> > >> > Signed-off-by: Xiao Guangrong <guangrong.xiao@intel.com> >> > --- > Seems to work for me. I don't see a 0th patch, but for the series: > > Tested-by: Alex Williamson <alex.williamson@redhat.com> In fact this is the same quirk already implemented for SVM as KVM_QUIRK_CD_NW_CLEARED, so we can reuse the bit. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/23/2015 02:21 PM, Paolo Bonzini wrote: > > > On 16/07/2015 06:10, Alex Williamson wrote: >> On Thu, 2015-07-16 at 03:25 +0800, Xiao Guangrong wrote: >>>> From: Xiao Guangrong <guangrong.xiao@intel.com> >>>> >>>> Currently code uses default memory type if MTRR is fully disabled, >>>> fix it by using UC instead >>>> >>>> Signed-off-by: Xiao Guangrong <guangrong.xiao@intel.com> >>>> --- >> Seems to work for me. I don't see a 0th patch, but for the series: >> >> Tested-by: Alex Williamson <alex.williamson@redhat.com> > > In fact this is the same quirk already implemented for SVM as > KVM_QUIRK_CD_NW_CLEARED, so we can reuse the bit. Sounds good to me. I will drop the new bit and reuse as your suggestion. And i think we need document this whole staff in API.txt ... -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 23/07/2015 08:29, Xiao Guangrong wrote: >> In fact this is the same quirk already implemented for SVM as >> KVM_QUIRK_CD_NW_CLEARED, so we can reuse the bit. > > Sounds good to me. I will drop the new bit and reuse as your suggestion. > And i think we need document this whole staff in API.txt ... No problem, I've already pushed it to kvm/queue. I'm running tests now and will send a pull request to Linus tomorrow if all goes well. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/kvm/mtrr.c b/arch/x86/kvm/mtrr.c index de1d2d8..e275013 100644 --- a/arch/x86/kvm/mtrr.c +++ b/arch/x86/kvm/mtrr.c @@ -120,6 +120,16 @@ static u8 mtrr_default_type(struct kvm_mtrr *mtrr_state) return mtrr_state->deftype & IA32_MTRR_DEF_TYPE_TYPE_MASK; } +static u8 mtrr_disabled_type(void) +{ + /* + * Intel SDM 11.11.2.2: all MTRRs are disabled when + * IA32_MTRR_DEF_TYPE.E bit is cleared, and the UC + * memory type is applied to all of physical memory. + */ + return MTRR_TYPE_UNCACHABLE; +} + /* * Three terms are used in the following code: * - segment, it indicates the address segments covered by fixed MTRRs. @@ -434,6 +444,8 @@ struct mtrr_iter { /* output fields. */ int mem_type; + /* mtrr is completely disabled? */ + bool mtrr_disabled; /* [start, end) is not fully covered in MTRRs? */ bool partial_map; @@ -549,7 +561,7 @@ static void mtrr_lookup_var_next(struct mtrr_iter *iter) static void mtrr_lookup_start(struct mtrr_iter *iter) { if (!mtrr_is_enabled(iter->mtrr_state)) { - iter->partial_map = true; + iter->mtrr_disabled = true; return; } @@ -563,6 +575,7 @@ static void mtrr_lookup_init(struct mtrr_iter *iter, iter->mtrr_state = mtrr_state; iter->start = start; iter->end = end; + iter->mtrr_disabled = false; iter->partial_map = false; iter->fixed = false; iter->range = NULL; @@ -656,6 +669,9 @@ u8 kvm_mtrr_get_guest_memory_type(struct kvm_vcpu *vcpu, gfn_t gfn) return MTRR_TYPE_WRBACK; } + if (iter.mtrr_disabled) + return mtrr_disabled_type(); + /* It is not covered by MTRRs. */ if (iter.partial_map) { /* @@ -689,6 +705,9 @@ bool kvm_mtrr_check_gfn_range_consistency(struct kvm_vcpu *vcpu, gfn_t gfn, return false; } + if (iter.mtrr_disabled) + return true; + if (!iter.partial_map) return true;