diff mbox

[RESEND,v2,11/17] target-ppc: implement darn instruction

Message ID 1473662506-27441-12-git-send-email-nikunj@linux.vnet.ibm.com
State New
Headers show

Commit Message

Nikunj A Dadhania Sept. 12, 2016, 6:41 a.m. UTC 
From: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>

darn: Deliver A Random Number

Currently return invalid random number for all the case. This needs
proper algorithm to provide cryptographically suitable random data.
Reading from /dev/random can block and that is not an expected behaviour
while the cpu instruction is getting executed. Moreover, /dev/random
would only work for linux-user

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target-ppc/helper.h     |  2 ++
 target-ppc/int_helper.c | 16 ++++++++++++++++
 target-ppc/translate.c  | 18 ++++++++++++++++++
 3 files changed, 36 insertions(+)

Comments

David Gibson Sept. 15, 2016, 1:07 a.m. UTC | #1 
On Mon, Sep 12, 2016 at 12:11:40PM +0530, Nikunj A Dadhania wrote:
> From: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> 
> darn: Deliver A Random Number
> 
> Currently return invalid random number for all the case. This needs
> proper algorithm to provide cryptographically suitable random data.
> Reading from /dev/random can block and that is not an expected behaviour
> while the cpu instruction is getting executed. Moreover, /dev/random
> would only work for linux-user
> 
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  target-ppc/helper.h     |  2 ++
>  target-ppc/int_helper.c | 16 ++++++++++++++++
>  target-ppc/translate.c  | 18 ++++++++++++++++++
>  3 files changed, 36 insertions(+)
> 
> diff --git a/target-ppc/helper.h b/target-ppc/helper.h
> index e75d070..966f2ce 100644
> --- a/target-ppc/helper.h
> +++ b/target-ppc/helper.h
> @@ -50,6 +50,8 @@ DEF_HELPER_FLAGS_1(cnttzd, TCG_CALL_NO_RWG_SE, tl, tl)
>  DEF_HELPER_FLAGS_1(popcntd, TCG_CALL_NO_RWG_SE, tl, tl)
>  DEF_HELPER_FLAGS_2(bpermd, TCG_CALL_NO_RWG_SE, i64, i64, i64)
>  DEF_HELPER_3(srad, tl, env, tl, tl)
> +DEF_HELPER_0(darn32, tl)
> +DEF_HELPER_0(darn64, tl)
>  #endif
>  
>  DEF_HELPER_FLAGS_1(cntlsw32, TCG_CALL_NO_RWG_SE, i32, i32)
> diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
> index 291fba0..51a9ac5 100644
> --- a/target-ppc/int_helper.c
> +++ b/target-ppc/int_helper.c
> @@ -182,6 +182,22 @@ target_ulong helper_cnttzd(target_ulong t)
>  {
>      return ctz64(t);
>  }
> +
> +/* Return invalid random number.
> + *
> + * FIXME: Add rng backend or other mechanism to get cryptographically suitable
> + * random number
> + */
> +target_ulong helper_darn32(void)
> +{
> +    return -1;
> +}
> +
> +target_ulong helper_darn64(void)
> +{
> +    return -1;
> +}
> +

TBH, I think you're going to want a single helper for both 32-bit and
64-bit cases.

>  #endif
>  
>  #if defined(TARGET_PPC64)
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 133c531..e9dad3f 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -528,6 +528,8 @@ EXTRACT_HELPER(FPW, 16, 1);
>  
>  /* addpcis */
>  EXTRACT_HELPER_DXFORM(DX, 10, 6, 6, 5, 16, 1, 1, 0, 0)
> +/* darn */
> +EXTRACT_HELPER(L, 16, 2);
>  
>  /***                            Jump target decoding                       ***/
>  /* Immediate address */
> @@ -1895,6 +1897,21 @@ static void gen_cnttzd(DisasContext *ctx)
>          gen_set_Rc0(ctx, cpu_gpr[rA(ctx->opcode)]);
>      }
>  }
> +
> +/* darn */
> +static void gen_darn(DisasContext *ctx)
> +{
> +    int l = L(ctx->opcode);
> +
> +    if (l == 0) {
> +        gen_helper_darn32(cpu_gpr[rD(ctx->opcode)]);
> +    } else if (l <= 2) {
> +        /* Return 64-bit random for both CRN and RRN */
> +        gen_helper_darn64(cpu_gpr[rD(ctx->opcode)]);

So it might be simpler to just leave out the helper stubs for now, and
always return the invalid value from the generated code.

> +    } else {
> +        tcg_gen_movi_i64(cpu_gpr[rD(ctx->opcode)], -1);
> +    }
> +}
>  #endif
>  
>  /***                             Integer rotate                            ***/
> @@ -6212,6 +6229,7 @@ GEN_HANDLER_E(prtyw, 0x1F, 0x1A, 0x04, 0x0000F801, PPC_NONE, PPC2_ISA205),
>  GEN_HANDLER(popcntd, 0x1F, 0x1A, 0x0F, 0x0000F801, PPC_POPCNTWD),
>  GEN_HANDLER(cntlzd, 0x1F, 0x1A, 0x01, 0x00000000, PPC_64B),
>  GEN_HANDLER_E(cnttzd, 0x1F, 0x1A, 0x11, 0x00000000, PPC_NONE, PPC2_ISA300),
> +GEN_HANDLER_E(darn, 0x1F, 0x13, 0x17, 0x001CF801, PPC_NONE, PPC2_ISA300),
>  GEN_HANDLER_E(prtyd, 0x1F, 0x1A, 0x05, 0x0000F801, PPC_NONE, PPC2_ISA205),
>  GEN_HANDLER_E(bpermd, 0x1F, 0x1C, 0x07, 0x00000001, PPC_NONE, PPC2_PERM_ISA206),
>  #endif
Nikunj A Dadhania Sept. 15, 2016, 6:40 a.m. UTC | #2 
David Gibson <david@gibson.dropbear.id.au> writes:

> [ Unknown signature status ]
> On Mon, Sep 12, 2016 at 12:11:40PM +0530, Nikunj A Dadhania wrote:
>> From: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
>> 
>> darn: Deliver A Random Number
>> 
>> Currently return invalid random number for all the case. This needs
>> proper algorithm to provide cryptographically suitable random data.
>> Reading from /dev/random can block and that is not an expected behaviour
>> while the cpu instruction is getting executed. Moreover, /dev/random
>> would only work for linux-user
>> 
>> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>> ---
>>  target-ppc/helper.h     |  2 ++
>>  target-ppc/int_helper.c | 16 ++++++++++++++++
>>  target-ppc/translate.c  | 18 ++++++++++++++++++
>>  3 files changed, 36 insertions(+)
>> 
>> diff --git a/target-ppc/helper.h b/target-ppc/helper.h
>> index e75d070..966f2ce 100644
>> --- a/target-ppc/helper.h
>> +++ b/target-ppc/helper.h
>> @@ -50,6 +50,8 @@ DEF_HELPER_FLAGS_1(cnttzd, TCG_CALL_NO_RWG_SE, tl, tl)
>>  DEF_HELPER_FLAGS_1(popcntd, TCG_CALL_NO_RWG_SE, tl, tl)
>>  DEF_HELPER_FLAGS_2(bpermd, TCG_CALL_NO_RWG_SE, i64, i64, i64)
>>  DEF_HELPER_3(srad, tl, env, tl, tl)
>> +DEF_HELPER_0(darn32, tl)
>> +DEF_HELPER_0(darn64, tl)
>>  #endif
>>  
>>  DEF_HELPER_FLAGS_1(cntlsw32, TCG_CALL_NO_RWG_SE, i32, i32)
>> diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
>> index 291fba0..51a9ac5 100644
>> --- a/target-ppc/int_helper.c
>> +++ b/target-ppc/int_helper.c
>> @@ -182,6 +182,22 @@ target_ulong helper_cnttzd(target_ulong t)
>>  {
>>      return ctz64(t);
>>  }
>> +
>> +/* Return invalid random number.
>> + *
>> + * FIXME: Add rng backend or other mechanism to get cryptographically suitable
>> + * random number
>> + */
>> +target_ulong helper_darn32(void)
>> +{
>> +    return -1;
>> +}
>> +
>> +target_ulong helper_darn64(void)
>> +{
>> +    return -1;
>> +}
>> +
>
> TBH, I think you're going to want a single helper for both 32-bit and
> 64-bit cases.

Initially, we used to split 32/64-bit in helper, Richard suggested to
make that decision in translate.c

>
>>  #endif
>>  
>>  #if defined(TARGET_PPC64)
>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
>> index 133c531..e9dad3f 100644
>> --- a/target-ppc/translate.c
>> +++ b/target-ppc/translate.c
>> @@ -528,6 +528,8 @@ EXTRACT_HELPER(FPW, 16, 1);
>>  
>>  /* addpcis */
>>  EXTRACT_HELPER_DXFORM(DX, 10, 6, 6, 5, 16, 1, 1, 0, 0)
>> +/* darn */
>> +EXTRACT_HELPER(L, 16, 2);
>>  
>>  /***                            Jump target decoding                       ***/
>>  /* Immediate address */
>> @@ -1895,6 +1897,21 @@ static void gen_cnttzd(DisasContext *ctx)
>>          gen_set_Rc0(ctx, cpu_gpr[rA(ctx->opcode)]);
>>      }
>>  }
>> +
>> +/* darn */
>> +static void gen_darn(DisasContext *ctx)
>> +{
>> +    int l = L(ctx->opcode);
>> +
>> +    if (l == 0) {
>> +        gen_helper_darn32(cpu_gpr[rD(ctx->opcode)]);
>> +    } else if (l <= 2) {
>> +        /* Return 64-bit random for both CRN and RRN */
>> +        gen_helper_darn64(cpu_gpr[rD(ctx->opcode)]);
>
> So it might be simpler to just leave out the helper stubs for now, and
> always return the invalid value from the generated code.

The idea was to retain all the translation work that we did as stubs.
And fill it whenever we have suitable implementation. Wouldn't hurt to
leave it like this. I am fine both ways.

Regards,
Nikunj
diff mbox

Patch

diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index e75d070..966f2ce 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -50,6 +50,8 @@  DEF_HELPER_FLAGS_1(cnttzd, TCG_CALL_NO_RWG_SE, tl, tl)
 DEF_HELPER_FLAGS_1(popcntd, TCG_CALL_NO_RWG_SE, tl, tl)
 DEF_HELPER_FLAGS_2(bpermd, TCG_CALL_NO_RWG_SE, i64, i64, i64)
 DEF_HELPER_3(srad, tl, env, tl, tl)
+DEF_HELPER_0(darn32, tl)
+DEF_HELPER_0(darn64, tl)
 #endif
 
 DEF_HELPER_FLAGS_1(cntlsw32, TCG_CALL_NO_RWG_SE, i32, i32)
diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index 291fba0..51a9ac5 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -182,6 +182,22 @@  target_ulong helper_cnttzd(target_ulong t)
 {
     return ctz64(t);
 }
+
+/* Return invalid random number.
+ *
+ * FIXME: Add rng backend or other mechanism to get cryptographically suitable
+ * random number
+ */
+target_ulong helper_darn32(void)
+{
+    return -1;
+}
+
+target_ulong helper_darn64(void)
+{
+    return -1;
+}
+
 #endif
 
 #if defined(TARGET_PPC64)
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 133c531..e9dad3f 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -528,6 +528,8 @@  EXTRACT_HELPER(FPW, 16, 1);
 
 /* addpcis */
 EXTRACT_HELPER_DXFORM(DX, 10, 6, 6, 5, 16, 1, 1, 0, 0)
+/* darn */
+EXTRACT_HELPER(L, 16, 2);
 
 /***                            Jump target decoding                       ***/
 /* Immediate address */
@@ -1895,6 +1897,21 @@  static void gen_cnttzd(DisasContext *ctx)
         gen_set_Rc0(ctx, cpu_gpr[rA(ctx->opcode)]);
     }
 }
+
+/* darn */
+static void gen_darn(DisasContext *ctx)
+{
+    int l = L(ctx->opcode);
+
+    if (l == 0) {
+        gen_helper_darn32(cpu_gpr[rD(ctx->opcode)]);
+    } else if (l <= 2) {
+        /* Return 64-bit random for both CRN and RRN */
+        gen_helper_darn64(cpu_gpr[rD(ctx->opcode)]);
+    } else {
+        tcg_gen_movi_i64(cpu_gpr[rD(ctx->opcode)], -1);
+    }
+}
 #endif
 
 /***                             Integer rotate                            ***/
@@ -6212,6 +6229,7 @@  GEN_HANDLER_E(prtyw, 0x1F, 0x1A, 0x04, 0x0000F801, PPC_NONE, PPC2_ISA205),
 GEN_HANDLER(popcntd, 0x1F, 0x1A, 0x0F, 0x0000F801, PPC_POPCNTWD),
 GEN_HANDLER(cntlzd, 0x1F, 0x1A, 0x01, 0x00000000, PPC_64B),
 GEN_HANDLER_E(cnttzd, 0x1F, 0x1A, 0x11, 0x00000000, PPC_NONE, PPC2_ISA300),
+GEN_HANDLER_E(darn, 0x1F, 0x13, 0x17, 0x001CF801, PPC_NONE, PPC2_ISA300),
 GEN_HANDLER_E(prtyd, 0x1F, 0x1A, 0x05, 0x0000F801, PPC_NONE, PPC2_ISA205),
 GEN_HANDLER_E(bpermd, 0x1F, 0x1C, 0x07, 0x00000001, PPC_NONE, PPC2_PERM_ISA206),
 #endif