Re: [PATCH v2 14/28] ARM: KVM: Add guest entry code

2016-02-09 Thread Christoffer Dall 
On Thu, Feb 04, 2016 at 11:00:31AM +, Marc Zyngier wrote:
> Add the very minimal piece of code that is now required to jump
> into the guest (and return from it). This code is only concerned
> with save/restoring the USR registers (r0-r12+lr for the guest,
> r4-r12+lr for the host), as everything else is dealt with in C
> (VFP is another matter though).
> 
> Signed-off-by: Marc Zyngier 
> ---
>  arch/arm/kvm/hyp/Makefile |  1 +
>  arch/arm/kvm/hyp/entry.S  | 70 
> +++
>  arch/arm/kvm/hyp/hyp.h|  2 ++
>  3 files changed, 73 insertions(+)
>  create mode 100644 arch/arm/kvm/hyp/entry.S
> 
> diff --git a/arch/arm/kvm/hyp/Makefile b/arch/arm/kvm/hyp/Makefile
> index 173bd1d..c779690 100644
> --- a/arch/arm/kvm/hyp/Makefile
> +++ b/arch/arm/kvm/hyp/Makefile
> @@ -8,3 +8,4 @@ obj-$(CONFIG_KVM_ARM_HOST) += timer-sr.o
>  obj-$(CONFIG_KVM_ARM_HOST) += vgic-v2-sr.o
>  obj-$(CONFIG_KVM_ARM_HOST) += vfp.o
>  obj-$(CONFIG_KVM_ARM_HOST) += banked-sr.o
> +obj-$(CONFIG_KVM_ARM_HOST) += entry.o
> diff --git a/arch/arm/kvm/hyp/entry.S b/arch/arm/kvm/hyp/entry.S
> new file mode 100644
> index 000..32f79b0
> --- /dev/null
> +++ b/arch/arm/kvm/hyp/entry.S
> @@ -0,0 +1,70 @@
> +/*
> + * Copyright (C) 2016 - ARM Ltd
> + * Author: Marc Zyngier 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see .
> +*/
> +
> +#include 
> +#include 
> +#include 
> +
> + .arch_extension virt
> +
> + .text
> + .pushsection.hyp.text, "ax"
> +
> +#define USR_REGS_OFFSET  (CPU_CTXT_GP_REGS + GP_REGS_USR)
> +
> +/* int __guest_enter(struct kvm_vcpu *vcpu, struct kvm_cpu_context *host) */
> +ENTRY(__guest_enter)
> + @ Save host registers
> + add r1, r1, #(USR_REGS_OFFSET + S_R4)
> + stm r1!, {r4-r12}
> + str lr, [r1, #4]@ Skip SP_usr (already saved)
> +
> + @ Restore guest registers
> + add r0, r0,  #(VCPU_GUEST_CTXT + USR_REGS_OFFSET + S_R0)

this really relies on offsetof(struct pt_regs, ARM_r0) == 0, which I
guess will likely never change, but given there's both a kernel and uapi
version of struct pt_regs, are we sure about this?

> + ldr lr, [r0, #S_LR]
> + ldm r0, {r0-r12}
> +
> + clrex
> + eret
> +ENDPROC(__guest_enter)
> +
> +ENTRY(__guest_exit)
> + /*
> +  * return convention:
> +  * guest r0, r1, r2 saved on the stack
> +  * r0: vcpu pointer
> +  * r1: exception code
> +  */
> +
> + add r2, r0, #(VCPU_GUEST_CTXT + USR_REGS_OFFSET + S_R3)
> + stm r2!, {r3-r12}
> + str lr, [r2, #4]
> + add r2, r0, #(VCPU_GUEST_CTXT + USR_REGS_OFFSET + S_R0)
> + pop {r3, r4, r5}@ r0, r1, r2
> + stm r2, {r3-r5}
> +
> + ldr r0, [r0, #VCPU_HOST_CTXT]
> + add r0, r0, #(USR_REGS_OFFSET + S_R4)
> + ldm r0!, {r4-r12}
> + ldr lr, [r0, #4]
> +
> + mov r0, r1
> + bx  lr
> +ENDPROC(__guest_exit)
> +
> + .popsection
> +
> diff --git a/arch/arm/kvm/hyp/hyp.h b/arch/arm/kvm/hyp/hyp.h
> index 278eb1f..b3f6ed2 100644
> --- a/arch/arm/kvm/hyp/hyp.h
> +++ b/arch/arm/kvm/hyp/hyp.h
> @@ -110,4 +110,6 @@ static inline bool __vfp_enabled(void)
>  void __hyp_text __banked_save_state(struct kvm_cpu_context *ctxt);
>  void __hyp_text __banked_restore_state(struct kvm_cpu_context *ctxt);
>  
> +int asmlinkage __guest_enter(struct kvm_vcpu *vcpu,
> +  struct kvm_cpu_context *host);
>  #endif /* __ARM_KVM_HYP_H__ */
> -- 
> 2.1.4
> 

Otherwise:
Reviewed-by: Christoffer Dall 
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v2 13/28] ARM: KVM: Add banked registers save/restore

2016-02-09 Thread Christoffer Dall 
On Thu, Feb 04, 2016 at 11:00:30AM +, Marc Zyngier wrote:
> Banked registers are one of the many perks of the 32bit architecture,
> and the world switch needs to cope with it.
> 
> This requires some "special" accessors, as these are not accessed
> using a standard coprocessor instruction.
> 
> Signed-off-by: Marc Zyngier 

Reviewed-by: Christoffer Dall 
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v2 02/28] ARM: KVM: Remove __kvm_hyp_code_start/__kvm_hyp_code_end

2016-02-09 Thread Christoffer Dall 
On Thu, Feb 04, 2016 at 11:00:19AM +, Marc Zyngier wrote:
> Now that we've unified the way we refer to the HYP text between
> arm and arm64, drop __kvm_hyp_code_start/end, and just use the
> __hyp_text_start/end symbols.
> 
> Signed-off-by: Marc Zyngier 

Acked-by: Christoffer Dall 
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v2 28/28] ARM: KVM: Remove __kvm_hyp_exit/__kvm_hyp_exit_end

2016-02-09 Thread Christoffer Dall 
On Thu, Feb 04, 2016 at 11:00:45AM +, Marc Zyngier wrote:
> I have no idea what these were for - probably a leftover from an
> early implementation. Good bye!
> 
> Signed-off-by: Marc Zyngier 

Acked-by: Christoffer Dall 
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v2 01/28] ARM: KVM: Move the HYP code to its own section

2016-02-09 Thread Christoffer Dall 
On Thu, Feb 04, 2016 at 11:00:18AM +, Marc Zyngier wrote:
> In order to be able to spread the HYP code into multiple compilation
> units, adopt a layout similar to that of arm64:
> - the HYP text is emited in its own section (.hyp.text)
> - two linker generated symbols are use to identify the boundaries
>   of that section
> 
> No functionnal change.
> 
> Signed-off-by: Marc Zyngier 

Acked-by: Christoffer Dall 
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v2 26/28] ARM: KVM: Remove unused hyp_pc field

