Re: [Xen-devel] [PATCH] x86/vm_event: Added support for VM_EVENT_REASON_INTERRUPT

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 11/09/2016 04:04 PM, Jan Beulich wrote:
>>>> On 09.11.16 at 12:32, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>> On 11/09/2016 01:17 PM, Jan Beulich wrote:
>>>>>> On 09.11.16 at 10:42, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>>>> --- a/xen/arch/x86/hvm/hvm.c
>>>> +++ b/xen/arch/x86/hvm/hvm.c
>>>> @@ -532,11 +532,23 @@ void hvm_do_resume(struct vcpu *v)
>>>>          }
>>>>      }
>>>>  
>>>> -    /* Inject pending hw/sw trap */
>>>> -    if ( v->arch.hvm_vcpu.inject_trap.vector != -1 )
>>>> -    {
>>>> +    /* Inject pending hw/sw trap if there are no other pending 
>>>> interrupts. */
>>>> +    if ( v->arch.hvm_vcpu.inject_trap.vector != -1 && 
>>>> !hvm_event_pending(v) )
>>>>          hvm_inject_trap(&v->arch.hvm_vcpu.inject_trap);
>>>> -        v->arch.hvm_vcpu.inject_trap.vector = -1;
>>>> +
>>>> +    v->arch.hvm_vcpu.inject_trap.vector = -1;
>>>
>>> I don't see why you pull this out of the if() body.
>>
>> That is intended, and covered by the "the patch also fixes the behaviour
>> of the xc_hvm_inject_trap() hypercall, which would lead to
>> non-architectural interrupts overwriting pending (specifically
>> reinjected) architectural ones" part of the patch description.
>>
>> If we couldn't inject the trap because there was a pending event (i.e.
>> the second if() condition, then not setting
>> v->arch.hvm_vcpu.inject_trap.vector to -1 would lead to the trap being
>> kept for injection at the first opportunity - and that could be when the
>> context has changed and we shouldn't inject it anymore. So
>> v->arch.hvm_vcpu.inject_trap.vector is therefore reset either way.
> 
> Ah, that's because you extend the condition. How about you leave
> the condition as is, and only make the actual call conditonal
> upon hvm_event_pending()'s return value? That's also make the
> patch better readable.

Of course, no problem.


Thanks,
Razvan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel