| Message ID | 20190820132854.110843-1-wipawel@amazon.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
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 { >
> 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
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,
> 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
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
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 {