|
From: | Greg Bellows |
Subject: | Re: [Qemu-devel] [PATCH v8 26/27] target-arm: make MAIR0/1 banked |
Date: | Tue, 4 Nov 2014 08:13:07 -0600 |
On 31 October 2014 12:31, Peter Maydell <address@hidden> wrote:Surely these need the big-endian ifdefs too if we're unioningOn 30 October 2014 21:28, Greg Bellows <address@hidden> wrote:
> Added CP register info entries for the ARMv7 MAIR0/1 secure banks.
>
> Signed-off-by: Greg Bellows <address@hidden>
>
> ---
>
> v5 -> v6
> - Changed _el field variants to be array based
> ---
> target-arm/cpu.h | 12 +++++++++++-
> target-arm/helper.c | 8 +++++---
> 2 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 348ce73..1a76fc6 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -305,7 +305,17 @@ typedef struct CPUARMState {
> uint32_t c9_pmxevtyper; /* perf monitor event type */
> uint32_t c9_pmuserenr; /* perf monitor user enable */
> uint32_t c9_pminten; /* perf monitor interrupt enables */
> - uint64_t mair_el1;
> + union { /* Memory attribute redirection */
> + struct {
> + uint64_t _unused_mair_0;
> + uint32_t mair0_ns;
> + uint32_t mair1_ns;
> + uint64_t _unused_mair_1;
> + uint32_t mair0_s;
> + uint32_t mair1_s;
uint32_ts with a uint64_t array?Fixed in v9.
(Can you check the other patches for this too, please? I might
have missed it when reviewing some of them, but a quick scan through
the cpu.h file in your tree ought to catch any other cases.)Will do, but I think there were only two places where I had to mix uint32 and uint64.Looking closer, I am thinking that this breaks with the ifdef in add_cpreg_to_hash as it seems it will reverse the structure logic. The function ifdef is really only designed to handle a single uint32 overlaying a uint64. I have to look into this more before we move forward with it.
Otherwise you can add my reviewed-by tag.
> + };
> + uint64_t mair_el[4];
> + };
> union { /* vector base address register */
> struct {
> uint64_t _unused_vbar;
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index d782897..fd5f074 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -939,7 +939,7 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
> */
> { .name = "MAIR_EL1", .state = ARM_CP_STATE_AA64,
> .opc0 = 3, .opc1 = 0, .crn = 10, .crm = 2, .opc2 = 0,
> - .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.mair_el1),
> + .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.mair_el[1]),
> .resetvalue = 0 },
> /* For non-long-descriptor page tables these are PRRR and NMRR;
> * regardless they still act as reads-as-written for QEMU.
> @@ -948,11 +948,13 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
> */
> { .name = "MAIR0", .state = ARM_CP_STATE_AA32, .type = ARM_CP_OVERRIDE,
> .cp = 15, .opc1 = 0, .crn = 10, .crm = 2, .opc2 = 0, .access = PL1_RW,
> - .fieldoffset = offsetoflow32(CPUARMState, cp15.mair_el1),
> + .bank_fieldoffsets = { offsetof(CPUARMState, cp15.mair0_s),
> + offsetof(CPUARMState, cp15.mair0_ns) },
> .resetfn = arm_cp_reset_ignore },
> { .name = "MAIR1", .state = ARM_CP_STATE_AA32, .type = ARM_CP_OVERRIDE,
> .cp = 15, .opc1 = 0, .crn = 10, .crm = 2, .opc2 = 1, .access = PL1_RW,
> - .fieldoffset = offsetofhigh32(CPUARMState, cp15.mair_el1),
> + .bank_fieldoffsets = { offsetof(CPUARMState, cp15.mair1_s),
> + offsetof(CPUARMState, cp15.mair1_ns) },
> .resetfn = arm_cp_reset_ignore },
> { .name = "ISR_EL1", .state = ARM_CP_STATE_BOTH,
> .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 1, .opc2 = 0,
> --
> 1.8.3.2
>
-- PMM
[Prev in Thread] | Current Thread | [Next in Thread] |