On 01/14/2016 09:47 PM, Konrad Rzeszutek Wilk wrote:
From: Ross Lagerwall <ross.lagerw...@citrix.com>
Implement support for the apply, revert and replace actions.
snip
+#include <xen/cpu.h>
#include <xen/guest_access.h>
#include <xen/keyhandler.h>
#include <xen/lib.h>
@@ -10,25 +11,38 @@
#include <xen/mm.h>
#include <xen/sched.h>
#include <xen/smp.h>
+#include <xen/softirq.h>
#include <xen/spinlock.h>
+#include <xen/wait.h>
#include <xen/xsplice_elf.h>
#include <xen/xsplice.h>
#include <asm/event.h>
+#include <asm/nmi.h>
#include <public/sysctl.h>
-static DEFINE_SPINLOCK(payload_list_lock);
+/*
+ * Protects against payload_list operations and also allows only one
+ * caller in schedule_work.
+ */
+static DEFINE_SPINLOCK(payload_lock);
I think it would be cleaner if all the payload_list_lock changes were
folded into Patch 3.
static LIST_HEAD(payload_list);
+static LIST_HEAD(applied_list);
+
static unsigned int payload_cnt;
static unsigned int payload_version = 1;
struct payload {
int32_t state; /* One of the XSPLICE_STATE_*. */
int32_t rc; /* 0 or -XEN_EXX. */
+ uint32_t timeout; /* Timeout to do the operation. */
This should go into struct xsplice_work.
struct list_head list; /* Linked to 'payload_list'. */
void *payload_address; /* Virtual address mapped. */
size_t payload_pages; /* Nr of the pages. */
+ struct list_head applied_list; /* Linked to 'applied_list'. */
+ struct xsplice_patch_func *funcs; /* The array of functions to patch. */
+ unsigned int nfuncs; /* Nr of functions to patch. */
char name[XEN_XSPLICE_NAME_SIZE + 1];/* Name of it. */
};
@@ -36,6 +50,23 @@ struct payload {
static int load_payload_data(struct payload *payload, uint8_t *raw, ssize_t
len);
static void free_payload_data(struct payload *payload);
+/* Defines an outstanding patching action. */
+struct xsplice_work
+{
+ atomic_t semaphore; /* Used for rendezvous. First to grab it will
+ do the patching. */
+ atomic_t irq_semaphore; /* Used to signal all IRQs disabled. */
+ struct payload *data; /* The payload on which to act. */
+ volatile bool_t do_work; /* Signals work to do. */
+ volatile bool_t ready; /* Signals all CPUs synchronized. */
+ uint32_t cmd; /* Action request: XSPLICE_ACTION_* */
+};
+
+/* There can be only one outstanding patching action. */
+static struct xsplice_work xsplice_work;
+
+static int schedule_work(struct payload *data, uint32_t cmd);
+
snip
+
+/*
+ * This function is executed having all other CPUs with no stack (we may
+ * have cpu_idle on it) and IRQs disabled.
+ */
+static int revert_payload(struct payload *data)
+{
+ unsigned int i;
+
+ printk(XENLOG_DEBUG "%s: Reverting.\n", data->name);
+
+ for ( i = 0; i < data->nfuncs; i++ )
+ xsplice_revert_jmp(data->funcs + i);
+
+ list_del(&data->applied_list);
+
+ return 0;
+}
+
+/* Must be holding the payload_list lock. */
payload lock?
+static int schedule_work(struct payload *data, uint32_t cmd)
+{
+ /* Fail if an operation is already scheduled. */
+ if ( xsplice_work.do_work )
+ return -EAGAIN;
Hmm, I don't think EAGAIN is correct. It will cause xen-xsplice to poll
for a status update, but the operation hasn't actually been submitted.
+
+ xsplice_work.cmd = cmd;
+ xsplice_work.data = data;
+ atomic_set(&xsplice_work.semaphore, -1);
+ atomic_set(&xsplice_work.irq_semaphore, -1);
+
+ xsplice_work.ready = 0;
+ smp_wmb();
+ xsplice_work.do_work = 1;
+ smp_wmb();
+
+ return 0;
+}
+
+/*
+ * Note that because of this NOP code the do_nmi is not safely patchable.
+ * Also if we do receive 'real' NMIs we have lost them.
+ */
+static int mask_nmi_callback(const struct cpu_user_regs *regs, int cpu)
+{
+ return 1;
+}
+
+static void reschedule_fn(void *unused)
+{
+ smp_mb(); /* Synchronize with setting do_work */
+ raise_softirq(SCHEDULE_SOFTIRQ);
+}
+
+static int xsplice_do_wait(atomic_t *counter, s_time_t timeout,
+ unsigned int total_cpus, const char *s)
+{
+ int rc = 0;
+
+ while ( atomic_read(counter) != total_cpus && NOW() < timeout )
+ cpu_relax();
+
+ /* Log & abort. */
+ if ( atomic_read(counter) != total_cpus )
+ {
+ printk(XENLOG_DEBUG "%s: %s %u/%u\n", xsplice_work.data->name,
+ s, atomic_read(counter), total_cpus);
+ rc = -EBUSY;
+ xsplice_work.data->rc = rc;
+ xsplice_work.do_work = 0;
+ smp_wmb();
+ return rc;
+ }
+ return rc;
+}
+
+static void xsplice_do_single(unsigned int total_cpus)
+{
+ nmi_callback_t saved_nmi_callback;
+ s_time_t timeout;
+ struct payload *data, *tmp;
+ int rc;
+
+ data = xsplice_work.data;
+ timeout = data->timeout ? data->timeout : MILLISECS(30);
The design doc says that a timeout of 0 means infinity.
+ printk(XENLOG_DEBUG "%s: timeout is %"PRI_stime"ms\n", data->name,
+ timeout / MILLISECS(1));
+
+ timeout += NOW();
+
+ if ( xsplice_do_wait(&xsplice_work.semaphore, timeout, total_cpus,
+ "Timed out on CPU semaphore") )
+ return;
+
+ /* "Mask" NMIs. */
+ saved_nmi_callback = set_nmi_callback(mask_nmi_callback);
+
+ /* All CPUs are waiting, now signal to disable IRQs. */
+ xsplice_work.ready = 1;
+ smp_wmb();
+
+ atomic_inc(&xsplice_work.irq_semaphore);
+ if ( xsplice_do_wait(&xsplice_work.irq_semaphore, timeout, total_cpus,
+ "Timed out on IRQ semaphore.") )
+ return;
+
+ local_irq_disable();
As far as I can tell, the mechanics of how this works haven't changed,
the code has just been reorganized. Which means the points that Martin
raised about this mechanism are still outstanding.
--
Ross Lagerwall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel