Re: [PATCH v2 5/5] hvf: save away type as well as vector so we can reinject them

[Cameron Esfahani]

I added some asserts to our internal version of QEMU.  It's a few months off of 
master and, specifically, doesn't have fd13f23b8c95311eff74426921557eee592b0ed3.

With the previous version of hvf_inject_interrupts(), before our fix, the code 
looked like the following:

>     if (env->interrupt_injected != -1) {
>         vector = env->interrupt_injected;
>         intr_type = VMCS_INTR_T_SWINTR;
>     } else if (env->exception_injected != -1) {


What we were seeing was, under heavy loads, running many concurrent macOS VMs, 
that we would get spurious interrupts.  Tracking it down, we discovered that 
VMCS_INTR_T_SWINTR was getting injected when VMCS_INTR_T_HWINTR was expected.

If I take our proposed patch code, which built on top of master from a few days 
ago, and has fd13f23b8c95311eff74426921557eee592b0ed3,  and add an assert, like 
the following:

>     if (env->interrupt_injected != -1) {
>         /* Type and vector are both saved in interrupt_injected. */
>         vector = env->interrupt_injected & VMCS_IDT_VEC_VECNUM;
>         intr_type = env->interrupt_injected & VMCS_IDT_VEC_TYPE;
>         if (VMCS_INTR_T_SWINTR != intr_type) {
>             printf("VMCS_INTR_T_SWINTR (%x) != intr_type (%llx)\n", 
> VMCS_INTR_T_SWINTR, intr_type);
>             assert(VMCS_INTR_T_SWINTR == intr_type);
>         }
>     } else if (env->exception_nr != -1) {


Then we will see the assert trigger and get the following output:

> VMCS_INTR_T_SWINTR (400) != intr_type (0)
> Assertion failed: (VMCS_INTR_T_SWINTR == intr_type), function 
> hvf_inject_interrupts, file qemu_upstream/target/i386/hvf/x86hvf.c, line 362.


So, as far as I can see, the proposed changes are still necessary.

Cameron Esfahani
address@hidden

"Americans are very skilled at creating a custom meaning from something that's 
mass-produced."

Ann Powers


> On Nov 26, 2019, at 12:04 PM, Cameron Esfahani via <address@hidden> wrote:
> 
> Our test case was booting many concurrent macOS VMs under heavy system load.  
> I don't know if I could create one to replicate that.
> 
> Cameron Esfahani
> address@hidden
> 
> "In the elder days of Art, Builders wrought with greatest care each minute 
> and unseen part; For the gods see everywhere."
> 
> "The Builders", H. W. Longfellow
> 
> 
> 
>> On Nov 25, 2019, at 2:26 AM, Paolo Bonzini <address@hidden> wrote:
>> 
>> On 24/11/19 21:05, Cameron Esfahani wrote:
>>> Save away type as well as vector in hvf_store_events() so we can
>>> correctly reinject both in hvf_inject_interrupts().
>>> 
>>> Make sure to clear ins_len and has_error_code when ins_len isn't
>>> valid and error_code isn't set.
>> 
>> Do you have a testcase for this?  (I could guess it's about the INT1
>> instruction).
>> 
>> Paolo
>> 
> 
>