Message ID | 1399356506-5609-6-git-send-email-edgar.iglesias@gmail.com |
---|---|
State | New |
Headers | show |
On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> > > Maps a given EL to the corresponding MMU index. > > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> > --- > target-arm/cpu.h | 21 ++++++++++++++++++++- > target-arm/translate-a64.c | 8 ++------ > 2 files changed, 22 insertions(+), 7 deletions(-) > > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > index ff86250..938f389 100644 > --- a/target-arm/cpu.h > +++ b/target-arm/cpu.h > @@ -1086,9 +1086,28 @@ static inline CPUARMState *cpu_init(const char *cpu_model) > #define MMU_MODE0_SUFFIX _kernel > #define MMU_MODE1_SUFFIX _user > #define MMU_USER_IDX 1 Blank line here. Otherwise: Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > +static inline int arm_el_to_mmu_idx(int current_el) > +{ > +#ifdef CONFIG_USER_ONLY > + return MMU_USER_IDX; > +#else > + switch (current_el) { > + case 0: > + return MMU_USER_IDX; > + case 1: > + return 0; > + default: > + /* Unsupported EL. */ > + assert(0); > + return 0; > + } > +#endif > +} > + > static inline int cpu_mmu_index (CPUARMState *env) > { > - return arm_current_pl(env) ? 0 : 1; > + int cur_el = arm_current_pl(env); > + return arm_el_to_mmu_idx(cur_el); > } > > #include "exec/cpu-all.h" > diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c > index 4f8246f..8523e76 100644 > --- a/target-arm/translate-a64.c > +++ b/target-arm/translate-a64.c > @@ -164,13 +164,9 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f, > } > } > > -static int get_mem_index(DisasContext *s) > +static inline int get_mem_index(DisasContext *s) > { > -#ifdef CONFIG_USER_ONLY > - return 1; > -#else > - return s->user; > -#endif > + return arm_el_to_mmu_idx(s->current_pl); > } > > void gen_a64_set_pc_im(uint64_t val) > -- > 1.8.3.2 > >
On 6 May 2014 07:08, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> > > Maps a given EL to the corresponding MMU index. > > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> > --- > target-arm/cpu.h | 21 ++++++++++++++++++++- > target-arm/translate-a64.c | 8 ++------ > 2 files changed, 22 insertions(+), 7 deletions(-) > > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > index ff86250..938f389 100644 > --- a/target-arm/cpu.h > +++ b/target-arm/cpu.h > @@ -1086,9 +1086,28 @@ static inline CPUARMState *cpu_init(const char *cpu_model) > #define MMU_MODE0_SUFFIX _kernel > #define MMU_MODE1_SUFFIX _user > #define MMU_USER_IDX 1 > +static inline int arm_el_to_mmu_idx(int current_el) > +{ > +#ifdef CONFIG_USER_ONLY > + return MMU_USER_IDX; > +#else > + switch (current_el) { > + case 0: > + return MMU_USER_IDX; > + case 1: > + return 0; > + default: > + /* Unsupported EL. */ > + assert(0); > + return 0; > + } > +#endif Can we just make the EL and the MMU index the same thing, or is secure-vs-nonsecure going to need its own MMU indexes anyway? thanks -- PMM
On Fri, May 16, 2014 at 03:24:42PM +0100, Peter Maydell wrote: > On 6 May 2014 07:08, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: > > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> > > > > Maps a given EL to the corresponding MMU index. > > > > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> > > --- > > target-arm/cpu.h | 21 ++++++++++++++++++++- > > target-arm/translate-a64.c | 8 ++------ > > 2 files changed, 22 insertions(+), 7 deletions(-) > > > > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > > index ff86250..938f389 100644 > > --- a/target-arm/cpu.h > > +++ b/target-arm/cpu.h > > @@ -1086,9 +1086,28 @@ static inline CPUARMState *cpu_init(const char *cpu_model) > > #define MMU_MODE0_SUFFIX _kernel > > #define MMU_MODE1_SUFFIX _user > > #define MMU_USER_IDX 1 > > +static inline int arm_el_to_mmu_idx(int current_el) > > +{ > > +#ifdef CONFIG_USER_ONLY > > + return MMU_USER_IDX; > > +#else > > + switch (current_el) { > > + case 0: > > + return MMU_USER_IDX; > > + case 1: > > + return 0; > > + default: > > + /* Unsupported EL. */ > > + assert(0); > > + return 0; > > + } > > +#endif > > Can we just make the EL and the MMU index the same thing, > or is secure-vs-nonsecure going to need its own MMU > indexes anyway? Right, I did the conversion to 1:1 mapping at an early stage but avoided it as we will need an indirect mapping for Secure EL0/1 anyway. I still have the patches around but they will conflict with the other trustzone patches on the list so I'd rather avoid them for now. Thanks, Edgar
On 17.05.14 00:10, Edgar E. Iglesias wrote: > On Fri, May 16, 2014 at 03:24:42PM +0100, Peter Maydell wrote: >> On 6 May 2014 07:08, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: >>> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> >>> >>> Maps a given EL to the corresponding MMU index. >>> >>> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> >>> --- >>> target-arm/cpu.h | 21 ++++++++++++++++++++- >>> target-arm/translate-a64.c | 8 ++------ >>> 2 files changed, 22 insertions(+), 7 deletions(-) >>> >>> diff --git a/target-arm/cpu.h b/target-arm/cpu.h >>> index ff86250..938f389 100644 >>> --- a/target-arm/cpu.h >>> +++ b/target-arm/cpu.h >>> @@ -1086,9 +1086,28 @@ static inline CPUARMState *cpu_init(const char *cpu_model) >>> #define MMU_MODE0_SUFFIX _kernel >>> #define MMU_MODE1_SUFFIX _user >>> #define MMU_USER_IDX 1 >>> +static inline int arm_el_to_mmu_idx(int current_el) >>> +{ >>> +#ifdef CONFIG_USER_ONLY >>> + return MMU_USER_IDX; >>> +#else >>> + switch (current_el) { >>> + case 0: >>> + return MMU_USER_IDX; >>> + case 1: >>> + return 0; >>> + default: >>> + /* Unsupported EL. */ >>> + assert(0); >>> + return 0; >>> + } >>> +#endif >> Can we just make the EL and the MMU index the same thing, >> or is secure-vs-nonsecure going to need its own MMU >> indexes anyway? > Right, I did the conversion to 1:1 mapping at an early stage > but avoided it as we will need an indirect mapping for > Secure EL0/1 anyway. How often do we switch between secure and non-secure? If it doesn't happen all that often, we could just flush the TLB on every transition. Alex
On Sat, May 17, 2014 at 12:13:06AM +0200, Alexander Graf wrote: > > On 17.05.14 00:10, Edgar E. Iglesias wrote: > >On Fri, May 16, 2014 at 03:24:42PM +0100, Peter Maydell wrote: > >>On 6 May 2014 07:08, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: > >>>From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> > >>> > >>>Maps a given EL to the corresponding MMU index. > >>> > >>>Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> > >>>--- > >>> target-arm/cpu.h | 21 ++++++++++++++++++++- > >>> target-arm/translate-a64.c | 8 ++------ > >>> 2 files changed, 22 insertions(+), 7 deletions(-) > >>> > >>>diff --git a/target-arm/cpu.h b/target-arm/cpu.h > >>>index ff86250..938f389 100644 > >>>--- a/target-arm/cpu.h > >>>+++ b/target-arm/cpu.h > >>>@@ -1086,9 +1086,28 @@ static inline CPUARMState *cpu_init(const char *cpu_model) > >>> #define MMU_MODE0_SUFFIX _kernel > >>> #define MMU_MODE1_SUFFIX _user > >>> #define MMU_USER_IDX 1 > >>>+static inline int arm_el_to_mmu_idx(int current_el) > >>>+{ > >>>+#ifdef CONFIG_USER_ONLY > >>>+ return MMU_USER_IDX; > >>>+#else > >>>+ switch (current_el) { > >>>+ case 0: > >>>+ return MMU_USER_IDX; > >>>+ case 1: > >>>+ return 0; > >>>+ default: > >>>+ /* Unsupported EL. */ > >>>+ assert(0); > >>>+ return 0; > >>>+ } > >>>+#endif > >>Can we just make the EL and the MMU index the same thing, > >>or is secure-vs-nonsecure going to need its own MMU > >>indexes anyway? > >Right, I did the conversion to 1:1 mapping at an early stage > >but avoided it as we will need an indirect mapping for > >Secure EL0/1 anyway. > > How often do we switch between secure and non-secure? If it doesn't happen > all that often, we could just flush the TLB on every transition. That's an option. I think this mostly affects aarch32 as that is where the TTBR regs are banked. For aarch64 I think the world switching FW has to rewrite the TTBR regs leading to TLB flushes anyway with current code. IIUC.. I can include a switch over to 1:1 mapping between EL and MMU-idx if preferd. I already have the patches but they will make this series conflict alot more with the TZ patches on list. Thanks, Edgar
diff --git a/target-arm/cpu.h b/target-arm/cpu.h index ff86250..938f389 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -1086,9 +1086,28 @@ static inline CPUARMState *cpu_init(const char *cpu_model) #define MMU_MODE0_SUFFIX _kernel #define MMU_MODE1_SUFFIX _user #define MMU_USER_IDX 1 +static inline int arm_el_to_mmu_idx(int current_el) +{ +#ifdef CONFIG_USER_ONLY + return MMU_USER_IDX; +#else + switch (current_el) { + case 0: + return MMU_USER_IDX; + case 1: + return 0; + default: + /* Unsupported EL. */ + assert(0); + return 0; + } +#endif +} + static inline int cpu_mmu_index (CPUARMState *env) { - return arm_current_pl(env) ? 0 : 1; + int cur_el = arm_current_pl(env); + return arm_el_to_mmu_idx(cur_el); } #include "exec/cpu-all.h" diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c index 4f8246f..8523e76 100644 --- a/target-arm/translate-a64.c +++ b/target-arm/translate-a64.c @@ -164,13 +164,9 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f, } } -static int get_mem_index(DisasContext *s) +static inline int get_mem_index(DisasContext *s) { -#ifdef CONFIG_USER_ONLY - return 1; -#else - return s->user; -#endif + return arm_el_to_mmu_idx(s->current_pl); } void gen_a64_set_pc_im(uint64_t val)