All of lore.kernel.org
 help / color / mirror / Atom feed
From: Roman Bolshakov <r.bolshakov@yadro.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Claudio Fontana <cfontana@suse.de>,
	Richard Henderson <rth@twiddle.net>,
	Eduardo Habkost <ehabkost@redhat.com>,
	Cameron Esfahani <dirty@apple.com>,
	qemu-devel@nongnu.org
Subject: Re: [PATCH 4/8] i386: hvf: Implement CPU kick
Date: Mon, 29 Jun 2020 14:31:07 +0300	[thread overview]
Message-ID: <20200629113107.GL25104@SPB-NB-133.local> (raw)
In-Reply-To: <6508d40b-0142-1b42-2f48-fcd2da66ea4b@redhat.com>

On Thu, Jun 25, 2020 at 08:34:14PM +0200, Paolo Bonzini wrote:
> On 25/06/20 17:57, Roman Bolshakov wrote:
> > So, the kick is not delivered to self and in case if destination cpu is
> > not running. I think it can't interrupt subsequent hv_vcpu_run.
> 
> Yes.
> 
> >> If not, you can reduce a bit the race window by setting a variable in
> >> cpu, like
> >>
> >> 	atomic_set(&cpu->deadline, 0);
> >> 	hv_vcpu_interrupt(...)
> >>
> >> and in the vCPU thread
> >>
> >> 	hv_vcpu_run_until(..., atomic_read(&cpu->deadline));
> >> 	atomic_set(&cpu->deadline, HV_DEADLINE_FOREVER);
> >>
> > 
> > Sure, could you please explain who'll be racing? There's a race if a
> > kick was sent after VMEXIT, right? So essentially we need a way to
> > "requeue" a kick that was received outside of hv_vcpu_run to avoid loss
> > of it?
> 
> Yes.  Note that this is not a new bug, it's pre-existing and it's common
> to all hypervisors except KVM/WHPX.  I mean not the QEMU code, it's the
> kernel APIs that are broken. :)
> 
> One way to do so is to keep the signal, and have the signal handler
> enable the preemption timer (with a deadline of 0) in the pin-based
> interrupt controls.  Hopefully macOS allows that, especially on 10.15+
> where hv_vcpu_run_until probably uses the preemption timer.
> 
> > hv_vcpu_run_until is only available on macOS 10.15+ and we can't use yet
> > because of three release support rule.
> > (https://developer.apple.com/documentation/hypervisor/3181548-hv_vcpu_run_until?language=objc)
> > 
> > BTW, I'm totally okay to send v2 if kicks are lost and/or the patch
> > needs improvements. (and I can address EFER to VMCS Entry Controls
> > synchronization as well)
> > 
> > Paolo, do you know any particular test in kvm-unit-tests that can
> > exhibit the issue?
> 
> No, it's a race and it's extremely rare, but I point it out because it's
> a kernel issue that Apple might want to fix anyway.  It might also be
> (depending on how the kernel side is written) that the next scheduler
> tick will end up unblocking the vCPU and papering over it.
> 

Hi Paolo,

I implemented what you proposed using VMX-preemption timer in Pin-based
controls and regular hv_vcpu_run(). It works fine without noticable
regressions, I'll send that in v2.

hv_vcpu_run_until() was also evaluated on macOS 10.15.5 but it degrades
VM performance significantly compared to explicit setting of
VMX-preepmtion timer value and hv_vcpu_run(). The performance issue was
observed on Broadwell-based MacBook Air and Ivy Bridge-based MacBook
Pro.

macOS 11.0 Beta deprecated hv_vcpu_run() and introduced a special
declaration for hv_vcpu_run_until(), that's not available 10.15 -
HV_DEADLINE_FOREVER (UINT64_MAX, which is bigger than maximum value of
VMX-preeemption counter). Perhaps the performance issue is addressed
there.

Regards,
Roman


  reply	other threads:[~2020-06-29 11:31 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-24 22:58 [PATCH 0/8] Improve synchronization between QEMU and HVF Roman Bolshakov
2020-06-24 22:58 ` [PATCH 1/8] i386: hvf: Set env->eip in macvm_set_rip() Roman Bolshakov
2020-06-24 22:58 ` [PATCH 2/8] i386: hvf: Move synchronize functions to sysemu Roman Bolshakov
2020-06-25  7:09   ` Claudio Fontana
2020-06-24 22:58 ` [PATCH 3/8] i386: hvf: Add hvf_cpu_synchronize_pre_loadvm() Roman Bolshakov
2020-06-24 22:58 ` [PATCH 4/8] i386: hvf: Implement CPU kick Roman Bolshakov
2020-06-25  7:07   ` Claudio Fontana
2020-06-25 10:51     ` Roman Bolshakov
2020-06-25 10:28   ` Paolo Bonzini
2020-06-25 15:57     ` Roman Bolshakov
2020-06-25 18:34       ` Paolo Bonzini
2020-06-29 11:31         ` Roman Bolshakov [this message]
2020-06-29 13:03           ` Paolo Bonzini
2020-06-29 13:29             ` Roman Bolshakov
2020-06-29 13:35               ` Paolo Bonzini
2020-06-29 14:04                 ` Roman Bolshakov
2020-06-29 14:18                   ` Paolo Bonzini
2020-06-30 10:12                     ` Roman Bolshakov
2020-06-30 10:43                       ` Paolo Bonzini
2020-06-24 22:58 ` [PATCH 5/8] i386: hvf: Don't duplicate register reset Roman Bolshakov
2020-06-24 22:58 ` [PATCH 6/8] i386: hvf: Drop hvf_reset_vcpu() Roman Bolshakov
2020-06-25 10:31   ` Paolo Bonzini
2020-06-25 12:36     ` Roman Bolshakov
2020-06-25 13:30       ` Paolo Bonzini
2020-06-25 15:02         ` Roman Bolshakov
2020-06-25 18:26           ` Paolo Bonzini
2020-06-29 12:58         ` Roman Bolshakov
2020-06-24 22:58 ` [PATCH 7/8] i386: hvf: Clean up synchronize functions Roman Bolshakov
2020-06-24 22:58 ` [PATCH 8/8] MAINTAINERS: Add Cameron as HVF co-maintainer Roman Bolshakov
2020-06-25 11:08 ` [PATCH 0/8] Improve synchronization between QEMU and HVF Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200629113107.GL25104@SPB-NB-133.local \
    --to=r.bolshakov@yadro.com \
    --cc=cfontana@suse.de \
    --cc=dirty@apple.com \
    --cc=ehabkost@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.