[PATCH 1/2] powerpc test_emulate_step: fix DS operand in ld encoding to appropriate value

[PATCH 1/2] powerpc test_emulate_step: fix DS operand in ld encoding to appropriate value

[Balamuruhan S]

ld instruction should have 14 bit immediate field (DS) concatenated with
0b00 on the right, encode it accordingly.

Fixes: 4ceae137bdab ("powerpc: emulate_step() tests for load/store instructions")
Reviewed-by: Sandipan Das <sandipan@linux.ibm.com>
Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
---
 arch/powerpc/lib/test_emulate_step.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/lib/test_emulate_step.c b/arch/powerpc/lib/test_emulate_step.c
index 42347067739c..51c254fd15b5 100644
--- a/arch/powerpc/lib/test_emulate_step.c
+++ b/arch/powerpc/lib/test_emulate_step.c
@@ -19,7 +19,7 @@
  * definitions.
  */
 #define TEST_LD(r, base, i)	(PPC_INST_LD | ___PPC_RT(r) |		\
-					___PPC_RA(base) | IMM_L(i))
+					___PPC_RA(base) | ((i) & 0xfffc))
 #define TEST_LWZ(r, base, i)	(PPC_INST_LWZ | ___PPC_RT(r) |		\
 					___PPC_RA(base) | IMM_L(i))
 #define TEST_LWZX(t, a, b)	(PPC_INST_LWZX | ___PPC_RT(t) |		\

base-commit: 5aa19adac1f3152a5fd3b865a1ab46bb845d3696
-- 
2.24.1

[PATCH 2/2] powerpc test_emulate_step: add macro for 14 bit immediate field

[Balamuruhan S]

introduce macro `IMM_DS()` to encode DS form instructions with
14 bit immediate field.

Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
---
 arch/powerpc/lib/test_emulate_step.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/lib/test_emulate_step.c b/arch/powerpc/lib/test_emulate_step.c
index 51c254fd15b5..007292a1ad01 100644
--- a/arch/powerpc/lib/test_emulate_step.c
+++ b/arch/powerpc/lib/test_emulate_step.c
@@ -13,19 +13,20 @@
 #include <asm/code-patching.h>
 
 #define IMM_L(i)		((uintptr_t)(i) & 0xffff)
+#define IMM_DS(i)		((uintptr_t)(i) & 0xfffc)
 
 /*
  * Defined with TEST_ prefix so it does not conflict with other
  * definitions.
  */
 #define TEST_LD(r, base, i)	(PPC_INST_LD | ___PPC_RT(r) |		\
-					___PPC_RA(base) | ((i) & 0xfffc))
+					___PPC_RA(base) | IMM_DS(i))
 #define TEST_LWZ(r, base, i)	(PPC_INST_LWZ | ___PPC_RT(r) |		\
 					___PPC_RA(base) | IMM_L(i))
 #define TEST_LWZX(t, a, b)	(PPC_INST_LWZX | ___PPC_RT(t) |		\
 					___PPC_RA(a) | ___PPC_RB(b))
 #define TEST_STD(r, base, i)	(PPC_INST_STD | ___PPC_RS(r) |		\
-					___PPC_RA(base) | ((i) & 0xfffc))
+					___PPC_RA(base) | IMM_DS(i))
 #define TEST_LDARX(t, a, b, eh)	(PPC_INST_LDARX | ___PPC_RT(t) |	\
 					___PPC_RA(a) | ___PPC_RB(b) |	\
 					__PPC_EH(eh))
-- 
2.24.1

Re: [PATCH 1/2] powerpc test_emulate_step: fix DS operand in ld encoding to appropriate value

[Christophe Leroy]

Le 11/03/2020 à 07:14, Balamuruhan S a écrit :
> ld instruction should have 14 bit immediate field (DS) concatenated with
> 0b00 on the right, encode it accordingly.
> 
> Fixes: 4ceae137bdab ("powerpc: emulate_step() tests for load/store instructions")
> Reviewed-by: Sandipan Das <sandipan@linux.ibm.com>
> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> ---
>   arch/powerpc/lib/test_emulate_step.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/lib/test_emulate_step.c b/arch/powerpc/lib/test_emulate_step.c
> index 42347067739c..51c254fd15b5 100644
> --- a/arch/powerpc/lib/test_emulate_step.c
> +++ b/arch/powerpc/lib/test_emulate_step.c
> @@ -19,7 +19,7 @@
>    * definitions.
>    */
>   #define TEST_LD(r, base, i)	(PPC_INST_LD | ___PPC_RT(r) |		\
> -					___PPC_RA(base) | IMM_L(i))
> +					___PPC_RA(base) | ((i) & 0xfffc))

I think you should squash patch 1 and 2 together. Or at least you should
put the new IMM_DS macro in patch 1 and use it instead of open coding.

>   #define TEST_LWZ(r, base, i)	(PPC_INST_LWZ | ___PPC_RT(r) |		\
>   					___PPC_RA(base) | IMM_L(i))
>   #define TEST_LWZX(t, a, b)	(PPC_INST_LWZX | ___PPC_RT(t) |		\
> 
> base-commit: 5aa19adac1f3152a5fd3b865a1ab46bb845d3696
> 

Christophe

Re: [PATCH 1/2] powerpc test_emulate_step: fix DS operand in ld encoding to appropriate value

[Balamuruhan S]

On Wed, 2020-03-11 at 08:01 +0100, Christophe Leroy wrote:
> 
> Le 11/03/2020 à 07:14, Balamuruhan S a écrit :
> > ld instruction should have 14 bit immediate field (DS) concatenated
> > with
> > 0b00 on the right, encode it accordingly.
> > 
> > Fixes: 4ceae137bdab ("powerpc: emulate_step() tests for load/store
> > instructions")
> > Reviewed-by: Sandipan Das <sandipan@linux.ibm.com>
> > Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> > ---
> >   arch/powerpc/lib/test_emulate_step.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/powerpc/lib/test_emulate_step.c
> > b/arch/powerpc/lib/test_emulate_step.c
> > index 42347067739c..51c254fd15b5 100644
> > --- a/arch/powerpc/lib/test_emulate_step.c
> > +++ b/arch/powerpc/lib/test_emulate_step.c
> > @@ -19,7 +19,7 @@
> >    * definitions.
> >    */
> >   #define TEST_LD(r, base, i)	(PPC_INST_LD | ___PPC_RT(r) |	
> > 	\
> > -					___PPC_RA(base) | IMM_L(i))
> > +					___PPC_RA(base) | ((i) &
> > 0xfffc))
> 
> I think you should squash patch 1 and 2 together. Or at least you
> should
> put the new IMM_DS macro in patch 1 and use it instead of open
> coding.

sure, I will make the changes as suggested.

-- Bala

> 
> >   #define TEST_LWZ(r, base, i)	(PPC_INST_LWZ | ___PPC_RT(r) |	
> > 	\
> >   					___PPC_RA(base) | IMM_L(i))
> >   #define TEST_LWZX(t, a, b)	(PPC_INST_LWZX | ___PPC_RT(t) |
> > 		\
> > 
> > base-commit: 5aa19adac1f3152a5fd3b865a1ab46bb845d3696
> > 
> 
> Christophe