Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1618348

Re: [PATCH RFC 1/6] KVM: fix guest_mode optimization in kvm_make_all_cpus_request()

From James Hogan <james.hogan@imgtec.com>
Newsgroups linux.kernel
Subject Re: [PATCH RFC 1/6] KVM: fix guest_mode optimization in kvm_make_all_cpus_request()
Date 2017-04-06 23:10 +0200
Message-ID <ttmPf-6c4-5@gated-at.bofh.it> (permalink)
References <ttmcx-5J2-7@gated-at.bofh.it> <ttmcx-5J2-19@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


[Multipart message — attachments visible in raw view] - view raw

On Thu, Apr 06, 2017 at 10:20:51PM +0200, Radim Krčmář wrote:
> We have kvm_arch_vcpu_should_kick() to decide whether the target cpu
> needs to be kicked.  The previous condition was wrong, because
> architectures that don't use vcpu->mode would not get interrupts and
> also suboptimal, because it sent IPI in cases where none was necessary.
> 
> The situation is even more convoluted.  MIPS and POWERPC return 1 from
> kvm_arch_vcpu_should_kick(), but implement vcpu->mode for some reason,
> so now they might kick uselessly.  This is not a huge problem.

Whoops. I hadn't spotted kvm_arch_vcpu_should_kick() when I added
vcpu->mode stuff in 4.11... I'm guessing I need to implement that
similar to ARM / x86... though MIPS doesn't use kvm_vcpu_kick() yet.

> 
> s390, on the other hand, never changed vcpu->mode, so it would always be
> OUTSIDE_GUEST_MODE before and therefore didn't send IPIs.
> I don't see a reason why s390 had kvm_make_all_cpus_request() that did
> nothing but set the bit in vcpu->request, so the new behavior seems
> better.
> 
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
>  arch/s390/kvm/kvm-s390.c | 4 +---
>  virt/kvm/kvm_main.c      | 2 +-
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index fd6cd05bb6a7..45b6d9ca5d24 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -2130,9 +2130,7 @@ static void kvm_gmap_notifier(struct gmap *gmap, unsigned long start,
>  
>  int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu)
>  {
> -	/* kvm common code refers to this, but never calls it */
> -	BUG();
> -	return 0;
> +	return 1;
>  }
>  
>  static int kvm_arch_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu,
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index f489167839c4..2389e9c41cd2 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -187,7 +187,7 @@ bool kvm_make_all_cpus_request(struct kvm *kvm, unsigned int req)
>  		smp_mb__after_atomic();
>  
>  		if (cpus != NULL && cpu != -1 && cpu != me &&
> -		      kvm_vcpu_exiting_guest_mode(vcpu) != OUTSIDE_GUEST_MODE)
> +				kvm_arch_vcpu_should_kick(vcpu))

This presumably changes the behaviour on x86, from != OUTSIDE_GUEST_MODE
to == IN_GUEST_MODE. so:
- you'll no longer get IPIs if its in READING_SHADOW_PAGE_TABLES (which
  MIPS also now uses when accessing mappings outside of guest mode and
  depends upon to wait until the old mappings are no longer in use).
- you'll no longer get IPIs if its in EXITING_GUEST_MODE (i.e. if you
  get two of these in quick succession only the first will wait for the
  IPI, which might work as long as they're already serialised but it
  still feels wrong).

But it doesn't seem right to change the kvm_arch_vcpu_should_kick()
implementations to check kvm_vcpu_exiting_guest_mode(vcpu) !=
OUTSIDE_GUEST_MODE to match condition either, since kvm_vcpu_kick()
doesn't seem to need synchronisation, only to know that it won't be
delayed in reaching hypervisor code.

Cheers
James


>  			cpumask_set_cpu(cpu, cpus);
>  	}
>  	if (unlikely(cpus == NULL))
> -- 
> 2.12.0
> 

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 0/6] KVM: towards maintainable kvm_make_all_cpus_request() Radim Krčmář <rkrcmar@redhat.com> - 2017-04-06 22:30 +0200
  [PATCH 4/6] KVM: remove #ifndef CONFIG_S390 around kvm_vcpu_wake_up Radim Krčmář <rkrcmar@redhat.com> - 2017-04-06 22:30 +0200
    Re: [PATCH 4/6] KVM: remove #ifndef CONFIG_S390 around  kvm_vcpu_wake_up Christian Borntraeger <borntraeger@de.ibm.com> - 2017-04-07 13:10 +0200
  [PATCH RFC 1/6] KVM: fix guest_mode optimization in kvm_make_all_cpus_request() Radim Krčmář <rkrcmar@redhat.com> - 2017-04-06 22:30 +0200
    Re: [PATCH RFC 1/6] KVM: fix guest_mode optimization in  kvm_make_all_cpus_request() James Hogan <james.hogan@imgtec.com> - 2017-04-06 23:10 +0200
    Re: [PATCH RFC 1/6] KVM: fix guest_mode optimization in  kvm_make_all_cpus_request() Christian Borntraeger <borntraeger@de.ibm.com> - 2017-04-07 12:50 +0200
  [PATCH 3/6] KVM: x86: use kvm_make_request instead of set_bit Radim Krčmář <rkrcmar@redhat.com> - 2017-04-06 22:30 +0200
    Re: [PATCH 3/6] KVM: x86: use kvm_make_request instead of set_bit David Hildenbrand <david@redhat.com> - 2017-04-07 10:20 +0200
  [PATCH 6/6] KVM: perform a wake_up in kvm_make_all_cpus_request Radim Krčmář <rkrcmar@redhat.com> - 2017-04-06 22:30 +0200
    Re: [PATCH 6/6] KVM: perform a wake_up in kvm_make_all_cpus_request Andrew Jones <drjones@redhat.com> - 2017-04-10 13:20 +0200
  [PATCH 2/6] KVM: use kvm_{test,clear}_request instead of {test,clear}_bit Radim Krčmář <rkrcmar@redhat.com> - 2017-04-06 22:30 +0200
    Re: [PATCH 2/6] KVM: use kvm_{test,clear}_request instead of  {test,clear}_bit Christian Borntraeger <borntraeger@de.ibm.com> - 2017-04-07 13:00 +0200
      Re: [PATCH 2/6] KVM: use kvm_{test,clear}_request instead of  {test,clear}_bit Radim Krčmář <rkrcmar@redhat.com> - 2017-04-07 14:30 +0200
        Re: [PATCH 2/6] KVM: use kvm_{test,clear}_request instead of  {test,clear}_bit Radim Krčmář <rkrcmar@redhat.com> - 2017-04-07 16:10 +0200

csiph-web