[PATCH 04/10] KVM: s390: add metric vcpus_in_sie to struct kvm_arch

[PATCH 04/10] KVM: s390: add metric vcpus_in_sie to struct kvm_arch

[Michael Mueller]

This metric will allow to identify how many vcpus are currently
running for a given kvm in SIE context. Its value is between 0 and
the number of vcpus defined for the kvm guest, but never lager than
the number of cpus available to the KVM host in total.

This metric is required to decide if the GISA IAM has to be restored
from the kvm arch field of the guest. That will be the case when no
vcpu is in SIE context. (vcpus_in_sie == 0)

Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
Reviewed-by: Pierre Morel <pmorel@linux.ibm.com>
---
 arch/s390/include/asm/kvm_host.h | 1 +
 arch/s390/kvm/kvm-s390.c         | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index 01be3b666b11..a9ec2da399b5 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -839,6 +839,7 @@ struct kvm_arch{
 	DECLARE_BITMAP(cpu_feat, KVM_S390_VM_CPU_FEAT_NR_BITS);
 	struct kvm_s390_gisa *gisa;
 	int gib_in_use;
+	atomic_t vcpus_in_sie;
 };
 
 #define KVM_HVA_ERR_BAD		(-1UL)
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index c75b01856092..213d16e5b16a 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -3358,6 +3358,8 @@ static int vcpu_pre_run(struct kvm_vcpu *vcpu)
 		kvm_s390_patch_guest_per_regs(vcpu);
 	}
 
+	atomic_inc(&vcpu->kvm->arch.vcpus_in_sie);
+
 	vcpu->arch.sie_block->icptcode = 0;
 	cpuflags = atomic_read(&vcpu->arch.sie_block->cpuflags);
 	VCPU_EVENT(vcpu, 6, "entering sie flags %x", cpuflags);
@@ -3417,6 +3419,8 @@ static int vcpu_post_run(struct kvm_vcpu *vcpu, int exit_reason)
 	vcpu->run->s.regs.gprs[14] = vcpu->arch.sie_block->gg14;
 	vcpu->run->s.regs.gprs[15] = vcpu->arch.sie_block->gg15;
 
+	atomic_dec(&vcpu->kvm->arch.vcpus_in_sie);
+
 	if (exit_reason == -EINTR) {
 		VCPU_EVENT(vcpu, 3, "%s", "machine check");
 		sie_page = container_of(vcpu->arch.sie_block,
-- 
2.17.0

Re: [PATCH 04/10] KVM: s390: add metric vcpus_in_sie to struct kvm_arch

[Michael Mueller]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 3016 bytes --]



On 08.11.18 02:05, Halil Pasic wrote:
>
> On 11/07/2018 10:52 PM, Pierre Morel wrote:
>>>>>> Mimu,�we�just�spoke�about�--�thus�a�reminder�for�us,�and�a�FYI�for
>>>>>> the�rest.�vcpus_in_sie�!=�0�does�not�necessarily�mean�that�FW�can
>>>>>> deliver�GISA�interrupts�to�the�guest�(and�thus�we�don't�have�to
>>>>>> dispatch�a�suitable�vcpu):�if�all�cpus�in�SIE�mask�IO�interrupts�it
>>>>>> does�not�really�matter�that�they�are�in�SIE.�I.e.�we�should�still�get
>>>>>> alerted�and�try�to�dispatch�an�eligible�vcpu.
>>>> If�I�understood�well,�I�do�not�agree.
>>> I�don't�think�you�understood�well.
>>>
>>>> If�all�the�vCPU�have�ISC�masked�in�CR6�we�do�not�need�to�dispatch�the�vCPU,�running�or�not.
>>>> If�running,
>>>> The�first�vCPU�that�will�load�CR6�with�at�least�one�bit�of�the�ISC�being�one�will�be�scheduled�by�the�firmware.
>>>>
>>> Let�me�help�you�with�an�example.�Our�guest�has�3�vCPUs.�One�(CPU1)�of�those�is�doing�something
>>> very�important�real-time-ish�stuff�and�is�currently�in�SIE�with�all�maskable�interrupts
>>> shut�off�(the�stuff�is�very�important).�The�other�two�vCPUs�(CPU2�and�CPU3)�are�open�for�IO
>>> interrupts�but�idle,�that�is�not�in�SIE.
>>>
>>> AFAIU�we�should�opt�for�getting�alerted�and�kicking�CPU2�or�CPU3�in�this�case,�as�we�can
>>> not�assume�that�CPU1�will�open�up�for�interrupts�any�time�soon.�I�read�your�text�like,
>>> you�think�we�can�make�that�assumption.�Did�I�misunderstand�you?�If�not,�why�do�you�think
>>> we�can�make�that�assumption?
>> OK, I undertand, you want to make an optimization and kick one idle CPU not�having�the�interrupt�masked.
>> Seems�right�for�me,�but�I�think�we�could�implement�this�later.
> I'm not sure if it is just an optimization or not. If we don't have another mechanism that
> will kick one of the two form the example, the guest will loose initiative. And that is a bug
> because it could wait for the IRQ forever. If we do have a different mechanism that we are going to
> keep on relying, then everything we do around GIB is just an optimization in the same sense
> as the point under discussion. I have not looked enough at the proposed code to tell if guest
> looses initiative or not.
>
> Important thing is we are roughly on the same page I think.
>
> Regards,
> Halil
I have looked into this scenario described by you and spend some time 
dealing with it. But before I will
add any potential unused code I'm asking you to provide a piece of code 
producing this scenario. Up to
then we shall go with the option provided in patch series v2.

Michael