[GIT PULL 04/23] KVM: s390: rewrite vcpu_post_run and drop out early

[Christian Borntraeger]

From: David Hildenbrand 

Let's rewrite this function to better reflect how we actually handle
exit_code. By dropping out early we can save a few cycles. This
especially speeds up sie exits caused by host irqs.

Also, let's move the special -EOPNOTSUPP for intercepts to
the place where it belongs and convert it to -EREMOTE.

Reviewed-by: Dominik Dingel 
Reviewed-by: Cornelia Huck 
Signed-off-by: David Hildenbrand 
Signed-off-by: Christian Borntraeger 
---
 arch/s390/kvm/intercept.c |  7 +++---
 arch/s390/kvm/kvm-s390.c  | 59 +--
 2 files changed, 24 insertions(+), 42 deletions(-)

diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c
index b4a5aa1..d53c107 100644
--- a/arch/s390/kvm/intercept.c
+++ b/arch/s390/kvm/intercept.c
@@ -54,9 +54,6 @@ void kvm_s390_rewind_psw(struct kvm_vcpu *vcpu, int ilc)
 static int handle_noop(struct kvm_vcpu *vcpu)
 {
switch (vcpu->arch.sie_block->icptcode) {
-   case 0x0:
-   vcpu->stat.exit_null++;
-   break;
case 0x10:
vcpu->stat.exit_external_request++;
break;
@@ -338,8 +335,10 @@ static int handle_partial_execution(struct kvm_vcpu *vcpu)
 
 int kvm_handle_sie_intercept(struct kvm_vcpu *vcpu)
 {
+   if (kvm_is_ucontrol(vcpu->kvm))
+   return -EOPNOTSUPP;
+
switch (vcpu->arch.sie_block->icptcode) {
-   case 0x00:
case 0x10:
case 0x18:
return handle_noop(vcpu);
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 8465892..5c36c8e 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2071,8 +2071,6 @@ static int vcpu_post_run_fault_in_sie(struct kvm_vcpu 
*vcpu)
 
 static int vcpu_post_run(struct kvm_vcpu *vcpu, int exit_reason)
 {
-   int rc = -1;
-
VCPU_EVENT(vcpu, 6, "exit sie icptcode %d",
   vcpu->arch.sie_block->icptcode);
trace_kvm_s390_sie_exit(vcpu, vcpu->arch.sie_block->icptcode);
@@ -2080,40 +2078,35 @@ static int vcpu_post_run(struct kvm_vcpu *vcpu, int 
exit_reason)
if (guestdbg_enabled(vcpu))
kvm_s390_restore_guest_per_regs(vcpu);
 
-   if (exit_reason >= 0) {
-   rc = 0;
+   memcpy(>run->s.regs.gprs[14], >arch.sie_block->gg14, 16);
+
+   if (vcpu->arch.sie_block->icptcode > 0) {
+   int rc = kvm_handle_sie_intercept(vcpu);
+
+   if (rc != -EOPNOTSUPP)
+   return rc;
+   vcpu->run->exit_reason = KVM_EXIT_S390_SIEIC;
+   vcpu->run->s390_sieic.icptcode = vcpu->arch.sie_block->icptcode;
+   vcpu->run->s390_sieic.ipa = vcpu->arch.sie_block->ipa;
+   vcpu->run->s390_sieic.ipb = vcpu->arch.sie_block->ipb;
+   return -EREMOTE;
+   } else if (exit_reason != -EFAULT) {
+   vcpu->stat.exit_null++;
+   return 0;
} else if (kvm_is_ucontrol(vcpu->kvm)) {
vcpu->run->exit_reason = KVM_EXIT_S390_UCONTROL;
vcpu->run->s390_ucontrol.trans_exc_code =
current->thread.gmap_addr;
vcpu->run->s390_ucontrol.pgm_code = 0x10;
-   rc = -EREMOTE;
-
+   return -EREMOTE;
} else if (current->thread.gmap_pfault) {
trace_kvm_s390_major_guest_pfault(vcpu);
current->thread.gmap_pfault = 0;
-   if (kvm_arch_setup_async_pf(vcpu)) {
-   rc = 0;
-   } else {
-   gpa_t gpa = current->thread.gmap_addr;
-   rc = kvm_arch_fault_in_page(vcpu, gpa, 1);
-   }
+   if (kvm_arch_setup_async_pf(vcpu))
+   return 0;
+   return kvm_arch_fault_in_page(vcpu, current->thread.gmap_addr, 
1);
}
-
-   if (rc == -1)
-   rc = vcpu_post_run_fault_in_sie(vcpu);
-
-   memcpy(>run->s.regs.gprs[14], >arch.sie_block->gg14, 16);
-
-   if (rc == 0) {
-   if (kvm_is_ucontrol(vcpu->kvm))
-   /* Don't exit for host interrupts. */
-   rc = vcpu->arch.sie_block->icptcode ? -EOPNOTSUPP : 0;
-   else
-   rc = kvm_handle_sie_intercept(vcpu);
-   }
-
-   return rc;
+   return vcpu_post_run_fault_in_sie(vcpu);
 }
 
 static int __vcpu_run(struct kvm_vcpu *vcpu)
@@ -2233,18 +2226,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, 
struct kvm_run *kvm_run)
rc = 0;
}
 
-   if (rc == -EOPNOTSUPP) {
-   /* intercept cannot be handled in-kernel, prepare kvm-run */
-   kvm_run->exit_reason = KVM_EXIT_S390_SIEIC;
-   kvm_run->s390_sieic.icptcode = vcpu->arch.sie_block->icptcode;
-   

Re: [GIT PULL 04/23] KVM: s390: rewrite vcpu_post_run and drop out early

[Paolo Bonzini]

On 02/12/2015 12:06, Christian Borntraeger wrote:
> + memcpy(>run->s.regs.gprs[14], >arch.sie_block->gg14, 16);

This is preexisting but... boy it's ugly. :)

Do you gain much over the simpler

vcpu->run->s.regs.gprs[14] = vcpu->arch.sie_block->gg14;
vcpu->run->s.regs.gprs[15] = vcpu->arch.sie_block->gg15;

?

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL 04/23] KVM: s390: rewrite vcpu_post_run and drop out early

[Christian Borntraeger]

On 12/02/2015 01:20 PM, Paolo Bonzini wrote:
> 
> 
> On 02/12/2015 12:06, Christian Borntraeger wrote:
>> +memcpy(>run->s.regs.gprs[14], >arch.sie_block->gg14, 16);
> 
> This is preexisting but... boy it's ugly. :)
> 
> Do you gain much over the simpler
> 
>   vcpu->run->s.regs.gprs[14] = vcpu->arch.sie_block->gg14;
>   vcpu->run->s.regs.gprs[15] = vcpu->arch.sie_block->gg15;
> 

Its just legacy code from the old days.
There is a difference, but it seems to a missed opportunity from gcc

vcpu->arch.sie_block->gg14 = vcpu->run->s.regs.gprs[14];
839c:   e3 30 f0 b8 00 04   lg  %r3,184(%r15)
83a2:   e3 10 32 40 00 04   lg  %r1,576(%r3)
83a8:   e3 20 30 80 00 04   lg  %r2,128(%r3)
83ae:   e3 20 21 b8 00 04   lg  %r2,440(%r2)
83b4:   e3 20 10 a0 00 24   stg %r2,160(%r1)
vcpu->arch.sie_block->gg15 = vcpu->run->s.regs.gprs[15];
83ba:   e3 10 32 40 00 04   lg  %r1,576(%r3)
83c0:   e3 20 30 80 00 04   lg  %r2,128(%r3)
83c6:   e3 20 21 c0 00 04   lg  %r2,448(%r2)
83cc:   e3 20 10 a8 00 24   stg %r2,168(%r1)

gcc seems to reuse and reload %r2 and %r3, maybe register pressure.



the memcpy gives

memcpy(>arch.sie_block->gg14, >run->s.regs.gprs[14], 16);
839c:   e3 30 f0 b8 00 04   lg  %r3,184(%r15)
83a2:   e3 10 32 40 00 04   lg  %r1,576(%r3)
83a8:   e3 20 30 80 00 04   lg  %r2,128(%r3)
83ae:   d2 0f 10 a0 21 b8   mvc 160(16,%r1),440(%r2)

I will prepare a patch and do my usual micro benchmark. Unless
things get much worse I will schedule this for the next pull.

Christian


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL 04/23] KVM: s390: rewrite vcpu_post_run and drop out early

[Paolo Bonzini]

On 02/12/2015 14:04, Christian Borntraeger wrote:
>> > Do you gain much over the simpler
>> > 
>> >vcpu->run->s.regs.gprs[14] = vcpu->arch.sie_block->gg14;
>> >vcpu->run->s.regs.gprs[15] = vcpu->arch.sie_block->gg15;
>> > 
> Its just legacy code from the old days.
> There is a difference, but it seems to a missed opportunity from gcc
> 
> vcpu->arch.sie_block->gg14 = vcpu->run->s.regs.gprs[14];
> 839c:   e3 30 f0 b8 00 04   lg  %r3,184(%r15)
> 83a2:   e3 10 32 40 00 04   lg  %r1,576(%r3)
> 83a8:   e3 20 30 80 00 04   lg  %r2,128(%r3)
> 83ae:   e3 20 21 b8 00 04   lg  %r2,440(%r2)
> 83b4:   e3 20 10 a0 00 24   stg %r2,160(%r1)
> vcpu->arch.sie_block->gg15 = vcpu->run->s.regs.gprs[15];
> 83ba:   e3 10 32 40 00 04   lg  %r1,576(%r3)
> 83c0:   e3 20 30 80 00 04   lg  %r2,128(%r3)
> 83c6:   e3 20 21 c0 00 04   lg  %r2,448(%r2)
> 83cc:   e3 20 10 a8 00 24   stg %r2,168(%r1)
> 
> gcc seems to reuse and reload %r2 and %r3, maybe register pressure.

More likely to be -fno-strict-aliasing. :(

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL 04/23] KVM: s390: rewrite vcpu_post_run and drop out early

[Christian Borntraeger]

On 12/02/2015 02:05 PM, Paolo Bonzini wrote:
> 
> 
> On 02/12/2015 14:04, Christian Borntraeger wrote:
 Do you gain much over the simpler

vcpu->run->s.regs.gprs[14] = vcpu->arch.sie_block->gg14;
vcpu->run->s.regs.gprs[15] = vcpu->arch.sie_block->gg15;

>> Its just legacy code from the old days.
>> There is a difference, but it seems to a missed opportunity from gcc
>>
>> vcpu->arch.sie_block->gg14 = vcpu->run->s.regs.gprs[14];
>> 839c:   e3 30 f0 b8 00 04   lg  %r3,184(%r15)
>> 83a2:   e3 10 32 40 00 04   lg  %r1,576(%r3)
>> 83a8:   e3 20 30 80 00 04   lg  %r2,128(%r3)
>> 83ae:   e3 20 21 b8 00 04   lg  %r2,440(%r2)
>> 83b4:   e3 20 10 a0 00 24   stg %r2,160(%r1)
>> vcpu->arch.sie_block->gg15 = vcpu->run->s.regs.gprs[15];
>> 83ba:   e3 10 32 40 00 04   lg  %r1,576(%r3)
>> 83c0:   e3 20 30 80 00 04   lg  %r2,128(%r3)
>> 83c6:   e3 20 21 c0 00 04   lg  %r2,448(%r2)
>> 83cc:   e3 20 10 a8 00 24   stg %r2,168(%r1)
>>
>> gcc seems to reuse and reload %r2 and %r3, maybe register pressure.
> 
> More likely to be -fno-strict-aliasing. :(

Yes its indeed the aliasing. 

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html