| Message ID | 1429853947-30681-2-git-send-email-paulus@samba.org |
|---|---|
| State | New, archived |
| Headers | show |
Paul Mackerras <paulus@samba.org> writes: > The reference (R) and change (C) bits in a HPT entry can be set by > hardware at any time up until the HPTE is invalidated and the TLB > invalidation sequence has completed. This means that when removing > a HPTE, we need to read the HPTE after the invalidation sequence has > completed in order to obtain reliable values of R and C. The code > in kvmppc_do_h_remove() used to do this. However, commit 6f22bd3265fb > ("KVM: PPC: Book3S HV: Make HTAB code LE host aware") removed the > read after invalidation as a side effect of other changes. This > restores the read of the HPTE after invalidation. > > The user-visible effect of this bug would be that when migrating a > guest, there is a small probability that a page modified by the guest > and then unmapped by the guest might not get re-transmitted and thus > the destination might end up with a stale copy of the page. > > Fixes: 6f22bd3265fb > Cc: stable@vger.kernel.org # v3.17+ > Signed-off-by: Paul Mackerras <paulus@samba.org> > --- > arch/powerpc/kvm/book3s_hv_rm_mmu.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c > index f6bf0b1..5c1737f 100644 > --- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c > +++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c > @@ -413,14 +413,12 @@ long kvmppc_do_h_remove(struct kvm *kvm, unsigned long flags, > rev = real_vmalloc_addr(&kvm->arch.revmap[pte_index]); > v = pte & ~HPTE_V_HVLOCK; > if (v & HPTE_V_VALID) { > - u64 pte1; > - > - pte1 = be64_to_cpu(hpte[1]); > hpte[0] &= ~cpu_to_be64(HPTE_V_VALID); > - rb = compute_tlbie_rb(v, pte1, pte_index); > + rb = compute_tlbie_rb(v, be64_to_cpu(hpte[1]), pte_index); > do_tlbies(kvm, &rb, 1, global_invalidates(kvm, flags), true); > /* Read PTE low word after tlbie to get final R/C values */ > - remove_revmap_chain(kvm, pte_index, rev, v, pte1); > + remove_revmap_chain(kvm, pte_index, rev, v, > + be64_to_cpu(hpte[1])); > } May be add the above commit message as a code comment ? > r = rev->guest_rpte & ~HPTE_GR_RESERVED; > note_hpte_modification(kvm, rev); > -- > 2.1.4 > > -- > 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
On Tue, Apr 28, 2015 at 10:36:52AM +0530, Aneesh Kumar K.V wrote: > Paul Mackerras <paulus@samba.org> writes: > > > The reference (R) and change (C) bits in a HPT entry can be set by > > hardware at any time up until the HPTE is invalidated and the TLB > > invalidation sequence has completed. This means that when removing > > a HPTE, we need to read the HPTE after the invalidation sequence has > > completed in order to obtain reliable values of R and C. The code > > in kvmppc_do_h_remove() used to do this. However, commit 6f22bd3265fb > > ("KVM: PPC: Book3S HV: Make HTAB code LE host aware") removed the > > read after invalidation as a side effect of other changes. This > > restores the read of the HPTE after invalidation. > > > > The user-visible effect of this bug would be that when migrating a > > guest, there is a small probability that a page modified by the guest > > and then unmapped by the guest might not get re-transmitted and thus > > the destination might end up with a stale copy of the page. > > > > Fixes: 6f22bd3265fb > > Cc: stable@vger.kernel.org # v3.17+ > > Signed-off-by: Paul Mackerras <paulus@samba.org> > > --- > > arch/powerpc/kvm/book3s_hv_rm_mmu.c | 8 +++----- > > 1 file changed, 3 insertions(+), 5 deletions(-) > > > > diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c > > index f6bf0b1..5c1737f 100644 > > --- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c > > +++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c > > @@ -413,14 +413,12 @@ long kvmppc_do_h_remove(struct kvm *kvm, unsigned long flags, > > rev = real_vmalloc_addr(&kvm->arch.revmap[pte_index]); > > v = pte & ~HPTE_V_HVLOCK; > > if (v & HPTE_V_VALID) { > > - u64 pte1; > > - > > - pte1 = be64_to_cpu(hpte[1]); > > hpte[0] &= ~cpu_to_be64(HPTE_V_VALID); > > - rb = compute_tlbie_rb(v, pte1, pte_index); > > + rb = compute_tlbie_rb(v, be64_to_cpu(hpte[1]), pte_index); > > do_tlbies(kvm, &rb, 1, global_invalidates(kvm, flags), true); > > /* Read PTE low word after tlbie to get final R/C values */ > > - remove_revmap_chain(kvm, pte_index, rev, v, pte1); > > + remove_revmap_chain(kvm, pte_index, rev, v, > > + be64_to_cpu(hpte[1])); > > } > > May be add the above commit message as a code comment ? Well, that's what "/* Read PTE low word after tlbie to get final R/C values */" was trying to be, originally, but maybe it would be helpful to expand on it. Paul. -- 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_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c index f6bf0b1..5c1737f 100644 --- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c +++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c @@ -413,14 +413,12 @@ long kvmppc_do_h_remove(struct kvm *kvm, unsigned long flags, rev = real_vmalloc_addr(&kvm->arch.revmap[pte_index]); v = pte & ~HPTE_V_HVLOCK; if (v & HPTE_V_VALID) { - u64 pte1; - - pte1 = be64_to_cpu(hpte[1]); hpte[0] &= ~cpu_to_be64(HPTE_V_VALID); - rb = compute_tlbie_rb(v, pte1, pte_index); + rb = compute_tlbie_rb(v, be64_to_cpu(hpte[1]), pte_index); do_tlbies(kvm, &rb, 1, global_invalidates(kvm, flags), true); /* Read PTE low word after tlbie to get final R/C values */ - remove_revmap_chain(kvm, pte_index, rev, v, pte1); + remove_revmap_chain(kvm, pte_index, rev, v, + be64_to_cpu(hpte[1])); } r = rev->guest_rpte & ~HPTE_GR_RESERVED; note_hpte_modification(kvm, rev);
The reference (R) and change (C) bits in a HPT entry can be set by hardware at any time up until the HPTE is invalidated and the TLB invalidation sequence has completed. This means that when removing a HPTE, we need to read the HPTE after the invalidation sequence has completed in order to obtain reliable values of R and C. The code in kvmppc_do_h_remove() used to do this. However, commit 6f22bd3265fb ("KVM: PPC: Book3S HV: Make HTAB code LE host aware") removed the read after invalidation as a side effect of other changes. This restores the read of the HPTE after invalidation. The user-visible effect of this bug would be that when migrating a guest, there is a small probability that a page modified by the guest and then unmapped by the guest might not get re-transmitted and thus the destination might end up with a stale copy of the page. Fixes: 6f22bd3265fb Cc: stable@vger.kernel.org # v3.17+ Signed-off-by: Paul Mackerras <paulus@samba.org> --- arch/powerpc/kvm/book3s_hv_rm_mmu.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)