Re: [PATCH 2/3] i386: hvf: In kick_vcpu use hv_vcpu_interrupt to force exit

[Roman Bolshakov <roman@roolebo.dev>]

On Sun, Oct 08, 2023 at 08:39:08PM +0200, Phil Dennis-Jordan wrote:
> On Sun, 8 Oct 2023 at 20:23, Roman Bolshakov <roman@roolebo.dev> wrote:
> 
> > On Fri, Sep 22, 2023 at 04:09:13PM +0200, Phil Dennis-Jordan wrote:
> > > When interrupting a vCPU thread, this patch actually tells the
> > hypervisor to
> > > stop running guest code on that vCPU.
> > >
> > > Calling hv_vcpu_interrupt actually forces a vCPU exit, analogously to
> > > hv_vcpus_exit on aarch64.
> > >
> > > Previously, hvf_kick_vcpu_thread relied upon hv_vcpu_run returning very
> > > frequently, including many spurious exits, which made it less of a
> > problem that
> > > nothing was actively done to stop the vCPU thread running guest code.
> > > The newer, more efficient hv_vcpu_run_until exits much more rarely, so a
> > true
> > > "kick" is needed.
> > >
> >
> 
> Hi Roman,
> 
> Thanks for the review and test of this patch and the preceding one!
> 

My pleasure. Thanks for submitting the patch and confirming the need of
the feature on x86 HVF.

> > I see severe performance regression with the patch on a Windows XP
> > guest. The display is not refreshed properly like a broken LVDS panel,
> > only some horizontal lines appear on it.
> 
> 
> I assume that's with patch 3/3 applied as well? The fact you've
> repro'd it with just these patch would explain why I've not been able
> to fix it on the APIC side…
> 

Yes, I applied with patch 3/3 and then retested only with the first two
patches.

> > FWIW. I recall a few years ago I submitted a similar patch that does
> > something similar but addresses a few more issues:
> >
> > https://patchwork.kernel.org/project/qemu-devel/patch/20200729124832.79375-1-r.bolshakov@yadro.com/
> >
> > I don't remember why it never got merged.
> >
> 
> Looks like the VM kick might be a more complex undertaking than I was
> anticipating. I'll try to repro the problem you ran into, and then look
> over your original patch and make sense of it. Hopefully an updated version
> of your 'kick' implementation will work well in combination with the
> newer hv_vcpu_run_until() API from patch 3/3. I'll keep you posted.
> 

Apparently I left a note that some interrupts weren't delivered even
with my patch and I was not able figure out the reason back then. I had
another attempt to debug this two weeks later after I submitted v4 and I
can find a WIP branch on github where I added a Debug Registers support
patch and some tracepoints:

https://github.com/qemu/qemu/compare/master...roolebo:qemu:hvf-debug-kick

Perhaps that's where we should start from besides the obvious need of
rebase.

With regards to hv_vcpu_run_until() I can find the following thread:
https://lists.nongnu.org/archive/html/qemu-devel/2020-06/msg09468.html

> 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.

All discussion with Paolo might be helpful, particurlarly:
https://lists.nongnu.org/archive/html/qemu-devel/2020-06/msg09893.html

> So, I've tried Big Sur Beta and it has exactly the same performance
> issue with hv_vcpu_run_until() while hv_vcpu_run() works as good as it
> worked on 10.15.5. I've submitted FB7827341 to Apple wrt the issue.

In November 2020, Apple responded to FB7827341 that there's an issue on
QEMU side.

Regards,
Roman