| Message ID | 20190510012458.22706-23-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add qemu_getrandom and ARMv8.5-RNG etc | expand |
On Fri, 10 May 2019 at 02:25, Richard Henderson <richard.henderson@linaro.org> wrote: > > Cc: qemu-arm@nongnu.org > Cc: Peter Maydell <peter.maydell@linaro.org> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > 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 <peter.maydell@linaro.org> thanks -- PMM
On 5/10/19 9:01 AM, Peter Maydell wrote: > On Fri, 10 May 2019 at 02:25, Richard Henderson > <richard.henderson@linaro.org> wrote: >> >> Cc: qemu-arm@nongnu.org >> Cc: Peter Maydell <peter.maydell@linaro.org> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> 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~
On Fri, 10 May 2019 at 17:17, Richard Henderson <richard.henderson@linaro.org> 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
On 5/10/19 9:20 AM, Peter Maydell wrote: > On Fri, 10 May 2019 at 17:17, Richard Henderson > <richard.henderson@linaro.org> 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~
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 1000000000 /* 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 = 0000. */ + env->NF = env->CF = env->VF = 0, env->ZF = 1; + + if (qemu_guest_getrandom(&ret, sizeof(ret), &err) < 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 /*
Cc: qemu-arm@nongnu.org Cc: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- 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(+) -- 2.17.1