| Message ID | 20180207090445.GC19335@fergus.ozlabs.ibm.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/2] KVM: PPC: Book3S HV: Fix handling of secondary HPTEG in HPT resizing code | expand |
On Wed, Feb 07, 2018 at 08:04:45PM +1100, Paul Mackerras wrote: > This fixes the computation of the HPTE index to use when the HPT > resizing code encounters a bolted HPTE which is stored in its > secondary HPTE group. The code inverts the HPTE group number, which > is correct, but doesn't then mask it with new_hash_mask. As a result, > new_pteg will be effectively negative, resulting in new_hptep > pointing before the new HPT, which will corrupt memory. > > In addition, this removes two BUG_ON statements. The condition that > the BUG_ONs were testing -- that we have computed the hash value > incorrectly -- has never been observed in testing, and if it did > occur, would only affect the guest, not the host. Given that > BUG_ON should only be used in conditions where the kernel (i.e. > the host kernel, in this case) can't possibly continue execution, > it is not appropriate here. Fair enough. I think those were there because they were useful during early development, but should have been removed after that. > > Signed-off-by: Paul Mackerras <paulus@ozlabs.org> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > --- > arch/powerpc/kvm/book3s_64_mmu_hv.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c > index 9660972..d196499 100644 > --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c > +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c > @@ -1329,12 +1329,8 @@ static unsigned long resize_hpt_rehash_hpte(struct kvm_resize_hpt *resize, > } > > new_pteg = hash & new_hash_mask; > - if (vpte & HPTE_V_SECONDARY) { > - BUG_ON(~pteg != (hash & old_hash_mask)); > - new_pteg = ~new_pteg; > - } else { > - BUG_ON(pteg != (hash & old_hash_mask)); > - } > + if (vpte & HPTE_V_SECONDARY) > + new_pteg = ~hash & new_hash_mask; > > new_idx = new_pteg * HPTES_PER_GROUP + (idx % HPTES_PER_GROUP); > new_hptep = (__be64 *)(new->virt + (new_idx << 4));
diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c index 9660972..d196499 100644 --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c @@ -1329,12 +1329,8 @@ static unsigned long resize_hpt_rehash_hpte(struct kvm_resize_hpt *resize, } new_pteg = hash & new_hash_mask; - if (vpte & HPTE_V_SECONDARY) { - BUG_ON(~pteg != (hash & old_hash_mask)); - new_pteg = ~new_pteg; - } else { - BUG_ON(pteg != (hash & old_hash_mask)); - } + if (vpte & HPTE_V_SECONDARY) + new_pteg = ~hash & new_hash_mask; new_idx = new_pteg * HPTES_PER_GROUP + (idx % HPTES_PER_GROUP); new_hptep = (__be64 *)(new->virt + (new_idx << 4));
This fixes the computation of the HPTE index to use when the HPT resizing code encounters a bolted HPTE which is stored in its secondary HPTE group. The code inverts the HPTE group number, which is correct, but doesn't then mask it with new_hash_mask. As a result, new_pteg will be effectively negative, resulting in new_hptep pointing before the new HPT, which will corrupt memory. In addition, this removes two BUG_ON statements. The condition that the BUG_ONs were testing -- that we have computed the hash value incorrectly -- has never been observed in testing, and if it did occur, would only affect the guest, not the host. Given that BUG_ON should only be used in conditions where the kernel (i.e. the host kernel, in this case) can't possibly continue execution, it is not appropriate here. Signed-off-by: Paul Mackerras <paulus@ozlabs.org> --- arch/powerpc/kvm/book3s_64_mmu_hv.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)