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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] PSCI SYSTEM_OFF handler fails to stop other vCPUs, leading to potential guest corruption during shutdown.
- [Medium] Empty stubs for exception injection cause guest vCPUs to hang in an infinite loop.
--

commit b5600b88451aab2daf96c3e367bc2f2db0abeb0c
Author: Steffen Eiden <seiden@linux.ibm.com>

KVM: s390: arm64: Implement required functions

This commit implements the required functions for s390 to support shared
arm64 KVM code. It also introduces a basic SMCCC handler to stop a vCPU.

> diff --git a/arch/s390/kvm/arm64/handle_exit.c b/arch/s390/kvm/arm64/handle_exit.c
> new file mode 100644
> index 0000000000000..a0ebe5ffa19a6
> --- /dev/null
> +++ b/arch/s390/kvm/arm64/handle_exit.c

[ ... ]

> +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);
> +		kvm_make_all_cpus_request(vcpu->kvm, KVM_REQ_SLEEP);

[Severity: High]
Does this actually stop all other vCPUs in the system?

Because the mp_state is only updated for the calling vCPU, broadcasting
KVM_REQ_SLEEP will leave the other vCPUs in their current state. When they
process the sleep request, won't they evaluate their own mp_state, see they
aren't STOPPED, and continue executing guest code while the VM is supposed
to be shutting down?

> +		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;
> +}

[ ... ]

> diff --git a/arch/s390/kvm/arm64/inject_fault.c b/arch/s390/kvm/arm64/inject_fault.c
> new file mode 100644
> index 0000000000000..425dbeaa421c6
> --- /dev/null
> +++ b/arch/s390/kvm/arm64/inject_fault.c
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <arm64/kvm_emulate.h>
> +
> +/**
> + * kvm_inject_undefined - inject an undefined instruction into the guest
> + * @vcpu: The vCPU in which to inject the exception
> + *
> + * It is assumed that this code is called from the VCPU thread and that the
> + * VCPU therefore is not currently executing guest code.
> + */
> +void kvm_inject_undefined(struct kvm_vcpu *vcpu)
> +{
> +	/* Stub until s390 supports arm64 sysregs TODO sysregs*/
> +}

[Severity: Medium]
Will this empty stub cause the guest to get stuck in an infinite exit loop?

Since the function does nothing, it doesn't advance the guest's Program
Counter or update exception state. When handle_exit() resumes the guest,
won't it immediately re-execute the same trapping instruction over and over,
hanging the vCPU and burning host CPU cycles?

> +
> +int kvm_inject_sea(struct kvm_vcpu *vcpu, bool iabt, u64 addr)
> +{
> +	/* Stub until s390 supports arm64 sysregs TODO sysregs*/
> +	return 1;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706085229.979525-1-seiden@linux.ibm.com?part=23