On Tue, May 20, 2014 at 09:47:47AM +0000, Aggeler Fabian wrote:
> I guess this makes sense. Shouldn’t we implement two more MMUs to separate
> S-EL0/EL0 and S-EL1/EL1
> at least for ARMv8 with EL3 running in Aarch64 state?
Maybe with future patches. My understanding is that on aarch64 the world
switch between S/NS requires EL3 firmware to reprogram the TTBR regs.
Currently in QEMU, the re-programming of TTBR will flush the TLBs. We would
need to do something about that before adding MMU tables for aarch64 Secure
EL0/1 does any good. I think it's better to keep it simple for now and leave
this as a possible future optimization.
Another possible future optimization is to add some kind of dynamic allocation
of a limited set of MMU tables for different ASIDs and VMIDs. For emulated
virtualization, it might help quite a bit.
> For ARMv7 and ARMv8 with EL3 in Aarch32 S-PL1
> is mapped to PL3, so we only need one additional MMU for S-PL0. If you agree
> I could add this change in
> the Security Extension patches after this patch makes it into the tree.
Yes, I avoided this patch in my v1 because I thought we would need a non
linear mapping for aarch32 S/NS anyway. But I agree that a combination is
good. Keeping a 1:1 mapping between EL -> MMU idx and have additional MMU
tables for specific features like S/NS.
Thanks,
Edgar
>
> Best,
> Fabian
>
> On 19 May 2014, at 11:23, Edgar E. Iglesias <edgar.igles...@gmail.com> wrote:
>
> > From: "Edgar E. Iglesias" <edgar.igles...@gmail.com>
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.igles...@gmail.com>
> > ---
> > target-arm/cpu.h | 26 ++++----------------------
> > target-arm/translate.h | 2 +-
> > 2 files changed, 5 insertions(+), 23 deletions(-)
> >
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index 566f9ed..3b7ef32 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -1084,32 +1084,14 @@ static inline CPUARMState *cpu_init(const char
> > *cpu_model)
> > #define cpu_list arm_cpu_list
> >
> > /* MMU modes definitions */
> > -#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
> > -}
> > +#define MMU_MODE0_SUFFIX _user
> > +#define MMU_MODE1_SUFFIX _kernel
> > +#define MMU_USER_IDX 0
> >
> > static inline int cpu_mmu_index (CPUARMState *env)
> > {
> > int cur_el = arm_current_pl(env);
> > - return arm_el_to_mmu_idx(cur_el);
> > + return cur_el;
> > }
> >
> > #include "exec/cpu-all.h"
> > diff --git a/target-arm/translate.h b/target-arm/translate.h
> > index db6f0af..31a0104 100644
> > --- a/target-arm/translate.h
> > +++ b/target-arm/translate.h
> > @@ -54,7 +54,7 @@ static inline int arm_dc_feature(DisasContext *dc, int
> > feature)
> >
> > static inline int get_mem_index(DisasContext *s)
> > {
> > - return arm_el_to_mmu_idx(s->current_pl);
> > + return s->current_pl;
> > }
> >
> > /* target-specific extra values for is_jmp */
> > --
> > 1.8.3.2
> >
>
