Re: [RFC Patch v2 45/45] x86/hvm: Always set pending event injection when loading VMC[BS] state.

[Wen Congyang <wency@cn.fujitsu.com>]

At 08/27/2014 10:58 PM, Aravind Gopalakrishnan Write:
> On 8/26/2014 7:46 PM, Wen Congyang wrote:
>> At 08/27/2014 12:02 AM, Jan Beulich Write:
>>>>>> On 08.08.14 at 09:01, <wency@cn.fujitsu.com> wrote:
>>>> In colo mode, secondary vm is running, so VM_ENTRY_INTR_INFO may
>>>> valid before restoring vmcs. If there is no pending event after
>>>> restoring vm, we should clear it.
>>>>
>>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>>>
>>>> Also clear pending software exceptions.
>>>> Copy the fix to SVM as well.
>>>>
>>>> Signed-off-by: Tim Deegan <tim@xen.org>
>>> I only now realized that it's no surprise we're not getting acks from
>>> the VMX maintainers on this - the majority of them wasn't Cc-ed.
>>> Now done, but please take care to do so yourself in the future.
>>>
>>> As to the SVM maintainers - Ping (I Cc-ed you on an earlier reply)?
>> Thanks for doing this.
>> I have repost it in the bugfix patchset, and cc vmx and svm maintainers
>>
> 
> Hi,
> Apologies for the delay.
> 
> As for the svm changes, the patch seems fairly straightforward and harmless.
> However, I am not familiar with 'colo mode', so I'm not sure I understand the problem..

In colo mode, secondary vm runs like this:
1. suspend
2. update the vm's state(All state is transfered from primary)
3. resume

Before resuming secondary vm, it is the same with primary vm. Because the secondary vm
is running before step1, so VM_ENTRY_INTR_INFO may be valid before resuming
secondary vm(step3). If there is no pending event after resuming secondary vm,
and we don't clear it, vmentry will fail(I only test vmx). I don't know the behavior
in svm. This part of patch is wrote by Tim Deegan.

> 
> Is this a 'fix' we need so that we don't duplicate a pending software interrupt on the secondary VM?
> Is there a way to test this?

I don't know any other way except colo to test it.

Thanks
Wen Congyang

> 
> Thanks,
> -Aravind.
> 
>>>> ---
>>>>   xen/arch/x86/hvm/svm/svm.c | 16 +++++++++-------
>>>>   xen/arch/x86/hvm/vmx/vmx.c | 25 ++++++++++++-------------
>>>>   2 files changed, 21 insertions(+), 20 deletions(-)
>>>>
>>>> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
>>>> index 71b8a6a..f7a0cb8 100644
>>>> --- a/xen/arch/x86/hvm/svm/svm.c
>>>> +++ b/xen/arch/x86/hvm/svm/svm.c
>>>> @@ -321,16 +321,18 @@ static int svm_vmcb_restore(struct vcpu *v, struct
>>>> hvm_hw_cpu *c)
>>>>           vmcb_set_h_cr3(vmcb, pagetable_get_paddr(p2m_get_pagetable(p2m)));
>>>>       }
>>>>   -    if ( c->pending_valid )
>>>> +    if ( c->pending_valid
>>>> +         && hvm_event_needs_reinjection(c->pending_type, c->pending_vector) )
>>>>       {
>>>>           gdprintk(XENLOG_INFO, "Re-injecting %#"PRIx32", %#"PRIx32"\n",
>>>>                    c->pending_event, c->error_code);
>>>> -
>>>> -        if ( hvm_event_needs_reinjection(c->pending_type, c->pending_vector) )
>>>> -        {
>>>> -            vmcb->eventinj.bytes = c->pending_event;
>>>> -            vmcb->eventinj.fields.errorcode = c->error_code;
>>>> -        }
>>>> +        vmcb->eventinj.bytes = c->pending_event;
>>>> +        vmcb->eventinj.fields.errorcode = c->error_code;
>>>> +    }
>>>> +    else
>>>> +    {
>>>> +        vmcb->eventinj.bytes = 0;
>>>> +        vmcb->eventinj.fields.errorcode = 0;
>>>>       }
>>>>         vmcb->cleanbits.bytes = 0;
>>>> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
>>>> index fb65c7d..5f143c0 100644
>>>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>>>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>>>> @@ -509,23 +509,22 @@ static int vmx_vmcs_restore(struct vcpu *v, struct
>>>> hvm_hw_cpu *c)
>>>>         __vmwrite(GUEST_DR7, c->dr7);
>>>>   -    vmx_vmcs_exit(v);
>>>> -
>>>> -    paging_update_paging_modes(v);
>>>> -
>>>> -    if ( c->pending_valid )
>>>> +    if ( c->pending_valid
>>>> +         && hvm_event_needs_reinjection(c->pending_type, c->pending_vector) )
>>>>       {
>>>>           gdprintk(XENLOG_INFO, "Re-injecting %#"PRIx32", %#"PRIx32"\n",
>>>>                    c->pending_event, c->error_code);
>>>> -
>>>> -        if ( hvm_event_needs_reinjection(c->pending_type, c->pending_vector) )
>>>> -        {
>>>> -            vmx_vmcs_enter(v);
>>>> -            __vmwrite(VM_ENTRY_INTR_INFO, c->pending_event);
>>>> -            __vmwrite(VM_ENTRY_EXCEPTION_ERROR_CODE, c->error_code);
>>>> -            vmx_vmcs_exit(v);
>>>> -        }
>>>> +        __vmwrite(VM_ENTRY_INTR_INFO, c->pending_event);
>>>> +        __vmwrite(VM_ENTRY_EXCEPTION_ERROR_CODE, c->error_code);
>>>>       }
>>>> +    else
>>>> +    {
>>>> +        __vmwrite(VM_ENTRY_INTR_INFO, 0);
>>>> +        __vmwrite(VM_ENTRY_EXCEPTION_ERROR_CODE, 0);
>>>> +    }
>>>> +    vmx_vmcs_exit(v);
>>>> +
>>>> +    paging_update_paging_modes(v);
>>>>         return 0;
>>>>   }
>>>> -- 
>>>> 1.9.3
>>>>
>>>>
>>>> _______________________________________________
>>>> Xen-devel mailing list
>>>> Xen-devel@lists.xen.org
>>>> http://lists.xen.org/xen-devel
>>>
>>>
>>> .
>>>
> 
> .
>