Re: [PATCH] KVM: s390: Implement CHECK_STOP support and fix GET_MP_STATE

[Christian Borntraeger <borntraeger@linux.ibm.com>]

Am 17.11.25 um 16:18 schrieb Josephine Pfeiffer:
> Add support for KVM_MP_STATE_CHECK_STOP to enable proper VM migration
> and error handling for s390 guests. The CHECK_STOP state represents a
> CPU that encountered a severe machine check and is halted in an error
> state.

I think the patch description is misleading. We do have proper VM
migration and we also have error handling in the kvm module. The host
machine check handler will forward guest machine checks to the guest.
This logic  is certainly not perfect but kind of good enough for most
cases.
Now: The architecture defines that state and the interface is certainly
there. So implementing it will allow userspace to put a CPU into checkstop
state if you ever need that. We also have a checkstop state that you
can put a secure CPU in.

The usecase is dubious though. The only case of the options from POP
chapter11 that makes sense to me in a virtualized environment is an exigent
machine check but a problem to actually deliver that (multiple reasons,
like the OS has machine checks disabled in PSW, or the prefix register
is broken).

So I am curious, do you have any specific usecase in mind?
I assume you have a related QEMU patch somewhere?

> 
> This implementation adds:
> - CPUSTAT_CHECK_STOP flag to track check-stopped CPUs
> - is_vcpu_check_stopped() helper macro for state checking
> - kvm_s390_vcpu_check_stop() function to transition CPUs to CHECK_STOP
> - Integration with Protected VM Ultravisor (PV_CPU_STATE_CHKSTP)
> - Interrupt blocking for check-stopped CPUs in deliverable_irqs()
> - Recovery path enabling CHECK_STOP -> OPERATING transitions
> - Proper state precedence where CHECK_STOP takes priority over STOPPED
> 
> Signed-off-by: Josephine Pfeiffer <hi@josie.lol>
> ---
>   arch/s390/include/asm/kvm_host_types.h |  1 +
>   arch/s390/kvm/interrupt.c              |  3 ++
>   arch/s390/kvm/kvm-s390.c               | 72 ++++++++++++++++++++++----
>   arch/s390/kvm/kvm-s390.h               |  6 +++
>   arch/s390/kvm/sigp.c                   |  8 ++-
>   5 files changed, 77 insertions(+), 13 deletions(-)
> 

 From a quick glance the patch in general does not look wrong but at least this is wrong:> diff --git a/arch/s390/include/asm/kvm_host_types.h b/arch/s390/include/asm/kvm_host_types.h
> index 1394d3fb648f..a86e326a2eee 100644
> --- a/arch/s390/include/asm/kvm_host_types.h
> +++ b/arch/s390/include/asm/kvm_host_types.h
> @@ -111,6 +111,7 @@ struct mcck_volatile_info {
>   	((((sie_block)->sidad & SIDAD_SIZE_MASK) + 1) * PAGE_SIZE)
>   
>   #define CPUSTAT_STOPPED    0x80000000
> +#define CPUSTAT_CHECK_STOP 0x40000000
Bit 1 of the sie control block is a hardware defined bit and
its meaning is not checkstop, so this is not the right way to do it.
Lets first clarify your usecase so that we can see what the right way forward is.

Christian