| Message ID | 20171027022524.22589-4-haozhong.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
* Haozhong Zhang <haozhong.zhang@intel.com> wrote: > By default, KVM treats a reserved page as for MMIO purpose, and maps > it to guest with UC memory type. However, some reserved pages are not > for MMIO, such as pages of DAX device (e.g., /dev/daxX.Y). Mapping > them with UC memory type will harm the performance. In order to > exclude those cases, we check the host cache mode in addition and only > treat UC/UC- pages as MMIO. > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> > Reported-by: Cuevas Escareno, Ivan D <ivan.d.cuevas.escareno@intel.com> > Reported-by: Kumar, Karthik <karthik.kumar@intel.com> > --- > arch/x86/kvm/mmu.c | 32 +++++++++++++++++++++++++++++--- > 1 file changed, 29 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c > index 0b481cc9c725..d4c821a6df3d 100644 > --- a/arch/x86/kvm/mmu.c > +++ b/arch/x86/kvm/mmu.c > @@ -2707,10 +2707,36 @@ static bool mmu_need_write_protect(struct kvm_vcpu *vcpu, gfn_t gfn, > > static bool kvm_is_mmio_pfn(kvm_pfn_t pfn) > { > - if (pfn_valid(pfn)) > - return !is_zero_pfn(pfn) && PageReserved(pfn_to_page(pfn)); > + bool is_mmio = true; > > - return true; > + if (pfn_valid(pfn)) { > + is_mmio = !is_zero_pfn(pfn) && PageReserved(pfn_to_page(pfn)); > + > + /* > + * By default, KVM treats a reserved page as for MMIO > + * purpose, and maps it to guest with UC memory type. > + * However, some reserved pages are not for MMIO, such > + * as pages of DAX device (e.g., /dev/daxX.Y). Mapping > + * them with UC memory type will harm the performance. > + * In order to exclude those cases, we check the host > + * cache mode in addition and only treat UC/UC- pages > + * as MMIO. > + * > + * track_pfn_insert() works only when PAT is enabled, > + * so add pat_enabled() here. > + */ > + if (is_mmio && pat_enabled()) { > + pgprot_t prot; > + enum page_cache_mode cm; > + > + track_pfn_insert(NULL, &prot, kvm_pfn_to_pfn(pfn)); > + cm = pgprot2cachemode(prot); > + is_mmio = (cm == _PAGE_CACHE_MODE_UC || > + cm == _PAGE_CACHE_MODE_UC_MINUS); > + } > + } > + > + return is_mmio; > } > > static int set_spte(struct kvm_vcpu *vcpu, u64 *sptep, s/harm the performance /harm performance> But I suspect the rest of the comment should be rewritten too to be more fluid. Beyond that - I think instead of exposing these low level details a properly named helper function should be put into pat.c instead - and KVM can use that. Thanks, Ingo
On 10/27/17 10:40 +0200, Ingo Molnar wrote: > > * Haozhong Zhang <haozhong.zhang@intel.com> wrote: > > > By default, KVM treats a reserved page as for MMIO purpose, and maps > > it to guest with UC memory type. However, some reserved pages are not > > for MMIO, such as pages of DAX device (e.g., /dev/daxX.Y). Mapping > > them with UC memory type will harm the performance. In order to > > exclude those cases, we check the host cache mode in addition and only > > treat UC/UC- pages as MMIO. > > > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> > > Reported-by: Cuevas Escareno, Ivan D <ivan.d.cuevas.escareno@intel.com> > > Reported-by: Kumar, Karthik <karthik.kumar@intel.com> > > --- > > arch/x86/kvm/mmu.c | 32 +++++++++++++++++++++++++++++--- > > 1 file changed, 29 insertions(+), 3 deletions(-) > > > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c > > index 0b481cc9c725..d4c821a6df3d 100644 > > --- a/arch/x86/kvm/mmu.c > > +++ b/arch/x86/kvm/mmu.c > > @@ -2707,10 +2707,36 @@ static bool mmu_need_write_protect(struct kvm_vcpu *vcpu, gfn_t gfn, > > > > static bool kvm_is_mmio_pfn(kvm_pfn_t pfn) > > { > > - if (pfn_valid(pfn)) > > - return !is_zero_pfn(pfn) && PageReserved(pfn_to_page(pfn)); > > + bool is_mmio = true; > > > > - return true; > > + if (pfn_valid(pfn)) { > > + is_mmio = !is_zero_pfn(pfn) && PageReserved(pfn_to_page(pfn)); > > + > > + /* > > + * By default, KVM treats a reserved page as for MMIO > > + * purpose, and maps it to guest with UC memory type. > > + * However, some reserved pages are not for MMIO, such > > + * as pages of DAX device (e.g., /dev/daxX.Y). Mapping > > + * them with UC memory type will harm the performance. > > + * In order to exclude those cases, we check the host > > + * cache mode in addition and only treat UC/UC- pages > > + * as MMIO. > > + * > > + * track_pfn_insert() works only when PAT is enabled, > > + * so add pat_enabled() here. > > + */ > > + if (is_mmio && pat_enabled()) { > > + pgprot_t prot; > > + enum page_cache_mode cm; > > + > > + track_pfn_insert(NULL, &prot, kvm_pfn_to_pfn(pfn)); > > + cm = pgprot2cachemode(prot); > > + is_mmio = (cm == _PAGE_CACHE_MODE_UC || > > + cm == _PAGE_CACHE_MODE_UC_MINUS); > > + } > > + } > > + > > + return is_mmio; > > } > > > > static int set_spte(struct kvm_vcpu *vcpu, u64 *sptep, > > s/harm the performance > /harm performance> > > But I suspect the rest of the comment should be rewritten too to be more fluid. I'll refactor the comment. > > Beyond that - I think instead of exposing these low level details a properly named > helper function should be put into pat.c instead - and KVM can use that. > KVM only needs the memory type information, so lookup_memtype() in pat.c is probably a better one to be exposed. Haozhong
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c index 0b481cc9c725..d4c821a6df3d 100644 --- a/arch/x86/kvm/mmu.c +++ b/arch/x86/kvm/mmu.c @@ -2707,10 +2707,36 @@ static bool mmu_need_write_protect(struct kvm_vcpu *vcpu, gfn_t gfn, static bool kvm_is_mmio_pfn(kvm_pfn_t pfn) { - if (pfn_valid(pfn)) - return !is_zero_pfn(pfn) && PageReserved(pfn_to_page(pfn)); + bool is_mmio = true; - return true; + if (pfn_valid(pfn)) { + is_mmio = !is_zero_pfn(pfn) && PageReserved(pfn_to_page(pfn)); + + /* + * By default, KVM treats a reserved page as for MMIO + * purpose, and maps it to guest with UC memory type. + * However, some reserved pages are not for MMIO, such + * as pages of DAX device (e.g., /dev/daxX.Y). Mapping + * them with UC memory type will harm the performance. + * In order to exclude those cases, we check the host + * cache mode in addition and only treat UC/UC- pages + * as MMIO. + * + * track_pfn_insert() works only when PAT is enabled, + * so add pat_enabled() here. + */ + if (is_mmio && pat_enabled()) { + pgprot_t prot; + enum page_cache_mode cm; + + track_pfn_insert(NULL, &prot, kvm_pfn_to_pfn(pfn)); + cm = pgprot2cachemode(prot); + is_mmio = (cm == _PAGE_CACHE_MODE_UC || + cm == _PAGE_CACHE_MODE_UC_MINUS); + } + } + + return is_mmio; } static int set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
By default, KVM treats a reserved page as for MMIO purpose, and maps it to guest with UC memory type. However, some reserved pages are not for MMIO, such as pages of DAX device (e.g., /dev/daxX.Y). Mapping them with UC memory type will harm the performance. In order to exclude those cases, we check the host cache mode in addition and only treat UC/UC- pages as MMIO. Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> Reported-by: Cuevas Escareno, Ivan D <ivan.d.cuevas.escareno@intel.com> Reported-by: Kumar, Karthik <karthik.kumar@intel.com> --- arch/x86/kvm/mmu.c | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-)