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

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 12/13/18 12:15 PM, Razvan Cojocaru wrote:
> On 12/13/18 2:04 PM, Julien Grall wrote:
> > Hi,
> >
> > On 12/13/18 8:03 AM, Razvan Cojocaru wrote:
> > > On 12/13/18 8:54 AM, Tian, Kevin wrote:
> > > > > From: Razvan Cojocaru [mailto:rcojocaru@xxxxxxxxxxxxxxx]
> > > > > Sent: Tuesday, December 11, 2018 8:33 PM
> > > > >
> > > > > > In any case, I think you want to rename the function and/or document
> > > > > > more that expected behavior.
> > > > > You're right, I should probably rename that function / variable to
> > > > > better reflect what it signifies - that sync vm_event processing is in
> > > > > progress. For VMX and SVM, that simply means that interrupts will be
> > > > > blocked, and the value of the variable will be correct and possibly
> > > > > useful for ARM as well.
> > > > what about vm_event_block_interrupt_injection? in that case
> > > > it's injection instead of interrupt itself being blocked. blocking
> > > > injection should mean same thing cross archs?
> > Why would you want to block all interrupts injections? When I looked at
> > the details, it feels more you want to block exceptions.
> >
> > I can see use for blocking exception on Arm, blocking all the interrupts
> > is likely going to bring more issues than solving anything.
> >
> > So a better name would be vm_event_block_exception_injection.
> I'd like to block the writing of anything, by vmx_intr_assist(), into
> VM_ENTRY_INTR_INFO, because an emulation attempt that happens
> post-vmx_intr_assist() (because the vm_event client application has
> requested it) may write an exception of its own there.
>
> Since vmx_intr_assist() is called on VMX between the time of sending out
> the vm_event and the emulation (which happens in
> hvm_vm_event_do_resume()), we want to block everything that it may write
> in the VMCS until the emulation is done. I think that's more than just
> exceptions.
I don't know in details how x86 virtualization works, so it is a bit 
hard to comment on that. However, it feels to me that you are 
introducing in common code a function that will workaround an 
architecture specific problem.
Can you try to explain it in agnostic word?

To expand what I said above, I think it is reasonable to request 
blocking exception (e.g page-fault...) because they can be generated by 
an instruction. However, all interrupts generated by the interrupt 
controller (e.g device, IPI..) should not be blocked.
AFAIU your description, it is the same path to handle the two on x86, right?

On Arm, there are 2 distinct paths, interrupt generated by the interrupt 
controller are queued. The exceptions will be generated using multiple 
different paths. Yet exceptions can still override each other.
I can't see any reason for Arm to block interrupt generated by the 
interrupt controller. This would actually be dangerous due to the way we 
handle them in Xen currently. Instead we may want to block the exception 
as they can be generated by an instruction.
> I've probably been confusing when I was talking about the exceptions
> that emulating the current instruction may trigger - we don't want to
> block those.
I understood that bit.

> Thanks,
> Razvan
--
Julien Grall

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