From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59184) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WQgWQ-0003n1-97 for qemu-devel@nongnu.org; Thu, 20 Mar 2014 13:12:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WQgWL-0000ER-R0 for qemu-devel@nongnu.org; Thu, 20 Mar 2014 13:12:50 -0400 Received: from mail-la0-f42.google.com ([209.85.215.42]:57145) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WQgWL-0000EG-JS for qemu-devel@nongnu.org; Thu, 20 Mar 2014 13:12:45 -0400 Received: by mail-la0-f42.google.com with SMTP id ec20so856800lab.1 for ; Thu, 20 Mar 2014 10:12:43 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <1394134385-1727-1-git-send-email-peter.maydell@linaro.org> <1394134385-1727-17-git-send-email-peter.maydell@linaro.org> From: Peter Maydell Date: Thu, 20 Mar 2014 17:12:23 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH v4 16/21] target-arm: Implement SP_EL0, SP_EL1 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite Cc: Rob Herring , Patch Tracking , Michael Matz , Claudio Fontana , Alexander Graf , "qemu-devel@nongnu.org Developers" , Laurent Desnogues , Dirk Mueller , Will Newton , =?UTF-8?B?QWxleCBCZW5uw6ll?= , "kvmarm@lists.cs.columbia.edu" , Christoffer Dall , Richard Henderson On 17 March 2014 07:02, Peter Crosthwaite wrote: > On Fri, Mar 7, 2014 at 5:33 AM, Peter Maydell wrote: >> Implement handling for the AArch64 SP_EL0 system register. >> This holds the EL0 stack pointer, and is only accessible when >> it's not being used as the stack pointer, ie when we're in EL1 >> and EL1 is using its own stack pointer. We also provide a >> definition of the SP_EL1 register; this isn't guest visible >> as a system register for an implementation like QEMU which >> doesn't provide EL2 or EL3; however it is useful for ensuring >> the underlying state is migrated. >> >> We need to update the state fields in the CPU state whenever > > "whenever we". Fixed, thanks. >> switch stack pointers; this happens when we take an exception >> and also when SPSEL is used to change the bit in PSTATE which >> indicates which stack pointer EL1 should use. >> >> Signed-off-by: Peter Maydell >> --- >> target-arm/cpu.h | 2 ++ >> target-arm/helper.c | 34 ++++++++++++++++++++++++++++++++++ >> target-arm/internals.h | 25 +++++++++++++++++++++++++ >> target-arm/kvm64.c | 37 ++++++++++++++++++++++++++++++++++--- >> target-arm/machine.c | 7 ++++--- >> target-arm/op_helper.c | 2 +- >> 6 files changed, 100 insertions(+), 7 deletions(-) >> >> diff --git a/target-arm/cpu.h b/target-arm/cpu.h >> index 7ef2c71..338edc3 100644 >> --- a/target-arm/cpu.h >> +++ b/target-arm/cpu.h >> @@ -163,6 +163,7 @@ typedef struct CPUARMState { >> uint64_t daif; /* exception masks, in the bits they are in in PSTATE */ >> >> uint64_t elr_el1; /* AArch64 ELR_EL1 */ >> + uint64_t sp_el[2]; /* AArch64 banked stack pointers */ >> > > Should the macro AARCH64_MAX_EL_LEVELS exist for this? I'm not really convinced, because the set of things that would need to change to make the maximum EL not 1 would be so huge that one array size more or less is not really very relevant. This seems to me like the kind of thing that will shake itself out when somebody gets round to adding EL2 or EL3 emulation support. >> const VMStateDescription vmstate_arm_cpu = { >> .name = "cpu", >> - .version_id = 15, >> - .minimum_version_id = 15, >> - .minimum_version_id_old = 15, >> + .version_id = 16, >> + .minimum_version_id = 16, >> + .minimum_version_id_old = 16, > > So in the past, I have squashed unrelated patches together to minimise > VMSD versions bumps. Whats more important - these versions numbers of > git patch separation? We have an entire 32 bit integer to work with for the ID numbers, so there's no reason to make patches harder to review by squashing together things that would be clearer handled separately. thanks -- PMM