| Message ID | 20130622071448.GD2791@iris.ozlabs.ibm.com |
|---|---|
| State | New, archived |
| Headers | show |
On 22.06.2013, at 09:14, Paul Mackerras wrote: > The loop in kvmppc_mmu_book3s_64_xlate() that looks up a translation > in the guest hashed page table (HPT) keeps going if it finds an > HPTE that matches but doesn't allow access. This is incorrect; it > is different from what the hardware does, and there should never be > more than one matching HPTE anyway. This fixes it to stop when any > matching HPTE is found. IIRC I put in that logic to make it compatible with how QEMU handled HTAB lookups. Since then this has changed in QEMU though, and doing it this way is architecturally as correct as the other (the spec just says undefined behavior for duplicate entries), so I'm fine with the change. However, does book3s_32 differ here? I would like to keep them at least behave identically for undefined behavior. So in this case, unless you know of a difference for 32bit, please also provide a patch that does this change in book3s_32_mmu.c. Alex > > Signed-off-by: Paul Mackerras <paulus@samba.org> > --- > arch/powerpc/kvm/book3s_64_mmu.c | 14 ++------------ > 1 file changed, 2 insertions(+), 12 deletions(-) > > diff --git a/arch/powerpc/kvm/book3s_64_mmu.c b/arch/powerpc/kvm/book3s_64_mmu.c > index b871721..2e93bb5 100644 > --- a/arch/powerpc/kvm/book3s_64_mmu.c > +++ b/arch/powerpc/kvm/book3s_64_mmu.c > @@ -167,7 +167,6 @@ static int kvmppc_mmu_book3s_64_xlate(struct kvm_vcpu *vcpu, gva_t eaddr, > int i; > u8 key = 0; > bool found = false; > - bool perm_err = false; > int second = 0; > ulong mp_ea = vcpu->arch.magic_page_ea; > > @@ -248,11 +247,6 @@ do_second: > break; > } > > - if (!gpte->may_read) { > - perm_err = true; > - continue; > - } > - > dprintk("KVM MMU: Translated 0x%lx [0x%llx] -> 0x%llx " > "-> 0x%lx\n", > eaddr, avpn, gpte->vpage, gpte->raddr); > @@ -281,6 +275,8 @@ do_second: > if (pteg[i+1] != oldr) > copy_to_user((void __user *)ptegp, pteg, sizeof(pteg)); > > + if (!gpte->may_read) > + return -EPERM; > return 0; > } else { > dprintk("KVM MMU: No PTE found (ea=0x%lx sdr1=0x%llx " > @@ -296,13 +292,7 @@ do_second: > } > } > > - > no_page_found: > - > - > - if (perm_err) > - return -EPERM; > - > return -ENOENT; > > no_seg_found: > -- > 1.8.3.1 > > -- > To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" 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/powerpc/kvm/book3s_64_mmu.c b/arch/powerpc/kvm/book3s_64_mmu.c index b871721..2e93bb5 100644 --- a/arch/powerpc/kvm/book3s_64_mmu.c +++ b/arch/powerpc/kvm/book3s_64_mmu.c @@ -167,7 +167,6 @@ static int kvmppc_mmu_book3s_64_xlate(struct kvm_vcpu *vcpu, gva_t eaddr, int i; u8 key = 0; bool found = false; - bool perm_err = false; int second = 0; ulong mp_ea = vcpu->arch.magic_page_ea; @@ -248,11 +247,6 @@ do_second: break; } - if (!gpte->may_read) { - perm_err = true; - continue; - } - dprintk("KVM MMU: Translated 0x%lx [0x%llx] -> 0x%llx " "-> 0x%lx\n", eaddr, avpn, gpte->vpage, gpte->raddr); @@ -281,6 +275,8 @@ do_second: if (pteg[i+1] != oldr) copy_to_user((void __user *)ptegp, pteg, sizeof(pteg)); + if (!gpte->may_read) + return -EPERM; return 0; } else { dprintk("KVM MMU: No PTE found (ea=0x%lx sdr1=0x%llx " @@ -296,13 +292,7 @@ do_second: } } - no_page_found: - - - if (perm_err) - return -EPERM; - return -ENOENT; no_seg_found:
The loop in kvmppc_mmu_book3s_64_xlate() that looks up a translation in the guest hashed page table (HPT) keeps going if it finds an HPTE that matches but doesn't allow access. This is incorrect; it is different from what the hardware does, and there should never be more than one matching HPTE anyway. This fixes it to stop when any matching HPTE is found. Signed-off-by: Paul Mackerras <paulus@samba.org> --- arch/powerpc/kvm/book3s_64_mmu.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-)