| Message ID | 1401787684-31895-22-git-send-email-aik@ozlabs.ru |
|---|---|
| State | New |
| Headers | show |
On 6/3/2014 4:27 AM, Alexey Kardashevskiy wrote: > This makes user-privileged read/write fail if TAR facility is not enabled > in FSCR. > > Since this is the very first check for enabled in FSCR facility, > this also adds gen_fscr_facility_check() for using in spr_write_tar()/ > spr_read_tar(). > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > target-ppc/translate_init.c | 29 ++++++++++++++++++++++++++++- > 1 file changed, 28 insertions(+), 1 deletion(-) > > diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c > index 6f0c36b..9b83d56 100644 > --- a/target-ppc/translate_init.c > +++ b/target-ppc/translate_init.c > @@ -7275,6 +7275,21 @@ enum BOOK3S_CPU_TYPE { > BOOK3S_CPU_POWER8 > }; > > +static void gen_fscr_facility_check(void *opaque, int facility_sprn, int bit, > + int sprn, int cause) > +{ > + TCGv_i32 t1 = tcg_const_i32(bit); > + TCGv_i32 t2 = tcg_const_i32(sprn); > + TCGv_i32 t3 = tcg_const_i32(cause); > + > + gen_update_current_nip(opaque); > + gen_helper_fscr_facility_check(cpu_env, t1, t2, t3); > + > + tcg_temp_free_i32(t3); > + tcg_temp_free_i32(t2); > + tcg_temp_free_i32(t1); > +} > + > static int check_pow_970 (CPUPPCState *env) > { > if (env->spr[SPR_HID0] & 0x01C00000) { > @@ -7568,10 +7583,22 @@ static void gen_spr_power6_common(CPUPPCState *env) > 0x00000000); > } > > +static void spr_read_tar(void *opaque, int gprn, int sprn) > +{ > + gen_fscr_facility_check(opaque, SPR_FSCR, FSCR_TAR, sprn, FSCR_IC_TAR); > + spr_read_generic(opaque, gprn, sprn); > +} > + > +static void spr_write_tar(void *opaque, int sprn, int gprn) > +{ > + gen_fscr_facility_check(opaque, SPR_FSCR, FSCR_TAR, sprn, FSCR_IC_TAR); > + spr_write_generic(opaque, sprn, gprn); > +} > + > static void gen_spr_power8_tce_address_control(CPUPPCState *env) > { > spr_register(env, SPR_TAR, "TAR", > - &spr_read_generic, &spr_write_generic, > + &spr_read_tar, &spr_write_tar, > &spr_read_generic, &spr_write_generic, > 0x00000000); > } > There are potential impacts to user mode here. If I am reading correctly, TAR would not be accessible in user mode. An obvious fix would be to initialize FSCR to enable TAR access in the user mode build targets.
On 06/04/2014 03:08 AM, Tom Musta wrote: > On 6/3/2014 4:27 AM, Alexey Kardashevskiy wrote: >> This makes user-privileged read/write fail if TAR facility is not enabled >> in FSCR. >> >> Since this is the very first check for enabled in FSCR facility, >> this also adds gen_fscr_facility_check() for using in spr_write_tar()/ >> spr_read_tar(). >> >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >> --- >> target-ppc/translate_init.c | 29 ++++++++++++++++++++++++++++- >> 1 file changed, 28 insertions(+), 1 deletion(-) >> >> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c >> index 6f0c36b..9b83d56 100644 >> --- a/target-ppc/translate_init.c >> +++ b/target-ppc/translate_init.c >> @@ -7275,6 +7275,21 @@ enum BOOK3S_CPU_TYPE { >> BOOK3S_CPU_POWER8 >> }; >> >> +static void gen_fscr_facility_check(void *opaque, int facility_sprn, int bit, >> + int sprn, int cause) >> +{ >> + TCGv_i32 t1 = tcg_const_i32(bit); >> + TCGv_i32 t2 = tcg_const_i32(sprn); >> + TCGv_i32 t3 = tcg_const_i32(cause); >> + >> + gen_update_current_nip(opaque); >> + gen_helper_fscr_facility_check(cpu_env, t1, t2, t3); >> + >> + tcg_temp_free_i32(t3); >> + tcg_temp_free_i32(t2); >> + tcg_temp_free_i32(t1); >> +} >> + >> static int check_pow_970 (CPUPPCState *env) >> { >> if (env->spr[SPR_HID0] & 0x01C00000) { >> @@ -7568,10 +7583,22 @@ static void gen_spr_power6_common(CPUPPCState *env) >> 0x00000000); >> } >> >> +static void spr_read_tar(void *opaque, int gprn, int sprn) >> +{ >> + gen_fscr_facility_check(opaque, SPR_FSCR, FSCR_TAR, sprn, FSCR_IC_TAR); >> + spr_read_generic(opaque, gprn, sprn); >> +} >> + >> +static void spr_write_tar(void *opaque, int sprn, int gprn) >> +{ >> + gen_fscr_facility_check(opaque, SPR_FSCR, FSCR_TAR, sprn, FSCR_IC_TAR); >> + spr_write_generic(opaque, sprn, gprn); >> +} >> + >> static void gen_spr_power8_tce_address_control(CPUPPCState *env) >> { >> spr_register(env, SPR_TAR, "TAR", >> - &spr_read_generic, &spr_write_generic, >> + &spr_read_tar, &spr_write_tar, >> &spr_read_generic, &spr_write_generic, >> 0x00000000); >> } >> > > There are potential impacts to user mode here. If I am reading correctly, TAR would not be accessible > in user mode. And this is bad why exactly? I definitely need to learn about linux-user more... > An obvious fix would be to initialize FSCR to enable TAR access in the user mode build targets. Like that? static void gen_spr_power8_fscr(CPUPPCState *env) { +#if defined(CONFIG_USER_ONLY) + target_ulong initval = 1ULL << FSCR_TAR; +#else + target_ulong initval = 0; +#endif spr_register_kvm(env, SPR_FSCR, "FSCR", SPR_NOACCESS, SPR_NOACCESS, &spr_read_generic, &spr_write_generic, - KVM_REG_PPC_FSCR, 0x00000000); + KVM_REG_PPC_FSCR, initval); }
On 6/3/2014 9:37 PM, Alexey Kardashevskiy wrote: > On 06/04/2014 03:08 AM, Tom Musta wrote: >> On 6/3/2014 4:27 AM, Alexey Kardashevskiy wrote: >>> This makes user-privileged read/write fail if TAR facility is not enabled >>> in FSCR. [ ...] >>> >> >> There are potential impacts to user mode here. If I am reading correctly, TAR would not be accessible >> in user mode. > > > And this is bad why exactly? I definitely need to learn about linux-user > more... > Because TAR and bctar are Book I additions to ISA 2.07 and thus we can expect them to show up in applications. Since FSCR is not user-writeable, if FSCR[TAR] is initially zero, there is no means to enable access. Any application using bctar would not run. > >> An obvious fix would be to initialize FSCR to enable TAR access in the user mode build targets. > > > Like that? > > static void gen_spr_power8_fscr(CPUPPCState *env) > { > +#if defined(CONFIG_USER_ONLY) > + target_ulong initval = 1ULL << FSCR_TAR; > +#else > + target_ulong initval = 0; > +#endif > spr_register_kvm(env, SPR_FSCR, "FSCR", > SPR_NOACCESS, SPR_NOACCESS, > &spr_read_generic, &spr_write_generic, > - KVM_REG_PPC_FSCR, 0x00000000); > + KVM_REG_PPC_FSCR, initval); > } > > Yes. I believe that would work.
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c index 6f0c36b..9b83d56 100644 --- a/target-ppc/translate_init.c +++ b/target-ppc/translate_init.c @@ -7275,6 +7275,21 @@ enum BOOK3S_CPU_TYPE { BOOK3S_CPU_POWER8 }; +static void gen_fscr_facility_check(void *opaque, int facility_sprn, int bit, + int sprn, int cause) +{ + TCGv_i32 t1 = tcg_const_i32(bit); + TCGv_i32 t2 = tcg_const_i32(sprn); + TCGv_i32 t3 = tcg_const_i32(cause); + + gen_update_current_nip(opaque); + gen_helper_fscr_facility_check(cpu_env, t1, t2, t3); + + tcg_temp_free_i32(t3); + tcg_temp_free_i32(t2); + tcg_temp_free_i32(t1); +} + static int check_pow_970 (CPUPPCState *env) { if (env->spr[SPR_HID0] & 0x01C00000) { @@ -7568,10 +7583,22 @@ static void gen_spr_power6_common(CPUPPCState *env) 0x00000000); } +static void spr_read_tar(void *opaque, int gprn, int sprn) +{ + gen_fscr_facility_check(opaque, SPR_FSCR, FSCR_TAR, sprn, FSCR_IC_TAR); + spr_read_generic(opaque, gprn, sprn); +} + +static void spr_write_tar(void *opaque, int sprn, int gprn) +{ + gen_fscr_facility_check(opaque, SPR_FSCR, FSCR_TAR, sprn, FSCR_IC_TAR); + spr_write_generic(opaque, sprn, gprn); +} + static void gen_spr_power8_tce_address_control(CPUPPCState *env) { spr_register(env, SPR_TAR, "TAR", - &spr_read_generic, &spr_write_generic, + &spr_read_tar, &spr_write_tar, &spr_read_generic, &spr_write_generic, 0x00000000); }
This makes user-privileged read/write fail if TAR facility is not enabled in FSCR. Since this is the very first check for enabled in FSCR facility, this also adds gen_fscr_facility_check() for using in spr_write_tar()/ spr_read_tar(). Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- target-ppc/translate_init.c | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-)