[PATCH V2] arm64/cpufeature: Validate hypervisor capabilities during CPU hotplug

2020-05-07 Thread Anshuman Khandual
This validates hypervisor capabilities like VMID width, IPA range for any
hot plug CPU against system finalized values. While here, it factors out
get_vmid_bits() for general use and also defines ID_AA64MMFR0_PARANGE_MASK.

Cc: Catalin Marinas 
Cc: Will Deacon 
Cc: Marc Zyngier 
Cc: Mark Rutland 
Cc: James Morse 
Cc: Suzuki K Poulose 
Cc: linux-arm-ker...@lists.infradead.org
Cc: kvmarm@lists.cs.columbia.edu
Cc: linux-ker...@vger.kernel.org

Suggested-by: Suzuki Poulose 
Signed-off-by: Anshuman Khandual 
---
Changes in V2:

- Added is_hyp_mode_available() check per Marc
- Moved verify_kvm_capabilities() into cpufeature.c per Marc
- Added helper get_kvm_ipa_limit() to fetch kvm_ipa_limit per Marc
- Renamed kvm as hyp including the commit message per Marc

Changes in V1: (https://patchwork.kernel.org/patch/11532565/)

 arch/arm64/include/asm/cpufeature.h | 20 +
 arch/arm64/include/asm/kvm_mmu.h|  2 +-
 arch/arm64/include/asm/sysreg.h |  1 +
 arch/arm64/kernel/cpufeature.c  | 33 +
 arch/arm64/kvm/reset.c  | 11 --
 5 files changed, 64 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/cpufeature.h 
b/arch/arm64/include/asm/cpufeature.h
index afe08251ff95..fbbb4d2216f0 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -745,6 +745,26 @@ static inline bool cpu_has_hw_af(void)
 extern bool cpu_has_amu_feat(int cpu);
 #endif
 
+static inline unsigned int get_vmid_bits(u64 mmfr1)
+{
+   int vmid_bits;
+
+   vmid_bits = cpuid_feature_extract_unsigned_field(mmfr1,
+   ID_AA64MMFR1_VMIDBITS_SHIFT);
+   if (vmid_bits == ID_AA64MMFR1_VMIDBITS_16)
+   return 16;
+
+   /*
+* Return the default here even if any reserved
+* value is fetched from the system register.
+*/
+   return 8;
+}
+
+#ifdef CONFIG_KVM_ARM_HOST
+u32 get_kvm_ipa_limit(void);
+#endif
+
 #endif /* __ASSEMBLY__ */
 
 #endif
diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
index 30b0e8d6b895..a7137e144b97 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -416,7 +416,7 @@ static inline unsigned int kvm_get_vmid_bits(void)
 {
int reg = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);
 
-   return (cpuid_feature_extract_unsigned_field(reg, 
ID_AA64MMFR1_VMIDBITS_SHIFT) == 2) ? 16 : 8;
+   return get_vmid_bits(reg);
 }
 
 /*
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index c4ac0ac25a00..3510a4668970 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -705,6 +705,7 @@
 #define ID_AA64MMFR0_TGRAN16_SUPPORTED 0x1
 #define ID_AA64MMFR0_PARANGE_480x5
 #define ID_AA64MMFR0_PARANGE_520x6
+#define ID_AA64MMFR0_PARANGE_MASK  0x7
 
 #ifdef CONFIG_ARM64_PA_BITS_52
 #define ID_AA64MMFR0_PARANGE_MAX   ID_AA64MMFR0_PARANGE_52
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 9fac745aa7bb..7e5ff452574c 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -2181,6 +2181,36 @@ static void verify_sve_features(void)
/* Add checks on other ZCR bits here if necessary */
 }
 
+#ifdef CONFIG_KVM_ARM_HOST
+void verify_hyp_capabilities(void)
+{
+   u64 safe_mmfr1, mmfr0, mmfr1;
+   int parange, ipa_max;
+   unsigned int safe_vmid_bits, vmid_bits;
+
+   safe_mmfr1 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);
+   mmfr0 = read_cpuid(ID_AA64MMFR0_EL1);
+   mmfr1 = read_cpuid(ID_AA64MMFR1_EL1);
+
+   /* Verify VMID bits */
+   safe_vmid_bits = get_vmid_bits(safe_mmfr1);
+   vmid_bits = get_vmid_bits(mmfr1);
+   if (vmid_bits < safe_vmid_bits) {
+   pr_crit("CPU%d: VMID width mismatch\n", smp_processor_id());
+   cpu_die_early();
+   }
+
+   /* Verify IPA range */
+   parange = mmfr0 & ID_AA64MMFR0_PARANGE_MASK;
+   ipa_max = id_aa64mmfr0_parange_to_phys_shift(parange);
+   if (ipa_max < get_kvm_ipa_limit()) {
+   pr_crit("CPU%d: IPA range mismatch\n", smp_processor_id());
+   cpu_die_early();
+   }
+}
+#else  /* !CONFIG_KVM_ARM_HOST */
+static inline void verify_hyp_capabilities(void) { }
+#endif /* CONFIG_KVM_ARM_HOST */
 
 /*
  * Run through the enabled system capabilities and enable() it on this CPU.
@@ -2206,6 +2236,9 @@ static void verify_local_cpu_capabilities(void)
 
if (system_supports_sve())
verify_sve_features();
+
+   if (is_hyp_mode_available())
+   verify_hyp_capabilities();
 }
 
 void check_local_cpu_capabilities(void)
diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index 30b7ea680f66..1131b112dda2 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -340,11 +340,17 @@ int kvm_reset_vcpu(struct kvm_vcpu 

Re: [PATCH 08/26] KVM: arm64: Use TTL hint in when invalidating stage-2 translations

2020-05-07 Thread Andrew Scull
> @@ -176,7 +177,7 @@ static void clear_stage2_pud_entry(struct kvm_s2_mmu 
> *mmu, pud_t *pud, phys_addr
>   pmd_t *pmd_table __maybe_unused = stage2_pmd_offset(kvm, pud, 0);
>   VM_BUG_ON(stage2_pud_huge(kvm, *pud));
>   stage2_pud_clear(kvm, pud);
> - kvm_tlb_flush_vmid_ipa(mmu, addr);
> + kvm_tlb_flush_vmid_ipa(mmu, addr, S2_NO_LEVEL_HINT);
>   stage2_pmd_free(kvm, pmd_table);
>   put_page(virt_to_page(pud));
>  }
> @@ -186,7 +187,7 @@ static void clear_stage2_pmd_entry(struct kvm_s2_mmu 
> *mmu, pmd_t *pmd, phys_addr
>   pte_t *pte_table = pte_offset_kernel(pmd, 0);
>   VM_BUG_ON(pmd_thp_or_huge(*pmd));
>   pmd_clear(pmd);
> - kvm_tlb_flush_vmid_ipa(mmu, addr);
> + kvm_tlb_flush_vmid_ipa(mmu, addr, S2_NO_LEVEL_HINT);
>   free_page((unsigned long)pte_table);
>   put_page(virt_to_page(pmd));
>  }

Going by the names, is it possible to give a better level hint for these
cases?
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH 07/26] KVM: arm64: Add a level hint to __kvm_tlb_flush_vmid_ipa

2020-05-07 Thread Andrew Scull
> -void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm_s2_mmu *mmu, phys_addr_t 
> ipa)
> +void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm_s2_mmu *mmu,
> +  phys_addr_t ipa, int level)

The level feels like a good opportunity for an enum to add some
documentation from the type.

>  static void kvm_tlb_flush_vmid_ipa(struct kvm_s2_mmu *mmu, phys_addr_t ipa)
>  {
> - kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, mmu, ipa);
> + kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, mmu, ipa, 0);

With the constants from the next patch brought forward, the magic 0 can
be given a name, although it's very temporary so..

Otherwise, looks good.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


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

2020-05-07 Thread Quentin Perret
On Thursday 07 May 2020 at 15:01:07 (+0100), Marc Zyngier wrote:
> >  /*
> > - * u64 __kvm_call_hyp(void *hypfn, ...);
> > + * u64 __kvm_call_hyp(unsigned long arg, ...);
> >   *
> >   * This is not really a variadic function in the classic C-way and care 
> > must
> >   * be taken when calling this to ensure parameters are passed in registers
> >   * only, since the stack will change between the caller and the callee.
> > - *
> > - * Call the function with the first argument containing a pointer to the
> > - * function you wish to call in Hyp mode, and subsequent arguments will be
> > - * passed as x0, x1, and x2 (a maximum of 3 arguments in addition to the
> > - * function pointer can be passed).  The function being called must be 
> > mapped
> > - * in Hyp mode (see init_hyp_mode in arch/arm/kvm/arm.c).  Return values 
> > are
> > - * passed in x0.
> > - *
> > - * A function pointer with a value 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.
> 
> I don't think any of this becomes obsolete with this patch (apart from
> the reference to 32bit), and only changes with patch #2. Or am I
> misunderstanding something?

Nope, I think you're right. To be fair, this patch has changed quite
a bit since the first version (which did change that comment a little
later IIRC), but David has done all the hard work on top so I'll let
him answer that one.

And David, feel free to take the authorship for this patch -- I barely
recognize it (for the better), so it's more than fair if you get the
credit :)

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


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

2020-05-07 Thread David Brazdil
Hi Marc,

> 
> What breaks without this constraint? Is it a fix that should go in
> early? Otherwise looks good.

This only becomes an issue when __hyp_call_panic_nvhe() and
__hyp_call_panic_vhe() are moved to separate files, so I don't think it's
necessary to go in early.

Currently the string variable (declared static) is seen by the C compiler as
used by __hyp_call_panic_vhe(). But when split, the variable in the nVHE source
file becomes unused, is dropped by the compiler and the inline assembly's
reference is unresolved. We could then alias __hyp_text___hyp_panic_string back
to the VHE copy, but this is the right way of addressing it.

Thanks for the review,
David

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


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

2020-05-07 Thread Quentin Perret
Hey Marc,

On Thursday 07 May 2020 at 14:10:30 (+0100), Marc Zyngier wrote:
> Hi David, Quentin,
> 
> On Thu, 30 Apr 2020 15:48:18 +0100,
> David Brazdil  wrote:
> > 
> > From: Quentin Perret 
> > 
> > In preparation for removing the hyp code from the TCB, convert the current
> 
> nit: Unless we have a different interpretation of the TCB acronym, I
> think you want to remove anything *but* the EL2 code from the TCB.

Argh! Right... This should have been 'removing the /host/ code' :-)


> > 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
> 
> This whole thing screams "enum" into my ears. Having to maintain these
> as #defines feels like a pain, specially if the numbers are never used
> in assembly code. (and for that, we have asm-offset.h).

This is essentially inspired from the various 'unistd.h' files we have
in the kernel, e.g. include/uapi/asm-generic/unistd.h, which have
exactly this type of construct. So, this was really just for consistency,
but no strong opinion from me.

> 
> > +
> > +/* 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)
> 
> I don't really get what is this offset for. It is too small to be used
> to skip the stub hypercalls (you'd need to start at 0x1000).

Oh, OK. I assumed anything above HVC_STUB_HCALL_NR would be fine. But
yes, this offset is really arbitrary, so if 0x1000 is better then that
totally works. For my education, where is that 0x1000 coming from ?

> Given
> that you store the whole thing in an array, you're wasting some
> memory. I'm sure you have a good story for it though! ;-)

Note that the array's length is __KVM_HOST_HCALL_TABLE_IDX_SIZE, which
doesn't include the offset, so we shouldn't be wasting memory here.

> > +
> > +/* 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 b/arch/arm64/kvm/hyp/Makefile
> > index 8c9880783839..29e2b2cd2fbc 100644
> > --- a/arch/arm64/kvm/hyp/Makefile
> > +++ b/arch/arm64/kvm/hyp/Makefile
> > @@ -9,7 +9,7 @@ ccflags-y += -fno-stack-protector 
> > -DDISABLE_BRANCH_PROFILING \
> >  obj-$(CONFIG_KVM) += hyp.o
> >  
> >  hyp-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
> > +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
> >  # compiler 

Re: [PATCH 09/26] KVM: arm64: vgic-v3: Take cpu_if pointer directly instead of vcpu

2020-05-07 Thread James Morse
Hi Marc, Christoffer,

On 22/04/2020 13:00, Marc Zyngier wrote:
> From: Christoffer Dall 
> 
> If we move the used_lrs field to the version-specific cpu interface
> structure, the following functions only operate on the struct
> vgic_v3_cpu_if and not the full vcpu:
> 
>   __vgic_v3_save_state
>   __vgic_v3_restore_state
>   __vgic_v3_activate_traps
>   __vgic_v3_deactivate_traps
>   __vgic_v3_save_aprs
>   __vgic_v3_restore_aprs
> 
> This is going to be very useful for nested virt, 

... because you don't need to consider whether the vcpu is running in vEL2?


> so move the used_lrs
> field and change the prototypes and implementations of these functions to
> take the cpu_if parameter directly.


> No functional change.

Looks like no change!

Reviewed-by: James Morse 


Thanks,

James

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


Re: [PATCH v4 02/14] arm: add support for folded p4d page tables

2020-05-07 Thread Mike Rapoport
Hi,

On Thu, May 07, 2020 at 02:16:56PM +0200, Marek Szyprowski wrote:
> Hi
> 
> On 14.04.2020 17:34, Mike Rapoport wrote:
> > From: Mike Rapoport 
> >
> > Implement primitives necessary for the 4th level folding, add walks of p4d
> > level where appropriate, and remove __ARCH_USE_5LEVEL_HACK.
> >
> > Signed-off-by: Mike Rapoport 
> 
> Today I've noticed that kexec is broken on ARM 32bit. Bisecting between 
> current linux-next and v5.7-rc1 pointed to this commit. I've tested this 
> on Odroid XU4 and Raspberry Pi4 boards. Here is the relevant log:
> 
> # kexec --kexec-syscall -l zImage --append "$(cat /proc/cmdline)"
> memory_range[0]:0x4000..0xbe9f
> memory_range[0]:0x4000..0xbe9f
> # kexec -e
> kexec_core: Starting new kernel
> 8<--- cut here ---
> Unable to handle kernel paging request at virtual address c010f1f4
> pgd = c6817793
> [c010f1f4] *pgd=441e(bad)
> Internal error: Oops: 80d [#1] PREEMPT ARM
> Modules linked in:
> CPU: 0 PID: 1329 Comm: kexec Tainted: G    W 
> 5.7.0-rc3-00127-g6cba81ed0f62 #611
> Hardware name: Samsung Exynos (Flattened Device Tree)
> PC is at machine_kexec+0x40/0xfc

Any chance you have the debug info in this kernel?
scripts/faddr2line would come handy here.

> LR is at 0x
> pc : []    lr : []    psr: 6013
> sp : ebc13e60  ip : 40008000  fp : 0001
> r10: 0058  r9 : fee1dead  r8 : 0001
> r7 : c121387c  r6 : 6c224000  r5 : ece40c00  r4 : ec222000
> r3 : c010f1f4  r2 : c110  r1 : c110  r0 : 418d
> Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
> Control: 10c5387d  Table: 6bc14059  DAC: 0051
> Process kexec (pid: 1329, stack limit = 0x366bb4dc)
> Stack: (0xebc13e60 to 0xebc14000)
> ...
> [] (machine_kexec) from [] (kernel_kexec+0x74/0x7c)
> [] (kernel_kexec) from [] (__do_sys_reboot+0x1f8/0x210)
> [] (__do_sys_reboot) from [] (ret_fast_syscall+0x0/0x28)
> Exception stack(0xebc13fa8 to 0xebc13ff0)
> ...
> ---[ end trace 3e8d6c81723c778d ]---
> 1329 Segmentation fault  ./kexec -e
> 
> > ---
> >   arch/arm/include/asm/pgtable.h |  1 -
> >   arch/arm/lib/uaccess_with_memcpy.c |  7 +-
> >   arch/arm/mach-sa1100/assabet.c |  2 +-
> >   arch/arm/mm/dump.c | 29 +-
> >   arch/arm/mm/fault-armv.c   |  7 +-
> >   arch/arm/mm/fault.c| 22 ++--
> >   arch/arm/mm/idmap.c|  3 ++-
> >   arch/arm/mm/init.c |  2 +-
> >   arch/arm/mm/ioremap.c  | 12 ++---
> >   arch/arm/mm/mm.h   |  2 +-
> >   arch/arm/mm/mmu.c  | 35 +-
> >   arch/arm/mm/pgd.c  | 40 --
> >   12 files changed, 125 insertions(+), 37 deletions(-)
> >
> > ...
> 
> Best regards
> -- 
> Marek Szyprowski, PhD
> Samsung R Institute Poland
> 

-- 
Sincerely yours,
Mike.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


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

2020-05-07 Thread Marc Zyngier

On 2020-05-07 16:07, Marc Zyngier wrote:

On Thu, 30 Apr 2020 15:48:21 +0100,
David Brazdil  wrote:


[...]

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,


That's not true. VHE does handle HVCs from the guest. If you make VHE
panic on guest exit, I'll come after you! ;-)


Duh, I can't read. "host HVCs". You can relax. ;-)

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 07/26] KVM: arm64: Add a level hint to __kvm_tlb_flush_vmid_ipa

2020-05-07 Thread Marc Zyngier

On 2020-05-07 16:08, Andrew Scull wrote:
-void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm_s2_mmu *mmu, 
phys_addr_t ipa)

+void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm_s2_mmu *mmu,
+phys_addr_t ipa, int level)


The level feels like a good opportunity for an enum to add some
documentation from the type.


Sure, why not. I'll give it a go.



 static void kvm_tlb_flush_vmid_ipa(struct kvm_s2_mmu *mmu, 
phys_addr_t ipa)

 {
-   kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, mmu, ipa);
+   kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, mmu, ipa, 0);


With the constants from the next patch brought forward, the magic 0 can
be given a name, although it's very temporary so..


Yup. To the point where I've now squashed this patch and the following
one together, and moved the constants to the previous patch.


Otherwise, looks good.


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 05/15] arm64: kvm: Build hyp-entry.S separately for VHE/nVHE

2020-05-07 Thread Marc Zyngier
On Thu, 30 Apr 2020 15:48:21 +0100,
David Brazdil  wrote:
> 
> This patch is part of a series which builds KVM's non-VHE hyp code separately
> from VHE and the rest of the kernel.

This sentence should be part of your cover letter, and not in the 5th
patch.

> 
> 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,

That's not true. VHE does handle HVCs from the guest. If you make VHE
panic on guest exit, I'll come after you! ;-)

>   * 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.

How comes? You certainly need to be able to use the generated code,
don't you? Or do you actually mean that the assembly code doesn't need
to live in the file that contains the vectors themselves (which I'd
agree with)?

> 
> 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)[];

This is becoming pretty ugly. I'd rather we have a helper that emits
the declaration for both symbols. Or something.

> +
> +#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));

I find it pretty annoying (again) that I have to know where a symbol
lives (kernel or nVHE-EL2) to know which kvm_*_ref() helper to use.
Maybe there is no good solution to this, but still...

>   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));

Please keep the assignment on a single line (and screw checkpatch).

>   unsigned long size = __BP_HARDEN_HYP_VECS_SZ;
>  
>   /*
> @@ 

Re: [PATCH] arm64/cpufeature: Verify KVM capabilities during CPU hotplug

2020-05-07 Thread Anshuman Khandual


On 05/07/2020 03:50 PM, Marc Zyngier wrote:
> On Thu,  7 May 2020 11:49:47 +0530
> Anshuman Khandual  wrote:
> 
> Hi Anshuman,

Hi Marc,

> 
>> This validates KVM capabilities like VMID width, IPA range for hotplug CPU
>> against system finalized values. While here, it factors out get_vmid_bits()
>> for general use and also defines ID_AA64MMFR0_PARANGE_MASK.
> 
> nit: these are not KVM-specific capabilities, but general
> virtualization features.

Sure, will change as (s/kvm/hyp) instead and update the commit
message here.

> 
>>
>> Cc: Catalin Marinas 
>> Cc: Will Deacon 
>> Cc: Marc Zyngier 
>> Cc: Mark Rutland 
>> Cc: James Morse 
>> Cc: Suzuki K Poulose 
>> Cc: linux-arm-ker...@lists.infradead.org
>> Cc: kvmarm@lists.cs.columbia.edu
>> Cc: linux-ker...@vger.kernel.org
>>
>> Suggested-by: Suzuki Poulose 
>> Signed-off-by: Anshuman Khandual 
>> ---
>>  arch/arm64/include/asm/cpufeature.h | 22 +++
>>  arch/arm64/include/asm/kvm_mmu.h|  2 +-
>>  arch/arm64/include/asm/sysreg.h |  1 +
>>  arch/arm64/kernel/cpufeature.c  |  2 ++
>>  arch/arm64/kvm/reset.c  | 33 +++--
>>  5 files changed, 57 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/cpufeature.h 
>> b/arch/arm64/include/asm/cpufeature.h
>> index afe08251ff95..6808a2091de4 100644
>> --- a/arch/arm64/include/asm/cpufeature.h
>> +++ b/arch/arm64/include/asm/cpufeature.h
>> @@ -745,6 +745,28 @@ static inline bool cpu_has_hw_af(void)
>>  extern bool cpu_has_amu_feat(int cpu);
>>  #endif
>>  
>> +static inline unsigned int get_vmid_bits(u64 mmfr1)
>> +{
>> +int vmid_bits;
>> +
>> +vmid_bits = cpuid_feature_extract_unsigned_field(mmfr1,
>> +ID_AA64MMFR1_VMIDBITS_SHIFT);
>> +if (vmid_bits == ID_AA64MMFR1_VMIDBITS_16)
>> +return 16;
>> +
>> +/*
>> + * Return the default here even if any reserved
>> + * value is fetched from the system register.
>> + */
>> +return 8;
>> +}
>> +
>> +#ifdef CONFIG_KVM_ARM_HOST
>> +void verify_kvm_capabilities(void);
>> +#else
>> +static inline void verify_kvm_capabilities(void) { }
>> +#endif
>> +
>>  #endif /* __ASSEMBLY__ */
>>  
>>  #endif
>> diff --git a/arch/arm64/include/asm/kvm_mmu.h 
>> b/arch/arm64/include/asm/kvm_mmu.h
>> index 30b0e8d6b895..a7137e144b97 100644
>> --- a/arch/arm64/include/asm/kvm_mmu.h
>> +++ b/arch/arm64/include/asm/kvm_mmu.h
>> @@ -416,7 +416,7 @@ static inline unsigned int kvm_get_vmid_bits(void)
>>  {
>>  int reg = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);
>>  
>> -return (cpuid_feature_extract_unsigned_field(reg,
>> ID_AA64MMFR1_VMIDBITS_SHIFT) == 2) ? 16 : 8;
>> +return get_vmid_bits(reg);
>>  }
>>  
>>  /*
>> diff --git a/arch/arm64/include/asm/sysreg.h
>> b/arch/arm64/include/asm/sysreg.h index c4ac0ac25a00..3510a4668970
>> 100644 --- a/arch/arm64/include/asm/sysreg.h
>> +++ b/arch/arm64/include/asm/sysreg.h
>> @@ -705,6 +705,7 @@
>>  #define ID_AA64MMFR0_TGRAN16_SUPPORTED  0x1
>>  #define ID_AA64MMFR0_PARANGE_48 0x5
>>  #define ID_AA64MMFR0_PARANGE_52 0x6
>> +#define ID_AA64MMFR0_PARANGE_MASK   0x7
>>  
>>  #ifdef CONFIG_ARM64_PA_BITS_52
>>  #define ID_AA64MMFR0_PARANGE_MAXID_AA64MMFR0_PARANGE_52
>> diff --git a/arch/arm64/kernel/cpufeature.c
>> b/arch/arm64/kernel/cpufeature.c index 9fac745aa7bb..041dd610b0f8
>> 100644 --- a/arch/arm64/kernel/cpufeature.c
>> +++ b/arch/arm64/kernel/cpufeature.c
>> @@ -2206,6 +2206,8 @@ static void verify_local_cpu_capabilities(void)
>>  
>>  if (system_supports_sve())
>>  verify_sve_features();
>> +
>> +verify_kvm_capabilities();
> 
> You should only check this if booted at EL2. Otherwise, it doesn't
> really matter.

Sure, will first check on is_hyp_mode_available() before calling into
verify_kvm_capabilities().

> 
>>  }
>>  
>>  void check_local_cpu_capabilities(void)
>> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
>> index 30b7ea680f66..1eebcc2a8396 100644
>> --- a/arch/arm64/kvm/reset.c
>> +++ b/arch/arm64/kvm/reset.c
>> @@ -340,11 +340,39 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
>>  return ret;
>>  }
>>  
>> +void verify_kvm_capabilities(void)
> 
> This is really in the wrong file. reset.c is supposed to contain things
> that are meaningful to the guest reset state. This clearly isn't. I'd
> suggest you add an accessor for the kvm_ipa_limit variable, and keep
> the function next to the other verify_* functions in cpufeature.c.

Sure, will do.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


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

2020-05-07 Thread Marc Zyngier
On Thu, 30 Apr 2020 15:48:17 +0100,
David Brazdil  wrote:
> 
> 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.

But the arguments still have a meaning, don't they? See below.

> 
> 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 

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

2020-05-07 Thread Will Deacon
On Tue, May 05, 2020 at 04:45:20PM +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 | 38 +
>  arch/arm64/kvm/hyp/Makefile | 15 ---
>  2 files changed, 17 insertions(+), 36 deletions(-)

Acked-by: Will Deacon 

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


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

2020-05-07 Thread Marc Zyngier
On Thu, 30 Apr 2020 15:48:20 +0100,
David Brazdil  wrote:
> 
> Add new folder arch/arm64/kvm/hyp/nvhe and a Makefile for building code that

s/folder/directory/

> 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`.

Madness. ;-)

> 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)))
> +

Things are becoming a bit redundant here: you have a has_vhe() test,
followed by a is_kernel_in_hyp_mode() test in kvm_ksym_ref(). Certainly
this could do with some simplification. Another thing to think about is
whether has_vhe() is the right thing to use as it isn't valid until you
have discovered the capabilities. I think it is fine, but you should
check it.

>  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 

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

2020-05-07 Thread Marc Zyngier
On Thu, 30 Apr 2020 15:48:19 +0100,
David Brazdil  wrote:
> 
> __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
> 
> 

What breaks without this constraint? Is it a fix that should go in
early? Otherwise looks good.

Thanks,

M.

-- 
Without deviation from the norm, progress is not possible.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v4 2/7] KVM: arm64: clean up redundant 'kvm_run' parameters

2020-05-07 Thread Tianjia Zhang



On 2020/5/5 16:39, Marc Zyngier wrote:

Hi Tianjia,

On 2020-04-27 05:35, Tianjia Zhang wrote:

In the current kvm version, 'kvm_run' has been included in the 'kvm_vcpu'
structure. For historical reasons, many kvm-related function parameters
retain the 'kvm_run' and 'kvm_vcpu' parameters at the same time. This
patch does a unified cleanup of these remaining redundant parameters.

Signed-off-by: Tianjia Zhang 


On the face of it, this looks OK, but I haven't tried to run the
resulting kernel. I'm not opposed to taking this patch *if* there
is an agreement across architectures to take the series (I value
consistency over the janitorial exercise).

Another thing is that this is going to conflict with the set of
patches that move the KVM/arm code back where it belongs (arch/arm64/kvm),
so I'd probably cherry-pick that one directly.

Thanks,

     M.



Do I need to submit this set of patches separately for each 
architecture? Could it be merged at once, if necessary, I will

resubmit based on the latest mainline.

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


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

2020-05-07 Thread Marc Zyngier
Hi David, Quentin,

On Thu, 30 Apr 2020 15:48:18 +0100,
David Brazdil  wrote:
> 
> From: Quentin Perret 
> 
> In preparation for removing the hyp code from the TCB, convert the current

nit: Unless we have a different interpretation of the TCB acronym, I
think you want to remove anything *but* the EL2 code from the TCB.

> 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_el21
> +__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_vmid3
> +__KVM_HOST_HCALL(__kvm_tlb_flush_local_vmid)
> +
> +#define __KVM_HOST_HCALL_TABLE_IDX___kvm_flush_vm_context4
> +__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_vmid  6
> +__KVM_HOST_HCALL(__kvm_tlb_flush_vmid)
> +
> +#define __KVM_HOST_HCALL_TABLE_IDX___kvm_tlb_flush_vmid_ipa  7
> +__KVM_HOST_HCALL(__kvm_tlb_flush_vmid_ipa)
> +
> +#define __KVM_HOST_HCALL_TABLE_IDX___vgic_v3_init_lrs8
> +__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_vmcr  10
> +__KVM_HOST_HCALL(__vgic_v3_write_vmcr)
> +
> +#define __KVM_HOST_HCALL_TABLE_IDX___vgic_v3_restore_aprs11
> +__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_SIZE  14

This whole 

[PATCH resend 1/2] KVM: arm64: Clean up the checking for huge mapping

2020-05-07 Thread Zenghui Yu
From: Suzuki K Poulose 

If we are checking whether the stage2 can map PAGE_SIZE,
we don't have to do the boundary checks as both the host
VMA and the guest memslots are page aligned. Bail the case
easily.

While we're at it, fixup a typo in the comment below.

Cc: Christoffer Dall 
Cc: Marc Zyngier 
Signed-off-by: Suzuki K Poulose 
Signed-off-by: Zenghui Yu 
---
 virt/kvm/arm/mmu.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
index e3b9ee268823..557f36866d1c 100644
--- a/virt/kvm/arm/mmu.c
+++ b/virt/kvm/arm/mmu.c
@@ -1607,6 +1607,10 @@ static bool fault_supports_stage2_huge_mapping(struct 
kvm_memory_slot *memslot,
hva_t uaddr_start, uaddr_end;
size_t size;
 
+   /* The memslot and the VMA are guaranteed to be aligned to PAGE_SIZE */
+   if (map_size == PAGE_SIZE)
+   return true;
+
size = memslot->npages * PAGE_SIZE;
 
