diff mbox

[1/5] KVM: PPC: Book3S HV: Fix computation of tlbie operand

Message ID 1414990320-6378-2-git-send-email-paulus@samba.org
State New, archived
Headers show

Commit Message

Paul Mackerras Nov. 3, 2014, 4:51 a.m. UTC
The B (segment size) field in the RB operand for the tlbie
instruction is two bits, which we get from the top two bits of
the first doubleword of the HPT entry to be invalidated.  These
bits go in bits 8 and 9 of the RB operand (bits 54 and 55 in IBM
bit numbering).

The compute_tlbie_rb() function gets these bits as v >> (62 - 8),
which is not correct as it will bring in the top 10 bits, not
just the top two.  These extra bits could corrupt the AP, AVAL
and L fields in the RB value.  To fix this we shift right 62 bits
and then shift left 8 bits, so we only get the two bits of the
B field.

The first doubleword of the HPT entry is under the control of the
guest kernel.  In fact, Linux guests will always put zeroes in bits
54 -- 61 (IBM bits 2 -- 9), but we should not rely on guests doing
this.

Cc: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: Paul Mackerras <paulus@samba.org>
---
 arch/powerpc/include/asm/kvm_book3s_64.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Aneesh Kumar K.V Nov. 3, 2014, 5:59 a.m. UTC | #1
Paul Mackerras <paulus@samba.org> writes:

> The B (segment size) field in the RB operand for the tlbie
> instruction is two bits, which we get from the top two bits of
> the first doubleword of the HPT entry to be invalidated.  These
> bits go in bits 8 and 9 of the RB operand (bits 54 and 55 in IBM
> bit numbering).
>
> The compute_tlbie_rb() function gets these bits as v >> (62 - 8),
> which is not correct as it will bring in the top 10 bits, not
> just the top two.  These extra bits could corrupt the AP, AVAL
> and L fields in the RB value.  To fix this we shift right 62 bits
> and then shift left 8 bits, so we only get the two bits of the
> B field.

Good catch.

>
> The first doubleword of the HPT entry is under the control of the
> guest kernel.  In fact, Linux guests will always put zeroes in bits
> 54 -- 61 (IBM bits 2 -- 9), but we should not rely on guests doing
> this.
>
> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> Signed-off-by: Paul Mackerras <paulus@samba.org>


Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

> ---
>  arch/powerpc/include/asm/kvm_book3s_64.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_book3s_64.h b/arch/powerpc/include/asm/kvm_book3s_64.h
> index 0aa8179..a37f1a4 100644
> --- a/arch/powerpc/include/asm/kvm_book3s_64.h
> +++ b/arch/powerpc/include/asm/kvm_book3s_64.h
> @@ -148,7 +148,7 @@ static inline unsigned long compute_tlbie_rb(unsigned long v, unsigned long r,
>  	/* This covers 14..54 bits of va*/
>  	rb = (v & ~0x7fUL) << 16;		/* AVA field */
>  
> -	rb |= v >> (62 - 8);			/*  B field */
> +	rb |= (v >> HPTE_V_SSIZE_SHIFT) << 8;	/*  B field */
>  	/*
>  	 * AVA in v had cleared lower 23 bits. We need to derive
>  	 * that from pteg index
> -- 
> 2.1.1
>
> --
> 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
Aneesh Kumar K.V Nov. 3, 2014, 6:29 a.m. UTC | #2
"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> writes:

> Paul Mackerras <paulus@samba.org> writes:
>
>> The B (segment size) field in the RB operand for the tlbie
>> instruction is two bits, which we get from the top two bits of
>> the first doubleword of the HPT entry to be invalidated.  These
>> bits go in bits 8 and 9 of the RB operand (bits 54 and 55 in IBM
>> bit numbering).
>>
>> The compute_tlbie_rb() function gets these bits as v >> (62 - 8),
>> which is not correct as it will bring in the top 10 bits, not
>> just the top two.  These extra bits could corrupt the AP, AVAL
>> and L fields in the RB value.  To fix this we shift right 62 bits
>> and then shift left 8 bits, so we only get the two bits of the
>> B field.
>
> Good catch.
>
>>
>> The first doubleword of the HPT entry is under the control of the
>> guest kernel.  In fact, Linux guests will always put zeroes in bits
>> 54 -- 61 (IBM bits 2 -- 9), but we should not rely on guests doing
>> this.
>>
>> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>> Signed-off-by: Paul Mackerras <paulus@samba.org>
>
>
> Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>
>> ---
>>  arch/powerpc/include/asm/kvm_book3s_64.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/include/asm/kvm_book3s_64.h b/arch/powerpc/include/asm/kvm_book3s_64.h
>> index 0aa8179..a37f1a4 100644
>> --- a/arch/powerpc/include/asm/kvm_book3s_64.h
>> +++ b/arch/powerpc/include/asm/kvm_book3s_64.h
>> @@ -148,7 +148,7 @@ static inline unsigned long compute_tlbie_rb(unsigned long v, unsigned long r,
>>  	/* This covers 14..54 bits of va*/
>>  	rb = (v & ~0x7fUL) << 16;		/* AVA field */
>>  
>> -	rb |= v >> (62 - 8);			/*  B field */
>> +	rb |= (v >> HPTE_V_SSIZE_SHIFT) << 8;	/*  B field */

or should we do. I guess the below is more closer to what we have in
rest of the code ?

       rb |= ((v >> (HPTE_V_SSIZE_SHIFT - 8)) & ~0xffUL); 


>>  	/*
>>  	 * AVA in v had cleared lower 23 bits. We need to derive
>>  	 * that from pteg index
>> -- 
>> 2.1.1
>>
>> --
>> 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
diff mbox

Patch

diff --git a/arch/powerpc/include/asm/kvm_book3s_64.h b/arch/powerpc/include/asm/kvm_book3s_64.h
index 0aa8179..a37f1a4 100644
--- a/arch/powerpc/include/asm/kvm_book3s_64.h
+++ b/arch/powerpc/include/asm/kvm_book3s_64.h
@@ -148,7 +148,7 @@  static inline unsigned long compute_tlbie_rb(unsigned long v, unsigned long r,
 	/* This covers 14..54 bits of va*/
 	rb = (v & ~0x7fUL) << 16;		/* AVA field */
 
-	rb |= v >> (62 - 8);			/*  B field */
+	rb |= (v >> HPTE_V_SSIZE_SHIFT) << 8;	/*  B field */
 	/*
 	 * AVA in v had cleared lower 23 bits. We need to derive
 	 * that from pteg index