| Message ID | CAFEAcA-GzmXg7OD5yoiyO37rG34_1XYVF+hdHCo31r3=WW58rQ@mail.gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
Sounds good. Thanks for catching and fixing this. On 9 December 2014 at 12:43, Peter Maydell <peter.maydell@linaro.org> wrote: > On 17 November 2014 at 16:47, Greg Bellows <greg.bellows@linaro.org> > wrote: > > Added additional NS-bit to CPREG hash encoding. Updated hash lookup > > locations to specify hash bit currently set to non-secure. > > > > Signed-off-by: Greg Bellows <greg.bellows@linaro.org> > > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > > > > --- > > > > v8 -> v9 > > - Fixed CP_REG_NS_MASK > > - Changed ENCODE_CP_REG argument order so ns follows is64 > > - Replaced use of CP_REG_NS_MASK with CP_REG_NS_SHIFT > > - Changed add_cpreg_to_hashtable argument order so ns follows is64 > > - Replaced use of SCR_NS with ARM_CP_SECSTATE_NS on registration > > - Undid global replace of Aarch# with AArch# in translate.c > > > > v5 -> v6 > > - Globally replace Aarch# with AArch# > > --- > > target-arm/cpu.h | 25 ++++++++++++++++++++----- > > target-arm/helper.c | 7 ++++--- > > target-arm/translate.c | 14 +++++++++----- > > 3 files changed, 33 insertions(+), 13 deletions(-) > > > > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > > index dd7d229..532f698 100644 > > --- a/target-arm/cpu.h > > +++ b/target-arm/cpu.h > > @@ -879,6 +879,7 @@ void armv7m_nvic_complete_irq(void *opaque, int irq); > > * Crn, Crm, opc1, opc2 fields > > * 32 or 64 bit register (ie is it accessed via MRC/MCR > > * or via MRRC/MCRR?) > > + * non-secure/secure bank (AArch32 only) > > * We allow 4 bits for opc1 because MRRC/MCRR have a 4 bit field. > > * (In this case crn and opc2 should be zero.) > > * For AArch64, there is no 32/64 bit size distinction; > > @@ -896,9 +897,16 @@ void armv7m_nvic_complete_irq(void *opaque, int > irq); > > #define CP_REG_AA64_SHIFT 28 > > #define CP_REG_AA64_MASK (1 << CP_REG_AA64_SHIFT) > > > > -#define ENCODE_CP_REG(cp, is64, crn, crm, opc1, opc2) \ > > - (((cp) << 16) | ((is64) << 15) | ((crn) << 11) | \ > > - ((crm) << 7) | ((opc1) << 3) | (opc2)) > > +/* To enable banking of coprocessor registers depending on ns-bit we > > + * add a bit to distinguish between secure and non-secure cpregs in the > > + * hashtable. > > + */ > > +#define CP_REG_NS_SHIFT 29 > > +#define CP_REG_NS_MASK (1 << CP_REG_NS_SHIFT) > > + > > +#define ENCODE_CP_REG(cp, is64, ns, crn, crm, opc1, opc2) \ > > + ((ns) << CP_REG_NS_SHIFT | ((cp) << 16) | ((is64) << 15) | \ > > + ((crn) << 11) | ((crm) << 7) | ((opc1) << 3) | (opc2)) > > We forgot about the uses of ENCODE_CP_REG in kvm32.c, so this > breaks compile on ARM. Since I've already put these patches into > target-arm.next, I'm going to just squash in the fix: > > --- a/target-arm/kvm32.c > +++ b/target-arm/kvm32.c > @@ -51,17 +51,17 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUClass > *ahcc) > struct kvm_one_reg idregs[] = { > { > .id = KVM_REG_ARM | KVM_REG_SIZE_U32 > - | ENCODE_CP_REG(15, 0, 0, 0, 0, 0), > + | ENCODE_CP_REG(15, 0, 0, 0, 0, 0, 0), > .addr = (uintptr_t)&midr, > }, > { > .id = KVM_REG_ARM | KVM_REG_SIZE_U32 > - | ENCODE_CP_REG(15, 0, 0, 1, 0, 0), > + | ENCODE_CP_REG(15, 0, 0, 0, 1, 0, 0), > .addr = (uintptr_t)&id_pfr0, > }, > { > .id = KVM_REG_ARM | KVM_REG_SIZE_U32 > - | ENCODE_CP_REG(15, 0, 0, 2, 0, 0), > + | ENCODE_CP_REG(15, 0, 0, 0, 2, 0, 0), > .addr = (uintptr_t)&id_isar0, > }, > { > > thanks > -- PMM >
--- a/target-arm/kvm32.c +++ b/target-arm/kvm32.c @@ -51,17 +51,17 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc) struct kvm_one_reg idregs[] = { { .id = KVM_REG_ARM | KVM_REG_SIZE_U32 - | ENCODE_CP_REG(15, 0, 0, 0, 0, 0), + | ENCODE_CP_REG(15, 0, 0, 0, 0, 0, 0), .addr = (uintptr_t)&midr, }, { .id = KVM_REG_ARM | KVM_REG_SIZE_U32 - | ENCODE_CP_REG(15, 0, 0, 1, 0, 0), + | ENCODE_CP_REG(15, 0, 0, 0, 1, 0, 0), .addr = (uintptr_t)&id_pfr0, }, { .id = KVM_REG_ARM | KVM_REG_SIZE_U32 - | ENCODE_CP_REG(15, 0, 0, 2, 0, 0), + | ENCODE_CP_REG(15, 0, 0, 0, 2, 0, 0), .addr = (uintptr_t)&id_isar0, }, {