[livepatch: independ. modules 1/3] livepatch: Always check hypervisor build ID upon hotpatch upload

[livepatch: independ. modules 1/3] livepatch: Always check hypervisor build ID upon hotpatch upload

[Pawel Wieczorkiewicz]

This change is part of a independant stacked hotpatch modules
feature. This feature allows to bypass dependencies between modules
upon loading, but still verifies Xen build ID matching.

In order to prevent (up)loading any hotpatches built for different
hypervisor version as indicated by the Xen Build ID, add checking for
the payload's vs Xen's build id match.

To achieve that embed into every hotpatch another section with a
dedicated hypervisor build id in it. After the payload is loaded and
the .livepatch.xen_depends section becomes available, perform the
check and reject the payload if there is no match.

Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
Reviewed-by: Andra-Irina Paraschiv <andraprs@amazon.com>
Reviewed-by: Bjoern Doebel <doebel@amazon.de>
Reviewed-by: Eslam Elnikety <elnikety@amazon.de>
Reviewed-by: Martin Pohlack <mpohlack@amazon.de>
---
 xen/common/livepatch.c      | 47 +++++++++++++++++++++++++++++++++++++++++++++
 xen/include/xen/livepatch.h |  7 ++++---
 2 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index d6eaae6d3b..6a4af6ce57 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -74,6 +74,7 @@ struct payload {
     unsigned int nsyms;                  /* Nr of entries in .strtab and symbols. */
     struct livepatch_build_id id;        /* ELFNOTE_DESC(.note.gnu.build-id) of the payload. */
     struct livepatch_build_id dep;       /* ELFNOTE_DESC(.livepatch.depends). */
+    struct livepatch_build_id xen_dep;   /* ELFNOTE_DESC(.livepatch.xen_depends). */
     livepatch_loadcall_t *const *load_funcs;   /* The array of funcs to call after */
     livepatch_unloadcall_t *const *unload_funcs;/* load and unload of the payload. */
     unsigned int n_load_funcs;           /* Nr of the funcs to load and execute. */
@@ -476,11 +477,34 @@ static bool section_ok(const struct livepatch_elf *elf,
     return true;
 }
 
+static int check_xen_build_id(const struct payload *payload)
+{
+    const void *id = NULL;
+    unsigned int len = 0;
+    int rc;
+
+    ASSERT(payload->xen_dep.len);
+    ASSERT(payload->xen_dep.p);
+
+    rc = xen_build_id(&id, &len);
+    if ( rc )
+        return rc;
+
+    if ( payload->xen_dep.len != len || memcmp(id, payload->xen_dep.p, len) ) {
+        dprintk(XENLOG_ERR, "%s%s: check against hypervisor build-id failed!\n",
+                LIVEPATCH, payload->name);
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
 static int check_special_sections(const struct livepatch_elf *elf)
 {
     unsigned int i;
     static const char *const names[] = { ELF_LIVEPATCH_FUNC,
                                          ELF_LIVEPATCH_DEPENDS,
+                                         ELF_LIVEPATCH_XEN_DEPENDS,
                                          ELF_BUILD_ID_NOTE};
     DECLARE_BITMAP(found, ARRAY_SIZE(names)) = { 0 };
 
@@ -632,6 +656,22 @@ static int prepare_payload(struct payload *payload,
             return -EINVAL;
     }
 
+    sec = livepatch_elf_sec_by_name(elf, ELF_LIVEPATCH_XEN_DEPENDS);
+    if ( sec )
+    {
+        n = sec->load_addr;
+
+        if ( sec->sec->sh_size <= sizeof(*n) )
+            return -EINVAL;
+
+        if ( xen_build_id_check(n, sec->sec->sh_size,
+                                &payload->xen_dep.p, &payload->xen_dep.len) )
+            return -EINVAL;
+
+        if ( !payload->xen_dep.len || !payload->xen_dep.p )
+            return -EINVAL;
+    }
+
     /* Setup the virtual region with proper data. */
     region = &payload->region;
 
@@ -882,6 +922,10 @@ static int load_payload_data(struct payload *payload, void *raw, size_t len)
     if ( rc )
         goto out;
 
+    rc = check_xen_build_id(payload);
+    if ( rc )
+        goto out;
+
     rc = build_symbol_table(payload, &elf);
     if ( rc )
         goto out;
@@ -1655,6 +1699,9 @@ static void livepatch_printall(unsigned char key)
 
         if ( data->dep.len )
             printk("depend-on=%*phN\n", data->dep.len, data->dep.p);
+
+        if ( data->xen_dep.len )
+            printk("depend-on-xen=%*phN\n", data->xen_dep.len, data->xen_dep.p);
     }
 
     spin_unlock(&payload_lock);
diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
index 1b1817ca0d..ed997aa4cc 100644
--- a/xen/include/xen/livepatch.h
+++ b/xen/include/xen/livepatch.h
@@ -29,9 +29,10 @@ struct xen_sysctl_livepatch_op;
 /* Convenience define for printk. */
 #define LIVEPATCH             "livepatch: "
 /* ELF payload special section names. */
-#define ELF_LIVEPATCH_FUNC    ".livepatch.funcs"
-#define ELF_LIVEPATCH_DEPENDS ".livepatch.depends"
-#define ELF_BUILD_ID_NOTE      ".note.gnu.build-id"
+#define ELF_LIVEPATCH_FUNC        ".livepatch.funcs"
+#define ELF_LIVEPATCH_DEPENDS     ".livepatch.depends"
+#define ELF_LIVEPATCH_XEN_DEPENDS ".livepatch.xen_depends"
+#define ELF_BUILD_ID_NOTE         ".note.gnu.build-id"
 /* Arbitrary limit for payload size and .bss section size. */
 #define LIVEPATCH_MAX_SIZE     MB(2)
 
-- 
2.16.5




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[livepatch: independ. modules 2/3] livepatch: Allow to override inter-modules buildid dependency

[Pawel Wieczorkiewicz]

By default Livepatch enforces the following buildid-based dependency
chain between hotpatch modules:
  1) first module depends on given hypervisor buildid
  2) every consecutive module depends on previous module's buildid
This way proper hotpatch stack order is maintained and enforced.
While it is important for production hotpatches it limits agility and
blocks usage of testing or debug hotpatches. These kinds of hotpatch
modules are typically expected to be loaded at any time irrespective
of current state of the modules stack.

To enable testing and debug hotpatches allow user dynamically ignore
the inter-modules dependency. In this case only hypervisor buildid
match is verified and enforced.

To allow userland pass additional paremeters for livepatch actions
add support for action flags.
Each of the apply, revert, unload and revert action gets additional
64-bit parameter 'flags' where extra flags can be applied in a mask
form.
Initially only one flag '--nodeps' is added for the apply action.
This flag modifies the default buildid dependency check as described
above.
The global sysctl interface input flag parameter is defined with a
single corresponding flag macro:
  LIVEPATCH_ACTION_APPLY_NODEPS (1 << 0)

The userland xen-livepatch tool is modified to support the '--nodeps'
flag for apply and load commands. A general mechanism for specifying
more flags in the future for apply and other action is however added.

Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
Reviewed-by: Andra-Irina Paraschiv <andraprs@amazon.com>
Reviewed-by: Eslam Elnikety <elnikety@amazon.de>
Reviewed-by: Petre Eftime <epetre@amazon.com>
Reviewed-by: Leonard Foerster <foersleo@amazon.de>
Reviewed-by: Martin Pohlack <mpohlack@amazon.de>
Reviewed-by: Norbert Manthey <nmanthey@amazon.de>
---
 tools/libxc/include/xenctrl.h |   9 ++--
 tools/libxc/xc_misc.c         |  20 +++----
 tools/misc/xen-livepatch.c    | 121 +++++++++++++++++++++++++++++++++++-------
 xen/common/livepatch.c        |  14 +++--
 xen/include/public/sysctl.h   |   9 ++++
 5 files changed, 138 insertions(+), 35 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index a3628e56bb..6e1e41c0c9 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2618,11 +2618,12 @@ int xc_livepatch_list(xc_interface *xch, unsigned int max, unsigned int start,
  * to complete them. The `timeout` offers an option to expire the
  * operation if it could not be completed within the specified time
  * (in ns). Value of 0 means let hypervisor decide the best timeout.
+ * The `flags` allows to pass extra parameters to the actions.
  */
-int xc_livepatch_apply(xc_interface *xch, char *name, uint32_t timeout);
-int xc_livepatch_revert(xc_interface *xch, char *name, uint32_t timeout);
-int xc_livepatch_unload(xc_interface *xch, char *name, uint32_t timeout);
-int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout);
+int xc_livepatch_apply(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags);
+int xc_livepatch_revert(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags);
+int xc_livepatch_unload(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags);
+int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags);
 
 /*
  * Ensure cache coherency after memory modifications. A call to this function
diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c
index 5e6714ae2b..b7289c2d4d 100644
--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -831,7 +831,8 @@ int xc_livepatch_list(xc_interface *xch, unsigned int max, unsigned int start,
 static int _xc_livepatch_action(xc_interface *xch,
                                 char *name,
                                 unsigned int action,
-                                uint32_t timeout)
+                                uint32_t timeout,
+                                uint64_t flags)
 {
     int rc;
     DECLARE_SYSCTL;
@@ -857,6 +858,7 @@ static int _xc_livepatch_action(xc_interface *xch,
     sysctl.u.livepatch.pad = 0;
     sysctl.u.livepatch.u.action.cmd = action;
     sysctl.u.livepatch.u.action.timeout = timeout;
+    sysctl.u.livepatch.u.action.flags = flags;
 
     sysctl.u.livepatch.u.action.name = def_name;
     set_xen_guest_handle(sysctl.u.livepatch.u.action.name.name, name);
@@ -868,24 +870,24 @@ static int _xc_livepatch_action(xc_interface *xch,
     return rc;
 }
 
-int xc_livepatch_apply(xc_interface *xch, char *name, uint32_t timeout)
+int xc_livepatch_apply(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags)
 {
-    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_APPLY, timeout);
+    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_APPLY, timeout, flags);
 }
 
-int xc_livepatch_revert(xc_interface *xch, char *name, uint32_t timeout)
+int xc_livepatch_revert(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags)
 {
-    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_REVERT, timeout);
+    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_REVERT, timeout, flags);
 }
 
-int xc_livepatch_unload(xc_interface *xch, char *name, uint32_t timeout)
+int xc_livepatch_unload(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags)
 {
-    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_UNLOAD, timeout);
+    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_UNLOAD, timeout, flags);
 }
 
-int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout)
+int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags)
 {
-    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_REPLACE, timeout);
+    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_REPLACE, timeout, flags);
 }
 
 /*
diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
index 3233472157..a37b2457ff 100644
--- a/tools/misc/xen-livepatch.c
+++ b/tools/misc/xen-livepatch.c
@@ -23,18 +23,23 @@ void show_help(void)
 {
     fprintf(stderr,
             "xen-livepatch: live patching tool\n"
-            "Usage: xen-livepatch <command> [args]\n"
+            "Usage: xen-livepatch <command> [args] [command-flags]\n"
             " <name> An unique name of payload. Up to %d characters.\n"
             "Commands:\n"
             "  help                   display this help\n"
             "  upload <name> <file>   upload file <file> with <name> name\n"
             "  list                   list payloads uploaded.\n"
-            "  apply <name>           apply <name> patch.\n"
+            "  apply <name> [flags]   apply <name> patch.\n"
+            "    Supported flags:\n"
+            "      --nodeps           Disable inter-module buildid dependency check.\n"
+            "                         Check only against hypervisor buildid.\n"
             "  revert <name>          revert name <name> patch.\n"
             "  replace <name>         apply <name> patch and revert all others.\n"
             "  unload <name>          unload name <name> patch.\n"
-            "  load  <file>           upload and apply <file>.\n"
-            "                         name is the <file> name\n",
+            "  load <file> [flags]    upload and apply <file> with name as the <file> name\n"
+            "    Supported flags:\n"
+            "      --nodeps           Disable inter-module buildid dependency check.\n"
+            "                         Check only against hypervisor buildid.\n",
             XEN_LIVEPATCH_NAME_SIZE);
 }
 
@@ -225,12 +230,13 @@ static int upload_func(int argc, char *argv[])
     return rc;
 }
 
-/* These MUST match to the 'action_options[]' array slots. */
+/* These MUST match to the 'action_options[]' and 'flag_options[]' array slots. */
 enum {
     ACTION_APPLY = 0,
     ACTION_REVERT = 1,
     ACTION_UNLOAD = 2,
     ACTION_REPLACE = 3,
+    ACTION_NUM
 };
 
 struct {
@@ -238,7 +244,7 @@ struct {
     int expected; /* The state to be in after the function. */
     const char *name;
     const char *verb;
-    int (*function)(xc_interface *xch, char *name, uint32_t timeout);
+    int (*function)(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags);
 } action_options[] = {
     {   .allow = LIVEPATCH_STATE_CHECKED,
         .expected = LIVEPATCH_STATE_APPLIED,
@@ -266,6 +272,66 @@ struct {
     },
 };
 
+/*
+ * This structure defines supported flag options for actions.
+ * It defines entries for each action and supports up to 64
+ * flags per action.
+ */
+struct {
+    const char *name;
+    const uint64_t flag;
+} flag_options[ACTION_NUM][8 * sizeof(uint64_t)] = {
+    { /* ACTION_APPLY */
+        {   .name = "--nodeps",
+            .flag = LIVEPATCH_ACTION_APPLY_NODEPS,
+        },
+    },
+    { /* ACTION_REVERT */
+    },
+    { /* ACTION_UNLOAD */
+    },
+    { /* ACTION_REPLACE */
+    }
+};
+
+/*
+ * Parse user provided action flags.
+ * This function expects to only receive an array of input parameters being flags.
+ * Expected action is specified via idx paramater (index of flag_options[]).
+ */
+static int get_flags(int argc, char *argv[], unsigned int idx, uint64_t *flags)
+{
+    int i, j;
+
+    if ( !flags || idx >= ARRAY_SIZE(flag_options) )
+        return -1;
+
+    *flags = 0;
+    for ( i = 0; i < argc; i++ )
+    {
+        for ( j = 0; j < ARRAY_SIZE(flag_options[idx]); j++ )
+        {
+            if ( !flag_options[idx][j].name )
+                goto error;
+
+            if ( !strcmp(flag_options[idx][j].name, argv[i]) )
+            {
+                *flags |= flag_options[idx][j].flag;
+                break;
+            }
+        }
+
+        if ( j == ARRAY_SIZE(flag_options[idx]) )
+            goto error;
+    }
+
+    return 0;
+error:
+    fprintf(stderr, "Unsupported flag: %s.\n", argv[i]);
+    errno = EINVAL;
+    return errno;
+}
+
 /* The hypervisor timeout for the live patching operation is 30 msec,
  * but it could take some time for the operation to start, so wait twice
  * that period. */
@@ -291,8 +357,9 @@ int action_func(int argc, char *argv[], unsigned int idx)
     char name[XEN_LIVEPATCH_NAME_SIZE];
     int rc;
     xen_livepatch_status_t status;
+    uint64_t flags;
 
-    if ( argc != 1 )
+    if ( argc < 1 )
     {
         show_help();
         return -1;
@@ -301,7 +368,10 @@ int action_func(int argc, char *argv[], unsigned int idx)
     if ( idx >= ARRAY_SIZE(action_options) )
         return -1;
 
-    if ( get_name(argc, argv, name) )
+    if ( get_name(argc--, argv++, name) )
+        return EINVAL;
+
+    if ( get_flags(argc, argv, idx, &flags) )
         return EINVAL;
 
     /* Check initial status. */
@@ -332,7 +402,7 @@ int action_func(int argc, char *argv[], unsigned int idx)
     if ( action_options[idx].allow & status.state )
     {
         printf("%s %s... ", action_options[idx].verb, name);
-        rc = action_options[idx].function(xch, name, HYPERVISOR_TIMEOUT_NS);
+        rc = action_options[idx].function(xch, name, HYPERVISOR_TIMEOUT_NS, flags);
         if ( rc )
         {
             int saved_errno = errno;
@@ -394,17 +464,23 @@ int action_func(int argc, char *argv[], unsigned int idx)
 
 static int load_func(int argc, char *argv[])
 {
-    int rc;
-    char *new_argv[2];
-    char *path, *name, *lastdot;
+    int i, rc = ENOMEM;
+    char *upload_argv[2];
+    char **apply_argv, *path, *name, *lastdot;
 
-    if ( argc != 1 )
+    if ( argc < 1 )
     {
         show_help();
         return -1;
     }
+
+    /* apply action has <id> [flags] input requirement, which must be constructed */
+    apply_argv = (char **) malloc(argc * sizeof(*apply_argv));
+    if ( !apply_argv )
+        return rc;
+
     /* <file> */
-    new_argv[1] = argv[0];
+    upload_argv[1] = argv[0];
 
     /* Synthesize the <id> */
     path = strdup(argv[0]);
@@ -413,16 +489,23 @@ static int load_func(int argc, char *argv[])
     lastdot = strrchr(name, '.');
     if ( lastdot != NULL )
         *lastdot = '\0';
-    new_argv[0] = name;
+    upload_argv[0] = name;
+    apply_argv[0] = name;
 
-    rc = upload_func(2 /* <id> <file> */, new_argv);
+    /* Fill in all user provided flags */
+    for ( i = 0; i < argc - 1; i++ )
+        apply_argv[i + 1] = argv[i + 1];
+
+    rc = upload_func(2 /* <id> <file> */, upload_argv);
     if ( rc )
-        return rc;
+        goto error;
 
-    rc = action_func(1 /* only <id> */, new_argv, ACTION_APPLY);
+    rc = action_func(argc, apply_argv, ACTION_APPLY);
     if ( rc )
-        action_func(1, new_argv, ACTION_UNLOAD);
+        action_func(1 /* only <id> */, upload_argv, ACTION_UNLOAD);
 
+error:
+    free(apply_argv);
     free(path);
     return rc;
 }
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 6a4af6ce57..fb91d5095c 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -1575,9 +1575,17 @@ static int livepatch_action(struct xen_sysctl_livepatch_action *action)
                 break;
             }
 
-            rc = build_id_dep(data, !!list_empty(&applied_list));
-            if ( rc )
-                break;
+            /*
+             * Check if action is issued with nodeps flags to ignore module
+             * stack dependencies.
+             */
+            if ( !(action->flags & LIVEPATCH_ACTION_APPLY_NODEPS) )
+            {
+                rc = build_id_dep(data, !!list_empty(&applied_list));
+                if ( rc )
+                    break;
+            }
+
             data->rc = -EAGAIN;
             rc = schedule_work(data, action->cmd, action->timeout);
         }
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index c49b4dcc99..270b5e6728 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -1037,6 +1037,15 @@ struct xen_sysctl_livepatch_action {
                                             /* hypervisor default. */
                                             /* Or upper bound of time (ns) */
                                             /* for operation to take. */
+
+/*
+ * Overwrite default inter-module buildid dependency chain enforcement.
+ * Check only if module is built for given hypervisor by comparing buildid.
+ */
+#define LIVEPATCH_ACTION_APPLY_NODEPS (1 << 0)
+    uint64_t flags;                         /* IN: action flags. */
+                                            /* Provide additional parameters */
+                                            /* for an action. */
 };
 
 struct xen_sysctl_livepatch_op {
-- 
2.16.5




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [livepatch: independ. modules v2 2/3] livepatch: Allow to override inter-modules buildid dependency

[Pawel Wieczorkiewicz]

By default Livepatch enforces the following buildid-based dependency
chain between hotpatch modules:
  1) first module depends on given hypervisor buildid
  2) every consecutive module depends on previous module's buildid
This way proper hotpatch stack order is maintained and enforced.
While it is important for production hotpatches it limits agility and
blocks usage of testing or debug hotpatches. These kinds of hotpatch
modules are typically expected to be loaded at any time irrespective
of current state of the modules stack.

To enable testing and debug hotpatches allow user dynamically ignore
the inter-modules dependency. In this case only hypervisor buildid
match is verified and enforced.

To allow userland pass additional paremeters for livepatch actions
add support for action flags.
Each of the apply, revert, unload and revert action gets additional
64-bit parameter 'flags' where extra flags can be applied in a mask
form.
Initially only one flag '--nodeps' is added for the apply action.
This flag modifies the default buildid dependency check as described
above.
The global sysctl interface input flag parameter is defined with a
single corresponding flag macro:
  LIVEPATCH_ACTION_APPLY_NODEPS (1 << 0)

The userland xen-livepatch tool is modified to support the '--nodeps'
flag for apply and load commands. A general mechanism for specifying
more flags in the future for apply and other action is however added.

Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
Reviewed-by: Andra-Irina Paraschiv <andraprs@amazon.com>
Reviewed-by: Eslam Elnikety <elnikety@amazon.de>
Reviewed-by: Petre Eftime <epetre@amazon.com>
Reviewed-by: Leonard Foerster <foersleo@amazon.de>
Reviewed-by: Martin Pohlack <mpohlack@amazon.de>
Reviewed-by: Norbert Manthey <nmanthey@amazon.de>
---
v2:
* Bump XEN_SYSCTL_INTERFACE_VERSION to 0x00000013
---
 tools/libxc/include/xenctrl.h |   9 ++--
 tools/libxc/xc_misc.c         |  20 +++----
 tools/misc/xen-livepatch.c    | 121 +++++++++++++++++++++++++++++++++++-------
 xen/common/livepatch.c        |  14 +++--
 xen/include/public/sysctl.h   |  11 +++-
 5 files changed, 139 insertions(+), 36 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 0ff6ed9e70..725697c132 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2607,11 +2607,12 @@ int xc_livepatch_list(xc_interface *xch, unsigned int max, unsigned int start,
  * to complete them. The `timeout` offers an option to expire the
  * operation if it could not be completed within the specified time
  * (in ns). Value of 0 means let hypervisor decide the best timeout.
+ * The `flags` allows to pass extra parameters to the actions.
  */
-int xc_livepatch_apply(xc_interface *xch, char *name, uint32_t timeout);
-int xc_livepatch_revert(xc_interface *xch, char *name, uint32_t timeout);
-int xc_livepatch_unload(xc_interface *xch, char *name, uint32_t timeout);
-int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout);
+int xc_livepatch_apply(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags);
+int xc_livepatch_revert(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags);
+int xc_livepatch_unload(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags);
+int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags);
 
 /*
  * Ensure cache coherency after memory modifications. A call to this function
diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c
index 8e60b6e9f0..a8e9e7d1e2 100644
--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -854,7 +854,8 @@ int xc_livepatch_list(xc_interface *xch, unsigned int max, unsigned int start,
 static int _xc_livepatch_action(xc_interface *xch,
                                 char *name,
                                 unsigned int action,
-                                uint32_t timeout)
+                                uint32_t timeout,
+                                uint64_t flags)
 {
     int rc;
     DECLARE_SYSCTL;
@@ -880,6 +881,7 @@ static int _xc_livepatch_action(xc_interface *xch,
     sysctl.u.livepatch.pad = 0;
     sysctl.u.livepatch.u.action.cmd = action;
     sysctl.u.livepatch.u.action.timeout = timeout;
+    sysctl.u.livepatch.u.action.flags = flags;
 
     sysctl.u.livepatch.u.action.name = def_name;
     set_xen_guest_handle(sysctl.u.livepatch.u.action.name.name, name);
@@ -891,24 +893,24 @@ static int _xc_livepatch_action(xc_interface *xch,
     return rc;
 }
 
-int xc_livepatch_apply(xc_interface *xch, char *name, uint32_t timeout)
+int xc_livepatch_apply(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags)
 {
-    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_APPLY, timeout);
+    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_APPLY, timeout, flags);
 }
 
-int xc_livepatch_revert(xc_interface *xch, char *name, uint32_t timeout)
+int xc_livepatch_revert(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags)
 {
-    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_REVERT, timeout);
+    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_REVERT, timeout, flags);
 }
 
-int xc_livepatch_unload(xc_interface *xch, char *name, uint32_t timeout)
+int xc_livepatch_unload(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags)
 {
-    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_UNLOAD, timeout);
+    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_UNLOAD, timeout, flags);
 }
 
-int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout)
+int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags)
 {
-    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_REPLACE, timeout);
+    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_REPLACE, timeout, flags);
 }
 
 /*
diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
index 3233472157..a37b2457ff 100644
--- a/tools/misc/xen-livepatch.c
+++ b/tools/misc/xen-livepatch.c
@@ -23,18 +23,23 @@ void show_help(void)
 {
     fprintf(stderr,
             "xen-livepatch: live patching tool\n"
-            "Usage: xen-livepatch <command> [args]\n"
+            "Usage: xen-livepatch <command> [args] [command-flags]\n"
             " <name> An unique name of payload. Up to %d characters.\n"
             "Commands:\n"
             "  help                   display this help\n"
             "  upload <name> <file>   upload file <file> with <name> name\n"
             "  list                   list payloads uploaded.\n"
-            "  apply <name>           apply <name> patch.\n"
+            "  apply <name> [flags]   apply <name> patch.\n"
+            "    Supported flags:\n"
+            "      --nodeps           Disable inter-module buildid dependency check.\n"
+            "                         Check only against hypervisor buildid.\n"
             "  revert <name>          revert name <name> patch.\n"
             "  replace <name>         apply <name> patch and revert all others.\n"
             "  unload <name>          unload name <name> patch.\n"
-            "  load  <file>           upload and apply <file>.\n"
-            "                         name is the <file> name\n",
+            "  load <file> [flags]    upload and apply <file> with name as the <file> name\n"
+            "    Supported flags:\n"
+            "      --nodeps           Disable inter-module buildid dependency check.\n"
+            "                         Check only against hypervisor buildid.\n",
             XEN_LIVEPATCH_NAME_SIZE);
 }
 
@@ -225,12 +230,13 @@ static int upload_func(int argc, char *argv[])
     return rc;
 }
 
-/* These MUST match to the 'action_options[]' array slots. */
+/* These MUST match to the 'action_options[]' and 'flag_options[]' array slots. */
 enum {
     ACTION_APPLY = 0,
     ACTION_REVERT = 1,
     ACTION_UNLOAD = 2,
     ACTION_REPLACE = 3,
+    ACTION_NUM
 };
 
 struct {
@@ -238,7 +244,7 @@ struct {
     int expected; /* The state to be in after the function. */
     const char *name;
     const char *verb;
-    int (*function)(xc_interface *xch, char *name, uint32_t timeout);
+    int (*function)(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags);
 } action_options[] = {
     {   .allow = LIVEPATCH_STATE_CHECKED,
         .expected = LIVEPATCH_STATE_APPLIED,
@@ -266,6 +272,66 @@ struct {
     },
 };
 
+/*
+ * This structure defines supported flag options for actions.
+ * It defines entries for each action and supports up to 64
+ * flags per action.
+ */
+struct {
+    const char *name;
+    const uint64_t flag;
+} flag_options[ACTION_NUM][8 * sizeof(uint64_t)] = {
+    { /* ACTION_APPLY */
+        {   .name = "--nodeps",
+            .flag = LIVEPATCH_ACTION_APPLY_NODEPS,
+        },
+    },
+    { /* ACTION_REVERT */
+    },
+    { /* ACTION_UNLOAD */
+    },
+    { /* ACTION_REPLACE */
+    }
+};
+
+/*
+ * Parse user provided action flags.
+ * This function expects to only receive an array of input parameters being flags.
+ * Expected action is specified via idx paramater (index of flag_options[]).
+ */
+static int get_flags(int argc, char *argv[], unsigned int idx, uint64_t *flags)
+{
+    int i, j;
+
+    if ( !flags || idx >= ARRAY_SIZE(flag_options) )
+        return -1;
+
+    *flags = 0;
+    for ( i = 0; i < argc; i++ )
+    {
+        for ( j = 0; j < ARRAY_SIZE(flag_options[idx]); j++ )
+        {
+            if ( !flag_options[idx][j].name )
+                goto error;
+
+            if ( !strcmp(flag_options[idx][j].name, argv[i]) )
+            {
+                *flags |= flag_options[idx][j].flag;
+                break;
+            }
+        }
+
+        if ( j == ARRAY_SIZE(flag_options[idx]) )
+            goto error;
+    }
+
+    return 0;
+error:
+    fprintf(stderr, "Unsupported flag: %s.\n", argv[i]);
+    errno = EINVAL;
+    return errno;
+}
+
 /* The hypervisor timeout for the live patching operation is 30 msec,
  * but it could take some time for the operation to start, so wait twice
  * that period. */
@@ -291,8 +357,9 @@ int action_func(int argc, char *argv[], unsigned int idx)
     char name[XEN_LIVEPATCH_NAME_SIZE];
     int rc;
     xen_livepatch_status_t status;
+    uint64_t flags;
 
-    if ( argc != 1 )
+    if ( argc < 1 )
     {
         show_help();
         return -1;
@@ -301,7 +368,10 @@ int action_func(int argc, char *argv[], unsigned int idx)
     if ( idx >= ARRAY_SIZE(action_options) )
         return -1;
 
-    if ( get_name(argc, argv, name) )
+    if ( get_name(argc--, argv++, name) )
+        return EINVAL;
+
+    if ( get_flags(argc, argv, idx, &flags) )
         return EINVAL;
 
     /* Check initial status. */
@@ -332,7 +402,7 @@ int action_func(int argc, char *argv[], unsigned int idx)
     if ( action_options[idx].allow & status.state )
     {
         printf("%s %s... ", action_options[idx].verb, name);
-        rc = action_options[idx].function(xch, name, HYPERVISOR_TIMEOUT_NS);
+        rc = action_options[idx].function(xch, name, HYPERVISOR_TIMEOUT_NS, flags);
         if ( rc )
         {
             int saved_errno = errno;
@@ -394,17 +464,23 @@ int action_func(int argc, char *argv[], unsigned int idx)
 
 static int load_func(int argc, char *argv[])
 {
-    int rc;
-    char *new_argv[2];
-    char *path, *name, *lastdot;
+    int i, rc = ENOMEM;
+    char *upload_argv[2];
+    char **apply_argv, *path, *name, *lastdot;
 
-    if ( argc != 1 )
+    if ( argc < 1 )
     {
         show_help();
         return -1;
     }
+
+    /* apply action has <id> [flags] input requirement, which must be constructed */
+    apply_argv = (char **) malloc(argc * sizeof(*apply_argv));
+    if ( !apply_argv )
+        return rc;
+
     /* <file> */
-    new_argv[1] = argv[0];
+    upload_argv[1] = argv[0];
 
     /* Synthesize the <id> */
     path = strdup(argv[0]);
@@ -413,16 +489,23 @@ static int load_func(int argc, char *argv[])
     lastdot = strrchr(name, '.');
     if ( lastdot != NULL )
         *lastdot = '\0';
-    new_argv[0] = name;
+    upload_argv[0] = name;
+    apply_argv[0] = name;
 
-    rc = upload_func(2 /* <id> <file> */, new_argv);
+    /* Fill in all user provided flags */
+    for ( i = 0; i < argc - 1; i++ )
+        apply_argv[i + 1] = argv[i + 1];
+
+    rc = upload_func(2 /* <id> <file> */, upload_argv);
     if ( rc )
-        return rc;
+        goto error;
 
-    rc = action_func(1 /* only <id> */, new_argv, ACTION_APPLY);
+    rc = action_func(argc, apply_argv, ACTION_APPLY);
     if ( rc )
-        action_func(1, new_argv, ACTION_UNLOAD);
+        action_func(1 /* only <id> */, upload_argv, ACTION_UNLOAD);
 
+error:
+    free(apply_argv);
     free(path);
     return rc;
 }
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 6a4af6ce57..fb91d5095c 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -1575,9 +1575,17 @@ static int livepatch_action(struct xen_sysctl_livepatch_action *action)
                 break;
             }
 
-            rc = build_id_dep(data, !!list_empty(&applied_list));
-            if ( rc )
-                break;
+            /*
+             * Check if action is issued with nodeps flags to ignore module
+             * stack dependencies.
+             */
+            if ( !(action->flags & LIVEPATCH_ACTION_APPLY_NODEPS) )
+            {
+                rc = build_id_dep(data, !!list_empty(&applied_list));
+                if ( rc )
+                    break;
+            }
+
             data->rc = -EAGAIN;
             rc = schedule_work(data, action->cmd, action->timeout);
         }
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 91c48dcae0..1b2b165a6d 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -35,7 +35,7 @@
 #include "domctl.h"
 #include "physdev.h"
 
-#define XEN_SYSCTL_INTERFACE_VERSION 0x00000012
+#define XEN_SYSCTL_INTERFACE_VERSION 0x00000013
 
 /*
  * Read console content from Xen buffer ring.
@@ -956,6 +956,15 @@ struct xen_sysctl_livepatch_action {
                                             /* hypervisor default. */
                                             /* Or upper bound of time (ns) */
                                             /* for operation to take. */
+
+/*
+ * Overwrite default inter-module buildid dependency chain enforcement.
+ * Check only if module is built for given hypervisor by comparing buildid.
+ */
+#define LIVEPATCH_ACTION_APPLY_NODEPS (1 << 0)
+    uint64_t flags;                         /* IN: action flags. */
+                                            /* Provide additional parameters */
+                                            /* for an action. */
 };
 
 struct xen_sysctl_livepatch_op {
-- 
2.16.5




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [livepatch: independ. modules v2 2/3] livepatch: Allow to override inter-modules buildid dependency

[Julien Grall]

Hi,

Something looks fishy in the threading:

   - The patch #1 is answered in reply-to the patch #1 of version 1.
   - This patch (#2) is answered in reply-to the patch #2 of version 1.
   - The patch #3 is labeled as v3 an in reply-to the patch #3 of version 1.

If you send them as series, then they should be sent together for a new version 
and in a new thread. Not mangled to the previous thread as this makes quite 
difficult to follow what's going on.

Also it looks like the series is still lacking of the cover letter. So I still 
have no clue what "livepatch: independ. modules" in your [...] refers to.

Cheers,

On 20/08/2019 14:28, Pawel Wieczorkiewicz wrote:
> By default Livepatch enforces the following buildid-based dependency
> chain between hotpatch modules:
>    1) first module depends on given hypervisor buildid
>    2) every consecutive module depends on previous module's buildid
> This way proper hotpatch stack order is maintained and enforced.
> While it is important for production hotpatches it limits agility and
> blocks usage of testing or debug hotpatches. These kinds of hotpatch
> modules are typically expected to be loaded at any time irrespective
> of current state of the modules stack.
> 
> To enable testing and debug hotpatches allow user dynamically ignore
> the inter-modules dependency. In this case only hypervisor buildid
> match is verified and enforced.
> 
> To allow userland pass additional paremeters for livepatch actions
> add support for action flags.
> Each of the apply, revert, unload and revert action gets additional
> 64-bit parameter 'flags' where extra flags can be applied in a mask
> form.
> Initially only one flag '--nodeps' is added for the apply action.
> This flag modifies the default buildid dependency check as described
> above.
> The global sysctl interface input flag parameter is defined with a
> single corresponding flag macro:
>    LIVEPATCH_ACTION_APPLY_NODEPS (1 << 0)
> 
> The userland xen-livepatch tool is modified to support the '--nodeps'
> flag for apply and load commands. A general mechanism for specifying
> more flags in the future for apply and other action is however added.
> 
> Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
> Reviewed-by: Andra-Irina Paraschiv <andraprs@amazon.com>
> Reviewed-by: Eslam Elnikety <elnikety@amazon.de>
> Reviewed-by: Petre Eftime <epetre@amazon.com>
> Reviewed-by: Leonard Foerster <foersleo@amazon.de>
> Reviewed-by: Martin Pohlack <mpohlack@amazon.de>
> Reviewed-by: Norbert Manthey <nmanthey@amazon.de>
> ---
> v2:
> * Bump XEN_SYSCTL_INTERFACE_VERSION to 0x00000013
> ---
>   tools/libxc/include/xenctrl.h |   9 ++--
>   tools/libxc/xc_misc.c         |  20 +++----
>   tools/misc/xen-livepatch.c    | 121 +++++++++++++++++++++++++++++++++++-------
>   xen/common/livepatch.c        |  14 +++--
>   xen/include/public/sysctl.h   |  11 +++-
>   5 files changed, 139 insertions(+), 36 deletions(-)
> 
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index 0ff6ed9e70..725697c132 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -2607,11 +2607,12 @@ int xc_livepatch_list(xc_interface *xch, unsigned int max, unsigned int start,
>    * to complete them. The `timeout` offers an option to expire the
>    * operation if it could not be completed within the specified time
>    * (in ns). Value of 0 means let hypervisor decide the best timeout.
> + * The `flags` allows to pass extra parameters to the actions.
>    */
> -int xc_livepatch_apply(xc_interface *xch, char *name, uint32_t timeout);
> -int xc_livepatch_revert(xc_interface *xch, char *name, uint32_t timeout);
> -int xc_livepatch_unload(xc_interface *xch, char *name, uint32_t timeout);
> -int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout);
> +int xc_livepatch_apply(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags);
> +int xc_livepatch_revert(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags);
> +int xc_livepatch_unload(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags);
> +int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags);
>   
>   /*
>    * Ensure cache coherency after memory modifications. A call to this function
> diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c
> index 8e60b6e9f0..a8e9e7d1e2 100644
> --- a/tools/libxc/xc_misc.c
> +++ b/tools/libxc/xc_misc.c
> @@ -854,7 +854,8 @@ int xc_livepatch_list(xc_interface *xch, unsigned int max, unsigned int start,
>   static int _xc_livepatch_action(xc_interface *xch,
>                                   char *name,
>                                   unsigned int action,
> -                                uint32_t timeout)
> +                                uint32_t timeout,
> +                                uint64_t flags)
>   {
>       int rc;
>       DECLARE_SYSCTL;
> @@ -880,6 +881,7 @@ static int _xc_livepatch_action(xc_interface *xch,
>       sysctl.u.livepatch.pad = 0;
>       sysctl.u.livepatch.u.action.cmd = action;
>       sysctl.u.livepatch.u.action.timeout = timeout;
> +    sysctl.u.livepatch.u.action.flags = flags;
>   
>       sysctl.u.livepatch.u.action.name = def_name;
>       set_xen_guest_handle(sysctl.u.livepatch.u.action.name.name, name);
> @@ -891,24 +893,24 @@ static int _xc_livepatch_action(xc_interface *xch,
>       return rc;
>   }
>   
> -int xc_livepatch_apply(xc_interface *xch, char *name, uint32_t timeout)
> +int xc_livepatch_apply(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags)
>   {
> -    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_APPLY, timeout);
> +    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_APPLY, timeout, flags);
>   }
>   
> -int xc_livepatch_revert(xc_interface *xch, char *name, uint32_t timeout)
> +int xc_livepatch_revert(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags)
>   {
> -    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_REVERT, timeout);
> +    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_REVERT, timeout, flags);
>   }
>   
> -int xc_livepatch_unload(xc_interface *xch, char *name, uint32_t timeout)
> +int xc_livepatch_unload(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags)
>   {
> -    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_UNLOAD, timeout);
> +    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_UNLOAD, timeout, flags);
>   }
>   
> -int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout)
> +int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags)
>   {
> -    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_REPLACE, timeout);
> +    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_REPLACE, timeout, flags);
>   }
>   
>   /*
> diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
> index 3233472157..a37b2457ff 100644
> --- a/tools/misc/xen-livepatch.c
> +++ b/tools/misc/xen-livepatch.c
> @@ -23,18 +23,23 @@ void show_help(void)
>   {
>       fprintf(stderr,
>               "xen-livepatch: live patching tool\n"
> -            "Usage: xen-livepatch <command> [args]\n"
> +            "Usage: xen-livepatch <command> [args] [command-flags]\n"
>               " <name> An unique name of payload. Up to %d characters.\n"
>               "Commands:\n"
>               "  help                   display this help\n"
>               "  upload <name> <file>   upload file <file> with <name> name\n"
>               "  list                   list payloads uploaded.\n"
> -            "  apply <name>           apply <name> patch.\n"
> +            "  apply <name> [flags]   apply <name> patch.\n"
> +            "    Supported flags:\n"
> +            "      --nodeps           Disable inter-module buildid dependency check.\n"
> +            "                         Check only against hypervisor buildid.\n"
>               "  revert <name>          revert name <name> patch.\n"
>               "  replace <name>         apply <name> patch and revert all others.\n"
>               "  unload <name>          unload name <name> patch.\n"
> -            "  load  <file>           upload and apply <file>.\n"
> -            "                         name is the <file> name\n",
> +            "  load <file> [flags]    upload and apply <file> with name as the <file> name\n"
> +            "    Supported flags:\n"
> +            "      --nodeps           Disable inter-module buildid dependency check.\n"
> +            "                         Check only against hypervisor buildid.\n",
>               XEN_LIVEPATCH_NAME_SIZE);
>   }
>   
> @@ -225,12 +230,13 @@ static int upload_func(int argc, char *argv[])
>       return rc;
>   }
>   
> -/* These MUST match to the 'action_options[]' array slots. */
> +/* These MUST match to the 'action_options[]' and 'flag_options[]' array slots. */
>   enum {
>       ACTION_APPLY = 0,
>       ACTION_REVERT = 1,
>       ACTION_UNLOAD = 2,
>       ACTION_REPLACE = 3,
> +    ACTION_NUM
>   };
>   
>   struct {
> @@ -238,7 +244,7 @@ struct {
>       int expected; /* The state to be in after the function. */
>       const char *name;
>       const char *verb;
> -    int (*function)(xc_interface *xch, char *name, uint32_t timeout);
> +    int (*function)(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags);
>   } action_options[] = {
>       {   .allow = LIVEPATCH_STATE_CHECKED,
>           .expected = LIVEPATCH_STATE_APPLIED,
> @@ -266,6 +272,66 @@ struct {
>       },
>   };
>   
> +/*
> + * This structure defines supported flag options for actions.
> + * It defines entries for each action and supports up to 64
> + * flags per action.
> + */
> +struct {
> +    const char *name;
> +    const uint64_t flag;
> +} flag_options[ACTION_NUM][8 * sizeof(uint64_t)] = {
> +    { /* ACTION_APPLY */
> +        {   .name = "--nodeps",
> +            .flag = LIVEPATCH_ACTION_APPLY_NODEPS,
> +        },
> +    },
> +    { /* ACTION_REVERT */
> +    },
> +    { /* ACTION_UNLOAD */
> +    },
> +    { /* ACTION_REPLACE */
> +    }
> +};
> +
> +/*
> + * Parse user provided action flags.
> + * This function expects to only receive an array of input parameters being flags.
> + * Expected action is specified via idx paramater (index of flag_options[]).
> + */
> +static int get_flags(int argc, char *argv[], unsigned int idx, uint64_t *flags)
> +{
> +    int i, j;
> +
> +    if ( !flags || idx >= ARRAY_SIZE(flag_options) )
> +        return -1;
> +
> +    *flags = 0;
> +    for ( i = 0; i < argc; i++ )
> +    {
> +        for ( j = 0; j < ARRAY_SIZE(flag_options[idx]); j++ )
> +        {
> +            if ( !flag_options[idx][j].name )
> +                goto error;
> +
> +            if ( !strcmp(flag_options[idx][j].name, argv[i]) )
> +            {
> +                *flags |= flag_options[idx][j].flag;
> +                break;
> +            }
> +        }
> +
> +        if ( j == ARRAY_SIZE(flag_options[idx]) )
> +            goto error;
> +    }
> +
> +    return 0;
> +error:
> +    fprintf(stderr, "Unsupported flag: %s.\n", argv[i]);
> +    errno = EINVAL;
> +    return errno;
> +}
> +
>   /* The hypervisor timeout for the live patching operation is 30 msec,
>    * but it could take some time for the operation to start, so wait twice
>    * that period. */
> @@ -291,8 +357,9 @@ int action_func(int argc, char *argv[], unsigned int idx)
>       char name[XEN_LIVEPATCH_NAME_SIZE];
>       int rc;
>       xen_livepatch_status_t status;
> +    uint64_t flags;
>   
> -    if ( argc != 1 )
> +    if ( argc < 1 )
>       {
>           show_help();
>           return -1;
> @@ -301,7 +368,10 @@ int action_func(int argc, char *argv[], unsigned int idx)
>       if ( idx >= ARRAY_SIZE(action_options) )
>           return -1;
>   
> -    if ( get_name(argc, argv, name) )
> +    if ( get_name(argc--, argv++, name) )
> +        return EINVAL;
> +
> +    if ( get_flags(argc, argv, idx, &flags) )
>           return EINVAL;
>   
>       /* Check initial status. */
> @@ -332,7 +402,7 @@ int action_func(int argc, char *argv[], unsigned int idx)
>       if ( action_options[idx].allow & status.state )
>       {
>           printf("%s %s... ", action_options[idx].verb, name);
> -        rc = action_options[idx].function(xch, name, HYPERVISOR_TIMEOUT_NS);
> +        rc = action_options[idx].function(xch, name, HYPERVISOR_TIMEOUT_NS, flags);
>           if ( rc )
>           {
>               int saved_errno = errno;
> @@ -394,17 +464,23 @@ int action_func(int argc, char *argv[], unsigned int idx)
>   
>   static int load_func(int argc, char *argv[])
>   {
> -    int rc;
> -    char *new_argv[2];
> -    char *path, *name, *lastdot;
> +    int i, rc = ENOMEM;
> +    char *upload_argv[2];
> +    char **apply_argv, *path, *name, *lastdot;
>   
> -    if ( argc != 1 )
> +    if ( argc < 1 )
>       {
>           show_help();
>           return -1;
>       }
> +
> +    /* apply action has <id> [flags] input requirement, which must be constructed */
> +    apply_argv = (char **) malloc(argc * sizeof(*apply_argv));
> +    if ( !apply_argv )
> +        return rc;
> +
>       /* <file> */
> -    new_argv[1] = argv[0];
> +    upload_argv[1] = argv[0];
>   
>       /* Synthesize the <id> */
>       path = strdup(argv[0]);
> @@ -413,16 +489,23 @@ static int load_func(int argc, char *argv[])
>       lastdot = strrchr(name, '.');
>       if ( lastdot != NULL )
>           *lastdot = '\0';
> -    new_argv[0] = name;
> +    upload_argv[0] = name;
> +    apply_argv[0] = name;
>   
> -    rc = upload_func(2 /* <id> <file> */, new_argv);
> +    /* Fill in all user provided flags */
> +    for ( i = 0; i < argc - 1; i++ )
> +        apply_argv[i + 1] = argv[i + 1];
> +
> +    rc = upload_func(2 /* <id> <file> */, upload_argv);
>       if ( rc )
> -        return rc;
> +        goto error;
>   
> -    rc = action_func(1 /* only <id> */, new_argv, ACTION_APPLY);
> +    rc = action_func(argc, apply_argv, ACTION_APPLY);
>       if ( rc )
> -        action_func(1, new_argv, ACTION_UNLOAD);
> +        action_func(1 /* only <id> */, upload_argv, ACTION_UNLOAD);
>   
> +error:
> +    free(apply_argv);
>       free(path);
>       return rc;
>   }
> diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
> index 6a4af6ce57..fb91d5095c 100644
> --- a/xen/common/livepatch.c
> +++ b/xen/common/livepatch.c
> @@ -1575,9 +1575,17 @@ static int livepatch_action(struct xen_sysctl_livepatch_action *action)
>                   break;
>               }
>   
> -            rc = build_id_dep(data, !!list_empty(&applied_list));
> -            if ( rc )
> -                break;
> +            /*
> +             * Check if action is issued with nodeps flags to ignore module
> +             * stack dependencies.
> +             */
> +            if ( !(action->flags & LIVEPATCH_ACTION_APPLY_NODEPS) )
> +            {
> +                rc = build_id_dep(data, !!list_empty(&applied_list));
> +                if ( rc )
> +                    break;
> +            }
> +
>               data->rc = -EAGAIN;
>               rc = schedule_work(data, action->cmd, action->timeout);
>           }
> diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
> index 91c48dcae0..1b2b165a6d 100644
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -35,7 +35,7 @@
>   #include "domctl.h"
>   #include "physdev.h"
>   
> -#define XEN_SYSCTL_INTERFACE_VERSION 0x00000012
> +#define XEN_SYSCTL_INTERFACE_VERSION 0x00000013
>   
>   /*
>    * Read console content from Xen buffer ring.
> @@ -956,6 +956,15 @@ struct xen_sysctl_livepatch_action {
>                                               /* hypervisor default. */
>                                               /* Or upper bound of time (ns) */
>                                               /* for operation to take. */
> +
> +/*
> + * Overwrite default inter-module buildid dependency chain enforcement.
> + * Check only if module is built for given hypervisor by comparing buildid.
> + */
> +#define LIVEPATCH_ACTION_APPLY_NODEPS (1 << 0)
> +    uint64_t flags;                         /* IN: action flags. */
> +                                            /* Provide additional parameters */
> +                                            /* for an action. */
>   };
>   
>   struct xen_sysctl_livepatch_op {
> 

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [livepatch: independ. modules v2 2/3] livepatch: Allow to override inter-modules buildid dependency

[Wieczorkiewicz, Pawel]

> On 20. Aug 2019, at 15:35, Julien Grall <julien.grall@arm.com> wrote:
> 
> Hi,
> 
> Something looks fishy in the threading:
> 
>  - The patch #1 is answered in reply-to the patch #1 of version 1.
>  - This patch (#2) is answered in reply-to the patch #2 of version 1.
>  - The patch #3 is labeled as v3 an in reply-to the patch #3 of version 1.
> 
> If you send them as series, then they should be sent together for a new version and in a new thread. Not mangled to the previous thread as this makes quite difficult to follow what's going on.
> 
> Also it looks like the series is still lacking of the cover letter. So I still have no clue what "livepatch: independ. modules" in your [...] refers to.
> 

Yeah, since I got feedback and reviews on various patches that I have already submitted the way I did,
I simply continue with what I have until all comments are addressed (I do not want to lose anything).

Then, I will re-send the patches in 2 series: livepatch-build-tools and xen with all changes,
Reviewed-by/Acked-by and cover letters. This is the way recommended by Andrew.

Unfortunately, it will be also quite confusing I think, because various changes belonging to different topics,
are distributed between those 2 distinct repos. 

Best,
Pawel

> Cheers,
> 
> On 20/08/2019 14:28, Pawel Wieczorkiewicz wrote:
>> By default Livepatch enforces the following buildid-based dependency
>> chain between hotpatch modules:
>>   1) first module depends on given hypervisor buildid
>>   2) every consecutive module depends on previous module's buildid
>> This way proper hotpatch stack order is maintained and enforced.
>> While it is important for production hotpatches it limits agility and
>> blocks usage of testing or debug hotpatches. These kinds of hotpatch
>> modules are typically expected to be loaded at any time irrespective
>> of current state of the modules stack.
>> To enable testing and debug hotpatches allow user dynamically ignore
>> the inter-modules dependency. In this case only hypervisor buildid
>> match is verified and enforced.
>> To allow userland pass additional paremeters for livepatch actions
>> add support for action flags.
>> Each of the apply, revert, unload and revert action gets additional
>> 64-bit parameter 'flags' where extra flags can be applied in a mask
>> form.
>> Initially only one flag '--nodeps' is added for the apply action.
>> This flag modifies the default buildid dependency check as described
>> above.
>> The global sysctl interface input flag parameter is defined with a
>> single corresponding flag macro:
>>   LIVEPATCH_ACTION_APPLY_NODEPS (1 << 0)
>> The userland xen-livepatch tool is modified to support the '--nodeps'
>> flag for apply and load commands. A general mechanism for specifying
>> more flags in the future for apply and other action is however added.
>> Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
>> Reviewed-by: Andra-Irina Paraschiv <andraprs@amazon.com>
>> Reviewed-by: Eslam Elnikety <elnikety@amazon.de>
>> Reviewed-by: Petre Eftime <epetre@amazon.com>
>> Reviewed-by: Leonard Foerster <foersleo@amazon.de>
>> Reviewed-by: Martin Pohlack <mpohlack@amazon.de>
>> Reviewed-by: Norbert Manthey <nmanthey@amazon.de>
>> ---
>> v2:
>> * Bump XEN_SYSCTL_INTERFACE_VERSION to 0x00000013
>> ---
>>  tools/libxc/include/xenctrl.h |   9 ++--
>>  tools/libxc/xc_misc.c         |  20 +++----
>>  tools/misc/xen-livepatch.c    | 121 +++++++++++++++++++++++++++++++++++-------
>>  xen/common/livepatch.c        |  14 +++--
>>  xen/include/public/sysctl.h   |  11 +++-
>>  5 files changed, 139 insertions(+), 36 deletions(-)
>> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
>> index 0ff6ed9e70..725697c132 100644
>> --- a/tools/libxc/include/xenctrl.h
>> +++ b/tools/libxc/include/xenctrl.h
>> @@ -2607,11 +2607,12 @@ int xc_livepatch_list(xc_interface *xch, unsigned int max, unsigned int start,
>>   * to complete them. The `timeout` offers an option to expire the
>>   * operation if it could not be completed within the specified time
>>   * (in ns). Value of 0 means let hypervisor decide the best timeout.
>> + * The `flags` allows to pass extra parameters to the actions.
>>   */
>> -int xc_livepatch_apply(xc_interface *xch, char *name, uint32_t timeout);
>> -int xc_livepatch_revert(xc_interface *xch, char *name, uint32_t timeout);
>> -int xc_livepatch_unload(xc_interface *xch, char *name, uint32_t timeout);
>> -int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout);
>> +int xc_livepatch_apply(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags);
>> +int xc_livepatch_revert(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags);
>> +int xc_livepatch_unload(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags);
>> +int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags);
>>    /*
>>   * Ensure cache coherency after memory modifications. A call to this function
>> diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c
>> index 8e60b6e9f0..a8e9e7d1e2 100644
>> --- a/tools/libxc/xc_misc.c
>> +++ b/tools/libxc/xc_misc.c
>> @@ -854,7 +854,8 @@ int xc_livepatch_list(xc_interface *xch, unsigned int max, unsigned int start,
>>  static int _xc_livepatch_action(xc_interface *xch,
>>                                  char *name,
>>                                  unsigned int action,
>> -                                uint32_t timeout)
>> +                                uint32_t timeout,
>> +                                uint64_t flags)
>>  {
>>      int rc;
>>      DECLARE_SYSCTL;
>> @@ -880,6 +881,7 @@ static int _xc_livepatch_action(xc_interface *xch,
>>      sysctl.u.livepatch.pad = 0;
>>      sysctl.u.livepatch.u.action.cmd = action;
>>      sysctl.u.livepatch.u.action.timeout = timeout;
>> +    sysctl.u.livepatch.u.action.flags = flags;
>>        sysctl.u.livepatch.u.action.name = def_name;
>>      set_xen_guest_handle(sysctl.u.livepatch.u.action.name.name, name);
>> @@ -891,24 +893,24 @@ static int _xc_livepatch_action(xc_interface *xch,
>>      return rc;
>>  }
>>  -int xc_livepatch_apply(xc_interface *xch, char *name, uint32_t timeout)
>> +int xc_livepatch_apply(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags)
>>  {
>> -    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_APPLY, timeout);
>> +    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_APPLY, timeout, flags);
>>  }
>>  -int xc_livepatch_revert(xc_interface *xch, char *name, uint32_t timeout)
>> +int xc_livepatch_revert(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags)
>>  {
>> -    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_REVERT, timeout);
>> +    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_REVERT, timeout, flags);
>>  }
>>  -int xc_livepatch_unload(xc_interface *xch, char *name, uint32_t timeout)
>> +int xc_livepatch_unload(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags)
>>  {
>> -    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_UNLOAD, timeout);
>> +    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_UNLOAD, timeout, flags);
>>  }
>>  -int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout)
>> +int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags)
>>  {
>> -    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_REPLACE, timeout);
>> +    return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_REPLACE, timeout, flags);
>>  }
>>    /*
>> diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
>> index 3233472157..a37b2457ff 100644
>> --- a/tools/misc/xen-livepatch.c
>> +++ b/tools/misc/xen-livepatch.c
>> @@ -23,18 +23,23 @@ void show_help(void)
>>  {
>>      fprintf(stderr,
>>              "xen-livepatch: live patching tool\n"
>> -            "Usage: xen-livepatch <command> [args]\n"
>> +            "Usage: xen-livepatch <command> [args] [command-flags]\n"
>>              " <name> An unique name of payload. Up to %d characters.\n"
>>              "Commands:\n"
>>              "  help                   display this help\n"
>>              "  upload <name> <file>   upload file <file> with <name> name\n"
>>              "  list                   list payloads uploaded.\n"
>> -            "  apply <name>           apply <name> patch.\n"
>> +            "  apply <name> [flags]   apply <name> patch.\n"
>> +            "    Supported flags:\n"
>> +            "      --nodeps           Disable inter-module buildid dependency check.\n"
>> +            "                         Check only against hypervisor buildid.\n"
>>              "  revert <name>          revert name <name> patch.\n"
>>              "  replace <name>         apply <name> patch and revert all others.\n"
>>              "  unload <name>          unload name <name> patch.\n"
>> -            "  load  <file>           upload and apply <file>.\n"
>> -            "                         name is the <file> name\n",
>> +            "  load <file> [flags]    upload and apply <file> with name as the <file> name\n"
>> +            "    Supported flags:\n"
>> +            "      --nodeps           Disable inter-module buildid dependency check.\n"
>> +            "                         Check only against hypervisor buildid.\n",
>>              XEN_LIVEPATCH_NAME_SIZE);
>>  }
>>  @@ -225,12 +230,13 @@ static int upload_func(int argc, char *argv[])
>>      return rc;
>>  }
>>  -/* These MUST match to the 'action_options[]' array slots. */
>> +/* These MUST match to the 'action_options[]' and 'flag_options[]' array slots. */
>>  enum {
>>      ACTION_APPLY = 0,
>>      ACTION_REVERT = 1,
>>      ACTION_UNLOAD = 2,
>>      ACTION_REPLACE = 3,
>> +    ACTION_NUM
>>  };
>>    struct {
>> @@ -238,7 +244,7 @@ struct {
>>      int expected; /* The state to be in after the function. */
>>      const char *name;
>>      const char *verb;
>> -    int (*function)(xc_interface *xch, char *name, uint32_t timeout);
>> +    int (*function)(xc_interface *xch, char *name, uint32_t timeout, uint64_t flags);
>>  } action_options[] = {
>>      {   .allow = LIVEPATCH_STATE_CHECKED,
>>          .expected = LIVEPATCH_STATE_APPLIED,
>> @@ -266,6 +272,66 @@ struct {
>>      },
>>  };
>>  +/*
>> + * This structure defines supported flag options for actions.
>> + * It defines entries for each action and supports up to 64
>> + * flags per action.
>> + */
>> +struct {
>> +    const char *name;
>> +    const uint64_t flag;
>> +} flag_options[ACTION_NUM][8 * sizeof(uint64_t)] = {
>> +    { /* ACTION_APPLY */
>> +        {   .name = "--nodeps",
>> +            .flag = LIVEPATCH_ACTION_APPLY_NODEPS,
>> +        },
>> +    },
>> +    { /* ACTION_REVERT */
>> +    },
>> +    { /* ACTION_UNLOAD */
>> +    },
>> +    { /* ACTION_REPLACE */
>> +    }
>> +};
>> +
>> +/*
>> + * Parse user provided action flags.
>> + * This function expects to only receive an array of input parameters being flags.
>> + * Expected action is specified via idx paramater (index of flag_options[]).
>> + */
>> +static int get_flags(int argc, char *argv[], unsigned int idx, uint64_t *flags)
>> +{
>> +    int i, j;
>> +
>> +    if ( !flags || idx >= ARRAY_SIZE(flag_options) )
>> +        return -1;
>> +
>> +    *flags = 0;
>> +    for ( i = 0; i < argc; i++ )
>> +    {
>> +        for ( j = 0; j < ARRAY_SIZE(flag_options[idx]); j++ )
>> +        {
>> +            if ( !flag_options[idx][j].name )
>> +                goto error;
>> +
>> +            if ( !strcmp(flag_options[idx][j].name, argv[i]) )
>> +            {
>> +                *flags |= flag_options[idx][j].flag;
>> +                break;
>> +            }
>> +        }
>> +
>> +        if ( j == ARRAY_SIZE(flag_options[idx]) )
>> +            goto error;
>> +    }
>> +
>> +    return 0;
>> +error:
>> +    fprintf(stderr, "Unsupported flag: %s.\n", argv[i]);
>> +    errno = EINVAL;
>> +    return errno;
>> +}
>> +
>>  /* The hypervisor timeout for the live patching operation is 30 msec,
>>   * but it could take some time for the operation to start, so wait twice
>>   * that period. */
>> @@ -291,8 +357,9 @@ int action_func(int argc, char *argv[], unsigned int idx)
>>      char name[XEN_LIVEPATCH_NAME_SIZE];
>>      int rc;
>>      xen_livepatch_status_t status;
>> +    uint64_t flags;
>>  -    if ( argc != 1 )
>> +    if ( argc < 1 )
>>      {
>>          show_help();
>>          return -1;
>> @@ -301,7 +368,10 @@ int action_func(int argc, char *argv[], unsigned int idx)
>>      if ( idx >= ARRAY_SIZE(action_options) )
>>          return -1;
>>  -    if ( get_name(argc, argv, name) )
>> +    if ( get_name(argc--, argv++, name) )
>> +        return EINVAL;
>> +
>> +    if ( get_flags(argc, argv, idx, &flags) )
>>          return EINVAL;
>>        /* Check initial status. */
>> @@ -332,7 +402,7 @@ int action_func(int argc, char *argv[], unsigned int idx)
>>      if ( action_options[idx].allow & status.state )
>>      {
>>          printf("%s %s... ", action_options[idx].verb, name);
>> -        rc = action_options[idx].function(xch, name, HYPERVISOR_TIMEOUT_NS);
>> +        rc = action_options[idx].function(xch, name, HYPERVISOR_TIMEOUT_NS, flags);
>>          if ( rc )
>>          {
>>              int saved_errno = errno;
>> @@ -394,17 +464,23 @@ int action_func(int argc, char *argv[], unsigned int idx)
>>    static int load_func(int argc, char *argv[])
>>  {
>> -    int rc;
>> -    char *new_argv[2];
>> -    char *path, *name, *lastdot;
>> +    int i, rc = ENOMEM;
>> +    char *upload_argv[2];
>> +    char **apply_argv, *path, *name, *lastdot;
>>  -    if ( argc != 1 )
>> +    if ( argc < 1 )
>>      {
>>          show_help();
>>          return -1;
>>      }
>> +
>> +    /* apply action has <id> [flags] input requirement, which must be constructed */
>> +    apply_argv = (char **) malloc(argc * sizeof(*apply_argv));
>> +    if ( !apply_argv )
>> +        return rc;
>> +
>>      /* <file> */
>> -    new_argv[1] = argv[0];
>> +    upload_argv[1] = argv[0];
>>        /* Synthesize the <id> */
>>      path = strdup(argv[0]);
>> @@ -413,16 +489,23 @@ static int load_func(int argc, char *argv[])
>>      lastdot = strrchr(name, '.');
>>      if ( lastdot != NULL )
>>          *lastdot = '\0';
>> -    new_argv[0] = name;
>> +    upload_argv[0] = name;
>> +    apply_argv[0] = name;
>>  -    rc = upload_func(2 /* <id> <file> */, new_argv);
>> +    /* Fill in all user provided flags */
>> +    for ( i = 0; i < argc - 1; i++ )
>> +        apply_argv[i + 1] = argv[i + 1];
>> +
>> +    rc = upload_func(2 /* <id> <file> */, upload_argv);
>>      if ( rc )
>> -        return rc;
>> +        goto error;
>>  -    rc = action_func(1 /* only <id> */, new_argv, ACTION_APPLY);
>> +    rc = action_func(argc, apply_argv, ACTION_APPLY);
>>      if ( rc )
>> -        action_func(1, new_argv, ACTION_UNLOAD);
>> +        action_func(1 /* only <id> */, upload_argv, ACTION_UNLOAD);
>>  +error:
>> +    free(apply_argv);
>>      free(path);
>>      return rc;
>>  }
>> diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
>> index 6a4af6ce57..fb91d5095c 100644
>> --- a/xen/common/livepatch.c
>> +++ b/xen/common/livepatch.c
>> @@ -1575,9 +1575,17 @@ static int livepatch_action(struct xen_sysctl_livepatch_action *action)
>>                  break;
>>              }
>>  -            rc = build_id_dep(data, !!list_empty(&applied_list));
>> -            if ( rc )
>> -                break;
>> +            /*
>> +             * Check if action is issued with nodeps flags to ignore module
>> +             * stack dependencies.
>> +             */
>> +            if ( !(action->flags & LIVEPATCH_ACTION_APPLY_NODEPS) )
>> +            {
>> +                rc = build_id_dep(data, !!list_empty(&applied_list));
>> +                if ( rc )
>> +                    break;
>> +            }
>> +
>>              data->rc = -EAGAIN;
>>              rc = schedule_work(data, action->cmd, action->timeout);
>>          }
>> diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
>> index 91c48dcae0..1b2b165a6d 100644
>> --- a/xen/include/public/sysctl.h
>> +++ b/xen/include/public/sysctl.h
>> @@ -35,7 +35,7 @@
>>  #include "domctl.h"
>>  #include "physdev.h"
>>  -#define XEN_SYSCTL_INTERFACE_VERSION 0x00000012
>> +#define XEN_SYSCTL_INTERFACE_VERSION 0x00000013
>>    /*
>>   * Read console content from Xen buffer ring.
>> @@ -956,6 +956,15 @@ struct xen_sysctl_livepatch_action {
>>                                              /* hypervisor default. */
>>                                              /* Or upper bound of time (ns) */
>>                                              /* for operation to take. */
>> +
>> +/*
>> + * Overwrite default inter-module buildid dependency chain enforcement.
>> + * Check only if module is built for given hypervisor by comparing buildid.
>> + */
>> +#define LIVEPATCH_ACTION_APPLY_NODEPS (1 << 0)
>> +    uint64_t flags;                         /* IN: action flags. */
>> +                                            /* Provide additional parameters */
>> +                                            /* for an action. */
>>  };
>>    struct xen_sysctl_livepatch_op {
> 
> -- 
> Julien Grall




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [livepatch: independ. modules v2 2/3] livepatch: Allow to override inter-modules buildid dependency

[Julien Grall]

Hi,

On 20/08/2019 15:09, Wieczorkiewicz, Pawel wrote:
> 
>> On 20. Aug 2019, at 15:35, Julien Grall <julien.grall@arm.com> wrote:
>>
>> Hi,
>>
>> Something looks fishy in the threading:
>>
>>   - The patch #1 is answered in reply-to the patch #1 of version 1.
>>   - This patch (#2) is answered in reply-to the patch #2 of version 1.
>>   - The patch #3 is labeled as v3 an in reply-to the patch #3 of version 1.
>>
>> If you send them as series, then they should be sent together for a new version and in a new thread. Not mangled to the previous thread as this makes quite difficult to follow what's going on.
>>
>> Also it looks like the series is still lacking of the cover letter. So I still have no clue what "livepatch: independ. modules" in your [...] refers to.
>>
> 
> Yeah, since I got feedback and reviews on various patches that I have already submitted the way I did,
> I simply continue with what I have until all comments are addressed (I do not want to lose anything).

What do you mean by "all comments are addressed"? Usually you gather a set of 
comments for a series, address them and then resend the series with all of them 
addressed.

> 
> Then, I will re-send the patches in 2 series: livepatch-build-tools and xen with all changes,
> Reviewed-by/Acked-by and cover letters. This is the way recommended by Andrew.

Please don't send the patch one by one to check if everyone is happy. Just 
resend all of them in one go once you gathered enough feedback.

> 
> Unfortunately, it will be also quite confusing I think, because various changes belonging to different topics,
> are distributed between those 2 distinct repos.

That also happen when you have multiple patches in a series. Feature implemented 
accross multiple patch needs a place to discuss. This can usually be done in the 
cover letter. For multi repo series, you can steer the discussion on a single 
repo and just replicate the changes in the other one once there are an agreement.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [livepatch: independ. modules v2 2/3] livepatch: Allow to override inter-modules buildid dependency

[Wieczorkiewicz, Pawel]

> On 20. Aug 2019, at 16:28, Julien Grall <julien.grall@arm.com> wrote:
> 
> Hi,
> 
>>> 
…snip...
>> Yeah, since I got feedback and reviews on various patches that I have already submitted the way I did,
>> I simply continue with what I have until all comments are addressed (I do not want to lose anything).
> 
> What do you mean by "all comments are addressed"? Usually you gather a set of comments for a series, address them and then resend the series with all of them addressed.
> 

Yeah, but people invested time in reviews and replied to my incorrectly sent threads, so I find it rude to ignore such comments and start a new thread instead.
But if this is the recommended practice, I will obey.

>> Then, I will re-send the patches in 2 series: livepatch-build-tools and xen with all changes,
>> Reviewed-by/Acked-by and cover letters. This is the way recommended by Andrew.
> 
> Please don't send the patch one by one to check if everyone is happy. Just resend all of them in one go once you gathered enough feedback.
> 

Ok.

>> Unfortunately, it will be also quite confusing I think, because various changes belonging to different topics,
>> are distributed between those 2 distinct repos.
> 
> That also happen when you have multiple patches in a series. Feature implemented accross multiple patch needs a place to discuss. This can usually be done in the cover letter. For multi repo series, you can steer the discussion on a single repo and just replicate the changes in the other one once there are an agreement.

Fine. Let me then send the 2 full series for each repo with all the changes and corresponding cover letters.

> 
> Cheers,
> 
> -- 
> Julien Grall


Best Regards,
Pawel Wieczorkiewicz




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [livepatch: independ. modules v2 2/3] livepatch: Allow to override inter-modules buildid dependency

[Julien Grall]

Hi Pawel,

On 20/08/2019 15:52, Wieczorkiewicz, Pawel wrote:
> 
>> On 20. Aug 2019, at 16:28, Julien Grall <julien.grall@arm.com> wrote:
>>
>> Hi,
>>
>>>>
> …snip...
>>> Yeah, since I got feedback and reviews on various patches that I have already submitted the way I did,
>>> I simply continue with what I have until all comments are addressed (I do not want to lose anything).
>>
>> What do you mean by "all comments are addressed"? Usually you gather a set of comments for a series, address them and then resend the series with all of them addressed.
>>
> 
> Yeah, but people invested time in reviews and replied to my incorrectly sent threads, so I find it rude to ignore such comments and start a new thread instead.

If you resend a new version in a few days with comments addressed, then most of 
the reviewers will be mostly likely happy :).

If I wanted to jump on the series now, I have no idea where to start, which 
version is the latest. It is also pretty not clear why one patch of the series 
is at version 3 when another is at version 2.

Keeping version separated make quite clear where we are and also allows to 
automatic tools to pick it up.


> But if this is the recommended practice, I will obey.

Most of (if not all) the recommended guidelines are gathered on the wiki ([1]).

> 
>>> Then, I will re-send the patches in 2 series: livepatch-build-tools and xen with all changes,
>>> Reviewed-by/Acked-by and cover letters. This is the way recommended by Andrew.
>>
>> Please don't send the patch one by one to check if everyone is happy. Just resend all of them in one go once you gathered enough feedback.
>>
> 
> Ok.
> 
>>> Unfortunately, it will be also quite confusing I think, because various changes belonging to different topics,
>>> are distributed between those 2 distinct repos.
>>
>> That also happen when you have multiple patches in a series. Feature implemented accross multiple patch needs a place to discuss. This can usually be done in the cover letter. For multi repo series, you can steer the discussion on a single repo and just replicate the changes in the other one once there are an agreement.
> 
> Fine. Let me then send the 2 full series for each repo with all the changes and corresponding cover letters.

Thank you!

Cheers,

[1] 
https://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches#Review.2C_Rinse_.26_Repeat

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[livepatch: independ. modules 3/3] python: Add XC binding for Xen build ID

[Pawel Wieczorkiewicz]

Extend the list of xc() object methods with additional one to display
Xen's buildid. The implementation follows the libxl implementation
(e.g. max buildid size assumption being XC_PAGE_SIZE).

Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
Reviewed-by: Martin Mazein <amazein@amazon.de>
Reviewed-by: Andra-Irina Paraschiv <andraprs@amazon.com>
Reviewed-by: Norbert Manthey <nmanthey@amazon.de>

CC: Marek Marczykowski-Gorecki <marmarek@invisiblethingslab.com>
---
 tools/python/xen/lowlevel/xc/xc.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
index cc8175a11e..66db18ec3a 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -1202,6 +1202,26 @@ out:
     return ret_obj ? ret_obj : pyxc_error_to_exception(self->xc_handle);
 }
 
+static PyObject *pyxc_xenbuildid(XcObject *self)
+{
+    xen_build_id_t *buildid;
+    int i, r;
+    char *str;
+
+    buildid = alloca(sizeof(buildid->len) + XC_PAGE_SIZE);
+    buildid->len = XC_PAGE_SIZE - sizeof(*buildid);
+
+    r = xc_version(self->xc_handle, XENVER_build_id, buildid);
+    if ( r <= 0 )
+        return pyxc_error_to_exception(self->xc_handle);
+
+    str = alloca((r * 2) + 1);
+    for ( i = 0; i < r; i++ )
+        snprintf(&str[i * 2], 3, "%02hhx", buildid->buf[i]);
+
+    return Py_BuildValue("s", str);
+}
+
 static PyObject *pyxc_xeninfo(XcObject *self)
 {
     xen_extraversion_t xen_extra;
@@ -2350,6 +2370,13 @@ static PyMethodDef pyxc_methods[] = {
       "Returns [dict]: information about Xen"
       "        [None]: on failure.\n" },
 
+    { "buildid",
+      (PyCFunction)pyxc_xenbuildid,
+      METH_NOARGS, "\n"
+      "Get Xen buildid\n"
+      "Returns [str]: Xen buildid"
+      "        [None]: on failure.\n" },
+
     { "shadow_control", 
       (PyCFunction)pyxc_shadow_control, 
       METH_VARARGS | METH_KEYWORDS, "\n"
-- 
2.16.5




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [livepatch: independ. modules 3/3] python: Add XC binding for Xen build ID

[Pawel Wieczorkiewicz]

Extend the list of xc() object methods with additional one to display
Xen's buildid. The implementation follows the libxl implementation
(e.g. max buildid size assumption being XC_PAGE_SIZE).

Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
Reviewed-by: Martin Mazein <amazein@amazon.de>
Reviewed-by: Andra-Irina Paraschiv <andraprs@amazon.com>
Reviewed-by: Norbert Manthey <nmanthey@amazon.de>
---
v2:
* No code change
* Adding maintainers
---
 tools/python/xen/lowlevel/xc/xc.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
index 522cbe3b9c..5459d6834d 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -1211,6 +1211,26 @@ out:
     return ret_obj ? ret_obj : pyxc_error_to_exception(self->xc_handle);
 }
 
+static PyObject *pyxc_xenbuildid(XcObject *self)
+{
+    xen_build_id_t *buildid;
+    int i, r;
+    char *str;
+
+    buildid = alloca(sizeof(buildid->len) + XC_PAGE_SIZE);
+    buildid->len = XC_PAGE_SIZE - sizeof(*buildid);
+
+    r = xc_version(self->xc_handle, XENVER_build_id, buildid);
+    if ( r <= 0 )
+        return pyxc_error_to_exception(self->xc_handle);
+
+    str = alloca((r * 2) + 1);
+    for ( i = 0; i < r; i++ )
+        snprintf(&str[i * 2], 3, "%02hhx", buildid->buf[i]);
+
+    return Py_BuildValue("s", str);
+}
+
 static PyObject *pyxc_xeninfo(XcObject *self)
 {
     xen_extraversion_t xen_extra;
@@ -2294,6 +2314,13 @@ static PyMethodDef pyxc_methods[] = {
       "Returns [dict]: information about Xen"
       "        [None]: on failure.\n" },
 
+    { "buildid",
+      (PyCFunction)pyxc_xenbuildid,
+      METH_NOARGS, "\n"
+      "Get Xen buildid\n"
+      "Returns [str]: Xen buildid"
+      "        [None]: on failure.\n" },
+
     { "shadow_control", 
       (PyCFunction)pyxc_shadow_control, 
       METH_VARARGS | METH_KEYWORDS, "\n"
-- 
2.16.5




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [livepatch: independ. modules 3/3] python: Add XC binding for Xen build ID

[Wei Liu]

On Thu, Aug 15, 2019 at 09:44:00AM +0000, Pawel Wieczorkiewicz wrote:
> Extend the list of xc() object methods with additional one to display
> Xen's buildid. The implementation follows the libxl implementation
> (e.g. max buildid size assumption being XC_PAGE_SIZE).
> 
> Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
> Reviewed-by: Martin Mazein <amazein@amazon.de>
> Reviewed-by: Andra-Irina Paraschiv <andraprs@amazon.com>
> Reviewed-by: Norbert Manthey <nmanthey@amazon.de>

I'm a bit confused by the tag in the subject line. Which series does
this patch belong to?

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [livepatch: independ. modules 3/3] python: Add XC binding for Xen build ID

[Wieczorkiewicz, Pawel]

[-- Attachment #1.1: Type: text/plain, Size: 1143 bytes --]


On 16. Aug 2019, at 14:47, Wei Liu <wl@xen.org<mailto:wl@xen.org>> wrote:

On Thu, Aug 15, 2019 at 09:44:00AM +0000, Pawel Wieczorkiewicz wrote:
Extend the list of xc() object methods with additional one to display
Xen's buildid. The implementation follows the libxl implementation
(e.g. max buildid size assumption being XC_PAGE_SIZE).

Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de<mailto:wipawel@amazon.de>>
Reviewed-by: Martin Mazein <amazein@amazon.de<mailto:amazein@amazon.de>>
Reviewed-by: Andra-Irina Paraschiv <andraprs@amazon.com<mailto:andraprs@amazon.com>>
Reviewed-by: Norbert Manthey <nmanthey@amazon.de<mailto:nmanthey@amazon.de>>

I'm a bit confused by the tag in the subject line. Which series does
this patch belong to?

Wei.

Thanks for taking a look.

This is the series: https://marc.info/?t=155541982300002&r=1&w=4

Best Regards,
Pawel Wieczorkiewicz



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879



[-- Attachment #1.2: Type: text/html, Size: 2424 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [livepatch: independ. modules 3/3] python: Add XC binding for Xen build ID

[Marek Marczykowski-Górecki]

[-- Attachment #1.1: Type: text/plain, Size: 2626 bytes --]

On Thu, Aug 15, 2019 at 09:44:00AM +0000, Pawel Wieczorkiewicz wrote:
> Extend the list of xc() object methods with additional one to display
> Xen's buildid. The implementation follows the libxl implementation
> (e.g. max buildid size assumption being XC_PAGE_SIZE).
> 
> Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
> Reviewed-by: Martin Mazein <amazein@amazon.de>
> Reviewed-by: Andra-Irina Paraschiv <andraprs@amazon.com>
> Reviewed-by: Norbert Manthey <nmanthey@amazon.de>
> ---
> v2:
> * No code change
> * Adding maintainers
> ---
>  tools/python/xen/lowlevel/xc/xc.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
> index 522cbe3b9c..5459d6834d 100644
> --- a/tools/python/xen/lowlevel/xc/xc.c
> +++ b/tools/python/xen/lowlevel/xc/xc.c
> @@ -1211,6 +1211,26 @@ out:
>      return ret_obj ? ret_obj : pyxc_error_to_exception(self->xc_handle);
>  }
>  
> +static PyObject *pyxc_xenbuildid(XcObject *self)
> +{
> +    xen_build_id_t *buildid;
> +    int i, r;
> +    char *str;
> +
> +    buildid = alloca(sizeof(buildid->len) + XC_PAGE_SIZE);
> +    buildid->len = XC_PAGE_SIZE - sizeof(*buildid);

Those doesn't match. You allocated XC_PAGE_SIZE in addition to
sizeof(buildid->len). I'd change to alloca(XC_PAGE_SIZE) - it is
unlikely that izeof(buildid->len) would be larger than XC_PAGE_SIZE and
we do assume it in other places anyway.

> +
> +    r = xc_version(self->xc_handle, XENVER_build_id, buildid);
> +    if ( r <= 0 )
> +        return pyxc_error_to_exception(self->xc_handle);
> +
> +    str = alloca((r * 2) + 1);
> +    for ( i = 0; i < r; i++ )
> +        snprintf(&str[i * 2], 3, "%02hhx", buildid->buf[i]);
> +
> +    return Py_BuildValue("s", str);
> +}
> +
>  static PyObject *pyxc_xeninfo(XcObject *self)
>  {
>      xen_extraversion_t xen_extra;
> @@ -2294,6 +2314,13 @@ static PyMethodDef pyxc_methods[] = {
>        "Returns [dict]: information about Xen"
>        "        [None]: on failure.\n" },
>  
> +    { "buildid",
> +      (PyCFunction)pyxc_xenbuildid,
> +      METH_NOARGS, "\n"
> +      "Get Xen buildid\n"
> +      "Returns [str]: Xen buildid"
> +      "        [None]: on failure.\n" },
> +
>      { "shadow_control", 
>        (PyCFunction)pyxc_shadow_control, 
>        METH_VARARGS | METH_KEYWORDS, "\n"

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [livepatch: independ. modules 3/3] python: Add XC binding for Xen build ID

[Wieczorkiewicz, Pawel]

> On 19. Aug 2019, at 22:40, Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> wrote:
> 
> On Thu, Aug 15, 2019 at 09:44:00AM +0000, Pawel Wieczorkiewicz wrote:
>> Extend the list of xc() object methods with additional one to display
>> Xen's buildid. The implementation follows the libxl implementation
>> (e.g. max buildid size assumption being XC_PAGE_SIZE).
>> 
>> Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
>> Reviewed-by: Martin Mazein <amazein@amazon.de>
>> Reviewed-by: Andra-Irina Paraschiv <andraprs@amazon.com>
>> Reviewed-by: Norbert Manthey <nmanthey@amazon.de>
>> ---
>> v2:
>> 
…snip...
>> 
>> +static PyObject *pyxc_xenbuildid(XcObject *self)
>> +{
>> +    xen_build_id_t *buildid;
>> +    int i, r;
>> +    char *str;
>> +
>> +    buildid = alloca(sizeof(buildid->len) + XC_PAGE_SIZE);
>> +    buildid->len = XC_PAGE_SIZE - sizeof(*buildid);
> 
> Those doesn't match. You allocated XC_PAGE_SIZE in addition to
> sizeof(buildid->len). I'd change to alloca(XC_PAGE_SIZE) - it is
> unlikely that izeof(buildid->len) would be larger than XC_PAGE_SIZE and
> we do assume it in other places anyway.

ACK. Will fix.

> 
>> +
>> +    r = xc_version(self->xc_handle, XENVER_build_id, buildid);
>> +    if ( r <= 0 )
>> +        return pyxc_error_to_exception(self->xc_handle);
>> +
>> 

…snip...

> 
> -- 
> Best Regards,
> Marek Marczykowski-Górecki
> Invisible Things Lab
> A: Because it messes up the order in which people normally read text.
> Q: Why is top-posting such a bad thing?


Best Regards,
Pawel Wieczorkiewicz






Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [livepatch: independ. modules v3 3/3] python: Add XC binding for Xen build ID

[Pawel Wieczorkiewicz]

Extend the list of xc() object methods with additional one to display
Xen's buildid. The implementation follows the libxl implementation
(e.g. max buildid size assumption being XC_PAGE_SIZE minus
sizeof(buildid->len)).

Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
Reviewed-by: Martin Mazein <amazein@amazon.de>
Reviewed-by: Andra-Irina Paraschiv <andraprs@amazon.com>
Reviewed-by: Norbert Manthey <nmanthey@amazon.de>
---
v3:
* Instead of allocating XC_PAGE_SIZE plus size of the len field,
 allocate one XC_PAGE_SIZE for the whole buildid struct.
* Modify commit message to reflect the above change.
v2:
* No code change
* Adding maintainers
---
 tools/python/xen/lowlevel/xc/xc.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
index 522cbe3b9c..7ed4a253de 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -1211,6 +1211,26 @@ out:
     return ret_obj ? ret_obj : pyxc_error_to_exception(self->xc_handle);
 }
 
+static PyObject *pyxc_xenbuildid(XcObject *self)
+{
+    xen_build_id_t *buildid;
+    int i, r;
+    char *str;
+
+    buildid = alloca(XC_PAGE_SIZE);
+    buildid->len = XC_PAGE_SIZE - sizeof(*buildid);
+
+    r = xc_version(self->xc_handle, XENVER_build_id, buildid);
+    if ( r <= 0 )
+        return pyxc_error_to_exception(self->xc_handle);
+
+    str = alloca((r * 2) + 1);
+    for ( i = 0; i < r; i++ )
+        snprintf(&str[i * 2], 3, "%02hhx", buildid->buf[i]);
+
+    return Py_BuildValue("s", str);
+}
+
 static PyObject *pyxc_xeninfo(XcObject *self)
 {
     xen_extraversion_t xen_extra;
@@ -2294,6 +2314,13 @@ static PyMethodDef pyxc_methods[] = {
       "Returns [dict]: information about Xen"
       "        [None]: on failure.\n" },
 
+    { "buildid",
+      (PyCFunction)pyxc_xenbuildid,
+      METH_NOARGS, "\n"
+      "Get Xen buildid\n"
+      "Returns [str]: Xen buildid"
+      "        [None]: on failure.\n" },
+
     { "shadow_control", 
       (PyCFunction)pyxc_shadow_control, 
       METH_VARARGS | METH_KEYWORDS, "\n"
-- 
2.16.5




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [livepatch: independ. modules v3 3/3] python: Add XC binding for Xen build ID

[Marek Marczykowski-Górecki]

[-- Attachment #1.1: Type: text/plain, Size: 2653 bytes --]

On Tue, Aug 20, 2019 at 12:51:08PM +0000, Pawel Wieczorkiewicz wrote:
> Extend the list of xc() object methods with additional one to display
> Xen's buildid. The implementation follows the libxl implementation
> (e.g. max buildid size assumption being XC_PAGE_SIZE minus
> sizeof(buildid->len)).
> 
> Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
> Reviewed-by: Martin Mazein <amazein@amazon.de>
> Reviewed-by: Andra-Irina Paraschiv <andraprs@amazon.com>
> Reviewed-by: Norbert Manthey <nmanthey@amazon.de>

Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

> ---
> v3:
> * Instead of allocating XC_PAGE_SIZE plus size of the len field,
>  allocate one XC_PAGE_SIZE for the whole buildid struct.
> * Modify commit message to reflect the above change.
> v2:
> * No code change
> * Adding maintainers
> ---
>  tools/python/xen/lowlevel/xc/xc.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
> index 522cbe3b9c..7ed4a253de 100644
> --- a/tools/python/xen/lowlevel/xc/xc.c
> +++ b/tools/python/xen/lowlevel/xc/xc.c
> @@ -1211,6 +1211,26 @@ out:
>      return ret_obj ? ret_obj : pyxc_error_to_exception(self->xc_handle);
>  }
>  
> +static PyObject *pyxc_xenbuildid(XcObject *self)
> +{
> +    xen_build_id_t *buildid;
> +    int i, r;
> +    char *str;
> +
> +    buildid = alloca(XC_PAGE_SIZE);
> +    buildid->len = XC_PAGE_SIZE - sizeof(*buildid);
> +
> +    r = xc_version(self->xc_handle, XENVER_build_id, buildid);
> +    if ( r <= 0 )
> +        return pyxc_error_to_exception(self->xc_handle);
> +
> +    str = alloca((r * 2) + 1);
> +    for ( i = 0; i < r; i++ )
> +        snprintf(&str[i * 2], 3, "%02hhx", buildid->buf[i]);
> +
> +    return Py_BuildValue("s", str);
> +}
> +
>  static PyObject *pyxc_xeninfo(XcObject *self)
>  {
>      xen_extraversion_t xen_extra;
> @@ -2294,6 +2314,13 @@ static PyMethodDef pyxc_methods[] = {
>        "Returns [dict]: information about Xen"
>        "        [None]: on failure.\n" },
>  
> +    { "buildid",
> +      (PyCFunction)pyxc_xenbuildid,
> +      METH_NOARGS, "\n"
> +      "Get Xen buildid\n"
> +      "Returns [str]: Xen buildid"
> +      "        [None]: on failure.\n" },
> +
>      { "shadow_control", 
>        (PyCFunction)pyxc_shadow_control, 
>        METH_VARARGS | METH_KEYWORDS, "\n"

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [livepatch: independ. modules 1/3] livepatch: Always check hypervisor build ID upon hotpatch upload

[Konrad Rzeszutek Wilk]

On Tue, Apr 16, 2019 at 12:58:30PM +0000, Pawel Wieczorkiewicz wrote:
> This change is part of a independant stacked hotpatch modules
> feature. This feature allows to bypass dependencies between modules
> upon loading, but still verifies Xen build ID matching.

OK, so build_id_dep would not be called for those?
I see patch #2 doing this. Cool.

> 
> In order to prevent (up)loading any hotpatches built for different
> hypervisor version as indicated by the Xen Build ID, add checking for
> the payload's vs Xen's build id match.
> 
> To achieve that embed into every hotpatch another section with a
> dedicated hypervisor build id in it. After the payload is loaded and
> the .livepatch.xen_depends section becomes available, perform the
> check and reject the payload if there is no match.
> 
> Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
> Reviewed-by: Andra-Irina Paraschiv <andraprs@amazon.com>
> Reviewed-by: Bjoern Doebel <doebel@amazon.de>
> Reviewed-by: Eslam Elnikety <elnikety@amazon.de>
> Reviewed-by: Martin Pohlack <mpohlack@amazon.de>
> ---
>  xen/common/livepatch.c      | 47 +++++++++++++++++++++++++++++++++++++++++++++
>  xen/include/xen/livepatch.h |  7 ++++---

Can you please include:

- Changes to the docs/misc/livepatch.markdown describing this change.
- Include a test-case exercising this.
- Fix the test-cases as I think they will now all fail? (As they don't have this section?)

Thank you.

>  2 files changed, 51 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
> index d6eaae6d3b..6a4af6ce57 100644
> --- a/xen/common/livepatch.c
> +++ b/xen/common/livepatch.c
> @@ -74,6 +74,7 @@ struct payload {
>      unsigned int nsyms;                  /* Nr of entries in .strtab and symbols. */
>      struct livepatch_build_id id;        /* ELFNOTE_DESC(.note.gnu.build-id) of the payload. */
>      struct livepatch_build_id dep;       /* ELFNOTE_DESC(.livepatch.depends). */
> +    struct livepatch_build_id xen_dep;   /* ELFNOTE_DESC(.livepatch.xen_depends). */
>      livepatch_loadcall_t *const *load_funcs;   /* The array of funcs to call after */
>      livepatch_unloadcall_t *const *unload_funcs;/* load and unload of the payload. */
>      unsigned int n_load_funcs;           /* Nr of the funcs to load and execute. */
> @@ -476,11 +477,34 @@ static bool section_ok(const struct livepatch_elf *elf,
>      return true;
>  }
>  
> +static int check_xen_build_id(const struct payload *payload)
> +{
> +    const void *id = NULL;
> +    unsigned int len = 0;
> +    int rc;
> +
> +    ASSERT(payload->xen_dep.len);
> +    ASSERT(payload->xen_dep.p);
> +
> +    rc = xen_build_id(&id, &len);
> +    if ( rc )
> +        return rc;
> +
> +    if ( payload->xen_dep.len != len || memcmp(id, payload->xen_dep.p, len) ) {
> +        dprintk(XENLOG_ERR, "%s%s: check against hypervisor build-id failed!\n",
> +                LIVEPATCH, payload->name);
> +        return -EINVAL;
> +    }
> +
> +    return 0;
> +}
> +
>  static int check_special_sections(const struct livepatch_elf *elf)
>  {
>      unsigned int i;
>      static const char *const names[] = { ELF_LIVEPATCH_FUNC,
>                                           ELF_LIVEPATCH_DEPENDS,
> +                                         ELF_LIVEPATCH_XEN_DEPENDS,
>                                           ELF_BUILD_ID_NOTE};
>      DECLARE_BITMAP(found, ARRAY_SIZE(names)) = { 0 };
>  
> @@ -632,6 +656,22 @@ static int prepare_payload(struct payload *payload,
>              return -EINVAL;
>      }
>  
> +    sec = livepatch_elf_sec_by_name(elf, ELF_LIVEPATCH_XEN_DEPENDS);
> +    if ( sec )
> +    {
> +        n = sec->load_addr;
> +
> +        if ( sec->sec->sh_size <= sizeof(*n) )
> +            return -EINVAL;
> +
> +        if ( xen_build_id_check(n, sec->sec->sh_size,
> +                                &payload->xen_dep.p, &payload->xen_dep.len) )
> +            return -EINVAL;
> +
> +        if ( !payload->xen_dep.len || !payload->xen_dep.p )
> +            return -EINVAL;
> +    }
> +
>      /* Setup the virtual region with proper data. */
>      region = &payload->region;
>  
> @@ -882,6 +922,10 @@ static int load_payload_data(struct payload *payload, void *raw, size_t len)
>      if ( rc )
>          goto out;
>  
> +    rc = check_xen_build_id(payload);
> +    if ( rc )
> +        goto out;
> +
>      rc = build_symbol_table(payload, &elf);
>      if ( rc )
>          goto out;
> @@ -1655,6 +1699,9 @@ static void livepatch_printall(unsigned char key)
>  
>          if ( data->dep.len )
>              printk("depend-on=%*phN\n", data->dep.len, data->dep.p);
> +
> +        if ( data->xen_dep.len )
> +            printk("depend-on-xen=%*phN\n", data->xen_dep.len, data->xen_dep.p);
>      }
>  
>      spin_unlock(&payload_lock);
> diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
> index 1b1817ca0d..ed997aa4cc 100644
> --- a/xen/include/xen/livepatch.h
> +++ b/xen/include/xen/livepatch.h
> @@ -29,9 +29,10 @@ struct xen_sysctl_livepatch_op;
>  /* Convenience define for printk. */
>  #define LIVEPATCH             "livepatch: "
>  /* ELF payload special section names. */
> -#define ELF_LIVEPATCH_FUNC    ".livepatch.funcs"
> -#define ELF_LIVEPATCH_DEPENDS ".livepatch.depends"
> -#define ELF_BUILD_ID_NOTE      ".note.gnu.build-id"
> +#define ELF_LIVEPATCH_FUNC        ".livepatch.funcs"
> +#define ELF_LIVEPATCH_DEPENDS     ".livepatch.depends"
> +#define ELF_LIVEPATCH_XEN_DEPENDS ".livepatch.xen_depends"
> +#define ELF_BUILD_ID_NOTE         ".note.gnu.build-id"
>  /* Arbitrary limit for payload size and .bss section size. */
>  #define LIVEPATCH_MAX_SIZE     MB(2)
>  
> -- 
> 2.16.5
> 
> 
> 
> 
> Amazon Development Center Germany GmbH
> Krausenstr. 38
> 10117 Berlin
> Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
> Ust-ID: DE 289 237 879
> Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
> 
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[livepatch: independ. modules v2 1/3] livepatch: Always check hypervisor build ID upon hotpatch upload

[Pawel Wieczorkiewicz]

This change is part of a independant stacked hotpatch modules
feature. This feature allows to bypass dependencies between modules
upon loading, but still verifies Xen build ID matching.

In order to prevent (up)loading any hotpatches built for different
hypervisor version as indicated by the Xen Build ID, add checking for
the payload's vs Xen's build id match.

To achieve that embed into every hotpatch another section with a
dedicated hypervisor build id in it. After the payload is loaded and
the .livepatch.xen_depends section becomes available, perform the
check and reject the payload if there is no match.

Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
Reviewed-by: Andra-Irina Paraschiv <andraprs@amazon.com>
Reviewed-by: Bjoern Doebel <doebel@amazon.de>
Reviewed-by: Eslam Elnikety <elnikety@amazon.de>
Reviewed-by: Martin Pohlack <mpohlack@amazon.de>
---
v2:
* Updated docs/misc/livepatch.pandoc
* Added xen_no_xen_buildid.livepatch test
* Fixed all tests by adding .livepatch.xen_depends section
---
 .gitignore                  |  1 +
 docs/misc/livepatch.pandoc  | 28 +++++++++++++++++++--------
 xen/common/livepatch.c      | 47 +++++++++++++++++++++++++++++++++++++++++++++
 xen/include/xen/livepatch.h |  7 ++++---
 xen/test/livepatch/Makefile | 31 +++++++++++++++++++++++++-----
 5 files changed, 98 insertions(+), 16 deletions(-)

diff --git a/.gitignore b/.gitignore
index 26bc583f74..f759daeeb7 100644
--- a/.gitignore
+++ b/.gitignore
@@ -326,6 +326,7 @@ xen/test/livepatch/xen_bye_world.livepatch
 xen/test/livepatch/xen_hello_world.livepatch
 xen/test/livepatch/xen_nop.livepatch
 xen/test/livepatch/xen_replace_world.livepatch
+xen/test/livepatch/xen_no_xen_buildid.livepatch
 xen/tools/kconfig/.tmp_gtkcheck
 xen/tools/kconfig/.tmp_qtcheck
 xen/tools/symbols
diff --git a/docs/misc/livepatch.pandoc b/docs/misc/livepatch.pandoc
index 6d9f72f49b..fd1f5d0126 100644
--- a/docs/misc/livepatch.pandoc
+++ b/docs/misc/livepatch.pandoc
@@ -270,6 +270,8 @@ like what the Linux kernel module loader does.
 The payload contains at least three sections:
 
  * `.livepatch.funcs` - which is an array of livepatch_func structures.
+ * `.livepatch.xen_depends` - which is an ELF Note that describes what Xen
+    build-id the payload depends on. **MUST** have one.
  * `.livepatch.depends` - which is an ELF Note that describes what the payload
     depends on. **MUST** have one.
  *  `.note.gnu.build-id` - the build-id of this payload. **MUST** have one.
@@ -383,16 +385,16 @@ The type definition of the function are as follow:
     typedef void (*livepatch_loadcall_t)(void);
     typedef void (*livepatch_unloadcall_t)(void);
 
-### .livepatch.depends and .note.gnu.build-id
+### .livepatch.xen_depends, .livepatch.depends and .note.gnu.build-id
 
 To support dependencies checking and safe loading (to load the
 appropiate payload against the right hypervisor) there is a need
 to embbed an build-id dependency.
 
-This is done by the payload containing an section `.livepatch.depends`
-which follows the format of an ELF Note. The contents of this
-(name, and description) are specific to the linker utilized to
-build the hypevisor and payload.
+This is done by the payload containing sections `.livepatch.xen_depends`
+and `.livepatch.depends` which follow the format of an ELF Note.
+The contents of these (name, and description) are specific to the linker
+utilized to build the hypevisor and payload.
 
 If GNU linker is used then the name is `GNU` and the description
 is a NT_GNU_BUILD_ID type ID. The description can be an SHA1
@@ -400,6 +402,13 @@ checksum, MD5 checksum or any unique value.
 
 The size of these structures varies with the `--build-id` linker option.
 
+There are two kinds of build-id dependencies:
+
+ * Xen build-id dependency (.livepatch.xen_depends section)
+ * previous payload build-id dependency (.livepatch.depends section)
+
+See "Live patch interdependencies" for more information.
+
 ## Hypercalls
 
 We will employ the sub operations of the system management hypercall (sysctl).
@@ -894,13 +903,16 @@ but is more complex to implement.
 The second option which requires an build-id of the hypervisor
 is implemented in the Xen hypervisor.
 
-Specifically each payload has two build-id ELF notes:
+Specifically each payload has three build-id ELF notes:
  * The build-id of the payload itself (generated via --build-id).
+ * The build-id of the Xen hypervisor it depends on (extracted from the
+   hypervisor during build time).
  * The build-id of the payload it depends on (extracted from the
    the previous payload or hypervisor during build time).
 
-This means that the very first payload depends on the hypervisor
-build-id.
+This means that every payload depends on the hypervisor build-id and on
+the build-id of the previous payload in the stack.
+The very first payload depends on the hypervisor build-id only.
 
 # Not Yet Done
 
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index d6eaae6d3b..6a4af6ce57 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -74,6 +74,7 @@ struct payload {
     unsigned int nsyms;                  /* Nr of entries in .strtab and symbols. */
     struct livepatch_build_id id;        /* ELFNOTE_DESC(.note.gnu.build-id) of the payload. */
     struct livepatch_build_id dep;       /* ELFNOTE_DESC(.livepatch.depends). */
+    struct livepatch_build_id xen_dep;   /* ELFNOTE_DESC(.livepatch.xen_depends). */
     livepatch_loadcall_t *const *load_funcs;   /* The array of funcs to call after */
     livepatch_unloadcall_t *const *unload_funcs;/* load and unload of the payload. */
     unsigned int n_load_funcs;           /* Nr of the funcs to load and execute. */
@@ -476,11 +477,34 @@ static bool section_ok(const struct livepatch_elf *elf,
     return true;
 }
 
+static int check_xen_build_id(const struct payload *payload)
+{
+    const void *id = NULL;
+    unsigned int len = 0;
+    int rc;
+
+    ASSERT(payload->xen_dep.len);
+    ASSERT(payload->xen_dep.p);
+
+    rc = xen_build_id(&id, &len);
+    if ( rc )
+        return rc;
+
+    if ( payload->xen_dep.len != len || memcmp(id, payload->xen_dep.p, len) ) {
+        dprintk(XENLOG_ERR, "%s%s: check against hypervisor build-id failed!\n",
+                LIVEPATCH, payload->name);
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
 static int check_special_sections(const struct livepatch_elf *elf)
 {
     unsigned int i;
     static const char *const names[] = { ELF_LIVEPATCH_FUNC,
                                          ELF_LIVEPATCH_DEPENDS,
+                                         ELF_LIVEPATCH_XEN_DEPENDS,
                                          ELF_BUILD_ID_NOTE};
     DECLARE_BITMAP(found, ARRAY_SIZE(names)) = { 0 };
 
@@ -632,6 +656,22 @@ static int prepare_payload(struct payload *payload,
             return -EINVAL;
     }
 
+    sec = livepatch_elf_sec_by_name(elf, ELF_LIVEPATCH_XEN_DEPENDS);
+    if ( sec )
+    {
+        n = sec->load_addr;
+
+        if ( sec->sec->sh_size <= sizeof(*n) )
+            return -EINVAL;
+
+        if ( xen_build_id_check(n, sec->sec->sh_size,
+                                &payload->xen_dep.p, &payload->xen_dep.len) )
+            return -EINVAL;
+
+        if ( !payload->xen_dep.len || !payload->xen_dep.p )
+            return -EINVAL;
+    }
+
     /* Setup the virtual region with proper data. */
     region = &payload->region;
 
@@ -882,6 +922,10 @@ static int load_payload_data(struct payload *payload, void *raw, size_t len)
     if ( rc )
         goto out;
 
+    rc = check_xen_build_id(payload);
+    if ( rc )
+        goto out;
+
     rc = build_symbol_table(payload, &elf);
     if ( rc )
         goto out;
@@ -1655,6 +1699,9 @@ static void livepatch_printall(unsigned char key)
 
         if ( data->dep.len )
             printk("depend-on=%*phN\n", data->dep.len, data->dep.p);
+
+        if ( data->xen_dep.len )
+            printk("depend-on-xen=%*phN\n", data->xen_dep.len, data->xen_dep.p);
     }
 
     spin_unlock(&payload_lock);
diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
index 1b1817ca0d..ed997aa4cc 100644
--- a/xen/include/xen/livepatch.h
+++ b/xen/include/xen/livepatch.h
@@ -29,9 +29,10 @@ struct xen_sysctl_livepatch_op;
 /* Convenience define for printk. */
 #define LIVEPATCH             "livepatch: "
 /* ELF payload special section names. */
-#define ELF_LIVEPATCH_FUNC    ".livepatch.funcs"
-#define ELF_LIVEPATCH_DEPENDS ".livepatch.depends"
-#define ELF_BUILD_ID_NOTE      ".note.gnu.build-id"
+#define ELF_LIVEPATCH_FUNC        ".livepatch.funcs"
+#define ELF_LIVEPATCH_DEPENDS     ".livepatch.depends"
+#define ELF_LIVEPATCH_XEN_DEPENDS ".livepatch.xen_depends"
+#define ELF_BUILD_ID_NOTE         ".note.gnu.build-id"
 /* Arbitrary limit for payload size and .bss section size. */
 #define LIVEPATCH_MAX_SIZE     MB(2)
 
diff --git a/xen/test/livepatch/Makefile b/xen/test/livepatch/Makefile
index 6831383db1..fdb82782d2 100644
--- a/xen/test/livepatch/Makefile
+++ b/xen/test/livepatch/Makefile
@@ -19,11 +19,13 @@ LIVEPATCH := xen_hello_world.livepatch
 LIVEPATCH_BYE := xen_bye_world.livepatch
 LIVEPATCH_REPLACE := xen_replace_world.livepatch
 LIVEPATCH_NOP := xen_nop.livepatch
+LIVEPATCH_NO_XEN_BUILDID := xen_no_xen_buildid.livepatch
 
 LIVEPATCHES += $(LIVEPATCH)
 LIVEPATCHES += $(LIVEPATCH_BYE)
 LIVEPATCHES += $(LIVEPATCH_REPLACE)
 LIVEPATCHES += $(LIVEPATCH_NOP)
+LIVEPATCHES += $(LIVEPATCH_NO_XEN_BUILDID)
 
 LIVEPATCH_DEBUG_DIR ?= $(DEBUG_DIR)/xen-livepatch
 
@@ -59,7 +61,7 @@ config.h: xen_hello_world_func.o
 xen_hello_world.o: config.h
 
 .PHONY: $(LIVEPATCH)
-$(LIVEPATCH): xen_hello_world_func.o xen_hello_world.o note.o
+$(LIVEPATCH): xen_hello_world_func.o xen_hello_world.o note.o xen_note.o
 	$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH) $^
 
 #
@@ -78,6 +80,17 @@ note.o:
 		   --rename-section=.data=.livepatch.depends,alloc,load,readonly,data,contents -S $@.bin $@
 	rm -f $@.bin
 
+#
+# Append .livepatch.xen_depends section
+# with Xen build-id derived from xen-syms.
+#
+.PHONY: xen_note.o
+xen_note.o:
+	$(OBJCOPY) -O binary --only-section=.note.gnu.build-id $(BASEDIR)/xen-syms $@.bin
+	$(OBJCOPY) $(OBJCOPY_MAGIC) \
+		   --rename-section=.data=.livepatch.xen_depends,alloc,load,readonly,data,contents -S $@.bin $@
+	rm -f $@.bin
+
 #
 # Extract the build-id of the xen_hello_world.livepatch
 # (which xen_bye_world will depend on).
@@ -92,20 +105,28 @@ hello_world_note.o: $(LIVEPATCH)
 xen_bye_world.o: config.h
 
 .PHONY: $(LIVEPATCH_BYE)
-$(LIVEPATCH_BYE): xen_bye_world_func.o xen_bye_world.o hello_world_note.o
+$(LIVEPATCH_BYE): xen_bye_world_func.o xen_bye_world.o hello_world_note.o xen_note.o
 	$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_BYE) $^
 
 xen_replace_world.o: config.h
 
 .PHONY: $(LIVEPATCH_REPLACE)
-$(LIVEPATCH_REPLACE): xen_replace_world_func.o xen_replace_world.o note.o
+$(LIVEPATCH_REPLACE): xen_replace_world_func.o xen_replace_world.o note.o xen_note.o
 	$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_REPLACE) $^
 
 xen_nop.o: config.h
 
 .PHONY: $(LIVEPATCH_NOP)
-$(LIVEPATCH_NOP): xen_nop.o note.o
+$(LIVEPATCH_NOP): xen_nop.o note.o xen_note.o
 	$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_NOP) $^
 
+# This one always fails upon upload, because it deliberetely
+# does not have a .livepatch.xen_depends (xen_note.o) section.
+xen_no_xen_buildid.o: config.h
+
+.PHONY: $(LIVEPATCH_NO_XEN_BUILDID)
+$(LIVEPATCH_NO_XEN_BUILDID): xen_nop.o note.o
+	$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_NO_XEN_BUILDID) $^
+
 .PHONY: livepatch
-livepatch: $(LIVEPATCH) $(LIVEPATCH_BYE) $(LIVEPATCH_REPLACE) $(LIVEPATCH_NOP)
+livepatch: $(LIVEPATCH) $(LIVEPATCH_BYE) $(LIVEPATCH_REPLACE) $(LIVEPATCH_NOP) $(LIVEPATCH_NO_XEN_BUILDID)
-- 
2.16.5




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel