target/ppc: fix tlbsync to check privilege level depending on GTSE

[Qemu-devel] [PATCH] target/ppc: fix tlbsync to check privilege level depending on GTSE

Posted by Cédric Le Goater 7 years, 7 months ago

tlbsync also needs to check the Guest Translation Shootdown Enable
(GTSE) bit in the Logical Partition Control Register (LPCR) to
determine at which privilege level it is running.

See commit c6fd28fd573d ("target/ppc: Update tlbie to check privilege
level based on GTSE")

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---

 This will have its importance when we activate the HV bit on the
 POWER9 CPU for the PowerNV machine.

 target/ppc/translate.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 0a0c090c9978..218665b4080b 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -4526,7 +4526,7 @@ static void gen_tlbie(DisasContext *ctx)
     TCGv_i32 t1;
 
     if (ctx->gtse) {
-        CHK_SV; /* If gtse is set then tblie is supervisor privileged */
+        CHK_SV; /* If gtse is set then tlbie is supervisor privileged */
     } else {
         CHK_HV; /* Else hypervisor privileged */
     }
@@ -4553,7 +4553,12 @@ static void gen_tlbsync(DisasContext *ctx)
 #if defined(CONFIG_USER_ONLY)
     GEN_PRIV;
 #else
-    CHK_HV;
+
+    if (ctx->gtse) {
+        CHK_SV; /* If gtse is set then tlbsync is supervisor privileged */
+    } else {
+        CHK_HV; /* Else hypervisor privileged */
+    }
 
     /* BookS does both ptesync and tlbsync make tlbsync a nop for server */
     if (ctx->insns_flags & PPC_BOOKE) {
-- 
2.13.6

Re: [Qemu-devel] [Qemu-ppc] [PATCH] target/ppc: fix tlbsync to check privilege level depending on GTSE

Posted by Suraj Jitindar Singh 7 years, 7 months ago

On Wed, 2018-03-14 at 18:33 +0100, Cédric Le Goater wrote:
> tlbsync also needs to check the Guest Translation Shootdown Enable
> (GTSE) bit in the Logical Partition Control Register (LPCR) to
> determine at which privilege level it is running.
> 
> See commit c6fd28fd573d ("target/ppc: Update tlbie to check privilege
> level based on GTSE")
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>

> ---
> 
>  This will have its importance when we activate the HV bit on the
>  POWER9 CPU for the PowerNV machine.
> 
>  target/ppc/translate.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 0a0c090c9978..218665b4080b 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -4526,7 +4526,7 @@ static void gen_tlbie(DisasContext *ctx)
>      TCGv_i32 t1;
>  
>      if (ctx->gtse) {
> -        CHK_SV; /* If gtse is set then tblie is supervisor
> privileged */
> +        CHK_SV; /* If gtse is set then tlbie is supervisor
> privileged */

^^^ Did that line actually change? :)

>      } else {
>          CHK_HV; /* Else hypervisor privileged */
>      }
> @@ -4553,7 +4553,12 @@ static void gen_tlbsync(DisasContext *ctx)
>  #if defined(CONFIG_USER_ONLY)
>      GEN_PRIV;
>  #else
> -    CHK_HV;
> +
> +    if (ctx->gtse) {
> +        CHK_SV; /* If gtse is set then tlbsync is supervisor
> privileged */
> +    } else {
> +        CHK_HV; /* Else hypervisor privileged */
> +    }
>  
>      /* BookS does both ptesync and tlbsync make tlbsync a nop for
> server */
>      if (ctx->insns_flags & PPC_BOOKE) {

Re: [Qemu-devel] [Qemu-ppc] [PATCH] target/ppc: fix tlbsync to check privilege level depending on GTSE

Posted by Cédric Le Goater 7 years, 7 months ago

>> --- a/target/ppc/translate.c
>> +++ b/target/ppc/translate.c
>> @@ -4526,7 +4526,7 @@ static void gen_tlbie(DisasContext *ctx)
>>      TCGv_i32 t1;
>>  
>>      if (ctx->gtse) {
>> -        CHK_SV; /* If gtse is set then tblie is supervisor
>> privileged */
>> +        CHK_SV; /* If gtse is set then tlbie is supervisor
>> privileged */
> 
> ^^^ Did that line actually change? :)

It did. Look closer :)

C.

Re: [Qemu-devel] [PATCH] target/ppc: fix tlbsync to check privilege level depending on GTSE

Posted by David Gibson 7 years, 7 months ago

On Wed, Mar 14, 2018 at 06:33:36PM +0100, Cédric Le Goater wrote:
> tlbsync also needs to check the Guest Translation Shootdown Enable
> (GTSE) bit in the Logical Partition Control Register (LPCR) to
> determine at which privilege level it is running.
> 
> See commit c6fd28fd573d ("target/ppc: Update tlbie to check privilege
> level based on GTSE")
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> 
>  This will have its importance when we activate the HV bit on the
>  POWER9 CPU for the PowerNV machine.
> 
>  target/ppc/translate.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)

Applied to ppc-for-2.12, thanks.

> 
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 0a0c090c9978..218665b4080b 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -4526,7 +4526,7 @@ static void gen_tlbie(DisasContext *ctx)
>      TCGv_i32 t1;
>  
>      if (ctx->gtse) {
> -        CHK_SV; /* If gtse is set then tblie is supervisor privileged */
> +        CHK_SV; /* If gtse is set then tlbie is supervisor privileged */
>      } else {
>          CHK_HV; /* Else hypervisor privileged */
>      }
> @@ -4553,7 +4553,12 @@ static void gen_tlbsync(DisasContext *ctx)
>  #if defined(CONFIG_USER_ONLY)
>      GEN_PRIV;
>  #else
> -    CHK_HV;
> +
> +    if (ctx->gtse) {
> +        CHK_SV; /* If gtse is set then tlbsync is supervisor privileged */
> +    } else {
> +        CHK_HV; /* Else hypervisor privileged */
> +    }
>  
>      /* BookS does both ptesync and tlbsync make tlbsync a nop for server */
>      if (ctx->insns_flags & PPC_BOOKE) {

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