[Qemu-devel] [PATCH] target/ppc: Fixup set_spr error in h_register_process_table

[Qemu-devel] [PATCH] target/ppc: Fixup set_spr error in h_register_process_table

[Suraj Jitindar Singh]

set_spr is used in the function h_register_process_table() to update the
LPCR_GTSE and LPCR_UPRT values based on the flags passed by the guest.
The set_spr function takes the last two arguments mask and value used to
mask and set the value of the spr respectively.

The current call site passes these arguments in the wrong order and thus
bot GTSE and UPRT will be set irrespective, which is obviously
incorrect.

Rearrange the function call so that these arguments are passed in the
correct order and the correct behaviour is exhibited.

It is worth noting that this wasn't detected earlier since these were
always both set in all cases where this H_CALL was made.

Fixes: 6de833070ca2 ("target/ppc: Set UPRT and GTSE on all cpus in H_REGISTER_PROCESS_TABLE")

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
 hw/ppc/spapr_hcall.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index aae5a62..aa1ffea 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -992,9 +992,10 @@ static target_ulong h_register_process_table(PowerPCCPU *cpu,
 
     /* Update the UPRT and GTSE bits in the LPCR for all cpus */
     CPU_FOREACH(cs) {
-        set_spr(cs, SPR_LPCR, LPCR_UPRT | LPCR_GTSE,
+        set_spr(cs, SPR_LPCR,
                 ((flags & (FLAG_RADIX | FLAG_HASH_PROC_TBL)) ? LPCR_UPRT : 0) |
-                ((flags & FLAG_GTSE) ? LPCR_GTSE : 0));
+                ((flags & FLAG_GTSE) ? LPCR_GTSE : 0),
+                LPCR_UPRT | LPCR_GTSE);
     }
 
     if (kvm_enabled()) {
-- 
2.9.4

Re: [Qemu-devel] [PATCH] target/ppc: Fixup set_spr error in h_register_process_table

[David Gibson]

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

On Mon, Jun 05, 2017 at 10:49:51AM +1000, Suraj Jitindar Singh wrote:
> set_spr is used in the function h_register_process_table() to update the
> LPCR_GTSE and LPCR_UPRT values based on the flags passed by the guest.
> The set_spr function takes the last two arguments mask and value used to
> mask and set the value of the spr respectively.
> 
> The current call site passes these arguments in the wrong order and thus
> bot GTSE and UPRT will be set irrespective, which is obviously
> incorrect.
> 
> Rearrange the function call so that these arguments are passed in the
> correct order and the correct behaviour is exhibited.
> 
> It is worth noting that this wasn't detected earlier since these were
> always both set in all cases where this H_CALL was made.
> 
> Fixes: 6de833070ca2 ("target/ppc: Set UPRT and GTSE on all cpus in H_REGISTER_PROCESS_TABLE")
> 
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>

Applied to ppc-for-2.10, thanks.

> ---
>  hw/ppc/spapr_hcall.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index aae5a62..aa1ffea 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -992,9 +992,10 @@ static target_ulong h_register_process_table(PowerPCCPU *cpu,
>  
>      /* Update the UPRT and GTSE bits in the LPCR for all cpus */
>      CPU_FOREACH(cs) {
> -        set_spr(cs, SPR_LPCR, LPCR_UPRT | LPCR_GTSE,
> +        set_spr(cs, SPR_LPCR,
>                  ((flags & (FLAG_RADIX | FLAG_HASH_PROC_TBL)) ? LPCR_UPRT : 0) |
> -                ((flags & FLAG_GTSE) ? LPCR_GTSE : 0));
> +                ((flags & FLAG_GTSE) ? LPCR_GTSE : 0),
> +                LPCR_UPRT | LPCR_GTSE);
>      }
>  
>      if (kvm_enabled()) {

-- 
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: 819 bytes --]