Re: [Xen-devel] [PATCH] xen/arm: Make local_events_need_delivery working with idle VPCU

[Tamas K Lengyel <tklengyel@xxxxxxxxxxxxx>]

IMHO we should check if op==RELINQUISH || op==MEMACCESS before callingÂhypercall_preempt_check. That was a change I made - previously it was only called if the op wasÂRELINQUISH and not the other way around.

Tamas

On Mon, Apr 27, 2015 at 4:39 PM, Julien Grall <julien.grall@xxxxxxxxxx> wrote:

The commit 569fb6c "xen/arm: Data abort exception (R/W) mem_access
events" makes apply_p2m_changes to call hypercall_preempt_check for any
operation rather than for relinquish.

The function hypercall_preempt_check call local_events_need_delivery
which rely on the current VCPU is not an idle VCPU.
Although, during DOM0 building the current VCPU is an idle one. This
would make Xen crash with the following stack trace:

(XEN) CPU0: Unexpected Trap: Data Abort
[...]
(XEN) Xen call trace:
(XEN)Â Â [<00256ef4>] apply_p2m_changes+0x210/0x1190 (PC)
(XEN)Â Â [<002506b4>] gic_events_need_delivery+0x5c/0x13c (LR)
(XEN)Â Â [<002580ec>] map_mmio_regions+0x64/0x74
(XEN)Â Â [<00251958>] gicv2v_setup+0xf8/0x150
(XEN)Â Â [<00250964>] gicv_setup+0x20/0x30
(XEN)Â Â [<0024cb3c>] arch_domain_create+0x170/0x244
(XEN)Â Â [<00207df0>] domain_create+0x2ac/0x4d8
(XEN)Â Â [<0028e3d0>] start_xen+0xcbc/0xee4
(XEN)Â Â [<00200540>] paging+0x94/0xd8
(XEN)
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 0:
(XEN) CPU0: Unexpected Trap: Data Abort
(XEN)
(XEN) ****************************************

As an idle VCPU can never receive an event, return 0 when the current
VCPU is an idle VCPU in local_events_need_delivery.

Reported-by: Riku Voipio <riku.voipio@xxxxxxxxxx>
Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
CC: Tamas K Lengyel <tklengyel@xxxxxxxxxxxxx>

---

This bug has been catched during boot on Mustang. This is because we
have to map large chunk of PCI memory region.

I was able to reproduce the bug on midway by lowering down
preempt_count_limit to 16 in apply_p2m_changes.

Although, I'm not sure this is the right fix for the bug.
---
Âxen/include/asm-arm/event.h | 7 ++++++-
Â1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/xen/include/asm-arm/event.h b/xen/include/asm-arm/event.h
index 5330dfe..0149d06 100644
--- a/xen/include/asm-arm/event.h
+++ b/xen/include/asm-arm/event.h
@@ -39,7 +39,12 @@ static inline int local_events_need_delivery_nomask(void)

Âstatic inline int local_events_need_delivery(void)
Â{
-Â Â if ( !vcpu_event_delivery_is_enabled(current) )
+Â Â struct vcpu *v = current;
+
+Â Â if ( unlikely(is_idle_vcpu(v)) )
+Â Â Â Â return 0;
+
+Â Â if ( !vcpu_event_delivery_is_enabled(v) )
    Âreturn 0;
  Âreturn local_events_need_delivery_nomask();
Â}
--
2.1.4

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