From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32802) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZZK6Q-0005ck-0v for qemu-devel@nongnu.org; Tue, 08 Sep 2015 10:42:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZZK6N-0000Al-46 for qemu-devel@nongnu.org; Tue, 08 Sep 2015 10:42:29 -0400 Received: from mail-yk0-f171.google.com ([209.85.160.171]:36802) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZZK6M-0000Ad-W1 for qemu-devel@nongnu.org; Tue, 08 Sep 2015 10:42:27 -0400 Received: by ykcf206 with SMTP id f206so121679967ykc.3 for ; Tue, 08 Sep 2015 07:42:26 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1441311266-8644-11-git-send-email-edgar.iglesias@gmail.com> References: <1441311266-8644-1-git-send-email-edgar.iglesias@gmail.com> <1441311266-8644-11-git-send-email-edgar.iglesias@gmail.com> From: Peter Maydell Date: Tue, 8 Sep 2015 15:42:07 +0100 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH v1 10/10] target-arm: Add VMPIDR_EL2 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Edgar E. Iglesias" Cc: Edgar Iglesias , Sergey Fedorov , =?UTF-8?B?QWxleCBCZW5uw6ll?= , QEMU Developers , Alexander Graf On 3 September 2015 at 21:14, Edgar E. Iglesias wrote: > From: "Edgar E. Iglesias" > > Signed-off-by: Edgar E. Iglesias > --- > target-arm/cpu.h | 1 + > target-arm/helper.c | 20 ++++++++++++++++++-- > 2 files changed, 19 insertions(+), 2 deletions(-) > > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > index cdecfdf..1929a2f 100644 > --- a/target-arm/cpu.h > +++ b/target-arm/cpu.h > @@ -385,6 +385,7 @@ typedef struct CPUARMState { > uint64_t c15_ccnt; > uint64_t pmccfiltr_el0; /* Performance Monitor Filter Register */ > uint64_t vpidr_el2; /* Virtualization Processor ID Register */ > + uint64_t vmpidr_el2; /* Virtualization Multiprocessor ID Register */ > } cp15; > > struct { > diff --git a/target-arm/helper.c b/target-arm/helper.c > index 3701207..e335f8f 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -2447,6 +2447,12 @@ static uint64_t mpidr_read(CPUARMState *env, const ARMCPRegInfo *ri) > { > ARMCPU *cpu = ARM_CPU(arm_env_get_cpu(env)); > uint64_t mpidr = cpu->mp_affinity; > + unsigned int cur_el = arm_current_el(env); > + bool secure = arm_is_secure(env); > + > + if (arm_feature(env, ARM_FEATURE_EL2) && !secure && cur_el == 2) { > + mpidr = env->cp15.vmpidr_el2; > + } Shouldn't we be returning the VMPIDR if we're in NS-EL1, not NS-EL2? > if (arm_feature(env, ARM_FEATURE_V7MP)) { > mpidr |= (1U << 31); > @@ -4124,6 +4130,11 @@ void register_cp_regs_for_features(ARMCPU *cpu) > .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any, > .resetvalue = cpu->midr, > .fieldoffset = offsetof(CPUARMState, cp15.vpidr_el2) }, > + { .name = "VMPIDR_EL2", .state = ARM_CP_STATE_BOTH, > + .opc0 = 3, .opc1 = 4, .crn = 0, .crm = 0, .opc2 = 5, > + .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any, > + .resetvalue = cpu->mp_affinity, This resetvalue is missing the M and U bits which are ORed in when we read the MPIDR. > + .fieldoffset = offsetof(CPUARMState, cp15.vmpidr_el2) }, > REGINFO_SENTINEL > }; > define_arm_cp_regs(cpu, vpidr_regs); > @@ -4142,8 +4153,8 @@ void register_cp_regs_for_features(ARMCPU *cpu) > * register the no_el2 reginfos. > */ > if (arm_feature(env, ARM_FEATURE_EL3)) { > - /* When EL3 exists but not EL2, VPIDR takes the value > - * of MIDR_EL1. > + /* When EL3 exists but not EL2, VPIDR and VMPIDR take the value > + * of MIDR_EL1 and MPIDR_EL1. > */ > ARMCPRegInfo vpidr_regs[] = { > { .name = "VPIDR_EL2", .state = ARM_CP_STATE_BOTH, > @@ -4152,6 +4163,11 @@ void register_cp_regs_for_features(ARMCPU *cpu) > .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any, > .type = ARM_CP_CONST, > .fieldoffset = offsetof(CPUARMState, cp15.vpidr_el2) }, > + { .name = "VMPIDR_EL2", .state = ARM_CP_STATE_AA64, > + .opc0 = 3, .opc1 = 4, .crn = 0, .crm = 0, .opc2 = 5, > + .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any, > + .type = ARM_CP_CONST | ARM_CP_NO_RAW, > + .readfn = mpidr_read }, CP_CONST and a readfn doesn't make much sense. > REGINFO_SENTINEL > }; > define_arm_cp_regs(cpu, vpidr_regs); > -- > 1.9.1 > thanks -- PMM