[Qemu-devel] [PATCH 0/2] ppc: Fixes for LSWX and LSWI instructions

[Qemu-devel] [PATCH 0/2] ppc: Fixes for LSWX and LSWI instructions

[Thomas Huth]

These two patches fix the bad range checks in the LSWI and LSWX
instructions.

To see the change in behavior for the lswx instruction, you can use the
"emulator" test from the kvm-unit-tests suite - code can be found here:

https://git.kernel.org/cgit/virt/kvm/kvm-unit-tests.git/tree/powerpc/emulator.c#n95

For testing the lswi instruction, an additional kvm-unit-test can be
used, see the patch that I've posted here:

https://patchwork.ozlabs.org/patch/610483/


Thomas Huth (2):
  ppc: Fix the range check in the LSWI instruction
  ppc: Fix the bad exception NIP value and the range check in LSWX

 target-ppc/cpu.h        | 10 ++++++++++
 target-ppc/mem_helper.c |  5 +++--
 target-ppc/translate.c  |  6 ++----
 3 files changed, 15 insertions(+), 6 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/2] ppc: Fix the range check in the LSWI instruction

[Thomas Huth]

There are two issues: First, the number of registers that are used has
to be calculated with "(nb + 3) / 4" (i.e. round always up, not down).
Second, the "start <= ra && (start + nr - 32) > ra" condition for the
wrap-around case is wrong: It has to be tested with "||" instead of "&&".
Since we can reuse this check later for the LSWX instruction, let's
place the fixed code into a helper function, too.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 target-ppc/cpu.h       | 10 ++++++++++
 target-ppc/translate.c |  6 ++----
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 9d4e43c..5282533 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -2415,6 +2415,16 @@ static inline bool msr_is_64bit(CPUPPCState *env, target_ulong msr)
     return msr & (1ULL << MSR_SF);
 }
 
+/**
+ * Check whether register rx is in the range between start and
+ * start + nregs (as needed by the LSWX and LSWI instructions)
+ */
+static inline bool lsw_reg_in_range(int start, int nregs, int rx)
+{
+    return (start + nregs <= 32 && rx >= start && rx < start + nregs) ||
+           (start + nregs > 32 && (rx >= start || rx < start + nregs - 32));
+}
+
 extern void (*cpu_ppc_hypercall)(PowerPCCPU *);
 
 #include "exec/exec-all.h"
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 6f0e7b4..b3860ec 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -3227,10 +3227,8 @@ static void gen_lswi(DisasContext *ctx)
 
     if (nb == 0)
         nb = 32;
-    nr = nb / 4;
-    if (unlikely(((start + nr) > 32  &&
-                  start <= ra && (start + nr - 32) > ra) ||
-                 ((start + nr) <= 32 && start <= ra && (start + nr) > ra))) {
+    nr = (nb + 3) / 4;
+    if (unlikely(lsw_reg_in_range(start, nr, ra))) {
         gen_inval_exception(ctx, POWERPC_EXCP_INVAL_LSWX);
         return;
     }
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/2] ppc: Fix the bad exception NIP value and the range check in LSWX

[Thomas Huth]

The range checks in the LSWX instruction are completely insufficient:
They do not take the wrap-around case into account, and the check
"reg < rx" should be "reg <= rx" instead. Fix it by using the new
lsw_reg_in_range() helper function that is already used for LSWI, too.

Then there is a second problem: In case the INVAL exception is generated,
the NIP value is wrong, it currently points to the instruction before
the LSWX instruction. This is because gen_lswx() already decreases the
NIP value by 4 (to be prepared for page fault exceptions), and
powerpc_excp() later decreases it again by 4 while handling the program
exception. So to get this right, we've got to undo the "- 4" from
gen_lswx() here before calling helper_raise_exception_err().

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 target-ppc/mem_helper.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target-ppc/mem_helper.c b/target-ppc/mem_helper.c
index 581d9fa..6d584c9 100644
--- a/target-ppc/mem_helper.c
+++ b/target-ppc/mem_helper.c
@@ -102,8 +102,9 @@ void helper_lswx(CPUPPCState *env, target_ulong addr, uint32_t reg,
 {
     if (likely(xer_bc != 0)) {
         int num_used_regs = (xer_bc + 3) / 4;
-        if (unlikely((ra != 0 && reg < ra && (reg + num_used_regs) > ra) ||
-                     (reg < rb && (reg + num_used_regs) > rb))) {
+        if (unlikely((ra != 0 && lsw_reg_in_range(reg, num_used_regs, ra)) ||
+                     lsw_reg_in_range(reg, num_used_regs, rb))) {
+            env->nip += 4;     /* Compensate the "nip - 4" from gen_lswx() */
             helper_raise_exception_err(env, POWERPC_EXCP_PROGRAM,
                                        POWERPC_EXCP_INVAL |
                                        POWERPC_EXCP_INVAL_LSWX);
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 0/2] ppc: Fixes for LSWX and LSWI instructions

[David Gibson]

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

On Thu, Apr 14, 2016 at 05:14:51PM +0200, Thomas Huth wrote:
> These two patches fix the bad range checks in the LSWI and LSWX
> instructions.
> 
> To see the change in behavior for the lswx instruction, you can use the
> "emulator" test from the kvm-unit-tests suite - code can be found here:
> 
> https://git.kernel.org/cgit/virt/kvm/kvm-unit-tests.git/tree/powerpc/emulator.c#n95
> 
> For testing the lswi instruction, an additional kvm-unit-test can be
> used, see the patch that I've posted here:
> 
> https://patchwork.ozlabs.org/patch/610483/

Thanks, applied to ppc-for-2.6.

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

Re: [Qemu-devel] [PATCH 0/2] ppc: Fixes for LSWX and LSWI instructions

[Mark Cave-Ayland]

On 14/04/16 16:14, Thomas Huth wrote:

> These two patches fix the bad range checks in the LSWI and LSWX
> instructions.
> 
> To see the change in behavior for the lswx instruction, you can use the
> "emulator" test from the kvm-unit-tests suite - code can be found here:
> 
> https://git.kernel.org/cgit/virt/kvm/kvm-unit-tests.git/tree/powerpc/emulator.c#n95
> 
> For testing the lswi instruction, an additional kvm-unit-test can be
> used, see the patch that I've posted here:
> 
> https://patchwork.ozlabs.org/patch/610483/
> 
> 
> Thomas Huth (2):
>   ppc: Fix the range check in the LSWI instruction
>   ppc: Fix the bad exception NIP value and the range check in LSWX
> 
>  target-ppc/cpu.h        | 10 ++++++++++
>  target-ppc/mem_helper.c |  5 +++--
>  target-ppc/translate.c  |  6 ++----
>  3 files changed, 15 insertions(+), 6 deletions(-)

I see David has already queued these, however I did give them a spin on
my OpenBIOS PPC test images and didn't see any obvious regressions.


ATB,

Mark.