Re: [PATCH 2/6] KVM: PPC: E500: Explicitly mark shadow maps invalid

[Scott Wood <scottwood@freescale.com>]

On 01/17/2013 08:34:53 PM, Alexander Graf wrote:
> When we invalidate shadow TLB maps on the host, we don't mark them
> as not valid. But we should.
> 
> Fix this by removing the E500_TLB_VALID from their flags when
> invalidating.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  arch/powerpc/kvm/e500_tlb.c |   13 ++++++++++---
>  1 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/e500_tlb.c b/arch/powerpc/kvm/e500_tlb.c
> index d38ad63..8efb2ac 100644
> --- a/arch/powerpc/kvm/e500_tlb.c
> +++ b/arch/powerpc/kvm/e500_tlb.c
> @@ -204,9 +204,13 @@ static void inval_gtlbe_on_host(struct  
> kvmppc_vcpu_e500 *vcpu_e500,
>  {
>  	struct kvm_book3e_206_tlb_entry *gtlbe >  		get_entry(vcpu_e500, tlbsel, esel);
> +	struct tlbe_ref *ref = &vcpu_e500->gtlb_priv[tlbsel][esel].ref;
> 
> -	if (tlbsel = 1 &&
> -	    vcpu_e500->gtlb_priv[1][esel].ref.flags & E500_TLB_BITMAP) {
> +	/* Don't bother with unmapped entries */
> +	if (!(ref->flags & E500_TLB_VALID))
> +		return;
> +
> +	if (tlbsel = 1 && ref->flags & E500_TLB_BITMAP) {
>  		u64 tmp = vcpu_e500->g2h_tlb1_map[esel];
>  		int hw_tlb_indx;
>  		unsigned long flags;
> @@ -224,7 +228,7 @@ static void inval_gtlbe_on_host(struct  
> kvmppc_vcpu_e500 *vcpu_e500,
>  		}
>  		mb();
>  		vcpu_e500->g2h_tlb1_map[esel] = 0;
> -		vcpu_e500->gtlb_priv[1][esel].ref.flags &=  
> ~E500_TLB_BITMAP;
> +		ref->flags &= ~(E500_TLB_BITMAP | E500_TLB_VALID);
>  		local_irq_restore(flags);
> 
>  		return;
> @@ -232,6 +236,9 @@ static void inval_gtlbe_on_host(struct  
> kvmppc_vcpu_e500 *vcpu_e500,
> 
>  	/* Guest tlbe is backed by at most one host tlbe per shadow  
> pid. */
>  	kvmppc_e500_tlbil_one(vcpu_e500, gtlbe);
> +
> +	/* Mark the TLB as not backed by the host anymore */
> +	ref->flags &= ~E500_TLB_VALID;
>  }

Invalidation paths that call kvmppc_e500_tlbil_all(), such as MMUCSR0  
and tlbivax, need a call to clear_tlb_refs() in order to get the valid  
bits cleared.

In looking this up, I also saw that tlbilxlpid (T=0) seems to be  
broken; it compares PID/TID as if it were T=1.  Don't be fooled by the  
"lpid" in the name; it's still relevant (and different from T=1) in the  
absence of E.HV, and should be treated as "tlbilx all".  Once  
implemented, that will also presumably use kvmppc_e500_tlbil_all().

-Scott