Re: [PATCH 1/2] KVM: PPC: Book3S HV: Correct tlbie usage

["Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>]

Paul Mackerras <paulus@samba.org> writes:

> This corrects the usage of the tlbie (TLB invalidate entry) instruction
> in HV KVM.  The tlbie instruction changed between PPC970 and POWER7.
> On the PPC970, the bit to select large vs. small page is in the instruction,
> not in the RB register value.  This changes the code to use the correct
> form on PPC970.

I guess we need a similar fix for __tlbiel ?

>
> On POWER7 we were calculating the AVAL (Abbreviated Virtual Address, Lower)
> field of the RB value incorrectly for 64k pages.  This fixes it.
>
> Since we now have several cases to handle for the tlbie instruction, this
> factors out the code to do a sequence of tlbies into a new function,
> do_tlbies(), and calls that from the various places where the code was
> doing tlbie instructions inline.  It also makes kvmppc_h_bulk_remove()
> use the same global_invalidates() function for determining whether to do
> local or global TLB invalidations as is used in other places, for
> consistency, and also to make sure that kvm->arch.need_tlb_flush gets
> updated properly.
>
> Signed-off-by: Paul Mackerras <paulus@samba.org>
> Cc: stable@vger.kernel.org
> ---
>  arch/powerpc/include/asm/kvm_book3s_64.h |   2 +-
>  arch/powerpc/kvm/book3s_hv_rm_mmu.c      | 139 ++++++++++++++++++-------------
>  2 files changed, 82 insertions(+), 59 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_book3s_64.h b/arch/powerpc/include/asm/kvm_book3s_64.h
> index 9c1ff33..dc6b84a 100644
> --- a/arch/powerpc/include/asm/kvm_book3s_64.h
> +++ b/arch/powerpc/include/asm/kvm_book3s_64.h
> @@ -100,7 +100,7 @@ static inline unsigned long compute_tlbie_rb(unsigned long v, unsigned long r,
>  			/* (masks depend on page size) */
>  			rb |= 0x1000;		/* page encoding in LP field */
>  			rb |= (va_low & 0x7f) << 16; /* 7b of VA in AVA/LP field */
> -			rb |= (va_low & 0xfe);	/* AVAL field (P7 doesn't seem to care) */
> +			rb |= ((va_low << 4) & 0xf0);	/* AVAL field
> (P7 doesn't seem to care) */


Can you explain this more ? Why shift by 4 ? and what about the three
bits ('e' part of 0xfe) ? 

>  		}
>  	} else {
>  		/* 4kB page */
> diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
> index 6dcbb49..105b00f 100644
> --- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
> +++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
> @@ -385,6 +385,80 @@ static inline int try_lock_tlbie(unsigned int *lock)
>  	return old == 0;
>  }
>  

-aneesh