KVM: iommu: fix releasing unmapped page
PATCH 1/9 KVM: iommu: fix releasing unmapped page
There are two bugs:
- the 'error page' is forgot to be released
[ it is unneeded after commit a2766325cf9f9, for backport, we
still do kvm_release_pfn_clean for the error pfn ]
- guest pages are always released regardless of the unmapped page (e,g, caused by hwpoison)
Signed-off-by Xiao Guangrong <[email protected]>
--- virt/kvm/iommu.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)
virt/kvm/iommu.c +7 -0
@@ -107,6 +107,7 @@ int kvm_iommu_map_pages(struct kvm *kvm, struct kvm_memory_slot *slot)*/ pfn =kvm_pin_pages (slot, gfn, page_size);if (is_error_pfn (pfn)) { +kvm_release_pfn_clean (pfn); gfn +=1 ;continue ; }
@@ -300,6 +301,12 @@ static void kvm_iommu_put_pages(struct kvm *kvm,/* Get physical address */ phys =iommu_iova_to_phys (domain,gfn_to_gpa (gfn)); + +if (!phys) { + gfn++; +continue ; + } + pfn = phys >>PAGE_SHIFT ;/* Unmap address from IO address space */
Signature · 1 line
1.7.7.6
PATCH 2/9 KVM: define kvm_fault_pfn statically
After that, the exported and un-inline function, get_fault_pfn, can be removed
Signed-off-by Xiao Guangrong <[email protected]>
--- arch/x86/kvm/mmu.c | 2 +- include/linux/kvm_host.h | 3 ++- virt/kvm/kvm_main.c | 12 +++--------- 3 files changed, 6 insertions(+), 11 deletions(-)
arch/x86/kvm/mmu.c +1 -1
@@ -2514,7 +2514,7 @@ static pfn_t pte_prefetch_gfn_to_pfn(struct kvm_vcpu *vcpu, gfn_t gfn, slot =gfn_to_memslot_dirty_bitmap (vcpu, gfn, no_dirty_log);if (!slot) -return get_fault_pfn (); +return kvm_fault_pfn; hva =gfn_to_hva_memslot (slot, gfn);
include/linux/kvm_host.h +2 -1
@@ -48,6 +48,8 @@#define KVM_MAX_MMIO_FRAGMENTS \ (KVM_MMIO_SIZE / KVM_USER_MMIO_SIZE + KVM_EXTRA_MMIO_FRAGMENTS) +#define kvm_fault_pfn (-EFAULT) +/* * vcpu->requests bit members */
@@ -444,7 +446,6 @@ void kvm_release_pfn_clean(pfn_t pfn);void kvm_set_pfn_dirty (pfn_t pfn);void kvm_set_pfn_accessed (pfn_t pfn);void kvm_get_pfn (pfn_t pfn); -pfn_t get_fault_pfn (void );int kvm_read_guest_page (struct kvm *kvm,gfn_t gfn,void *data,int offset,int len);
virt/kvm/kvm_main.c +3 -9
@@ -948,12 +948,6 @@ static pfn_t get_bad_pfn(void)return -ENOENT ; } -pfn_t get_fault_pfn (void ) -{ -return -EFAULT ; -} -EXPORT_SYMBOL_GPL (get_fault_pfn); -static pfn_t get_hwpoison_pfn (void ) {return -EHWPOISON ;
@@ -1124,7 +1118,7 @@ static pfn_t hva_to_pfn(unsigned long addr, bool atomic, bool *async,struct vm_area_struct *vma;if (atomic) -return get_fault_pfn (); +return kvm_fault_pfn;down_read (¤t->mm->mmap_sem); if (npages == -EHWPOISON ||
@@ -1136,7 +1130,7 @@ static pfn_t hva_to_pfn(unsigned long addr, bool atomic, bool *async, vma =find_vma_intersection (current->mm, addr, addr+1 );if (vma ==NULL ) - pfn =get_fault_pfn (); + pfn = kvm_fault_pfn;else if ((vma->vm_flags &VM_PFNMAP )) { pfn = ((addr - vma->vm_start) >>PAGE_SHIFT ) + vma->vm_pgoff;
@@ -1144,7 +1138,7 @@ static pfn_t hva_to_pfn(unsigned long addr, bool atomic, bool *async, }else {if (async && (vma->vm_flags &VM_WRITE )) *async = true; - pfn =get_fault_pfn (); + pfn = kvm_fault_pfn; }up_read (¤t->mm->mmap_sem); }else
Signature · 1 line
1.7.7.6
Re: [PATCH 2/9] KVM: define kvm_fault_pfn statically
Quoted 28 lines · Xiao Guangrong
After that, the exported and un-inline function,
get_fault_pfn, can be removedSigned-off-by Xiao Guangrong <[email protected]>--- arch/x86/kvm/mmu.c | 2 +- include/linux/kvm_host.h | 3 ++- virt/kvm/kvm_main.c | 12 +++--------- 3 files changed, 6 insertions(+), 11 deletions(-)arch/x86/kvm/mmu.c +1
-1@@ -2514,7 +2514,7 @@ static pfn_t pte_prefetch_gfn_to_pfn(struct kvm_vcpu *vcpu, gfn_t gfn, slot =gfn_to_memslot_dirty_bitmap (vcpu, gfn, no_dirty_log);if (!slot) -return get_fault_pfn (); +return kvm_fault_pfn; hva =gfn_to_hva_memslot (slot, gfn);include/linux/kvm_host.h +0
-0
include/linux/kvm_host.h line 52
Quoted 5 lines
include/linux/kvm_host.h +2
-0@@ -48,6 +48,8 @@#define KVM_MAX_MMIO_FRAGMENTS \ (KVM_MMIO_SIZE / KVM_USER_MMIO_SIZE + KVM_EXTRA_MMIO_FRAGMENTS) +#define kvm_fault_pfn (-EFAULT) +
This is confusing, it appears to be a variable not a definition. Use "#define KVM_FAULT_PFN_ERR" please.
PATCH 3/9 KVM: define kvm_hwpoison_pfn statically
In reply to Xiao GuangrongThen, get_hwpoison_pfn and is_hwpoison_pfn can be removed
Signed-off-by Xiao Guangrong <[email protected]>
--- arch/x86/kvm/mmu.c | 2 +- include/linux/kvm_host.h | 4 ++-- virt/kvm/kvm_main.c | 13 +------------ 3 files changed, 4 insertions(+), 15 deletions(-)
arch/x86/kvm/mmu.c +1 -1
@@ -2651,7 +2651,7 @@ static void kvm_send_hwpoison_signal(unsigned long address, struct task_struct *static int kvm_handle_bad_page (struct kvm_vcpu *vcpu,gfn_t gfn,pfn_t pfn) {kvm_release_pfn_clean (pfn); -if (is_hwpoison_pfn (pfn)) { +if (pfn == kvm_hwpoison_pfn) {kvm_send_hwpoison_signal (gfn_to_hva (vcpu->kvm, gfn), current);return 0 ; }
include/linux/kvm_host.h +2 -2
@@ -48,7 +48,8 @@#define KVM_MAX_MMIO_FRAGMENTS \ (KVM_MMIO_SIZE / KVM_USER_MMIO_SIZE + KVM_EXTRA_MMIO_FRAGMENTS) -#define kvm_fault_pfn (-EFAULT) +#define kvm_fault_pfn (-EFAULT) +#define kvm_hwpoison_pfn (-EHWPOISON)/* * vcpu->requests bit members
@@ -396,7 +397,6 @@ extern struct page *bad_page;int is_error_page (struct page *page);int is_error_pfn (pfn_t pfn); -int is_hwpoison_pfn (pfn_t pfn);int is_noslot_pfn (pfn_t pfn);int is_invalid_pfn (pfn_t pfn);int kvm_is_error_hva (unsigned long addr);
virt/kvm/kvm_main.c +1 -12
@@ -948,17 +948,6 @@ static pfn_t get_bad_pfn(void)return -ENOENT ; } -static pfn_t get_hwpoison_pfn (void ) -{ -return -EHWPOISON ; -} - -int is_hwpoison_pfn (pfn_t pfn) -{ -return pfn == -EHWPOISON ; -} -EXPORT_SYMBOL_GPL (is_hwpoison_pfn); -int is_noslot_pfn (pfn_t pfn) {return pfn == -ENOENT ;
@@ -1124,7 +1113,7 @@ static pfn_t hva_to_pfn(unsigned long addr, bool atomic, bool *async,if (npages == -EHWPOISON || (!async &&check_user_page_hwpoison (addr))) {up_read (¤t->mm->mmap_sem); -return get_hwpoison_pfn (); +return kvm_hwpoison_pfn; } vma =find_vma_intersection (current->mm, addr, addr+1 );
Signature · 1 line
1.7.7.6
Re: [PATCH 3/9] KVM: define kvm_hwpoison_pfn statically
Quoted 27 lines · Xiao Guangrong
Then,
get_hwpoison_pfnandis_hwpoison_pfncan be removedSigned-off-by Xiao Guangrong <[email protected]>--- arch/x86/kvm/mmu.c | 2 +- include/linux/kvm_host.h | 4 ++-- virt/kvm/kvm_main.c | 13 +------------ 3 files changed, 4 insertions(+), 15 deletions(-)arch/x86/kvm/mmu.c +1
-1@@ -2651,7 +2651,7 @@ static void kvm_send_hwpoison_signal(unsigned long address, struct task_struct *static int kvm_handle_bad_page (struct kvm_vcpu *vcpu,gfn_t gfn,pfn_t pfn) {kvm_release_pfn_clean (pfn); -if (is_hwpoison_pfn (pfn)) { +if (pfn == kvm_hwpoison_pfn) {kvm_send_hwpoison_signal (gfn_to_hva (vcpu->kvm, gfn), current);return 0 ; }include/linux/kvm_host.h +0
-0
include/linux/kvm_host.h line 52
Quoted 6 lines
include/linux/kvm_host.h +2
-1@@ -48,7 +48,8 @@#define KVM_MAX_MMIO_FRAGMENTS \ (KVM_MMIO_SIZE / KVM_USER_MMIO_SIZE + KVM_EXTRA_MMIO_FRAGMENTS) -#define kvm_fault_pfn (-EFAULT) +#define kvm_fault_pfn (-EFAULT) +#define kvm_hwpoison_pfn (-EHWPOISON)
Same here as kvm_fault_pfn.
PATCH 4/9 KVM: define kvm_bad_pfn statically
In reply to Xiao GuangrongThen, remove get_bad_pfn
Signed-off-by Xiao Guangrong <[email protected]>
--- include/linux/kvm_host.h | 1 + virt/kvm/kvm_main.c | 7 +------ 2 files changed, 2 insertions(+), 6 deletions(-)
include/linux/kvm_host.h +1 -0
@@ -50,6 +50,7 @@#define kvm_fault_pfn (-EFAULT)#define kvm_hwpoison_pfn (-EHWPOISON) +#define kvm_bad_pfn (-ENOENT)/* * vcpu->requests bit members
virt/kvm/kvm_main.c +1 -6
@@ -943,11 +943,6 @@ int is_error_pfn(pfn_t pfn) }EXPORT_SYMBOL_GPL (is_error_pfn); -static pfn_t get_bad_pfn (void ) -{ -return -ENOENT ; -} -int is_noslot_pfn (pfn_t pfn) {return pfn == -ENOENT ;
@@ -1152,7 +1147,7 @@ static pfn_t __gfn_to_pfn(struct kvm *kvm, gfn_t gfn, bool atomic, bool *async, addr =gfn_to_hva (kvm, gfn);if (kvm_is_error_hva (addr)) -return get_bad_pfn (); +return kvm_bad_pfn;return hva_to_pfn (addr, atomic, async, write_fault, writable); }
Signature · 1 line
1.7.7.6
Re: [PATCH 4/9] KVM: define kvm_bad_pfn statically
Quoted 13 lines · Xiao Guangrong
Then, remove
get_bad_pfnSigned-off-by Xiao Guangrong <[email protected]>--- include/linux/kvm_host.h | 1 + virt/kvm/kvm_main.c | 7 +------ 2 files changed, 2 insertions(+), 6 deletions(-)include/linux/kvm_host.h +0
-0
include/linux/kvm_host.h line 53
Quoted 4 lines
include/linux/kvm_host.h +1
-0@@ -50,6 +50,7 @@#define kvm_fault_pfn (-EFAULT)#define kvm_hwpoison_pfn (-EHWPOISON) +#define kvm_bad_pfn (-ENOENT)
Remind me what is the guarantee that -Exxx does not clash with a valid pfn number?
Re: [PATCH 4/9] KVM: define kvm_bad_pfn statically
Quoted 2 lines · Marcelo Tosatti
Remind me what is the guarantee that -Exxx does not clash with a valid pfn number?
A pfn number is an address >> PAGE_SHIFT, so it will have the top 12 (at least) bits clear, whereas -Exxx will have the top bit set.
Paul.
Re: [PATCH 4/9] KVM: define kvm_bad_pfn statically
Marcelo, Paul,
Thanks for your review!
Quoted 8 lines · Paul Mackerras
Quoted 2 lines · Marcelo Tosatti
Remind me what is the guarantee that -Exxx does not clash with a valid pfn number?
A pfn number is an address >>
PAGE_SHIFT, so it will have the top 12 (at least) bits clear, whereas -Exxx will have the top bit set.
Yes.
As this way is hard to understand and it will break huge memory support on PAE 32bit cpu, i have used a new way in the v2:
http://marc.info/?l=linux-kernel&m=134398012027025&w=2
Please review the new version.
PATCH 5/9 KVM: inline is_*_pfn functions
In reply to Xiao GuangrongThese functions are exported and can not inline, move them to kvm_host.h to eliminate the overload of function call
Signed-off-by Xiao Guangrong <[email protected]>
--- include/linux/kvm_host.h | 19 ++++++++++++++++--- virt/kvm/kvm_main.c | 18 ------------------ 2 files changed, 16 insertions(+), 21 deletions(-)
include/linux/kvm_host.h +16 -3
@@ -21,6 +21,7 @@#include <linux/slab.h> #include <linux/rcupdate.h> #include <linux/ratelimit.h> +#include <linux/err.h> #include <asm/signal.h> #include <linux/kvm.h>
@@ -52,6 +53,21 @@#define kvm_hwpoison_pfn (-EHWPOISON)#define kvm_bad_pfn (-ENOENT) +static inline int is_error_pfn (pfn_t pfn) +{ +return IS_ERR_VALUE (pfn); +} + +static inline int is_noslot_pfn (pfn_t pfn) +{ +return pfn == -ENOENT ; +} + +static inline int is_invalid_pfn (pfn_t pfn) +{ +return !is_noslot_pfn (pfn) &&is_error_pfn (pfn); +} +/* * vcpu->requests bit members */
@@ -397,9 +413,6 @@ id_to_memslot(struct kvm_memslots *slots, int id)extern struct page *bad_page;int is_error_page (struct page *page); -int is_error_pfn (pfn_t pfn); -int is_noslot_pfn (pfn_t pfn); -int is_invalid_pfn (pfn_t pfn);int kvm_is_error_hva (unsigned long addr); int kvm_set_memory_region(struct kvm *kvm, struct kvm_userspace_memory_region *mem,
virt/kvm/kvm_main.c +0 -18
@@ -937,24 +937,6 @@ int is_error_page(struct page *page) }EXPORT_SYMBOL_GPL (is_error_page); -int is_error_pfn (pfn_t pfn) -{ -return IS_ERR_VALUE (pfn); -} -EXPORT_SYMBOL_GPL (is_error_pfn); - -int is_noslot_pfn (pfn_t pfn) -{ -return pfn == -ENOENT ; -} -EXPORT_SYMBOL_GPL (is_noslot_pfn); - -int is_invalid_pfn (pfn_t pfn) -{ -return !is_noslot_pfn (pfn) &&is_error_pfn (pfn); -} -EXPORT_SYMBOL_GPL (is_invalid_pfn); -struct page *get_bad_page (void ) {return ERR_PTR (-ENOENT );
Signature · 1 line
1.7.7.6
PATCH 6/9 KVM: remove the unused declare
In reply to Xiao GuangrongRemove it since it is not used anymore
Signed-off-by Xiao Guangrong <[email protected]>
--- include/linux/kvm_host.h | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
include/linux/kvm_host.h +0 -2
@@ -410,8 +410,6 @@ id_to_memslot(struct kvm_memslots *slots, int id)return slot; } -extern struct page *bad_page; -int is_error_page (struct page *page);int kvm_is_error_hva (unsigned long addr); int kvm_set_memory_region(struct kvm *kvm,
Signature · 1 line
1.7.7.6
PATCH 7/9 KVM: define kvm_bad_page statically
In reply to Xiao GuangrongIt is used to eliminate the overload of function call and cleanup the code
Signed-off-by Xiao Guangrong <[email protected]>
--- include/linux/kvm_host.h | 9 +++++++-- virt/kvm/async_pf.c | 2 +- virt/kvm/kvm_main.c | 13 +------------ 3 files changed, 9 insertions(+), 15 deletions(-)
include/linux/kvm_host.h +7 -2
@@ -68,6 +68,13 @@ static inline int is_invalid_pfn(pfn_t pfn)return !is_noslot_pfn (pfn) &&is_error_pfn (pfn); } +#define kvm_bad_page (ERR_PTR(-ENOENT)) + +static inline int is_error_page (struct page *page) +{ +return IS_ERR (page); +} +/* * vcpu->requests bit members */
@@ -410,7 +417,6 @@ id_to_memslot(struct kvm_memslots *slots, int id)return slot; } -int is_error_page (struct page *page);int kvm_is_error_hva (unsigned long addr); int kvm_set_memory_region(struct kvm *kvm, struct kvm_userspace_memory_region *mem,
@@ -437,7 +443,6 @@ void kvm_arch_flush_shadow(struct kvm *kvm);int gfn_to_page_many_atomic (struct kvm *kvm,gfn_t gfn,struct page **pages,int nr_pages); -struct page *get_bad_page (void );struct page *gfn_to_page (struct kvm *kvm,gfn_t gfn);unsigned long gfn_to_hva (struct kvm *kvm,gfn_t gfn);void kvm_release_page_clean (struct page *page);
virt/kvm/async_pf.c +1 -1
@@ -203,7 +203,7 @@ int kvm_async_pf_wakeup_all(struct kvm_vcpu *vcpu)if (!work)return -ENOMEM ; - work->page =get_bad_page (); + work->page = kvm_bad_page;INIT_LIST_HEAD (&work->queue);/* for list_del to work */ spin_lock (&vcpu->async_pf.lock);
virt/kvm/kvm_main.c +1 -12
@@ -931,17 +931,6 @@ void kvm_disable_largepages(void) }EXPORT_SYMBOL_GPL (kvm_disable_largepages); -int is_error_page (struct page *page) -{ -return IS_ERR (page); -} -EXPORT_SYMBOL_GPL (is_error_page); - -struct page *get_bad_page (void ) -{ -return ERR_PTR (-ENOENT ); -} -static inline unsigned long bad_hva (void ) {return PAGE_OFFSET ;
@@ -1188,7 +1177,7 @@ static struct page *kvm_pfn_to_page(pfn_t pfn)WARN_ON (kvm_is_mmio_pfn (pfn));if (is_error_pfn (pfn) ||kvm_is_mmio_pfn (pfn)) -return get_bad_page (); +return kvm_bad_page;return pfn_to_page (pfn); }
Signature · 1 line
1.7.7.6
Re: [PATCH 7/9] KVM: define kvm_bad_page statically
Quoted 15 lines · Xiao Guangrong
It is used to eliminate the overload of function call and cleanup the code
Signed-off-by Xiao Guangrong <[email protected]>--- include/linux/kvm_host.h | 9 +++++++-- virt/kvm/async_pf.c | 2 +- virt/kvm/kvm_main.c | 13 +------------ 3 files changed, 9 insertions(+), 15 deletions(-)include/linux/kvm_host.h +0
-0
include/linux/kvm_host.h static inline int is_invalid_pfn(pfn_t pfn) · line 71
Quoted 4 lines
include/linux/kvm_host.h +1
-0@@ -68,6 +68,13 @@ static inline int is_invalid_pfn(pfn_t pfn)return !is_noslot_pfn (pfn) &&is_error_pfn (pfn); } +#define kvm_bad_page (ERR_PTR(-ENOENT))
KVM_ERR_PTR_BAD_PAGE ...
PATCH 8/9 KVM: do not release the error pfn
In reply to Xiao GuangrongAfter commit a2766325cf9f9, the error pfn is replaced by the error code, it need not be released anymore
[ The patch is compiling tested for powerpc ]
Signed-off-by Xiao Guangrong <[email protected]>
--- arch/powerpc/kvm/e500_tlb.c | 1 - arch/x86/kvm/mmu.c | 6 +++--- arch/x86/kvm/mmu_audit.c | 4 +--- arch/x86/kvm/paging_tmpl.h | 8 ++------ virt/kvm/iommu.c | 1 - virt/kvm/kvm_main.c | 14 ++++++++------ 6 files changed, 14 insertions(+), 20 deletions(-)
arch/powerpc/kvm/e500_tlb.c +0 -1
@@ -524,7 +524,6 @@ static inline void kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,if (is_error_pfn (pfn)) {printk (KERN_ERR "Couldn't get real page for gfn %lx!\n" , (long )gfn); -kvm_release_pfn_clean (pfn);return ; }
arch/x86/kvm/mmu.c +3 -3
@@ -2498,7 +2498,9 @@ static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *sptep,rmap_recycle (vcpu, sptep, gfn); } } -kvm_release_pfn_clean (pfn); + +if (!is_error_pfn (pfn)) +kvm_release_pfn_clean (pfn); }static void nonpaging_new_cr3 (struct kvm_vcpu *vcpu)
@@ -3275,8 +3277,6 @@ static bool try_async_pf(struct kvm_vcpu *vcpu, bool prefault, gfn_t gfn,if (!async)return false;/* *pfn has correct page already */ -kvm_release_pfn_clean (*pfn); -if (!prefault &&can_do_async_pf (vcpu)) {trace_kvm_try_async_get_page (gva, gfn);if (kvm_find_async_pf_gfn (vcpu, gfn)) {
arch/x86/kvm/mmu_audit.c +1 -3
@@ -116,10 +116,8 @@ 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)) { -kvm_release_pfn_clean (pfn); +if (is_error_pfn (pfn))return ; - } hpa = pfn <<PAGE_SHIFT ;if ((*sptep &PT64_BASE_ADDR_MASK ) != hpa)
arch/x86/kvm/paging_tmpl.h +2 -6
@@ -370,10 +370,8 @@ static void FNAME(update_pte)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,pgprintk ("%s: gpte %llx spte %p\n" , __func__, (u64 )gpte, spte); pte_access = sp->role.access &FNAME (gpte_access)(vcpu, gpte, true); pfn =gfn_to_pfn_atomic (vcpu->kvm,gpte_to_gfn (gpte)); -if (mmu_invalid_pfn (pfn)) { -kvm_release_pfn_clean (pfn); +if (mmu_invalid_pfn (pfn))return ; - }/* * we call mmu_set_spte() with host_writable = true because that
@@ -448,10 +446,8 @@ 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)) { -kvm_release_pfn_clean (pfn); +if (mmu_invalid_pfn (pfn))break ; - } mmu_set_spte(vcpu, spte, sp->role.access, pte_access, 0, 0, NULL, PT_PAGE_TABLE_LEVEL, gfn,
virt/kvm/iommu.c +0 -1
@@ -107,7 +107,6 @@ int kvm_iommu_map_pages(struct kvm *kvm, struct kvm_memory_slot *slot)*/ pfn =kvm_pin_pages (slot, gfn, page_size);if (is_error_pfn (pfn)) { -kvm_release_pfn_clean (pfn); gfn +=1 ;continue ; }
virt/kvm/kvm_main.c +8 -6
@@ -102,9 +102,6 @@ static bool largepages_enabled = true;bool kvm_is_mmio_pfn (pfn_t pfn) { -if (is_error_pfn (pfn)) -return false; -if (pfn_valid (pfn)) {int reserved;struct page *tail =pfn_to_page (pfn);
@@ -1174,10 +1171,13 @@ EXPORT_SYMBOL_GPL(gfn_to_page_many_atomic);static struct page *kvm_pfn_to_page (pfn_t pfn) { -WARN_ON (kvm_is_mmio_pfn (pfn)); +if (is_error_pfn (pfn)) +return kvm_bad_page; -if (is_error_pfn (pfn) ||kvm_is_mmio_pfn (pfn)) +if (kvm_is_mmio_pfn (pfn)) { +WARN_ON (1 );return kvm_bad_page; + }return pfn_to_page (pfn); }
@@ -1202,7 +1202,9 @@ 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)) +WARN_ON (is_error_pfn (pfn)); + +if (!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 8/9] KVM: do not release the error pfn
kvm_release_pfn_clean in kvm_handle_bad_page() also can be removed, please review this one instead.
Changelog:
remove kvm_release_pfn_clean in kvm_handle_bad_page()
From: Xiao Guangrong <[email protected]>
Subject: [PATCH 08/21] KVM: do not release the error pfn
After commit a2766325cf9f9, the error pfn is replaced by the error code, it need not be released anymore
[ The patch is compiling tested for powerpc ]
Signed-off-by Xiao Guangrong <[email protected]>
--- arch/powerpc/kvm/e500_tlb.c | 1 - arch/x86/kvm/mmu.c | 7 +++---- arch/x86/kvm/mmu_audit.c | 4 +--- arch/x86/kvm/paging_tmpl.h | 8 ++------ virt/kvm/iommu.c | 1 - virt/kvm/kvm_main.c | 14 ++++++++------ 6 files changed, 14 insertions(+), 21 deletions(-)
arch/powerpc/kvm/e500_tlb.c +0 -1
@@ -524,7 +524,6 @@ static inline void kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,if (is_error_pfn (pfn)) {printk (KERN_ERR "Couldn't get real page for gfn %lx!\n" , (long )gfn); -kvm_release_pfn_clean (pfn);return ; }
arch/x86/kvm/mmu.c +3 -4
@@ -2498,7 +2498,9 @@ static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *sptep,rmap_recycle (vcpu, sptep, gfn); } } -kvm_release_pfn_clean (pfn); + +if (!is_error_pfn (pfn)) +kvm_release_pfn_clean (pfn); }static void nonpaging_new_cr3 (struct kvm_vcpu *vcpu)
@@ -2650,7 +2652,6 @@ static void kvm_send_hwpoison_signal(unsigned long address, struct task_struct *static int kvm_handle_bad_page (struct kvm_vcpu *vcpu,gfn_t gfn,pfn_t pfn) { -kvm_release_pfn_clean (pfn);if (pfn == kvm_hwpoison_pfn) {kvm_send_hwpoison_signal (gfn_to_hva (vcpu->kvm, gfn), current);return 0 ;
@@ -3275,8 +3276,6 @@ static bool try_async_pf(struct kvm_vcpu *vcpu, bool prefault, gfn_t gfn,if (!async)return false;/* *pfn has correct page already */ -kvm_release_pfn_clean (*pfn); -if (!prefault &&can_do_async_pf (vcpu)) {trace_kvm_try_async_get_page (gva, gfn);if (kvm_find_async_pf_gfn (vcpu, gfn)) {
arch/x86/kvm/mmu_audit.c +1 -3
@@ -116,10 +116,8 @@ 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)) { -kvm_release_pfn_clean (pfn); +if (is_error_pfn (pfn))return ; - } hpa = pfn <<PAGE_SHIFT ;if ((*sptep &PT64_BASE_ADDR_MASK ) != hpa)
arch/x86/kvm/paging_tmpl.h +2 -6
@@ -370,10 +370,8 @@ static void FNAME(update_pte)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,pgprintk ("%s: gpte %llx spte %p\n" , __func__, (u64 )gpte, spte); pte_access = sp->role.access &FNAME (gpte_access)(vcpu, gpte, true); pfn =gfn_to_pfn_atomic (vcpu->kvm,gpte_to_gfn (gpte)); -if (mmu_invalid_pfn (pfn)) { -kvm_release_pfn_clean (pfn); +if (mmu_invalid_pfn (pfn))return ; - }/* * we call mmu_set_spte() with host_writable = true because that
@@ -448,10 +446,8 @@ 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)) { -kvm_release_pfn_clean (pfn); +if (mmu_invalid_pfn (pfn))break ; - } mmu_set_spte(vcpu, spte, sp->role.access, pte_access, 0, 0, NULL, PT_PAGE_TABLE_LEVEL, gfn,
virt/kvm/iommu.c +0 -1
@@ -107,7 +107,6 @@ int kvm_iommu_map_pages(struct kvm *kvm, struct kvm_memory_slot *slot)*/ pfn =kvm_pin_pages (slot, gfn, page_size);if (is_error_pfn (pfn)) { -kvm_release_pfn_clean (pfn); gfn +=1 ;continue ; }
virt/kvm/kvm_main.c +8 -6
@@ -102,9 +102,6 @@ static bool largepages_enabled = true;bool kvm_is_mmio_pfn (pfn_t pfn) { -if (is_error_pfn (pfn)) -return false; -if (pfn_valid (pfn)) {int reserved;struct page *tail =pfn_to_page (pfn);
@@ -1174,10 +1171,13 @@ EXPORT_SYMBOL_GPL(gfn_to_page_many_atomic);static struct page *kvm_pfn_to_page (pfn_t pfn) { -WARN_ON (kvm_is_mmio_pfn (pfn)); +if (is_error_pfn (pfn)) +return kvm_bad_page; -if (is_error_pfn (pfn) ||kvm_is_mmio_pfn (pfn)) +if (kvm_is_mmio_pfn (pfn)) { +WARN_ON (1 );return kvm_bad_page; + }return pfn_to_page (pfn); }
@@ -1202,7 +1202,9 @@ 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)) +WARN_ON (is_error_pfn (pfn)); + +if (!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 8/9] KVM: do not release the error pfn
In reply to Xiao GuangrongQuoted 32 lines · Xiao Guangrong
After commit
a2766325cf9f9, the error pfn is replaced by the error code, it need not be released anymore[ The patch is compiling tested for powerpc ]
Signed-off-by Xiao Guangrong <[email protected]>--- arch/powerpc/kvm/e500_tlb.c | 1 - arch/x86/kvm/mmu.c | 6 +++--- arch/x86/kvm/mmu_audit.c | 4 +--- arch/x86/kvm/paging_tmpl.h | 8 ++------ virt/kvm/iommu.c | 1 - virt/kvm/kvm_main.c | 14 ++++++++------ 6 files changed, 14 insertions(+), 20 deletions(-)arch/powerpc/kvm/e500_tlb.c +0
-1@@ -524,7 +524,6 @@ static inline void kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,if (is_error_pfn (pfn)) {printk (KERN_ERR "Couldn't get real page for gfn %lx!\n" , (long )gfn); -kvm_release_pfn_clean (pfn);return ; }arch/x86/kvm/mmu.c +0
-0
arch/x86/kvm/mmu.c static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *sptep, · line 2504
Quoted 8 lines
arch/x86/kvm/mmu.c +3
-1@@ -2498,7 +2498,9 @@ static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *sptep,rmap_recycle (vcpu, sptep, gfn); } } -kvm_release_pfn_clean (pfn); + +if (!is_error_pfn (pfn)) +kvm_release_pfn_clean (pfn); }
Can it ever be error_pfn? Seems a problem if so.
Re: [PATCH 8/9] KVM: do not release the error pfn
Quoted 28 lines · Marcelo Tosatti
Quoted 27 lines · Xiao Guangrong
After commit
a2766325cf9f9, the error pfn is replaced by the error code, it need not be released anymore[ The patch is compiling tested for powerpc ]
Signed-off-by Xiao Guangrong <[email protected]>--- arch/powerpc/kvm/e500_tlb.c | 1 - arch/x86/kvm/mmu.c | 6 +++--- arch/x86/kvm/mmu_audit.c | 4 +--- arch/x86/kvm/paging_tmpl.h | 8 ++------ virt/kvm/iommu.c | 1 - virt/kvm/kvm_main.c | 14 ++++++++------ 6 files changed, 14 insertions(+), 20 deletions(-)arch/powerpc/kvm/e500_tlb.c +0
-1@@ -524,7 +524,6 @@ static inline void kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,if (is_error_pfn (pfn)) {printk (KERN_ERR "Couldn't get real page for gfn %lx!\n" , (long )gfn); -kvm_release_pfn_clean (pfn);return ; }
Quoted 16 lines
arch/x86/kvm/mmu.c static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *sptep, · line 2504
Quoted 13 lines
arch/x86/kvm/mmu.c +3
-1@@ -2498,7 +2498,9 @@ static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *sptep,rmap_recycle (vcpu, sptep, gfn); } } -kvm_release_pfn_clean (pfn); + +if (!is_error_pfn (pfn)) +kvm_release_pfn_clean (pfn); }Can it ever be
error_pfn? Seems a problem if so.
Yes, the no-slot-pfn, we will cache the mmio access into spte.
PATCH 9/9 KVM: do not release the error page
In reply to Xiao GuangrongAfter commit a2766325cf9f9, the error page is replaced by the error code, it need not be released anymore
[ The patch is compiling tested for powerpc ]
Signed-off-by Xiao Guangrong <[email protected]>
--- arch/powerpc/kvm/44x_tlb.c | 1 - arch/powerpc/kvm/book3s_pr.c | 4 +--- arch/x86/kvm/svm.c | 1 - arch/x86/kvm/vmx.c | 5 ++--- arch/x86/kvm/x86.c | 9 +++------ include/linux/kvm_host.h | 2 +- virt/kvm/async_pf.c | 4 ++-- virt/kvm/kvm_main.c | 5 +++-- 8 files changed, 12 insertions(+), 19 deletions(-)
arch/powerpc/kvm/44x_tlb.c +0 -1
@@ -319,7 +319,6 @@ void kvmppc_mmu_map(struct kvm_vcpu *vcpu, u64 gvaddr, gpa_t gpaddr,if (is_error_page (new_page)) {printk (KERN_ERR "Couldn't get guest page for gfn %llx!\n" , (unsigned long long )gfn); -kvm_release_page_clean (new_page);return ; } hpaddr =page_to_phys (new_page);
arch/powerpc/kvm/book3s_pr.c +1 -3
@@ -242,10 +242,8 @@ static void kvmppc_patch_dcbz(struct kvm_vcpu *vcpu, struct kvmppc_pte *pte)int i; hpage =gfn_to_page (vcpu->kvm, pte->raddr >>PAGE_SHIFT ); -if (is_error_page (hpage)) { -kvm_release_page_clean (hpage); +if (is_error_page (hpage))return ; - } hpage_offset = pte->raddr & ~PAGE_MASK ; hpage_offset &= ~0xFFFULL ;
arch/x86/kvm/svm.c +0 -1
@@ -2105,7 +2105,6 @@ static void *nested_svm_map(struct vcpu_svm *svm, u64 gpa, struct page **_page)return kmap (page); error: -kvm_release_page_clean (page);kvm_inject_gp (&svm->vcpu,0 );return NULL ;
arch/x86/kvm/vmx.c +2 -3
@@ -596,10 +596,9 @@ static inline struct vmcs12 *get_vmcs12(struct kvm_vcpu *vcpu)static struct page *nested_get_page (struct kvm_vcpu *vcpu,gpa_t addr) {struct page *page =gfn_to_page (vcpu->kvm, addr >>PAGE_SHIFT ); -if (is_error_page (page)) { -kvm_release_page_clean (page); +if (is_error_page (page))return NULL ; - } +return page; }
arch/x86/kvm/x86.c +3 -6
@@ -1635,10 +1635,9 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data) vcpu->arch.time_page =gfn_to_page (vcpu->kvm, data >>PAGE_SHIFT ); -if (is_error_page (vcpu->arch.time_page)) { -kvm_release_page_clean (vcpu->arch.time_page); +if (is_error_page (vcpu->arch.time_page)) vcpu->arch.time_page =NULL ; - } +break ; }case MSR_KVM_ASYNC_PF_EN :
@@ -3941,10 +3940,8 @@ static int emulator_cmpxchg_emulated(struct x86_emulate_ctxt *ctxt, goto emul_write; page =gfn_to_page (vcpu->kvm, gpa >>PAGE_SHIFT ); -if (is_error_page (page)) { -kvm_release_page_clean (page); +if (is_error_page (page)) goto emul_write; - } kaddr =kmap_atomic (page); kaddr +=offset_in_page (gpa);
include/linux/kvm_host.h +1 -1
@@ -458,7 +458,7 @@ pfn_t gfn_to_pfn(struct kvm *kvm, gfn_t gfn);pfn_t gfn_to_pfn_prot (struct kvm *kvm,gfn_t gfn,bool write_fault,bool *writable);pfn_t gfn_to_pfn_memslot (struct kvm_memory_slot *slot,gfn_t gfn); -void kvm_release_pfn_dirty (pfn_t ); +void kvm_release_pfn_dirty (pfn_t pfn);void kvm_release_pfn_clean (pfn_t pfn);void kvm_set_pfn_dirty (pfn_t pfn);void kvm_set_pfn_accessed (pfn_t pfn);
virt/kvm/async_pf.c +2 -2
@@ -111,7 +111,7 @@ void kvm_clear_async_pf_completion_queue(struct kvm_vcpu *vcpu)list_entry (vcpu->async_pf.done.next,typeof (*work), link);list_del (&work->link); -if (work->page) +if (!is_error_page (work->page))kvm_release_page_clean (work->page);kmem_cache_free (async_pf_cache, work); }
@@ -138,7 +138,7 @@ void kvm_check_async_pf_completion(struct kvm_vcpu *vcpu)list_del (&work->queue); vcpu->async_pf.queued--; -if (work->page) +if (!is_error_page (work->page))kvm_release_page_clean (work->page);kmem_cache_free (async_pf_cache, work); }
virt/kvm/kvm_main.c +3 -2
@@ -1195,8 +1195,9 @@ EXPORT_SYMBOL_GPL(gfn_to_page);void kvm_release_page_clean (struct page *page) { -if (!is_error_page (page)) -kvm_release_pfn_clean (page_to_pfn (page)); +WARN_ON (is_error_page (page)); + +kvm_release_pfn_clean (page_to_pfn (page)); }EXPORT_SYMBOL_GPL (kvm_release_page_clean);
Signature · 1 line
1.7.7.6