2016-02-09 Thread Christoffer Dall 
On Thu, Feb 04, 2016 at 11:00:43AM +, Marc Zyngier wrote:
> This field was never populated, and the panic code already
> does something similar. Delete the related code.
> 
> Signed-off-by: Marc Zyngier 
> ---
>  arch/arm/include/asm/kvm_emulate.h | 5 -
>  arch/arm/include/asm/kvm_host.h| 1 -
>  arch/arm/kernel/asm-offsets.c  | 1 -
>  arch/arm/kvm/handle_exit.c | 5 -
>  4 files changed, 12 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_emulate.h 
> b/arch/arm/include/asm/kvm_emulate.h
> index f710616..8a8c6de 100644
> --- a/arch/arm/include/asm/kvm_emulate.h
> +++ b/arch/arm/include/asm/kvm_emulate.h
> @@ -108,11 +108,6 @@ static inline phys_addr_t kvm_vcpu_get_fault_ipa(struct 
> kvm_vcpu *vcpu)
>   return ((phys_addr_t)vcpu->arch.fault.hpfar & HPFAR_MASK) << 8;
>  }
>  
> -static inline unsigned long kvm_vcpu_get_hyp_pc(struct kvm_vcpu *vcpu)
> -{
> - return vcpu->arch.fault.hyp_pc;
> -}
> -
>  static inline bool kvm_vcpu_dabt_isvalid(struct kvm_vcpu *vcpu)
>  {
>   return kvm_vcpu_get_hsr(vcpu) & HSR_ISV;
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index daf6a71..19e9aba 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -85,7 +85,6 @@ struct kvm_vcpu_fault_info {
>   u32 hsr;/* Hyp Syndrome Register */
>   u32 hxfar;  /* Hyp Data/Inst. Fault Address Register */
>   u32 hpfar;  /* Hyp IPA Fault Address Register */
> - u32 hyp_pc; /* PC when exception was taken from Hyp mode */
>  };
>  
>  /*
> diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c
> index 1f24c32..27d0581 100644
> --- a/arch/arm/kernel/asm-offsets.c
> +++ b/arch/arm/kernel/asm-offsets.c
> @@ -175,7 +175,6 @@ int main(void)
>DEFINE(CPU_CTXT_VFP,   offsetof(struct kvm_cpu_context, vfp));
>DEFINE(CPU_CTXT_GP_REGS,   offsetof(struct kvm_cpu_context, gp_regs));
>DEFINE(GP_REGS_USR,offsetof(struct kvm_regs, usr_regs));
> -  DEFINE(VCPU_HYP_PC,offsetof(struct kvm_vcpu, 
> arch.fault.hyp_pc));
>  #endif
>BLANK();
>  #ifdef CONFIG_VDSO
> diff --git a/arch/arm/kvm/handle_exit.c b/arch/arm/kvm/handle_exit.c
> index 3ede90d..5377d753 100644
> --- a/arch/arm/kvm/handle_exit.c
> +++ b/arch/arm/kvm/handle_exit.c
> @@ -147,11 +147,6 @@ int handle_exit(struct kvm_vcpu *vcpu, struct kvm_run 
> *run,
>   switch (exception_index) {
>   case ARM_EXCEPTION_IRQ:
>   return 1;
> - case ARM_EXCEPTION_UNDEFINED:
> - kvm_err("Undefined exception in Hyp mode at: %#08lx\n",
> - kvm_vcpu_get_hyp_pc(vcpu));
> - BUG();
> - panic("KVM: Hypervisor undefined exception!\n");
>   case ARM_EXCEPTION_DATA_ABORT:
>   case ARM_EXCEPTION_PREF_ABORT:
>   case ARM_EXCEPTION_HVC:
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Acked-by: Christoffer Dall 
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [RFC PATCH 0/5] arm/arm64: Making HYP vgic/timer save/restore common

2016-02-09 Thread Christoffer Dall 
On Thu, Jan 28, 2016 at 03:56:00PM +, Marc Zyngier wrote:
> With the current state of the C conversion of arm and arm64 world
> switches, we are still unable to share some of the most obvious
> candidates (GIC and timer save/restore). In order to reduce the bloat,
> let's move these files to a common location (virt/kvm/arm/hyp).
> 
> The changes are extremely mechanical, with a small hack to deal with
> system register names on the 32bit side (I've decided to align on the
> 64bit names).
> 
> I'd like to know what people think of the common location. Does it
> makes sense to have a "hyp" subdirectory to indicate that this is not
> "normal" kernel code?

I think so, it means to me typically executing in "hyp/el2" or just
low-level hypervisor logic, in the case of VHE.


> 
> These patches are on top of 4.5-rc1, plus the VHE and 32bit WS rewrite
> patches, and I've pushed out a branch at
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git 
> kvm-arm64/mov-hyp
> 
> Marc Zyngier (5):
>   arm64: KVM: Move kvm/hyp/hyp.h to include/asm/kvm_hyp.h
>   arm64: KVM: Move vgic-v2 and timer save/restore to virt/kvm/arm/hyp
>   ARM: KVM: Move kvm/hyp/hyp.h to include/asm/kvm_hyp.h
>   ARM: KVM: Use common version of vgic-v2-sr.c
>   ARM: KVM: Use common version of timer-sr.c
> 
>  arch/arm/{kvm/hyp/hyp.h => include/asm/kvm_hyp.h}  |  9 +++
>  arch/arm/kvm/hyp/Makefile  |  7 +-
>  arch/arm/kvm/hyp/banked-sr.c   |  2 +-
>  arch/arm/kvm/hyp/cp15-sr.c |  2 +-
>  arch/arm/kvm/hyp/switch.c  |  2 +-
>  arch/arm/kvm/hyp/timer-sr.c| 71 --
>  arch/arm/kvm/hyp/tlb.c |  2 +-
>  arch/arm/kvm/hyp/vgic-v2-sr.c  | 84 
> --
>  .../arm64/{kvm/hyp/hyp.h => include/asm/kvm_hyp.h} |  0
>  arch/arm64/kvm/hyp/Makefile|  7 +-
>  arch/arm64/kvm/hyp/debug-sr.c  |  4 +-
>  arch/arm64/kvm/hyp/switch.c|  3 +-
>  arch/arm64/kvm/hyp/sysreg-sr.c |  4 +-
>  arch/arm64/kvm/hyp/tlb.c   |  2 +-
>  arch/arm64/kvm/hyp/vgic-v3-sr.c|  4 +-
>  {arch/arm64/kvm => virt/kvm/arm}/hyp/timer-sr.c|  4 +-
>  {arch/arm64/kvm => virt/kvm/arm}/hyp/vgic-v2-sr.c  |  4 +-
>  17 files changed, 30 insertions(+), 181 deletions(-)
>  rename arch/arm/{kvm/hyp/hyp.h => include/asm/kvm_hyp.h} (93%)
>  delete mode 100644 arch/arm/kvm/hyp/timer-sr.c
>  delete mode 100644 arch/arm/kvm/hyp/vgic-v2-sr.c
>  rename arch/arm64/{kvm/hyp/hyp.h => include/asm/kvm_hyp.h} (100%)
>  rename {arch/arm64/kvm => virt/kvm/arm}/hyp/timer-sr.c (97%)
>  rename {arch/arm64/kvm => virt/kvm/arm}/hyp/vgic-v2-sr.c (98%)
> 
> -- 
> 2.1.4
> 
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v2 06/28] ARM: KVM: Add a HYP-specific header file

2016-02-09 Thread Christoffer Dall 
On Thu, Feb 04, 2016 at 11:00:23AM +, Marc Zyngier wrote:
> In order to expose the various HYP services that are private to
> the hypervisor, add a new hyp.h file.
> 
> So far, it only contains mundane things such as section annotation
> and VA manipulation.
> 
> Signed-off-by: Marc Zyngier 

Acked-by: Christoffer Dall 
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v2 10/28] ARM: KVM: Add timer save/restore

2016-02-09 Thread Christoffer Dall 
On Thu, Feb 04, 2016 at 11:00:27AM +, Marc Zyngier wrote:
> This patch shouldn't exist, as we should be able to reuse the
> arm64 version for free. I'll get there eventually, but in the
> meantime I need a timer ticking.
> 
> Signed-off-by: Marc Zyngier 
> ---
>  arch/arm/kvm/hyp/Makefile   |  1 +
>  arch/arm/kvm/hyp/hyp.h  |  8 +
>  arch/arm/kvm/hyp/timer-sr.c | 71 
> +
>  3 files changed, 80 insertions(+)
>  create mode 100644 arch/arm/kvm/hyp/timer-sr.c
> 
> diff --git a/arch/arm/kvm/hyp/Makefile b/arch/arm/kvm/hyp/Makefile
> index 9f96fcb..9241ae8 100644
> --- a/arch/arm/kvm/hyp/Makefile
> +++ b/arch/arm/kvm/hyp/Makefile
> @@ -4,3 +4,4 @@
>  
>  obj-$(CONFIG_KVM_ARM_HOST) += tlb.o
>  obj-$(CONFIG_KVM_ARM_HOST) += cp15-sr.o
> +obj-$(CONFIG_KVM_ARM_HOST) += timer-sr.o
> diff --git a/arch/arm/kvm/hyp/hyp.h b/arch/arm/kvm/hyp/hyp.h
> index ab2cb82..4924418 100644
> --- a/arch/arm/kvm/hyp/hyp.h
> +++ b/arch/arm/kvm/hyp/hyp.h
> @@ -46,6 +46,9 @@
>  #define TTBR1__ACCESS_CP15_64(1, c2)
>  #define VTTBR__ACCESS_CP15_64(6, c2)
>  #define PAR  __ACCESS_CP15_64(0, c7)
> +#define CNTV_CVAL__ACCESS_CP15_64(3, c14)
> +#define CNTVOFF  __ACCESS_CP15_64(4, c14)
> +
>  #define CSSELR   __ACCESS_CP15(c0, 2, c0, 0)
>  #define VMPIDR   __ACCESS_CP15(c0, 4, c0, 5)
>  #define SCTLR__ACCESS_CP15(c1, 0, c0, 0)
> @@ -71,6 +74,11 @@
>  #define TID_URO  __ACCESS_CP15(c13, 0, c0, 3)
>  #define TID_PRIV __ACCESS_CP15(c13, 0, c0, 4)
>  #define CNTKCTL  __ACCESS_CP15(c14, 0, c1, 0)
> +#define CNTV_CTL __ACCESS_CP15(c14, 0, c3, 1)
> +#define CNTHCTL  __ACCESS_CP15(c14, 4, c1, 0)
> +
> +void __timer_save_state(struct kvm_vcpu *vcpu);
> +void __timer_restore_state(struct kvm_vcpu *vcpu);
>  
>  void __sysreg_save_state(struct kvm_cpu_context *ctxt);
>  void __sysreg_restore_state(struct kvm_cpu_context *ctxt);
> diff --git a/arch/arm/kvm/hyp/timer-sr.c b/arch/arm/kvm/hyp/timer-sr.c
> new file mode 100644
> index 000..d7535fd
> --- /dev/null
> +++ b/arch/arm/kvm/hyp/timer-sr.c
> @@ -0,0 +1,71 @@
> +/*
> + * Copyright (C) 2012-2015 - ARM Ltd
> + * Author: Marc Zyngier 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see .
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +#include "hyp.h"
> +
> +/* vcpu is already in the HYP VA space */
> +void __hyp_text __timer_save_state(struct kvm_vcpu *vcpu)
> +{
> + struct kvm *kvm = kern_hyp_va(vcpu->kvm);
> + struct arch_timer_cpu *timer = >arch.timer_cpu;
> + u64 val;
> +
> + if (kvm->arch.timer.enabled) {
> + timer->cntv_ctl = read_sysreg(CNTV_CTL);
> + timer->cntv_cval = read_sysreg(CNTV_CVAL);
> + }
> +
> + /* Disable the virtual timer */
> + write_sysreg(0, CNTV_CTL);
> +
> + /* Allow physical timer/counter access for the host */
> + val = read_sysreg(CNTHCTL);
> + val |= CNTHCTL_EL1PCTEN | CNTHCTL_EL1PCEN;
> + write_sysreg(val, CNTHCTL);
> +
> + /* Clear cntvoff for the host */
> + write_sysreg(0, CNTVOFF);

in the asm version we only did this if the timer was enabled, probably
the theory being that only in that case did we mody the offset.  But it
should be safe to just clear the cntvoff in any case, right?

> +}
> +
> +void __hyp_text __timer_restore_state(struct kvm_vcpu *vcpu)
> +{
> + struct kvm *kvm = kern_hyp_va(vcpu->kvm);
> + struct arch_timer_cpu *timer = >arch.timer_cpu;
> + u64 val;
> +
> + /*
> +  * Disallow physical timer access for the guest
> +  * Physical counter access is allowed
> +  */
> + val = read_sysreg(CNTHCTL);
> + val &= ~CNTHCTL_EL1PCEN;
> + val |= CNTHCTL_EL1PCTEN;
> + write_sysreg(val, CNTHCTL);
> +
> + if (kvm->arch.timer.enabled) {
> + write_sysreg(kvm->arch.timer.cntvoff, CNTVOFF);
> + write_sysreg(timer->cntv_cval, CNTV_CVAL);
> + isb();
> + write_sysreg(timer->cntv_ctl, CNTV_CTL);
> + }
> +}
> -- 
> 2.1.4
> 

Reviewed-by: Christoffer Dall 
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v2 12/28] ARM: KVM: Add VFP save/restore

2016-02-09 Thread Christoffer Dall 
On Thu, Feb 04, 2016 at 11:00:29AM +, Marc Zyngier wrote:
> This is almost a copy/paste of the existing version, with a couple
> of subtle differences:
> - Only write to FPEXC once on the save path
> - Add an isb when enabling VFP access
> 
> The patch also defines a few sysreg accessors and a __vfp_enabled
> predicate that test the VFP trapping state.
> 
> Signed-off-by: Marc Zyngier 

Reviewed-by: Christoffer Dall 
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v2 15/28] ARM: KVM: Add VFP lazy save/restore handler

2016-02-09 Thread Christoffer Dall 
On Thu, Feb 04, 2016 at 11:00:32AM +, Marc Zyngier wrote:
> Similar to the arm64 version, add the code that deals with VFP traps,
> re-enabling VFP, save/restoring the registers and resuming the guest.
> 
> Signed-off-by: Marc Zyngier 

Reviewed-by: Christoffer Dall 
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v2 00/28] ARM: KVM: Rewrite the world switch in C (mostly)

2016-02-09 Thread Christoffer Dall 
On Thu, Feb 04, 2016 at 11:00:17AM +, Marc Zyngier wrote:
> Now that the arm64 rewrite is in mainline, I've taken a stab at fixing
> the 32bit code the same way. This is fairly straightforward (once
> you've been through it once...), with a few patches that adapt the
> code to be similar to the 64bit version.
> 
> Note that the timer and GIC code should be made common between the two
> architectures, as this is litterally the exact same code (I've posted
> some proof of concept for that a while ago, see
> http://www.spinics.net/lists/kvm/msg126775.html).
> 
> This has been tested on a Dual A7, and the code is pushed on a branch:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git 
> kvm-arm/wsinc
> 

This looks good overall.

I tested it briefly on TC2 and it seems generally happy.

-Christoffer
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 3/5] irqchip/gic-v2: Parse and export virtual GIC information

2016-02-09 Thread Christoffer Dall 
On Mon, Feb 08, 2016 at 04:47:27PM +, Julien Grall wrote:
> For now, the firmware tables are parsed 2 times: once in the GIC
> drivers, the other timer when initializing the vGIC. It means code
> duplication and make more tedious to add the support for another
> firmware table (like ACPI).
> 
> Introduce a new structure and set of helpers to get/set the virtual GIC
> information. Also fill up the structure for GICv2.
> 
> Signed-off-by: Julien Grall 
> ---
> 
> Cc: Thomas Gleixner 
> Cc: Jason Cooper 
> Cc: Marc Zyngier 
> 
>  drivers/irqchip/irq-gic-common.c   | 13 ++
>  drivers/irqchip/irq-gic-common.h   |  3 ++
>  drivers/irqchip/irq-gic.c  | 78 
> +-
>  include/linux/irqchip/arm-gic-common.h | 34 +++
>  4 files changed, 127 insertions(+), 1 deletion(-)
>  create mode 100644 include/linux/irqchip/arm-gic-common.h
> 
> diff --git a/drivers/irqchip/irq-gic-common.c 
> b/drivers/irqchip/irq-gic-common.c
> index f174ce0..704caf4 100644
> --- a/drivers/irqchip/irq-gic-common.c
> +++ b/drivers/irqchip/irq-gic-common.c
> @@ -21,6 +21,19 @@
>  
>  #include "irq-gic-common.h"
>  
> +static const struct gic_kvm_info *gic_kvm_info;
> +
> +const struct gic_kvm_info *gic_get_kvm_info(void)
> +{
> + return gic_kvm_info;
> +}
> +
> +void gic_set_kvm_info(const struct gic_kvm_info *info)
> +{
> + WARN(gic_kvm_info != NULL, "gic_kvm_info already set\n");
> + gic_kvm_info = info;
> +}
> +
>  void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks,
>   void *data)
>  {
> diff --git a/drivers/irqchip/irq-gic-common.h 
> b/drivers/irqchip/irq-gic-common.h
> index fff697d..205e5fd 100644
> --- a/drivers/irqchip/irq-gic-common.h
> +++ b/drivers/irqchip/irq-gic-common.h
> @@ -19,6 +19,7 @@
>  
>  #include 
>  #include 
> +#include 
>  
>  struct gic_quirk {
>   const char *desc;
> @@ -35,4 +36,6 @@ void gic_cpu_config(void __iomem *base, void 
> (*sync_access)(void));
>  void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks,
>   void *data);
>  
> +void gic_set_kvm_info(const struct gic_kvm_info *info);
> +
>  #endif /* _IRQ_GIC_COMMON_H */
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index 911758c..d3a09a4 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -102,6 +102,8 @@ static struct static_key supports_deactivate = 
> STATIC_KEY_INIT_TRUE;
>  
>  static struct gic_chip_data gic_data[CONFIG_ARM_GIC_MAX_NR] __read_mostly;
>  
> +static struct gic_kvm_info gic_v2_kvm_info;
> +
>  #ifdef CONFIG_GIC_NON_BANKED
>  static void __iomem *gic_get_percpu_base(union gic_base *base)
>  {
> @@ -1190,6 +1192,44 @@ static bool gic_check_eoimode(struct device_node 
> *node, void __iomem **base)
>   return true;
>  }
>  
> +static void __init gic_of_setup_kvm_info(struct device_node *node)
> +{
> + int ret;
> + struct resource r;
> + unsigned int irq;
> +
> + gic_v2_kvm_info.type = GIC_V2;
> +
> + irq = irq_of_parse_and_map(node, 0);
> + if (!irq)
> + gic_v2_kvm_info.maint_irq = -1;
> + else
> + gic_v2_kvm_info.maint_irq = irq;
> +
> + ret = of_address_to_resource(node, 2, );
> + if (!ret) {
> + gic_v2_kvm_info.vctrl_base = r.start;
> + gic_v2_kvm_info.vctrl_size = resource_size();
> + }
> +
> + ret = of_address_to_resource(node, 3, );
> + if (!ret) {
> + if (!PAGE_ALIGNED(r.start))
> + pr_warn("GICV physical address 0x%llx not page 
> aligned\n",
> + (unsigned long long)r.start);
> + else if (!PAGE_ALIGNED(resource_size()))
> + pr_warn("GICV size 0x%llx not a multiple of page size 
> 0x%lx\n",
> + (unsigned long long)resource_size(),
> + PAGE_SIZE);
> + else {
> + gic_v2_kvm_info.vcpu_base = r.start;
> + gic_v2_kvm_info.vcpu_size = resource_size();
> + }
> + }
> +
> + gic_set_kvm_info(_v2_kvm_info);
> +}
> +
>  int __init
>  gic_of_init(struct device_node *node, struct device_node *parent)
>  {
> @@ -1219,8 +1259,10 @@ gic_of_init(struct device_node *node, struct 
> device_node *parent)
>  
>   __gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset,
>>fwnode);
> - if (!gic_cnt)
> + if (!gic_cnt) {
>   gic_init_physaddr(node);
> + gic_of_setup_kvm_info(node);
> + }
>  
>   if (parent) {
>   irq = irq_of_parse_and_map(node, 0);
> @@ -1247,6 +1289,32 @@ IRQCHIP_DECLARE(pl390, "arm,pl390", gic_of_init);
>  
>  #ifdef CONFIG_ACPI
>  static phys_addr_t cpu_phy_base __initdata;
> +static struct
> +{
> + u32 maint_irq;
> + int maint_irq_mode;
> + phys_addr_t

Re: [PATCH 0/8] KVM/ARM: Guest Entry/Exit optimizations

2016-02-09 Thread Christoffer Dall 
On Mon, Feb 08, 2016 at 11:40:14AM +, Marc Zyngier wrote:
> I've recently been looking at our entry/exit costs, and profiling
> figures did show some very low hanging fruits.
> 
> The most obvious cost is that accessing the GIC HW is slow. As in
> "deadly slow", specially when GICv2 is involved. So not hammering the
> HW when there is nothing to write is immediately beneficial, as this
> is the most common cases (whatever people seem to think, interrupts
> are a *rare* event).
> 
> Another easy thing to fix is the way we handle trapped system
> registers. We do insist on (mostly) sorting them, but we do perform a
> linear search on trap. We can switch to a binary search for free, and
> get immediate benefits (the PMU code, being extremely trap-happy,
> benefits immediately from this).
> 
> With these in place, I see an improvement of 20 to 30% (depending on
> the platform) on our world-switch cycle count when running a set of
> hand-crafted guests that are designed to only perform traps.

I'm curious about the weight of these two?  My guess based on the
measurement work I did is that the GIC is by far the worst sinner, but
that was exacerbated on X-Gene compared to Seattle.

> 
> Methodology:
> 
> * NULL-hypercall guest: Perform 65536 PSCI_0_2_FN_PSCI_VERSION calls,
> and then a power-off:
> 
> __start:
>   mov x19, #(1 << 16)
> 1:mov x0, #0x8400
>   hvc #0
>   sub x19, x19, #1
>   cbnzx19, 1b
>   mov x0, #0x8400
>   add x0, x0, #9
>   hvc #0
>   b   .
> 
> * sysreg trap guest: Perform 2^20 PMSELR_EL0 accesses, and power-off:
> 
> __start:
>   mov x19, #(1 << 20)
> 1:mrs x0, PMSELR_EL0
>   sub x19, x19, #1
>   cbnzx19, 1b
>   mov x0, #0x8400
>   add x0, x0, #9
>   hvc #0
>   b   .
> 
> * These guests are profiled using perf and kvmtool:
> 
> taskset -c 1 perf stat -e cycles:kh lkvm run -c1 --kernel do_sysreg.bin 2>&1 
> >/dev/null| grep cycles

these would be good to add to kvm-unit-tests so we can keep an eye on
this sort of thing...


> 
> The result is then divided by the number of iterations (2^16 or 2^20).
> 
> These tests have been run on Seattle, Mustang, and LS2085, and shown
> significant improvements in all cases. I've only touched the arm64
> GIC code, but obviously the 32bit code should use it as well once
> we've migrated it to C.
> 
> I've pushed out a branch (kvm-arm64/suck-less) to the usual location.
> 

Looks promising!

-Christoffer
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH V2] arm64: KVM: Configure TCR_EL2.ps at runtime

2016-02-09 Thread tchalamarla 
From: Tirumalesh Chalamarla 

Setting TCR_EL2.ps to 40 bits is wrong on systems with ps size is
less than 40 bits. and with systems where RAM is at higher address,
this will break KVM.

This patch sets TCR_EL2.ps at runtime similar to VTCR_EL.ps

Signed-off-by: Tirumalesh Chalamarla 
---
 arch/arm64/include/asm/kvm_arm.h |  5 -
 arch/arm64/kvm/hyp-init.S| 10 ++
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 738a95f..92f2ffd 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -107,7 +107,10 @@
 #define TCR_EL2_MASK   (TCR_EL2_TG0 | TCR_EL2_SH0 | \
 TCR_EL2_ORGN0 | TCR_EL2_IRGN0 | TCR_EL2_T0SZ)
 
-#define TCR_EL2_FLAGS  (TCR_EL2_RES1 | TCR_EL2_PS_40B)
+/*
+ * TCR_EL2.ps is configured at runtime similar to VTCR_EL2
+ */
+#define TCR_EL2_FLAGS  (TCR_EL2_RES1)
 
 /* VTCR_EL2 Registers bits */
 #define VTCR_EL2_RES1  (1 << 31)
diff --git a/arch/arm64/kvm/hyp-init.S b/arch/arm64/kvm/hyp-init.S
index 3e568dc..b246de1e 100644
--- a/arch/arm64/kvm/hyp-init.S
+++ b/arch/arm64/kvm/hyp-init.S
@@ -85,15 +85,17 @@ __do_hyp_init:
ldr_l   x5, idmap_t0sz
bfi x4, x5, TCR_T0SZ_OFFSET, TCR_TxSZ_WIDTH
 #endif
-   msr tcr_el2, x4
-
-   ldr x4, =VTCR_EL2_FLAGS
/*
 * Read the PARange bits from ID_AA64MMFR0_EL1 and set the PS bits in
-* VTCR_EL2.
+* TCR_EL2 and VTCR_EL2.
 */
mrs x5, ID_AA64MMFR0_EL1
bfi x4, x5, #16, #3
+
+   msr tcr_el2, x4
+
+   ldr x4, =VTCR_EL2_FLAGS
+   bfi x4, x5, #16, #3
/*
 * Read the VMIDBits bits from ID_AA64MMFR1_EL1 and set the VS bit in
 * VTCR_EL2.
-- 
2.1.0

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 3/5] irqchip/gic-v2: Parse and export virtual GIC information

2016-02-09 Thread Wei Huang 


On 02/09/2016 02:49 PM, Christoffer Dall wrote:
> On Mon, Feb 08, 2016 at 04:47:27PM +, Julien Grall wrote:
>> For now, the firmware tables are parsed 2 times: once in the GIC
>> drivers, the other timer when initializing the vGIC. It means code
>> duplication and make more tedious to add the support for another
>> firmware table (like ACPI).
>>
>> Introduce a new structure and set of helpers to get/set the virtual GIC
>> information. Also fill up the structure for GICv2.
>>
>> Signed-off-by: Julien Grall 
>> ---
>>
>> Cc: Thomas Gleixner 
>> Cc: Jason Cooper 
>> Cc: Marc Zyngier 
>>
>>  drivers/irqchip/irq-gic-common.c   | 13 ++
>>  drivers/irqchip/irq-gic-common.h   |  3 ++
>>  drivers/irqchip/irq-gic.c  | 78 
>> +-
>>  include/linux/irqchip/arm-gic-common.h | 34 +++
>>  4 files changed, 127 insertions(+), 1 deletion(-)
>>  create mode 100644 include/linux/irqchip/arm-gic-common.h
>>
>> diff --git a/drivers/irqchip/irq-gic-common.c 
>> b/drivers/irqchip/irq-gic-common.c
>> index f174ce0..704caf4 100644
>> --- a/drivers/irqchip/irq-gic-common.c
>> +++ b/drivers/irqchip/irq-gic-common.c
>> @@ -21,6 +21,19 @@
>>  
>>  #include "irq-gic-common.h"
>>  
>> +static const struct gic_kvm_info *gic_kvm_info;
>> +
>> +const struct gic_kvm_info *gic_get_kvm_info(void)
>> +{
>> +return gic_kvm_info;
>> +}
>> +
>> +void gic_set_kvm_info(const struct gic_kvm_info *info)
>> +{
>> +WARN(gic_kvm_info != NULL, "gic_kvm_info already set\n");
>> +gic_kvm_info = info;
>> +}
>> +
>>  void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks,
>>  void *data)
>>  {
>> diff --git a/drivers/irqchip/irq-gic-common.h 
>> b/drivers/irqchip/irq-gic-common.h
>> index fff697d..205e5fd 100644
>> --- a/drivers/irqchip/irq-gic-common.h
>> +++ b/drivers/irqchip/irq-gic-common.h
>> @@ -19,6 +19,7 @@
>>  
>>  #include 
>>  #include 
>> +#include 
>>  
>>  struct gic_quirk {
>>  const char *desc;
>> @@ -35,4 +36,6 @@ void gic_cpu_config(void __iomem *base, void 
>> (*sync_access)(void));
>>  void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks,
>>  void *data);
>>  
>> +void gic_set_kvm_info(const struct gic_kvm_info *info);
>> +
>>  #endif /* _IRQ_GIC_COMMON_H */
>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>> index 911758c..d3a09a4 100644
>> --- a/drivers/irqchip/irq-gic.c
>> +++ b/drivers/irqchip/irq-gic.c
>> @@ -102,6 +102,8 @@ static struct static_key supports_deactivate = 
>> STATIC_KEY_INIT_TRUE;
>>  
>>  static struct gic_chip_data gic_data[CONFIG_ARM_GIC_MAX_NR] __read_mostly;
>>  
>> +static struct gic_kvm_info gic_v2_kvm_info;
>> +
>>  #ifdef CONFIG_GIC_NON_BANKED
>>  static void __iomem *gic_get_percpu_base(union gic_base *base)
>>  {
>> @@ -1190,6 +1192,44 @@ static bool gic_check_eoimode(struct device_node 
>> *node, void __iomem **base)
>>  return true;
>>  }
>>  
>> +static void __init gic_of_setup_kvm_info(struct device_node *node)
>> +{
>> +int ret;
>> +struct resource r;
>> +unsigned int irq;
>> +
>> +gic_v2_kvm_info.type = GIC_V2;
>> +
>> +irq = irq_of_parse_and_map(node, 0);
>> +if (!irq)
>> +gic_v2_kvm_info.maint_irq = -1;
>> +else
>> +gic_v2_kvm_info.maint_irq = irq;
>> +
>> +ret = of_address_to_resource(node, 2, );
>> +if (!ret) {
>> +gic_v2_kvm_info.vctrl_base = r.start;
>> +gic_v2_kvm_info.vctrl_size = resource_size();
>> +}
>> +
>> +ret = of_address_to_resource(node, 3, );
>> +if (!ret) {
>> +if (!PAGE_ALIGNED(r.start))
>> +pr_warn("GICV physical address 0x%llx not page 
>> aligned\n",
>> +(unsigned long long)r.start);
>> +else if (!PAGE_ALIGNED(resource_size()))
>> +pr_warn("GICV size 0x%llx not a multiple of page size 
>> 0x%lx\n",
>> +(unsigned long long)resource_size(),
>> +PAGE_SIZE);
>> +else {
>> +gic_v2_kvm_info.vcpu_base = r.start;
>> +gic_v2_kvm_info.vcpu_size = resource_size();
>> +}
>> +}
>> +
>> +gic_set_kvm_info(_v2_kvm_info);
>> +}
>> +
>>  int __init
>>  gic_of_init(struct device_node *node, struct device_node *parent)
>>  {
>> @@ -1219,8 +1259,10 @@ gic_of_init(struct device_node *node, struct 
>> device_node *parent)
>>  
>>  __gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset,
>>   >fwnode);
>> -if (!gic_cnt)
>> +if (!gic_cnt) {
>>  gic_init_physaddr(node);
>> +gic_of_setup_kvm_info(node);
>> +}
>>  
>>  if (parent) {
>>  irq = irq_of_parse_and_map(node, 0);
>> @@ -1247,6 +1289,32 @@ IRQCHIP_DECLARE(pl390, "arm,pl390", gic_of_init);
>>  
>>  #ifdef

Re: [PATCH v2 19/28] ARM: KVM: Add panic handling code

2016-02-09 Thread Christoffer Dall 
On Thu, Feb 04, 2016 at 11:00:36AM +, Marc Zyngier wrote:
> Instead of spinning forever, let's "properly" handle any unexpected
> exception ("properly" meaning "print a spat on the console and die").
> 
> This has proved useful quite a few times...
> 
> Signed-off-by: Marc Zyngier 
> ---
>  arch/arm/kvm/hyp/hyp-entry.S | 28 +---
>  arch/arm/kvm/hyp/switch.c| 38 ++
>  2 files changed, 59 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/kvm/hyp/hyp-entry.S b/arch/arm/kvm/hyp/hyp-entry.S
> index 44bc11f..ca412ad 100644
> --- a/arch/arm/kvm/hyp/hyp-entry.S
> +++ b/arch/arm/kvm/hyp/hyp-entry.S
> @@ -75,15 +75,29 @@ __kvm_hyp_vector:
>  
>  .macro invalid_vector label, cause
>   .align
> -\label:  b   .
> +\label:  mov r0, #\cause
> + b   __hyp_panic
>  .endm
>  
> - invalid_vector  hyp_reset
> - invalid_vector  hyp_undef
> - invalid_vector  hyp_svc
> - invalid_vector  hyp_pabt
> - invalid_vector  hyp_dabt
> - invalid_vector  hyp_fiq
> + invalid_vector  hyp_reset   ARM_EXCEPTION_RESET
> + invalid_vector  hyp_undef   ARM_EXCEPTION_UNDEFINED
> + invalid_vector  hyp_svc ARM_EXCEPTION_SOFTWARE
> + invalid_vector  hyp_pabtARM_EXCEPTION_PREF_ABORT
> + invalid_vector  hyp_dabtARM_EXCEPTION_DATA_ABORT
> + invalid_vector  hyp_fiq ARM_EXCEPTION_FIQ
> +
> +ENTRY(__hyp_do_panic)
> + mrs lr, cpsr
> + bic lr, lr, #MODE_MASK
> + orr lr, lr, #SVC_MODE
> +THUMB(   orr lr, lr, #PSR_T_BIT  )
> + msr spsr_cxsf, lr
> + ldr lr, =panic
> + msr ELR_hyp, lr
> + ldr lr, =kvm_call_hyp
> + clrex
> + eret
> +ENDPROC(__hyp_do_panic)
>  
>  hyp_hvc:
>   /*
> diff --git a/arch/arm/kvm/hyp/switch.c b/arch/arm/kvm/hyp/switch.c
> index 8bfd729..67f3944 100644
> --- a/arch/arm/kvm/hyp/switch.c
> +++ b/arch/arm/kvm/hyp/switch.c
> @@ -188,3 +188,41 @@ again:
>  }
>  
>  __alias(__guest_run) int __weak __kvm_vcpu_run(struct kvm_vcpu *vcpu);
> +
> +static const char * const __hyp_panic_string[] = {
> + [ARM_EXCEPTION_RESET]  = "\nHYP panic: RST?? PC:%08x CPSR:%08x",
> + [ARM_EXCEPTION_UNDEFINED]  = "\nHYP panic: UNDEF PC:%08x CPSR:%08x",
> + [ARM_EXCEPTION_SOFTWARE]   = "\nHYP panic: SVC?? PC:%08x CPSR:%08x",
> + [ARM_EXCEPTION_PREF_ABORT] = "\nHYP panic: PABRT PC:%08x CPSR:%08x",
> + [ARM_EXCEPTION_DATA_ABORT] = "\nHYP panic: DABRT PC:%08x ADDR:%08x",
> + [ARM_EXCEPTION_IRQ]= "\nHYP panic: IRQ?? PC:%08x CPSR:%08x",
> + [ARM_EXCEPTION_FIQ]= "\nHYP panic: FIQ?? PC:%08x CPSR:%08x",
> + [ARM_EXCEPTION_HVC]= "\nHYP panic: HVC?? PC:%08x CPSR:%08x",

Why the question marks?

> +};
> +
> +void __hyp_text __noreturn __hyp_panic(int cause)
> +{
> + u32 elr = read_special(ELR_hyp);
> + u32 val;
> +
> + if (cause == ARM_EXCEPTION_DATA_ABORT)
> + val = read_sysreg(HDFAR);
> + else
> + val = read_special(SPSR);
> +
> + if (read_sysreg(VTTBR)) {
> + struct kvm_vcpu *vcpu;
> + struct kvm_cpu_context *host_ctxt;
> +
> + vcpu = (struct kvm_vcpu *)read_sysreg(HTPIDR);
> + host_ctxt = kern_hyp_va(vcpu->arch.host_cpu_context);
> + __deactivate_traps(vcpu);
> + __deactivate_vm(vcpu);
> + __sysreg_restore_state(host_ctxt);
> + }
> +
> + /* Call panic for real */
> + __hyp_do_panic(__hyp_panic_string[cause], elr, val);
> +
> + unreachable();
> +}
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Otherwise:

Reviewed-by: Christoffer Dall 
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v2 24/28] ARM: KVM: Turn CP15 defines to an enum

2016-02-09 Thread Christoffer Dall 
On Thu, Feb 04, 2016 at 11:00:41AM +, Marc Zyngier wrote:
> Just like on arm64, having the CP15 registers expressed as a set
> of #defines has been very conflict-prone. Let's turn it into an
> enum, which should make it more manageable.
> 
> Signed-off-by: Marc Zyngier 
> ---

Acked-by: Christoffer Dall 
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v2 22/28] ARM: KVM: Switch to C-based stage2 init

2016-02-09 Thread Christoffer Dall 
Mising (sarcastic) commit message?

On Thu, Feb 04, 2016 at 11:00:39AM +, Marc Zyngier wrote:
> Signed-off-by: Marc Zyngier 
> ---

Reviewed-by: Christoffer Dall 
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v2 22/28] ARM: KVM: Switch to C-based stage2 init

2016-02-09 Thread Marc Zyngier 
On Tue, 9 Feb 2016 19:45:13 +0100
Christoffer Dall  wrote:

> Mising (sarcastic) commit message?

Ah! I'll try to think of something along the lines of:

"Let's now retire the code that bravely served us for three
years, and enjoy a brand new set of bugs. At least, we can blame the
compiler this time."

> On Thu, Feb 04, 2016 at 11:00:39AM +, Marc Zyngier wrote:
> > Signed-off-by: Marc Zyngier 
> > ---
> 
> Reviewed-by: Christoffer Dall 
> 

Thanks,

M.
-- 
Jazz is not dead. It just smells funny.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH V2] arm64: KVM: Configure TCR_EL2.ps at runtime

2016-02-09 Thread Marc Zyngier 
On Tue, 9 Feb 2016 14:15:13 -0800
 wrote:

Tirumalesh,

> From: Tirumalesh Chalamarla 
> 
> Setting TCR_EL2.ps to 40 bits is wrong on systems with ps size is
> less than 40 bits. and with systems where RAM is at higher address,
> this will break KVM.
> 
> This patch sets TCR_EL2.ps at runtime similar to VTCR_EL.ps

VTCR_EL2.PS (and please use capital letters to designate a field in a
given register).

> 
> Signed-off-by: Tirumalesh Chalamarla 
> ---
>  arch/arm64/include/asm/kvm_arm.h |  5 -
>  arch/arm64/kvm/hyp-init.S| 10 ++
>  2 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_arm.h 
> b/arch/arm64/include/asm/kvm_arm.h
> index 738a95f..92f2ffd 100644
> --- a/arch/arm64/include/asm/kvm_arm.h
> +++ b/arch/arm64/include/asm/kvm_arm.h
> @@ -107,7 +107,10 @@
>  #define TCR_EL2_MASK (TCR_EL2_TG0 | TCR_EL2_SH0 | \
>TCR_EL2_ORGN0 | TCR_EL2_IRGN0 | TCR_EL2_T0SZ)
>  
> -#define TCR_EL2_FLAGS(TCR_EL2_RES1 | TCR_EL2_PS_40B)
> +/*
> + * TCR_EL2.ps is configured at runtime similar to VTCR_EL2
> + */
> +#define TCR_EL2_FLAGS(TCR_EL2_RES1)

Now that you've reduced the flags to be the reserved bits only, why not
clean it up for good, and directly use TCR_EL2_RES1? Also, that comment
is pretty pointless, as the whole of TCR_EL2 is now dynamically
configured, not just the PS field.

>  
>  /* VTCR_EL2 Registers bits */
>  #define VTCR_EL2_RES1(1 << 31)
> diff --git a/arch/arm64/kvm/hyp-init.S b/arch/arm64/kvm/hyp-init.S
> index 3e568dc..b246de1e 100644
> --- a/arch/arm64/kvm/hyp-init.S
> +++ b/arch/arm64/kvm/hyp-init.S
> @@ -85,15 +85,17 @@ __do_hyp_init:
>   ldr_l   x5, idmap_t0sz
>   bfi x4, x5, TCR_T0SZ_OFFSET, TCR_TxSZ_WIDTH
>  #endif
> - msr tcr_el2, x4
> -
> - ldr x4, =VTCR_EL2_FLAGS
>   /*
>* Read the PARange bits from ID_AA64MMFR0_EL1 and set the PS bits in
> -  * VTCR_EL2.
> +  * TCR_EL2 and VTCR_EL2.
>*/
>   mrs x5, ID_AA64MMFR0_EL1
>   bfi x4, x5, #16, #3
> +
> + msr tcr_el2, x4
> +
> + ldr x4, =VTCR_EL2_FLAGS
> + bfi x4, x5, #16, #3
>   /*
>* Read the VMIDBits bits from ID_AA64MMFR1_EL1 and set the VS bit in
>* VTCR_EL2.

Otherwise look OK to me.

Thanks,

M.
-- 
Jazz is not dead. It just smells funny.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm