Re: [PATCH v4 23/27] KVM: s390: arm64: Implement required functions

[Marc Zyngier <maz@kernel.org>]

On Mon, 06 Jul 2026 09:52:23 +0100,
Steffen Eiden <seiden@linux.ibm.com> wrote:
> 
> Implement the mostly trivial functions that the shared arm64 (kvm)
> code & headers oblige s390 to implement.
> 
> Implement a very basic smccc handler that (non-compliantly) is just able
> to stop a vcpu.
> 
> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
> ---
>  arch/s390/include/arm64/kvm_emulate.h | 135 ++++++++++++++++++++++++++
>  arch/s390/include/arm64/kvm_nested.h  |  11 +++
>  arch/s390/kvm/arm64/handle_exit.c     | 110 +++++++++++++++++++++
>  arch/s390/kvm/arm64/inject_fault.c    |  21 ++++
>  4 files changed, 277 insertions(+)
>  create mode 100644 arch/s390/include/arm64/kvm_emulate.h
>  create mode 100644 arch/s390/include/arm64/kvm_nested.h
>  create mode 100644 arch/s390/kvm/arm64/handle_exit.c
>  create mode 100644 arch/s390/kvm/arm64/inject_fault.c
> 
> diff --git a/arch/s390/include/arm64/kvm_emulate.h b/arch/s390/include/arm64/kvm_emulate.h
> new file mode 100644
> index 000000000000..6ad21398f86f
> --- /dev/null
> +++ b/arch/s390/include/arm64/kvm_emulate.h
> @@ -0,0 +1,135 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __S390_ARM64_KVM_EMULATE_H__
> +#define __S390_ARM64_KVM_EMULATE_H__
> +
> +#include <asm/fault.h>
> +#include <asm/ptrace.h>
> +#include <linux/kvm_host.h>
> +
> +#include <arm64/kvm_arm.h>
> +#include <arm64/sysreg.h>
> +
> +static __always_inline unsigned long *vcpu_pc(const struct kvm_vcpu *vcpu)
> +{
> +	return (unsigned long *)&vcpu->arch.sae_block.pc;
> +}
> +
> +static __always_inline unsigned long *vcpu_cpsr(const struct kvm_vcpu *vcpu)
> +{
> +	return (unsigned long *)&vcpu->arch.sae_block.pstate;
> +}
> +
> +static __always_inline unsigned long *vcpu_sp_el0(const struct kvm_vcpu *vcpu)
> +{
> +	return (unsigned long *)&vcpu->arch.sae_block.sp_el0;
> +}
> +
> +static __always_inline bool vcpu_mode_is_32bit(const struct kvm_vcpu *vcpu)
> +{
> +	return false;
> +}
> +
> +static __always_inline u64 kvm_vcpu_get_esr(const struct kvm_vcpu *vcpu)
> +{
> +	return vcpu->arch.sae_block.hai.esr_elz;
> +}
> +
> +static __always_inline unsigned long kvm_vcpu_get_hfar(const struct kvm_vcpu *vcpu)
> +{
> +	return vcpu->arch.sae_block.hai.far_elz;
> +}
> +
> +static __always_inline phys_addr_t kvm_vcpu_get_fault_ipa(const struct kvm_vcpu *vcpu)
> +{
> +	return vcpu->arch.sae_block.hai.teid.addr * PAGE_SIZE;
> +}
> +
> +static inline u16 kvm_vcpu_fault_pic(const struct kvm_vcpu *vcpu)
> +{
> +	return vcpu->arch.sae_block.hai.pic & PGM_INT_CODE_MASK;
> +}
> +
> +static __always_inline
> +bool kvm_vcpu_trap_is_permission_fault(const struct kvm_vcpu *vcpu)
> +{
> +	return kvm_vcpu_fault_pic(vcpu) == PGM_PROTECTION;
> +}
> +
> +static __always_inline bool kvm_condition_valid(const struct kvm_vcpu *vcpu)
> +{
> +	return true;
> +}
> +
> +static __always_inline bool vcpu_el1_is_32bit(struct kvm_vcpu *vcpu)
> +{
> +	return false;
> +}
> +
> +static inline void vcpu_reset_hcr(struct kvm_vcpu *vcpu)
> +{
> +	vcpu->arch.hcr_elz = HCR_E2H | HCR_RW | HCR_AMO | HCR_IMO | HCR_FMO |
> +			     HCR_PTW;
> +	/* traps */
> +	vcpu->arch.hcr_elz |= HCR_TSC | HCR_TID1 | HCR_TID2 | HCR_TID3 |
> +			      HCR_TID4 | HCR_TID5 | HCR_TIDCP;

Since this is new code, consider using the generated symbols
(HCR_EL2_*). I intend to get rid of the compat symbols at some point.

> +}
> +
> +static inline unsigned long vcpu_get_vsesr(struct kvm_vcpu *vcpu)
> +{
> +	WARN(true, "not implemented, just feat RAS");

WARN_ONCE() should be enough. And really, this code shouldn't be
reachable at all.

> +
> +	return 0L;
> +}
> +
> +static inline void vcpu_set_vsesr(struct kvm_vcpu *vcpu, u64 vsesr)
> +{
> +	WARN(true, "not implemented, just feat RAS");

Same thing.

> +}
> +
> +static inline bool vcpu_el2_tge_is_set(const struct kvm_vcpu *vcpu)
> +{
> +	return false;
> +}
> +
> +static inline bool kvm_vcpu_is_be(struct kvm_vcpu *vcpu)
> +{
> +	return false;
> +}
> +
> +static inline int kvm_vcpu_abt_gltl(struct kvm_vcpu *vcpu)
> +{
> +	return vcpu->arch.sae_block.hai.gltl;
> +}
> +
> +static inline bool is_hyp_ctxt(const struct kvm_vcpu *vcpu)
> +{
> +	return false;
> +}
> +
> +static inline bool is_nested_ctxt(struct kvm_vcpu *vcpu)
> +{
> +	return false;
> +}
> +
> +static inline bool vcpu_mode_priv(const struct kvm_vcpu *vcpu)
> +{
> +	u32 mode = *vcpu_cpsr(vcpu) & PSR_MODE_MASK;
> +
> +	return mode != PSR_MODE_EL0t;
> +}
> +
> +#define SPSR_SS BIT(21)

We may be better off defining SPSR_ELx in the sysreg file and use that
all over the place.

> +
> +static inline void kvm_skip_instr(struct kvm_vcpu *vcpu)
> +{
> +	*vcpu_pc(vcpu) += 4;
> +	*vcpu_cpsr(vcpu) &= ~PSR_BTYPE_MASK;
> +
> +	/* advance the singlestep state machine */
> +	*vcpu_cpsr(vcpu) &= ~SPSR_SS;
> +}
> +
> +#include <arm64/kvm_emulate-part.h>
> +
> +#endif /* __S390_ARM64_KVM_EMULATE_H__ */
> diff --git a/arch/s390/include/arm64/kvm_nested.h b/arch/s390/include/arm64/kvm_nested.h
> new file mode 100644
> index 000000000000..e950b1a10c41
> --- /dev/null
> +++ b/arch/s390/include/arm64/kvm_nested.h
> @@ -0,0 +1,11 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef ASM_KVM_NESTED_H
> +#define ASM_KVM_NESTED_H
> +
> +static inline bool vcpu_has_nv(const struct kvm_vcpu *vcpu)
> +{
> +	return false;
> +}
> +
> +#endif /* ASM_KVM_NESTED_H */
> diff --git a/arch/s390/kvm/arm64/handle_exit.c b/arch/s390/kvm/arm64/handle_exit.c
> new file mode 100644
> index 000000000000..a0ebe5ffa19a
> --- /dev/null
> +++ b/arch/s390/kvm/arm64/handle_exit.c
> @@ -0,0 +1,110 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/kvm_host.h>
> +
> +#include <arm64/esr.h>
> +#include <arm64/kvm_emulate.h>
> +
> +typedef int (*exit_handle_fn)(struct kvm_vcpu *);
> +exit_handle_fn arm_exit_handlers[ESR_ELx_EC_MAX + 1];
> +
> +#define __INCL_GEN_ARM_FILE
> +#include "generated/handle_exit.inc"
> +#undef __INCL_GEN_ARM_FILE
> +
> +#define PSCI_0_2_FN_SYSTEM_OFF		0x84000008
> +#define PSCI_RET_NOT_SUPPORTED		-1
> +#define PSCI_RET_INTERNAL_FAILURE	-6

All of this exists in include/uapi/linux/psci.h.

> +/*
> + * Temporary smc/hvc handler. Non-compliant implementation (features missing).
> + * Implements only system off so that test programs are able to end their execution
> + */
> +static int kvm_smccc_call_handler(struct kvm_vcpu *vcpu)
> +{
> +	u32 func_id = vcpu_get_reg(vcpu, 0);
> +	u64 val = PSCI_RET_NOT_SUPPORTED;
> +	int ret = 1;
> +
> +	if (func_id == PSCI_0_2_FN_SYSTEM_OFF) {
> +		spin_lock(&vcpu->arch.mp_state_lock);
> +		WRITE_ONCE(vcpu->arch.mp_state.mp_state, KVM_MP_STATE_STOPPED);
> +		spin_unlock(&vcpu->arch.mp_state_lock);

scoped_guard()?

> +		kvm_make_all_cpus_request(vcpu->kvm, KVM_REQ_SLEEP);
> +		memset(&vcpu->run->system_event, 0,
> +		       sizeof(vcpu->run->system_event));
> +		vcpu->run->system_event.type = KVM_SYSTEM_EVENT_SHUTDOWN;
> +		vcpu->run->system_event.ndata = 1;
> +		vcpu->run->system_event.data[0] = 0;
> +		vcpu->run->exit_reason = KVM_EXIT_SYSTEM_EVENT;
> +		val = PSCI_RET_INTERNAL_FAILURE;
> +		ret = 0;
> +	}
> +	vcpu_set_reg(vcpu, 0, val);
> +
> +	return ret;
> +}
> +
> +static int handle_hvc(struct kvm_vcpu *vcpu)
> +{
> +	vcpu->stat.hvc_exit_stat++;
> +	return kvm_smccc_call_handler(vcpu);
> +}
> +
> +exit_handle_fn arm_exit_handlers[] = {
> +	[0 ... ESR_ELx_EC_MAX]	= kvm_handle_unknown_ec,
> +	[ESR_ELx_EC_HVC64]	= handle_hvc,
> +};
> +
> +/* manually copied from arch/arm64/kernel/traps.c */
> +static const char * const esr_class_str[] = {
> +	[0 ... ESR_ELx_EC_MAX]		= "UNRECOGNIZED EC",
> +	[ESR_ELx_EC_UNKNOWN]		= "Unknown/Uncategorized",
> +	[ESR_ELx_EC_WFx]		= "WFI/WFE",
> +	[ESR_ELx_EC_CP15_32]		= "CP15 MCR/MRC",
> +	[ESR_ELx_EC_CP15_64]		= "CP15 MCRR/MRRC",
> +	[ESR_ELx_EC_CP14_MR]		= "CP14 MCR/MRC",
> +	[ESR_ELx_EC_CP14_LS]		= "CP14 LDC/STC",
> +	[ESR_ELx_EC_FP_ASIMD]		= "ASIMD",
> +	[ESR_ELx_EC_CP10_ID]		= "CP10 MRC/VMRS",
> +	[ESR_ELx_EC_PAC]		= "PAC",
> +	[ESR_ELx_EC_CP14_64]		= "CP14 MCRR/MRRC",
> +	[ESR_ELx_EC_BTI]		= "BTI",
> +	[ESR_ELx_EC_ILL]		= "PSTATE.IL",
> +	[ESR_ELx_EC_SVC32]		= "SVC (AArch32)",
> +	[ESR_ELx_EC_HVC32]		= "HVC (AArch32)",
> +	[ESR_ELx_EC_SMC32]		= "SMC (AArch32)",
> +	[ESR_ELx_EC_SVC64]		= "SVC (AArch64)",
> +	[ESR_ELx_EC_HVC64]		= "HVC (AArch64)",
> +	[ESR_ELx_EC_SMC64]		= "SMC (AArch64)",
> +	[ESR_ELx_EC_SYS64]		= "MSR/MRS (AArch64)",
> +	[ESR_ELx_EC_SVE]		= "SVE",
> +	[ESR_ELx_EC_ERET]		= "ERET/ERETAA/ERETAB",
> +	[ESR_ELx_EC_FPAC]		= "FPAC",
> +	[ESR_ELx_EC_SME]		= "SME",
> +	[ESR_ELx_EC_IMP_DEF]		= "EL3 IMP DEF",
> +	[ESR_ELx_EC_IABT_LOW]		= "IABT (lower EL)",
> +	[ESR_ELx_EC_IABT_CUR]		= "IABT (current EL)",
> +	[ESR_ELx_EC_PC_ALIGN]		= "PC Alignment",
> +	[ESR_ELx_EC_DABT_LOW]		= "DABT (lower EL)",
> +	[ESR_ELx_EC_DABT_CUR]		= "DABT (current EL)",
> +	[ESR_ELx_EC_SP_ALIGN]		= "SP Alignment",
> +	[ESR_ELx_EC_MOPS]		= "MOPS",
> +	[ESR_ELx_EC_FP_EXC32]		= "FP (AArch32)",
> +	[ESR_ELx_EC_FP_EXC64]		= "FP (AArch64)",
> +	[ESR_ELx_EC_GCS]		= "Guarded Control Stack",
> +	[ESR_ELx_EC_SERROR]		= "SError",
> +	[ESR_ELx_EC_BREAKPT_LOW]	= "Breakpoint (lower EL)",
> +	[ESR_ELx_EC_BREAKPT_CUR]	= "Breakpoint (current EL)",
> +	[ESR_ELx_EC_SOFTSTP_LOW]	= "Software Step (lower EL)",
> +	[ESR_ELx_EC_SOFTSTP_CUR]	= "Software Step (current EL)",
> +	[ESR_ELx_EC_WATCHPT_LOW]	= "Watchpoint (lower EL)",
> +	[ESR_ELx_EC_WATCHPT_CUR]	= "Watchpoint (current EL)",
> +	[ESR_ELx_EC_BKPT32]		= "BKPT (AArch32)",
> +	[ESR_ELx_EC_VECTOR32]		= "Vector catch (AArch32)",
> +	[ESR_ELx_EC_BRK64]		= "BRK (AArch64)",
> +};

There are a lot of AArch32-specific ECs here that you may not need.

Thanks,

	M.

-- 
Jazz isn't dead. It just smells funny.