From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49603) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZjrW5-0007lP-4y for qemu-devel@nongnu.org; Wed, 07 Oct 2015 12:24:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZjrW1-0006Ot-UH for qemu-devel@nongnu.org; Wed, 07 Oct 2015 12:24:33 -0400 Received: from mail-wi0-f173.google.com ([209.85.212.173]:34756) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZjrW1-0006Oc-Ky for qemu-devel@nongnu.org; Wed, 07 Oct 2015 12:24:29 -0400 Received: by wicfx3 with SMTP id fx3so220705650wic.1 for ; Wed, 07 Oct 2015 09:24:29 -0700 (PDT) References: <1443911939-2825-1-git-send-email-edgar.iglesias@gmail.com> <1443911939-2825-6-git-send-email-edgar.iglesias@gmail.com> From: Alex =?utf-8?Q?Benn=C3=A9e?= In-reply-to: <1443911939-2825-6-git-send-email-edgar.iglesias@gmail.com> Date: Wed, 07 Oct 2015 17:24:27 +0100 Message-ID: <874mi2y7d0.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v3 5/9] target-arm: Add ARMMMUFaultInfo List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Edgar E. Iglesias" Cc: edgar.iglesias@xilinx.com, peter.maydell@linaro.org, qemu-devel@nongnu.org, agraf@suse.de, laurent.desnogues@gmail.com, serge.fdrv@gmail.com Edgar E. Iglesias writes: > From: "Edgar E. Iglesias" > > Introduce ARMMMUFaultInfo to propagate MMU Fault information > across the MMU translation code path. This is in preparation for > adding State-2 translation. s/State/stage/? > > No functional changes. > > Signed-off-by: Edgar E. Iglesias > --- > target-arm/helper.c | 32 ++++++++++++++++++++------------ > target-arm/internals.h | 15 ++++++++++++++- > target-arm/op_helper.c | 3 ++- > 3 files changed, 36 insertions(+), 14 deletions(-) > > diff --git a/target-arm/helper.c b/target-arm/helper.c > index cbc1570..a429ff2 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -18,7 +18,8 @@ > static bool get_phys_addr(CPUARMState *env, target_ulong address, > int access_type, ARMMMUIdx mmu_idx, > hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot, > - target_ulong *page_size, uint32_t *fsr); > + target_ulong *page_size, uint32_t *fsr, > + ARMMMUFaultInfo *fi); > > /* Definitions for the PMCCNTR and PMCR registers */ > #define PMCRD 0x8 > @@ -1774,9 +1775,10 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t value, > bool ret; > uint64_t par64; > MemTxAttrs attrs = {}; > + ARMMMUFaultInfo fi = {}; > > ret = get_phys_addr(env, value, access_type, mmu_idx, > - &phys_addr, &attrs, &prot, &page_size, &fsr); > + &phys_addr, &attrs, &prot, &page_size, &fsr, &fi); > if (extended_addresses_enabled(env)) { > /* fsr is a DFSR/IFSR value for the long descriptor > * translation table format, but with WnR always clear. > @@ -6167,7 +6169,8 @@ static uint64_t arm_ldq_ptw(CPUState *cs, hwaddr addr, bool is_secure) > static bool get_phys_addr_v5(CPUARMState *env, uint32_t address, > int access_type, ARMMMUIdx mmu_idx, > hwaddr *phys_ptr, int *prot, > - target_ulong *page_size, uint32_t *fsr) > + target_ulong *page_size, uint32_t *fsr, > + ARMMMUFaultInfo *fi) > { > CPUState *cs = CPU(arm_env_get_cpu(env)); > int code; > @@ -6280,7 +6283,8 @@ do_fault: > static bool get_phys_addr_v6(CPUARMState *env, uint32_t address, > int access_type, ARMMMUIdx mmu_idx, > hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot, > - target_ulong *page_size, uint32_t *fsr) > + target_ulong *page_size, uint32_t *fsr, > + ARMMMUFaultInfo *fi) > { > CPUState *cs = CPU(arm_env_get_cpu(env)); > int code; > @@ -6431,7 +6435,8 @@ typedef enum { > static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address, > int access_type, ARMMMUIdx mmu_idx, > hwaddr *phys_ptr, MemTxAttrs *txattrs, int *prot, > - target_ulong *page_size_ptr, uint32_t *fsr) > + target_ulong *page_size_ptr, uint32_t *fsr, > + ARMMMUFaultInfo *fi) > { > CPUState *cs = CPU(arm_env_get_cpu(env)); > /* Read an LPAE long-descriptor translation table. */ > @@ -6975,7 +6980,8 @@ static bool get_phys_addr_pmsav5(CPUARMState *env, uint32_t address, > static bool get_phys_addr(CPUARMState *env, target_ulong address, > int access_type, ARMMMUIdx mmu_idx, > hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot, > - target_ulong *page_size, uint32_t *fsr) > + target_ulong *page_size, uint32_t *fsr, > + ARMMMUFaultInfo *fi) > { > if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) { > /* TODO: when we support EL2 we should here call ourselves recursively > @@ -7034,13 +7040,13 @@ static bool get_phys_addr(CPUARMState *env, target_ulong address, > > if (regime_using_lpae_format(env, mmu_idx)) { > return get_phys_addr_lpae(env, address, access_type, mmu_idx, phys_ptr, > - attrs, prot, page_size, fsr); > + attrs, prot, page_size, fsr, fi); > } else if (regime_sctlr(env, mmu_idx) & SCTLR_XP) { > return get_phys_addr_v6(env, address, access_type, mmu_idx, phys_ptr, > - attrs, prot, page_size, fsr); > + attrs, prot, page_size, fsr, fi); > } else { > return get_phys_addr_v5(env, address, access_type, mmu_idx, phys_ptr, > - prot, page_size, fsr); > + prot, page_size, fsr, fi); > } > } > > @@ -7049,7 +7055,8 @@ static bool get_phys_addr(CPUARMState *env, target_ulong address, > * fsr with ARM DFSR/IFSR fault register format value on failure. > */ > bool arm_tlb_fill(CPUState *cs, vaddr address, > - int access_type, int mmu_idx, uint32_t *fsr) > + int access_type, int mmu_idx, uint32_t *fsr, > + ARMMMUFaultInfo *fi) > { > ARMCPU *cpu = ARM_CPU(cs); > CPUARMState *env = &cpu->env; > @@ -7060,7 +7067,7 @@ bool arm_tlb_fill(CPUState *cs, vaddr address, > MemTxAttrs attrs = {}; > > ret = get_phys_addr(env, address, access_type, mmu_idx, &phys_addr, > - &attrs, &prot, &page_size, fsr); > + &attrs, &prot, &page_size, fsr, fi); > if (!ret) { > /* Map a single [sub]page. */ > phys_addr &= TARGET_PAGE_MASK; > @@ -7083,9 +7090,10 @@ hwaddr arm_cpu_get_phys_page_debug(CPUState *cs, vaddr addr) > bool ret; > uint32_t fsr; > MemTxAttrs attrs = {}; > + ARMMMUFaultInfo fi = {}; > > ret = get_phys_addr(env, addr, 0, cpu_mmu_index(env, false), &phys_addr, > - &attrs, &prot, &page_size, &fsr); > + &attrs, &prot, &page_size, &fsr, &fi); > > if (ret) { > return -1; > diff --git a/target-arm/internals.h b/target-arm/internals.h > index 36a56aa..3be23be 100644 > --- a/target-arm/internals.h > +++ b/target-arm/internals.h > @@ -389,8 +389,21 @@ bool arm_is_psci_call(ARMCPU *cpu, int excp_type); > void arm_handle_psci_call(ARMCPU *cpu); > #endif > > +/** > + * ARMMMUFaultInfo: Information describing an ARM MMU Fault > + * @s2addr: Address that caused a fault at stage 2 > + * @stage2: True if we faulted at stage 2 > + * @s1ptw: True if we faulted at stage 2 while doing a stage 1 page-table walk > + */ > +typedef struct ARMMMUFaultInfo ARMMMUFaultInfo; > +struct ARMMMUFaultInfo { > + target_ulong s2addr; > + bool stage2; > + bool s1ptw; I guess the compiler packs the bools down pretty well but why not just encode the faulting stage in a single variable? Perhaps I'm misunderstanding the potential combinations here. > +}; > + > /* Do a page table walk and add page to TLB if possible */ > bool arm_tlb_fill(CPUState *cpu, vaddr address, int rw, int mmu_idx, > - uint32_t *fsr); > + uint32_t *fsr, ARMMMUFaultInfo *fi); > > #endif > diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c > index 1425a1d..7ff3c61 100644 > --- a/target-arm/op_helper.c > +++ b/target-arm/op_helper.c > @@ -83,8 +83,9 @@ void tlb_fill(CPUState *cs, target_ulong addr, int is_write, int mmu_idx, > { > bool ret; > uint32_t fsr = 0; > + struct ARMMMUFaultInfo fi = {0}; > > - ret = arm_tlb_fill(cs, addr, is_write, mmu_idx, &fsr); > + ret = arm_tlb_fill(cs, addr, is_write, mmu_idx, &fsr, &fi); > if (unlikely(ret)) { > ARMCPU *cpu = ARM_CPU(cs); > CPUARMState *env = &cpu->env; -- Alex Bennée