| Message ID | 1391183143-30724-12-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Sat, Feb 1, 2014 at 1:45 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > Several of the system registers handled via the ARMCPRegInfo > mechanism have access trap control bits controlling whether the > registers are accessible to lower privilege levels. Replace > the existing mechanism (allowing the read and write functions > to return EXCP_UDEF if access is denied) with a dedicated > "check access rights" function pointer in the ARMCPRegInfo. > This will allow us to simplify some of the register definitions, > which no longer need read/write functions purely to handle > the access checks. > > We take the opportunity to define the return value from the > access checking function in a way that allows us to set the > correct exception syndrome information for exceptions taken > to AArch64 (which may need to distinguish access failures due > to a configurable trap or enable from other kinds of access > failure). > > This commit defines the new mechanism but does not move any > of the registers across to use it. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > target-arm/cpu.h | 29 +++++++++++++++++++++++++---- > target-arm/helper.h | 1 + > target-arm/op_helper.c | 18 ++++++++++++++++++ > target-arm/translate-a64.c | 11 +++++++++++ > target-arm/translate.c | 11 +++++++++++ > 5 files changed, 66 insertions(+), 4 deletions(-) > > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > index e66d464..30b1a1c 100644 > --- a/target-arm/cpu.h > +++ b/target-arm/cpu.h > @@ -812,14 +812,29 @@ static inline int arm_current_pl(CPUARMState *env) > > typedef struct ARMCPRegInfo ARMCPRegInfo; > > -/* Access functions for coprocessor registers. These should return > - * 0 on success, or one of the EXCP_* constants if access should cause > - * an exception (in which case *value is not written). > - */ > +typedef enum CPAccessResult { > + /* Access is permitted */ > + CP_ACCESS_OK = 0, > + /* Access fails due to a configurable trap or enable which would > + * result in a categorized exception syndrome giving information about > + * the failing instruction (ie syndrome category 0x3, 0x4, 0x5, 0x6, > + * 0xc or 0x18). > + */ > + CP_ACCESS_TRAP = 1, > + /* Access fails and results in an exception syndrome 0x0 ("uncategorized"). > + * Note that this is not a catch-all case -- the set of cases which may > + * result in this failure is specifically defined by the architecture. > + */ > + CP_ACCESS_TRAP_UNCATEGORIZED = 2, Hi Peter, So its not obvious to me just yet why these two types or traps needs separate encoding on this level. From your commentary, the two different types of traps are mutually exclusive and identifyable by the syndrome information. I.E _TRAP is syndrome != 0 and _TRAP_UNCAT is syndrome == 0. Cant the access checkers return boolean if is_trap then the syndrome can be used to make this distinction? Regards, Peter > +} CPAccessResult; > + > +/* Access functions for coprocessor registers. These should always succeed. */ > typedef int CPReadFn(CPUARMState *env, const ARMCPRegInfo *opaque, > uint64_t *value); > typedef int CPWriteFn(CPUARMState *env, const ARMCPRegInfo *opaque, > uint64_t value); > +/* Access permission check functions for coprocessor registers. */ > +typedef CPAccessResult CPAccessFn(CPUARMState *env, const ARMCPRegInfo *opaque); > /* Hook function for register reset */ > typedef void CPResetFn(CPUARMState *env, const ARMCPRegInfo *opaque); > > @@ -873,6 +888,12 @@ struct ARMCPRegInfo { > * 2. both readfn and writefn are specified > */ > ptrdiff_t fieldoffset; /* offsetof(CPUARMState, field) */ > + /* Function for making any access checks for this register in addition to > + * those specified by the 'access' permissions bits. If NULL, no extra > + * checks required. The access check is performed at runtime, not at > + * translate time. > + */ > + CPAccessFn *accessfn; > /* Function for handling reads of this register. If NULL, then reads > * will be done by loading from the offset into CPUARMState specified > * by fieldoffset. > diff --git a/target-arm/helper.h b/target-arm/helper.h > index 93a27ce..1d83293 100644 > --- a/target-arm/helper.h > +++ b/target-arm/helper.h > @@ -57,6 +57,7 @@ DEF_HELPER_1(cpsr_read, i32, env) > DEF_HELPER_3(v7m_msr, void, env, i32, i32) > DEF_HELPER_2(v7m_mrs, i32, env, i32) > > +DEF_HELPER_2(access_check_cp_reg, void, env, ptr) > DEF_HELPER_3(set_cp_reg, void, env, ptr, i32) > DEF_HELPER_2(get_cp_reg, i32, env, ptr) > DEF_HELPER_3(set_cp_reg64, void, env, ptr, i64) > diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c > index c812a9f..89b0978 100644 > --- a/target-arm/op_helper.c > +++ b/target-arm/op_helper.c > @@ -273,6 +273,24 @@ void HELPER(set_user_reg)(CPUARMState *env, uint32_t regno, uint32_t val) > } > } > > +void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip) > +{ > + const ARMCPRegInfo *ri = rip; > + switch (ri->accessfn(env, ri)) { > + case CP_ACCESS_OK: > + return; > + case CP_ACCESS_TRAP: > + case CP_ACCESS_TRAP_UNCATEGORIZED: > + /* These cases will eventually need to generate different > + * syndrome information. > + */ > + break; > + default: > + g_assert_not_reached(); > + } > + raise_exception(env, EXCP_UDEF); > +} > + > void HELPER(set_cp_reg)(CPUARMState *env, void *rip, uint32_t value) > { > const ARMCPRegInfo *ri = rip; > diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c > index a942609..d90ffd1 100644 > --- a/target-arm/translate-a64.c > +++ b/target-arm/translate-a64.c > @@ -1210,6 +1210,17 @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread, > return; > } > > + if (ri->accessfn) { > + /* Emit code to perform further access permissions checks at > + * runtime; this may result in an exception. > + */ > + TCGv_ptr tmpptr; > + gen_a64_set_pc_im(s->pc - 4); > + tmpptr = tcg_const_ptr(ri); > + gen_helper_access_check_cp_reg(cpu_env, tmpptr); > + tcg_temp_free_ptr(tmpptr); > + } > + > /* Handle special cases first */ > switch (ri->type & ~(ARM_CP_FLAG_MASK & ~ARM_CP_SPECIAL)) { > case ARM_CP_NOP: > diff --git a/target-arm/translate.c b/target-arm/translate.c > index 45886f9..2713081 100644 > --- a/target-arm/translate.c > +++ b/target-arm/translate.c > @@ -6798,6 +6798,17 @@ static int disas_coproc_insn(CPUARMState * env, DisasContext *s, uint32_t insn) > return 1; > } > > + if (ri->accessfn) { > + /* Emit code to perform further access permissions checks at > + * runtime; this may result in an exception. > + */ > + TCGv_ptr tmpptr; > + gen_set_pc_im(s, s->pc); > + tmpptr = tcg_const_ptr(ri); > + gen_helper_access_check_cp_reg(cpu_env, tmpptr); > + tcg_temp_free_ptr(tmpptr); > + } > + > /* Handle special cases first */ > switch (ri->type & ~(ARM_CP_FLAG_MASK & ~ARM_CP_SPECIAL)) { > case ARM_CP_NOP: > -- > 1.8.5 > >
On 9 February 2014 02:50, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > On Sat, Feb 1, 2014 at 1:45 AM, Peter Maydell <peter.maydell@linaro.org> wrote: >> +typedef enum CPAccessResult { >> + /* Access is permitted */ >> + CP_ACCESS_OK = 0, >> + /* Access fails due to a configurable trap or enable which would >> + * result in a categorized exception syndrome giving information about >> + * the failing instruction (ie syndrome category 0x3, 0x4, 0x5, 0x6, >> + * 0xc or 0x18). >> + */ >> + CP_ACCESS_TRAP = 1, >> + /* Access fails and results in an exception syndrome 0x0 ("uncategorized"). >> + * Note that this is not a catch-all case -- the set of cases which may >> + * result in this failure is specifically defined by the architecture. >> + */ >> + CP_ACCESS_TRAP_UNCATEGORIZED = 2, > > Hi Peter, > > So its not obvious to me just yet why these two types or traps needs > separate encoding on this level. From your commentary, the two > different types of traps are mutually exclusive and identifyable by > the syndrome information. I.E _TRAP is syndrome != 0 and _TRAP_UNCAT > is syndrome == 0. Cant the access checkers return boolean if is_trap > then the syndrome can be used to make this distinction? No, because the access checker is the thing that tells us what the syndrome register is supposed to be. That is, the caller of the access functions will do something like: ret = ri->accessfn(env, ri); switch (ret) { case CP_ACCESS_OK: return; case CP_ACCESS_TRAP: env->exception.syndrome = syn_cp_trap(stuff); break; case CP_ACCESS_TRAP_UNCATEGORIZED: env->exception_syndrome = syn_uncategorized(); break; } raise_exception(EXCP_UDEF); (This code is in the syndrome work I have which sits on top of this patchset; it's pretty nearly ready to post but I have a few odds and ends to tie off first.) The actual value of the syndrome register depends on a pile of stuff that includes various fields from the instruction, so we don't want to burden the access checkers with actually setting or returning us a syndrome value. (Also the syndrome encoding covers the whole uint32_t space so it's not possible to return "syndrome or some value meaning no exception", which makes that an awkward API choice anyway.) So we just have the checker tell us "OK, or this set of syndromes, or that set?" and we can calculate the syndrome from just that three-way bit of information. The phrasing of the comments above is supposed to help somebody implementing a cpreg know what they should be returning : the ARM ARM docs will let them know which kind of syndrome category the access failure is, which in turn indicates which CP_ value to return to cause QEMU to generate the syndrome information that is required. thanks -- PMM
On Sat, Feb 1, 2014 at 1:45 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > Several of the system registers handled via the ARMCPRegInfo > mechanism have access trap control bits controlling whether the > registers are accessible to lower privilege levels. Replace > the existing mechanism (allowing the read and write functions > to return EXCP_UDEF if access is denied) with a dedicated > "check access rights" function pointer in the ARMCPRegInfo. > This will allow us to simplify some of the register definitions, > which no longer need read/write functions purely to handle > the access checks. > > We take the opportunity to define the return value from the > access checking function in a way that allows us to set the > correct exception syndrome information for exceptions taken > to AArch64 (which may need to distinguish access failures due > to a configurable trap or enable from other kinds of access > failure). > > This commit defines the new mechanism but does not move any > of the registers across to use it. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > --- > target-arm/cpu.h | 29 +++++++++++++++++++++++++---- > target-arm/helper.h | 1 + > target-arm/op_helper.c | 18 ++++++++++++++++++ > target-arm/translate-a64.c | 11 +++++++++++ > target-arm/translate.c | 11 +++++++++++ > 5 files changed, 66 insertions(+), 4 deletions(-) > > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > index e66d464..30b1a1c 100644 > --- a/target-arm/cpu.h > +++ b/target-arm/cpu.h > @@ -812,14 +812,29 @@ static inline int arm_current_pl(CPUARMState *env) > > typedef struct ARMCPRegInfo ARMCPRegInfo; > > -/* Access functions for coprocessor registers. These should return > - * 0 on success, or one of the EXCP_* constants if access should cause > - * an exception (in which case *value is not written). > - */ > +typedef enum CPAccessResult { > + /* Access is permitted */ > + CP_ACCESS_OK = 0, > + /* Access fails due to a configurable trap or enable which would > + * result in a categorized exception syndrome giving information about > + * the failing instruction (ie syndrome category 0x3, 0x4, 0x5, 0x6, > + * 0xc or 0x18). > + */ > + CP_ACCESS_TRAP = 1, > + /* Access fails and results in an exception syndrome 0x0 ("uncategorized"). > + * Note that this is not a catch-all case -- the set of cases which may > + * result in this failure is specifically defined by the architecture. > + */ > + CP_ACCESS_TRAP_UNCATEGORIZED = 2, > +} CPAccessResult; > + > +/* Access functions for coprocessor registers. These should always succeed. */ > typedef int CPReadFn(CPUARMState *env, const ARMCPRegInfo *opaque, > uint64_t *value); > typedef int CPWriteFn(CPUARMState *env, const ARMCPRegInfo *opaque, > uint64_t value); > +/* Access permission check functions for coprocessor registers. */ > +typedef CPAccessResult CPAccessFn(CPUARMState *env, const ARMCPRegInfo *opaque); > /* Hook function for register reset */ > typedef void CPResetFn(CPUARMState *env, const ARMCPRegInfo *opaque); > > @@ -873,6 +888,12 @@ struct ARMCPRegInfo { > * 2. both readfn and writefn are specified > */ > ptrdiff_t fieldoffset; /* offsetof(CPUARMState, field) */ > + /* Function for making any access checks for this register in addition to > + * those specified by the 'access' permissions bits. If NULL, no extra > + * checks required. The access check is performed at runtime, not at > + * translate time. > + */ > + CPAccessFn *accessfn; > /* Function for handling reads of this register. If NULL, then reads > * will be done by loading from the offset into CPUARMState specified > * by fieldoffset. > diff --git a/target-arm/helper.h b/target-arm/helper.h > index 93a27ce..1d83293 100644 > --- a/target-arm/helper.h > +++ b/target-arm/helper.h > @@ -57,6 +57,7 @@ DEF_HELPER_1(cpsr_read, i32, env) > DEF_HELPER_3(v7m_msr, void, env, i32, i32) > DEF_HELPER_2(v7m_mrs, i32, env, i32) > > +DEF_HELPER_2(access_check_cp_reg, void, env, ptr) > DEF_HELPER_3(set_cp_reg, void, env, ptr, i32) > DEF_HELPER_2(get_cp_reg, i32, env, ptr) > DEF_HELPER_3(set_cp_reg64, void, env, ptr, i64) > diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c > index c812a9f..89b0978 100644 > --- a/target-arm/op_helper.c > +++ b/target-arm/op_helper.c > @@ -273,6 +273,24 @@ void HELPER(set_user_reg)(CPUARMState *env, uint32_t regno, uint32_t val) > } > } > > +void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip) > +{ > + const ARMCPRegInfo *ri = rip; > + switch (ri->accessfn(env, ri)) { > + case CP_ACCESS_OK: > + return; > + case CP_ACCESS_TRAP: > + case CP_ACCESS_TRAP_UNCATEGORIZED: > + /* These cases will eventually need to generate different > + * syndrome information. > + */ > + break; > + default: > + g_assert_not_reached(); > + } > + raise_exception(env, EXCP_UDEF); > +} > + > void HELPER(set_cp_reg)(CPUARMState *env, void *rip, uint32_t value) > { > const ARMCPRegInfo *ri = rip; > diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c > index a942609..d90ffd1 100644 > --- a/target-arm/translate-a64.c > +++ b/target-arm/translate-a64.c > @@ -1210,6 +1210,17 @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread, > return; > } > > + if (ri->accessfn) { > + /* Emit code to perform further access permissions checks at > + * runtime; this may result in an exception. > + */ > + TCGv_ptr tmpptr; > + gen_a64_set_pc_im(s->pc - 4); > + tmpptr = tcg_const_ptr(ri); > + gen_helper_access_check_cp_reg(cpu_env, tmpptr); > + tcg_temp_free_ptr(tmpptr); > + } > + > /* Handle special cases first */ > switch (ri->type & ~(ARM_CP_FLAG_MASK & ~ARM_CP_SPECIAL)) { > case ARM_CP_NOP: > diff --git a/target-arm/translate.c b/target-arm/translate.c > index 45886f9..2713081 100644 > --- a/target-arm/translate.c > +++ b/target-arm/translate.c > @@ -6798,6 +6798,17 @@ static int disas_coproc_insn(CPUARMState * env, DisasContext *s, uint32_t insn) > return 1; > } > > + if (ri->accessfn) { > + /* Emit code to perform further access permissions checks at > + * runtime; this may result in an exception. > + */ > + TCGv_ptr tmpptr; > + gen_set_pc_im(s, s->pc); > + tmpptr = tcg_const_ptr(ri); > + gen_helper_access_check_cp_reg(cpu_env, tmpptr); > + tcg_temp_free_ptr(tmpptr); > + } > + > /* Handle special cases first */ > switch (ri->type & ~(ARM_CP_FLAG_MASK & ~ARM_CP_SPECIAL)) { > case ARM_CP_NOP: > -- > 1.8.5 > >
On Sun, Feb 9, 2014 at 10:02 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 9 February 2014 02:50, Peter Crosthwaite > <peter.crosthwaite@xilinx.com> wrote: >> On Sat, Feb 1, 2014 at 1:45 AM, Peter Maydell <peter.maydell@linaro.org> wrote: >>> +typedef enum CPAccessResult { >>> + /* Access is permitted */ >>> + CP_ACCESS_OK = 0, >>> + /* Access fails due to a configurable trap or enable which would >>> + * result in a categorized exception syndrome giving information about >>> + * the failing instruction (ie syndrome category 0x3, 0x4, 0x5, 0x6, >>> + * 0xc or 0x18). >>> + */ >>> + CP_ACCESS_TRAP = 1, >>> + /* Access fails and results in an exception syndrome 0x0 ("uncategorized"). >>> + * Note that this is not a catch-all case -- the set of cases which may >>> + * result in this failure is specifically defined by the architecture. >>> + */ >>> + CP_ACCESS_TRAP_UNCATEGORIZED = 2, >> >> Hi Peter, >> >> So its not obvious to me just yet why these two types or traps needs >> separate encoding on this level. From your commentary, the two >> different types of traps are mutually exclusive and identifyable by >> the syndrome information. I.E _TRAP is syndrome != 0 and _TRAP_UNCAT >> is syndrome == 0. Cant the access checkers return boolean if is_trap >> then the syndrome can be used to make this distinction? > > No, because the access checker is the thing that tells us what the > syndrome register is supposed to be. That is, the caller of the access > functions will do something like: > > ret = ri->accessfn(env, ri); > switch (ret) { > case CP_ACCESS_OK: > return; > case CP_ACCESS_TRAP: > env->exception.syndrome = syn_cp_trap(stuff); > break; > case CP_ACCESS_TRAP_UNCATEGORIZED: > env->exception_syndrome = syn_uncategorized(); > break; > } > raise_exception(EXCP_UDEF); > > (This code is in the syndrome work I have which sits on top of this > patchset; it's pretty nearly ready to post but I have a few odds and > ends to tie off first.) > > The actual value of the syndrome register depends on a pile > of stuff that includes various fields from the instruction, so we > don't want to burden the access checkers with actually setting > or returning us a syndrome value. (Also the syndrome encoding > covers the whole uint32_t space so it's not possible to return > "syndrome or some value meaning no exception", which makes > that an awkward API choice anyway.) So we just have the checker > tell us "OK, or this set of syndromes, or that set?" and we can calculate > the syndrome from just that three-way bit of information. > > The phrasing of the comments above is supposed to help somebody > implementing a cpreg know what they should be returning : the > ARM ARM docs will let them know which kind of syndrome category > the access failure is, which in turn indicates which CP_ value to > return to cause QEMU to generate the syndrome information that > is required. > Thanks for the clarification. I was assuming that checkers were going to do syndromes. Regards, Peter > thanks > -- PMM >
diff --git a/target-arm/cpu.h b/target-arm/cpu.h index e66d464..30b1a1c 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -812,14 +812,29 @@ static inline int arm_current_pl(CPUARMState *env) typedef struct ARMCPRegInfo ARMCPRegInfo; -/* Access functions for coprocessor registers. These should return - * 0 on success, or one of the EXCP_* constants if access should cause - * an exception (in which case *value is not written). - */ +typedef enum CPAccessResult { + /* Access is permitted */ + CP_ACCESS_OK = 0, + /* Access fails due to a configurable trap or enable which would + * result in a categorized exception syndrome giving information about + * the failing instruction (ie syndrome category 0x3, 0x4, 0x5, 0x6, + * 0xc or 0x18). + */ + CP_ACCESS_TRAP = 1, + /* Access fails and results in an exception syndrome 0x0 ("uncategorized"). + * Note that this is not a catch-all case -- the set of cases which may + * result in this failure is specifically defined by the architecture. + */ + CP_ACCESS_TRAP_UNCATEGORIZED = 2, +} CPAccessResult; + +/* Access functions for coprocessor registers. These should always succeed. */ typedef int CPReadFn(CPUARMState *env, const ARMCPRegInfo *opaque, uint64_t *value); typedef int CPWriteFn(CPUARMState *env, const ARMCPRegInfo *opaque, uint64_t value); +/* Access permission check functions for coprocessor registers. */ +typedef CPAccessResult CPAccessFn(CPUARMState *env, const ARMCPRegInfo *opaque); /* Hook function for register reset */ typedef void CPResetFn(CPUARMState *env, const ARMCPRegInfo *opaque); @@ -873,6 +888,12 @@ struct ARMCPRegInfo { * 2. both readfn and writefn are specified */ ptrdiff_t fieldoffset; /* offsetof(CPUARMState, field) */ + /* Function for making any access checks for this register in addition to + * those specified by the 'access' permissions bits. If NULL, no extra + * checks required. The access check is performed at runtime, not at + * translate time. + */ + CPAccessFn *accessfn; /* Function for handling reads of this register. If NULL, then reads * will be done by loading from the offset into CPUARMState specified * by fieldoffset. diff --git a/target-arm/helper.h b/target-arm/helper.h index 93a27ce..1d83293 100644 --- a/target-arm/helper.h +++ b/target-arm/helper.h @@ -57,6 +57,7 @@ DEF_HELPER_1(cpsr_read, i32, env) DEF_HELPER_3(v7m_msr, void, env, i32, i32) DEF_HELPER_2(v7m_mrs, i32, env, i32) +DEF_HELPER_2(access_check_cp_reg, void, env, ptr) DEF_HELPER_3(set_cp_reg, void, env, ptr, i32) DEF_HELPER_2(get_cp_reg, i32, env, ptr) DEF_HELPER_3(set_cp_reg64, void, env, ptr, i64) diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c index c812a9f..89b0978 100644 --- a/target-arm/op_helper.c +++ b/target-arm/op_helper.c @@ -273,6 +273,24 @@ void HELPER(set_user_reg)(CPUARMState *env, uint32_t regno, uint32_t val) } } +void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip) +{ + const ARMCPRegInfo *ri = rip; + switch (ri->accessfn(env, ri)) { + case CP_ACCESS_OK: + return; + case CP_ACCESS_TRAP: + case CP_ACCESS_TRAP_UNCATEGORIZED: + /* These cases will eventually need to generate different + * syndrome information. + */ + break; + default: + g_assert_not_reached(); + } + raise_exception(env, EXCP_UDEF); +} + void HELPER(set_cp_reg)(CPUARMState *env, void *rip, uint32_t value) { const ARMCPRegInfo *ri = rip; diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c index a942609..d90ffd1 100644 --- a/target-arm/translate-a64.c +++ b/target-arm/translate-a64.c @@ -1210,6 +1210,17 @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread, return; } + if (ri->accessfn) { + /* Emit code to perform further access permissions checks at + * runtime; this may result in an exception. + */ + TCGv_ptr tmpptr; + gen_a64_set_pc_im(s->pc - 4); + tmpptr = tcg_const_ptr(ri); + gen_helper_access_check_cp_reg(cpu_env, tmpptr); + tcg_temp_free_ptr(tmpptr); + } + /* Handle special cases first */ switch (ri->type & ~(ARM_CP_FLAG_MASK & ~ARM_CP_SPECIAL)) { case ARM_CP_NOP: diff --git a/target-arm/translate.c b/target-arm/translate.c index 45886f9..2713081 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -6798,6 +6798,17 @@ static int disas_coproc_insn(CPUARMState * env, DisasContext *s, uint32_t insn) return 1; } + if (ri->accessfn) { + /* Emit code to perform further access permissions checks at + * runtime; this may result in an exception. + */ + TCGv_ptr tmpptr; + gen_set_pc_im(s, s->pc); + tmpptr = tcg_const_ptr(ri); + gen_helper_access_check_cp_reg(cpu_env, tmpptr); + tcg_temp_free_ptr(tmpptr); + } + /* Handle special cases first */ switch (ri->type & ~(ARM_CP_FLAG_MASK & ~ARM_CP_SPECIAL)) { case ARM_CP_NOP:
Several of the system registers handled via the ARMCPRegInfo mechanism have access trap control bits controlling whether the registers are accessible to lower privilege levels. Replace the existing mechanism (allowing the read and write functions to return EXCP_UDEF if access is denied) with a dedicated "check access rights" function pointer in the ARMCPRegInfo. This will allow us to simplify some of the register definitions, which no longer need read/write functions purely to handle the access checks. We take the opportunity to define the return value from the access checking function in a way that allows us to set the correct exception syndrome information for exceptions taken to AArch64 (which may need to distinguish access failures due to a configurable trap or enable from other kinds of access failure). This commit defines the new mechanism but does not move any of the registers across to use it. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target-arm/cpu.h | 29 +++++++++++++++++++++++++---- target-arm/helper.h | 1 + target-arm/op_helper.c | 18 ++++++++++++++++++ target-arm/translate-a64.c | 11 +++++++++++ target-arm/translate.c | 11 +++++++++++ 5 files changed, 66 insertions(+), 4 deletions(-)