From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52473) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wmk6b-00018i-1e for qemu-devel@nongnu.org; Tue, 20 May 2014 09:29:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wmk6S-0004Op-0W for qemu-devel@nongnu.org; Tue, 20 May 2014 09:29:20 -0400 Received: from mail-pd0-x235.google.com ([2607:f8b0:400e:c02::235]:59961) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wmk6R-0004OP-N3 for qemu-devel@nongnu.org; Tue, 20 May 2014 09:29:11 -0400 Received: by mail-pd0-f181.google.com with SMTP id z10so320851pdj.40 for ; Tue, 20 May 2014 06:29:08 -0700 (PDT) Date: Tue, 20 May 2014 23:28:46 +1000 From: "Edgar E. Iglesias" Message-ID: <20140520132846.GJ18802@zapo.iiNet> References: <1400491383-6725-1-git-send-email-edgar.iglesias@gmail.com> <1400491383-6725-23-git-send-email-edgar.iglesias@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Subject: Re: [Qemu-devel] [PATCH v3 22/22] RFC: target-arm: Use a 1:1 mapping between EL and MMU index List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Aggeler Fabian Cc: "peter.maydell@linaro.org" , "peter.crosthwaite@xilinx.com" , "rob.herring@linaro.org" , "qemu-devel@nongnu.org" , "agraf@suse.de" , "john.williams@xilinx.com" , "alex.bennee@linaro.org" , "christoffer.dall@linaro.org" , "rth@twiddle.net" 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 wrote: > > > From: "Edgar E. Iglesias" > > > > Signed-off-by: Edgar E. Iglesias > > --- > > 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 > > >