[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 28/32] target-arm: make DFSR banked
From: |
Edgar E. Iglesias |
Subject: |
Re: [Qemu-devel] [PATCH v3 28/32] target-arm: make DFSR banked |
Date: |
Tue, 17 Jun 2014 08:12:32 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Fri, Jun 13, 2014 at 05:06:15PM -0500, Greg Bellows wrote:
> I just wanted to point out that the change from array-notation to hard-code
> numbers in the names undoes Edgar's EL2/EL3 changes. I prefer this way
> over the array notation.
Hi,
This was discussed briefly here
http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03561.html
IMO, for some regs the array version doesn't make sense but for regs
that need to be indexed by EL it does. Just look at what this patch
results in for aarch64_cpu_do_interrupt(). AArch64 has a simpler/cleaner
architecture in this respect, IMO we should keep the
AArch64 simple and clean and take the banking pain in the AArch32 port.
>
>
> On 10 June 2014 18:55, Fabian Aggeler <address@hidden> wrote:
>
> > When EL3 is running in Aarch32 (or ARMv7 with Security Extensions)
> > DFSR has a secure and a non-secure instance.
> >
> > Signed-off-by: Fabian Aggeler <address@hidden>
> > ---
> > target-arm/cpu.h | 13 ++++++++++++-
> > target-arm/helper-a64.c | 17 ++++++++++++++---
> > target-arm/helper.c | 15 ++++++++-------
> > 3 files changed, 34 insertions(+), 11 deletions(-)
> >
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index 54c51a4..71782cf 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -266,7 +266,18 @@ typedef struct CPUARMState {
> > uint32_t ifsr32_el2;
> > };
> > };
> > - uint64_t esr_el[4];
> > + union {
> > + struct {
> > + uint64_t dfsr_ns;
> > + uint64_t hsr;
> > + uint64_t dfsr_s;
> > + };
> > + struct {
> > + uint64_t esr_el1;
> > + uint64_t esr_el2;
> > + uint64_t esr_el3;
> > + };
> > + };
I'd prefer:
- uint64_t esr_el[4];
+ union {
+ struct {
+ uint64_t dummy
+ uint64_t dfsr_ns;
+ uint64_t hsr;
+ uint64_t dfsr_s;
+ };
+ struct {
+ uint64_t esr_el[4];
+ };
+ };
And avoid the whole target_esr pointer thing in aarch64_cpu_do_interrupt().
Cheers,
Edgar
> > uint32_t c6_region[8]; /* MPU base/size registers. */
> > uint64_t far_el[4]; /* Fault address registers. */
> > uint64_t par_el1; /* Translation result. */
> > diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> > index d7522b6..dbbf012 100644
> > --- a/target-arm/helper-a64.c
> > +++ b/target-arm/helper-a64.c
> > @@ -447,6 +447,18 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
> > target_ulong addr = env->cp15.vbar_el[new_el];
> > unsigned int new_mode = aarch64_pstate_mode(new_el, true);
> > int i;
> > + uint64_t *target_esr;
> > + switch (new_el) {
> > + case 3:
> > + target_esr = &env->cp15.esr_el3;
> > + break;
> > + case 2:
> > + target_esr = &env->cp15.esr_el2;
> > + break;
> > + case 1:
> > + target_esr = &env->cp15.esr_el1;
> > + break;
> > + }
> >
> > if (arm_current_pl(env) < new_el) {
> > if (env->aarch64) {
> > @@ -477,8 +489,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
> > case EXCP_SWI:
> > case EXCP_HVC:
> > case EXCP_SMC:
> > - env->cp15.esr_el[new_el] = env->exception.syndrome;
> > - break;
> > + *target_esr = env->exception.syndrome;
> > case EXCP_IRQ:
> > case EXCP_VIRQ:
> > addr += 0x80;
> > @@ -498,7 +509,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
> > } else {
> > env->banked_spsr[0] = cpsr_read(env);
> > if (!env->thumb) {
> > - env->cp15.esr_el[new_el] |= 1 << 25;
> > + *target_esr |= 1 << 25;
> > }
> > env->elr_el[new_el] = env->regs[15];
> >
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index f51498a..793985e 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -1492,7 +1492,8 @@ static void vmsa_ttbr_write(CPUARMState *env, const
> > ARMCPRegInfo *ri,
> > static const ARMCPRegInfo vmsa_cp_reginfo[] = {
> > { .name = "DFSR", .cp = 15, .crn = 5, .crm = 0, .opc1 = 0, .opc2 = 0,
> > .access = PL1_RW, .type = ARM_CP_NO_MIGRATE,
> > - .fieldoffset = offsetoflow32(CPUARMState, cp15.esr_el[1]),
> > + .bank_fieldoffsets = { offsetoflow32(CPUARMState, cp15.dfsr_s),
> > + offsetoflow32(CPUARMState, cp15.dfsr_ns) },
> > .resetfn = arm_cp_reset_ignore, },
> > { .name = "IFSR", .cp = 15, .crn = 5, .crm = 0, .opc1 = 0, .opc2 = 1,
> > .access = PL1_RW, .resetvalue = 0,
> > @@ -1501,7 +1502,7 @@ static const ARMCPRegInfo vmsa_cp_reginfo[] = {
> > { .name = "ESR_EL1", .state = ARM_CP_STATE_AA64,
> > .opc0 = 3, .crn = 5, .crm = 2, .opc1 = 0, .opc2 = 0,
> > .access = PL1_RW,
> > - .fieldoffset = offsetof(CPUARMState, cp15.esr_el[1]), .resetvalue =
> > 0, },
> > + .fieldoffset = offsetof(CPUARMState, cp15.esr_el1), .resetvalue =
> > 0, },
> > { .name = "TTBR0_EL1", .state = ARM_CP_STATE_BOTH,
> > .opc0 = 3, .crn = 2, .crm = 0, .opc1 = 0, .opc2 = 0,
> > .access = PL1_RW, .writefn = vmsa_ttbr_write, .resetvalue = 0,
> > @@ -1565,7 +1566,7 @@ static void omap_cachemaint_write(CPUARMState *env,
> > const ARMCPRegInfo *ri,
> > static const ARMCPRegInfo omap_cp_reginfo[] = {
> > { .name = "DFSR", .cp = 15, .crn = 5, .crm = CP_ANY,
> > .opc1 = CP_ANY, .opc2 = CP_ANY, .access = PL1_RW, .type =
> > ARM_CP_OVERRIDE,
> > - .fieldoffset = offsetoflow32(CPUARMState, cp15.esr_el[1]),
> > + .fieldoffset = offsetoflow32(CPUARMState, cp15.esr_el1),
> > .resetvalue = 0, },
> > { .name = "", .cp = 15, .crn = 15, .crm = 0, .opc1 = 0, .opc2 = 0,
> > .access = PL1_RW, .type = ARM_CP_NOP },
> > @@ -2187,7 +2188,7 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
> > { .name = "ESR_EL2", .state = ARM_CP_STATE_AA64,
> > .type = ARM_CP_NO_MIGRATE,
> > .opc0 = 3, .opc1 = 4, .crn = 5, .crm = 2, .opc2 = 0,
> > - .access = PL2_RW, .fieldoffset = offsetof(CPUARMState,
> > cp15.esr_el[2]) },
> > + .access = PL2_RW, .fieldoffset = offsetof(CPUARMState,
> > cp15.esr_el2) },
> > { .name = "FAR_EL2", .state = ARM_CP_STATE_AA64,
> > .opc0 = 3, .opc1 = 4, .crn = 6, .crm = 0, .opc2 = 0,
> > .access = PL2_RW, .fieldoffset = offsetof(CPUARMState,
> > cp15.far_el[2]) },
> > @@ -2299,7 +2300,7 @@ static const ARMCPRegInfo v8_el3_cp_reginfo[] = {
> > { .name = "ESR_EL3", .state = ARM_CP_STATE_AA64,
> > .type = ARM_CP_NO_MIGRATE,
> > .opc0 = 3, .opc1 = 6, .crn = 5, .crm = 2, .opc2 = 0,
> > - .access = PL3_RW, .fieldoffset = offsetof(CPUARMState,
> > cp15.esr_el[3]) },
> > + .access = PL3_RW, .fieldoffset = offsetof(CPUARMState,
> > cp15.esr_el3) },
> > { .name = "FAR_EL3", .state = ARM_CP_STATE_AA64,
> > .opc0 = 3, .opc1 = 6, .crn = 6, .crm = 0, .opc2 = 0,
> > .access = PL3_RW, .fieldoffset = offsetof(CPUARMState,
> > cp15.far_el[3]) },
> > @@ -3847,11 +3848,11 @@ void arm_cpu_do_interrupt(CPUState *cs)
> > offset = 4;
> > break;
> > case EXCP_DATA_ABORT:
> > - env->cp15.esr_el[1] = env->exception.fsr;
> > + A32_BANKED_CURRENT_REG_SET(env, dfsr, env->exception.fsr);
> > env->cp15.far_el[1] = deposit64(env->cp15.far_el[1], 0, 32,
> > env->exception.vaddress);
> > qemu_log_mask(CPU_LOG_INT, "...with DFSR 0x%x DFAR 0x%x\n",
> > - (uint32_t)env->cp15.esr_el[1],
> > + env->exception.fsr,
> > (uint32_t)env->exception.vaddress);
> > new_mode = ARM_CPU_MODE_ABT;
> > addr = 0x10;
> > --
> > 1.8.3.2
> >
> >
- Re: [Qemu-devel] [PATCH v3 19/32] target-arm: insert Aarch32 cpregs twice into hashtable, (continued)
- [Qemu-devel] [PATCH v3 20/32] target-arm: arrayfying fieldoffset for banking, Fabian Aggeler, 2014/06/10
- [Qemu-devel] [PATCH v3 21/32] target-arm: add SCTLR_EL3 and make SCTLR banked, Fabian Aggeler, 2014/06/10
- [Qemu-devel] [PATCH v3 22/32] target-arm: make CSSELR banked, Fabian Aggeler, 2014/06/10
- [Qemu-devel] [PATCH v3 23/32] target-arm: add TTBR0_EL3 and make TTBR0/1 banked, Fabian Aggeler, 2014/06/10
- [Qemu-devel] [PATCH v3 24/32] target-arm: add TCR_EL3 and make TTBCR banked, Fabian Aggeler, 2014/06/10
- [Qemu-devel] [PATCH v3 25/32] target-arm: make c2_mask and c2_base_mask banked, Fabian Aggeler, 2014/06/10
- [Qemu-devel] [PATCH v3 28/32] target-arm: make DFSR banked, Fabian Aggeler, 2014/06/10
- [Qemu-devel] [PATCH v3 26/32] target-arm: make DACR banked, Fabian Aggeler, 2014/06/10
- [Qemu-devel] [PATCH v3 27/32] target-arm: make IFSR banked, Fabian Aggeler, 2014/06/10
- [Qemu-devel] [PATCH v3 29/32] target-arm: make IFAR/DFAR banked, Fabian Aggeler, 2014/06/10
- [Qemu-devel] [PATCH v3 32/32] target-arm: make c13 cp regs banked (FCSEIDR, ...), Fabian Aggeler, 2014/06/10
- [Qemu-devel] [PATCH v3 30/32] target-arm: make PAR banked, Fabian Aggeler, 2014/06/10