PowerPC KVM fixes for 2.6.26
Subsystems (3)
PATCH 1/1 PowerPC KVM fixes for 2.6.26
Hi Avi, these bug fixes should be committed for 2.6.26. Most are pretty harmless. Although the mmap_sem locking problem is fatal if you hit it, in practice we simply shouldn't hit that error case anyways.
-Hollis
PATCH 1/1 KVM: ppc: add lwzx/stwz emulation
Somehow these load/store instructions got missed before, but weren't used by the guest so didn't break anything.
arch/powerpc/kvm/emulate.c +12 -0
@@ -246,6 +246,11 @@ int kvmppc_emulate_instruction(struct kvcase 31 :switch (get_xop (inst)) { +case 23 :/* lwzx */ + rt =get_rt (inst); + emulated =kvmppc_handle_load (run, vcpu, rt,4 ,1 ); +break ; +case 83 :/* mfmsr */ rt =get_rt (inst); vcpu->arch.gpr[rt] = vcpu->arch.msr;
@@ -265,6 +270,13 @@ int kvmppc_emulate_instruction(struct kvcase 146 :/* mtmsr */ rs =get_rs (inst);kvmppc_set_msr (vcpu, vcpu->arch.gpr[rs]); +break ; + +case 151 :/* stwx */ + rs =get_rs (inst); + emulated =kvmppc_handle_store (run, vcpu, + vcpu->arch.gpr[rs], +4 ,1 );break ;case 163 :/* wrteei */
PATCH 1/1 KVM: ppc: Remove duplicate function
In reply to Hollis BlanchardThis was left behind from some code movement.
arch/powerpc/kvm/booke_guest.c +0 -33
@@ -225,39 +225,6 @@ void kvmppc_check_and_deliver_interruptsBITS_PER_BYTE *sizeof (*pending), priority + 1); } -} - -static int kvmppc_emulate_mmio (struct kvm_run *run,struct kvm_vcpu *vcpu) -{ -enum emulation_result er; -int r; - - er =kvmppc_emulate_instruction (run, vcpu); -switch (er) { -case EMULATE_DONE : -/* Future optimization: only reload non-volatiles if they were -* actually modified. */ - r =RESUME_GUEST_NV ; -break ; -case EMULATE_DO_MMIO : - run->exit_reason =KVM_EXIT_MMIO ; -/* We must reload nonvolatiles because "update" load/store -* instructions modify register state. */ -/* Future optimization: only reload non-volatiles if they were -* actually modified. */ - r =RESUME_HOST_NV ; -break ; -case EMULATE_FAIL : -/* XXX Deliver Program interrupt to guest. */ -printk (KERN_EMERG "%s: emulation failed (%08x)\n" , __func__, - vcpu->arch.last_inst); - r =RESUME_HOST ; -break ; -default : -BUG (); - } - -return r; }/**
include/asm-powerpc/kvm_ppc.h +1 -0
@@ -57,6 +57,7 @@ extern int kvmppc_handle_store(struct kvextern int kvmppc_emulate_instruction (struct kvm_run *run,struct kvm_vcpu *vcpu); +extern int kvmppc_emulate_mmio (struct kvm_run *run,struct kvm_vcpu *vcpu);extern void kvmppc_mmu_map (struct kvm_vcpu *vcpu,u64 gvaddr,gfn_t gfn,u64 asid,u32 flags);
PATCH 1/1 KVM: ppc: Remove unmatched kunmap() call
In reply to Hollis BlanchardWe're not calling kmap() now, so we shouldn't call kunmap() either. This has no practical effect in the non-highmem case, which is why it hasn't caused more obvious problems.
Pointed out by Anthony Liguori.
arch/powerpc/kvm/44x_tlb.c +0 -2
@@ -116,8 +116,6 @@ static void kvmppc_44x_shadow_release(ststruct tlbe *stlbe = &vcpu->arch.shadow_tlb[index];struct page *page = vcpu->arch.shadow_pages[index]; -kunmap (vcpu->arch.shadow_pages[index]); -if (get_tlb_v (stlbe)) {if (kvmppc_44x_tlbe_is_writable (stlbe))kvm_release_page_dirty (page);
PATCH 1/1 Report bad GFNs
In reply to Hollis BlanchardThis code shouldn't be hit anyways, but when it is, it's useful to have a little more information about the failure.
arch/powerpc/kvm/44x_tlb.c +1 -1
@@ -145,7 +145,7 @@ void kvmppc_mmu_map(struct kvm_vcpu *vcpdown_read (¤t->mm->mmap_sem); new_page =gfn_to_page (vcpu->kvm, gfn);if (is_error_page (new_page)) { -printk (KERN_ERR "Couldn't get guest page!\n" ); +printk (KERN_ERR "Couldn't get guest page for gfn %lx!\n" , gfn);kvm_release_page_clean (new_page);up_read (¤t->mm->mmap_sem);return ;
Re: [PATCH 5 of 5] [kvm powerpc] Report bad GFNs
Quoted 10 lines · Hollis Blanchard
This code shouldn't be hit anyways, but when it is, it's useful to have a little more information about the failure.
Signed-off-by Hollis Blanchard <[email protected]>
arch/powerpc/kvm/44x_tlb.c void kvmppc_mmu_map(struct kvm_vcpu *vcp · line 151
Quoted 8 lines
arch/powerpc/kvm/44x_tlb.c +1
-1@@ -145,7 +145,7 @@ void kvmppc_mmu_map(struct kvm_vcpu *vcpdown_read (¤t->mm->mmap_sem); new_page =gfn_to_page (vcpu->kvm, gfn);if (is_error_page (new_page)) { -printk (KERN_ERR "Couldn't get guest page!\n" ); +printk (KERN_ERR "Couldn't get guest page for gfn %lx!\n" , gfn);kvm_release_page_clean (new_page);up_read (¤t->mm->mmap_sem);return ;
FWIW, you should probably rate limit this sort of thing if this is what would get triggered if the guest tried to program the tlb with an invalid gfn.
Regards,
Anthony Liguori
Quoted 5 lines
--
To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5 of 5] [kvm powerpc] Report bad GFNs
Quoted 15 lines · On Wednesday 21 May 2008 21:47:40 Anthony Liguori wrote:
Quoted 14 lines · Hollis Blanchard
This code shouldn't be hit anyways, but when it is, it's useful to have a little more information about the failure.
Signed-off-by Hollis Blanchard <[email protected]>arch/powerpc/kvm/44x_tlb.c +1
-1@@ -145,7 +145,7 @@ void kvmppc_mmu_map(struct kvm_vcpu *vcpdown_read (¤t->mm->mmap_sem); new_page =gfn_to_page (vcpu->kvm, gfn);if (is_error_page (new_page)) { -printk (KERN_ERR "Couldn't get guest page!\n" ); +printk (KERN_ERR "Couldn't get guest page for gfn %lx!\n" , gfn);
Quoted 8 lines
arch/powerpc/kvm/44x_tlb.c void kvmppc_mmu_map(struct kvm_vcpu *vcp · line 151
Quoted 4 lines
arch/powerpc/kvm/44x_tlb.c +0
-0kvm_release_page_clean (new_page);up_read (¤t->mm->mmap_sem);return ;FWIW, you should probably rate limit this sort of thing if this is what would get triggered if the guest tried to program the tlb with an invalid gfn.
That was my initial thought as well, but in general, kvmppc_mmu_map() is only called if a TLB entry is "safe" -- meaning kvm_is_visible_gfn(kvm, gfn) was true. So in the absence of bugs elsewhere, this should never happen... but we know how that goes. ;)
Anyways, that's why I'm not worried about rate limiting here.
Signature · 2 lines
Hollis Blanchard IBM Linux Technology Center
PATCH 1/1 KVM: ppc: Use a read lock around MMU operations, and release it on error
In reply to Hollis Blanchardgfn_to_page() and kvm_release_page_clean() are called from other contexts with mmap_sem locked only for reading.
arch/powerpc/kvm/44x_tlb.c +3 -2
@@ -142,18 +142,19 @@ void kvmppc_mmu_map(struct kvm_vcpu *vcp stlbe = &vcpu->arch.shadow_tlb[victim];/* Get reference to new page. */ -down_write (¤t->mm->mmap_sem); +down_read (¤t->mm->mmap_sem); new_page =gfn_to_page (vcpu->kvm, gfn);if (is_error_page (new_page)) {printk (KERN_ERR "Couldn't get guest page!\n" );kvm_release_page_clean (new_page); +up_read (¤t->mm->mmap_sem);return ; } hpaddr =page_to_phys (new_page);/* Drop reference to old page. */ kvmppc_44x_shadow_release (vcpu, victim); -up_write (¤t->mm->mmap_sem); +up_read (¤t->mm->mmap_sem); vcpu->arch.shadow_pages[victim] = new_page;
Quoted 4 lines · Hollis Blanchard
Hi Avi, these bug fixes should be committed for 2.6.26. Most are pretty harmless. Although the
mmap_semlocking problem is fatal if you hit it, in practice we simply shouldn't hit that error case anyways.
Thanks; applied all. AFAICS only the locking fix is necessary for 2.6.26?
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 7 lines · On Sunday 25 May 2008 05:35:36 Avi Kivity wrote:
Quoted 4 lines · Hollis Blanchard
Hi Avi, these bug fixes should be committed for 2.6.26. Most are pretty harmless. Although the
mmap_semlocking problem is fatal if you hit it, in practice we simply shouldn't hit that error case anyways.Thanks; applied all. AFAICS only the locking fix is necessary for 2.6.26?
Yes, the locking fix is necessary, since it could crash the host. Technically speaking this is the only critical patch, but I think the chance of the others causing regressions is very low (especially since there was no KVM PPC support in 2.6.25, so by definition... ;) .
Signature · 2 lines
Hollis Blanchard IBM Linux Technology Center