Re: [PATCH v4 4/4] KVM: arm64: Clean up kvm makefiles

2020-04-30 Thread Will Deacon
On Tue, Apr 21, 2020 at 02:17:34PM +0100, Fuad Tabba wrote:
> Consolidate references to the CONFIG_KVM configuration item to encompass
> entire folders rather than per line.
> 
> Signed-off-by: Fuad Tabba 
> ---
>  arch/arm64/kvm/Makefile | 40 -
>  arch/arm64/kvm/hyp/Makefile | 15 --
>  2 files changed, 17 insertions(+), 38 deletions(-)

Seems like a sensible clean-up to me:

Acked-by: Will Deacon 

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


Re: [PATCH 00/15] Split off nVHE hyp code

2020-04-30 Thread David Brazdil
Hey Marc,

> Do you have any figure on how much bigger the final kernel becomes once this
> is applied? I guess I can find out pretty easily, but this is the kind of
> thing
> that would be useful to make part of your cover letter.
Bloat-o-meter puts the diff at 21KB:
  add/remove: 152/45 grow/shrink: 10/32 up/down: 27882/-6240 (21642)
That said, the size of `Image` hasn't changed at all, not sure why that is.
The size of Image.gz has gone up by 10KB.

> 
> I'll try to review this shortly.
Cheers, no rush.

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


Re: [PATCH 00/15] Split off nVHE hyp code

2020-04-30 Thread Marc Zyngier

Hi David,

Thanks for posting this, looks quite interesting!

On 2020-04-30 15:48, David Brazdil wrote:
Refactor files in arch/arm64/kvm/hyp to compile all code which runs in 
EL2
under nVHE into separate object files from the rest of KVM. This is 
done in
preparation for being able to unmap .hyp.text from EL1 but has other 
benefits,

notably:
 * safe use of KASAN/UBSAN/GCOV instrumentation on VHE code,
 * cleaner HVC API,
 * no need for __hyp_text annotations.

nVHE-specific code is moved to hyp/nvhe and compiled with custom build 
rules
similar to those used by EFI stub. Shared source files are compiled 
under both
VHE and nVHE build rules. Where a source file contained both VHE and 
nVHE code,
it is split into a shared header file and two C source files. This is 
done one

file per commit to make review easier.


Do you have any figure on how much bigger the final kernel becomes once 
this
is applied? I guess I can find out pretty easily, but this is the kind 
of thing

that would be useful to make part of your cover letter.

I'll try to review this shortly.

Thanks,

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


Re: [PATCH V2 00/16] arm64/cpufeature: Introduce ID_PFR2, ID_DFR1, ID_MMFR5 and other changes

2020-04-30 Thread Will Deacon
On Thu, Apr 30, 2020 at 08:29:44AM +0530, Anshuman Khandual wrote:
> On 04/30/2020 02:56 AM, Will Deacon wrote:
> > On Wed, Apr 29, 2020 at 03:07:15PM +0530, Anshuman Khandual wrote:
> >> On 04/14/2020 03:18 PM, Anshuman Khandual wrote:
> >>> Changes in V2:
> >>>
> >>> - Added Suggested-by tag from Mark Rutland for all changes he had proposed
> >>> - Added comment for SpecSEI feature on why it is HIGHER_SAFE per Suzuki
> >>> - Added a patch which makes ID_AA64DFR0_DOUBLELOCK a signed feature per 
> >>> Suzuki
> >>> - Added ID_DFR1 and ID_MMFR5 system register definitions per Will
> >>> - Added remaining features bits for relevant 64 bit system registers per 
> >>> Will
> >>> - Changed commit message on [PATCH 5/7] regarding TraceFilt feature per 
> >>> Suzuki
> >>> - Changed ID_PFR2.CSV3 (FTR_STRICT -> FTR_NONSTRICT) as 64 bit registers 
> >>> per Will
> >>> - Changed ID_PFR0.CSV2 (FTR_STRICT -> FTR_NONSTRICT) as 64 bit registers 
> >>> per Will 
> >>> - Changed some commit messages
> >>
> >> Just a gentle ping. I am wondering if you had a chance to glance
> >> through this updated series.
> > 
> > Please can you resend based on for-next/cpufeature?
> 
> Sure, will do.

Thanks. I'll keep an eye out for them.

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


[PATCH 12/15] arm64: kvm: Compile remaining hyp/ files for both VHE/nVHE

2020-04-30 Thread David Brazdil
This patch is part of a series which builds KVM's non-VHE hyp code separately
from VHE and the rest of the kernel.

The following files in hyp/ contain only code shared by VHE/nVHE:
  vgic-v3-sr.c, aarch32.c, vgic-v2-cpuif-proxy.c, entry.S, fpsimd.S
Compile them under both configurations. Deletions in image-vars.h reflect
eliminated dependencies of nVHE code on the rest of the kernel.

Signed-off-by: David Brazdil 
---
 arch/arm64/kernel/image-vars.h   | 19 ---
 arch/arm64/kvm/hyp/nvhe/Makefile |  5 +++--
 2 files changed, 3 insertions(+), 21 deletions(-)

diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index b3de24d7ecd1..e272eedfe19a 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -61,26 +61,8 @@ __efistub__ctype = _ctype;
  * memory mappings.
  */
 
-__hyp_text___fpsimd_restore_state = __fpsimd_restore_state;
-__hyp_text___fpsimd_save_state = __fpsimd_save_state;
-__hyp_text___guest_enter = __guest_enter;
-__hyp_text___guest_exit = __guest_exit;
 __hyp_text___icache_flags = __icache_flags;
 __hyp_text___kvm_handle_stub_hvc = __kvm_handle_stub_hvc;
-__hyp_text___vgic_v2_perform_cpuif_access = __vgic_v2_perform_cpuif_access;
-__hyp_text___vgic_v3_activate_traps = __vgic_v3_activate_traps;
-__hyp_text___vgic_v3_deactivate_traps = __vgic_v3_deactivate_traps;
-__hyp_text___vgic_v3_get_ich_vtr_el2 = __vgic_v3_get_ich_vtr_el2;
-__hyp_text___vgic_v3_init_lrs = __vgic_v3_init_lrs;
-__hyp_text___vgic_v3_perform_cpuif_access = __vgic_v3_perform_cpuif_access;
-__hyp_text___vgic_v3_read_vmcr = __vgic_v3_read_vmcr;
-__hyp_text___vgic_v3_restore_aprs = __vgic_v3_restore_aprs;
-__hyp_text___vgic_v3_restore_state = __vgic_v3_restore_state;
-__hyp_text___vgic_v3_save_aprs = __vgic_v3_save_aprs;
-__hyp_text___vgic_v3_save_state = __vgic_v3_save_state;
-__hyp_text___vgic_v3_write_vmcr = __vgic_v3_write_vmcr;
-__hyp_text_abort_guest_exit_end = abort_guest_exit_end;
-__hyp_text_abort_guest_exit_start = abort_guest_exit_start;
 __hyp_text_arm64_const_caps_ready = arm64_const_caps_ready;
 __hyp_text_arm64_enable_wa2_handling = arm64_enable_wa2_handling;
 __hyp_text_arm64_ssbd_callback_required = arm64_ssbd_callback_required;
@@ -89,7 +71,6 @@ __hyp_text_cpu_hwcaps = cpu_hwcaps;
 __hyp_text_kimage_voffset = kimage_voffset;
 __hyp_text_kvm_host_data = kvm_host_data;
 __hyp_text_kvm_patch_vector_branch = kvm_patch_vector_branch;
-__hyp_text_kvm_skip_instr32 = kvm_skip_instr32;
 __hyp_text_kvm_update_va_mask = kvm_update_va_mask;
 __hyp_text_kvm_vgic_global_state = kvm_vgic_global_state;
 __hyp_text_panic = panic;
diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile
index 2b8286ee8138..41018d25118c 100644
--- a/arch/arm64/kvm/hyp/nvhe/Makefile
+++ b/arch/arm64/kvm/hyp/nvhe/Makefile
@@ -7,8 +7,9 @@ asflags-y := -D__HYPERVISOR__
 ccflags-y := -D__HYPERVISOR__ -fno-stack-protector -DDISABLE_BRANCH_PROFILING \
 $(DISABLE_STACKLEAK_PLUGIN)
 
-obj-y := ../timer-sr.o timer-sr.o sysreg-sr.o debug-sr.o switch.o tlb.o \
-host_hypercall.o ../hyp-entry.o
+obj-y := ../vgic-v3-sr.o ../timer-sr.o timer-sr.o ../aarch32.o \
+../vgic-v2-cpuif-proxy.o sysreg-sr.o debug-sr.o ../entry.o switch.o \
+../fpsimd.o tlb.o host_hypercall.o ../hyp-entry.o
 
 obj-y := $(patsubst %.o,%.hyp.o,$(obj-y))
 extra-y := $(patsubst %.hyp.o,%.hyp.tmp.o,$(obj-y))
-- 
2.26.1

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


[PATCH 11/15] arm64: kvm: Split hyp/timer-sr.c to VHE/nVHE

2020-04-30 Thread David Brazdil
This patch is part of a series which builds KVM's non-VHE hyp code separately
from VHE and the rest of the kernel.

timer-sr.c contains a HVC handler for setting CNTVOFF_EL2 and two helper
functions for controlling access to physical counter. The former is shared
between VHE/nVHE and is kept in timer-sr.c but compiled under both configs.
The latter are nVHE-specific and are moved to nvhe/timer-sr.c.

Signed-off-by: David Brazdil 
---
 arch/arm64/include/asm/kvm_hyp.h   |  2 ++
 arch/arm64/kernel/image-vars.h |  3 ---
 arch/arm64/kvm/hyp/nvhe/Makefile   |  3 ++-
 arch/arm64/kvm/hyp/nvhe/timer-sr.c | 43 ++
 arch/arm64/kvm/hyp/timer-sr.c  | 36 -
 5 files changed, 47 insertions(+), 40 deletions(-)
 create mode 100644 arch/arm64/kvm/hyp/nvhe/timer-sr.c

diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
index c2bcd5dea030..3320a22a5fb1 100644
--- a/arch/arm64/include/asm/kvm_hyp.h
+++ b/arch/arm64/include/asm/kvm_hyp.h
@@ -64,8 +64,10 @@ void __vgic_v3_save_aprs(struct kvm_vcpu *vcpu);
 void __vgic_v3_restore_aprs(struct kvm_vcpu *vcpu);
 int __vgic_v3_perform_cpuif_access(struct kvm_vcpu *vcpu);
 
+#ifdef __HYPERVISOR__
 void __timer_enable_traps(struct kvm_vcpu *vcpu);
 void __timer_disable_traps(struct kvm_vcpu *vcpu);
+#endif
 
 #ifdef __HYPERVISOR__
 void __sysreg_save_state_nvhe(struct kvm_cpu_context *ctxt);
diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index c4ff4a61eb5d..b3de24d7ecd1 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -67,9 +67,6 @@ __hyp_text___guest_enter = __guest_enter;
 __hyp_text___guest_exit = __guest_exit;
 __hyp_text___icache_flags = __icache_flags;
 __hyp_text___kvm_handle_stub_hvc = __kvm_handle_stub_hvc;
-__hyp_text___kvm_timer_set_cntvoff = __kvm_timer_set_cntvoff;
-__hyp_text___timer_disable_traps = __timer_disable_traps;
-__hyp_text___timer_enable_traps = __timer_enable_traps;
 __hyp_text___vgic_v2_perform_cpuif_access = __vgic_v2_perform_cpuif_access;
 __hyp_text___vgic_v3_activate_traps = __vgic_v3_activate_traps;
 __hyp_text___vgic_v3_deactivate_traps = __vgic_v3_deactivate_traps;
diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile
index cfb55c01b3ff..2b8286ee8138 100644
--- a/arch/arm64/kvm/hyp/nvhe/Makefile
+++ b/arch/arm64/kvm/hyp/nvhe/Makefile
@@ -7,7 +7,8 @@ asflags-y := -D__HYPERVISOR__
 ccflags-y := -D__HYPERVISOR__ -fno-stack-protector -DDISABLE_BRANCH_PROFILING \
 $(DISABLE_STACKLEAK_PLUGIN)
 
-obj-y := sysreg-sr.o debug-sr.o switch.o tlb.o host_hypercall.o ../hyp-entry.o
+obj-y := ../timer-sr.o timer-sr.o sysreg-sr.o debug-sr.o switch.o tlb.o \
+host_hypercall.o ../hyp-entry.o
 
 obj-y := $(patsubst %.o,%.hyp.o,$(obj-y))
 extra-y := $(patsubst %.hyp.o,%.hyp.tmp.o,$(obj-y))
diff --git a/arch/arm64/kvm/hyp/nvhe/timer-sr.c 
b/arch/arm64/kvm/hyp/nvhe/timer-sr.c
new file mode 100644
index ..f0e694743883
--- /dev/null
+++ b/arch/arm64/kvm/hyp/nvhe/timer-sr.c
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2012-2015 - ARM Ltd
+ * Author: Marc Zyngier 
+ */
+
+#include 
+#include 
+#include 
+
+#include 
+
+/*
+ * Should only be called on non-VHE systems.
+ * VHE systems use EL2 timers and configure EL1 timers in kvm_timer_init_vhe().
+ */
+void __hyp_text __timer_disable_traps(struct kvm_vcpu *vcpu)
+{
+   u64 val;
+
+   /* Allow physical timer/counter access for the host */
+   val = read_sysreg(cnthctl_el2);
+   val |= CNTHCTL_EL1PCTEN | CNTHCTL_EL1PCEN;
+   write_sysreg(val, cnthctl_el2);
+}
+
+/*
+ * Should only be called on non-VHE systems.
+ * VHE systems use EL2 timers and configure EL1 timers in kvm_timer_init_vhe().
+ */
+void __hyp_text __timer_enable_traps(struct kvm_vcpu *vcpu)
+{
+   u64 val;
+
+   /*
+* Disallow physical timer access for the guest
+* Physical counter access is allowed
+*/
+   val = read_sysreg(cnthctl_el2);
+   val &= ~CNTHCTL_EL1PCEN;
+   val |= CNTHCTL_EL1PCTEN;
+   write_sysreg(val, cnthctl_el2);
+}
diff --git a/arch/arm64/kvm/hyp/timer-sr.c b/arch/arm64/kvm/hyp/timer-sr.c
index ff76e6845fe4..46e303281a2c 100644
--- a/arch/arm64/kvm/hyp/timer-sr.c
+++ b/arch/arm64/kvm/hyp/timer-sr.c
@@ -4,10 +4,6 @@
  * Author: Marc Zyngier 
  */
 
-#include 
-#include 
-#include 
-
 #include 
 
 void __hyp_text __kvm_timer_set_cntvoff(u32 cntvoff_low, u32 cntvoff_high)
@@ -15,35 +11,3 @@ void __hyp_text __kvm_timer_set_cntvoff(u32 cntvoff_low, u32 
cntvoff_high)
u64 cntvoff = (u64)cntvoff_high << 32 | cntvoff_low;
write_sysreg(cntvoff, cntvoff_el2);
 }
