Re: [PATCH 1/2] KVM: PPC: Book3S HV: Fix handling of secondary HPTEG in HPT resizing code

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2226 bytes --]

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));

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]