Hi Konrad,
On 12/09/17 01:37, Konrad Rzeszutek Wilk wrote:
Under ARM64 the vmap calls were all done with IRQs disabled which
didn't trip the spinlock debug check (as seen on x86):
Well, I think it does not happen because spin_debug_enable() is never
called on ARM at boot. So atomic_read(&spin_debug) can never return a
positive value and hence the check will not be performed.
We should probably enable spin_debug on ARM.
livepatch.c:1330: livepatch: xen_hello_world: timeout is 30000000ns
livepatch.c:1437: livepatch: xen_hello_world: CPU3 - IPIing the other 3 CPUs
Applying xen_hello_world... (XEN) livepatch: xen_hello_world: Applying 1
functions
Xen BUG at spinlock.c:47
..snip..
[<ffff82d0802379b1>] spinlock.c#check_lock+0x3e/0x44
[<ffff82d080237a70>] _spin_lock+0x11/0x4a
[<ffff82d08023e2de>] __vmap+0x78/0x381
[<ffff82d080219c13>] livepatch.c#livepatch_quiesce+0xc4/0x1bf
[<ffff82d080219ee3>] livepatch.c#apply_payload+0x3a/0x102
[<ffff82d08021a19e>] check_for_livepatch_work+0x1f3/0x390
[<ffff82d08027b7f4>] domain.c#continue_idle_domain+0x1b/0x22
But are definitly the case on x86 - so we expand the scope
s/definitly/definitely/
of the spin_debug_disable.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
---
xen/common/livepatch.c | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 93083cda1a..2f5ee1ae75 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -1154,22 +1154,22 @@ static int apply_payload(struct payload *data)
printk(XENLOG_INFO LIVEPATCH "%s: Applying %u functions\n",
data->name, data->nfuncs);
+ /*
+ * Since we are running with IRQs disabled and the hooks may call common
+ * code - which expects certain spinlocks to run with IRQs enabled - we
+ * temporarily disable the spin locks IRQ state checks.
+ */
+ spin_debug_disable();
rc = livepatch_quiesce(data->funcs, data->nfuncs);
if ( rc )
{
+ spin_debug_enable();
printk(XENLOG_ERR LIVEPATCH "%s: unable to quiesce!\n", data->name);
return rc;
}
- /*
- * Since we are running with IRQs disabled and the hooks may call common
- * code - which expects certain spinlocks to run with IRQs enabled - we
- * temporarily disable the spin locks IRQ state checks.
- */
- spin_debug_disable();
for ( i = 0; i < data->n_load_funcs; i++ )
data->load_funcs[i]();
- spin_debug_enable();
ASSERT(!local_irq_is_enabled());
@@ -1177,6 +1177,7 @@ static int apply_payload(struct payload *data)
arch_livepatch_apply(&data->funcs[i]);
livepatch_revive();
+ spin_debug_enable();
/*
* We need RCU variant (which has barriers) in case we crash here.
@@ -1195,9 +1196,16 @@ static int revert_payload(struct payload *data)
printk(XENLOG_INFO LIVEPATCH "%s: Reverting\n", data->name);
+ /*
+ * Since we are running with IRQs disabled and the hooks may call common
+ * code - which expects certain spinlocks to run with IRQs enabled - we
+ * temporarily disable the spin locks IRQ state checks.
+ */
+ spin_debug_disable();
rc = livepatch_quiesce(data->funcs, data->nfuncs);
if ( rc )
{
+ spin_debug_enable();
printk(XENLOG_ERR LIVEPATCH "%s: unable to quiesce!\n", data->name);
return rc;
}
@@ -1205,19 +1213,13 @@ static int revert_payload(struct payload *data)
for ( i = 0; i < data->nfuncs; i++ )
livepatch_revert(&data->funcs[i]);
- /*
- * Since we are running with IRQs disabled and the hooks may call common
- * code - which expects certain spinlocks to run with IRQs enabled - we
- * temporarily disable the spin locks IRQ state checks.
- */
- spin_debug_disable();
for ( i = 0; i < data->n_unload_funcs; i++ )
data->unload_funcs[i]();
- spin_debug_enable();
ASSERT(!local_irq_is_enabled());
livepatch_revive();
+ spin_debug_enable();
/*
* We need RCU variant (which has barriers) in case we crash here.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
