Re: [Qemu-devel] [PATCH v5 22/24] target/arm: Implement ARMv8.5-RNG
On 5/10/19 9:20 AM, Peter Maydell wrote: > On Fri, 10 May 2019 at 17:17, Richard Henderson > wrote: >> >> On 5/10/19 9:01 AM, Peter Maydell wrote: >>> Don't these need to be marked ARM_CP_IO for the benefit >>> of -icount ? >> >> I don't think so. There's no lock taken, as for mmio devices. It's not not >> related to time, virtual or otherwise. There are no possible exceptions. I >> can't think of anything that would make icount care. >> >> Have I missed something? > > If icount decides it needs to replay execution of the TB > which the RNDR access is in, won't it get a different > number back the second time it executes ? Yes, it will. I forgot about replay. r~
Re: [Qemu-devel] [PATCH v5 22/24] target/arm: Implement ARMv8.5-RNG
On 5/10/19 9:01 AM, Peter Maydell wrote:
> On Fri, 10 May 2019 at 02:25, Richard Henderson
> wrote:
>>
>> Cc: qemu-...@nongnu.org
>> Cc: Peter Maydell
>> Signed-off-by: Richard Henderson
>> ---
>> v3: Log errors with -d unimp, for lack of a better flag.
>> ---
>> target/arm/cpu.h| 5 +
>> target/arm/cpu64.c | 1 +
>> target/arm/helper.c | 44
>> 3 files changed, 50 insertions(+)
>
>> +/* We do not support re-seeding, so the two registers operate the same. */
>> +static const ARMCPRegInfo rndr_reginfo[] = {
>> +{ .name = "RNDR", .state = ARM_CP_STATE_AA64,
>> + .type = ARM_CP_NO_RAW | ARM_CP_SUPPRESS_TB_END,
>> + .opc0 = 3, .opc1 = 3, .crn = 2, .crm = 4, .opc2 = 0,
>> + .access = PL0_R, .readfn = rndr_readfn },
>> +{ .name = "RNDRRS", .state = ARM_CP_STATE_AA64,
>> + .type = ARM_CP_NO_RAW | ARM_CP_SUPPRESS_TB_END,
>> + .opc0 = 3, .opc1 = 3, .crn = 2, .crm = 4, .opc2 = 1,
>> + .access = PL0_R, .readfn = rndr_readfn },
>
> Don't these need to be marked ARM_CP_IO for the benefit
> of -icount ?
I don't think so. There's no lock taken, as for mmio devices. It's not not
related to time, virtual or otherwise. There are no possible exceptions. I
can't think of anything that would make icount care.
Have I missed something?
r~
Re: [Qemu-devel] [PATCH v5 22/24] target/arm: Implement ARMv8.5-RNG
On Fri, 10 May 2019 at 17:17, Richard Henderson wrote: > > On 5/10/19 9:01 AM, Peter Maydell wrote: > > Don't these need to be marked ARM_CP_IO for the benefit > > of -icount ? > > I don't think so. There's no lock taken, as for mmio devices. It's not not > related to time, virtual or otherwise. There are no possible exceptions. I > can't think of anything that would make icount care. > > Have I missed something? If icount decides it needs to replay execution of the TB which the RNDR access is in, won't it get a different number back the second time it executes ? thanks -- PMM
Re: [Qemu-devel] [PATCH v5 22/24] target/arm: Implement ARMv8.5-RNG
On Fri, 10 May 2019 at 02:25, Richard Henderson
wrote:
>
> Cc: qemu-...@nongnu.org
> Cc: Peter Maydell
> Signed-off-by: Richard Henderson
> ---
> v3: Log errors with -d unimp, for lack of a better flag.
> ---
> target/arm/cpu.h| 5 +
> target/arm/cpu64.c | 1 +
> target/arm/helper.c | 44
> 3 files changed, 50 insertions(+)
> +/* We do not support re-seeding, so the two registers operate the same. */
> +static const ARMCPRegInfo rndr_reginfo[] = {
> +{ .name = "RNDR", .state = ARM_CP_STATE_AA64,
> + .type = ARM_CP_NO_RAW | ARM_CP_SUPPRESS_TB_END,
> + .opc0 = 3, .opc1 = 3, .crn = 2, .crm = 4, .opc2 = 0,
> + .access = PL0_R, .readfn = rndr_readfn },
> +{ .name = "RNDRRS", .state = ARM_CP_STATE_AA64,
> + .type = ARM_CP_NO_RAW | ARM_CP_SUPPRESS_TB_END,
> + .opc0 = 3, .opc1 = 3, .crn = 2, .crm = 4, .opc2 = 1,
> + .access = PL0_R, .readfn = rndr_readfn },
Don't these need to be marked ARM_CP_IO for the benefit
of -icount ?
Otherwise
Reviewed-by: Peter Maydell
thanks
-- PMM
[Qemu-devel] [PATCH v5 22/24] target/arm: Implement ARMv8.5-RNG
Cc: qemu-...@nongnu.org
Cc: Peter Maydell
Signed-off-by: Richard Henderson
---
v3: Log errors with -d unimp, for lack of a better flag.
---
target/arm/cpu.h| 5 +
target/arm/cpu64.c | 1 +
target/arm/helper.c | 44
3 files changed, 50 insertions(+)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 892f9a4ad2..c34207611b 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3521,6 +3521,11 @@ static inline bool isar_feature_aa64_condm_5(const
ARMISARegisters *id)
return FIELD_EX64(id->id_aa64isar0, ID_AA64ISAR0, TS) >= 2;
}
+static inline bool isar_feature_aa64_rndr(const ARMISARegisters *id)
+{
+return FIELD_EX64(id->id_aa64isar0, ID_AA64ISAR0, RNDR) != 0;
+}
+
static inline bool isar_feature_aa64_jscvt(const ARMISARegisters *id)
{
return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, JSCVT) != 0;
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 228906f267..835f73cceb 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -310,6 +310,7 @@ static void aarch64_max_initfn(Object *obj)
t = FIELD_DP64(t, ID_AA64ISAR0, DP, 1);
t = FIELD_DP64(t, ID_AA64ISAR0, FHM, 1);
t = FIELD_DP64(t, ID_AA64ISAR0, TS, 2); /* v8.5-CondM */
+t = FIELD_DP64(t, ID_AA64ISAR0, RNDR, 1);
cpu->isar.id_aa64isar0 = t;
t = cpu->isar.id_aa64isar1;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 7e88b2cadd..9642070194 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -22,6 +22,8 @@
#include "fpu/softfloat.h"
#include "qemu/range.h"
#include "qapi/qapi-commands-target.h"
+#include "qapi/error.h"
+#include "qemu/guest-random.h"
#define ARM_CPU_FREQ 10 /* FIXME: 1 GHz, should be configurable */
@@ -5746,6 +5748,45 @@ static const ARMCPRegInfo pauth_reginfo[] = {
.fieldoffset = offsetof(CPUARMState, keys.apib.hi) },
REGINFO_SENTINEL
};
+
+static uint64_t rndr_readfn(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+Error *err = NULL;
+uint64_t ret;
+
+/* Success sets NZCV = . */
+env->NF = env->CF = env->VF = 0, env->ZF = 1;
+
+if (qemu_guest_getrandom(, sizeof(ret), ) < 0) {
+/*
+ * ??? Failed, for unknown reasons in the crypto subsystem.
+ * The best we can do is log the reason and return the
+ * timed-out indication to the guest. There is no reason
+ * we know to expect this failure to be transitory, so the
+ * guest may well hang retrying the operation.
+ */
+qemu_log_mask(LOG_UNIMP, "%s: Crypto failure: %s",
+ ri->name, error_get_pretty(err));
+error_free(err);
+
+env->ZF = 0; /* NZCF = 0100 */
+return 0;
+}
+return ret;
+}
+
+/* We do not support re-seeding, so the two registers operate the same. */
+static const ARMCPRegInfo rndr_reginfo[] = {
+{ .name = "RNDR", .state = ARM_CP_STATE_AA64,
+ .type = ARM_CP_NO_RAW | ARM_CP_SUPPRESS_TB_END,
+ .opc0 = 3, .opc1 = 3, .crn = 2, .crm = 4, .opc2 = 0,
+ .access = PL0_R, .readfn = rndr_readfn },
+{ .name = "RNDRRS", .state = ARM_CP_STATE_AA64,
+ .type = ARM_CP_NO_RAW | ARM_CP_SUPPRESS_TB_END,
+ .opc0 = 3, .opc1 = 3, .crn = 2, .crm = 4, .opc2 = 1,
+ .access = PL0_R, .readfn = rndr_readfn },
+REGINFO_SENTINEL
+};
#endif
static CPAccessResult access_predinv(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -6690,6 +6731,9 @@ void register_cp_regs_for_features(ARMCPU *cpu)
if (cpu_isar_feature(aa64_pauth, cpu)) {
define_arm_cp_regs(cpu, pauth_reginfo);
}
+if (cpu_isar_feature(aa64_rndr, cpu)) {
+define_arm_cp_regs(cpu, rndr_reginfo);
+}
#endif
/*
--
2.17.1
