[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH] xen/arm: Make local_events_need_delivery working with idle VPCU
Hi,
On 04/05/2015 12:44, Riku Voipio wrote:
On 27 April 2015 at 19:32, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
Hi Stefano,
On 27/04/15 16:36, Stefano Stabellini wrote:
On Mon, 27 Apr 2015, Julien Grall 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();
}
Is it actually considered correct in Xen to call hypercall_preempt_check
and/or local_events_need_delivery on the idle vcpu?
It seems that the x86 version of hypercall_preempt_check is able to cope
with idle VCPU. Although, I'm not sure if there is path where
hypercall_preempt_check can be called on an idle VCPU (cc x86
maintainers for this purpose).
Shouldn't it be avoided and maybe a BUG_ON added here instead?
This patch was the simple way to fix the bug. I have other ideas in mind
which require some rework in apply_p2m_changes.
I'll wait to see what x86 maintainers think.
Any news on this front? The head Xen is still failing in LAVA. We are
risking that
new regressions get introduced undetected while we wait for the fix for this...
Most of the maintainers were at the Xen hackathon last week and it's a
bank holiday in England. I expect to have feedback during this week.
Although, no big changes has been pushed to Xen upstream as the
committers were away too. Therefore, we should not see any major regression.
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
