[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


On 11/12/2018 10:21, Razvan Cojocaru wrote:
On 12/11/18 12:14 PM, Roger Pau Monné wrote:
On Tue, Dec 11, 2018 at 12:01:53PM +0200, Razvan Cojocaru wrote:
On 12/10/18 6:59 PM, Razvan Cojocaru wrote:
On 12/10/18 6:49 PM, Roger Pau Monné wrote:
On Mon, Dec 10, 2018 at 06:01:49PM +0200, Razvan Cojocaru wrote:
diff --git a/xen/include/asm-arm/vm_event.h b/xen/include/asm-arm/vm_event.h
index 66f2474..b63249e 100644
--- a/xen/include/asm-arm/vm_event.h
+++ b/xen/include/asm-arm/vm_event.h
@@ -52,4 +52,10 @@ void vm_event_emulate_check(struct vcpu *v, 
vm_event_response_t *rsp)
      /* Not supported on ARM. */
+static inline
+void vm_event_block_interrupts(struct vcpu *v, bool value)
+    /* Not supported on ARM. */


Will do (although if you look at the rest of the function in that header
it'll break what appears to be the prior convention).

Sorry, on second thought we can't do that, because that function is
being called from the common code - which is why the function became
necessary. Specifically, this it unconditionally called in
monitor_traps(), which is used for all events (ARM and otherwise).

So it's valid to call monitor_traps() for ARM vm_events and expect it to
run without issue, which ASSERT_UNREACHABLE() would of course break.

But then the functionality that makes use of vm_event_block_interrupts
cannot work reliably on ARM and should not be used?

Well, it's currently a no-op on ARM so it doesn't make anything worse.
Can an vm-event app rely on the interrupts to be blocked?

I don't have access to ARM hardware and am unfamiliar with the specifics
of handling interrupts on ARM with regard to vm_events (or even if this
specific problem applies to ARM) - so it's the best that I am able to do
at the moment.

At the first, the name of the function looks quite wrong for Arm. If you block interrupts, you will never receive them again. I read the commit message and I am not sure to understand the exact behavior of this function.

Do you mind to provide more explanation what you are trying to achieve?

Of course, this patch can be the basis of a future one for ARM if that
work makes sense (perhaps Tamas has more to say about this), or if an
ARM maintaner can point out what modifications should be done I can
compile-test for ARM with a cross-compiler, _hope_ it works, and
re-submit the patch.

Before pointing out the modifications, I need to understand what you exactly want to achieve with it. But then, I think such code should be tested before getting merged.

That's fine by me if you don't want to implement it for Arm. However, we need to make sure that vm-event app does not expect that behavior.

In any case, I think you want to rename the function and/or document more that expected behavior.


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.