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

Re: [Xen-devel] [PATCH v3 13/17] livepatch: Expand spin_debug_disable in [apply|revert]_payload



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.wilk@xxxxxxxxxx>
---
  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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

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