gpa_start = memslot->base_gfn << PAGE_SHIFT;
@@ -1626,7 +1630,7 @@ static bool fault_supports_stage2_huge_mapping(struct 
kvm_memory_slot *memslot,
 *|abcde|fgh  Stage-1 block  |Stage-1 block tv|xyz|
 *+-+++---+
 *
-*memslot->base_gfn << PAGE_SIZE:
+*memslot->base_gfn << PAGE_SHIFT:
 *  +---+++-+
 *  |abc|def  Stage-2 block  |Stage-2 block   |tvxyz|
 *  +---+++-+
-- 
2.19.1


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


[PATCH resend 0/2] KVM: arm64: Unify stage2 mapping for THP backed memory

2020-05-07 Thread Zenghui Yu
This series was originally posted by Suzuki K Poulose a year ago [*],
with the aim of cleaning up the handling of the stage2 huge mapping for
THP. I think this still helps to make the current code cleaner, so
rebase it on top of kvmarm/master and repost it for acceptance.

Thanks!

[*] 
https://lore.kernel.org/kvm/1554909832-7169-1-git-send-email-suzuki.poul...@arm.com/

Suzuki K Poulose (2):
  KVM: arm64: Clean up the checking for huge mapping
  KVM: arm64: Unify handling THP backed host memory

 virt/kvm/arm/mmu.c | 121 -
 1 file changed, 65 insertions(+), 56 deletions(-)

-- 
2.19.1


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


[PATCH resend 2/2] KVM: arm64: Unify handling THP backed host memory

2020-05-07 Thread Zenghui Yu
From: Suzuki K Poulose 

We support mapping host memory backed by PMD transparent hugepages
at stage2 as huge pages. However the checks are now spread across
two different places. Let us unify the handling of the THPs to
keep the code cleaner (and future proof for PUD THP support).
This patch moves transparent_hugepage_adjust() closer to the caller
to avoid a forward declaration for fault_supports_stage2_huge_mappings().

Also, since we already handle the case where the host VA and the guest
PA may not be aligned, the explicit VM_BUG_ON() is not required.

Cc: Marc Zyngier 
Cc: Christoffer Dall 
Signed-off-by: Suzuki K Poulose 
Signed-off-by: Zenghui Yu 
---
 virt/kvm/arm/mmu.c | 115 +++--
 1 file changed, 60 insertions(+), 55 deletions(-)

diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
index 557f36866d1c..93a770fd2b5e 100644
--- a/virt/kvm/arm/mmu.c
+++ b/virt/kvm/arm/mmu.c
@@ -1372,47 +1372,6 @@ int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t 
guest_ipa,
return ret;
 }
 
