| Message ID | 20180625160009.17437-3-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | support reading some CPUID/CNT registers from user-space | expand |
On 06/25/2018 09:00 AM, Alex Bennée wrote: > +#ifdef CONFIG_USER_ONLY > + /* Some AArch64 CPU ID/feature are exported to userspace > + * by the kernel (see HWCAP_CPUID) */ > + if (r->opc0 == 3 && r->crn == 0 && > + (r->crm == 0 || > + (r->crm >= 4 && r->crm <= 7))) { > + mask = PL0_R; > + break; > + } > +#endif Why not just set mask to PL0U_R | PL1_RW? This mask doesn't affect the actual permissions, just the check. Then of course merge with the next patch. r~
On 25 June 2018 at 17:00, Alex Bennée <alex.bennee@linaro.org> wrote: > Although technically not visible to userspace the kernel does make > them visible via trap and emulate. For user mode we can provide the > value directly but we need to relax our permission checks to do this. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > target/arm/helper.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/target/arm/helper.c b/target/arm/helper.c > index 6e6b1762e8..9d81feb124 100644 > --- a/target/arm/helper.c > +++ b/target/arm/helper.c > @@ -5813,7 +5813,19 @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu, > if (r->state != ARM_CP_STATE_AA32) { > int mask = 0; > switch (r->opc1) { > - case 0: case 1: case 2: > + case 0: > +#ifdef CONFIG_USER_ONLY > + /* Some AArch64 CPU ID/feature are exported to userspace > + * by the kernel (see HWCAP_CPUID) */ > + if (r->opc0 == 3 && r->crn == 0 && > + (r->crm == 0 || > + (r->crm >= 4 && r->crm <= 7))) { > + mask = PL0_R; > + break; > + } > +#endif > + /* fall-through */ > + case 1: case 2: > /* min_EL EL1 */ > mask = PL1_RW; > break; This looks like a rather inelegant place to shove a CONFIG_USER_ONLY special case. Isn't there a cleaner way to do whatever this is trying to achieve? thanks -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On 25 June 2018 at 17:00, Alex Bennée <alex.bennee@linaro.org> wrote: >> Although technically not visible to userspace the kernel does make >> them visible via trap and emulate. For user mode we can provide the >> value directly but we need to relax our permission checks to do this. >> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> --- >> target/arm/helper.c | 14 +++++++++++++- >> 1 file changed, 13 insertions(+), 1 deletion(-) >> >> diff --git a/target/arm/helper.c b/target/arm/helper.c >> index 6e6b1762e8..9d81feb124 100644 >> --- a/target/arm/helper.c >> +++ b/target/arm/helper.c >> @@ -5813,7 +5813,19 @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu, >> if (r->state != ARM_CP_STATE_AA32) { >> int mask = 0; >> switch (r->opc1) { >> - case 0: case 1: case 2: >> + case 0: >> +#ifdef CONFIG_USER_ONLY >> + /* Some AArch64 CPU ID/feature are exported to userspace >> + * by the kernel (see HWCAP_CPUID) */ >> + if (r->opc0 == 3 && r->crn == 0 && >> + (r->crm == 0 || >> + (r->crm >= 4 && r->crm <= 7))) { >> + mask = PL0_R; >> + break; >> + } >> +#endif >> + /* fall-through */ >> + case 1: case 2: >> /* min_EL EL1 */ >> mask = PL1_RW; >> break; > > This looks like a rather inelegant place to shove a CONFIG_USER_ONLY > special case. Isn't there a cleaner way to do whatever this is trying > to achieve? Well technically those registers aren't accessible to user space and this is a sanity check to ensure we don't accidentally make them accessible. But it does get in the way of emulating the traps for USER_ONLY. > > thanks > -- PMM -- Alex Bennée
diff --git a/target/arm/helper.c b/target/arm/helper.c index 6e6b1762e8..9d81feb124 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -5813,7 +5813,19 @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu, if (r->state != ARM_CP_STATE_AA32) { int mask = 0; switch (r->opc1) { - case 0: case 1: case 2: + case 0: +#ifdef CONFIG_USER_ONLY + /* Some AArch64 CPU ID/feature are exported to userspace + * by the kernel (see HWCAP_CPUID) */ + if (r->opc0 == 3 && r->crn == 0 && + (r->crm == 0 || + (r->crm >= 4 && r->crm <= 7))) { + mask = PL0_R; + break; + } +#endif + /* fall-through */ + case 1: case 2: /* min_EL EL1 */ mask = PL1_RW; break;
Although technically not visible to userspace the kernel does make them visible via trap and emulate. For user mode we can provide the value directly but we need to relax our permission checks to do this. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- target/arm/helper.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) -- 2.17.1