-
-/*
- * Should only be called on non-VHE systems.
- * VHE systems use EL2 timers and configure EL1 timers in kvm_timer_init_vhe().
- */
-void __hyp_text __timer_disable_traps(struct kvm_vcpu *vcpu)
-{
-   u64 val;
-
-   /* Allow physical timer/counter access for 

[PATCH 14/15] arm64: kvm: Remove __hyp_text macro, use build rules instead

2020-04-30 Thread David Brazdil
With nVHE code now fully separated from the rest of the kernel, the effects of
the __hyp_text macro (which had to be applied on all nVHE code) can be
achieved with build rules instead. The macro used to:
  (a) move code to .hyp.text ELF section, now done by renaming .text using
  `objcopy`, and
  (b) `notrace` would negate effects of CC_FLAGS_FTRACE, now those flags are
  erased from KBUILD_CFLAGS (same way as in EFI stub).

Note that by removing __hyp_text from code shared with VHE, all VHE code is now
compiled into .text and without `notrace`.

Use of '.pushsection .hyp.text' removed from assembly files as this is now also
covered by the build rules.

For MAINTAINERS: if needed to re-run, uses of macro were removed with the
following command. Formatting was fixed up manually.

  find arch/arm64/kvm/hyp -type f -name '*.c' -o -name '*.h' \
   -exec sed -i 's/ __hyp_text//g' {} +

Signed-off-by: David Brazdil 
---
 arch/arm64/include/asm/kvm_emulate.h |   2 +-
 arch/arm64/include/asm/kvm_hyp.h |   4 +-
 arch/arm64/kvm/hyp/aarch32.c |   6 +-
 arch/arm64/kvm/hyp/debug-sr.h|  18 ++--
 arch/arm64/kvm/hyp/entry.S   |   1 -
 arch/arm64/kvm/hyp/fpsimd.S  |   1 -
 arch/arm64/kvm/hyp/hyp-entry.S   |   3 -
 arch/arm64/kvm/hyp/nvhe/Makefile |   7 +-
 arch/arm64/kvm/hyp/nvhe/debug-sr.c   |  10 +-
 arch/arm64/kvm/hyp/nvhe/host_hypercall.c |   2 +-
 arch/arm64/kvm/hyp/nvhe/switch.c |  18 ++--
 arch/arm64/kvm/hyp/nvhe/sysreg-sr.c  |  10 +-
 arch/arm64/kvm/hyp/nvhe/timer-sr.c   |   4 +-
 arch/arm64/kvm/hyp/nvhe/tlb.c|  14 ++-
 arch/arm64/kvm/hyp/switch.h  |  35 +++---
 arch/arm64/kvm/hyp/sysreg-sr.h   |  27 ++---
 arch/arm64/kvm/hyp/timer-sr.c|   2 +-
 arch/arm64/kvm/hyp/tlb.c |   6 +-
 arch/arm64/kvm/hyp/tlb.h |  15 ++-
 arch/arm64/kvm/hyp/vgic-v2-cpuif-proxy.c |   4 +-
 arch/arm64/kvm/hyp/vgic-v3-sr.c  | 130 ++-
 21 files changed, 142 insertions(+), 177 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_emulate.h 
b/arch/arm64/include/asm/kvm_emulate.h
index a30b4eec7cb4..1666ecbfaac7 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -520,7 +520,7 @@ static __always_inline void kvm_skip_instr(struct kvm_vcpu 
*vcpu, bool is_wide_i
  * Skip an instruction which has been emulated at hyp while most guest sysregs
  * are live.
  */
-static __always_inline void __hyp_text __kvm_skip_instr(struct kvm_vcpu *vcpu)
+static __always_inline void __kvm_skip_instr(struct kvm_vcpu *vcpu)
 {
*vcpu_pc(vcpu) = read_sysreg_el2(SYS_ELR);
vcpu->arch.ctxt.gp_regs.regs.pstate = read_sysreg_el2(SYS_SPSR);
diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
index 3320a22a5fb1..ab0bc4fd5f47 100644
--- a/arch/arm64/include/asm/kvm_hyp.h
+++ b/arch/arm64/include/asm/kvm_hyp.h
@@ -13,8 +13,6 @@
 #include 
 #include 
 
-#define __hyp_text __section(.hyp.text) notrace
-
 #define read_sysreg_elx(r,nvh,vh)  \
({  \
u64 reg;\
@@ -103,7 +101,7 @@ void __noreturn __hyp_do_panic(unsigned long, ...);
  * Must be called from hyp code running at EL2 with an updated VTTBR
  * and interrupts disabled.
  */
-static __always_inline void __hyp_text __load_guest_stage2(struct kvm *kvm)
+static __always_inline void __load_guest_stage2(struct kvm *kvm)
 {
write_sysreg(kvm->arch.vtcr, vtcr_el2);
write_sysreg(kvm_get_vttbr(kvm), vttbr_el2);
diff --git a/arch/arm64/kvm/hyp/aarch32.c b/arch/arm64/kvm/hyp/aarch32.c
index d31f267961e7..44fecab99bbe 100644
--- a/arch/arm64/kvm/hyp/aarch32.c
+++ b/arch/arm64/kvm/hyp/aarch32.c
@@ -44,7 +44,7 @@ static const unsigned short cc_map[16] = {
 /*
  * Check if a trapped instruction should have been executed or not.
  */
-bool __hyp_text kvm_condition_valid32(const struct kvm_vcpu *vcpu)
+bool kvm_condition_valid32(const struct kvm_vcpu *vcpu)
 {
unsigned long cpsr;
u32 cpsr_cond;
@@ -93,7 +93,7 @@ bool __hyp_text kvm_condition_valid32(const struct kvm_vcpu 
*vcpu)
  *
  * IT[7:0] -> CPSR[26:25],CPSR[15:10]
  */
-static void __hyp_text kvm_adjust_itstate(struct kvm_vcpu *vcpu)
+static void kvm_adjust_itstate(struct kvm_vcpu *vcpu)
 {
unsigned long itbits, cond;
unsigned long cpsr = *vcpu_cpsr(vcpu);
@@ -123,7 +123,7 @@ static void __hyp_text kvm_adjust_itstate(struct kvm_vcpu 
*vcpu)
  * kvm_skip_instr - skip a trapped instruction and proceed to the next
  * @vcpu: The vcpu pointer
  */
-void __hyp_text kvm_skip_instr32(struct kvm_vcpu *vcpu, bool is_wide_instr)
+void kvm_skip_instr32(struct kvm_vcpu *vcpu, bool is_wide_instr)
 {
bool is_thumb;
 
diff --git a/arch/arm64/kvm/hyp/debug-sr.h 

[PATCH 08/15] arm64: kvm: Split hyp/switch.c to VHE/nVHE

2020-04-30 Thread David Brazdil
This patch is part of a series which builds KVM's non-VHE hyp code separately
from VHE and the rest of the kernel.

switch.c implements context-switching for KVM, with large parts shared between
VHE/nVHE. These common routines are moved to switch.h, VHE-specific code is
left in switch.c and nVHE-specific code is moved to nvhe/switch.c.

Previously __kvm_vcpu_run needed a different symbol name for VHE/nVHE. This
is cleaned up and the caller in arm.c simplified.

Signed-off-by: David Brazdil 
---
 arch/arm64/include/asm/kvm_asm.h |   4 +-
 arch/arm64/include/asm/kvm_host_hypercalls.h |   4 +-
 arch/arm64/include/asm/kvm_hyp.h |   5 +
 arch/arm64/kernel/image-vars.h   |  25 +-
 arch/arm64/kvm/arm.c |   6 +-
 arch/arm64/kvm/hyp/hyp-entry.S   |   2 +
 arch/arm64/kvm/hyp/nvhe/Makefile |   2 +-
 arch/arm64/kvm/hyp/nvhe/switch.c | 271 
 arch/arm64/kvm/hyp/switch.c  | 688 +--
 arch/arm64/kvm/hyp/switch.h  | 441 
 arch/arm64/kvm/hyp/sysreg-sr.c   |   4 +-
 11 files changed, 764 insertions(+), 688 deletions(-)
 create mode 100644 arch/arm64/kvm/hyp/nvhe/switch.c
 create mode 100644 arch/arm64/kvm/hyp/switch.h

diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
index cdaf3df8085d..0cb229b9e148 100644
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -86,9 +86,7 @@ extern void __kvm_tlb_flush_local_vmid(struct kvm_vcpu *vcpu);
 
 extern void __kvm_timer_set_cntvoff(u32 cntvoff_low, u32 cntvoff_high);
 
-extern int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu);
-
-extern int __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu);
+extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu);
 
 extern u64 __vgic_v3_get_ich_vtr_el2(void);
 extern u64 __vgic_v3_read_vmcr(void);
diff --git a/arch/arm64/include/asm/kvm_host_hypercalls.h 
b/arch/arm64/include/asm/kvm_host_hypercalls.h
index af8ad505d816..cc45930fdc76 100644
--- a/arch/arm64/include/asm/kvm_host_hypercalls.h
+++ b/arch/arm64/include/asm/kvm_host_hypercalls.h
@@ -22,8 +22,8 @@ __KVM_HOST_HCALL(__kvm_tlb_flush_local_vmid)
 #define __KVM_HOST_HCALL_TABLE_IDX___kvm_flush_vm_context  4
 __KVM_HOST_HCALL(__kvm_flush_vm_context)
 
-#define __KVM_HOST_HCALL_TABLE_IDX___kvm_vcpu_run_nvhe 5
-__KVM_HOST_HCALL(__kvm_vcpu_run_nvhe)
+#define __KVM_HOST_HCALL_TABLE_IDX___kvm_vcpu_run  5
+__KVM_HOST_HCALL(__kvm_vcpu_run)
 
 #define __KVM_HOST_HCALL_TABLE_IDX___kvm_tlb_flush_vmid6
 __KVM_HOST_HCALL(__kvm_tlb_flush_vmid)
diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
index fe57f60f06a8..b5895040c16a 100644
--- a/arch/arm64/include/asm/kvm_hyp.h
+++ b/arch/arm64/include/asm/kvm_hyp.h
@@ -82,11 +82,16 @@ void __debug_switch_to_host(struct kvm_vcpu *vcpu);
 void __fpsimd_save_state(struct user_fpsimd_state *fp_regs);
 void __fpsimd_restore_state(struct user_fpsimd_state *fp_regs);
 
+#ifndef __HYPERVISOR__
 void activate_traps_vhe_load(struct kvm_vcpu *vcpu);
 void deactivate_traps_vhe_put(void);
+#endif
 
 u64 __guest_enter(struct kvm_vcpu *vcpu, struct kvm_cpu_context *host_ctxt);
+
+#ifdef __HYPERVISOR__
 void __noreturn __hyp_do_panic(unsigned long, ...);
+#endif
 
 /*
  * Must be called from hyp code running at EL2 with an updated VTTBR
diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index c0a0ec238854..e2282a4304c5 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -61,18 +61,34 @@ __efistub__ctype= _ctype;
  * memory mappings.
  */
 
+__hyp_text___debug_switch_to_guest = __debug_switch_to_guest;
+__hyp_text___debug_switch_to_host = __debug_switch_to_host;
+__hyp_text___fpsimd_restore_state = __fpsimd_restore_state;
+__hyp_text___fpsimd_save_state = __fpsimd_save_state;
+__hyp_text___guest_enter = __guest_enter;
 __hyp_text___guest_exit = __guest_exit;
 __hyp_text___icache_flags = __icache_flags;
 __hyp_text___kvm_enable_ssbs = __kvm_enable_ssbs;
 __hyp_text___kvm_get_mdcr_el2 = __kvm_get_mdcr_el2;
 __hyp_text___kvm_handle_stub_hvc = __kvm_handle_stub_hvc;
 __hyp_text___kvm_timer_set_cntvoff = __kvm_timer_set_cntvoff;
-__hyp_text___kvm_vcpu_run_nvhe = __kvm_vcpu_run_nvhe;
+__hyp_text___sysreg32_restore_state = __sysreg32_restore_state;
+__hyp_text___sysreg32_save_state = __sysreg32_save_state;
+__hyp_text___sysreg_restore_state_nvhe = __sysreg_restore_state_nvhe;
+__hyp_text___sysreg_save_state_nvhe = __sysreg_save_state_nvhe;
+__hyp_text___timer_disable_traps = __timer_disable_traps;
+__hyp_text___timer_enable_traps = __timer_enable_traps;
+__hyp_text___vgic_v2_perform_cpuif_access = __vgic_v2_perform_cpuif_access;
+__hyp_text___vgic_v3_activate_traps = __vgic_v3_activate_traps;
+__hyp_text___vgic_v3_deactivate_traps = __vgic_v3_deactivate_traps;
 __hyp_text___vgic_v3_get_ich_vtr_el2 = __vgic_v3_get_ich_vtr_el2;
 

[PATCH 10/15] arm64: kvm: Split hyp/sysreg-sr.c to VHE/nVHE

2020-04-30 Thread David Brazdil
This patch is part of a series which builds KVM's non-VHE hyp code separately
from VHE and the rest of the kernel.

sysreg-sr.c contains KVM's code for saving/restoring system registers, with
some parts shared between VHE/nVHE. These common routines are moved to
sysreg-sr.h, VHE-specific code is left in sysreg-sr.c and nVHE-specific code is
moved to nvhe/sysreg-sr.c.

Naming of helper functions is simplified as VHE/nVHE symbol names cannot clash
anymore.

Signed-off-by: David Brazdil 
---
 arch/arm64/include/asm/kvm_asm.h|   4 +
 arch/arm64/include/asm/kvm_host.h   |   4 +-
 arch/arm64/include/asm/kvm_hyp.h|   4 +
 arch/arm64/kernel/image-vars.h  |   5 -
 arch/arm64/kvm/hyp/nvhe/Makefile|   2 +-
 arch/arm64/kvm/hyp/nvhe/sysreg-sr.c |  56 +++
 arch/arm64/kvm/hyp/sysreg-sr.c  | 227 +---
 arch/arm64/kvm/hyp/sysreg-sr.h  | 218 ++
 8 files changed, 290 insertions(+), 230 deletions(-)
 create mode 100644 arch/arm64/kvm/hyp/nvhe/sysreg-sr.c
 create mode 100644 arch/arm64/kvm/hyp/sysreg-sr.h

diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
index 0cb229b9e148..646201a6576e 100644
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -88,6 +88,10 @@ extern void __kvm_timer_set_cntvoff(u32 cntvoff_low, u32 
cntvoff_high);
 
 extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu);
 
+#ifdef __HYPERVISOR__
+extern void __kvm_enable_ssbs(void);
+#endif
+
 extern u64 __vgic_v3_get_ich_vtr_el2(void);
 extern u64 __vgic_v3_read_vmcr(void);
 extern void __vgic_v3_write_vmcr(u32 vmcr);
diff --git a/arch/arm64/include/asm/kvm_host.h 
b/arch/arm64/include/asm/kvm_host.h
index 5dec3b06f2b7..e4abe9d759bc 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -547,8 +547,6 @@ static inline void kvm_init_host_cpu_context(struct 
kvm_cpu_context *cpu_ctxt)
cpu_ctxt->sys_regs[MPIDR_EL1] = read_cpuid_mpidr();
 }
 
-void __kvm_enable_ssbs(void);
-
 static inline void __cpu_init_hyp_mode(phys_addr_t pgd_ptr,
   unsigned long hyp_stack_ptr,
   unsigned long vector_ptr)
@@ -577,7 +575,7 @@ static inline void __cpu_init_hyp_mode(phys_addr_t pgd_ptr,
 */
if (!has_vhe() && this_cpu_has_cap(ARM64_SSBS) &&
arm64_get_ssbd_state() == ARM64_SSBD_FORCE_DISABLE) {
-   kvm_call_hyp(__kvm_enable_ssbs);
+   kvm_call_hyp_nvhe(__kvm_enable_ssbs);
}
 }
 
diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
index b5895040c16a..c2bcd5dea030 100644
--- a/arch/arm64/include/asm/kvm_hyp.h
+++ b/arch/arm64/include/asm/kvm_hyp.h
@@ -67,12 +67,16 @@ int __vgic_v3_perform_cpuif_access(struct kvm_vcpu *vcpu);
 void __timer_enable_traps(struct kvm_vcpu *vcpu);
 void __timer_disable_traps(struct kvm_vcpu *vcpu);
 
+#ifdef __HYPERVISOR__
 void __sysreg_save_state_nvhe(struct kvm_cpu_context *ctxt);
 void __sysreg_restore_state_nvhe(struct kvm_cpu_context *ctxt);
+#else
 void sysreg_save_host_state_vhe(struct kvm_cpu_context *ctxt);
 void sysreg_restore_host_state_vhe(struct kvm_cpu_context *ctxt);
 void sysreg_save_guest_state_vhe(struct kvm_cpu_context *ctxt);
 void sysreg_restore_guest_state_vhe(struct kvm_cpu_context *ctxt);
+#endif
+
 void __sysreg32_save_state(struct kvm_vcpu *vcpu);
 void __sysreg32_restore_state(struct kvm_vcpu *vcpu);
 
diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index 0e9ede9c473a..c4ff4a61eb5d 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -66,13 +66,8 @@ __hyp_text___fpsimd_save_state = __fpsimd_save_state;
 __hyp_text___guest_enter = __guest_enter;
 __hyp_text___guest_exit = __guest_exit;
 __hyp_text___icache_flags = __icache_flags;
-__hyp_text___kvm_enable_ssbs = __kvm_enable_ssbs;
 __hyp_text___kvm_handle_stub_hvc = __kvm_handle_stub_hvc;
 __hyp_text___kvm_timer_set_cntvoff = __kvm_timer_set_cntvoff;
-__hyp_text___sysreg32_restore_state = __sysreg32_restore_state;
-__hyp_text___sysreg32_save_state = __sysreg32_save_state;
-__hyp_text___sysreg_restore_state_nvhe = __sysreg_restore_state_nvhe;
-__hyp_text___sysreg_save_state_nvhe = __sysreg_save_state_nvhe;
 __hyp_text___timer_disable_traps = __timer_disable_traps;
 __hyp_text___timer_enable_traps = __timer_enable_traps;
 __hyp_text___vgic_v2_perform_cpuif_access = __vgic_v2_perform_cpuif_access;
diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile
index bca3b862927c..cfb55c01b3ff 100644
--- a/arch/arm64/kvm/hyp/nvhe/Makefile
+++ b/arch/arm64/kvm/hyp/nvhe/Makefile
@@ -7,7 +7,7 @@ asflags-y := -D__HYPERVISOR__
 ccflags-y := -D__HYPERVISOR__ -fno-stack-protector -DDISABLE_BRANCH_PROFILING \
 $(DISABLE_STACKLEAK_PLUGIN)
 
-obj-y := debug-sr.o switch.o tlb.o host_hypercall.o ../hyp-entry.o
+obj-y := sysreg-sr.o debug-sr.o switch.o tlb.o host_hypercall.o 

[PATCH 09/15] arm64: kvm: Split hyp/debug-sr.c to VHE/nVHE

2020-04-30 Thread David Brazdil
This patch is part of a series which builds KVM's non-VHE hyp code separately
from VHE and the rest of the kernel.

debug-sr.c contains KVM's code for context-switching debug registers, with some
parts shared between VHE/nVHE. These common routines are moved to debug-sr.h,
VHE-specific code is left in debug-sr.c and nVHE-specific code is moved to
nvhe/debug-sr.c.

Functions are slightly refactored to move code hidden behind `has_vhe()` checks
to the corresponding .c file.

Signed-off-by: David Brazdil 
---
 arch/arm64/kernel/image-vars.h |   3 -
 arch/arm64/kvm/hyp/debug-sr.c  | 214 +
 arch/arm64/kvm/hyp/debug-sr.h  | 167 ++
 arch/arm64/kvm/hyp/nvhe/Makefile   |   2 +-
 arch/arm64/kvm/hyp/nvhe/debug-sr.c |  77 +++
 5 files changed, 251 insertions(+), 212 deletions(-)
 create mode 100644 arch/arm64/kvm/hyp/debug-sr.h
 create mode 100644 arch/arm64/kvm/hyp/nvhe/debug-sr.c

diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index e2282a4304c5..0e9ede9c473a 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -61,15 +61,12 @@ __efistub__ctype= _ctype;
  * memory mappings.
  */
 
-__hyp_text___debug_switch_to_guest = __debug_switch_to_guest;
-__hyp_text___debug_switch_to_host = __debug_switch_to_host;
 __hyp_text___fpsimd_restore_state = __fpsimd_restore_state;
 __hyp_text___fpsimd_save_state = __fpsimd_save_state;
 __hyp_text___guest_enter = __guest_enter;
 __hyp_text___guest_exit = __guest_exit;
 __hyp_text___icache_flags = __icache_flags;
 __hyp_text___kvm_enable_ssbs = __kvm_enable_ssbs;
-__hyp_text___kvm_get_mdcr_el2 = __kvm_get_mdcr_el2;
 __hyp_text___kvm_handle_stub_hvc = __kvm_handle_stub_hvc;
 __hyp_text___kvm_timer_set_cntvoff = __kvm_timer_set_cntvoff;
 __hyp_text___sysreg32_restore_state = __sysreg32_restore_state;
diff --git a/arch/arm64/kvm/hyp/debug-sr.c b/arch/arm64/kvm/hyp/debug-sr.c
index 0fc9872a1467..39e0b9bcc8b7 100644
--- a/arch/arm64/kvm/hyp/debug-sr.c
+++ b/arch/arm64/kvm/hyp/debug-sr.c
@@ -4,221 +4,19 @@
  * Author: Marc Zyngier 
  */
 
-#include 
-#include 
+#include "debug-sr.h"
 
-#include 
-#include 
-#include 
-#include 
-
-#define read_debug(r,n)read_sysreg(r##n##_el1)
-#define write_debug(v,r,n) write_sysreg(v, r##n##_el1)
-
-#define save_debug(ptr,reg,nr) \
-   switch (nr) {   \
-   case 15:ptr[15] = read_debug(reg, 15);  \
-   /* Fall through */  \
-   case 14:ptr[14] = read_debug(reg, 14);  \
-   /* Fall through */  \
-   case 13:ptr[13] = read_debug(reg, 13);  \
-   /* Fall through */  \
-   case 12:ptr[12] = read_debug(reg, 12);  \
-   /* Fall through */  \
-   case 11:ptr[11] = read_debug(reg, 11);  \
-   /* Fall through */  \
-   case 10:ptr[10] = read_debug(reg, 10);  \
-   /* Fall through */  \
-   case 9: ptr[9] = read_debug(reg, 9);\
-   /* Fall through */  \
-   case 8: ptr[8] = read_debug(reg, 8);\
-   /* Fall through */  \
-   case 7: ptr[7] = read_debug(reg, 7);\
-   /* Fall through */  \
-   case 6: ptr[6] = read_debug(reg, 6);\
-   /* Fall through */  \
-   case 5: ptr[5] = read_debug(reg, 5);\
-   /* Fall through */  \
-   case 4: ptr[4] = read_debug(reg, 4);\
-   /* Fall through */  \
-   case 3: ptr[3] = read_debug(reg, 3);\
-   /* Fall through */  \
-   case 2: ptr[2] = read_debug(reg, 2);\
-   /* Fall through */  \
-   case 1: ptr[1] = read_debug(reg, 1);\
-   /* Fall through */  \
-   default:ptr[0] = read_debug(reg, 0);\
-   }
-
-#define restore_debug(ptr,reg,nr)  \
-   switch (nr) {   \
-   case 15:write_debug(ptr[15], reg, 

[PATCH 15/15] arm64: kvm: Lift instrumentation restrictions on VHE

2020-04-30 Thread David Brazdil
With VHE and nVHE executable code completely separated, remove build config
that disabled GCOV/KASAN/UBSAN/KCOV instrumentation for VHE as these now
execute under the same memory mappings as the rest of the kernel.

No violations are currently being reported by either KASAN or UBSAN.

Signed-off-by: David Brazdil 
---
 arch/arm64/kvm/hyp/Makefile | 8 
 1 file changed, 8 deletions(-)

diff --git a/arch/arm64/kvm/hyp/Makefile b/arch/arm64/kvm/hyp/Makefile
index a6883f4fed9e..f877ac404b46 100644
--- a/arch/arm64/kvm/hyp/Makefile
+++ b/arch/arm64/kvm/hyp/Makefile
@@ -10,11 +10,3 @@ obj-$(CONFIG_KVM) += vhe.o nvhe/
 
 vhe-y := vgic-v3-sr.o timer-sr.o aarch32.o vgic-v2-cpuif-proxy.o sysreg-sr.o \
 debug-sr.o entry.o switch.o fpsimd.o tlb.o hyp-entry.o
-
-# KVM code is run at a different exception code with a different map, so
-# compiler instrumentation that inserts callbacks or checks into the code may
-# cause crashes. Just disable it.
-GCOV_PROFILE   := n
-KASAN_SANITIZE := n
-UBSAN_SANITIZE := n
-KCOV_INSTRUMENT:= n
-- 
2.26.1

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


[PATCH 07/15] arm64: kvm: Split hyp/tlb.c to VHE/nVHE

2020-04-30 Thread David Brazdil
This patch is part of a series which builds KVM's non-VHE hyp code separately
from VHE and the rest of the kernel.

tlb.c contains code for flushing the TLB, with parts shared between VHE/nVHE.
These common routines are moved into a header file tlb.h, VHE-specific code
remains in tlb.c and nVHE-specific code is moved to nvhe/tlb.c.

The header file expects its users to implement two helper functions declared
at the top of the file.

Signed-off-by: David Brazdil 
---
 arch/arm64/kernel/image-vars.h   |   8 +-
 arch/arm64/kvm/hyp/nvhe/Makefile |   2 +-
 arch/arm64/kvm/hyp/nvhe/tlb.c|  69 +
 arch/arm64/kvm/hyp/tlb.c | 170 +++
 arch/arm64/kvm/hyp/tlb.h | 129 +++
 5 files changed, 216 insertions(+), 162 deletions(-)
 create mode 100644 arch/arm64/kvm/hyp/nvhe/tlb.c
 create mode 100644 arch/arm64/kvm/hyp/tlb.h

diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index 59c53566eceb..c0a0ec238854 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -62,14 +62,11 @@ __efistub__ctype= _ctype;
  */
 
 __hyp_text___guest_exit = __guest_exit;
+__hyp_text___icache_flags = __icache_flags;
 __hyp_text___kvm_enable_ssbs = __kvm_enable_ssbs;
-__hyp_text___kvm_flush_vm_context = __kvm_flush_vm_context;
 __hyp_text___kvm_get_mdcr_el2 = __kvm_get_mdcr_el2;
 __hyp_text___kvm_handle_stub_hvc = __kvm_handle_stub_hvc;
 __hyp_text___kvm_timer_set_cntvoff = __kvm_timer_set_cntvoff;
-__hyp_text___kvm_tlb_flush_local_vmid = __kvm_tlb_flush_local_vmid;
-__hyp_text___kvm_tlb_flush_vmid = __kvm_tlb_flush_vmid;
-__hyp_text___kvm_tlb_flush_vmid_ipa = __kvm_tlb_flush_vmid_ipa;
 __hyp_text___kvm_vcpu_run_nvhe = __kvm_vcpu_run_nvhe;
 __hyp_text___vgic_v3_get_ich_vtr_el2 = __vgic_v3_get_ich_vtr_el2;
 __hyp_text___vgic_v3_init_lrs = __vgic_v3_init_lrs;
@@ -79,8 +76,11 @@ __hyp_text___vgic_v3_save_aprs = __vgic_v3_save_aprs;
 __hyp_text___vgic_v3_write_vmcr = __vgic_v3_write_vmcr;
 __hyp_text_abort_guest_exit_end = abort_guest_exit_end;
 __hyp_text_abort_guest_exit_start = abort_guest_exit_start;
+__hyp_text_arm64_const_caps_ready = arm64_const_caps_ready;
 __hyp_text_arm64_enable_wa2_handling = arm64_enable_wa2_handling;
 __hyp_text_arm64_ssbd_callback_required = arm64_ssbd_callback_required;
+__hyp_text_cpu_hwcap_keys = cpu_hwcap_keys;
+__hyp_text_cpu_hwcaps = cpu_hwcaps;
 __hyp_text_hyp_panic = hyp_panic;
 __hyp_text_kimage_voffset = kimage_voffset;
 __hyp_text_kvm_host_data = kvm_host_data;
diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile
index fa20b2723652..0da836cb5580 100644
--- a/arch/arm64/kvm/hyp/nvhe/Makefile
+++ b/arch/arm64/kvm/hyp/nvhe/Makefile
@@ -7,7 +7,7 @@ asflags-y := -D__HYPERVISOR__
 ccflags-y := -D__HYPERVISOR__ -fno-stack-protector -DDISABLE_BRANCH_PROFILING \
 $(DISABLE_STACKLEAK_PLUGIN)
 
-obj-y := host_hypercall.o ../hyp-entry.o
+obj-y := tlb.o host_hypercall.o ../hyp-entry.o
 
 obj-y := $(patsubst %.o,%.hyp.o,$(obj-y))
 extra-y := $(patsubst %.hyp.o,%.hyp.tmp.o,$(obj-y))
diff --git a/arch/arm64/kvm/hyp/nvhe/tlb.c b/arch/arm64/kvm/hyp/nvhe/tlb.c
new file mode 100644
index ..1b8f4000f98c
--- /dev/null
+++ b/arch/arm64/kvm/hyp/nvhe/tlb.c
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2015 - ARM Ltd
+ * Author: Marc Zyngier 
+ */
+
+#include 
+
+#include 
+#include 
+#include 
+
+#include "../tlb.h"
+
+static void __hyp_text __tlb_switch_to_guest(struct kvm *kvm,
+struct tlb_inv_context *cxt)
+{
+   if (cpus_have_final_cap(ARM64_WORKAROUND_SPECULATIVE_AT_NVHE)) {
+   u64 val;
+
+   /*
+* For CPUs that are affected by ARM 1319367, we need to
+* avoid a host Stage-1 walk while we have the guest's
+* VMID set in the VTTBR in order to invalidate TLBs.
+* We're guaranteed that the S1 MMU is enabled, so we can
+* simply set the EPD bits to avoid any further TLB fill.
+*/
+   val = cxt->tcr = read_sysreg_el1(SYS_TCR);
+   val |= TCR_EPD1_MASK | TCR_EPD0_MASK;
+   write_sysreg_el1(val, SYS_TCR);
+   isb();
+   }
+
+   __load_guest_stage2(kvm);
+   isb();
+}
+
+static void __hyp_text __tlb_switch_to_host(struct kvm *kvm,
+   struct tlb_inv_context *cxt)
+{
+   write_sysreg(0, vttbr_el2);
+
+   if (cpus_have_final_cap(ARM64_WORKAROUND_SPECULATIVE_AT_NVHE)) {
+   /* Ensure write of the host VMID */
+   isb();
+   /* Restore the host's TCR_EL1 */
+   write_sysreg_el1(cxt->tcr, SYS_TCR);
+   }
+}
+
+void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
+{
+   __tlb_flush_vmid_ipa(kvm, ipa);
+}
+
+void __hyp_text __kvm_tlb_flush_vmid(struct kvm *kvm)
+{
+  

[PATCH 13/15] arm64: kvm: Add comments around __hyp_text_ symbol aliases

2020-04-30 Thread David Brazdil
This patch is part of a series which builds KVM's non-VHE hyp code separately
from VHE and the rest of the kernel.

With all source files split between VHE/nVHE, add comments around the list of
symbols where nVHE code still links against kernel proper. Split them into
groups and explain how each group is currently used.

Some of these dependencies will be removed in the future.

Signed-off-by: David Brazdil 
---
 arch/arm64/kernel/image-vars.h | 49 +-
 1 file changed, 31 insertions(+), 18 deletions(-)

diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index e272eedfe19a..04a3ee21e694 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -61,24 +61,37 @@ __efistub__ctype= _ctype;
  * memory mappings.
  */
 
-__hyp_text___icache_flags = __icache_flags;
-__hyp_text___kvm_handle_stub_hvc = __kvm_handle_stub_hvc;
-__hyp_text_arm64_const_caps_ready = arm64_const_caps_ready;
-__hyp_text_arm64_enable_wa2_handling = arm64_enable_wa2_handling;
-__hyp_text_arm64_ssbd_callback_required = arm64_ssbd_callback_required;
-__hyp_text_cpu_hwcap_keys = cpu_hwcap_keys;
-__hyp_text_cpu_hwcaps = cpu_hwcaps;
-__hyp_text_kimage_voffset = kimage_voffset;
-__hyp_text_kvm_host_data = kvm_host_data;
-__hyp_text_kvm_patch_vector_branch = kvm_patch_vector_branch;
-__hyp_text_kvm_update_va_mask = kvm_update_va_mask;
-__hyp_text_kvm_vgic_global_state = kvm_vgic_global_state;
-__hyp_text_panic = panic;
-__hyp_text_physvirt_offset = physvirt_offset;
-__hyp_text_sve_load_state = sve_load_state;
-__hyp_text_sve_save_state = sve_save_state;
-__hyp_text_vgic_v2_cpuif_trap = vgic_v2_cpuif_trap;
-__hyp_text_vgic_v3_cpuif_trap = vgic_v3_cpuif_trap;
+/* If nVHE code panics, it ERETs into panic() in EL1. */
+__hyp_text_panic   = panic;
+
+/* Stub HVC IDs are routed to a handler in .hyp.idmap.text. Executed in EL2. */
+__hyp_text___kvm_handle_stub_hvc   = __kvm_handle_stub_hvc;
+
+/* Alternative callbacks, referenced in .altinstructions. Executed in EL1. */
+__hyp_text_arm64_enable_wa2_handling   = arm64_enable_wa2_handling;
+__hyp_text_kvm_patch_vector_branch = kvm_patch_vector_branch;
+__hyp_text_kvm_update_va_mask  = kvm_update_va_mask;
+
+/* Values used to convert between memory mappings, read-only after init. */
+__hyp_text_kimage_voffset  = kimage_voffset;
+__hyp_text_physvirt_offset = physvirt_offset;
+
+/* Data shared with the kernel. */
+__hyp_text_cpu_hwcaps  = cpu_hwcaps;
+__hyp_text_cpu_hwcap_keys  = cpu_hwcap_keys;
+__hyp_text___icache_flags  = __icache_flags;
+__hyp_text_kvm_vgic_global_state   = kvm_vgic_global_state;
+__hyp_text_arm64_ssbd_callback_required= arm64_ssbd_callback_required;
+__hyp_text_kvm_host_data   = kvm_host_data;
+
+/* Static keys shared with the kernel. */
+__hyp_text_arm64_const_caps_ready  = arm64_const_caps_ready;
+__hyp_text_vgic_v2_cpuif_trap  = vgic_v2_cpuif_trap;
+__hyp_text_vgic_v3_cpuif_trap  = vgic_v3_cpuif_trap;
+
+/* SVE support, currently unused by nVHE. */
+__hyp_text_sve_save_state  = sve_save_state;
+__hyp_text_sve_load_state  = sve_load_state;
 
 #endif /* CONFIG_KVM */
 
-- 
2.26.1

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


[PATCH 04/15] arm64: kvm: Add build rules for separate nVHE object files

2020-04-30 Thread David Brazdil
Add new folder arch/arm64/kvm/hyp/nvhe and a Makefile for building code that
runs in EL2 under nVHE KVM.

Compile each source file into a `.hyp.tmp.o` object first, then prefix all
its symbols with "__hyp_text_" using `objcopy` and produce a `.hyp.o`.
Suffixes were chosen so that it would be possible for VHE and nVHE to share
some source files, but compiled with different CFLAGS. nVHE build rules add
-D__HYPERVISOR__.

Macros are added for prefixing a nVHE symbol name when referenced from kernel
proper.

Signed-off-by: David Brazdil 
---
 arch/arm64/include/asm/kvm_asm.h | 13 
 arch/arm64/kernel/image-vars.h   | 12 +++
 arch/arm64/kvm/hyp/Makefile  |  4 ++--
 arch/arm64/kvm/hyp/nvhe/Makefile | 35 
 scripts/kallsyms.c   |  1 +
 5 files changed, 63 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm64/kvm/hyp/nvhe/Makefile

diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
index 7c7eeeaab9fa..99ab204519ca 100644
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -42,6 +42,12 @@
 
 #include 
 
+/* Translate the name of @sym to its nVHE equivalent. */
+#define kvm_nvhe_sym(sym)  __hyp_text_##sym
+
+/* Choose between VHE and nVHE variants of a symbol. */
+#define kvm_hyp_sym(sym)   (has_vhe() ? sym : kvm_nvhe_sym(sym))
+
 /* Translate a kernel address of @sym into its equivalent linear mapping */
 #define kvm_ksym_ref(sym)  \
({  \
@@ -51,6 +57,13 @@
val;\
 })
 
+/*
+ * Translate a kernel address of @sym into its equivalent linear mapping,
+ * choosing between VHE and nVHE variant of @sym accordingly.
+ */
+#define kvm_hyp_ref(sym) \
+   (has_vhe() ? kvm_ksym_ref(sym) : kvm_ksym_ref(kvm_nvhe_sym(sym)))
+
 struct kvm;
 struct kvm_vcpu;
 
diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index 7f06ad93fc95..13850134fc28 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -51,4 +51,16 @@ __efistub__ctype = _ctype;
 
 #endif
 
+#ifdef CONFIG_KVM
+
+/*
+ * KVM nVHE code has its own symbol namespace prefixed by __hyp_text_, to
+ * isolate it from the kernel proper. The following symbols are legally
+ * accessed by it, therefore provide aliases to make them linkable.
+ * Do not include symbols which may not be safely accessed under hypervisor
+ * memory mappings.
+ */
+
+#endif /* CONFIG_KVM */
+
 #endif /* __ARM64_KERNEL_IMAGE_VARS_H */
diff --git a/arch/arm64/kvm/hyp/Makefile b/arch/arm64/kvm/hyp/Makefile
index 29e2b2cd2fbc..79bf822a484b 100644
--- a/arch/arm64/kvm/hyp/Makefile
+++ b/arch/arm64/kvm/hyp/Makefile
@@ -6,9 +6,9 @@
 ccflags-y += -fno-stack-protector -DDISABLE_BRANCH_PROFILING \
$(DISABLE_STACKLEAK_PLUGIN)
 
-obj-$(CONFIG_KVM) += hyp.o
+obj-$(CONFIG_KVM) += vhe.o nvhe/
 
-hyp-y := vgic-v3-sr.o timer-sr.o aarch32.o vgic-v2-cpuif-proxy.o sysreg-sr.o \
+vhe-y := vgic-v3-sr.o timer-sr.o aarch32.o vgic-v2-cpuif-proxy.o sysreg-sr.o \
 debug-sr.o entry.o switch.o fpsimd.o tlb.o host_hypercall.o hyp-entry.o
 
 # KVM code is run at a different exception code with a different map, so
diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile
new file mode 100644
index ..873d3ab0fd68
--- /dev/null
+++ b/arch/arm64/kvm/hyp/nvhe/Makefile
@@ -0,0 +1,35 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for Kernel-based Virtual Machine module, HYP/nVHE part
+#
+
+asflags-y := -D__HYPERVISOR__
+ccflags-y := -D__HYPERVISOR__ -fno-stack-protector -DDISABLE_BRANCH_PROFILING \
+$(DISABLE_STACKLEAK_PLUGIN)
+
+obj-y :=
+
+obj-y := $(patsubst %.o,%.hyp.o,$(obj-y))
+extra-y := $(patsubst %.hyp.o,%.hyp.tmp.o,$(obj-y))
+
+$(obj)/%.hyp.tmp.o: $(src)/%.c FORCE
+   $(call if_changed_rule,cc_o_c)
+$(obj)/%.hyp.tmp.o: $(src)/%.S FORCE
+   $(call if_changed_rule,as_o_S)
+$(obj)/%.hyp.o: $(obj)/%.hyp.tmp.o FORCE
+   $(call if_changed,hypcopy)
+
+quiet_cmd_hypcopy = HYPCOPY $@
+  cmd_hypcopy = $(OBJCOPY) --prefix-symbols=__hyp_text_ $< $@
+
+# KVM nVHE code is run at a different exception code with a different map, so
+# compiler instrumentation that inserts callbacks or checks into the code may
+# cause crashes. Just disable it.
+GCOV_PROFILE   := n
+KASAN_SANITIZE := n
+UBSAN_SANITIZE := n
+KCOV_INSTRUMENT:= n
+
+# Skip objtool checking for this directory because nVHE code is compiled with
+# non-standard build rules.
+OBJECT_FILES_NON_STANDARD := y
diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 3e8dea6e0a95..b5c9dc6de38d 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -109,6 +109,7 @@ static bool is_ignored_symbol(const char *name, char type)
".LASANPC", /* s390 kasan local symbols */
 

[PATCH 06/15] arm64: kvm: Move __smccc_workaround_1_smc to .rodata

2020-04-30 Thread David Brazdil
This snippet of assembly is used by cpu_errata.c to overwrite parts of KVM hyp
vector. It is never directly executed, so move it from .text to .rodata.

Signed-off-by: David Brazdil 
---
 arch/arm64/kvm/hyp/hyp-entry.S | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/kvm/hyp/hyp-entry.S b/arch/arm64/kvm/hyp/hyp-entry.S
index 5986e1d78d3f..7e5f386c5c2d 100644
--- a/arch/arm64/kvm/hyp/hyp-entry.S
+++ b/arch/arm64/kvm/hyp/hyp-entry.S
@@ -364,6 +364,11 @@ SYM_CODE_END(__bp_harden_hyp_vecs)
.popsection
 
 #ifndef __HYPERVISOR__
+   /*
+* This is not executed directly and is instead copied into the vectors
+* by install_bp_hardening_cb().
+*/
+   .pushsection.rodata
 SYM_CODE_START(__smccc_workaround_1_smc)
esb
sub sp, sp, #(8 * 4)
@@ -377,5 +382,6 @@ SYM_CODE_START(__smccc_workaround_1_smc)
 1: .org __smccc_workaround_1_smc + __SMCCC_WORKAROUND_1_SMC_SZ
.org 1b
 SYM_CODE_END(__smccc_workaround_1_smc)
+   .popsection
 #endif /* __HYPERVISOR__ */
 #endif /* CONFIG_KVM_INDIRECT_VECTORS */
-- 
2.26.1

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


[PATCH 00/15] Split off nVHE hyp code

2020-04-30 Thread David Brazdil
Refactor files in arch/arm64/kvm/hyp to compile all code which runs in EL2
under nVHE into separate object files from the rest of KVM. This is done in
preparation for being able to unmap .hyp.text from EL1 but has other benefits,
notably:
 * safe use of KASAN/UBSAN/GCOV instrumentation on VHE code,
 * cleaner HVC API,
 * no need for __hyp_text annotations.

nVHE-specific code is moved to hyp/nvhe and compiled with custom build rules
similar to those used by EFI stub. Shared source files are compiled under both
VHE and nVHE build rules. Where a source file contained both VHE and nVHE code,
it is split into a shared header file and two C source files. This is done one
file per commit to make review easier.

All nVHE symbols are prefixed with "__hyp_text_" to avoid collisions with VHE
variants (also inspired by EFI stub). Since this prefixes unresolved symbols
too, image-vars.h contains a list of kernel symbol aliases where nVHE code
still refers to kernel proper. This list will be further reduced in the future.

No functional changes are intended but code was simplified whenever the
refactoring made it possible.

Tested by running kvm-unit-tests on QEMU 5.0 with VHE/nVHE and GIC v2/v3.

This is based off Fuad Tabba's patch series "KVM: arm64: Tidy up arch Kconfig
and Makefiles". Available in branch 'topic/el2-obj' of git repo:
  https://android-kvm.googlesource.com/linux

-David

David Brazdil (13):
  arm64: kvm: Fix symbol dependency in __hyp_call_panic_nvhe
  arm64: kvm: Add build rules for separate nVHE object files
  arm64: kvm: Build hyp-entry.S separately for VHE/nVHE
  arm64: kvm: Move __smccc_workaround_1_smc to .rodata
  arm64: kvm: Split hyp/tlb.c to VHE/nVHE
  arm64: kvm: Split hyp/switch.c to VHE/nVHE
  arm64: kvm: Split hyp/debug-sr.c to VHE/nVHE
  arm64: kvm: Split hyp/sysreg-sr.c to VHE/nVHE
  arm64: kvm: Split hyp/timer-sr.c to VHE/nVHE
  arm64: kvm: Compile remaining hyp/ files for both VHE/nVHE
  arm64: kvm: Add comments around __hyp_text_ symbol aliases
  arm64: kvm: Remove __hyp_text macro, use build rules instead
  arm64: kvm: Lift instrumentation restrictions on VHE

Quentin Perret (2):
  arm64: kvm: Unify users of HVC instruction
  arm64: kvm: Formalize host-hyp hypcall ABI

 arch/arm64/include/asm/kvm_asm.h |  26 +-
 arch/arm64/include/asm/kvm_emulate.h |   2 +-
 arch/arm64/include/asm/kvm_host.h|  32 +-
 arch/arm64/include/asm/kvm_host_hypercalls.h |  62 ++
 arch/arm64/include/asm/kvm_hyp.h |  15 +-
 arch/arm64/include/asm/kvm_mmu.h |  13 +-
 arch/arm64/include/asm/mmu.h |   7 -
 arch/arm64/include/asm/virt.h|  33 +-
 arch/arm64/kernel/cpu_errata.c   |   2 +-
 arch/arm64/kernel/hyp-stub.S |  34 -
 arch/arm64/kernel/image-vars.h   |  44 ++
 arch/arm64/kvm/arm.c |   6 +-
 arch/arm64/kvm/hyp.S |  13 +-
 arch/arm64/kvm/hyp/Makefile  |  12 +-
 arch/arm64/kvm/hyp/aarch32.c |   6 +-
 arch/arm64/kvm/hyp/debug-sr.c| 214 +-
 arch/arm64/kvm/hyp/debug-sr.h| 165 +
 arch/arm64/kvm/hyp/entry.S   |   1 -
 arch/arm64/kvm/hyp/fpsimd.S  |   1 -
 arch/arm64/kvm/hyp/hyp-entry.S   |  62 +-
 arch/arm64/kvm/hyp/nvhe/Makefile |  42 ++
 arch/arm64/kvm/hyp/nvhe/debug-sr.c   |  77 +++
 arch/arm64/kvm/hyp/nvhe/host_hypercall.c |  22 +
 arch/arm64/kvm/hyp/nvhe/switch.c | 271 
 arch/arm64/kvm/hyp/nvhe/sysreg-sr.c  |  56 ++
 arch/arm64/kvm/hyp/nvhe/timer-sr.c   |  43 ++
 arch/arm64/kvm/hyp/nvhe/tlb.c|  67 ++
 arch/arm64/kvm/hyp/switch.c  | 688 +--
 arch/arm64/kvm/hyp/switch.h  | 438 
 arch/arm64/kvm/hyp/sysreg-sr.c   | 227 +-
 arch/arm64/kvm/hyp/sysreg-sr.h   | 211 ++
 arch/arm64/kvm/hyp/timer-sr.c|  38 +-
 arch/arm64/kvm/hyp/tlb.c | 168 +
 arch/arm64/kvm/hyp/tlb.h | 126 
 arch/arm64/kvm/hyp/vgic-v2-cpuif-proxy.c |   4 +-
 arch/arm64/kvm/hyp/vgic-v3-sr.c  | 130 ++--
 arch/arm64/kvm/va_layout.c   |   2 +-
 scripts/kallsyms.c   |   1 +
 38 files changed, 1887 insertions(+), 1474 deletions(-)
 create mode 100644 arch/arm64/include/asm/kvm_host_hypercalls.h
 create mode 100644 arch/arm64/kvm/hyp/debug-sr.h
 create mode 100644 arch/arm64/kvm/hyp/nvhe/Makefile
 create mode 100644 arch/arm64/kvm/hyp/nvhe/debug-sr.c
 create mode 100644 arch/arm64/kvm/hyp/nvhe/host_hypercall.c
 create mode 100644 arch/arm64/kvm/hyp/nvhe/switch.c
 create mode 100644 arch/arm64/kvm/hyp/nvhe/sysreg-sr.c
 create mode 100644 arch/arm64/kvm/hyp/nvhe/timer-sr.c
 create mode 100644 arch/arm64/kvm/hyp/nvhe/tlb.c
 create mode 100644 arch/arm64/kvm/hyp/switch.h
 create mode 100644 

[PATCH 02/15] arm64: kvm: Formalize host-hyp hypcall ABI

2020-04-30 Thread David Brazdil
From: Quentin Perret 

In preparation for removing the hyp code from the TCB, convert the current
EL1 - EL2 KVM ABI to use hypercall numbers in lieu of direct function pointers.
While this in itself does not provide any isolation, it is a first step towards
having a properly defined EL2 ABI.

The implementation is based on a kvm_hcall_table which holds function pointers
to the hyp_text functions corresponding to each hypercall. This is highly
inspired from the arm64 sys_call_table, the main difference being the lack of
hcall wrappers at this stage.

Signed-off-by: Quentin Perret 
Signed-off-by: David Brazdil 
---
 arch/arm64/include/asm/kvm_host.h| 20 ++-
 arch/arm64/include/asm/kvm_host_hypercalls.h | 62 
 arch/arm64/kvm/hyp/Makefile  |  2 +-
 arch/arm64/kvm/hyp/host_hypercall.c  | 22 +++
 arch/arm64/kvm/hyp/hyp-entry.S   | 36 +++-
 5 files changed, 137 insertions(+), 5 deletions(-)
 create mode 100644 arch/arm64/include/asm/kvm_host_hypercalls.h
 create mode 100644 arch/arm64/kvm/hyp/host_hypercall.c

diff --git a/arch/arm64/include/asm/kvm_host.h 
b/arch/arm64/include/asm/kvm_host.h
index e61143d6602d..5dec3b06f2b7 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -447,10 +448,25 @@ int kvm_test_age_hva(struct kvm *kvm, unsigned long hva);
 void kvm_arm_halt_guest(struct kvm *kvm);
 void kvm_arm_resume_guest(struct kvm *kvm);
 
-#define kvm_call_hyp_nvhe(hypfn, ...) \
-   __kvm_call_hyp((unsigned long)kvm_ksym_ref(hypfn), ##__VA_ARGS__)
+/*
+ * Call the hypervisor via HVC. The hcall parameter must be the name of
+ * a hypercall as defined in .
+ *
+ * Hypercalls take at most 6 parameters.
+ */
+#define kvm_call_hyp_nvhe(hcall, ...) \
+   __kvm_call_hyp(KVM_HOST_HCALL_NR(hcall), ##__VA_ARGS__)
 
 /*
+ * u64 kvm_call_hyp(hcall, ...);
+ *
+ * Call the hypervisor. The hcall parameter must be the name of a hypercall
+ * defined in . In the VHE case, this will be
+ * translated into a direct function call.
+ *
+ * A hcall value of less than 0xfff has a special meaning and is used to
+ * implement hyp stubs in the same way as in arch/arm64/kernel/hyp_stub.S.
+ *
  * The couple of isb() below are there to guarantee the same behaviour
  * on VHE as on !VHE, where the eret to EL1 acts as a context
  * synchronization event.
diff --git a/arch/arm64/include/asm/kvm_host_hypercalls.h 
b/arch/arm64/include/asm/kvm_host_hypercalls.h
new file mode 100644
index ..af8ad505d816
--- /dev/null
+++ b/arch/arm64/include/asm/kvm_host_hypercalls.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2020 Google, inc
+ */
+
+#ifndef __KVM_HOST_HCALL
+#define __KVM_HOST_HCALL(x)
+#endif
+
+#define __KVM_HOST_HCALL_TABLE_IDX___kvm_enable_ssbs   0
+__KVM_HOST_HCALL(__kvm_enable_ssbs)
+
+#define __KVM_HOST_HCALL_TABLE_IDX___kvm_get_mdcr_el2  1
+__KVM_HOST_HCALL(__kvm_get_mdcr_el2)
+
+#define __KVM_HOST_HCALL_TABLE_IDX___kvm_timer_set_cntvoff 2
+__KVM_HOST_HCALL(__kvm_timer_set_cntvoff)
+
+#define __KVM_HOST_HCALL_TABLE_IDX___kvm_tlb_flush_local_vmid  3
+__KVM_HOST_HCALL(__kvm_tlb_flush_local_vmid)
+
+#define __KVM_HOST_HCALL_TABLE_IDX___kvm_flush_vm_context  4
+__KVM_HOST_HCALL(__kvm_flush_vm_context)
+
+#define __KVM_HOST_HCALL_TABLE_IDX___kvm_vcpu_run_nvhe 5
+__KVM_HOST_HCALL(__kvm_vcpu_run_nvhe)
+
+#define __KVM_HOST_HCALL_TABLE_IDX___kvm_tlb_flush_vmid6
+__KVM_HOST_HCALL(__kvm_tlb_flush_vmid)
+
+#define __KVM_HOST_HCALL_TABLE_IDX___kvm_tlb_flush_vmid_ipa7
+__KVM_HOST_HCALL(__kvm_tlb_flush_vmid_ipa)
+
+#define __KVM_HOST_HCALL_TABLE_IDX___vgic_v3_init_lrs  8
+__KVM_HOST_HCALL(__vgic_v3_init_lrs)
+
+#define __KVM_HOST_HCALL_TABLE_IDX___vgic_v3_get_ich_vtr_el2   9
+__KVM_HOST_HCALL(__vgic_v3_get_ich_vtr_el2)
+
+#define __KVM_HOST_HCALL_TABLE_IDX___vgic_v3_write_vmcr10
+__KVM_HOST_HCALL(__vgic_v3_write_vmcr)
+
+#define __KVM_HOST_HCALL_TABLE_IDX___vgic_v3_restore_aprs  11
+__KVM_HOST_HCALL(__vgic_v3_restore_aprs)
+
+#define __KVM_HOST_HCALL_TABLE_IDX___vgic_v3_read_vmcr 12
+__KVM_HOST_HCALL(__vgic_v3_read_vmcr)
+
+#define __KVM_HOST_HCALL_TABLE_IDX___vgic_v3_save_aprs 13
+__KVM_HOST_HCALL(__vgic_v3_save_aprs)
+
+#define __KVM_HOST_HCALL_TABLE_IDX_SIZE14
+
+/* XXX - Arbitrary offset for KVM-related hypercalls */
+#define __KVM_HOST_HCALL_BASE_SHIFT 8
+#define __KVM_HOST_HCALL_BASE (1ULL << __KVM_HOST_HCALL_BASE_SHIFT)
+#define __KVM_HOST_HCALL_END (__KVM_HOST_HCALL_BASE + \
+ __KVM_HOST_HCALL_TABLE_IDX_SIZE)
+
+/* Hypercall number = kvm offset + table idx */
+#define KVM_HOST_HCALL_NR(name) (__KVM_HOST_HCALL_TABLE_IDX_##name + \
+__KVM_HOST_HCALL_BASE)
diff --git a/arch/arm64/kvm/hyp/Makefile 

[PATCH 01/15] arm64: kvm: Unify users of HVC instruction

2020-04-30 Thread David Brazdil
From: Quentin Perret 

Currently, the arm64 KVM code provides __kvm_call_hyp assembly procedure which
does nothing but call the HVC instruction. This is used to call functions by
their pointer in EL2 under nVHE, and abused by __cpu_init_hyp_mode to pass
a data pointer. The hyp-stub code, on the other hand, has its own assembly
procedures for (re)setting hyp vectors.

In preparation for a clean-up of the KVM hypercall interface, unify all HVC
users behind __kvm_call_hyp and remove comments about expected meaning of
arguments.

No functional changes intended.

Signed-off-by: Quentin Perret 
Signed-off-by: David Brazdil 
---
 arch/arm64/include/asm/kvm_host.h | 12 ++-
 arch/arm64/include/asm/virt.h | 33 --
 arch/arm64/kernel/hyp-stub.S  | 34 ---
 arch/arm64/kvm/hyp.S  | 13 +---
 4 files changed, 39 insertions(+), 53 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h 
b/arch/arm64/include/asm/kvm_host.h
index 32c8a675e5a4..e61143d6602d 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define __KVM_HAVE_ARCH_INTC_INITIALIZED
 
@@ -446,7 +447,8 @@ int kvm_test_age_hva(struct kvm *kvm, unsigned long hva);
 void kvm_arm_halt_guest(struct kvm *kvm);
 void kvm_arm_resume_guest(struct kvm *kvm);
 
-u64 __kvm_call_hyp(void *hypfn, ...);
+#define kvm_call_hyp_nvhe(hypfn, ...) \
+   __kvm_call_hyp((unsigned long)kvm_ksym_ref(hypfn), ##__VA_ARGS__)
 
 /*
  * The couple of isb() below are there to guarantee the same behaviour
@@ -459,7 +461,7 @@ u64 __kvm_call_hyp(void *hypfn, ...);
f(__VA_ARGS__); \
isb();  \
} else {\
-   __kvm_call_hyp(kvm_ksym_ref(f), ##__VA_ARGS__); \
+   kvm_call_hyp_nvhe(f, ##__VA_ARGS__);\
}   \
} while(0)
 
@@ -471,8 +473,7 @@ u64 __kvm_call_hyp(void *hypfn, ...);
ret = f(__VA_ARGS__);   \
isb();  \
} else {\
-   ret = __kvm_call_hyp(kvm_ksym_ref(f),   \
-##__VA_ARGS__);\
+   ret = kvm_call_hyp_nvhe(f, ##__VA_ARGS__);  \
}   \
\
ret;\
@@ -551,7 +552,8 @@ static inline void __cpu_init_hyp_mode(phys_addr_t pgd_ptr,
 * cpus_have_const_cap() wrapper.
 */
BUG_ON(!system_capabilities_finalized());
-   __kvm_call_hyp((void *)pgd_ptr, hyp_stack_ptr, vector_ptr, tpidr_el2);
+   __kvm_call_hyp((unsigned long)pgd_ptr, hyp_stack_ptr, vector_ptr,
+  tpidr_el2);
 
/*
 * Disabling SSBD on a non-VHE system requires us to enable SSBS
diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h
index 61fd26752adc..fdc11f819b06 100644
--- a/arch/arm64/include/asm/virt.h
+++ b/arch/arm64/include/asm/virt.h
@@ -62,8 +62,37 @@
  */
 extern u32 __boot_cpu_mode[2];
 
-void __hyp_set_vectors(phys_addr_t phys_vector_base);
-void __hyp_reset_vectors(void);
+/* Make HVC call into the hypervisor. */
+extern u64 __kvm_call_hyp(unsigned long arg, ...);
+
+/*
+ * __hyp_set_vectors: Call this after boot to set the initial hypervisor
+ * vectors as part of hypervisor installation.  On an SMP system, this should
+ * be called on each CPU.
+ *
+ * @phys_vector_base must be the physical address of the new vector table, and
+ * must be 2KB aligned.
+ *
+ * Before calling this, you must check that the stub hypervisor is installed
+ * everywhere, by waiting for any secondary CPUs to be brought up and then
+ * checking that is_hyp_mode_available() is true.
+ *
+ * If not, there is a pre-existing hypervisor, some CPUs failed to boot, or
+ * something else went wrong... in such cases, trying to install a new
+ * hypervisor is unlikely to work as desired.
+ *
+ * When you call into your shiny new hypervisor, sp_el2 will contain junk,
+ * so you will need to set that to something sensible at the new hypervisor's
+ * initialisation entry point.
+ */
+static inline void __hyp_set_vectors(phys_addr_t phys_vector_base)
+{
+   __kvm_call_hyp(HVC_SET_VECTORS, phys_vector_base);
+}
+static inline void __hyp_reset_vectors(void)
+{
+   __kvm_call_hyp(HVC_RESET_VECTORS);
+}
 
 /* Reports the availability of HYP mode */
 static inline bool is_hyp_mode_available(void)

[PATCH 05/15] arm64: kvm: Build hyp-entry.S separately for VHE/nVHE

2020-04-30 Thread David Brazdil
This patch is part of a series which builds KVM's non-VHE hyp code separately
from VHE and the rest of the kernel.

hyp-entry.S contains implementation of KVM hyp vectors. This code is mostly
shared between VHE/nVHE, therefore compile it under both VHE and nVHE build
rules, with small differences hidden behind '#ifdef __HYPERVISOR__'. These are:
  * only nVHE should handle host HVCs, VHE will now panic,
  * only nVHE needs kvm_hcall_table, so move host_hypcall.c to nvhe/,
  * __smccc_workaround_1_smc is not needed by nVHE, only cpu_errata.c in
kernel proper.

Adjust code which selects which KVM hyp vecs to install to choose the correct
VHE/nVHE symbol.

Signed-off-by: David Brazdil 
---
 arch/arm64/include/asm/kvm_asm.h  |  7 +
 arch/arm64/include/asm/kvm_mmu.h  | 13 +
 arch/arm64/include/asm/mmu.h  |  7 -
 arch/arm64/kernel/cpu_errata.c|  2 +-
 arch/arm64/kernel/image-vars.h| 28 +++
 arch/arm64/kvm/hyp/Makefile   |  2 +-
 arch/arm64/kvm/hyp/hyp-entry.S| 27 --
 arch/arm64/kvm/hyp/nvhe/Makefile  |  2 +-
 .../arm64/kvm/hyp/{ => nvhe}/host_hypercall.c |  0
 arch/arm64/kvm/va_layout.c|  2 +-
 10 files changed, 65 insertions(+), 25 deletions(-)
 rename arch/arm64/kvm/hyp/{ => nvhe}/host_hypercall.c (100%)

diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
index 99ab204519ca..cdaf3df8085d 100644
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -71,6 +71,13 @@ extern char __kvm_hyp_init[];
 extern char __kvm_hyp_init_end[];
 
 extern char __kvm_hyp_vector[];
+extern char kvm_nvhe_sym(__kvm_hyp_vector)[];
+
+#ifdef CONFIG_KVM_INDIRECT_VECTORS
+extern char __bp_harden_hyp_vecs[];
+extern char kvm_nvhe_sym(__bp_harden_hyp_vecs)[];
+extern atomic_t arm64_el2_vector_last_slot;
+#endif
 
 extern void __kvm_flush_vm_context(void);
 extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa);
diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
index 30b0e8d6b895..0a5fa033422c 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -468,7 +468,7 @@ static inline int kvm_write_guest_lock(struct kvm *kvm, 
gpa_t gpa,
  * VHE, as we don't have hypervisor-specific mappings. If the system
  * is VHE and yet selects this capability, it will be ignored.
  */
-#include 
+#include 
 
 extern void *__kvm_bp_vect_base;
 extern int __kvm_harden_el2_vector_slot;
@@ -477,11 +477,11 @@ extern int __kvm_harden_el2_vector_slot;
 static inline void *kvm_get_hyp_vector(void)
 {
struct bp_hardening_data *data = arm64_get_bp_hardening_data();
-   void *vect = kern_hyp_va(kvm_ksym_ref(__kvm_hyp_vector));
+   void *vect = kern_hyp_va(kvm_hyp_ref(__kvm_hyp_vector));
int slot = -1;
 
if (cpus_have_const_cap(ARM64_HARDEN_BRANCH_PREDICTOR) && data->fn) {
-   vect = kern_hyp_va(kvm_ksym_ref(__bp_harden_hyp_vecs));
+   vect = kern_hyp_va(kvm_hyp_ref(__bp_harden_hyp_vecs));
slot = data->hyp_vectors_slot;
}
 
@@ -510,12 +510,13 @@ static inline int kvm_map_vectors(void)
 *  HBP +  HEL2 -> use hardened vertors and use exec mapping
 */
if (cpus_have_const_cap(ARM64_HARDEN_BRANCH_PREDICTOR)) {
-   __kvm_bp_vect_base = kvm_ksym_ref(__bp_harden_hyp_vecs);
+   __kvm_bp_vect_base = kvm_hyp_ref(__bp_harden_hyp_vecs);
__kvm_bp_vect_base = kern_hyp_va(__kvm_bp_vect_base);
}
 
if (cpus_have_const_cap(ARM64_HARDEN_EL2_VECTORS)) {
-   phys_addr_t vect_pa = __pa_symbol(__bp_harden_hyp_vecs);
+   phys_addr_t vect_pa =
+   __pa_symbol(kvm_nvhe_sym(__bp_harden_hyp_vecs));
unsigned long size = __BP_HARDEN_HYP_VECS_SZ;
 
/*
@@ -534,7 +535,7 @@ static inline int kvm_map_vectors(void)
 #else
 static inline void *kvm_get_hyp_vector(void)
 {
-   return kern_hyp_va(kvm_ksym_ref(__kvm_hyp_vector));
+   return kern_hyp_va(kvm_hyp_ref(__kvm_hyp_vector));
 }
 
 static inline int kvm_map_vectors(void)
diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
index 68140fdd89d6..4d913f6dd366 100644
--- a/arch/arm64/include/asm/mmu.h
+++ b/arch/arm64/include/asm/mmu.h
@@ -42,13 +42,6 @@ struct bp_hardening_data {
bp_hardening_cb_t   fn;
 };
 
-#if (defined(CONFIG_HARDEN_BRANCH_PREDICTOR) ||\
- defined(CONFIG_HARDEN_EL2_VECTORS))
-
-extern char __bp_harden_hyp_vecs[];
-extern atomic_t arm64_el2_vector_last_slot;
-#endif  /* CONFIG_HARDEN_BRANCH_PREDICTOR || CONFIG_HARDEN_EL2_VECTORS */
-
 #ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
 DECLARE_PER_CPU_READ_MOSTLY(struct bp_hardening_data, bp_hardening_data);
 
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 

[PATCH 03/15] arm64: kvm: Fix symbol dependency in __hyp_call_panic_nvhe

2020-04-30 Thread David Brazdil
__hyp_call_panic_nvhe contains inline assembly which did not declare
its dependency on the __hyp_panic_string symbol.

Signed-off-by: David Brazdil 
---
 arch/arm64/kvm/hyp/switch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 8a1e81a400e0..7a7c08029d81 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -836,7 +836,7 @@ static void __hyp_text __hyp_call_panic_nvhe(u64 spsr, u64 
elr, u64 par,
 * making sure it is a kernel address and not a PC-relative
 * reference.
 */
-   asm volatile("ldr %0, =__hyp_panic_string" : "=r" (str_va));
+   asm volatile("ldr %0, =%1" : "=r" (str_va) : "S" (__hyp_panic_string));
 
__hyp_do_panic(str_va,
   spsr, elr,
-- 
2.26.1

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


Re: [PATCH] KVM: arm64: Fix 32bit PC wrap-around

2020-04-30 Thread Will Deacon
On Thu, Apr 30, 2020 at 01:45:51PM +0100, Marc Zyngier wrote:
> On 2020-04-30 13:31, Will Deacon wrote:
> > On Thu, Apr 30, 2020 at 11:59:05AM +0100, Marc Zyngier wrote:
> > > On 2020-04-30 11:25, Will Deacon wrote:
> > > > On Thu, Apr 30, 2020 at 11:15:13AM +0100, Marc Zyngier wrote:
> > > > > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> > > > > index 23ebe51410f0..2a159af82429 100644
> > > > > --- a/arch/arm64/kvm/guest.c
> > > > > +++ b/arch/arm64/kvm/guest.c
> > > > > @@ -200,6 +200,10 @@ static int set_core_reg(struct kvm_vcpu *vcpu,
> > > > > const struct kvm_one_reg *reg)
> > > > >   }
> > > > >
> > > > >   memcpy((u32 *)regs + off, valp, KVM_REG_SIZE(reg->id));
> > > > > +
> > > > > + if (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK)
> > > > > + *vcpu_pc(vcpu) = lower_32_bits(*vcpu_pc(vcpu));
> > > >
> > > > It seems slightly odd to me that we don't enforce this for *all* the
> > > > registers when running as a 32-bit guest. Couldn't userspace be equally
> > > > confused by a 64-bit lr or sp?
> > > 
> > > Fair point. How about this on top, which wipes the upper 32 bits for
> > > each and every register in the current mode:
> > > 
> > > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> > > index 2a159af82429..f958c3c7bf65 100644
> > > --- a/arch/arm64/kvm/guest.c
> > > +++ b/arch/arm64/kvm/guest.c
> > > @@ -201,9 +201,12 @@ static int set_core_reg(struct kvm_vcpu *vcpu,
> > > const
> > > struct kvm_one_reg *reg)
> > > 
> > >   memcpy((u32 *)regs + off, valp, KVM_REG_SIZE(reg->id));
> > > 
> > > - if (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK)
> > > - *vcpu_pc(vcpu) = lower_32_bits(*vcpu_pc(vcpu));
> > > + if (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK) {
> > > + int i;
> > > 
> > > + for (i = 0; i < 16; i++)
> > > + *vcpu_reg32(vcpu, i) = (u32)*vcpu_reg32(vcpu, i);
> > 
> > I think you're missing all the funny banked registers that live all the
> > way
> > up to x30 iirc.
> 
> No, they are all indirected via vcpu_reg32(), which has the magic tables.
> And the whole point is that we only want to affect the current mode (no
> point
> in repainting the FIQ registers if the PSR says USR).
> 
> Or am I missing something obvious?

Nope, just my inability to parse vcpu_reg32 the first time around! So, for
the updated patch:

Acked-by: Will Deacon https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH] KVM: arm64: Fix 32bit PC wrap-around

2020-04-30 Thread Marc Zyngier

On 2020-04-30 13:31, Will Deacon wrote:

On Thu, Apr 30, 2020 at 11:59:05AM +0100, Marc Zyngier wrote:

On 2020-04-30 11:25, Will Deacon wrote:
> On Thu, Apr 30, 2020 at 11:15:13AM +0100, Marc Zyngier wrote:
> > In the unlikely event that a 32bit vcpu traps into the hypervisor
> > on an instruction that is located right at the end of the 32bit
> > range, the emulation of that instruction is going to increment
> > PC past the 32bit range. This isn't great, as userspace can then
> > observe this value and get a bit confused.
> >
> > Conversly, userspace can do things like (in the context of a 64bit
> > guest that is capable of 32bit EL0) setting PSTATE to AArch64-EL0,
> > set PC to a 64bit value, change PSTATE to AArch32-USR, and observe
> > that PC hasn't been truncated. More confusion.
> >
> > Fix both by:
> > - truncating PC increments for 32bit guests
> > - sanitize PC every time a core reg is changed by userspace, and
> >   that PSTATE indicates a 32bit mode.
>
> It's not clear to me whether this needs a cc stable. What do you think?
> I
> suppose that it really depends on how confused e.g. QEMU gets.

It isn't so much QEMU itself that I'm worried about (the emulation 
shouldn't
really care about the PC), but the likes of GDB. So yes, a cc stable 
seems

to
be in order.


Okey doke.


> > Signed-off-by: Marc Zyngier 
> > ---
> >  arch/arm64/kvm/guest.c | 4 
> >  virt/kvm/arm/hyp/aarch32.c | 8 ++--
> >  2 files changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> > index 23ebe51410f0..2a159af82429 100644
> > --- a/arch/arm64/kvm/guest.c
> > +++ b/arch/arm64/kvm/guest.c
> > @@ -200,6 +200,10 @@ static int set_core_reg(struct kvm_vcpu *vcpu,
> > const struct kvm_one_reg *reg)
> >   }
> >
> >   memcpy((u32 *)regs + off, valp, KVM_REG_SIZE(reg->id));
> > +
> > + if (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK)
> > + *vcpu_pc(vcpu) = lower_32_bits(*vcpu_pc(vcpu));
>
> It seems slightly odd to me that we don't enforce this for *all* the
> registers when running as a 32-bit guest. Couldn't userspace be equally
> confused by a 64-bit lr or sp?

Fair point. How about this on top, which wipes the upper 32 bits for
each and every register in the current mode:

diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 2a159af82429..f958c3c7bf65 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -201,9 +201,12 @@ static int set_core_reg(struct kvm_vcpu *vcpu, 
const

struct kvm_one_reg *reg)

memcpy((u32 *)regs + off, valp, KVM_REG_SIZE(reg->id));

-   if (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK)
-   *vcpu_pc(vcpu) = lower_32_bits(*vcpu_pc(vcpu));
+   if (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK) {
+   int i;

+   for (i = 0; i < 16; i++)
+   *vcpu_reg32(vcpu, i) = (u32)*vcpu_reg32(vcpu, i);


I think you're missing all the funny banked registers that live all the 
way

up to x30 iirc.


No, they are all indirected via vcpu_reg32(), which has the magic 
tables.
And the whole point is that we only want to affect the current mode (no 
point

in repainting the FIQ registers if the PSR says USR).

Or am I missing something obvious?

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] KVM: arm64: Fix 32bit PC wrap-around

2020-04-30 Thread Will Deacon
On Thu, Apr 30, 2020 at 11:59:05AM +0100, Marc Zyngier wrote:
> On 2020-04-30 11:25, Will Deacon wrote:
> > On Thu, Apr 30, 2020 at 11:15:13AM +0100, Marc Zyngier wrote:
> > > In the unlikely event that a 32bit vcpu traps into the hypervisor
> > > on an instruction that is located right at the end of the 32bit
> > > range, the emulation of that instruction is going to increment
> > > PC past the 32bit range. This isn't great, as userspace can then
> > > observe this value and get a bit confused.
> > > 
> > > Conversly, userspace can do things like (in the context of a 64bit
> > > guest that is capable of 32bit EL0) setting PSTATE to AArch64-EL0,
> > > set PC to a 64bit value, change PSTATE to AArch32-USR, and observe
> > > that PC hasn't been truncated. More confusion.
> > > 
> > > Fix both by:
> > > - truncating PC increments for 32bit guests
> > > - sanitize PC every time a core reg is changed by userspace, and
> > >   that PSTATE indicates a 32bit mode.
> > 
> > It's not clear to me whether this needs a cc stable. What do you think?
> > I
> > suppose that it really depends on how confused e.g. QEMU gets.
> 
> It isn't so much QEMU itself that I'm worried about (the emulation shouldn't
> really care about the PC), but the likes of GDB. So yes, a cc stable seems
> to
> be in order.

Okey doke.

> > > Signed-off-by: Marc Zyngier 
> > > ---
> > >  arch/arm64/kvm/guest.c | 4 
> > >  virt/kvm/arm/hyp/aarch32.c | 8 ++--
> > >  2 files changed, 10 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> > > index 23ebe51410f0..2a159af82429 100644
> > > --- a/arch/arm64/kvm/guest.c
> > > +++ b/arch/arm64/kvm/guest.c
> > > @@ -200,6 +200,10 @@ static int set_core_reg(struct kvm_vcpu *vcpu,
> > > const struct kvm_one_reg *reg)
> > >   }
> > > 
> > >   memcpy((u32 *)regs + off, valp, KVM_REG_SIZE(reg->id));
> > > +
> > > + if (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK)
> > > + *vcpu_pc(vcpu) = lower_32_bits(*vcpu_pc(vcpu));
> > 
> > It seems slightly odd to me that we don't enforce this for *all* the
> > registers when running as a 32-bit guest. Couldn't userspace be equally
> > confused by a 64-bit lr or sp?
> 
> Fair point. How about this on top, which wipes the upper 32 bits for
> each and every register in the current mode:
> 
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index 2a159af82429..f958c3c7bf65 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -201,9 +201,12 @@ static int set_core_reg(struct kvm_vcpu *vcpu, const
> struct kvm_one_reg *reg)
> 
>   memcpy((u32 *)regs + off, valp, KVM_REG_SIZE(reg->id));
> 
> - if (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK)
> - *vcpu_pc(vcpu) = lower_32_bits(*vcpu_pc(vcpu));
> + if (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK) {
> + int i;
> 
> + for (i = 0; i < 16; i++)
> + *vcpu_reg32(vcpu, i) = (u32)*vcpu_reg32(vcpu, i);

I think you're missing all the funny banked registers that live all the way
up to x30 iirc.

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


Re: [PATCH] KVM: arm64: vgic-v4: Initialize GICv4.1 even in the absence of a virtual ITS

2020-04-30 Thread Marc Zyngier

Hi Zenghui,

On 2020-04-30 12:24, Zenghui Yu wrote:

Hi Marc,

On 2020/4/25 17:44, Marc Zyngier wrote:

KVM now expects to be able to use HW-accelerated delivery of vSGIs
as soon as the guest has enabled thm. Unfortunately, we only

them

initialize the GICv4 context if we have a virtual ITS exposed to
the guest.

Fix it by always initializing the GICv4.1 context if it is
available on the host.

Fixes: 2291ff2f2a56 ("KVM: arm64: GICv4.1: Plumb SGI implementation 
selection in the distributor")

Signed-off-by: Marc Zyngier 
---
  virt/kvm/arm/vgic/vgic-init.c| 9 -
  virt/kvm/arm/vgic/vgic-mmio-v3.c | 3 ++-
  2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/arm/vgic/vgic-init.c 
b/virt/kvm/arm/vgic/vgic-init.c

index a963b9d766b73..8e6f350c3bcd1 100644
--- a/virt/kvm/arm/vgic/vgic-init.c
+++ b/virt/kvm/arm/vgic/vgic-init.c
@@ -294,8 +294,15 @@ int vgic_init(struct kvm *kvm)
}
}
  - if (vgic_has_its(kvm)) {
+   if (vgic_has_its(kvm))
vgic_lpi_translation_cache_init(kvm);
+
+   /*
+* If we have GICv4.1 enabled, unconditionnaly request enable the
+* v4 support so that we get HW-accelerated vSGIs. Otherwise, only
+* enable it if we present a virtual ITS to the guest.
+*/
+   if (vgic_supports_direct_msis(kvm)) {
ret = vgic_v4_init(kvm);
if (ret)
goto out;
diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c 
b/virt/kvm/arm/vgic/vgic-mmio-v3.c

index e72dcc4542475..26b11dcd45524 100644
--- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
+++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
@@ -50,7 +50,8 @@ bool vgic_has_its(struct kvm *kvm)
bool vgic_supports_direct_msis(struct kvm *kvm)
  {
-   return kvm_vgic_global_state.has_gicv4 && vgic_has_its(kvm);
+   return (kvm_vgic_global_state.has_gicv4_1 ||
+   (kvm_vgic_global_state.has_gicv4 && vgic_has_its(kvm)));
  }


Not related to this patch, but I think that the function name can be
improved a bit after this change. It now indicates whether the vGIC
supports direct MSIs injection *or* direct SGIs injection, not just
MSIs. And if vgic_has_its() is false, we don't even support MSIs.


Yes, I noticed that too. But in the spirit of keeping the change minimal
and avoid later conflicts with potential fixes, I decided against 
changing

it right now.


The fix itself looks correct to me,

Reviewed-by: Zenghui Yu 


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] KVM: arm64: vgic-v4: Initialize GICv4.1 even in the absence of a virtual ITS

2020-04-30 Thread Zenghui Yu

Hi Marc,

On 2020/4/25 17:44, Marc Zyngier wrote:

KVM now expects to be able to use HW-accelerated delivery of vSGIs
as soon as the guest has enabled thm. Unfortunately, we only

them

initialize the GICv4 context if we have a virtual ITS exposed to
the guest.

Fix it by always initializing the GICv4.1 context if it is
available on the host.

Fixes: 2291ff2f2a56 ("KVM: arm64: GICv4.1: Plumb SGI implementation selection in the 
distributor")
Signed-off-by: Marc Zyngier 
---
  virt/kvm/arm/vgic/vgic-init.c| 9 -
  virt/kvm/arm/vgic/vgic-mmio-v3.c | 3 ++-
  2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
index a963b9d766b73..8e6f350c3bcd1 100644
--- a/virt/kvm/arm/vgic/vgic-init.c
+++ b/virt/kvm/arm/vgic/vgic-init.c
@@ -294,8 +294,15 @@ int vgic_init(struct kvm *kvm)
}
}
  
-	if (vgic_has_its(kvm)) {

+   if (vgic_has_its(kvm))
vgic_lpi_translation_cache_init(kvm);
+
+   /*
+* If we have GICv4.1 enabled, unconditionnaly request enable the
+* v4 support so that we get HW-accelerated vSGIs. Otherwise, only
+* enable it if we present a virtual ITS to the guest.
+*/
+   if (vgic_supports_direct_msis(kvm)) {
ret = vgic_v4_init(kvm);
if (ret)
goto out;
diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
index e72dcc4542475..26b11dcd45524 100644
--- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
+++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
@@ -50,7 +50,8 @@ bool vgic_has_its(struct kvm *kvm)
  
  bool vgic_supports_direct_msis(struct kvm *kvm)

  {
-   return kvm_vgic_global_state.has_gicv4 && vgic_has_its(kvm);
+   return (kvm_vgic_global_state.has_gicv4_1 ||
+   (kvm_vgic_global_state.has_gicv4 && vgic_has_its(kvm)));
  }


Not related to this patch, but I think that the function name can be
improved a bit after this change. It now indicates whether the vGIC
supports direct MSIs injection *or* direct SGIs injection, not just
MSIs. And if vgic_has_its() is false, we don't even support MSIs.

The fix itself looks correct to me,

Reviewed-by: Zenghui Yu 


Thanks.

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


Re: [PATCH] KVM: arm64: Fix 32bit PC wrap-around

2020-04-30 Thread Marc Zyngier

On 2020-04-30 11:25, Will Deacon wrote:

On Thu, Apr 30, 2020 at 11:15:13AM +0100, Marc Zyngier wrote:

In the unlikely event that a 32bit vcpu traps into the hypervisor
on an instruction that is located right at the end of the 32bit
range, the emulation of that instruction is going to increment
PC past the 32bit range. This isn't great, as userspace can then
observe this value and get a bit confused.

Conversly, userspace can do things like (in the context of a 64bit
guest that is capable of 32bit EL0) setting PSTATE to AArch64-EL0,
set PC to a 64bit value, change PSTATE to AArch32-USR, and observe
that PC hasn't been truncated. More confusion.

Fix both by:
- truncating PC increments for 32bit guests
- sanitize PC every time a core reg is changed by userspace, and
  that PSTATE indicates a 32bit mode.


It's not clear to me whether this needs a cc stable. What do you think? 
I

suppose that it really depends on how confused e.g. QEMU gets.


It isn't so much QEMU itself that I'm worried about (the emulation 
shouldn't
really care about the PC), but the likes of GDB. So yes, a cc stable 
seems to

be in order.




Signed-off-by: Marc Zyngier 
---
 arch/arm64/kvm/guest.c | 4 
 virt/kvm/arm/hyp/aarch32.c | 8 ++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 23ebe51410f0..2a159af82429 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -200,6 +200,10 @@ static int set_core_reg(struct kvm_vcpu *vcpu, 
const struct kvm_one_reg *reg)

}

memcpy((u32 *)regs + off, valp, KVM_REG_SIZE(reg->id));
+
+   if (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK)
+   *vcpu_pc(vcpu) = lower_32_bits(*vcpu_pc(vcpu));


It seems slightly odd to me that we don't enforce this for *all* the
registers when running as a 32-bit guest. Couldn't userspace be equally
confused by a 64-bit lr or sp?


Fair point. How about this on top, which wipes the upper 32 bits for
each and every register in the current mode:

diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 2a159af82429..f958c3c7bf65 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -201,9 +201,12 @@ static int set_core_reg(struct kvm_vcpu *vcpu, 
const struct kvm_one_reg *reg)


memcpy((u32 *)regs + off, valp, KVM_REG_SIZE(reg->id));

-   if (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK)
-   *vcpu_pc(vcpu) = lower_32_bits(*vcpu_pc(vcpu));
+   if (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK) {
+   int i;

+   for (i = 0; i < 16; i++)
+   *vcpu_reg32(vcpu, i) = (u32)*vcpu_reg32(vcpu, i);
+   }
 out:
return err;
 }

I'm tempted to make the whole SET_REG hunk a separate patch though.

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] KVM: arm64: Save/restore sp_el0 as part of __guest_enter

2020-04-30 Thread Mark Rutland
On Sat, Apr 25, 2020 at 10:43:21AM +0100, Marc Zyngier wrote:
> We currently save/restore sp_el0 in C code. This is a bit unsafe,
> as a lot of the C code expects 'current' to be accessible from
> there (and the opportunity to run kernel code in HYP is specially
> great with VHE).
> 
> Instead, let's move the save/restore of sp_el0 to the assembly
> code (in __guest_enter), making sure that sp_el0 is correct
> very early on when we exit the guest, and is preserved as long
> as possible to its host value when we enter the guest.
> 
> Signed-off-by: Marc Zyngier 

Makes sense to me in principle, but I haven't reviewed the code in
detail:

Acked-by: Mark Rutland 

Mark.

> ---
>  arch/arm64/kvm/hyp/entry.S | 23 +++
>  arch/arm64/kvm/hyp/sysreg-sr.c | 17 +++--
>  2 files changed, 26 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
> index d22d0534dd600..90186cf6473e0 100644
> --- a/arch/arm64/kvm/hyp/entry.S
> +++ b/arch/arm64/kvm/hyp/entry.S
> @@ -18,6 +18,7 @@
>  
>  #define CPU_GP_REG_OFFSET(x) (CPU_GP_REGS + x)
>  #define CPU_XREG_OFFSET(x)   CPU_GP_REG_OFFSET(CPU_USER_PT_REGS + 8*x)
> +#define CPU_SP_EL0_OFFSET(CPU_XREG_OFFSET(30) + 8)
>  
>   .text
>   .pushsection.hyp.text, "ax"
> @@ -47,6 +48,16 @@
>   ldp x29, lr,  [\ctxt, #CPU_XREG_OFFSET(29)]
>  .endm
>  
> +.macro save_sp_el0 ctxt, tmp
> + mrs \tmp,   sp_el0
> + str \tmp,   [\ctxt, #CPU_SP_EL0_OFFSET]
> +.endm
> +
> +.macro restore_sp_el0 ctxt, tmp
> + ldr \tmp, [\ctxt, #CPU_SP_EL0_OFFSET]
> + msr sp_el0, \tmp
> +.endm
> +
>  /*
>   * u64 __guest_enter(struct kvm_vcpu *vcpu,
>   *struct kvm_cpu_context *host_ctxt);
> @@ -60,6 +71,9 @@ SYM_FUNC_START(__guest_enter)
>   // Store the host regs
>   save_callee_saved_regs x1
>  
> + // Save the host's sp_el0
> + save_sp_el0 x1, x2
> +
>   // Now the host state is stored if we have a pending RAS SError it must
>   // affect the host. If any asynchronous exception is pending we defer
>   // the guest entry. The DSB isn't necessary before v8.2 as any SError
> @@ -83,6 +97,9 @@ alternative_else_nop_endif
>   // when this feature is enabled for kernel code.
>   ptrauth_switch_to_guest x29, x0, x1, x2
>  
> + // Restore the guest's sp_el0
> + restore_sp_el0 x29, x0
> +
>   // Restore guest regs x0-x17
>   ldp x0, x1,   [x29, #CPU_XREG_OFFSET(0)]
>   ldp x2, x3,   [x29, #CPU_XREG_OFFSET(2)]
> @@ -130,6 +147,9 @@ SYM_INNER_LABEL(__guest_exit, SYM_L_GLOBAL)
>   // Store the guest regs x18-x29, lr
>   save_callee_saved_regs x1
>  
> + // Store the guest's sp_el0
> + save_sp_el0 x1, x2
> +
>   get_host_ctxt   x2, x3
>  
>   // Macro ptrauth_switch_to_guest format:
> @@ -139,6 +159,9 @@ SYM_INNER_LABEL(__guest_exit, SYM_L_GLOBAL)
>   // when this feature is enabled for kernel code.
>   ptrauth_switch_to_host x1, x2, x3, x4, x5
>  
> + // Restore the hosts's sp_el0
> + restore_sp_el0 x2, x3
> +
>   // Now restore the host regs
>   restore_callee_saved_regs x2
>  
> diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
> index 75b1925763f16..6d2df9fe0b5d2 100644
> --- a/arch/arm64/kvm/hyp/sysreg-sr.c
> +++ b/arch/arm64/kvm/hyp/sysreg-sr.c
> @@ -15,8 +15,9 @@
>  /*
>   * Non-VHE: Both host and guest must save everything.
>   *
> - * VHE: Host and guest must save mdscr_el1 and sp_el0 (and the PC and pstate,
> - * which are handled as part of the el2 return state) on every switch.
> + * VHE: Host and guest must save mdscr_el1 and sp_el0 (and the PC and
> + * pstate, which are handled as part of the el2 return state) on every
> + * switch (sp_el0 is being dealt with in the assembly code).
>   * tpidr_el0 and tpidrro_el0 only need to be switched when going
>   * to host userspace or a different VCPU.  EL1 registers only need to be
>   * switched when potentially going to run a different VCPU.  The latter two
> @@ -26,12 +27,6 @@
>  static void __hyp_text __sysreg_save_common_state(struct kvm_cpu_context 
> *ctxt)
>  {
>   ctxt->sys_regs[MDSCR_EL1]   = read_sysreg(mdscr_el1);
> -
> - /*
> -  * The host arm64 Linux uses sp_el0 to point to 'current' and it must
> -  * therefore be saved/restored on every entry/exit to/from the guest.
> -  */
> - ctxt->gp_regs.regs.sp   = read_sysreg(sp_el0);
>  }
>  
>  static void __hyp_text __sysreg_save_user_state(struct kvm_cpu_context *ctxt)
> @@ -99,12 +94,6 @@ NOKPROBE_SYMBOL(sysreg_save_guest_state_vhe);
>  static void __hyp_text __sysreg_restore_common_state(struct kvm_cpu_context 
> *ctxt)
>  {
>   write_sysreg(ctxt->sys_regs[MDSCR_EL1],   mdscr_el1);
> -
> - /*
> -  * The host arm64 Linux uses sp_el0 to point to 'current' and it must
> -  * therefore be saved/restored on every entry/exit to/from the guest.
> -  */
> -   

Re: [RFC PATCH v11 5/9] psci: Add hypercall service for ptp_kvm.

2020-04-30 Thread Mark Rutland
On Tue, Apr 28, 2020 at 07:14:52AM +0100, Jianyong Wu wrote:
> On 2020/4/24 6:39 PM, Mark Rutland wrote:
> > On Fri, Apr 24, 2020 at 03:50:22AM +0100, Jianyong Wu wrote:
> >> On 2020/4/21 5:57 PM, Mark Rutland wrote:
> >>> On Tue, Apr 21, 2020 at 11:23:00AM +0800, Jianyong Wu wrote:
>  diff --git a/virt/kvm/arm/hypercalls.c b/virt/kvm/arm/hypercalls.c
>  index 550dfa3e53cd..a5309c28d4dc 100644
>  --- a/virt/kvm/arm/hypercalls.c
>  +++ b/virt/kvm/arm/hypercalls.c
>  @@ -62,6 +66,44 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
> if (gpa != GPA_INVALID)
> val = gpa;
> break;
>  +/*
>  + * This serves virtual kvm_ptp.
>  + * Four values will be passed back.
>  + * reg0 stores high 32-bit host ktime;
>  + * reg1 stores low 32-bit host ktime;
>  + * reg2 stores high 32-bit difference of host cycles and cntvoff;
>  + * reg3 stores low 32-bit difference of host cycles and cntvoff.
>  + */
>  +case ARM_SMCCC_HYP_KVM_PTP_FUNC_ID:
> >>> Shouldn't the host opt-in to providing this to the guest, as with other
> >>> features?
> >> er, do you mean that "ARM_SMCCC_HV_PV_TIME_XXX" as "opt-in"? if so, I
> >> think this
> >>
> >> kvm_ptp doesn't need a buddy. the driver in guest will call this service
> >> in a definite way.
> > I mean that when creating the VM, userspace should be able to choose
> > whether the PTP service is provided to the guest. The host shouldn't
> > always provide it as there may be cases where doing so is undesireable.
> >
> I think I have implemented in patch 9/9 that userspace can get the info
> that if the host offers the kvm_ptp service. But for now, the host
> kernel will always offer the kvm_ptp capability in the current
> implementation. I think x86 follow the same behavior (see [1]). so I
> have not considered when and how to disable this kvm_ptp service in
> host. Do you think we should offer this opt-in?

I think taht should be opt-in, yes.

[...]

> > It's also not clear to me what notion of host time is being exposed to
> > the guest (and consequently how this would interact with time changes on
> > the host, time namespaces, etc). Having some description of that would
> > be very helpful.
> 
> sorry to have not made it clear.
> 
> Time will not change in host and only time in guest will change to sync
> with host. host time is the target that time in guest want to adjust to.
> guest need to get the host time then compute the different of the time
> in guest and host, so the guest can adjust the time base on the difference.

I understood that host time wouldn't change here, but what was not clear
is which notion of host time is being exposed to the guest.

e.g. is that a raw monotonic clock, or one subject to periodic adjument,
or wall time in the host? What is the epoch of the host time?

> I will add the base principle of time sync service in guest using
> kvm_ptp in commit message.

That would be great; thanks!

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


Re: [PATCH] KVM: arm64: Fix 32bit PC wrap-around

2020-04-30 Thread Will Deacon
On Thu, Apr 30, 2020 at 11:15:13AM +0100, Marc Zyngier wrote:
> In the unlikely event that a 32bit vcpu traps into the hypervisor
> on an instruction that is located right at the end of the 32bit
> range, the emulation of that instruction is going to increment
> PC past the 32bit range. This isn't great, as userspace can then
> observe this value and get a bit confused.
> 
> Conversly, userspace can do things like (in the context of a 64bit
> guest that is capable of 32bit EL0) setting PSTATE to AArch64-EL0,
> set PC to a 64bit value, change PSTATE to AArch32-USR, and observe
> that PC hasn't been truncated. More confusion.
> 
> Fix both by:
> - truncating PC increments for 32bit guests
> - sanitize PC every time a core reg is changed by userspace, and
>   that PSTATE indicates a 32bit mode.

It's not clear to me whether this needs a cc stable. What do you think? I
suppose that it really depends on how confused e.g. QEMU gets.

> Signed-off-by: Marc Zyngier 
> ---
>  arch/arm64/kvm/guest.c | 4 
>  virt/kvm/arm/hyp/aarch32.c | 8 ++--
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index 23ebe51410f0..2a159af82429 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -200,6 +200,10 @@ static int set_core_reg(struct kvm_vcpu *vcpu, const 
> struct kvm_one_reg *reg)
>   }
>  
>   memcpy((u32 *)regs + off, valp, KVM_REG_SIZE(reg->id));
> +
> + if (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK)
> + *vcpu_pc(vcpu) = lower_32_bits(*vcpu_pc(vcpu));

It seems slightly odd to me that we don't enforce this for *all* the
registers when running as a 32-bit guest. Couldn't userspace be equally
confused by a 64-bit lr or sp?

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


[PATCH] KVM: arm64: Fix 32bit PC wrap-around

2020-04-30 Thread Marc Zyngier
In the unlikely event that a 32bit vcpu traps into the hypervisor
on an instruction that is located right at the end of the 32bit
range, the emulation of that instruction is going to increment
PC past the 32bit range. This isn't great, as userspace can then
observe this value and get a bit confused.

Conversly, userspace can do things like (in the context of a 64bit
guest that is capable of 32bit EL0) setting PSTATE to AArch64-EL0,
set PC to a 64bit value, change PSTATE to AArch32-USR, and observe
that PC hasn't been truncated. More confusion.

Fix both by:
- truncating PC increments for 32bit guests
- sanitize PC every time a core reg is changed by userspace, and
  that PSTATE indicates a 32bit mode.

Signed-off-by: Marc Zyngier 
---
 arch/arm64/kvm/guest.c | 4 
 virt/kvm/arm/hyp/aarch32.c | 8 ++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 23ebe51410f0..2a159af82429 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -200,6 +200,10 @@ static int set_core_reg(struct kvm_vcpu *vcpu, const 
struct kvm_one_reg *reg)
}
 
memcpy((u32 *)regs + off, valp, KVM_REG_SIZE(reg->id));
+
+   if (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK)
+   *vcpu_pc(vcpu) = lower_32_bits(*vcpu_pc(vcpu));
+
 out:
return err;
 }
diff --git a/virt/kvm/arm/hyp/aarch32.c b/virt/kvm/arm/hyp/aarch32.c
index d31f267961e7..25c0e47d57cb 100644
--- a/virt/kvm/arm/hyp/aarch32.c
+++ b/virt/kvm/arm/hyp/aarch32.c
@@ -125,12 +125,16 @@ static void __hyp_text kvm_adjust_itstate(struct kvm_vcpu 
*vcpu)
  */
 void __hyp_text kvm_skip_instr32(struct kvm_vcpu *vcpu, bool is_wide_instr)
 {
+   u32 pc = *vcpu_pc(vcpu);
bool is_thumb;
 
is_thumb = !!(*vcpu_cpsr(vcpu) & PSR_AA32_T_BIT);
if (is_thumb && !is_wide_instr)
-   *vcpu_pc(vcpu) += 2;
+   pc += 2;
else
-   *vcpu_pc(vcpu) += 4;
+   pc += 4;
+
+   *vcpu_pc(vcpu) = pc;
+
kvm_adjust_itstate(vcpu);
 }
-- 
2.26.2

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


Re: [PATCH] arm64: kvm: fix gcc-10 shift warning

2020-04-30 Thread Will Deacon
On Thu, Apr 30, 2020 at 07:39:10PM +1000, Stephen Rothwell wrote:
> On Thu, 30 Apr 2020 09:29:28 +0100 Will Deacon  wrote:
> > On Thu, Apr 30, 2020 at 09:02:51AM +0100, Marc Zyngier wrote:
> > > On Wed, 29 Apr 2020 20:56:20 +0200
> > > Arnd Bergmann  wrote:
> > >   
> > > > Fixes: 22998131ab33 ("arm64: add support for folded p4d page tables")
> > 
> > Happy to queue via arm64 for 5.8. Does that work for you, Arnd, or were you
> > planning to get this in sooner than that?
> 
> The commit that this fixes is in Andrew's patch series in linux-next,
> so it should just go in there.

Yes, you're right. Sorry for the noise.

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


Re: [PATCH v2] arm64: Unify WORKAROUND_SPECULATIVE_AT_{NVHE,VHE}

2020-04-30 Thread Andrew Scull
On Thu, Apr 30, 2020 at 10:16:03AM +0100, Marc Zyngier wrote:
> On 2020-04-29 22:21, Will Deacon wrote:
> > Hi Andrew,
> > 
> > On Wed, Apr 22, 2020 at 05:13:46PM +0100, Andrew Scull wrote:
> > > Errata 1165522, 1319367 and 1530923 each allow TLB entries to be
> > > allocated as a result of a speculative AT instruction. In order to
> > > avoid mandating VHE on certain affected CPUs, apply the workaround to
> > > both the nVHE and the VHE case for all affected CPUs.
> > > 
> > > Signed-off-by: Andrew Scull 
> > > CC: Marc Zyngier 
> > > CC: James Morse 
> > > CC: Suzuki K Poulose 
> > > CC: Will Deacon 
> > > CC: Steven Price 
> > > ---
> > > * From v1 <20200327143941.195626-1-asc...@google.com>:
> > >   - Restored registers in VHE path
> > 
> > This largely looks good to me, but I don't understand these bits:
> > 
> > > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> > > index 8a1e81a400e0..651820f537fb 100644
> > > --- a/arch/arm64/kvm/hyp/switch.c
> > > +++ b/arch/arm64/kvm/hyp/switch.c
> > > @@ -138,7 +138,7 @@ static void __hyp_text
> > > __activate_traps_nvhe(struct kvm_vcpu *vcpu)
> > > 
> > >   write_sysreg(val, cptr_el2);
> > > 
> > > - if (cpus_have_final_cap(ARM64_WORKAROUND_SPECULATIVE_AT_NVHE)) {
> > > + if (cpus_have_const_cap(ARM64_WORKAROUND_SPECULATIVE_AT)) {
> > 
> > It seems like you consistently replace cpus_have_final_cap() with
> > cpus_have_const_cap(), but I can't figure out why that's required.
> 
> Seems like a bad conflict resolution. We definitely want to keep the
> final_cap checks for anything that will run at EL2, and probably
> everywhere else (if capabilities are not final by the time we hit KVM,
> we have bigger problems to solve).
> 
> Thanks,
> 
> M.
> -- 
> Jazz is not dead. It just smells funny...

Indeed, those weren't the 5 characters my eye was interested in. Looks
like we'll be having a v3..
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


RE: [PATCH v11 00/13] SMMUv3 Nested Stage Setup (IOMMU part)

2020-04-30 Thread Shameerali Kolothum Thodi
Hi Eric,

> -Original Message-
> From: Auger Eric [mailto:eric.au...@redhat.com]
> Sent: 16 April 2020 08:45
> To: Zhangfei Gao ; eric.auger@gmail.com;
> io...@lists.linux-foundation.org; linux-ker...@vger.kernel.org;
> k...@vger.kernel.org; kvmarm@lists.cs.columbia.edu; w...@kernel.org;
> j...@8bytes.org; m...@kernel.org; robin.mur...@arm.com
> Cc: jean-phili...@linaro.org; Shameerali Kolothum Thodi
> ; alex.william...@redhat.com;
> jacob.jun@linux.intel.com; yi.l@intel.com; peter.mayd...@linaro.org;
> t...@semihalf.com; bbhush...@marvell.com
> Subject: Re: [PATCH v11 00/13] SMMUv3 Nested Stage Setup (IOMMU part)
> 
> Hi Zhangfei,
> 
> On 4/16/20 6:25 AM, Zhangfei Gao wrote:
> >
> >
> > On 2020/4/14 下午11:05, Eric Auger wrote:
> >> This version fixes an issue observed by Shameer on an SMMU 3.2,
> >> when moving from dual stage config to stage 1 only config.
> >> The 2 high 64b of the STE now get reset. Otherwise, leaving the
> >> S2TTB set may cause a C_BAD_STE error.
> >>
> >> This series can be found at:
> >> https://github.com/eauger/linux/tree/v5.6-2stage-v11_10.1
> >> (including the VFIO part)
> >> The QEMU fellow series still can be found at:
> >> https://github.com/eauger/qemu/tree/v4.2.0-2stage-rfcv6
> >>
> >> Users have expressed interest in that work and tested v9/v10:
> >> - https://patchwork.kernel.org/cover/11039995/#23012381
> >> - https://patchwork.kernel.org/cover/11039995/#23197235
> >>
> >> Background:
> >>
> >> This series brings the IOMMU part of HW nested paging support
> >> in the SMMUv3. The VFIO part is submitted separately.
> >>
> >> The IOMMU API is extended to support 2 new API functionalities:
> >> 1) pass the guest stage 1 configuration
> >> 2) pass stage 1 MSI bindings
> >>
> >> Then those capabilities gets implemented in the SMMUv3 driver.
> >>
> >> The virtualizer passes information through the VFIO user API
> >> which cascades them to the iommu subsystem. This allows the guest
> >> to own stage 1 tables and context descriptors (so-called PASID
> >> table) while the host owns stage 2 tables and main configuration
> >> structures (STE).
> >>
> >>
> >
> > Thanks Eric
> >
> > Tested v11 on Hisilicon kunpeng920 board via hardware zip accelerator.
> > 1. no-sva works, where guest app directly use physical address via ioctl.
> Thank you for the testing. Glad it works for you.
> > 2. vSVA still not work, same as v10,
> Yes that's normal this series is not meant to support vSVM at this stage.
> 
> I intend to add the missing pieces during the next weeks.

Thanks for that. I have made an attempt to add the vSVA based on 
your v10 + JPBs sva patches. The host kernel and Qemu changes can 
be found here[1][2].

This basically adds multiple pasid support on top of your changes.
I have done some basic sanity testing and we have some initial success
with the zip vf dev on our D06 platform. Please note that the STALL event is
not yet supported though, but works fine if we mlock() guest usr mem.

I also noted that Intel patches for vSVA has couple of changes in the vfio 
interfaces
and hope there will be a convergence soon. Please let me know your plans
of a respin of this series and see whether incorporating the changes for 
multiple
pasid make sense or not for now.

Thanks,
Shameer

[1]https://github.com/hisilicon/qemu/tree/v4.2.0-2stage-rfcv6-vsva-prototype-v1
[2]https://github.com/hisilicon/kernel-dev/tree/vsva-prototype-host-v1

> Thanks
> 
> Eric
> > 3.  the v10 issue reported by Shameer has been solved,  first start qemu
> > with  iommu=smmuv3, then start qemu without  iommu=smmuv3
> > 4. no-sva also works without  iommu=smmuv3
> >
> > Test details in https://docs.qq.com/doc/DRU5oR1NtUERseFNL
> >
> > Thanks
> >

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


Re: [PATCH] arm64: kvm: fix gcc-10 shift warning

2020-04-30 Thread Stephen Rothwell
Hi Will,

On Thu, 30 Apr 2020 09:29:28 +0100 Will Deacon  wrote:
>
> On Thu, Apr 30, 2020 at 09:02:51AM +0100, Marc Zyngier wrote:
> > On Wed, 29 Apr 2020 20:56:20 +0200
> > Arnd Bergmann  wrote:
> >   
> > > Fixes: 22998131ab33 ("arm64: add support for folded p4d page tables")
> 
> Happy to queue via arm64 for 5.8. Does that work for you, Arnd, or were you
> planning to get this in sooner than that?

The commit that this fixes is in Andrew's patch series in linux-next,
so it should just go in there.

-- 
Cheers,
Stephen Rothwell


pgpZIc7BsGLVm.pgp
Description: OpenPGP digital signature
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v2] arm64: Unify WORKAROUND_SPECULATIVE_AT_{NVHE,VHE}

2020-04-30 Thread Marc Zyngier

On 2020-04-29 22:21, Will Deacon wrote:

Hi Andrew,

On Wed, Apr 22, 2020 at 05:13:46PM +0100, Andrew Scull wrote:

Errata 1165522, 1319367 and 1530923 each allow TLB entries to be
allocated as a result of a speculative AT instruction. In order to
avoid mandating VHE on certain affected CPUs, apply the workaround to
both the nVHE and the VHE case for all affected CPUs.

Signed-off-by: Andrew Scull 
CC: Marc Zyngier 
CC: James Morse 
CC: Suzuki K Poulose 
CC: Will Deacon 
CC: Steven Price 
---
* From v1 <20200327143941.195626-1-asc...@google.com>:
  - Restored registers in VHE path


This largely looks good to me, but I don't understand these bits:


diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 8a1e81a400e0..651820f537fb 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -138,7 +138,7 @@ static void __hyp_text 
__activate_traps_nvhe(struct kvm_vcpu *vcpu)


write_sysreg(val, cptr_el2);

-   if (cpus_have_final_cap(ARM64_WORKAROUND_SPECULATIVE_AT_NVHE)) {
+   if (cpus_have_const_cap(ARM64_WORKAROUND_SPECULATIVE_AT)) {


It seems like you consistently replace cpus_have_final_cap() with
cpus_have_const_cap(), but I can't figure out why that's required.


Seems like a bad conflict resolution. We definitely want to keep the
final_cap checks for anything that will run at EL2, and probably
everywhere else (if capabilities are not final by the time we hit KVM,
we have bigger problems to solve).

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] arm64: kvm: fix gcc-10 shift warning

2020-04-30 Thread Will Deacon
On Thu, Apr 30, 2020 at 09:02:51AM +0100, Marc Zyngier wrote:
> On Wed, 29 Apr 2020 20:56:20 +0200
> Arnd Bergmann  wrote:
> 
> > gcc-10 warns that the 32-bit zero cannot be shifted more than
> > 32 bits to the right:
> > 
> > arch/arm64/kvm/../../../virt/kvm/arm/mmu.c: In function 
> > 'clear_hyp_p4d_entry':
> > arch/arm64/include/asm/pgtable.h:630:35: error: right shift count >= width 
> > of type [-Werror=shift-count-overflow]
> >   630 | #define pud_index(addr)  (((addr) >> PUD_SHIFT) & (PTRS_PER_PUD - 
> > 1))
> >   |   ^~
> > arch/arm64/include/asm/memory.h:271:45: note: in definition of macro 
> > '__phys_to_virt'
> >   271 | #define __phys_to_virt(x) ((unsigned long)((x) - physvirt_offset))
> >   | ^
> > arch/arm64/include/asm/pgtable.h:633:42: note: in expansion of macro '__va'
> >   633 | #define pud_offset(dir, addr)  ((pud_t 
> > *)__va(pud_offset_phys((dir), (addr
> >   |  ^~~~
> > arch/arm64/include/asm/pgtable.h:632:73: note: in expansion of macro 
> > 'pud_index'
> >   632 | #define pud_offset_phys(dir, addr) 
> > (p4d_page_paddr(READ_ONCE(*(dir))) + pud_index(addr) * sizeof(pud_t))
> >   | 
> > ^
> > arch/arm64/include/asm/pgtable.h:633:47: note: in expansion of macro 
> > 'pud_offset_phys'
> >   633 | #define pud_offset(dir, addr)  ((pud_t 
> > *)__va(pud_offset_phys((dir), (addr
> >   |   ^~~
> > arch/arm64/kvm/../../../virt/kvm/arm/mmu.c:510:36: note: in expansion of 
> > macro 'pud_offset'
> >   510 |  pud_t *pud_table __maybe_unused = pud_offset(p4d, 0);
> >   |^~
> > 
> > This is harmless, and the warning is a little bit silly for
> > a zero constant, but it's trivial to fix by making it an
> > unsigned long, so do that.
> > 
> > Fixes: 22998131ab33 ("arm64: add support for folded p4d page tables")
> > Signed-off-by: Arnd Bergmann 
> > ---
> >  virt/kvm/arm/mmu.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
> > index 48d4288c5f1b..534d9798c3cb 100644
> > --- a/virt/kvm/arm/mmu.c
> > +++ b/virt/kvm/arm/mmu.c
> > @@ -507,7 +507,7 @@ static void clear_hyp_pgd_entry(pgd_t *pgd)
> >  
> >  static void clear_hyp_p4d_entry(p4d_t *p4d)
> >  {
> > -   pud_t *pud_table __maybe_unused = pud_offset(p4d, 0);
> > +   pud_t *pud_table __maybe_unused = pud_offset(p4d, 0UL);
> > VM_BUG_ON(p4d_huge(*p4d));
> > p4d_clear(p4d);
> > pud_free(NULL, pud_table);
> 
> Acked-by: Marc Zyngier 

Happy to queue via arm64 for 5.8. Does that work for you, Arnd, or were you
planning to get this in sooner than that?

Cheers,

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


Re: [PATCH] arm64: kvm: fix gcc-10 shift warning

2020-04-30 Thread Marc Zyngier
On Wed, 29 Apr 2020 20:56:20 +0200
Arnd Bergmann  wrote:

> gcc-10 warns that the 32-bit zero cannot be shifted more than
> 32 bits to the right:
> 
> arch/arm64/kvm/../../../virt/kvm/arm/mmu.c: In function 'clear_hyp_p4d_entry':
> arch/arm64/include/asm/pgtable.h:630:35: error: right shift count >= width of 
> type [-Werror=shift-count-overflow]
>   630 | #define pud_index(addr)  (((addr) >> PUD_SHIFT) & (PTRS_PER_PUD - 1))
>   |   ^~
> arch/arm64/include/asm/memory.h:271:45: note: in definition of macro 
> '__phys_to_virt'
>   271 | #define __phys_to_virt(x) ((unsigned long)((x) - physvirt_offset))
>   | ^
> arch/arm64/include/asm/pgtable.h:633:42: note: in expansion of macro '__va'
>   633 | #define pud_offset(dir, addr)  ((pud_t *)__va(pud_offset_phys((dir), 
> (addr
>   |  ^~~~
> arch/arm64/include/asm/pgtable.h:632:73: note: in expansion of macro 
> 'pud_index'
>   632 | #define pud_offset_phys(dir, addr) (p4d_page_paddr(READ_ONCE(*(dir))) 
> + pud_index(addr) * sizeof(pud_t))
>   |   
>   ^
> arch/arm64/include/asm/pgtable.h:633:47: note: in expansion of macro 
> 'pud_offset_phys'
>   633 | #define pud_offset(dir, addr)  ((pud_t *)__va(pud_offset_phys((dir), 
> (addr
>   |   ^~~
> arch/arm64/kvm/../../../virt/kvm/arm/mmu.c:510:36: note: in expansion of 
> macro 'pud_offset'
>   510 |  pud_t *pud_table __maybe_unused = pud_offset(p4d, 0);
>   |^~
> 
> This is harmless, and the warning is a little bit silly for
> a zero constant, but it's trivial to fix by making it an
> unsigned long, so do that.
> 
> Fixes: 22998131ab33 ("arm64: add support for folded p4d page tables")
> Signed-off-by: Arnd Bergmann 
> ---
>  virt/kvm/arm/mmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
> index 48d4288c5f1b..534d9798c3cb 100644
> --- a/virt/kvm/arm/mmu.c
> +++ b/virt/kvm/arm/mmu.c
> @@ -507,7 +507,7 @@ static void clear_hyp_pgd_entry(pgd_t *pgd)
>  
>  static void clear_hyp_p4d_entry(p4d_t *p4d)
>  {
> - pud_t *pud_table __maybe_unused = pud_offset(p4d, 0);
> + pud_t *pud_table __maybe_unused = pud_offset(p4d, 0UL);
>   VM_BUG_ON(p4d_huge(*p4d));
>   p4d_clear(p4d);
>   pud_free(NULL, pud_table);

Acked-by: Marc Zyngier 

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