KVM: MMU: fix release no-slot pfn and clean up mmu code
v4 cover KVM: MMU: fix release no-slot pfn and clean up mmu code
Changlog:
- filter error pfn out in the common function - kvm_release_pfn instead of doing it in mmu code
- remove the patches which clean up for release noslot pfn path
Release pfn in the mmu code is little special for we allow no-slot pfn go to spte walk on page fault path, that means, on page fault fail path, we can not directly call kvm_release_pfn_clean.
This patchset fixes the bug which adds a branch to filter error pfn out in kvm_release_pfn_clean and clean up current mmu code.
PATCH v4 1/5 KVM: MMU: fix release noslot pfn
We can not directly call kvm_release_pfn_clean to release the pfn since we can meet noslot pfn which is used to cache mmio info into spte
--- arch/x86/kvm/mmu.c | 3 +-- virt/kvm/kvm_main.c | 4 +--- 2 files changed, 2 insertions(+), 5 deletions(-)
arch/x86/kvm/mmu.c +1 -2
@@ -2497,8 +2497,7 @@ static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *sptep, } } -if (!is_error_pfn (pfn)) -kvm_release_pfn_clean (pfn); +kvm_release_pfn_clean (pfn); }static void nonpaging_new_cr3 (struct kvm_vcpu *vcpu)
virt/kvm/kvm_main.c +1 -3
@@ -1322,9 +1322,7 @@ EXPORT_SYMBOL_GPL(kvm_release_page_clean);void kvm_release_pfn_clean (pfn_t pfn) { -WARN_ON (is_error_pfn (pfn)); - -if (!kvm_is_mmio_pfn (pfn)) +if (!is_error_pfn (pfn) && !kvm_is_mmio_pfn (pfn))put_page (pfn_to_page (pfn)); }EXPORT_SYMBOL_GPL (kvm_release_pfn_clean);
Signature · 1 line
1.7.7.6
Re: [PATCH v4 1/5] KVM: MMU: fix release noslot pfn
Quoted 41 lines · Xiao Guangrong
We can not directly call
kvm_release_pfn_cleanto release the pfn since we can meet noslot pfn which is used to cache mmio info into spteSigned-off-by Xiao Guangrong <[email protected]>--- arch/x86/kvm/mmu.c | 3 +-- virt/kvm/kvm_main.c | 4 +--- 2 files changed, 2 insertions(+), 5 deletions(-)arch/x86/kvm/mmu.c +1
-2@@ -2497,8 +2497,7 @@ static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *sptep, } } -if (!is_error_pfn (pfn)) -kvm_release_pfn_clean (pfn); +kvm_release_pfn_clean (pfn); }static void nonpaging_new_cr3 (struct kvm_vcpu *vcpu)virt/kvm/kvm_main.c +1
-3@@ -1322,9 +1322,7 @@ EXPORT_SYMBOL_GPL(kvm_release_page_clean);void kvm_release_pfn_clean (pfn_t pfn) { -WARN_ON (is_error_pfn (pfn)); - -if (!kvm_is_mmio_pfn (pfn)) +if (!is_error_pfn (pfn) && !kvm_is_mmio_pfn (pfn))put_page (pfn_to_page (pfn)); }EXPORT_SYMBOL_GPL (kvm_release_pfn_clean);Signature · 1 line
1.7.7.6
Why does is_error_pfn() return true for mmio spte? Its not an "error", after all.
Please kill is_invalid_pfn and use
-> is_error_pfn for checking for errors (mmio spte is not an error pfn, its a special pfn)
-> add explicit is_noslot_pfn checks where necessary in the code (say to avoid interpreting a noslot_pfn's pfn "address" bits).
(should have noticed this earlier, sorry).
Re: [PATCH v4 1/5] KVM: MMU: fix release noslot pfn
Quoted 6 lines · Marcelo Tosatti
Why does
is_error_pfn()return true for mmio spte? Its not an "error", after all.Please kill
is_invalid_pfnand use
Quoted 7 lines
->
is_error_pfnfor checking for errors (mmio spte is not an error pfn, its a special pfn)-> add explicit
is_noslot_pfnchecks where necessary in the code (say to avoid interpreting a noslot_pfn's pfn "address" bits).(should have noticed this earlier, sorry).
Never mind, your comments are always appreciated! ;)
Marcelo, is it good to you?
(will post it after your check and full test)
arch/powerpc/kvm/book3s_32_mmu_host.c +1 -1
@@ -155,7 +155,7 @@ int kvmppc_mmu_map_page(struct kvm_vcpu *vcpu, struct kvmppc_pte *orig_pte)/* Get host physical address for gpa */ hpaddr =kvmppc_gfn_to_pfn (vcpu, orig_pte->raddr >>PAGE_SHIFT ); -if (is_error_pfn (hpaddr)) { +if (is_error_noslot_pfn (hpaddr)) {printk (KERN_INFO "Couldn't get guest page for gfn %lx!\n" , orig_pte->eaddr); r = -EINVAL ;
arch/powerpc/kvm/book3s_64_mmu_host.c +1 -1
@@ -92,7 +92,7 @@ int kvmppc_mmu_map_page(struct kvm_vcpu *vcpu, struct kvmppc_pte *orig_pte)/* Get host physical address for gpa */ hpaddr =kvmppc_gfn_to_pfn (vcpu, orig_pte->raddr >>PAGE_SHIFT ); -if (is_error_pfn (hpaddr)) { +if (is_error_noslot_pfn (hpaddr)) {printk (KERN_INFO "Couldn't get guest page for gfn %lx!\n" , orig_pte->eaddr); r = -EINVAL ; goto out;
arch/powerpc/kvm/e500_tlb.c +1 -1
@@ -521,7 +521,7 @@ static inline void kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,if (likely (!pfnmap)) {unsigned long tsize_pages =1 << (tsize +10 -PAGE_SHIFT ); pfn =gfn_to_pfn_memslot (slot, gfn); -if (is_error_pfn (pfn)) { +if (is_error_noslot_pfn (pfn)) {printk (KERN_ERR "Couldn't get real page for gfn %lx!\n" , (long )gfn);return ;
arch/x86/kvm/mmu.c +2 -2
@@ -2699,7 +2699,7 @@ static void transparent_hugepage_adjust(struct kvm_vcpu *vcpu,* PT_PAGE_TABLE_LEVEL and there would be no adjustment done * here. */ -if (!is_error_pfn (pfn) && !kvm_is_mmio_pfn (pfn) && +if (!is_error_noslot_pfn (pfn) && !kvm_is_mmio_pfn (pfn) && level ==PT_PAGE_TABLE_LEVEL &&PageTransCompound (pfn_to_page (pfn)) && !has_wrprotected_page (vcpu->kvm, gfn,PT_DIRECTORY_LEVEL )) {
@@ -2733,7 +2733,7 @@ static bool handle_abnormal_pfn(struct kvm_vcpu *vcpu, gva_t gva, gfn_t gfn,bool ret = true;/* The pfn is invalid, report the error! */ -if (unlikely (is_invalid_pfn (pfn))) { +if (unlikely (is_error_pfn (pfn))) { *ret_val =kvm_handle_bad_page (vcpu, gfn, pfn); goto exit; }
arch/x86/kvm/mmu_audit.c +1 -1
@@ -116,7 +116,7 @@ static void audit_mappings(struct kvm_vcpu *vcpu, u64 *sptep, int level) gfn =kvm_mmu_page_get_gfn (sp, sptep - sp->spt); pfn =gfn_to_pfn_atomic (vcpu->kvm, gfn); -if (is_error_pfn (pfn)) +if (is_error_noslot_pfn (pfn))return ; hpa = pfn <<PAGE_SHIFT ;
arch/x86/kvm/paging_tmpl.h +1 -1
@@ -323,7 +323,7 @@ FNAME(prefetch_gpte)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,protect_clean_gpte (&pte_access, gpte); pfn =pte_prefetch_gfn_to_pfn (vcpu, gfn, no_dirty_log && (pte_access &ACC_WRITE_MASK )); -if (is_invalid_pfn (pfn)) +if (is_error_pfn (pfn))return false;/*
arch/x86/kvm/x86.c +1 -1
@@ -4502,7 +4502,7 @@ static bool reexecute_instruction(struct kvm_vcpu *vcpu, gva_t gva)* instruction -> ... */ pfn =gfn_to_pfn (vcpu->kvm,gpa_to_gfn (gpa)); -if (!is_error_pfn (pfn)) { +if (!is_error_noslot_pfn (pfn)) {kvm_release_pfn_clean (pfn);return true; }
include/linux/kvm_host.h +10 -8
@@ -58,28 +58,30 @@/* * For the normal pfn, the highest 12 bits should be zero, -* so we can mask these bits to indicate the error. +* so we can mask bit 62 ~ bit 52 to indicate the error pfn, +* mask bit 63 to indicate the noslot pfn. */ -#define KVM_PFN_ERR_MASK (0xfffULL << 52) +#define KVM_PFN_ERR_MASK (0x7ffULL << 52) +#define KVM_PFN_ERR_NOSLOT_MASK (0xfffULL << 52) +#define KVM_PFN_NOSLOT (0x1ULL << 63)#define KVM_PFN_ERR_FAULT (KVM_PFN_ERR_MASK)#define KVM_PFN_ERR_HWPOISON (KVM_PFN_ERR_MASK + 1) -#define KVM_PFN_ERR_BAD (KVM_PFN_ERR_MASK + 2) -#define KVM_PFN_ERR_RO_FAULT (KVM_PFN_ERR_MASK + 3) +#define KVM_PFN_ERR_RO_FAULT (KVM_PFN_ERR_MASK + 2)static inline bool is_error_pfn (pfn_t pfn) {return !!(pfn &KVM_PFN_ERR_MASK ); } -static inline bool is_noslot_pfn (pfn_t pfn) +static inline bool is_error_noslot_pfn (pfn_t pfn) { -return pfn ==KVM_PFN_ERR_BAD ; +return !!(pfn &KVM_PFN_ERR_NOSLOT_MASK ); } -static inline bool is_invalid_pfn (pfn_t pfn) +static inline bool is_noslot_pfn (pfn_t pfn) { -return !is_noslot_pfn (pfn) &&is_error_pfn (pfn); +return pfn ==KVM_PFN_NOSLOT ; }#define KVM_HVA_ERR_BAD (PAGE_OFFSET)
virt/kvm/iommu.c +2 -2
@@ -52,7 +52,7 @@ static pfn_t kvm_pin_pages(struct kvm_memory_slot *slot, gfn_t gfn, end_gfn = gfn + (size >>PAGE_SHIFT ); gfn +=1 ; -if (is_error_pfn (pfn)) +if (is_error_noslot_pfn (pfn))return pfn;while (gfn < end_gfn)
@@ -106,7 +106,7 @@ int kvm_iommu_map_pages(struct kvm *kvm, struct kvm_memory_slot *slot)* important because we unmap and unpin in 4kb steps later. */ pfn =kvm_pin_pages (slot, gfn, page_size); -if (is_error_pfn (pfn)) { +if (is_error_noslot_pfn (pfn)) { gfn +=1 ;continue ; }
virt/kvm/kvm_main.c +3 -3
@@ -1208,7 +1208,7 @@ __gfn_to_pfn_memslot(struct kvm_memory_slot *slot, gfn_t gfn, bool atomic,return KVM_PFN_ERR_RO_FAULT ;if (kvm_is_error_hva (addr)) -return KVM_PFN_ERR_BAD ; +return KVM_PFN_NOSLOT ;/* Do not map writable pfn in the readonly memslot. */ if (writable &&memslot_is_readonly (slot)) {
@@ -1290,7 +1290,7 @@ EXPORT_SYMBOL_GPL(gfn_to_page_many_atomic);static struct page *kvm_pfn_to_page (pfn_t pfn) { -if (is_error_pfn (pfn)) +if (is_error_noslot_pfn (pfn))return KVM_ERR_PTR_BAD_PAGE ;if (kvm_is_mmio_pfn (pfn)) {
@@ -1322,7 +1322,7 @@ EXPORT_SYMBOL_GPL(kvm_release_page_clean);void kvm_release_pfn_clean (pfn_t pfn) { -if (!is_error_pfn (pfn) && !kvm_is_mmio_pfn (pfn)) +if (!is_error_noslot_pfn (pfn) && !kvm_is_mmio_pfn (pfn))put_page (pfn_to_page (pfn)); }EXPORT_SYMBOL_GPL (kvm_release_pfn_clean);
Re: [PATCH v4 1/5] KVM: MMU: fix release noslot pfn
Quoted 14 lines · Xiao Guangrong
Quoted 12 lines · Marcelo Tosatti
Why does
is_error_pfn()return true for mmio spte? Its not an "error", after all.Please kill
is_invalid_pfnand use->
is_error_pfnfor checking for errors (mmio spte is not an error pfn, its a special pfn)-> add explicit
is_noslot_pfnchecks where necessary in the code (say to avoid interpreting a noslot_pfn's pfn "address" bits).
Quoted 6 lines
Quoted 1 line
(should have noticed this earlier, sorry).
Never mind, your comments are always appreciated! ;)
Marcelo, is it good to you?
(will post it after your check and full test)
Yes, this works (please check the validity of each case in addition to testing, haven't done it).
Also add a oneline comment on top of each
is_error_pfn,is_noslot_pfn,is_error_noslot_pfn
/* is_noslot_pfn: userspace translation valid but no memory slot */ /* is_error_pfn: ... */
etc.
Thanks.
Quoted 196 lines
arch/powerpc/kvm/book3s_32_mmu_host.c +1
-1@@ -155,7 +155,7 @@ int kvmppc_mmu_map_page(struct kvm_vcpu *vcpu, struct kvmppc_pte *orig_pte)/* Get host physical address for gpa */ hpaddr =kvmppc_gfn_to_pfn (vcpu, orig_pte->raddr >>PAGE_SHIFT ); -if (is_error_pfn (hpaddr)) { +if (is_error_noslot_pfn (hpaddr)) {printk (KERN_INFO "Couldn't get guest page for gfn %lx!\n" , orig_pte->eaddr); r = -EINVAL ;arch/powerpc/kvm/book3s_64_mmu_host.c +1
-1@@ -92,7 +92,7 @@ int kvmppc_mmu_map_page(struct kvm_vcpu *vcpu, struct kvmppc_pte *orig_pte)/* Get host physical address for gpa */ hpaddr =kvmppc_gfn_to_pfn (vcpu, orig_pte->raddr >>PAGE_SHIFT ); -if (is_error_pfn (hpaddr)) { +if (is_error_noslot_pfn (hpaddr)) {printk (KERN_INFO "Couldn't get guest page for gfn %lx!\n" , orig_pte->eaddr); r = -EINVAL ; goto out;arch/powerpc/kvm/e500_tlb.c +1
-1@@ -521,7 +521,7 @@ static inline void kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,if (likely (!pfnmap)) {unsigned long tsize_pages =1 << (tsize +10 -PAGE_SHIFT ); pfn =gfn_to_pfn_memslot (slot, gfn); -if (is_error_pfn (pfn)) { +if (is_error_noslot_pfn (pfn)) {printk (KERN_ERR "Couldn't get real page for gfn %lx!\n" , (long )gfn);return ;arch/x86/kvm/mmu.c +2
-2@@ -2699,7 +2699,7 @@ static void transparent_hugepage_adjust(struct kvm_vcpu *vcpu,* PT_PAGE_TABLE_LEVEL and there would be no adjustment done * here. */ -if (!is_error_pfn (pfn) && !kvm_is_mmio_pfn (pfn) && +if (!is_error_noslot_pfn (pfn) && !kvm_is_mmio_pfn (pfn) && level ==PT_PAGE_TABLE_LEVEL &&PageTransCompound (pfn_to_page (pfn)) && !has_wrprotected_page (vcpu->kvm, gfn,PT_DIRECTORY_LEVEL )) {@@ -2733,7 +2733,7 @@ static bool handle_abnormal_pfn(struct kvm_vcpu *vcpu, gva_t gva, gfn_t gfn,bool ret = true;/* The pfn is invalid, report the error! */ -if (unlikely (is_invalid_pfn (pfn))) { +if (unlikely (is_error_pfn (pfn))) { *ret_val =kvm_handle_bad_page (vcpu, gfn, pfn); goto exit; }arch/x86/kvm/mmu_audit.c +1
-1@@ -116,7 +116,7 @@ static void audit_mappings(struct kvm_vcpu *vcpu, u64 *sptep, int level) gfn =kvm_mmu_page_get_gfn (sp, sptep - sp->spt); pfn =gfn_to_pfn_atomic (vcpu->kvm, gfn); -if (is_error_pfn (pfn)) +if (is_error_noslot_pfn (pfn))return ; hpa = pfn <<PAGE_SHIFT ;arch/x86/kvm/paging_tmpl.h +1
-1@@ -323,7 +323,7 @@ FNAME(prefetch_gpte)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,protect_clean_gpte (&pte_access, gpte); pfn =pte_prefetch_gfn_to_pfn (vcpu, gfn, no_dirty_log && (pte_access &ACC_WRITE_MASK )); -if (is_invalid_pfn (pfn)) +if (is_error_pfn (pfn))return false;/* arch/x86/kvm/x86.c +1
-1@@ -4502,7 +4502,7 @@ static bool reexecute_instruction(struct kvm_vcpu *vcpu, gva_t gva)* instruction -> ... */ pfn =gfn_to_pfn (vcpu->kvm,gpa_to_gfn (gpa)); -if (!is_error_pfn (pfn)) { +if (!is_error_noslot_pfn (pfn)) {kvm_release_pfn_clean (pfn);return true; }include/linux/kvm_host.h +10
-8@@ -58,28 +58,30 @@/* * For the normal pfn, the highest 12 bits should be zero, -* so we can mask these bits to indicate the error. +* so we can mask bit 62 ~ bit 52 to indicate the error pfn, +* mask bit 63 to indicate the noslot pfn. */ -#define KVM_PFN_ERR_MASK (0xfffULL << 52) +#define KVM_PFN_ERR_MASK (0x7ffULL << 52) +#define KVM_PFN_ERR_NOSLOT_MASK (0xfffULL << 52) +#define KVM_PFN_NOSLOT (0x1ULL << 63)#define KVM_PFN_ERR_FAULT (KVM_PFN_ERR_MASK)#define KVM_PFN_ERR_HWPOISON (KVM_PFN_ERR_MASK + 1) -#define KVM_PFN_ERR_BAD (KVM_PFN_ERR_MASK + 2) -#define KVM_PFN_ERR_RO_FAULT (KVM_PFN_ERR_MASK + 3) +#define KVM_PFN_ERR_RO_FAULT (KVM_PFN_ERR_MASK + 2)static inline bool is_error_pfn (pfn_t pfn) {return !!(pfn &KVM_PFN_ERR_MASK ); } -static inline bool is_noslot_pfn (pfn_t pfn) +static inline bool is_error_noslot_pfn (pfn_t pfn) { -return pfn ==KVM_PFN_ERR_BAD ; +return !!(pfn &KVM_PFN_ERR_NOSLOT_MASK ); } -static inline bool is_invalid_pfn (pfn_t pfn) +static inline bool is_noslot_pfn (pfn_t pfn) { -return !is_noslot_pfn (pfn) &&is_error_pfn (pfn); +return pfn ==KVM_PFN_NOSLOT ; }#define KVM_HVA_ERR_BAD (PAGE_OFFSET)virt/kvm/iommu.c +2
-2@@ -52,7 +52,7 @@ static pfn_t kvm_pin_pages(struct kvm_memory_slot *slot, gfn_t gfn, end_gfn = gfn + (size >>PAGE_SHIFT ); gfn +=1 ; -if (is_error_pfn (pfn)) +if (is_error_noslot_pfn (pfn))return pfn;while (gfn < end_gfn)@@ -106,7 +106,7 @@ int kvm_iommu_map_pages(struct kvm *kvm, struct kvm_memory_slot *slot)* important because we unmap and unpin in 4kb steps later. */ pfn =kvm_pin_pages (slot, gfn, page_size); -if (is_error_pfn (pfn)) { +if (is_error_noslot_pfn (pfn)) { gfn +=1 ;continue ; }virt/kvm/kvm_main.c +3
-3@@ -1208,7 +1208,7 @@ __gfn_to_pfn_memslot(struct kvm_memory_slot *slot, gfn_t gfn, bool atomic,return KVM_PFN_ERR_RO_FAULT ;if (kvm_is_error_hva (addr)) -return KVM_PFN_ERR_BAD ; +return KVM_PFN_NOSLOT ;/* Do not map writable pfn in the readonly memslot. */ if (writable &&memslot_is_readonly (slot)) {@@ -1290,7 +1290,7 @@ EXPORT_SYMBOL_GPL(gfn_to_page_many_atomic);static struct page *kvm_pfn_to_page (pfn_t pfn) { -if (is_error_pfn (pfn)) +if (is_error_noslot_pfn (pfn))return KVM_ERR_PTR_BAD_PAGE ;if (kvm_is_mmio_pfn (pfn)) {@@ -1322,7 +1322,7 @@ EXPORT_SYMBOL_GPL(kvm_release_page_clean);void kvm_release_pfn_clean (pfn_t pfn) { -if (!is_error_pfn (pfn) && !kvm_is_mmio_pfn (pfn)) +if (!is_error_noslot_pfn (pfn) && !kvm_is_mmio_pfn (pfn))put_page (pfn_to_page (pfn)); }EXPORT_SYMBOL_GPL (kvm_release_pfn_clean);
Re: [PATCH v4 1/5] KVM: MMU: fix release noslot pfn
Quoted 25 lines · Marcelo Tosatti
Quoted 14 lines · Xiao Guangrong
Quoted 12 lines · Marcelo Tosatti
Why does
is_error_pfn()return true for mmio spte? Its not an "error", after all.Please kill
is_invalid_pfnand use->
is_error_pfnfor checking for errors (mmio spte is not an error pfn, its a special pfn)-> add explicit
is_noslot_pfnchecks where necessary in the code (say to avoid interpreting a noslot_pfn's pfn "address" bits).Quoted 6 lines
Quoted 1 line
(should have noticed this earlier, sorry).
Never mind, your comments are always appreciated! ;)
Marcelo, is it good to you?
(will post it after your check and full test)Yes, this works (please check the validity of each case in addition to testing, haven't done it).
Quoted 8 lines
Also add a oneline comment on top of each
is_error_pfn,is_noslot_pfn,is_error_noslot_pfn/*
is_noslot_pfn: userspace translation valid but no memory slot */ /*is_error_pfn: ... */etc.
Marcelo, i think this fix should be backport and your idea can be a separate patchset. Yes?
Re: [PATCH v4 1/5] KVM: MMU: fix release noslot pfn
Quoted 29 lines · Xiao Guangrong
Quoted 28 lines · Marcelo Tosatti
Quoted 14 lines · Xiao Guangrong
Quoted 12 lines · Marcelo Tosatti
Why does
is_error_pfn()return true for mmio spte? Its not an "error", after all.Please kill
is_invalid_pfnand use->
is_error_pfnfor checking for errors (mmio spte is not an error pfn, its a special pfn)-> add explicit
is_noslot_pfnchecks where necessary in the code (say to avoid interpreting a noslot_pfn's pfn "address" bits).Quoted 6 lines
Quoted 1 line
(should have noticed this earlier, sorry).
Never mind, your comments are always appreciated! ;)
Marcelo, is it good to you?
(will post it after your check and full test)Yes, this works (please check the validity of each case in addition to testing, haven't done it).
Also add a oneline comment on top of each
is_error_pfn,is_noslot_pfn,is_error_noslot_pfn
Quoted 8 lines
Quoted 5 lines
/*
is_noslot_pfn: userspace translation valid but no memory slot */ /*is_error_pfn: ... */etc.
Marcelo, i think this fix should be backport and your idea can be a separate patchset. Yes?
The current invalid/is_error/noslot_pfn separation is confusing, leading to one immediate bug and IMO more future bugs.
The proposed patch you sent is quite small, why is it troublesome to backport? (and i am just asking one line of comment, summing to 3 total of lines of comments).
Can't see the advantage of a special easily backportable fix?
PATCH v4 2/5 KVM: MMU: remove mmu_is_invalid
In reply to Xiao GuangrongRemove mmu_is_invalid and use is_invalid_pfn instead
--- arch/x86/kvm/mmu.c | 5 ----- arch/x86/kvm/paging_tmpl.h | 4 ++-- 2 files changed, 2 insertions(+), 7 deletions(-)
arch/x86/kvm/mmu.c +0 -5
@@ -2699,11 +2699,6 @@ static void transparent_hugepage_adjust(struct kvm_vcpu *vcpu, } } -static bool mmu_invalid_pfn (pfn_t pfn) -{ -return unlikely (is_invalid_pfn (pfn)); -} -static bool handle_abnormal_pfn (struct kvm_vcpu *vcpu,gva_t gva,gfn_t gfn,pfn_t pfn,unsigned access,int *ret_val) {
arch/x86/kvm/paging_tmpl.h +2 -2
@@ -340,7 +340,7 @@ static void FNAME(update_pte)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp, pte_access = sp->role.access &gpte_access (vcpu, gpte);protect_clean_gpte (&pte_access, gpte); pfn =gfn_to_pfn_atomic (vcpu->kvm,gpte_to_gfn (gpte)); -if (mmu_invalid_pfn (pfn)) +if (is_invalid_pfn (pfn))return ;/*
@@ -416,7 +416,7 @@ static void FNAME(pte_prefetch)(struct kvm_vcpu *vcpu, struct guest_walker *gw, gfn =gpte_to_gfn (gpte); pfn =pte_prefetch_gfn_to_pfn (vcpu, gfn, pte_access &ACC_WRITE_MASK ); -if (mmu_invalid_pfn (pfn)) +if (is_invalid_pfn (pfn))break ; mmu_set_spte(vcpu, spte, sp->role.access, pte_access, 0, 0,
Signature · 1 line
1.7.7.6
PATCH v4 3/5 KVM: MMU: cleanup FNAME(page_fault)
In reply to Xiao GuangrongLet it return emulate state instead of spte like __direct_map
--- arch/x86/kvm/paging_tmpl.h | 32 +++++++++++++------------------- 1 files changed, 13 insertions(+), 19 deletions(-)
arch/x86/kvm/paging_tmpl.h +13 -19
@@ -427,21 +427,21 @@ static void FNAME(pte_prefetch)(struct kvm_vcpu *vcpu, struct guest_walker *gw,/* * Fetch a shadow pte for a specific level in the paging hierarchy. +* If the guest tries to write a write-protected page, we need to +* emulate this operation, return 1 to indicate this case. */ -static u64 *FNAME (fetch )(struct kvm_vcpu *vcpu,gva_t addr, +static int FNAME (fetch )(struct kvm_vcpu *vcpu,gva_t addr,struct guest_walker *gw,int user_fault,int write_fault,int hlevel, -int *emulate,pfn_t pfn,bool map_writable, -bool prefault) +pfn_t pfn,bool map_writable,bool prefault) { -unsigned access = gw->pt_access;struct kvm_mmu_page *sp =NULL ; -int top_level; -unsigned direct_access;struct kvm_shadow_walk_iterator it; +unsigned direct_access, access = gw->pt_access; +int top_level, emulate =0 ;if (!is_present_gpte (gw->ptes[gw->level -1 ])) -return NULL ; +return 0 ; direct_access = gw->pte_access;
@@ -505,17 +505,17 @@ static u64 *FNAME(fetch)(struct kvm_vcpu *vcpu, gva_t addr,clear_sp_write_flooding_count (it.sptep);mmu_set_spte (vcpu, it.sptep, access, gw->pte_access, - user_fault, write_fault, emulate, it.level, + user_fault, write_fault, &emulate, it.level, gw->gfn, pfn, prefault, map_writable);FNAME (pte_prefetch)(vcpu, gw, it.sptep); -return it.sptep; +return emulate; out_gpte_changed:if (sp)kvm_mmu_put_page (sp, it.sptep);kvm_release_pfn_clean (pfn); -return NULL ; +return 0 ; }/*
@@ -538,8 +538,6 @@ static int FNAME(page_fault)(struct kvm_vcpu *vcpu, gva_t addr, u32 error_code,int write_fault = error_code &PFERR_WRITE_MASK ;int user_fault = error_code &PFERR_USER_MASK ;struct guest_walker walker; -u64 *sptep; -int emulate =0 ;int r;pfn_t pfn;int level =PT_PAGE_TABLE_LEVEL ;
@@ -601,17 +599,13 @@ static int FNAME(page_fault)(struct kvm_vcpu *vcpu, gva_t addr, u32 error_code,kvm_mmu_free_some_pages (vcpu);if (!force_pt_level)transparent_hugepage_adjust (vcpu, &walker.gfn, &pfn, &level); - sptep =FNAME (fetch)(vcpu, addr, &walker, user_fault, write_fault, - level, &emulate, pfn, map_writable, prefault); - (void )sptep; -pgprintk ("%s: shadow pte %p %llx emulate %d\n" , __func__, - sptep, *sptep, emulate); - + r =FNAME (fetch)(vcpu, addr, &walker, user_fault, write_fault, + level, pfn, map_writable, prefault); ++vcpu->stat.pf_fixed;kvm_mmu_audit (vcpu,AUDIT_POST_PAGE_FAULT );spin_unlock (&vcpu->kvm->mmu_lock); -return emulate; +return r; out_unlock:spin_unlock (&vcpu->kvm->mmu_lock);
Signature · 1 line
1.7.7.6
PATCH v4 4/5 KVM: MMU: move prefetch_invalid_gpte out of pagaing_tmp.h
In reply to Xiao GuangrongThe function does not depend on guest mmu mode, move it out from paging_tmpl.h
--- arch/x86/kvm/mmu.c | 36 ++++++++++++++++++++++++++++-------- arch/x86/kvm/paging_tmpl.h | 26 +++----------------------- 2 files changed, 31 insertions(+), 31 deletions(-)
arch/x86/kvm/mmu.c +28 -8
@@ -2505,6 +2505,14 @@ static void nonpaging_new_cr3(struct kvm_vcpu *vcpu)mmu_free_roots (vcpu); } +static bool is_rsvd_bits_set (struct kvm_mmu *mmu,u64 gpte,int level) +{ +int bit7; + + bit7 = (gpte >>7 ) &1 ; +return (gpte & mmu->rsvd_bits_mask[bit7][level-1 ]) !=0 ; +} +static pfn_t pte_prefetch_gfn_to_pfn (struct kvm_vcpu *vcpu,gfn_t gfn,bool no_dirty_log) {
@@ -2517,6 +2525,26 @@ static pfn_t pte_prefetch_gfn_to_pfn(struct kvm_vcpu *vcpu, gfn_t gfn,return gfn_to_pfn_memslot_atomic (slot, gfn); } +static bool prefetch_invalid_gpte (struct kvm_vcpu *vcpu, +struct kvm_mmu_page *sp,u64 *spte, +u64 gpte) +{ +if (is_rsvd_bits_set (&vcpu->arch.mmu, gpte,PT_PAGE_TABLE_LEVEL )) + goto no_present; + +if (!is_present_gpte (gpte)) + goto no_present; + +if (!(gpte &PT_ACCESSED_MASK )) + goto no_present; + +return false; + +no_present: +drop_spte (vcpu->kvm, spte); +return true; +} +static int direct_pte_prefetch_many (struct kvm_vcpu *vcpu,struct kvm_mmu_page *sp,u64 *start,u64 *end)
@@ -3394,14 +3422,6 @@ static void paging_free(struct kvm_vcpu *vcpu)nonpaging_free (vcpu); } -static bool is_rsvd_bits_set (struct kvm_mmu *mmu,u64 gpte,int level) -{ -int bit7; - - bit7 = (gpte >>7 ) &1 ; -return (gpte & mmu->rsvd_bits_mask[bit7][level-1 ]) !=0 ; -} -static inline void protect_clean_gpte (unsigned *access,unsigned gpte) {unsigned mask;
arch/x86/kvm/paging_tmpl.h +3 -23
@@ -305,26 +305,6 @@ static int FNAME(walk_addr_nested)(struct guest_walker *walker, addr, access); } -static bool FNAME (prefetch_invalid_gpte )(struct kvm_vcpu *vcpu, -struct kvm_mmu_page *sp,u64 *spte, -pt_element_t gpte) -{ -if (is_rsvd_bits_set (&vcpu->arch.mmu, gpte,PT_PAGE_TABLE_LEVEL )) - goto no_present; - -if (!is_present_gpte (gpte)) - goto no_present; - -if (!(gpte &PT_ACCESSED_MASK )) - goto no_present; - -return false; - -no_present: -drop_spte (vcpu->kvm, spte); -return true; -} -static void FNAME (update_pte )(struct kvm_vcpu *vcpu,struct kvm_mmu_page *sp,u64 *spte,const void *pte) {
@@ -333,7 +313,7 @@ static void FNAME(update_pte)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,pfn_t pfn; gpte = *(const pt_element_t *)pte; -if (FNAME (prefetch_invalid_gpte)(vcpu, sp, spte, gpte)) +if (prefetch_invalid_gpte (vcpu, sp, spte, gpte))return ;pgprintk ("%s: gpte %llx spte %p\n" , __func__, (u64 )gpte, spte);
@@ -408,7 +388,7 @@ static void FNAME(pte_prefetch)(struct kvm_vcpu *vcpu, struct guest_walker *gw, gpte = gptep[i]; -if (FNAME (prefetch_invalid_gpte)(vcpu, sp, spte, gpte)) +if (prefetch_invalid_gpte (vcpu, sp, spte, gpte))continue ; pte_access = sp->role.access &gpte_access (vcpu, gpte);
@@ -751,7 +731,7 @@ static int FNAME(sync_page)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp) sizeof(pt_element_t)))return -EINVAL ; -if (FNAME (prefetch_invalid_gpte)(vcpu, sp, &sp->spt[i], gpte)) { +if (prefetch_invalid_gpte (vcpu, sp, &sp->spt[i], gpte)) { vcpu->kvm->tlbs_dirty++;continue ; }
Signature · 1 line
1.7.7.6
PATCH v4 5/5 KVM: MMU: introduce FNAME(prefetch_gpte)
In reply to Xiao GuangrongThe only difference between FNAME(update_pte) and FNAME(pte_prefetch) is that the former is allowed to prefetch gfn from dirty logged slot, so introduce a common function to prefetch spte
--- arch/x86/kvm/paging_tmpl.h | 55 +++++++++++++++++++------------------------ 1 files changed, 24 insertions(+), 31 deletions(-)
arch/x86/kvm/paging_tmpl.h +24 -31
@@ -305,31 +305,43 @@ static int FNAME(walk_addr_nested)(struct guest_walker *walker, addr, access); } -static void FNAME (update_pte )(struct kvm_vcpu *vcpu,struct kvm_mmu_page *sp, -u64 *spte,const void *pte) +static bool +FNAME (prefetch_gpte )(struct kvm_vcpu *vcpu,struct kvm_mmu_page *sp, +u64 *spte,pt_element_t gpte,bool no_dirty_log) { -pt_element_t gpte;unsigned pte_access; +gfn_t gfn;pfn_t pfn; - gpte = *(const pt_element_t *)pte;if (prefetch_invalid_gpte (vcpu, sp, spte, gpte)) -return ; +return false;pgprintk ("%s: gpte %llx spte %p\n" , __func__, (u64 )gpte, spte); + + gfn =gpte_to_gfn (gpte); pte_access = sp->role.access &gpte_access (vcpu, gpte);protect_clean_gpte (&pte_access, gpte); - pfn =gfn_to_pfn_atomic (vcpu->kvm,gpte_to_gfn (gpte)); + pfn =pte_prefetch_gfn_to_pfn (vcpu, gfn, + no_dirty_log && (pte_access &ACC_WRITE_MASK ));if (is_invalid_pfn (pfn)) -return ; +return false;/* -* we call mmu_set_spte() with host_writable = true because that -* vcpu->arch.update_pte.pfn was fetched from get_user_pages(write = 1). +* we call mmu_set_spte() with host_writable = true because +* pte_prefetch_gfn_to_pfn always gets a writable pfn. */ mmu_set_spte (vcpu, spte, sp->role.access, pte_access,0 ,0 , -NULL ,PT_PAGE_TABLE_LEVEL , -gpte_to_gfn (gpte), pfn, true, true); +NULL ,PT_PAGE_TABLE_LEVEL , gfn, pfn, true, true); + +return true; +} + +static void FNAME (update_pte )(struct kvm_vcpu *vcpu,struct kvm_mmu_page *sp, +u64 *spte,const void *pte) +{ +pt_element_t gpte = *(const pt_element_t *)pte; + +FNAME (prefetch_gpte)(vcpu, sp, spte, gpte, false); } static bool FNAME(gpte_changed)(struct kvm_vcpu *vcpu,
@@ -375,33 +387,14 @@ static void FNAME(pte_prefetch)(struct kvm_vcpu *vcpu, struct guest_walker *gw, spte = sp->spt + i;for (i =0 ; i <PTE_PREFETCH_NUM ; i++, spte++) { -pt_element_t gpte; -unsigned pte_access; -gfn_t gfn; -pfn_t pfn; -if (spte == sptep)continue ;if (is_shadow_present_pte (*spte))continue ; - gpte = gptep[i]; - -if (prefetch_invalid_gpte (vcpu, sp, spte, gpte)) -continue ; - - pte_access = sp->role.access &gpte_access (vcpu, gpte); -protect_clean_gpte (&pte_access, gpte); - gfn =gpte_to_gfn (gpte); - pfn =pte_prefetch_gfn_to_pfn (vcpu, gfn, - pte_access &ACC_WRITE_MASK ); -if (is_invalid_pfn (pfn)) +if (!FNAME (prefetch_gpte)(vcpu, sp, spte, gptep[i], true))break ; - -mmu_set_spte (vcpu, spte, sp->role.access, pte_access,0 ,0 , -NULL ,PT_PAGE_TABLE_LEVEL , gfn, - pfn, true, true); } }
Signature · 1 line
1.7.7.6
Re: [PATCH v4 5/5] KVM: MMU: introduce FNAME(prefetch_gpte)
Quoted 31 lines · Xiao Guangrong
The only difference between
FNAME(update_pte)andFNAME(pte_prefetch)is that the former is allowed to prefetch gfn from dirty logged slot, so introduce a common function to prefetch spteSigned-off-by Xiao Guangrong <[email protected]>--- arch/x86/kvm/paging_tmpl.h | 55 +++++++++++++++++++------------------------ 1 files changed, 24 insertions(+), 31 deletions(-)arch/x86/kvm/paging_tmpl.h +4
-5@@ -305,31 +305,43 @@ static int FNAME(walk_addr_nested)(struct guest_walker *walker, addr, access); } -static void FNAME (update_pte )(struct kvm_vcpu *vcpu,struct kvm_mmu_page *sp, -u64 *spte,const void *pte) +static bool +FNAME (prefetch_gpte )(struct kvm_vcpu *vcpu,struct kvm_mmu_page *sp, +u64 *spte,pt_element_t gpte,bool no_dirty_log) { -pt_element_t gpte;unsigned pte_access; +gfn_t gfn;pfn_t pfn; - gpte = *(const pt_element_t *)pte;if (prefetch_invalid_gpte (vcpu, sp, spte, gpte)) -return ;
arch/x86/kvm/paging_tmpl.h static int FNAME(walk_addr_nested)(struct guest_walker *walker, · line 325
Quoted 10 lines
arch/x86/kvm/paging_tmpl.h +5
-1+return false;pgprintk ("%s: gpte %llx spte %p\n" , __func__, (u64 )gpte, spte); + + gfn =gpte_to_gfn (gpte); pte_access = sp->role.access &gpte_access (vcpu, gpte);protect_clean_gpte (&pte_access, gpte); - pfn =gfn_to_pfn_atomic (vcpu->kvm,gpte_to_gfn (gpte)); + pfn =pte_prefetch_gfn_to_pfn (vcpu, gfn, + no_dirty_log && (pte_access &ACC_WRITE_MASK ));
Is this a bugfix?
Re: [PATCH v4 5/5] KVM: MMU: introduce FNAME(prefetch_gpte)
Quoted 10 lines · Marcelo Tosatti
arch/x86/kvm/paging_tmpl.h static int FNAME(walk_addr_nested)(struct guest_walker *walker, · line 325
Quoted 8 lines
arch/x86/kvm/paging_tmpl.h +4
-1pgprintk ("%s: gpte %llx spte %p\n" , __func__, (u64 )gpte, spte); + + gfn =gpte_to_gfn (gpte); pte_access = sp->role.access &gpte_access (vcpu, gpte);protect_clean_gpte (&pte_access, gpte); - pfn =gfn_to_pfn_atomic (vcpu->kvm,gpte_to_gfn (gpte)); + pfn =pte_prefetch_gfn_to_pfn (vcpu, gfn, + no_dirty_log && (pte_access &ACC_WRITE_MASK ));Is this a bugfix?
No. It is a cleanup.
Actually, pte_prefetch_gfn_to_pfn(vcpu, gfn, false) is the same as gfn_to_pfn_atomic