Re: [PATCH v4 01/18] arm64: stacktrace: Add shared header for common stack unwinding code
On Mon, Jul 18, 2022 at 08:26:14AM -0700, Kalesh Singh wrote: > On Mon, Jul 18, 2022 at 5:52 AM Russell King (Oracle) > wrote: > > > > Hi, > > > > Can you please explain why you are targetting my @oracle.com email > > address with this patch set? > > > > This causes me problems as I use Outlook's Web interface for that > > which doesn't appear to cope with the threading, and most certainly > > is only capable of top-reply only which is against Linux kernel email > > standards. > > Hi Russell, > > Sorry I wasn't aware of it (I got your oracle email from > get_maintainer script). Going forward I'll use the one you responded > from instead. Oh, this is the very annoying behaviour of get_maintainer.pl default mode to think that if someone touches a file, they're interested in future changes to it. In this case, it's because we both touched arch/arm64/include/asm/memory.h back in November 2021, and this silly script thinks I'll still be interested. b89ddf4cca43 arm64/bpf: Remove 128MB limit for BPF JIT programs (The patch was originally developed for Oracle's UEK kernels, hence why it's got my @oracle.com address, but was later merged upstream. Interestingly, no one spotted that Alan Maguire's s-o-b should've been on it, as he was involved in the submission path to mainline.) -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v4 01/18] arm64: stacktrace: Add shared header for common stack unwinding code
Hi,
Can you please explain why you are targetting my @oracle.com email
address with this patch set?
This causes me problems as I use Outlook's Web interface for that
which doesn't appear to cope with the threading, and most certainly
is only capable of top-reply only which is against Linux kernel email
standards.
Thanks.
On Thu, Jul 14, 2022 at 11:10:10PM -0700, Kalesh Singh wrote:
> In order to reuse the arm64 stack unwinding logic for the nVHE
> hypervisor stack, move the common code to a shared header
> (arch/arm64/include/asm/stacktrace/common.h).
>
> The nVHE hypervisor cannot safely link against kernel code, so we
> make use of the shared header to avoid duplicated logic later in
> this series.
>
> Signed-off-by: Kalesh Singh
> ---
> arch/arm64/include/asm/stacktrace.h| 35 +--
> arch/arm64/include/asm/stacktrace/common.h | 105 +
> arch/arm64/kernel/stacktrace.c | 57 ---
> 3 files changed, 106 insertions(+), 91 deletions(-)
> create mode 100644 arch/arm64/include/asm/stacktrace/common.h
>
> diff --git a/arch/arm64/include/asm/stacktrace.h
> b/arch/arm64/include/asm/stacktrace.h
> index aec9315bf156..79f455b37c84 100644
> --- a/arch/arm64/include/asm/stacktrace.h
> +++ b/arch/arm64/include/asm/stacktrace.h
> @@ -8,52 +8,19 @@
> #include
> #include
> #include
> -#include
> #include
>
> #include
> #include
> #include
>
> -enum stack_type {
> - STACK_TYPE_UNKNOWN,
> - STACK_TYPE_TASK,
> - STACK_TYPE_IRQ,
> - STACK_TYPE_OVERFLOW,
> - STACK_TYPE_SDEI_NORMAL,
> - STACK_TYPE_SDEI_CRITICAL,
> - __NR_STACK_TYPES
> -};
> -
> -struct stack_info {
> - unsigned long low;
> - unsigned long high;
> - enum stack_type type;
> -};
> +#include
>
> extern void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk,
> const char *loglvl);
>
> DECLARE_PER_CPU(unsigned long *, irq_stack_ptr);
>
> -static inline bool on_stack(unsigned long sp, unsigned long size,
> - unsigned long low, unsigned long high,
> - enum stack_type type, struct stack_info *info)
> -{
> - if (!low)
> - return false;
> -
> - if (sp < low || sp + size < sp || sp + size > high)
> - return false;
> -
> - if (info) {
> - info->low = low;
> - info->high = high;
> - info->type = type;
> - }
> - return true;
> -}
> -
> static inline bool on_irq_stack(unsigned long sp, unsigned long size,
> struct stack_info *info)
> {
> diff --git a/arch/arm64/include/asm/stacktrace/common.h
> b/arch/arm64/include/asm/stacktrace/common.h
> new file mode 100644
> index ..64ae4f6b06fe
> --- /dev/null
> +++ b/arch/arm64/include/asm/stacktrace/common.h
> @@ -0,0 +1,105 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Common arm64 stack unwinder code.
> + *
> + * Copyright (C) 2012 ARM Ltd.
> + */
> +#ifndef __ASM_STACKTRACE_COMMON_H
> +#define __ASM_STACKTRACE_COMMON_H
> +
> +#include
> +#include
> +#include
> +
> +enum stack_type {
> + STACK_TYPE_UNKNOWN,
> + STACK_TYPE_TASK,
> + STACK_TYPE_IRQ,
> + STACK_TYPE_OVERFLOW,
> + STACK_TYPE_SDEI_NORMAL,
> + STACK_TYPE_SDEI_CRITICAL,
> + __NR_STACK_TYPES
> +};
> +
> +struct stack_info {
> + unsigned long low;
> + unsigned long high;
> + enum stack_type type;
> +};
> +
> +/*
> + * A snapshot of a frame record or fp/lr register values, along with some
> + * accounting information necessary for robust unwinding.
> + *
> + * @fp: The fp value in the frame record (or the real fp)
> + * @pc: The lr value in the frame record (or the real lr)
> + *
> + * @stacks_done: Stacks which have been entirely unwound, for which it is no
> + * longer valid to unwind to.
> + *
> + * @prev_fp: The fp that pointed to this frame record, or a synthetic
> value
> + * of 0. This is used to ensure that within a stack, each
> + * subsequent frame record is at an increasing address.
> + * @prev_type: The type of stack this frame record was on, or a synthetic
> + * value of STACK_TYPE_UNKNOWN. This is used to detect a
> + * transition from one stack to another.
> + *
> + * @kr_cur: When KRETPROBES is selected, holds the kretprobe instance
> + * associated with the most recently encountered replacement lr
> + * value.
> + *
> + * @task:The task being unwound.
> + */
> +struct unwind_state {
> + unsigned long fp;
> + unsigned long pc;
> + DECLARE_BITMAP(stacks_done, __NR_STACK_TYPES);
> + unsigned long prev_fp;
> + enum stack_type prev_type;
> +#ifdef CONFIG_KRETPROBES
> + struct llist_node *kr_cur;
> +#endif
> + struct task_struct *task;
> +};
> +
> +static inline bool on_stack(unsigned long sp, unsigned long size,
Re: [PATCH v6 19/64] KVM: arm64: nv: Trap SPSR_EL1, ELR_EL1 and VBAR_EL1 from virtual EL2
On Fri, Jan 28, 2022 at 12:18:27PM +, Marc Zyngier wrote: > From: Jintack Lim > > For the same reason we trap virtual memory register accesses at virtual > EL2, we need to trap SPSR_EL1, ELR_EL1 and VBAR_EL1 accesses. ARM v8.3 > introduces the HCR_EL2.NV1 bit to be able to trap on those register > accesses in EL1. Do not set this bit until the whole nesting support is Maybe: , but will be done in a future patch once nested support is complete. > completed. > > Signed-off-by: Jintack Lim > Signed-off-by: Marc Zyngier Reviewed-by: Russell King (Oracle) -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v6 18/64] KVM: arm64: nv: Trap EL1 VM register accesses in virtual EL2
On Fri, Jan 28, 2022 at 12:18:26PM +, Marc Zyngier wrote: > From: Christoffer Dall > > When running in virtual EL2 mode, we actually run the hardware in EL1 > and therefore have to use the EL1 registers to ensure correct operation. > > By setting the HCR.TVM and HCR.TVRM we ensure that the virtual EL2 mode > doesn't shoot itself in the foot when setting up what it believes to be > a different mode's system register state (for example when preparing to > switch to a VM). > > We can leverage the existing sysregs infrastructure to support trapped > accesses to these registers. > > Signed-off-by: Christoffer Dall > Signed-off-by: Marc Zyngier Reviewed-by: Russell King (Oracle) -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v6 17/64] KVM: arm64: nv: Emulate PSTATE.M for a guest hypervisor
On Fri, Jan 28, 2022 at 12:18:25PM +, Marc Zyngier wrote:
> From: Christoffer Dall
>
> We can no longer blindly copy the VCPU's PSTATE into SPSR_EL2 and return
> to the guest and vice versa when taking an exception to the hypervisor,
> because we emulate virtual EL2 in EL1 and therefore have to translate
> the mode field from EL2 to EL1 and vice versa.
>
> This requires keeping track of the state we enter the guest, for which
> we transiently use a dedicated flag.
>
> Signed-off-by: Marc Zyngier
> ---
> arch/arm64/include/asm/kvm_host.h | 1 +
> arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 19 -
> arch/arm64/kvm/hyp/vhe/switch.c| 24 ++
> 3 files changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h
> b/arch/arm64/include/asm/kvm_host.h
> index 8fffe2888403..fa253f08e0fd 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -472,6 +472,7 @@ struct kvm_vcpu_arch {
> #define KVM_ARM64_DEBUG_STATE_SAVE_SPE (1 << 12) /* Save SPE context
> if active */
> #define KVM_ARM64_DEBUG_STATE_SAVE_TRBE (1 << 13) /* Save TRBE context
> if active */
> #define KVM_ARM64_FP_FOREIGN_FPSTATE (1 << 14)
> +#define KVM_ARM64_IN_HYP_CONTEXT (1 << 15) /* Guest running in HYP
> context */
>
> #define KVM_GUESTDBG_VALID_MASK (KVM_GUESTDBG_ENABLE | \
>KVM_GUESTDBG_USE_SW_BP | \
> diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> index 283f780f5f56..e3689c6ce4cc 100644
> --- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> @@ -157,9 +157,26 @@ static inline void __sysreg_restore_el1_state(struct
> kvm_cpu_context *ctxt,
> write_sysreg_el1(ctxt_sys_reg(ctxt, SPSR_EL1), SYS_SPSR);
> }
>
> +/* Read the VCPU state's PSTATE, but translate (v)EL2 to EL1. */
> +static inline u64 to_hw_pstate(const struct kvm_cpu_context *ctxt)
> +{
> + u64 mode = ctxt->regs.pstate & (PSR_MODE_MASK | PSR_MODE32_BIT);
> +
> + switch (mode) {
> + case PSR_MODE_EL2t:
> + mode = PSR_MODE_EL1t;
> + break;
> + case PSR_MODE_EL2h:
> + mode = PSR_MODE_EL1h;
> + break;
> + }
> +
> + return (ctxt->regs.pstate & ~(PSR_MODE_MASK | PSR_MODE32_BIT)) | mode;
> +}
> +
Wondering if it makes sense to also have the reverse translation as an
inline function after the above too, so the two translations are
together - but as it's only used (in this patch at least) in switch.c
there probably isn't too much point.
So:
Reviewed-by: Russell King (Oracle)
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v6 15/64] KVM: arm64: nv: Handle HCR_EL2.E2H specially
On Fri, Jan 28, 2022 at 12:18:23PM +, Marc Zyngier wrote: > HCR_EL2.E2H is nasty, as a flip of this bit completely changes the way > we deal with a lot of the state. So when the guest flips this bit > (sysregs are live), do the put/load dance so that we have a consistent > state. > > Yes, this is slow. Don't do it. I'd hope this is very unlikely! > > Suggested-by: Alexandru Elisei > Signed-off-by: Marc Zyngier Reviewed-by: Russell King (Oracle) -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v6 14/64] KVM: arm64: nv: Handle SPSR_EL2 specially
On Fri, Jan 28, 2022 at 12:18:22PM +, Marc Zyngier wrote: > SPSR_EL2 needs special attention when running nested on ARMv8.3: > > If taking an exception while running at vEL2 (actually EL1), the > HW will update the SPSR_EL1 register with the EL1 mode. We need > to track this in order to make sure that accesses to the virtual > view of SPSR_EL2 is correct. > > To do so, we place an illegal value in SPSR_EL1.M, and patch it > accordingly if required when accessing it. > > Reviewed-by: Alexandru Elisei > Signed-off-by: Marc Zyngier Reviewed-by: Russell King (Oracle) -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v6 13/64] KVM: arm64: nv: Handle virtual EL2 registers in vcpu_read/write_sys_reg()
On Fri, Jan 28, 2022 at 12:18:21PM +, Marc Zyngier wrote: > KVM internally uses accessor functions when reading or writing the > guest's system registers. This takes care of accessing either the stored > copy or using the "live" EL1 system registers when the host uses VHE. > > With the introduction of virtual EL2 we add a bunch of EL2 system > registers, which now must also be taken care of: > - If the guest is running in vEL2, and we access an EL1 sysreg, we must > revert to the stored version of that, and not use the CPU's copy. > - If the guest is running in vEL1, and we access an EL2 sysreg, we must > also use the stored version, since the CPU carries the EL1 copy. > - Some EL2 system registers are supposed to affect the current execution > of the system, so we need to put them into their respective EL1 > counterparts. For this we need to define a mapping between the two. > This is done using the newly introduced struct el2_sysreg_map. > - Some EL2 system registers have a different format than their EL1 > counterpart, so we need to translate them before writing them to the > CPU. This is done using an (optional) translate function in the map. > - There are the three special registers SP_EL2, SPSR_EL2 and ELR_EL2, > which need some separate handling (SPSR_EL2 is being handled in a > separate patch). > > All of these cases are now wrapped into the existing accessor functions, > so KVM users wouldn't need to care whether they access EL2 or EL1 > registers and also which state the guest is in. > > This handles what was formerly known as the "shadow state" dynamically, > without requiring a separate copy for each vCPU EL. > > Reviewed-by: Ganapatrao Kulkarni > Reviewed-by: Alexandru Elisei > Co-developed-by: Andre Przywara > Signed-off-by: Andre Przywara > Signed-off-by: Marc Zyngier Reviewed-by: Russell King (Oracle) -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v6 12/64] KVM: arm64: nv: Add non-VHE-EL2->EL1 translation helpers
On Fri, Jan 28, 2022 at 12:18:20PM +, Marc Zyngier wrote:
> Some EL2 system registers immediately affect the current execution
> of the system, so we need to use their respective EL1 counterparts.
> For this we need to define a mapping between the two. In general,
> this only affects non-VHE guest hypervisors, as VHE system registers
> are compatible with the EL1 counterparts.
>
> These helpers will get used in subsequent patches.
>
> Co-developed-by: Andre Przywara
> Signed-off-by: Andre Przywara
> Signed-off-by: Marc Zyngier
Reviewed-by: Russell King (Oracle)
> ---
> arch/arm64/include/asm/kvm_nested.h | 54 +
> 1 file changed, 54 insertions(+)
>
> diff --git a/arch/arm64/include/asm/kvm_nested.h
> b/arch/arm64/include/asm/kvm_nested.h
> index fd601ea68d13..5a85be6d8eb3 100644
> --- a/arch/arm64/include/asm/kvm_nested.h
> +++ b/arch/arm64/include/asm/kvm_nested.h
> @@ -2,6 +2,7 @@
> #ifndef __ARM64_KVM_NESTED_H
> #define __ARM64_KVM_NESTED_H
>
> +#include
> #include
>
> static inline bool vcpu_has_nv(const struct kvm_vcpu *vcpu)
> @@ -11,4 +12,57 @@ static inline bool vcpu_has_nv(const struct kvm_vcpu *vcpu)
> test_bit(KVM_ARM_VCPU_HAS_EL2, vcpu->arch.features));
> }
>
> +/* Translation helpers from non-VHE EL2 to EL1 */
> +static inline u64 tcr_el2_ps_to_tcr_el1_ips(u64 tcr_el2)
> +{
> + return (u64)FIELD_GET(TCR_EL2_PS_MASK, tcr_el2) << TCR_IPS_SHIFT;
I frowned about the use of FIELD_GET() but not FIELD_PREP(), which
would be:
return FIELD_PREP(TCR_IPS_MASK, FIELD_GET(TCR_EL2_PS_MASK, tcr_el2));
However, I'm not bothered by this beyond frowning!
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v6 07/64] KVM: arm64: nv: Handle HCR_EL2.NV system register traps
On Fri, Jan 28, 2022 at 12:18:15PM +, Marc Zyngier wrote: > From: Jintack Lim > > ARM v8.3 introduces a new bit in the HCR_EL2, which is the NV bit. When > this bit is set, accessing EL2 registers in EL1 traps to EL2. In > addition, executing the following instructions in EL1 will trap to EL2: > tlbi, at, eret, and msr/mrs instructions to access SP_EL1. Most of the > instructions that trap to EL2 with the NV bit were undef at EL1 prior to > ARM v8.3. The only instruction that was not undef is eret. > > This patch sets up a handler for EL2 registers and SP_EL1 register > accesses at EL1. The host hypervisor keeps those register values in > memory, and will emulate their behavior. > > This patch doesn't set the NV bit yet. It will be set in a later patch > once nested virtualization support is completed. > > Signed-off-by: Jintack Lim > [maz: EL2_REG() macros] > Signed-off-by: Marc Zyngier Reviewed-by: Russell King (Oracle) Thanks! -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v6 01/64] arm64: Add ARM64_HAS_NESTED_VIRT cpufeature
On Fri, Jan 28, 2022 at 12:18:09PM +, Marc Zyngier wrote: > From: Jintack Lim > > Add a new ARM64_HAS_NESTED_VIRT feature to indicate that the > CPU has the ARMv8.3 nested virtualization capability, together > with the 'kvm-arm.mode=nested' command line option. > > This will be used to support nested virtualization in KVM. > > Signed-off-by: Jintack Lim > Signed-off-by: Andre Przywara > Signed-off-by: Christoffer Dall > [maz: moved the command-line option to kvm-arm.mode] > Signed-off-by: Marc Zyngier Reviewed-by: Russell King (Oracle) -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 16/69] KVM: arm64: nv: Handle trapped ERET from virtual EL2
On Mon, Nov 29, 2021 at 08:00:57PM +, Marc Zyngier wrote: > From: Christoffer Dall > > When a guest hypervisor running virtual EL2 in EL1 executes an ERET > instruction, we will have set HCR_EL2.NV which traps ERET to EL2, so > that we can emulate the exception return in software. > > Signed-off-by: Christoffer Dall > Signed-off-by: Marc Zyngier Reviewed-by: Russell King (Oracle) -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 15/69] KVM: arm64: nv: Inject HVC exceptions to the virtual EL2
On Mon, Nov 29, 2021 at 08:00:56PM +, Marc Zyngier wrote: > From: Jintack Lim > > As we expect all PSCI calls from the L1 hypervisor to be performed > using SMC when nested virtualization is enabled, it is clear that > all HVC instruction from the VM (including from the virtual EL2) > are supposed to handled in the virtual EL2. > > Forward these to EL2 as required. > > Signed-off-by: Jintack Lim > [maz: add handling of HCR_EL2.HCD] > Signed-off-by: Marc Zyngier > --- > arch/arm64/kvm/handle_exit.c | 11 +++ Reviewed-by: Russell King (Oracle) -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 14/69] KVM: arm64: nv: Support virtual EL2 exceptions
On Mon, Nov 29, 2021 at 08:00:55PM +, Marc Zyngier wrote:
> From: Jintack Lim
>
> Support injecting exceptions and performing exception returns to and
> from virtual EL2. This must be done entirely in software except when
> taking an exception from vEL0 to vEL2 when the virtual HCR_EL2.{E2H,TGE}
> == {1,1} (a VHE guest hypervisor).
>
> Signed-off-by: Jintack Lim
> Signed-off-by: Christoffer Dall
> [maz: switch to common exception injection framework]
> Signed-off-by: Marc Zyngier
> ---
...
> +void kvm_emulate_nested_eret(struct kvm_vcpu *vcpu)
> +{
> + u64 spsr, elr, mode;
> + bool direct_eret;
> +
> + /*
> + * Going through the whole put/load motions is a waste of time
> + * if this is a VHE guest hypervisor returning to its own
> + * userspace, or the hypervisor performing a local exception
> + * return. No need to save/restore registers, no need to
> + * switch S2 MMU. Just do the canonical ERET.
> + */
> + spsr = vcpu_read_sys_reg(vcpu, SPSR_EL2);
> + mode = spsr & (PSR_MODE_MASK | PSR_MODE32_BIT);
> +
> + direct_eret = (mode == PSR_MODE_EL0t &&
> + vcpu_el2_e2h_is_set(vcpu) &&
> + vcpu_el2_tge_is_set(vcpu));
> + direct_eret |= (mode == PSR_MODE_EL2h || mode == PSR_MODE_EL2t);
There are excessive parens on the RHS of the above two.
> +static void kvm_inject_el2_exception(struct kvm_vcpu *vcpu, u64 esr_el2,
> + enum exception_type type)
> +{
> + trace_kvm_inject_nested_exception(vcpu, esr_el2, type);
> +
> + switch (type) {
> + case except_type_sync:
> + vcpu->arch.flags |= KVM_ARM64_EXCEPT_AA64_ELx_SYNC;
> + break;
> + case except_type_irq:
> + vcpu->arch.flags |= KVM_ARM64_EXCEPT_AA64_ELx_IRQ;
> + break;
> + default:
> + WARN_ONCE(1, "Unsupported EL2 exception injection %d\n", type);
> + }
> +
> + vcpu->arch.flags |= (KVM_ARM64_EXCEPT_AA64_EL2 |
> + KVM_ARM64_PENDING_EXCEPTION);
Ditto, and the "|" has too much white space before it with no good
reason.
> +/*
> + * Emulate taking an exception to EL2.
> + * See ARM ARM J8.1.2 AArch64.TakeException()
> + */
> +static int kvm_inject_nested(struct kvm_vcpu *vcpu, u64 esr_el2,
> + enum exception_type type)
> +{
> + u64 pstate, mode;
> + bool direct_inject;
> +
> + if (!nested_virt_in_use(vcpu)) {
> + kvm_err("Unexpected call to %s for the non-nesting
> configuration\n",
> + __func__);
Too much indentation. I'm guessing this "unexpected" condition isn't
something that can be caused by a rogue guest? If it can, doesn't this
need to be rate limited?
> + return -EINVAL;
> + }
> +
> + /*
> + * As for ERET, we can avoid doing too much on the injection path by
> + * checking that we either took the exception from a VHE host
> + * userspace or from vEL2. In these cases, there is no change in
> + * translation regime (or anything else), so let's do as little as
> + * possible.
> + */
> + pstate = *vcpu_cpsr(vcpu);
> + mode = pstate & (PSR_MODE_MASK | PSR_MODE32_BIT);
> +
> + direct_inject = (mode == PSR_MODE_EL0t &&
> + vcpu_el2_e2h_is_set(vcpu) &&
> + vcpu_el2_tge_is_set(vcpu));
> + direct_inject |= (mode == PSR_MODE_EL2h || mode == PSR_MODE_EL2t);
Too many parens again...
> diff --git a/arch/arm64/kvm/hyp/exception.c b/arch/arm64/kvm/hyp/exception.c
> index 0418399e0a20..4ef5e86efd8b 100644
> --- a/arch/arm64/kvm/hyp/exception.c
> +++ b/arch/arm64/kvm/hyp/exception.c
...
> @@ -149,7 +167,7 @@ static void enter_exception64(struct kvm_vcpu *vcpu,
> unsigned long target_mode,
> new |= target_mode;
>
> *vcpu_cpsr(vcpu) = new;
> - __vcpu_write_spsr(vcpu, old);
> + __vcpu_write_spsr(vcpu, target_mode, old);
> }
>
> /*
> @@ -320,11 +338,22 @@ static void kvm_inject_exception(struct kvm_vcpu *vcpu)
> KVM_ARM64_EXCEPT_AA64_EL1):
> enter_exception64(vcpu, PSR_MODE_EL1h,
> except_type_sync);
> break;
> +
> + case (KVM_ARM64_EXCEPT_AA64_ELx_SYNC |
> + KVM_ARM64_EXCEPT_AA64_EL2):
Too many parens...
> + enter_exception64(vcpu, PSR_MODE_EL2h,
> except_type_sync);
> + break;
> +
> + case (KVM_ARM64_EXCEPT_AA64_ELx_IRQ |
> + KVM_ARM64_EXCEPT_AA64_EL2):
Too many parens...
> diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c
> index b47df73e98d7..5dcf3f8b08b8 100644
> --- a/arch/arm64/kvm/inject_fault.c
> +++ b/arch/arm64/kvm/inject_fault.c
> @@ -12,19 +12,58 @@
>
> #include
> #include
> +#include
> #include
>
> +static void pend_sync_exception(struct kvm_vcpu *vcpu)
> +{
> + vcpu->arch.flags |= (
Re: [PATCH v5 13/69] KVM: arm64: nv: Reset VMPIDR_EL2 and VPIDR_EL2 to sane values
On Mon, Nov 29, 2021 at 08:00:54PM +, Marc Zyngier wrote: > From: Christoffer Dall > > The VMPIDR_EL2 and VPIDR_EL2 are architecturally UNKNOWN at reset, but > let's be nice to a guest hypervisor behaving foolishly and reset these > to something reasonable anyway. > > Signed-off-by: Christoffer Dall > Signed-off-by: Marc Zyngier Reviewed-by: Russell King (Oracle) -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 12/69] KVM: arm64: nv: Handle HCR_EL2.NV system register traps
On Mon, Nov 29, 2021 at 08:00:53PM +, Marc Zyngier wrote:
> From: Jintack Lim
>
> ARM v8.3 introduces a new bit in the HCR_EL2, which is the NV bit. When
> this bit is set, accessing EL2 registers in EL1 traps to EL2. In
> addition, executing the following instructions in EL1 will trap to EL2:
> tlbi, at, eret, and msr/mrs instructions to access SP_EL1. Most of the
> instructions that trap to EL2 with the NV bit were undef at EL1 prior to
> ARM v8.3. The only instruction that was not undef is eret.
>
> This patch sets up a handler for EL2 registers and SP_EL1 register
> accesses at EL1. The host hypervisor keeps those register values in
> memory, and will emulate their behavior.
>
> This patch doesn't set the NV bit yet. It will be set in a later patch
> once nested virtualization support is completed.
>
> Signed-off-by: Jintack Lim
> [maz: added SCTLR_EL2 RES0/RES1 handling]
> Signed-off-by: Marc Zyngier
> ---
...
> @@ -1825,9 +1882,51 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> { PMU_SYS_REG(SYS_PMCCFILTR_EL0), .access = access_pmu_evtyper,
> .reset = reset_val, .reg = PMCCFILTR_EL0, .val = 0 },
>
> + { SYS_DESC(SYS_VPIDR_EL2), access_rw, reset_val, VPIDR_EL2, 0 },
> + { SYS_DESC(SYS_VMPIDR_EL2), access_rw, reset_val, VMPIDR_EL2, 0 },
> +
> + { SYS_DESC(SYS_SCTLR_EL2), access_sctlr_el2, reset_val, SCTLR_EL2,
> SCTLR_EL2_RES1 },
> + { SYS_DESC(SYS_ACTLR_EL2), access_rw, reset_val, ACTLR_EL2, 0 },
> + { SYS_DESC(SYS_HCR_EL2), access_rw, reset_val, HCR_EL2, 0 },
> + { SYS_DESC(SYS_MDCR_EL2), access_rw, reset_val, MDCR_EL2, 0 },
> + { SYS_DESC(SYS_CPTR_EL2), access_rw, reset_val, CPTR_EL2,
> CPTR_EL2_DEFAULT },
> + { SYS_DESC(SYS_HSTR_EL2), access_rw, reset_val, HSTR_EL2, 0 },
> + { SYS_DESC(SYS_HACR_EL2), access_rw, reset_val, HACR_EL2, 0 },
> +
> + { SYS_DESC(SYS_TTBR0_EL2), access_rw, reset_val, TTBR0_EL2, 0 },
> + { SYS_DESC(SYS_TTBR1_EL2), access_rw, reset_val, TTBR1_EL2, 0 },
> + { SYS_DESC(SYS_TCR_EL2), access_rw, reset_val, TCR_EL2, TCR_EL2_RES1 },
> + { SYS_DESC(SYS_VTTBR_EL2), access_rw, reset_val, VTTBR_EL2, 0 },
> + { SYS_DESC(SYS_VTCR_EL2), access_rw, reset_val, VTCR_EL2, 0 },
> +
> { SYS_DESC(SYS_DACR32_EL2), NULL, reset_unknown, DACR32_EL2 },
> + { SYS_DESC(SYS_SPSR_EL2), access_rw, reset_val, SPSR_EL2, 0 },
> + { SYS_DESC(SYS_ELR_EL2), access_rw, reset_val, ELR_EL2, 0 },
> + { SYS_DESC(SYS_SP_EL1), access_sp_el1},
> +
> { SYS_DESC(SYS_IFSR32_EL2), NULL, reset_unknown, IFSR32_EL2 },
> + { SYS_DESC(SYS_AFSR0_EL2), access_rw, reset_val, AFSR0_EL2, 0 },
> + { SYS_DESC(SYS_AFSR1_EL2), access_rw, reset_val, AFSR1_EL2, 0 },
> + { SYS_DESC(SYS_ESR_EL2), access_rw, reset_val, ESR_EL2, 0 },
> { SYS_DESC(SYS_FPEXC32_EL2), NULL, reset_val, FPEXC32_EL2, 0x700 },
> +
> + { SYS_DESC(SYS_FAR_EL2), access_rw, reset_val, FAR_EL2, 0 },
> + { SYS_DESC(SYS_HPFAR_EL2), access_rw, reset_val, HPFAR_EL2, 0 },
> +
> + { SYS_DESC(SYS_MAIR_EL2), access_rw, reset_val, MAIR_EL2, 0 },
> + { SYS_DESC(SYS_AMAIR_EL2), access_rw, reset_val, AMAIR_EL2, 0 },
> +
> + { SYS_DESC(SYS_VBAR_EL2), access_rw, reset_val, VBAR_EL2, 0 },
> + { SYS_DESC(SYS_RVBAR_EL2), access_rw, reset_val, RVBAR_EL2, 0 },
> + { SYS_DESC(SYS_RMR_EL2), trap_undef },
> +
> + { SYS_DESC(SYS_CONTEXTIDR_EL2), access_rw, reset_val, CONTEXTIDR_EL2, 0
> },
> + { SYS_DESC(SYS_TPIDR_EL2), access_rw, reset_val, TPIDR_EL2, 0 },
> +
> + { SYS_DESC(SYS_CNTVOFF_EL2), access_rw, reset_val, CNTVOFF_EL2, 0 },
> + { SYS_DESC(SYS_CNTHCTL_EL2), access_rw, reset_val, CNTHCTL_EL2, 0 },
> +
> + { SYS_DESC(SYS_SP_EL2), NULL, reset_unknown, SP_EL2 },
Doesn't this have an effect on the ability to migrate guests between
identical hardware but running kernels with vs without this patch?
>From what I remember, QEMU fails a migration if the migration target
has less system registers than the migration source.
If so, this should at the very least be spelt out in the commit
message - it's a user experience breaking change. Maybe also preventing
the exposure of these when NV is disabled would be a good idea?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 11/69] KVM: arm64: nv: Add nested virt VCPU primitives for vEL2 VCPU state
On Mon, Nov 29, 2021 at 08:00:52PM +, Marc Zyngier wrote: > From: Christoffer Dall > > When running a nested hypervisor we commonly have to figure out if > the VCPU mode is running in the context of a guest hypervisor or guest > guest, or just a normal guest. > > Add convenient primitives for this. > > Signed-off-by: Christoffer Dall > Signed-off-by: Marc Zyngier Reviewed-by: Russell King (Oracle) -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 10/69] KVM: arm64: nv: Add EL2 system registers to vcpu context
On Mon, Nov 29, 2021 at 08:00:51PM +, Marc Zyngier wrote: > Add the minimal set of EL2 system registers to the vcpu context. > Nothing uses them just yet. > > Reviewed-by: Andre Przywara > Signed-off-by: Marc Zyngier Reviewed-by: Russell King (Oracle) -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 09/69] KVM: arm64: nv: Allow userspace to set PSR_MODE_EL2x
On Mon, Nov 29, 2021 at 08:00:50PM +, Marc Zyngier wrote: > From: Christoffer Dall > > We were not allowing userspace to set a more privileged mode for the VCPU > than EL1, but we should allow this when nested virtualization is enabled > for the VCPU. > > Signed-off-by: Christoffer Dall > Signed-off-by: Marc Zyngier Reviewed-by: Russell King (Oracle) -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 08/69] KVM: arm64: nv: Reset VCPU to EL2 registers if VCPU nested virt is set
On Mon, Nov 29, 2021 at 08:00:49PM +, Marc Zyngier wrote:
> From: Christoffer Dall
>
> Reset the VCPU with PSTATE.M = EL2h when the nested virtualization
> feature is enabled on the VCPU.
>
> Signed-off-by: Christoffer Dall
> [maz: rework register reset not to use empty data structures]
> Signed-off-by: Marc Zyngier
Reviewed-by: Russell King (Oracle)
However, a couple of comments below.
> ---
> arch/arm64/kvm/reset.c | 10 --
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> index 426bd7fbc3fd..38a7182819fb 100644
> --- a/arch/arm64/kvm/reset.c
> +++ b/arch/arm64/kvm/reset.c
> @@ -27,6 +27,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> /* Maximum phys_shift supported for any VM on this host */
> @@ -38,6 +39,9 @@ static u32 kvm_ipa_limit;
> #define VCPU_RESET_PSTATE_EL1(PSR_MODE_EL1h | PSR_A_BIT | PSR_I_BIT
> | \
>PSR_F_BIT | PSR_D_BIT)
>
> +#define VCPU_RESET_PSTATE_EL2(PSR_MODE_EL2h | PSR_A_BIT | PSR_I_BIT
> | \
> + PSR_F_BIT | PSR_D_BIT)
> +
> #define VCPU_RESET_PSTATE_SVC(PSR_AA32_MODE_SVC | PSR_AA32_A_BIT | \
>PSR_AA32_I_BIT | PSR_AA32_F_BIT)
>
> @@ -176,8 +180,8 @@ static bool vcpu_allowed_register_width(struct kvm_vcpu
> *vcpu)
> if (!cpus_have_const_cap(ARM64_HAS_32BIT_EL1) && is32bit)
> return false;
>
> - /* MTE is incompatible with AArch32 */
> - if (kvm_has_mte(vcpu->kvm) && is32bit)
> + /* MTE and NV are incompatible with AArch32 */
> + if ((kvm_has_mte(vcpu->kvm) || nested_virt_in_use(vcpu)) && is32bit)
> return false;
It seems we have a bunch of:
if (something && is32bit)
return false;
tests here - would it make sense to do:
if (is32bit) {
if (!cpus_have_const_cap(ARM64_HAS_32BIT_EL1))
return false;
/* MTE is incompatible with AArch32 */
if (kvm_has_mte(vcpu->kvm))
return false;
/* NV is incompatible with AArch32 */
if (nested_virt_in_use(vcpu))
return false;
}
in terms of improved readability?
> @@ -255,6 +259,8 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
> default:
> if (test_bit(KVM_ARM_VCPU_EL1_32BIT, vcpu->arch.features)) {
> pstate = VCPU_RESET_PSTATE_SVC;
> + } else if (nested_virt_in_use(vcpu)) {
> + pstate = VCPU_RESET_PSTATE_EL2;
> } else {
> pstate = VCPU_RESET_PSTATE_EL1;
> }
Not an issue with your patch, but the switch around this looks useless.
The only case is this default case, so it's entirely a no-op.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 07/69] KVM: arm64: nv: Introduce nested virtualization VCPU feature
On Mon, Nov 29, 2021 at 08:00:48PM +, Marc Zyngier wrote: > From: Christoffer Dall > > Introduce the feature bit and a primitive that checks if the feature is > set behind a static key check based on the cpus_have_const_cap check. > > Checking nested_virt_in_use() on systems without nested virt enabled > should have neglgible overhead. > > We don't yet allow userspace to actually set this feature. > > Signed-off-by: Christoffer Dall > Signed-off-by: Marc Zyngier Reviewed-by: Russell King (Oracle) -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 05/69] KVM: arm64: Allow preservation of the S2 SW bits
On Mon, Nov 29, 2021 at 08:00:46PM +, Marc Zyngier wrote: > The S2 page table code has a limited use the SW bits, but we are about I think the opening clause needs to be rewritten. > to need them to encode some guest Stage-2 information (its mapping size > in the form of the TTL encoding). > > Propagate the SW bits specified by the caller, and store them into > the corresponding entry. > > Signed-off-by: Marc Zyngier Reviewed-by: Russell King (Oracle) -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 04/69] KVM: arm64: Rework kvm_pgtable initialisation
On Mon, Nov 29, 2021 at 08:00:45PM +, Marc Zyngier wrote: > Ganapatrao reported that the kvm_pgtable->mmu pointer is more or > less hardcoded to the main S2 mmu structure, while the nested > code needs it to point to other instances (as we have one instance > per nested context). > > Rework the initialisation of the kvm_pgtable structure so that > this assumtion doesn't hold true anymore. This requires some > minor changes to the order in which things are initialised > (the mmu->arch pointer being the critical one). > > Reported-by: Ganapatrao Kulkarni > Reviewed-by: Ganapatrao Kulkarni > Signed-off-by: Marc Zyngier Looks fairly simple. Reviewed-by: Russell King (Oracle) -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 03/69] KVM: arm64: Add minimal handling for the ARMv8.7 PMU
On Mon, Nov 29, 2021 at 08:00:44PM +, Marc Zyngier wrote: > When running a KVM guest hosted on an ARMv8.7 machine, the host > kernel complains that it doesn't know about the architected number > of events. > > Fix it by adding the PMUver code corresponding to PMUv3 for ARMv8.7. > > Reviewed-by: Alexandru Elisei > Tested-by: Alexandru Elisei > Signed-off-by: Marc Zyngier > Link: https://lore.kernel.org/r/20211126115533.217903-1-...@kernel.org Reviewed-by: Russell King (Oracle) -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 01/69] KVM: arm64: Save PSTATE early on exit
On Mon, Nov 29, 2021 at 08:00:42PM +, Marc Zyngier wrote: > In order to be able to use primitives such as vcpu_mode_is_32bit(), > we need to synchronize the guest PSTATE. However, this is currently > done deep into the bowels of the world-switch code, and we do have > helpers evaluating this much earlier (__vgic_v3_perform_cpuif_access > and handle_aarch32_guest, for example). > > Move the saving of the guest pstate into the early fixups, which > cures the first issue. The second one will be addressed separately. > > Tested-by: Fuad Tabba > Reviewed-by: Fuad Tabba > Signed-off-by: Marc Zyngier Reviewed-by: Russell King (Oracle) -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 02/69] KVM: arm64: Move pkvm's special 32bit handling into a generic infrastructure
On Mon, Nov 29, 2021 at 08:00:43PM +, Marc Zyngier wrote: > diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c > b/arch/arm64/kvm/hyp/nvhe/switch.c > index c0e3fed26d93..d13115a12434 100644 > --- a/arch/arm64/kvm/hyp/nvhe/switch.c > +++ b/arch/arm64/kvm/hyp/nvhe/switch.c > @@ -233,7 +233,7 @@ static const exit_handler_fn > *kvm_get_exit_handler_array(struct kvm_vcpu *vcpu) > * Returns false if the guest ran in AArch32 when it shouldn't have, and > * thus should exit to the host, or true if a the guest run loop can > continue. > */ > -static bool handle_aarch32_guest(struct kvm_vcpu *vcpu, u64 *exit_code) > +static void early_exit_filter(struct kvm_vcpu *vcpu, u64 *exit_code) The comment above this function needs fixing up - it talks about the return value, but this patch changes the return type to be void. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: REGRESSION: Upgrading host kernel from 5.11 to 5.13 breaks QEMU guests - perf/fw_devlink/kvm
On Sun, Sep 19, 2021 at 02:36:46PM +0100, Marc Zyngier wrote: > Urgh. That's a bummer. T1he PMU driver only comes up once it has found > its interrupt controller, which on the Armada 8040 is not the GIC, but > some weird thing on the side that doesn't actually serve any real > purpose. On HW where the PMU is directly wired into the GIC, it all > works fine, though by luck rather than by design. > > Anyway, rant over. This is a bug that needs addressing so that KVM can > initialise correctly irrespective of the probing order. This probably > means that the static key controlling KVM's behaviour wrt the PMU must > be controlled by the PMU infrastructure itself, rather than KVM trying > to probe for it. > > Can you please give the following hack a go (on top of 5.15-rc1)? I've > briefly tested it on my McBin, and it did the trick. I've also tested > it on the M1 (which really doesn't have an architectural PMU) to > verify that it was correctly failing. My test program that derives the number of registers qemu uses now reports 236 registers again and I see: kvm [7]: PMU detected and enabled in the kernel boot log. Tested-by: Russell King (Oracle) Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
REGRESSION: Upgrading host kernel from 5.11 to 5.13 breaks QEMU guests - perf/fw_devlink/kvm
Hi, This morning, I upgraded my VM host from Debian Buster to Debian Bullseye. This host (Armada 8040) runs libvirt. I placed all the VMs into managedsave, which basically means their state is saved out by QEMU ready to be resumed once the upgrade is complete. Initially, I was running 5.11 on the host, which didn't have POSIX ACLs enabled on tmpfs. Sadly, upgrading to Bullseye now requires this to be enabled, so I upgraded the kernel to 5.13 to avoid this problem - without POSIX ACLs on tmpfs, qemu refuses to even start. Under Bullseye with 5.13, I could start new VMs, but I could not resume the saved VMs - it would spit out: 2021-09-18T11:14:14.456227Z qemu-system-aarch64: Invalid value 236 expecting positive value <= 162 2021-09-18T11:14:14.456269Z qemu-system-aarch64: Failed to load cpu:cpreg_vmstate_array_len 2021-09-18T11:14:14.456279Z qemu-system-aarch64: error while loading state for instance 0x0 of device 'cpu' 2021-09-18T11:14:14.456887Z qemu-system-aarch64: load of migration failed: Invalid argument Essentially, what this is complaining about is that the register list returned by the KVM_GET_REG_LIST ioctl has reduced in size, meaning that the saved VM has more registers that need to be set (236 of them, after QEMU's filtering) than those which are actually available under 5.13 (162 after QEMU's filtering). After much debugging, and writing a program to create a VM and CPU, and retrieve the register list etc, I have finally tracked down exactly what is going on... Under 5.13, KVM believes there is no PMU, so it doesn't advertise the PMU registers, despite the hardware having a PMU and the kernel having support. kvm_arm_support_pmu_v3() determines whether the PMU_V3 feature is available or not. Under 5.11, this was determined via: perf_num_counters() > 0 which in turn was derived from (essentially): __oprofile_cpu_pmu && __oprofile_cpu_pmu->num_events > 0 __oprofile_cpu_pmu can be set at any time when the PMU has been initialised, and due to initialisation ordering, it appears to happen much later in the kernel boot. However, under 5.13, oprofile has been removed, and this test is no longer present. Instead, kvm attempts to determine the availability of PMUv3 when it initialises, and fails to because the PMU has not yet been initialised. If there is no PMU at the point KVM initialises, then KVM will never advertise a PMU. This change of behaviour is what breaks KVM on Armada 8040, causing a userspace regression. What makes this more confusing is the PMU errors have gone: 5.13: [0.170514] PCI: CLS 0 bytes, default 64 [0.171085] kvm [1]: IPA Size Limit: 44 bits [0.172532] kvm [1]: vgic interrupt IRQ9 [0.172650] kvm: pmu event creation failed -2 [0.172688] kvm [1]: Hyp mode initialized successfully ... [1.479833] hw perfevents: enabled with armv8_cortex_a72 PMU driver, 7 counters available 5.11: [0.138831] PCI: CLS 0 bytes, default 64 [0.139245] hw perfevents: unable to count PMU IRQs [0.139251] hw perfevents: /ap806/config-space@f000/pmu: failed to register PMU devices! [0.139503] kvm [1]: IPA Size Limit: 44 bits [0.140735] kvm [1]: vgic interrupt IRQ9 [0.140836] kvm [1]: Hyp mode initialized successfully ... [1.447789] hw perfevents: enabled with armv8_cortex_a72 PMU driver, 7 counters available Overall, what this means is that the only way to safely upgrade from 5.11 to 5.13 (I don't know where 5.12 fits in this) is to completely shut down all your VMs. You can't even migrate them to another identical machine across these kernels. You have to completely shut them down. I did manage to eventually rescue my VMs after many hours, by booting back into 5.11, and then screwing around with bind mounts to replace /dev with a filesystem that did have POSIX ACLs enabled, so libvirt and qemu could then resume the VMs and cleanly shut them down all down. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2] memblock: make memblock_find_in_range method private
On Mon, Aug 02, 2021 at 09:37:37AM +0300, Mike Rapoport wrote: > From: Mike Rapoport > > There are a lot of uses of memblock_find_in_range() along with > memblock_reserve() from the times memblock allocation APIs did not exist. > > memblock_find_in_range() is the very core of memblock allocations, so any > future changes to its internal behaviour would mandate updates of all the > users outside memblock. > > Replace the calls to memblock_find_in_range() with an equivalent calls to > memblock_phys_alloc() and memblock_phys_alloc_range() and make > memblock_find_in_range() private method of memblock. > > This simplifies the callers, ensures that (unlikely) errors in > memblock_reserve() are handled and improves maintainability of > memblock_find_in_range(). > > Signed-off-by: Mike Rapoport Acked-by: Russell King (Oracle) Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH] memblock: make memblock_find_in_range method private
On Fri, Jul 30, 2021 at 01:40:39PM +0300, Mike Rapoport wrote:
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index f97eb2371672..1f8ef9fd5215 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -1012,31 +1012,25 @@ static void __init reserve_crashkernel(void)
> unsigned long long lowmem_max = __pa(high_memory - 1) + 1;
> if (crash_max > lowmem_max)
> crash_max = lowmem_max;
> - crash_base = memblock_find_in_range(CRASH_ALIGN, crash_max,
> - crash_size, CRASH_ALIGN);
> +
> + crash_base = memblock_phys_alloc_range(crash_size, CRASH_ALIGN,
> +CRASH_ALIGN, crash_max);
> if (!crash_base) {
> pr_err("crashkernel reservation failed - No suitable
> area found.\n");
> return;
> }
> } else {
> + unsigned long long crash_max = crash_base + crash_size;
> unsigned long long start;
>
> - start = memblock_find_in_range(crash_base,
> -crash_base + crash_size,
> -crash_size, SECTION_SIZE);
> + start = memblock_phys_alloc_range(crash_size, SECTION_SIZE,
> + crash_base, crash_max);
> if (start != crash_base) {
> - pr_err("crashkernel reservation failed - memory is in
> use.\n");
> + pr_err("crashkernel reservation failed - No suitable
> area found.\n");
This change to the error message is incorrect. In this code block, we
are trying to get the exact specified memory block - it is not about
there being "no suitable area" - the requested memory block is not
available. So, the original message carries the exact correct meaning.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 1/3] KVM: arm64: Narrow PMU sysreg reset values to architectural requirements
On Tue, Jul 13, 2021 at 04:59:58PM +0100, Marc Zyngier wrote:
> On Tue, 13 Jul 2021 15:39:49 +0100,
> "Russell King (Oracle)" wrote:
> >
> > On Tue, Jul 13, 2021 at 02:58:58PM +0100, Marc Zyngier wrote:
> > > +static void reset_pmu_reg(struct kvm_vcpu *vcpu, const struct
> > > sys_reg_desc *r)
> > > +{
> > > + u64 n, mask;
> > > +
> > > + /* No PMU available, any PMU reg may UNDEF... */
> > > + if (!kvm_arm_support_pmu_v3())
> > > + return;
> > > +
> > > + n = read_sysreg(pmcr_el0) >> ARMV8_PMU_PMCR_N_SHIFT;
> > > + n &= ARMV8_PMU_PMCR_N_MASK;
> > > +
> > > + reset_unknown(vcpu, r);
> > > +
> > > + mask = BIT(ARMV8_PMU_CYCLE_IDX);
> > > + if (n)
> > > + mask |= GENMASK(n - 1, 0);
> > > +
> > > + __vcpu_sys_reg(vcpu, r->reg) &= mask;
> >
> > Would this read more logically to structure it as:
> >
> > mask = BIT(ARMV8_PMU_CYCLE_IDX);
> >
> > n = read_sysreg(pmcr_el0) >> ARMV8_PMU_PMCR_N_SHIFT;
> > n &= ARMV8_PMU_PMCR_N_MASK;
> > if (n)
> > mask |= GENMASK(n - 1, 0);
> >
> > reset_unknown(vcpu, r);
> > __vcpu_sys_reg(vcpu, r->reg) &= mask;
> >
> > ?
>
> Yup, that's nicer. Amended locally.
Thanks Marc.
For the whole series:
Acked-by: Russell King (Oracle)
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 1/3] KVM: arm64: Narrow PMU sysreg reset values to architectural requirements
On Tue, Jul 13, 2021 at 02:58:58PM +0100, Marc Zyngier wrote:
> +static void reset_pmu_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc
> *r)
> +{
> + u64 n, mask;
> +
> + /* No PMU available, any PMU reg may UNDEF... */
> + if (!kvm_arm_support_pmu_v3())
> + return;
> +
> + n = read_sysreg(pmcr_el0) >> ARMV8_PMU_PMCR_N_SHIFT;
> + n &= ARMV8_PMU_PMCR_N_MASK;
> +
> + reset_unknown(vcpu, r);
> +
> + mask = BIT(ARMV8_PMU_CYCLE_IDX);
> + if (n)
> + mask |= GENMASK(n - 1, 0);
> +
> + __vcpu_sys_reg(vcpu, r->reg) &= mask;
Would this read more logically to structure it as:
mask = BIT(ARMV8_PMU_CYCLE_IDX);
n = read_sysreg(pmcr_el0) >> ARMV8_PMU_PMCR_N_SHIFT;
n &= ARMV8_PMU_PMCR_N_MASK;
if (n)
mask |= GENMASK(n - 1, 0);
reset_unknown(vcpu, r);
__vcpu_sys_reg(vcpu, r->reg) &= mask;
?
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 4/5] KVM: arm64: Pass hypercalls to userspace
On Tue, Jun 08, 2021 at 05:48:05PM +0200, Jean-Philippe Brucker wrote: > * Untested for AArch32 guests. That really needs testing; you may notice Will Deacon has been posting patches for aarch32 guests over the last few months, and there are other users out there who run aarch32 guests on their aarch64 hardware. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
