[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: |
Aggeler Fabian |
Subject: |
Re: [Qemu-devel] [PATCH v3 28/32] target-arm: make DFSR banked |
Date: |
Tue, 24 Jun 2014 11:05:35 +0000 |
On 17 Jun 2014, at 08:12, Edgar E. Iglesias <address@hidden> wrote:
> 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.
>
You’re right. I didn’t like the outcome of this change either.
Since we can combine the advantages of both concepts in this compromise I
also vote for changing it to your proposed solution.
>>
>>
>> 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 20/32] target-arm: arrayfying fieldoffset for banking, (continued)
- [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
- [Qemu-devel] [PATCH v3 31/32] target-arm: make VBAR banked, Fabian Aggeler, 2014/06/10