Re: [RFC v3 2/8] cpus: prepare new CpusAccel cpu accelerator interface

[Claudio Fontana]

Hi all,

could you give a check to this detail, marked as a comment here?

While doing the refactoring and looking at the history,
I _think_ I noticed something that could be wrong related to whpx and hax,

and I marked this as a comment. Maybe Paolo?


On 8/3/20 11:05 AM, Claudio Fontana wrote:
[...]

  
> -static void qemu_wait_io_event(CPUState *cpu)
> +void qemu_wait_io_event(CPUState *cpu)
>  {
>      bool slept = false;
>  
> @@ -437,7 +538,8 @@ static void qemu_wait_io_event(CPUState *cpu)
>      }
>  
>  #ifdef _WIN32
> -    /* Eat dummy APC queued by qemu_cpu_kick_thread.  */
> +    /* Eat dummy APC queued by qemu_cpu_kick_thread. */
> +    /* NB!!! Should not this be if (hax_enabled)? Is this wrong for whpx? */
>      if (!tcg_enabled()) {
>          SleepEx(0, TRUE);
>      }


Looking at the history here, I think this should be if (hax_enabled());
this check was added at a time when whpx did not exist, so I _think_ there 
might have been an assumption here
that !tcg_enabled() on windows means actually hax_enabled() for eating this 
dummy APC.

Probably it does not cause problems, because whpx does not end up calling 
qemu_wait_io_event,
instead it calls qemu_wait_io_event_common. But it would be more expressive to 
use if (hax_enabled()) I think.

Could be separately patched.. relevant commits in history follow.

Thanks,

Claudio


commit db08b687cdd5319286665aabd34f82665630416f
Author: Paolo Bonzini <pbonzini@redhat.com>
Date:   Thu Jan 11 13:53:12 2018 +0100

    cpus: unify qemu_*_wait_io_event
    
    Except for round-robin TCG, every other accelerator is using more or
    less the same code around qemu_wait_io_event_common.  The exception
    is HAX, which also has to eat the dummy APC that is queued by
    qemu_cpu_kick_thread.
    
    We can add the SleepEx call to qemu_wait_io_event under "if
    (!tcg_enabled())", since that is the condition that is used in
    qemu_cpu_kick_thread, and unify the function for KVM, HAX, HVF and
    multi-threaded TCG.  Single-threaded TCG code can also be simplified
    since it is only used in the round-robin, sleep-if-all-CPUs-idle case.
    
    Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>


commit 19306806ae30b7fb5fe61a9130c6995402acad00
Author: Justin Terry (VM) <juterry@microsoft.com>
Date:   Mon Jan 22 13:07:49 2018 -0800

    Add the WHPX acceleration enlightenments
    
    Implements the WHPX accelerator cpu enlightenments to actually use the 
whpx-all
    accelerator on Windows platforms.
    
    Signed-off-by: Justin Terry (VM) <juterry@microsoft.com>
    Message-Id: <1516655269-1785-5-git-send-email-juterry@microsoft.com>
    [Register/unregister VCPU thread with RCU. - Paolo]
    Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

commit b0cb0a66d6d535112aa513568ef21dcb1ad283ed
Author: Vincent Palatin <vpalatin@chromium.org>
Date:   Tue Jan 10 11:59:57 2017 +0100

    Plumb the HAXM-based hardware acceleration support
    
    Use the Intel HAX is kernel-based hardware acceleration module for
    Windows (similar to KVM on Linux).
    
    Based on the "target/i386: Add Intel HAX to android emulator" patch
    from David Chou <david.j.chou@intel.com>
    
    Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
    Message-Id: 
<7b9cae28a0c379ab459c7a8545c9a39762bd394f.1484045952.git.vpalatin@chromium.org>
    [Drop hax_populate_ram stub. - Paolo]
    Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>