Re: [Xen-devel] [PATCH] x86/vm_event: block interrupt injection for sync vm_events

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 12/10/18 4:32 PM, Jan Beulich wrote:
>>>> On 08.12.18 at 21:48, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>> Block interrupts (in vmx_intr_assist()) for the duration of
>> processing a sync vm_event (similarly to the strategy
>> currently used for single-stepping). Otherwise, attempting
>> to emulate an instruction when requested by a vm_event
>> reply may legitimately need to call hvm_inject_page_fault(),
>> which then overwrites the active interrupt in the VMCS.
> 
> I take it that hvm_inject_page_fault() is just the most prominent
> example. If so, wold you mind adding "e.g."?

Indeed. I'll add "e.g.".

>> @@ -239,6 +241,12 @@ void vmx_intr_assist(void)
>>          return;
>>      }
>>  
>> +    /* Block event injection while handling a sync vm_event. */
>> +    if ( unlikely(v->arch.vm_event) &&
>> +         vm_event_check_ring(v->domain->vm_event_monitor) &&
> 
> Is this check really needed?
> 
>> +         v->arch.vm_event->intr_block )
> 
> This part certainly doesn't look to depend on it. Perhaps
> ->intr_block would better be cleared when tearing down a ring?

True, that would look cleaner.

> Also is there really no similar change needed on the AMD (SVM)
> side?

Well, for all practical purposes no, since we don't currently have
NPT-fault vm_events on AMD (I'll have to dig up the old thread to
remember where that got left off), and those are the only events for
which it is sane to reply with "please emulate".

But I see your point, it would be better to add that code for SVM as
well while we're at it rather than have a surprise once everything else
is in place. So I'll add that early return to svm_intr_assist() as well.


Thanks,
Razvan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel