Petercs target-arm queue
cover Petercs target-arm queue
From: Peter Crosthwaite <[email protected]>
Hi Peter,
Heres is a mixed bag of target-arm patches from my tree. They are independent of each other and only threaded together for review convenience.
Regards,
Peter
Nathan Rossi (1):
target-arm: Add CP15 VBAR support
Peter Crosthwaite (3):
target-arm/helper.c: OMAP/StrongARM cp15 crn=0 cleanup target-arm/helper.c: Implement MIDR aliases target-arm/helper.c: Allow const opaques in arm CP
target-arm/cpu.h | 1 + target-arm/helper.c | 55 ++++++++++++++++++++++++++++++++++++++--------------- 2 files changed, 41 insertions(+), 15 deletions(-)
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
PATCH 1/4 target-arm/helper.c: OMAP/StrongARM cp15 crn=0 cleanup
From: Peter Crosthwaite <[email protected]>
The if block detecting OMAP/StrongARM modifies the id_cp_reginfo .access fields in place. So there is no need to replicate the call to define_arm_cp_reg(). Dropped, and let the OMAP case fall through to the normal behaviour after the in-place modification.
---
target-arm/helper.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-)
target-arm/helper.c +4 -9
@@ -1435,21 +1435,16 @@ void register_cp_regs_for_features(ARMCPU *cpu) arm_feature(env, ARM_FEATURE_STRONGARM)) {ARMCPRegInfo *r;/* Register the blanket "writes ignored" value first to cover the -* whole space. Then define the specific ID registers, but update -* their access field to allow write access, so that they ignore -* writes rather than causing them to UNDEF. +* whole space. Then update the specific ID registers to allow write +* access, so that they ignore writes rather than causing them to +* UNDEF. */ define_one_arm_cp_reg (cpu, &crn0_wi_reginfo);for (r = id_cp_reginfo; r->type !=ARM_CP_SENTINEL ; r++) { r->access =PL1_RW ; -define_one_arm_cp_reg (cpu, r); } - } else { -/* Just register the standard ID registers (read-only, meaning -* that writes will UNDEF). -*/ -define_arm_cp_regs (cpu, id_cp_reginfo); } +define_arm_cp_regs (cpu, id_cp_reginfo); }if (arm_feature (env,ARM_FEATURE_AUXCR )) {
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
PATCH 2/4 target-arm/helper.c: Implement MIDR aliases
In reply to [email protected]From: Peter Crosthwaite <[email protected]>
Unimplemented registers in the cp15, CRn=0, opc1=0, CRm=0 space default to aliasing the MIDR register. Set all registers in the space to access MIDR by default.
---
target-arm/helper.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
target-arm/helper.c +11 -5
@@ -1378,9 +1378,6 @@ void register_cp_regs_for_features(ARMCPU *cpu)if (arm_feature (env,ARM_FEATURE_DUMMY_C15_REGS )) {define_arm_cp_regs (cpu, dummy_c15_cp_reginfo); } -if (arm_feature (env,ARM_FEATURE_MPIDR )) { -define_arm_cp_regs (cpu, mpidr_cp_reginfo); - }if (arm_feature (env,ARM_FEATURE_LPAE )) {define_arm_cp_regs (cpu, lpae_cp_reginfo); }
@@ -1393,12 +1390,17 @@ void register_cp_regs_for_features(ARMCPU *cpu)/* Note that the MIDR isn't a simple constant register because * of the TI925 behaviour where writes to another register can * cause the MIDR value to change. +* +* Unimlplemented register in the c15 0 0 0 space default to +* MIDR. Define MIDR first as this entire space, then CTR, TCMTR +* and friends override accordingly. */ { .name = "MIDR", - .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 0, + .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = CP_ANY, .access = PL1_R, .resetvalue = cpu->midr, .writefn = arm_cp_write_ignore, .raw_writefn = raw_write, - .fieldoffset = offsetof(CPUARMState, cp15.c0_cpuid) }, + .fieldoffset = offsetof(CPUARMState, cp15.c0_cpuid), + .type = ARM_CP_OVERRIDE }, { .name = "CTR", .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 1, .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = cpu->ctr },
@@ -1447,6 +1449,10 @@ void register_cp_regs_for_features(ARMCPU *cpu)define_arm_cp_regs (cpu, id_cp_reginfo); } +if (arm_feature (env,ARM_FEATURE_MPIDR )) { +define_arm_cp_regs (cpu, mpidr_cp_reginfo); + } +if (arm_feature (env,ARM_FEATURE_AUXCR )) {ARMCPRegInfo auxcr = { .name ="AUXCR" , .cp =15 , .crn =1 , .crm =0 , .opc1 =0 , .opc2 =1 ,
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
PATCH 3/4 target-arm/helper.c: Allow const opaques in arm CP
In reply to [email protected]From: Peter Crosthwaite <[email protected]>
Allow for defining const opaque data in ARM CP register definitions by setting .opaque = foo. If non null opaque is passes into define_one_arm_cp_reg_with_opaque then that opaque will take precedence, otherwise if null opaque is passed, the original opaque data will be used.
---
target-arm/helper.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
target-arm/helper.c +3 -1
@@ -1608,7 +1608,9 @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu,ARMCPRegInfo *r2 =g_memdup (r,sizeof (ARMCPRegInfo));int is64 = (r->type &ARM_CP_64BIT ) ?1 :0 ; *key =ENCODE_CP_REG (r->cp, is64, r->crn, crm, opc1, opc2); - r2->opaque = opaque; +if (opaque) { + r2->opaque = opaque; + }/* Make sure reginfo passed to helpers for wildcarded regs * has the correct crm/opc1/opc2 for this reg, not CP_ANY: */
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
Re: [Qemu-devel] [PATCH v1 3/4] target-arm/helper.c: Allow const opaques in arm CP
Quoted 7 lines · On 10 July 2013 05:22, <[email protected]> wrote:
From: Peter Crosthwaite <[email protected]>
Allow for defining const opaque data in ARM CP register definitions by setting .opaque = foo. If non null opaque is passes into
define_one_arm_cp_reg_with_opaquethen that opaque will take precedence, otherwise if null opaque is passed, the original opaque data will be used.
This doesn't seem to have any motivating use in later patches in the series, or did I miss it? I guess being able to have the opaque data in the data-tables is nice, though.
thanks
-- PMM
Re: [Qemu-devel] [PATCH v1 3/4] target-arm/helper.c: Allow const opaques in arm CP
Hi Peter,
Quoted 9 lines · Peter Maydell
Quoted 7 lines · On 10 July 2013 05:22, <[email protected]> wrote:
From: Peter Crosthwaite <[email protected]>
Allow for defining const opaque data in ARM CP register definitions by setting .opaque = foo. If non null opaque is passes into
define_one_arm_cp_reg_with_opaquethen that opaque will take precedence, otherwise if null opaque is passed, the original opaque data will be used.
Quoted 4 lines
This doesn't seem to have any motivating use in later patches in the series, or did I miss it? I guess being able to have the opaque data in the data-tables is nice, though.
No on-list motivation but I thought I might send this through to get opinion on its validity before I start relying on it out of tree.
Regards,
peter
Quoted 3 lines
thanks
-- PMM
PATCH 4/4 target-arm: Add CP15 VBAR support
In reply to [email protected]From: Nathan Rossi <[email protected]>
Added Vector Base Address remapping on ARM v7.
---
changed since v1:
Removed obsolete VMSD logic (rebase)
Forced lower 5 bits of VBAR ro 0 (PMM review) Simplified if-else logic to not worry about pre-v7 (PMM review)
target-arm/cpu.h | 1 + target-arm/helper.c | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+)
target-arm/cpu.h +1 -0
@@ -138,6 +138,7 @@ 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 */ +uint32_t c12_vbar;/* vector base address register */ uint32_t c13_fcse;/* FCSE PID. */ uint32_t c13_context;/* Context ID. */ uint32_t c13_tls1;/* User RW Thread register. */
target-arm/helper.c +22 -0
@@ -520,6 +520,14 @@ static int pmintenclr_write(CPUARMState *env, const ARMCPRegInfo *ri,return 0 ; } +static int vbar_write (CPUARMState *env,const ARMCPRegInfo *ri, +uint64_t value) +{ + value &= (1 <<31 ); + env->cp15.c12_vbar = value & ~0x1Ful ; +return 0 ; +} +static int ccsidr_read (CPUARMState *env,const ARMCPRegInfo *ri,uint64_t *value) {
@@ -605,6 +613,10 @@ static const ARMCPRegInfo v7_cp_reginfo[] = { .access = PL1_RW, .type = ARM_CP_NO_MIGRATE, .fieldoffset = offsetof(CPUARMState, cp15.c9_pminten), .resetvalue = 0, .writefn = pmintenclr_write, }, + { .name = "VBAR", .cp = 15, .crn = 12, .crm = 0, .opc1 = 0, .opc2 = 0, + .access = PL1_RW, .writefn = vbar_write, + .fieldoffset = offsetof(CPUARMState, cp15.c12_vbar), + .resetvalue = 0 }, { .name = "SCR", .cp = 15, .crn = 1, .crm = 1, .opc1 = 0, .opc2 = 0, .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.c1_scr), .resetvalue = 0, },
@@ -2134,7 +2146,17 @@ void arm_cpu_do_interrupt(CPUState *cs) }/* High vectors. */ if (env->cp15.c1_sys & (1 <<13 )) { +/* when enabled, base address cannot be remapped. */ addr +=0xffff0000 ; + }else { +/* ARM v7 architectures provide a vector base address register to remap +* the interrupt vector table. +* This register is only followed in non-monitor mode, and has a secure +* and un-secure copy. Since the cpu is always in a un-secure operation +* and is never in monitor mode this feature is always active. +* Note: only bits 31:5 are valid. +*/ + addr += env->cp15.c12_vbar; }switch_mode (env, new_mode); env->spsr =cpsr_read (env);
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
Re: [Qemu-devel] [PATCH v1 4/4] target-arm: Add CP15 VBAR support
Quoted 3 lines · On 10 July 2013 05:23, <[email protected]> wrote:
From: Nathan Rossi <[email protected]>
Added Vector Base Address remapping on ARM v7.
This is only present on CPUs with TrustZone. On the other hand we already implement one or two TZ-only registers for pragmatic get-code-working reasons. I'll try to find time over the next day or two for a more careful think about what we should do here.
thanks
-- PMM
Re: [Qemu-devel] [PATCH v1 4/4] target-arm: Add CP15 VBAR support
Ping!
Quoted 8 lines · Peter Maydell
Quoted 3 lines · On 10 July 2013 05:23, <[email protected]> wrote:
From: Nathan Rossi <[email protected]>
Added Vector Base Address remapping on ARM v7.
This is only present on CPUs with TrustZone. On the other hand we already implement one or two TZ-only registers for pragmatic get-code-working reasons.
Does this perhaps just fall in that basket? I have guests that are broken without this and would like to fix mainline.
Quoted 4 lines
I'll try to find
time over the next day or two for a more careful think about what we should do here.
Any further thoughts? Can we perhaps /* FIXME: */ merge this (perhaps even FIXME: flag the other TZ specific regs along with it) and come back to it 1.7?
Regards,
Peter
Quoted 3 lines
thanks
-- PMM
Re: [Qemu-devel] [PATCH v1 4/4] target-arm: Add CP15 VBAR support
In reply to [email protected]Hello,
it would be really nice to get the CP15 VBAR support integrated. This allows unit test suites of single address space real-time systems to catch NULL pointer read/write access for example.
The ARM documentation says that this is a banked register that is only present in an implementation that includes the Security Extensions. Is this equivalent to having TrustZone?
In the ID_PFR1, Processor Feature Register 1, the bits [4:7] indicate if a particular CPU has this feature. Is it possible to use this register to determine the availability of the VBAR register in Qemu? This may avoid adding a new enum arm_features.
Signature · 9 lines
Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : [email protected] PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
Re: [Qemu-devel] [PATCH v1 4/4] target-arm: Add CP15 VBAR support
Quoted 3 lines · Sebastian Huber
it would be really nice to get the CP15 VBAR support integrated. This allows unit test suites of single address space real-time systems to catch NULL pointer read/write access for example.
I agree it would be useful. As I said, it needs thought about whether it makes sense to add it.
Quoted 3 lines
The ARM documentation says that this is a banked register that is only present in an implementation that includes the Security Extensions. Is this equivalent to having TrustZone?
Yes; the ARM ARM uses the term "Security Extensions" but this is interchangeable with TrustZone.
Quoted 4 lines
In the
ID_PFR1, Processor Feature Register 1, the bits [4:7] indicate if a particular CPU has this feature. Is it possible to use this register to determine the availability of the VBAR register in Qemu? This may avoid adding a new enumarm_features.
Unfortunately QEMU tends to not pass the right values in the PFRs:
we usually report what the real h/w CPU has even if we don't implement everything.
In any case a new arm_features bit is hardly a big deal: the actual question is whether it makes sense to provide this register that shouldn't really exist for the cpu configurations we're modelling.
-- PMM
Re: [Qemu-devel] [PATCH v1 4/4] target-arm: Add CP15 VBAR support
Quoted 3 lines · Peter Maydell
In any case a new
arm_featuresbit is hardly a big deal: the actual question is whether it makes sense to provide this register that shouldn't really exist for the cpu configurations we're modelling.
I use Qemu to run unit tests for a real-time operating system that runs on real Xilinx Zynq or Altera Cyclone-V for example. Both systems use an ARM Cortex-A9MPCore with Security Extensions. The operating system will run in secure mode all the time within a single address space and one-to-one mapped addresses. Everything we use from this Security Extensions is the VBAR. So for me the only missing Qemu feature is the VBAR if I want to use nearly the same binary on Qemu and the real hardware.
Signature · 9 lines
Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : [email protected] PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
Re: [Qemu-devel] [PATCH v1 4/4] target-arm: Add CP15 VBAR support
In reply to [email protected]target-arm/helper.c static int pmintenclr_write(CPUARMState *env, const ARMCPRegInfo *ri, · line 526
Quoted 4 lines · On 2013-07-10 06:23, [email protected] wrote:
target-arm/helper.c +4
-0+static int vbar_write (CPUARMState *env,const ARMCPRegInfo *ri, +uint64_t value) +{ + value &= (1 <<31 );
Is the above correct?
target-arm/helper.c static int pmintenclr_write(CPUARMState *env, const ARMCPRegInfo *ri, · line 529
Quoted 3 lines
target-arm/helper.c +3
-0+ env->cp15.c12_vbar = value & ~0x1Ful ; +return 0 ; +}
Shouldn't this be
{
env->cp15.c12_vbar = (uint32_t) (value & ~0x1FUL); return 0;
}
?
Signature · 9 lines
Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : [email protected] PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
Re: [Qemu-devel] [PATCH v1 4/4] target-arm: Add CP15 VBAR support
In reply to [email protected]Quoted 3 lines · On 10 July 2013 05:23, <[email protected]> wrote:
From: Nathan Rossi <[email protected]>
Added Vector Base Address remapping on ARM v7.
Apologies for this dropping off my radar for so long. I've had a bit of a think and I think that you're right that we can put in this register as part of our "random things we provide even though we aren't strictly implementing all of TZ", like the existing SCR register support.
target-arm/helper.c static int pmintenclr_write(CPUARMState *env, const ARMCPRegInfo *ri, · line 526
Quoted 4 lines
target-arm/helper.c +4
-0+static int vbar_write (CPUARMState *env,const ARMCPRegInfo *ri, +uint64_t value) +{ + value &= (1 <<31 );
This seems spurious -- we end up ignoring all but
the high bit of the written value.
target-arm/helper.c static int pmintenclr_write(CPUARMState *env, const ARMCPRegInfo *ri, · line 530
Quoted 4 lines
target-arm/helper.c +4
-0+ env->cp15.c12_vbar = value & ~0x1Ful ; +return 0 ; +} +
Otherwise looks OK. We should probably put in an ARM_FEATURE_TZ at some point, but for now putting it in the v7 regs with SCR is OK I think.
-- PMM
Re: [Qemu-devel] [PATCH v1 4/4] target-arm: Add CP15 VBAR support
Quoted 19 lines · Peter Maydell
Quoted 3 lines · On 10 July 2013 05:23, <[email protected]> wrote:
From: Nathan Rossi <[email protected]>
Added Vector Base Address remapping on ARM v7.
Apologies for this dropping off my radar for so long. I've had a bit of a think and I think that you're right that we can put in this register as part of our "random things we provide even though we aren't strictly implementing all of TZ", like the existing SCR register support.
target-arm/helper.c static int pmintenclr_write(CPUARMState *env, const ARMCPRegInfo *ri, · line 526
Quoted 4 lines
target-arm/helper.c +4
-0+static int vbar_write (CPUARMState *env,const ARMCPRegInfo *ri, +uint64_t value) +{ + value &= (1 <<31 );This seems spurious -- we end up ignoring all but the high bit of the written value.
Quoted 6 lines
target-arm/helper.c static int pmintenclr_write(CPUARMState *env, const ARMCPRegInfo *ri, · line 530
Quoted 4 lines
target-arm/helper.c +4
-0+ env->cp15.c12_vbar = value & ~0x1Ful ; +return 0 ; +} +Otherwise looks OK.
Update: I've removed the spurious mask line noted above and added the edited patch to target-arm.next.
thanks
-- PMM
Re: [Qemu-devel] [PATCH v1 4/4] target-arm: Add CP15 VBAR support
Hi,
Quoted 33 lines
-----Original Message-----
From: Peter Maydell [mailto:[email protected]]
Sent: Saturday, October 19, 2013 12:06 AM
To: Peter Crosthwaite
Cc: QEMU Developers
Subject: Re: [PATCH v1 4/4] target-arm: Add CP15 VBAR supportQuoted 18 lines · Peter Maydell
Quoted 3 lines · On 10 July 2013 05:23, <[email protected]> wrote:
From: Nathan Rossi <[email protected]>
Added Vector Base Address remapping on ARM v7.
Apologies for this dropping off my radar for so long. I've had a bit of a think and I think that you're right that we can put in this register as part of our "random things we provide even though we aren't strictly implementing all of TZ", like the existing SCR register support.
target-arm/helper.c static int pmintenclr_write(CPUARMState *env, const ARMCPRegInfo *ri, · line 526
Quoted 3 lines
target-arm/helper.c +3
-0+static int vbar_write (CPUARMState *env,const ARMCPRegInfo *ri, +uint64_t value) { + value &= (1 <<31 );This seems spurious -- we end up ignoring all but the high bit of the written value.
Quoted 6 lines
target-arm/helper.c static int pmintenclr_write(CPUARMState *env, const ARMCPRegInfo *ri, · line 530
Quoted 4 lines
target-arm/helper.c +4
-0+ env->cp15.c12_vbar = value & ~0x1Ful ; +return 0 ; +} +Otherwise looks OK.
Quoted 3 lines
Update: I've removed the spurious mask line noted above and added the edited patch to target-arm.next.
Thanks Peter,
Sorry about the delays my end.
Regards,
Peter
Quoted 2 lines
thanks
-- PMM
This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
Re: [Qemu-devel] [PATCH v1 4/4] target-arm: Add CP15 VBAR support
In reply to Peter MaydellQuoted 27 lines · Peter Maydell
Quoted 19 lines · Peter Maydell
Quoted 3 lines · On 10 July 2013 05:23, <[email protected]> wrote:
From: Nathan Rossi <[email protected]>
Added Vector Base Address remapping on ARM v7.
Apologies for this dropping off my radar for so long. I've had a bit of a think and I think that you're right that we can put in this register as part of our "random things we provide even though we aren't strictly implementing all of TZ", like the existing SCR register support.
target-arm/helper.c static int pmintenclr_write(CPUARMState *env, const ARMCPRegInfo *ri, · line 526
Quoted 4 lines
target-arm/helper.c +4
-0+static int vbar_write (CPUARMState *env,const ARMCPRegInfo *ri, +uint64_t value) +{ + value &= (1 <<31 );This seems spurious -- we end up ignoring all but the high bit of the written value.
Quoted 6 lines
target-arm/helper.c static int pmintenclr_write(CPUARMState *env, const ARMCPRegInfo *ri, · line 530
Quoted 4 lines
target-arm/helper.c +4
-0+ env->cp15.c12_vbar = value & ~0x1Ful ; +return 0 ; +} +Otherwise looks OK.
Quoted 6 lines
Update: I've removed the spurious mask line noted above and added the edited patch to target-arm.next.
thanks
-- PMM
Hi Peter,
Glad to see this go in. Is your target-arm.next branch available
in a public repo?
I found one on git.linaro.org but that is out of date.
Thanks,
Roy
Re: [Qemu-devel] [PATCH v1 4/4] target-arm: Add CP15 VBAR support
Quoted 2 lines · Roy Franz
Glad to see this go in. Is your target-arm.next branch available
in a public repo?
No, not generally.
-- PMM
Quoted 3 lines · On 10 July 2013 05:21, <[email protected]> wrote:
Heres is a mixed bag of target-arm patches from my tree. They are independent of each other and only threaded together for review convenience.
Applied 1-3 to target-arm.next, thanks (the VBAR one I still need to think about.)
-- PMM