Re: [Xen-devel] [PATCH V2] x86/vm_event: block interrupt injection for sync vm_events
On 12/13/18 4:58 PM, Jan Beulich wrote: On 13.12.18 at 14:18, wrote: >> So, long story short, on VMX we first send out the vm_event, while >> processing it an interrupt / exception may become pending, before >> resuming the VCPU that has sent out the vm_event there's a Xen function >> that picks up the pending interrupt and schedules it (writes it in the >> VMCS), and only then we attempt the emulation, which may overwrite it >> (because there's only one place we can write to schedule interrupts / >> exceptions). > > So perhaps the solution is indeed to change the order of how things > get done, instead of blocking interrupts? You seem to think this way > too, as per ... > >> 2. Interrupts are not blocked indefinitely - only until the emulation is >> done. It could be argued that that's really the proper place for them to >> be processed anyway - on an instruction boundary, _after_ the >> in-progress instruction has finished executing. It's just that with the >> vm_event introspection thing you could say that executing the current >> instruction may take a bit longer. > > ... this. Quite possibly, following the lead of the singlestepping code just seemed like the most straightforward way out of the problem. Of course an alternative way of handling interrupts would probably be preferrable, however we'd need a bit of guidance on how to go about it and in the meantime I don't see how this small fix would hurt. I remember Andrew suggesting taking something like this on at the Xen Developer Summit in Budapest but then of course much more important things happened with Meltdown & al. Thanks, Razvan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V2] x86/vm_event: block interrupt injection for sync vm_events
>>> On 13.12.18 at 14:18, wrote: > So, long story short, on VMX we first send out the vm_event, while > processing it an interrupt / exception may become pending, before > resuming the VCPU that has sent out the vm_event there's a Xen function > that picks up the pending interrupt and schedules it (writes it in the > VMCS), and only then we attempt the emulation, which may overwrite it > (because there's only one place we can write to schedule interrupts / > exceptions). So perhaps the solution is indeed to change the order of how things get done, instead of blocking interrupts? You seem to think this way too, as per ... > 2. Interrupts are not blocked indefinitely - only until the emulation is > done. It could be argued that that's really the proper place for them to > be processed anyway - on an instruction boundary, _after_ the > in-progress instruction has finished executing. It's just that with the > vm_event introspection thing you could say that executing the current > instruction may take a bit longer. ... this. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V2] x86/vm_event: block interrupt injection for sync vm_events
On 12/13/18 2:39 PM, Julien Grall wrote:
> 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:rcojoc...@bitdefender.com]
>> 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?
I'll certainly do my best. :)
Assume the following scenario:
1. A guest instruction tries to write into read-only memory (as set in
the EPT), with monitoring active for the domain.
2. An EPT violation exit occurs, and in the course of it, the VCPU that
was running the code that produced the violation is paused and a
vm_event is sent to an introspection application.
3. The introspection application is processing the vm_event. During this
time, some event may occur (which will remain pending).
4. The introspection agent replies with "please emulate" the current
instruction - since the emulator (currently) does not care about EPT
restrictions, so this is a cheap way of proceeding without lifting them.
5. With x86, we have {vmx,svm}_intr_assist(), which is guaranteed to be
called _before_ the VCPU is woken up again. This is the designated "pick
up pending interrupts" function on x86. Perhaps this (real-life)
backtrace is helpful:
(XEN) Xen call trace:
(XEN)[] vmx.c#__vmx_inject_exception+0xa1/0xda
(XEN)[] vmx_inject_extint+0x94/0x9f
(XEN)[] vmx_intr_assist+0x4ee/0x5ad
(XEN)[] vmx_asm_vmexit_handler+0xff/0x270
On x86, there can (currently) be only one scheduled interrupt /
exception, and that is written into VM_ENTRY_INTR_INFO in the VMCS on Intel.
6. _After_ vmx_intr_assist() has run, we are now trying to emulate the
current instruction, which may cause an exception, which will overwrite
the pending interrupt.
So, long story short, on VMX we first send out the vm_event, while
processing it an interrupt / exception may become pending, before
resuming the VCPU that has sent out the vm_event there's a Xen function
that picks up the pending interrupt and schedules it (writes it in the
VMCS), and only then we attempt the emulation, which may overwrite it
(because there's only one place we can write to schedule interrupts /
exceptions).
> 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?
Pretty much, yes. Technically speaking there are two that I am aware of,
the second of them being the IDT vectoring case just before the EPT
fault exit - but that's outside the scope of this patch. Just mentioning
it for completeness.
Also, it is worth mentioning that:
1. This is the exact same strategy employed by the single-stepping
functionality on VMX / Intel. In fact, if you look at
xen/arch/x86/hvm/vmx/intr.c, in vmx_intr_assist(), you'll see an early
return blocking injection of interrupts for the duration of
Re: [Xen-devel] [PATCH V2] x86/vm_event: block interrupt injection for sync vm_events
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:rcojoc...@bitdefender.com] 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@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V2] x86/vm_event: block interrupt injection for sync vm_events
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:rcojoc...@bitdefender.com] 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'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. Thanks, Razvan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V2] x86/vm_event: block interrupt injection for sync vm_events
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:rcojoc...@bitdefender.com] 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. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V2] x86/vm_event: block interrupt injection for sync vm_events
On 12/13/18 8:54 AM, Tian, Kevin wrote: >> From: Razvan Cojocaru [mailto:rcojoc...@bitdefender.com] >> 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? Of course, if Julien agrees with the change I'll rename it as suggested. Thanks, Razvan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V2] x86/vm_event: block interrupt injection for sync vm_events
On 12/11/18 1:59 PM, Julien Grall wrote:
> Hi,
>
> 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. */
>>
>> ASSERT_UNREACHABLE?
>
> 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?
So on x86 what happens is this:
1. A sync vm_event is sent out, for an EPT fault. This happens in
xen/arch/x86/hvm/hvm.c, which for VMX / Intel is called in
ept_handle_violation(), which in turn is called in vmx_vmexit_handler(),
as a consequence of handling an EXIT_REASON_EPT_VIOLATION exit.
2. Since this is a _sync_ event, this means that the VCPU is now paused
until the introspection agent replies to it. Let's assume that at this
step, the introspection agent does reply, and tells Xen to emulate the
current instruction in the reply.
3. After Xen receives the reply and re-schedules the VCPU to run, we may
see this backtrace (collected on Xen 4.7, but it applies to staging as
well):
(XEN) [] vmx.c#__vmx_inject_exception+0x74/0xc7
(XEN) [] vmx.c#vmx_inject_trap+0x1e1/0x29f
(XEN) [] hvm_inject_trap+0xb0/0xb5
(XEN) [] hvm_inject_page_fault+0x2a/0x2c
(XEN) [] hvm.c#__hvm_copy+0xdd/0x37f
(XEN) [] hvm_copy_to_guest_virt+0x1d/0x1f
(XEN) [] emulate.c#hvmemul_write+0xe0/0x148
(XEN) [] x86_emulate+0xd148/0x11405
(XEN) [] emulate.c#_hvm_emulate_one+0x188/0x2bc
(XEN) [] hvm_emulate_one+0x10/0x12
(XEN) [] hvm_mem_access_emulate_one+0x7a/0xee
(XEN) [] hvm_do_resume+0x246/0x3c5
(XEN) [] vmx_do_resume+0x102/0x119
(XEN) [] context_switch+0xf52/0xfad
(XEN) [] schedule.c#schedule+0x753/0x792
(XEN) [] softirq.c#__do_softirq+0x85/0x90
(XEN) [] do_softirq+0x13/0x15
(XEN) [] domain.c#idle_loop+0x61/0x6e
Now, vmx_inject_exception() calls __vmwrite(VM_ENTRY_INTR_INFO,
intr_fields);, and _before_ we get here, the asm code has already
previously called vmx_intr_assist(), which may have placed some valid
value into VM_ENTRY_INTR_INFO as well. This, of course, means that the
previous value will now be overwritten, and so lost.
The current patch ensures that vmx_intr_info() will _not_ pick that
pending interrupt up until after the sync vm_event has been handled
(which is also essentially how the interrupt should be processed anyway,
since the current instruction is sort-of-in-progress until the sync
vm_event is handled; it's also the strategy VMX single-step has employed).
>> 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
Re: [Xen-devel] [PATCH V2] x86/vm_event: block interrupt injection for sync vm_events
Hi,
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. */
ASSERT_UNREACHABLE?
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.
Cheers,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V2] x86/vm_event: block interrupt injection for sync vm_events
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. */
ASSERT_UNREACHABLE?
>>>
>>> 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.
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.
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.
Thanks,
Razvan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V2] x86/vm_event: block interrupt injection for sync vm_events
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. */
> >>
> >> ASSERT_UNREACHABLE?
> >
> > 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?
Thanks, Roger.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V2] x86/vm_event: block interrupt injection for sync vm_events
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. */
>>
>> ASSERT_UNREACHABLE?
>
> 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.
Thanks,
Razvan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V2] x86/vm_event: block interrupt injection for sync vm_events
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. */
>
> ASSERT_UNREACHABLE?
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).
Thanks,
Razvan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V2] x86/vm_event: block interrupt injection for sync vm_events
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. */
ASSERT_UNREACHABLE?
Thanks, Roger.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