-static bool transparent_hugepage_adjust(kvm_pfn_t *pfnp, phys_addr_t *ipap)
-{
-   kvm_pfn_t pfn = *pfnp;
-   gfn_t gfn = *ipap >> PAGE_SHIFT;
-
-   if (kvm_is_transparent_hugepage(pfn)) {
-   unsigned long mask;
-   /*
-* The address we faulted on is backed by a transparent huge
-* page.  However, because we map the compound huge page and
-* not the individual tail page, we need to transfer the
-* refcount to the head page.  We have to be careful that the
-* THP doesn't start to split while we are adjusting the
-* refcounts.
-*
-* We are sure this doesn't happen, because mmu_notifier_retry
-* was successful and we are holding the mmu_lock, so if this
-* THP is trying to split, it will be blocked in the mmu
-* notifier before touching any of the pages, specifically
-* before being able to call __split_huge_page_refcount().
-*
-* We can therefore safely transfer the refcount from PG_tail
-* to PG_head and switch the pfn from a tail page to the head
-* page accordingly.
-*/
-   mask = PTRS_PER_PMD - 1;
-   VM_BUG_ON((gfn & mask) != (pfn & mask));
-   if (pfn & mask) {
-   *ipap &= PMD_MASK;
-   kvm_release_pfn_clean(pfn);
-   pfn &= ~mask;
-   kvm_get_pfn(pfn);
-   *pfnp = pfn;
-   }
-
-   return true;
-   }
-
-   return false;
-}
-
 /**
  * stage2_wp_ptes - write protect PMD range
  * @pmd:   pointer to pmd entry
@@ -1660,6 +1619,59 @@ static bool fault_supports_stage2_huge_mapping(struct 
kvm_memory_slot *memslot,
   (hva & ~(map_size - 1)) + map_size <= uaddr_end;
 }
 
+/*
+ * Check if the given hva is backed by a transparent huge page (THP) and
+ * whether it can be mapped using block mapping in stage2. If so, adjust
+ * the stage2 PFN and IPA accordingly. Only PMD_SIZE THPs are currently
+ * supported. This will need to be updated to support other THP sizes.
+ *
+ * Returns the size of the mapping.
+ */
+static unsigned long
+transparent_hugepage_adjust(struct kvm_memory_slot *memslot,
+   unsigned long hva, kvm_pfn_t *pfnp,
+   phys_addr_t *ipap)
+{
+   kvm_pfn_t pfn = *pfnp;
+
+   /*
+* Make sure the adjustment is done only for THP pages. Also make
+* sure that the HVA and IPA are sufficiently aligned and that the
+* block map is contained within the memslot.
+*/
+   if (kvm_is_transparent_hugepage(pfn) &&
+   fault_supports_stage2_huge_mapping(memslot, hva, PMD_SIZE)) {
+   /*
+* The address we faulted on is backed by a transparent huge
+* page.  However, because we map the compound huge page and
+* not the individual tail page, we need to transfer the
+* refcount to the head page.  We have to be careful that the
+* THP doesn't start to split while we are adjusting the
+* refcounts.
+*
+* We are sure this doesn't happen, because mmu_notifier_retry
+* was successful and we are holding the mmu_lock, so if this
+* THP is trying to split, it will be blocked in the mmu
+* notifier before touching any of the pages, specifically
+* before being able to call __split_huge_page_refcount().
+*
+* We can therefore safely transfer the refcount from PG_tail
+* to PG_head and switch the pfn from a tail page to the head
+* page accordingly.
+ 

Re: [PATCH v4 02/14] arm: add support for folded p4d page tables

2020-05-07 Thread Marek Szyprowski
Hi

On 14.04.2020 17:34, Mike Rapoport wrote:
> From: Mike Rapoport 
>
> Implement primitives necessary for the 4th level folding, add walks of p4d
> level where appropriate, and remove __ARCH_USE_5LEVEL_HACK.
>
> Signed-off-by: Mike Rapoport 

Today I've noticed that kexec is broken on ARM 32bit. Bisecting between 
current linux-next and v5.7-rc1 pointed to this commit. I've tested this 
on Odroid XU4 and Raspberry Pi4 boards. Here is the relevant log:

# kexec --kexec-syscall -l zImage --append "$(cat /proc/cmdline)"
memory_range[0]:0x4000..0xbe9f
memory_range[0]:0x4000..0xbe9f
# kexec -e
kexec_core: Starting new kernel
8<--- cut here ---
Unable to handle kernel paging request at virtual address c010f1f4
pgd = c6817793
[c010f1f4] *pgd=441e(bad)
Internal error: Oops: 80d [#1] PREEMPT ARM
Modules linked in:
CPU: 0 PID: 1329 Comm: kexec Tainted: G    W 
5.7.0-rc3-00127-g6cba81ed0f62 #611
Hardware name: Samsung Exynos (Flattened Device Tree)
PC is at machine_kexec+0x40/0xfc
LR is at 0x
pc : []    lr : []    psr: 6013
sp : ebc13e60  ip : 40008000  fp : 0001
r10: 0058  r9 : fee1dead  r8 : 0001
r7 : c121387c  r6 : 6c224000  r5 : ece40c00  r4 : ec222000
r3 : c010f1f4  r2 : c110  r1 : c110  r0 : 418d
Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
Control: 10c5387d  Table: 6bc14059  DAC: 0051
Process kexec (pid: 1329, stack limit = 0x366bb4dc)
Stack: (0xebc13e60 to 0xebc14000)
...
[] (machine_kexec) from [] (kernel_kexec+0x74/0x7c)
[] (kernel_kexec) from [] (__do_sys_reboot+0x1f8/0x210)
[] (__do_sys_reboot) from [] (ret_fast_syscall+0x0/0x28)
Exception stack(0xebc13fa8 to 0xebc13ff0)
...
---[ end trace 3e8d6c81723c778d ]---
1329 Segmentation fault  ./kexec -e

> ---
>   arch/arm/include/asm/pgtable.h |  1 -
>   arch/arm/lib/uaccess_with_memcpy.c |  7 +-
>   arch/arm/mach-sa1100/assabet.c |  2 +-
>   arch/arm/mm/dump.c | 29 +-
>   arch/arm/mm/fault-armv.c   |  7 +-
>   arch/arm/mm/fault.c| 22 ++--
>   arch/arm/mm/idmap.c|  3 ++-
>   arch/arm/mm/init.c |  2 +-
>   arch/arm/mm/ioremap.c  | 12 ++---
>   arch/arm/mm/mm.h   |  2 +-
>   arch/arm/mm/mmu.c  | 35 +-
>   arch/arm/mm/pgd.c  | 40 --
>   12 files changed, 125 insertions(+), 37 deletions(-)
>
> ...

Best regards
-- 
Marek Szyprowski, PhD
Samsung R Institute Poland

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


Re: [PATCH 01/26] KVM: arm64: Check advertised Stage-2 page size capability

2020-05-07 Thread Alexandru Elisei
Hi,

On 4/22/20 1:00 PM, Marc Zyngier wrote:
> With ARMv8.5-GTG, the hardware (or more likely a hypervisor) can
> advertise the supported Stage-2 page sizes.
>
> Let's check this at boot time.
>
> Signed-off-by: Marc Zyngier 
> ---
>  arch/arm64/include/asm/kvm_host.h |  2 +-
>  arch/arm64/include/asm/sysreg.h   |  3 +++
>  arch/arm64/kernel/cpufeature.c|  8 +++
>  arch/arm64/kvm/reset.c| 40 ---
>  virt/kvm/arm/arm.c|  4 +---
>  5 files changed, 50 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h 
> b/arch/arm64/include/asm/kvm_host.h
> index 32c8a675e5a4a..7dd8fefa6aecd 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -670,7 +670,7 @@ static inline int kvm_arm_have_ssbd(void)
>  void kvm_vcpu_load_sysregs(struct kvm_vcpu *vcpu);
>  void kvm_vcpu_put_sysregs(struct kvm_vcpu *vcpu);
>  
> -void kvm_set_ipa_limit(void);
> +int kvm_set_ipa_limit(void);
>  
>  #define __KVM_HAVE_ARCH_VM_ALLOC
>  struct kvm *kvm_arch_alloc_vm(void);
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index ebc6224328318..5d10c9148e844 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -686,6 +686,9 @@
>  #define ID_AA64ZFR0_SVEVER_SVE2  0x1
>  
>  /* id_aa64mmfr0 */
> +#define ID_AA64MMFR0_TGRAN4_2_SHIFT  40
> +#define ID_AA64MMFR0_TGRAN64_2_SHIFT 36
> +#define ID_AA64MMFR0_TGRAN16_2_SHIFT 32
>  #define ID_AA64MMFR0_TGRAN4_SHIFT28
>  #define ID_AA64MMFR0_TGRAN64_SHIFT   24
>  #define ID_AA64MMFR0_TGRAN16_SHIFT   20
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 9fac745aa7bb2..9892a845d06c9 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -208,6 +208,14 @@ static const struct arm64_ftr_bits ftr_id_aa64zfr0[] = {
>  };
>  
>  static const struct arm64_ftr_bits ftr_id_aa64mmfr0[] = {
> + /*
> +  * Page size not being supported at Stage-2 are not fatal. You

s/are not fatal/is not fatal

> +  * just give up KVM if PAGE_SIZE isn't supported there. Go fix
> +  * your favourite nesting hypervisor.
> +  */
> + ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_EXACT, 
> ID_AA64MMFR0_TGRAN4_2_SHIFT, 4, 1),
> + ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_EXACT, 
> ID_AA64MMFR0_TGRAN64_2_SHIFT, 4, 1),
> + ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_EXACT, 
> ID_AA64MMFR0_TGRAN16_2_SHIFT, 4, 1),
>   /*
>* We already refuse to boot CPUs that don't support our configured
>* page size, so we can only detect mismatches for a page size other
> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> index 30b7ea680f66c..241db35a7ef4f 100644
> --- a/arch/arm64/kvm/reset.c
> +++ b/arch/arm64/kvm/reset.c
> @@ -9,6 +9,7 @@
>   */
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -340,11 +341,42 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
>   return ret;
>  }
>  
> -void kvm_set_ipa_limit(void)
> +int kvm_set_ipa_limit(void)
>  {
> - unsigned int ipa_max, pa_max, va_max, parange;
> + unsigned int ipa_max, pa_max, va_max, parange, tgran_2;
> + u64 mmfr0 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1);
>  
> - parange = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1) & 0x7;
> + /*
> +  * Check with ARMv8.5-GTG that our PAGE_SIZE is supported at
> +  * Stage-2. If not, things will stop very quickly.
> +  */
> + switch (PAGE_SIZE) {
> + default:
> + case SZ_4K:
> + tgran_2 = ID_AA64MMFR0_TGRAN4_2_SHIFT;
> + break;
> + case SZ_16K:
> + tgran_2 = ID_AA64MMFR0_TGRAN16_2_SHIFT;
> + break;
> + case SZ_64K:
> + tgran_2 = ID_AA64MMFR0_TGRAN64_2_SHIFT;
> + break;
> + }
> +
> + switch (FIELD_GET(0xFUL << tgran_2, mmfr0)) {
> + default:
> + case 1:
> + kvm_err("PAGE_SIZE not supported at Stage-2, giving up\n");
> + return -EINVAL;
> + case 0:
> + kvm_debug("PAGE_SIZE supported at Stage-2 (default)\n");
> + break;
> + case 2:
> + kvm_debug("PAGE_SIZE supported at Stage-2 (advertised)\n");
> + break;
> + }
> +
> + parange = mmfr0 & 0x7;
>   pa_max = id_aa64mmfr0_parange_to_phys_shift(parange);
>  
>   /* Clamp the IPA limit to the PA size supported by the kernel */
> @@ -378,6 +410,8 @@ void kvm_set_ipa_limit(void)
>"KVM IPA limit (%d bit) is smaller than default size\n", ipa_max);
>   kvm_ipa_limit = ipa_max;
>   kvm_info("IPA Size Limit: %dbits\n", kvm_ipa_limit);
> +
> + return 0;
>  }
>  
>  /*
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index 48d0ec44ad77e..53b3ba9173ba7 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -1387,9 +1387,7 @@ static inline void hyp_cpu_pm_exit(void)
>  
>  

Re: [PATCH] KVM: arm/arm64: release kvm->mmu_lock in loop to prevent starvation

2020-05-07 Thread Suzuki K Poulose

On 04/15/2020 09:42 AM, Jiang Yi wrote:

Do cond_resched_lock() in stage2_flush_memslot() like what is done in
unmap_stage2_range() and other places holding mmu_lock while processing
a possibly large range of memory.

Signed-off-by: Jiang Yi 
---
  virt/kvm/arm/mmu.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
index e3b9ee268823..7315af2c52f8 100644
--- a/virt/kvm/arm/mmu.c
+++ b/virt/kvm/arm/mmu.c
@@ -417,16 +417,19 @@ static void stage2_flush_memslot(struct kvm *kvm,
phys_addr_t next;
pgd_t *pgd;
  
  	pgd = kvm->arch.pgd + stage2_pgd_index(kvm, addr);

do {
next = stage2_pgd_addr_end(kvm, addr, end);
if (!stage2_pgd_none(kvm, *pgd))
stage2_flush_puds(kvm, pgd, addr, next);
+
+   if (next != end)
+   cond_resched_lock(>mmu_lock);
} while (pgd++, addr = next, addr != end);
  }


Given that this is called under the srcu_lock this looks
good to me:

Reviewed-by: Suzuki K Poulose 

  
  /**

   * stage2_flush_vm - Invalidate cache for pages mapped in stage 2
   * @kvm: The struct kvm pointer
   *
   * Go through the stage 2 page tables and invalidate any cache lines



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


Re: [PATCH] arm64/cpufeature: Verify KVM capabilities during CPU hotplug

2020-05-07 Thread Marc Zyngier
On Thu,  7 May 2020 11:49:47 +0530
Anshuman Khandual  wrote:

Hi Anshuman,

> This validates KVM capabilities like VMID width, IPA range for hotplug CPU
> against system finalized values. While here, it factors out get_vmid_bits()
> for general use and also defines ID_AA64MMFR0_PARANGE_MASK.

nit: these are not KVM-specific capabilities, but general
virtualization features.

> 
> Cc: Catalin Marinas 
> Cc: Will Deacon 
> Cc: Marc Zyngier 
> Cc: Mark Rutland 
> Cc: James Morse 
> Cc: Suzuki K Poulose 
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: kvmarm@lists.cs.columbia.edu
> Cc: linux-ker...@vger.kernel.org
> 
> Suggested-by: Suzuki Poulose 
> Signed-off-by: Anshuman Khandual 
> ---
>  arch/arm64/include/asm/cpufeature.h | 22 +++
>  arch/arm64/include/asm/kvm_mmu.h|  2 +-
>  arch/arm64/include/asm/sysreg.h |  1 +
>  arch/arm64/kernel/cpufeature.c  |  2 ++
>  arch/arm64/kvm/reset.c  | 33 +++--
>  5 files changed, 57 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/cpufeature.h 
> b/arch/arm64/include/asm/cpufeature.h
> index afe08251ff95..6808a2091de4 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -745,6 +745,28 @@ static inline bool cpu_has_hw_af(void)
>  extern bool cpu_has_amu_feat(int cpu);
>  #endif
>  
> +static inline unsigned int get_vmid_bits(u64 mmfr1)
> +{
> + int vmid_bits;
> +
> + vmid_bits = cpuid_feature_extract_unsigned_field(mmfr1,
> + ID_AA64MMFR1_VMIDBITS_SHIFT);
> + if (vmid_bits == ID_AA64MMFR1_VMIDBITS_16)
> + return 16;
> +
> + /*
> +  * Return the default here even if any reserved
> +  * value is fetched from the system register.
> +  */
> + return 8;
> +}
> +
> +#ifdef CONFIG_KVM_ARM_HOST
> +void verify_kvm_capabilities(void);
> +#else
> +static inline void verify_kvm_capabilities(void) { }
> +#endif
> +
>  #endif /* __ASSEMBLY__ */
>  
>  #endif
> diff --git a/arch/arm64/include/asm/kvm_mmu.h 
> b/arch/arm64/include/asm/kvm_mmu.h
> index 30b0e8d6b895..a7137e144b97 100644
> --- a/arch/arm64/include/asm/kvm_mmu.h
> +++ b/arch/arm64/include/asm/kvm_mmu.h
> @@ -416,7 +416,7 @@ static inline unsigned int kvm_get_vmid_bits(void)
>  {
>   int reg = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);
>  
> - return (cpuid_feature_extract_unsigned_field(reg,
> ID_AA64MMFR1_VMIDBITS_SHIFT) == 2) ? 16 : 8;
> + return get_vmid_bits(reg);
>  }
>  
>  /*
> diff --git a/arch/arm64/include/asm/sysreg.h
> b/arch/arm64/include/asm/sysreg.h index c4ac0ac25a00..3510a4668970
> 100644 --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -705,6 +705,7 @@
>  #define ID_AA64MMFR0_TGRAN16_SUPPORTED   0x1
>  #define ID_AA64MMFR0_PARANGE_48  0x5
>  #define ID_AA64MMFR0_PARANGE_52  0x6
> +#define ID_AA64MMFR0_PARANGE_MASK0x7
>  
>  #ifdef CONFIG_ARM64_PA_BITS_52
>  #define ID_AA64MMFR0_PARANGE_MAX ID_AA64MMFR0_PARANGE_52
> diff --git a/arch/arm64/kernel/cpufeature.c
> b/arch/arm64/kernel/cpufeature.c index 9fac745aa7bb..041dd610b0f8
> 100644 --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -2206,6 +2206,8 @@ static void verify_local_cpu_capabilities(void)
>  
>   if (system_supports_sve())
>   verify_sve_features();
> +
> + verify_kvm_capabilities();

You should only check this if booted at EL2. Otherwise, it doesn't
really matter.

>  }
>  
>  void check_local_cpu_capabilities(void)
> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> index 30b7ea680f66..1eebcc2a8396 100644
> --- a/arch/arm64/kvm/reset.c
> +++ b/arch/arm64/kvm/reset.c
> @@ -340,11 +340,39 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
>   return ret;
>  }
>  
> +void verify_kvm_capabilities(void)

This is really in the wrong file. reset.c is supposed to contain things
that are meaningful to the guest reset state. This clearly isn't. I'd
suggest you add an accessor for the kvm_ipa_limit variable, and keep
the function next to the other verify_* functions in cpufeature.c.

> +{
> + u64 safe_mmfr1, mmfr0, mmfr1;
> + int parange, ipa_max;
> + unsigned int safe_vmid_bits, vmid_bits;
> +
> + safe_mmfr1 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);
> + mmfr0 = read_cpuid(ID_AA64MMFR0_EL1);
> + mmfr1 = read_cpuid(ID_AA64MMFR1_EL1);
> +
> + /* Verify VMID bits */
> + safe_vmid_bits = get_vmid_bits(safe_mmfr1);
> + vmid_bits = get_vmid_bits(mmfr1);
> + if (vmid_bits < safe_vmid_bits) {
> + pr_crit("CPU%d: VMID width mismatch\n",
> smp_processor_id());
> + cpu_die_early();
> + }
> +
> + /* Verify IPA range */
> + parange = mmfr0 & ID_AA64MMFR0_PARANGE_MASK;
> + ipa_max = id_aa64mmfr0_parange_to_phys_shift(parange);
> + if (ipa_max < kvm_ipa_limit) {
> + pr_crit("CPU%d: IPA range mismatch\n",
> 

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

2020-05-07 Thread Auger Eric
Hi Shameer,

On 5/7/20 8:59 AM, Shameerali Kolothum Thodi wrote:
> Hi Eric,
> 
>> -Original Message-
>> From: Shameerali Kolothum Thodi
>> Sent: 30 April 2020 10:38
>> To: 'Auger Eric' ; 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; 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 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 have added STALL support for our vSVA prototype and it seems to be
> working(on our hardware). I have updated the kernel and qemu branches with
> the same[1][2]. I should warn you though that these are prototype code and I 
> am pretty
> much re-using the VFIO_IOMMU_SET_PASID_TABLE interface for almost everything.
> But thought of sharing, in case if it is useful somehow!.

Thank you very much for your work. I intend to look at your additions by
beginning of next week.

Best Regards

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

___
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-05-07 Thread Shameerali Kolothum Thodi
Hi Eric,

> -Original Message-
> From: Shameerali Kolothum Thodi
> Sent: 30 April 2020 10:38
> To: 'Auger Eric' ; 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; 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 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 have added STALL support for our vSVA prototype and it seems to be
working(on our hardware). I have updated the kernel and qemu branches with
the same[1][2]. I should warn you though that these are prototype code and I am 
pretty
much re-using the VFIO_IOMMU_SET_PASID_TABLE interface for almost everything.
But thought of sharing, in case if it is useful somehow!.

Thanks,
Shameer

[1]https://github.com/hisilicon/kernel-dev/commits/vsva-prototype-host-v1

[2]https://github.com/hisilicon/qemu/tree/v4.2.0-2stage-rfcv6-vsva-prototype-v1
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH] arm64/cpufeature: Verify KVM capabilities during CPU hotplug

2020-05-07 Thread Anshuman Khandual
This validates KVM capabilities like VMID width, IPA range for hotplug CPU
against system finalized values. While here, it factors out get_vmid_bits()
for general use and also defines ID_AA64MMFR0_PARANGE_MASK.

Cc: Catalin Marinas 
Cc: Will Deacon 
Cc: Marc Zyngier 
Cc: Mark Rutland 
Cc: James Morse 
Cc: Suzuki K Poulose 
Cc: linux-arm-ker...@lists.infradead.org
Cc: kvmarm@lists.cs.columbia.edu
Cc: linux-ker...@vger.kernel.org

Suggested-by: Suzuki Poulose 
Signed-off-by: Anshuman Khandual 
---
 arch/arm64/include/asm/cpufeature.h | 22 +++
 arch/arm64/include/asm/kvm_mmu.h|  2 +-
 arch/arm64/include/asm/sysreg.h |  1 +
 arch/arm64/kernel/cpufeature.c  |  2 ++
 arch/arm64/kvm/reset.c  | 33 +++--
 5 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/cpufeature.h 
b/arch/arm64/include/asm/cpufeature.h
index afe08251ff95..6808a2091de4 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -745,6 +745,28 @@ static inline bool cpu_has_hw_af(void)
 extern bool cpu_has_amu_feat(int cpu);
 #endif
 
+static inline unsigned int get_vmid_bits(u64 mmfr1)
+{
+   int vmid_bits;
+
+   vmid_bits = cpuid_feature_extract_unsigned_field(mmfr1,
+   ID_AA64MMFR1_VMIDBITS_SHIFT);
+   if (vmid_bits == ID_AA64MMFR1_VMIDBITS_16)
+   return 16;
+
+   /*
+* Return the default here even if any reserved
+* value is fetched from the system register.
+*/
+   return 8;
+}
+
+#ifdef CONFIG_KVM_ARM_HOST
+void verify_kvm_capabilities(void);
+#else
+static inline void verify_kvm_capabilities(void) { }
+#endif
+
 #endif /* __ASSEMBLY__ */
 
 #endif
diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
index 30b0e8d6b895..a7137e144b97 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -416,7 +416,7 @@ static inline unsigned int kvm_get_vmid_bits(void)
 {
int reg = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);
 
-   return (cpuid_feature_extract_unsigned_field(reg, 
ID_AA64MMFR1_VMIDBITS_SHIFT) == 2) ? 16 : 8;
+   return get_vmid_bits(reg);
 }
 
 /*
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index c4ac0ac25a00..3510a4668970 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -705,6 +705,7 @@
 #define ID_AA64MMFR0_TGRAN16_SUPPORTED 0x1
 #define ID_AA64MMFR0_PARANGE_480x5
 #define ID_AA64MMFR0_PARANGE_520x6
+#define ID_AA64MMFR0_PARANGE_MASK  0x7
 
 #ifdef CONFIG_ARM64_PA_BITS_52
 #define ID_AA64MMFR0_PARANGE_MAX   ID_AA64MMFR0_PARANGE_52
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 9fac745aa7bb..041dd610b0f8 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -2206,6 +2206,8 @@ static void verify_local_cpu_capabilities(void)
 
if (system_supports_sve())
verify_sve_features();
+
+   verify_kvm_capabilities();
 }
 
 void check_local_cpu_capabilities(void)
diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index 30b7ea680f66..1eebcc2a8396 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -340,11 +340,39 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
return ret;
 }
 
+void verify_kvm_capabilities(void)
+{
+   u64 safe_mmfr1, mmfr0, mmfr1;
+   int parange, ipa_max;
+   unsigned int safe_vmid_bits, vmid_bits;
+
+   safe_mmfr1 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);
+   mmfr0 = read_cpuid(ID_AA64MMFR0_EL1);
+   mmfr1 = read_cpuid(ID_AA64MMFR1_EL1);
+
+   /* Verify VMID bits */
+   safe_vmid_bits = get_vmid_bits(safe_mmfr1);
+   vmid_bits = get_vmid_bits(mmfr1);
+   if (vmid_bits < safe_vmid_bits) {
+   pr_crit("CPU%d: VMID width mismatch\n", smp_processor_id());
+   cpu_die_early();
+   }
+
+   /* Verify IPA range */
+   parange = mmfr0 & ID_AA64MMFR0_PARANGE_MASK;
+   ipa_max = id_aa64mmfr0_parange_to_phys_shift(parange);
+   if (ipa_max < kvm_ipa_limit) {
+   pr_crit("CPU%d: IPA range mismatch\n", smp_processor_id());
+   cpu_die_early();
+   }
+}
+
 void kvm_set_ipa_limit(void)
 {
unsigned int ipa_max, pa_max, va_max, parange;
 
-   parange = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1) & 0x7;
+   parange = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1) &
+   ID_AA64MMFR0_PARANGE_MASK;
pa_max = id_aa64mmfr0_parange_to_phys_shift(parange);
 
/* Clamp the IPA limit to the PA size supported by the kernel */
@@ -406,7 +434,8 @@ int kvm_arm_setup_stage2(struct kvm *kvm, unsigned long 
type)
phys_shift = KVM_PHYS_SHIFT;
}
 
-   parange = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1) & 7;
+