Re: [PATCH v2 1/1] s390:kvm: diag9c forwarding

[Pierre Morel <pmorel@linux.ibm.com>]

On 2/12/21 9:39 AM, Pierre Morel wrote:
> When we receive intercept a DIAG_9C from the guest we verify
> that the target real CPU associated with the virtual CPU
> designated by the guest is running and if not we forward the
> DIAG_9C to the target real CPU.
> 
> To avoid a diag9c storm we allow a maximal rate of diag9c forwarding.
> 
> The rate is calculated as a count per second defined as a
> new parameter of the s390 kvm module: diag9c_forwarding_hz .
> 
> The default value is to not forward diag9c.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>   Documentation/virt/kvm/s390-diag.rst | 33 ++++++++++++++++++++++++++++
>   arch/s390/include/asm/kvm_host.h     |  1 +
>   arch/s390/include/asm/smp.h          |  1 +
>   arch/s390/kernel/smp.c               |  1 +
>   arch/s390/kvm/diag.c                 | 31 +++++++++++++++++++++++---
>   arch/s390/kvm/kvm-s390.c             |  6 +++++
>   arch/s390/kvm/kvm-s390.h             |  8 +++++++
>   7 files changed, 78 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/virt/kvm/s390-diag.rst b/Documentation/virt/kvm/s390-diag.rst
> index eaac4864d3d6..a6371bc4ea90 100644
> --- a/Documentation/virt/kvm/s390-diag.rst
> +++ b/Documentation/virt/kvm/s390-diag.rst
> @@ -84,3 +84,36 @@ If the function code specifies 0x501, breakpoint functions may be performed.
>   This function code is handled by userspace.
>   
>   This diagnose function code has no subfunctions and uses no parameters.
> +
> +
> +DIAGNOSE function code 'X'9C - Voluntary Time Slice Yield
> +---------------------------------------------------------
> +
> +General register 1 contains the target CPU address.
> +
> +In a guest of a hypervisor like LPAR, KVM or z/VM using shared host CPUs,
> +DIAGNOSE with function code 'X'9C may improve system performance by
> +yielding the host CPU on which the guest CPU is running to be assigned
> +to another guest CPU, preferably the logical CPU containing the specified
> +target CPU.
> +
> +
> +DIAG 'X'9C forwarding
> ++++++++++++++++++++++
> +
> +Under KVM, the guest operating system may send a DIAGNOSE code 'X'9C to
> +the host when it fails to acquire a spinlock for a virtual CPU
> +and detects that the host CPU on which the virtual guest CPU owner is
> +assigned to is not running to try to get this host CPU running and
> +consequently the guest virtual CPU running and freeing the lock.
> +
> +However, on the logical partition the real CPU on which the previously
> +targeted host CPU is assign may itself not be running.
> +By forwarding the DIAGNOSE code 'X'9C, initially sent by the guest,
> +from the host to LPAR hypervisor, this one will hopefully schedule
> +the host CPU which will let KVM run the target guest CPU.
> +
> +diag9c_forwarding_hz
> +    KVM kernel parameter allowing to specify the maximum number of DIAGNOSE
> +    'X'9C forwarding per second in the purpose of avoiding a DIAGNOSE 'X'9C
> +    forwarding storm.
> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
> index 527776a1f076..cb19508c22fb 100644
> --- a/arch/s390/include/asm/kvm_host.h
> +++ b/arch/s390/include/asm/kvm_host.h
> @@ -456,6 +456,7 @@ struct kvm_vcpu_stat {
>   	u64 diagnose_44;
>   	u64 diagnose_9c;
>   	u64 diagnose_9c_ignored;
> +	u64 diagnose_9c_forward;
>   	u64 diagnose_258;
>   	u64 diagnose_308;
>   	u64 diagnose_500;
> diff --git a/arch/s390/include/asm/smp.h b/arch/s390/include/asm/smp.h
> index 01e360004481..e317fd4866c1 100644
> --- a/arch/s390/include/asm/smp.h
> +++ b/arch/s390/include/asm/smp.h
> @@ -63,5 +63,6 @@ extern void __noreturn cpu_die(void);
>   extern void __cpu_die(unsigned int cpu);
>   extern int __cpu_disable(void);
>   extern void schedule_mcck_handler(void);
> +void notrace smp_yield_cpu(int cpu);
>   
>   #endif /* __ASM_SMP_H */
> diff --git a/arch/s390/kernel/smp.c b/arch/s390/kernel/smp.c
> index 27c763014114..15e207a671fd 100644
> --- a/arch/s390/kernel/smp.c
> +++ b/arch/s390/kernel/smp.c
> @@ -422,6 +422,7 @@ void notrace smp_yield_cpu(int cpu)
>   	asm volatile("diag %0,0,0x9c"
>   		     : : "d" (pcpu_devices[cpu].address));
>   }
> +EXPORT_SYMBOL(smp_yield_cpu);
>   
>   /*
>    * Send cpus emergency shutdown signal. This gives the cpus the
> diff --git a/arch/s390/kvm/diag.c b/arch/s390/kvm/diag.c
> index 5b8ec1c447e1..34cf41fa6fa2 100644
> --- a/arch/s390/kvm/diag.c
> +++ b/arch/s390/kvm/diag.c
> @@ -150,6 +150,19 @@ static int __diag_time_slice_end(struct kvm_vcpu *vcpu)
>   	return 0;
>   }
>   
> +static unsigned int forward_cnt;
> +static unsigned long cur_slice;
> +
> +static int diag9c_forwarding_overrun(void)
> +{
> +	/* Reset the count on a new slice */
> +	if (time_after(jiffies, cur_slice)) {
> +		cur_slice = jiffies;
> +		forward_cnt = diag9c_forwarding_hz / HZ;
> +	}
> +	return forward_cnt-- ? 1 : 0;

/o\

seems a "<= 0 " has been forgotten here

I send the update asap.

regards,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen