[Xen-devel] [replace tabs with spaces] convert tabs into spaces; preserving indentation
Convert tabs into spaces; preserving indentation No functional changes Signed-off-by: Eric DeVolder --- tools/libxc/xc_kexec.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tools/libxc/xc_kexec.c b/tools/libxc/xc_kexec.c index 989e225192..59e2f076f4 100644 --- a/tools/libxc/xc_kexec.c +++ b/tools/libxc/xc_kexec.c @@ -27,8 +27,8 @@ int xc_kexec_exec(xc_interface *xch, int type) exec->type = type; ret = xencall2(xch->xcall, __HYPERVISOR_kexec_op, - KEXEC_CMD_kexec, - HYPERCALL_BUFFER_AS_ARG(exec)); + KEXEC_CMD_kexec, + HYPERCALL_BUFFER_AS_ARG(exec)); out: xc_hypercall_buffer_free(xch, exec); @@ -53,8 +53,8 @@ int xc_kexec_get_range(xc_interface *xch, int range, int nr, get_range->nr = nr; ret = xencall2(xch->xcall, __HYPERVISOR_kexec_op, - KEXEC_CMD_kexec_get_range, - HYPERCALL_BUFFER_AS_ARG(get_range)); + KEXEC_CMD_kexec_get_range, + HYPERCALL_BUFFER_AS_ARG(get_range)); *size = get_range->size; *start = get_range->start; @@ -93,8 +93,8 @@ int xc_kexec_load(xc_interface *xch, uint8_t type, uint16_t arch, set_xen_guest_handle(load->segments.h, segments); ret = xencall2(xch->xcall, __HYPERVISOR_kexec_op, - KEXEC_CMD_kexec_load, - HYPERCALL_BUFFER_AS_ARG(load)); + KEXEC_CMD_kexec_load, + HYPERCALL_BUFFER_AS_ARG(load)); out: xc_hypercall_buffer_free(xch, load); @@ -118,8 +118,8 @@ int xc_kexec_unload(xc_interface *xch, int type) unload->type = type; ret = xencall2(xch->xcall, __HYPERVISOR_kexec_op, - KEXEC_CMD_kexec_unload, - HYPERCALL_BUFFER_AS_ARG(unload)); + KEXEC_CMD_kexec_unload, + HYPERCALL_BUFFER_AS_ARG(unload)); out: xc_hypercall_buffer_free(xch, unload); -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH] convert tabs into spaces; preserving indentation
Convert tabs into spaces; preserving indentation No functional changes Signed-off-by: Eric DeVolder --- tools/libxc/xc_kexec.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tools/libxc/xc_kexec.c b/tools/libxc/xc_kexec.c index 989e225192..59e2f076f4 100644 --- a/tools/libxc/xc_kexec.c +++ b/tools/libxc/xc_kexec.c @@ -27,8 +27,8 @@ int xc_kexec_exec(xc_interface *xch, int type) exec->type = type; ret = xencall2(xch->xcall, __HYPERVISOR_kexec_op, - KEXEC_CMD_kexec, - HYPERCALL_BUFFER_AS_ARG(exec)); + KEXEC_CMD_kexec, + HYPERCALL_BUFFER_AS_ARG(exec)); out: xc_hypercall_buffer_free(xch, exec); @@ -53,8 +53,8 @@ int xc_kexec_get_range(xc_interface *xch, int range, int nr, get_range->nr = nr; ret = xencall2(xch->xcall, __HYPERVISOR_kexec_op, - KEXEC_CMD_kexec_get_range, - HYPERCALL_BUFFER_AS_ARG(get_range)); + KEXEC_CMD_kexec_get_range, + HYPERCALL_BUFFER_AS_ARG(get_range)); *size = get_range->size; *start = get_range->start; @@ -93,8 +93,8 @@ int xc_kexec_load(xc_interface *xch, uint8_t type, uint16_t arch, set_xen_guest_handle(load->segments.h, segments); ret = xencall2(xch->xcall, __HYPERVISOR_kexec_op, - KEXEC_CMD_kexec_load, - HYPERCALL_BUFFER_AS_ARG(load)); + KEXEC_CMD_kexec_load, + HYPERCALL_BUFFER_AS_ARG(load)); out: xc_hypercall_buffer_free(xch, load); @@ -118,8 +118,8 @@ int xc_kexec_unload(xc_interface *xch, int type) unload->type = type; ret = xencall2(xch->xcall, __HYPERVISOR_kexec_op, - KEXEC_CMD_kexec_unload, - HYPERCALL_BUFFER_AS_ARG(unload)); + KEXEC_CMD_kexec_unload, + HYPERCALL_BUFFER_AS_ARG(unload)); out: xc_hypercall_buffer_free(xch, unload); -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2] kexec: implement STATUS hypercall to check if image is loaded
The tools that use kexec are asynchronous in nature and do not keep
state changes. As such provide an hypercall to find out whether an
image has been loaded for either type.
Note: No need to modify XSM as it has one size fits all check and
does not check for subcommands.
Note: No need to check KEXEC_FLAG_IN_PROGRESS (and error out of
kexec_status()) as this flag is set only once by the first/only
cpu on the crash path.
Note: The __XEN_LATEST_INTERFACE_VERSION__ has been bumped to
0x00040900 due to the introduction of a new hypervisor call.
Note: This is just the Xen side of the hypercall, kexec-tools patch
to come separately.
Signed-off-by: Konrad Rzeszutek Wilk
Signed-off-by: Eric DeVolder
Reviewed-by: Daniel Kiper
---
CC: Elena Ufimtseva
CC: Daniel Kiper
v0: Internal version.
v1: Dropped Reviewed-by, posting on xen-devel.
v2: Incorporated xen-devel feedback
---
tools/libxc/include/xenctrl.h | 10 ++
tools/libxc/xc_kexec.c | 24
xen/common/kexec.c | 19 +++
xen/include/public/kexec.h | 13 +
xen/include/public/xen-compat.h | 2 +-
5 files changed, 67 insertions(+), 1 deletion(-)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 4ab0f57117..63c616ff6a 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2574,6 +2574,16 @@ int xc_kexec_load(xc_interface *xch, uint8_t type,
uint16_t arch,
*/
int xc_kexec_unload(xc_interface *xch, int type);
+/*
+ * Find out whether the image has been succesfully loaded.
+ *
+ * The type can be either KEXEC_TYPE_DEFAULT or KEXEC_TYPE_CRASH.
+ * If zero is returned, that means no image is loaded for the type.
+ * If one is returned, that means an image is loaded for the type.
+ * Otherwise, negative return value indicates error.
+ */
+int xc_kexec_status(xc_interface *xch, int type);
+
typedef xenpf_resource_entry_t xc_resource_entry_t;
/*
diff --git a/tools/libxc/xc_kexec.c b/tools/libxc/xc_kexec.c
index 989e225192..a0a9fd3841 100644
--- a/tools/libxc/xc_kexec.c
+++ b/tools/libxc/xc_kexec.c
@@ -126,3 +126,27 @@ out:
return ret;
}
+
+int xc_kexec_status(xc_interface *xch, int type)
+{
+DECLARE_HYPERCALL_BUFFER(xen_kexec_status_t, status);
+int ret = -1;
+
+status = xc_hypercall_buffer_alloc(xch, status, sizeof(*status));
+if ( status == NULL )
+{
+PERROR("Could not alloc buffer for kexec status hypercall");
+goto out;
+}
+
+status->type = type;
+
+ret = xencall2(xch->xcall, __HYPERVISOR_kexec_op,
+ KEXEC_CMD_kexec_status,
+ HYPERCALL_BUFFER_AS_ARG(status));
+
+out:
+xc_hypercall_buffer_free(xch, status);
+
+return ret;
+}
diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index c83d48fc79..aa808cb2f2 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -1169,6 +1169,22 @@ static int kexec_unload(XEN_GUEST_HANDLE_PARAM(void)
uarg)
return kexec_do_unload(&unload);
}
+static int kexec_status(XEN_GUEST_HANDLE_PARAM(void) uarg)
+{
+xen_kexec_status_t status;
+int base, bit;
+
+if ( unlikely(copy_from_guest(&status, uarg, 1)) )
+return -EFAULT;
+
+/* No need to check KEXEC_FLAG_IN_PROGRESS. */
+
+if ( kexec_load_get_bits(status.type, &base, &bit) )
+return -EINVAL;
+
+return test_bit(bit, &kexec_flags);
+}
+
static int do_kexec_op_internal(unsigned long op,
XEN_GUEST_HANDLE_PARAM(void) uarg,
bool_t compat)
@@ -1208,6 +1224,9 @@ static int do_kexec_op_internal(unsigned long op,
case KEXEC_CMD_kexec_unload:
ret = kexec_unload(uarg);
break;
+case KEXEC_CMD_kexec_status:
+ret = kexec_status(uarg);
+break;
}
return ret;
diff --git a/xen/include/public/kexec.h b/xen/include/public/kexec.h
index a6a0a88f4f..c200e8ceee 100644
--- a/xen/include/public/kexec.h
+++ b/xen/include/public/kexec.h
@@ -227,6 +227,19 @@ typedef struct xen_kexec_unload {
} xen_kexec_unload_t;
DEFINE_XEN_GUEST_HANDLE(xen_kexec_unload_t);
+/*
+ * Figure out whether we have an image loaded. A return value of
+ * zero indicates no image loaded. A return value of one
+ * indicates an image is loaded. A negative return value
+ * indicates an error.
+ *
+ * Type must be one of KEXEC_TYPE_DEFAULT or KEXEC_TYPE_CRASH.
+ */
+#define KEXEC_CMD_kexec_status 6
+typedef struct xen_kexec_status {
+uint8_t type;
+} xen_kexec_status_t;
+DEFINE_XEN_GUEST_HANDLE(xen_kexec_status_t);
#else /* __XEN_INTERFACE_VERSION__ < 0x00040400 */
#define KEXEC_CMD_kexec_load KEXEC_CMD_kexec_load_v1
diff --git a/xen/include/public/xen-compat.h b/xen/include/public/xen-compat.h
index dd8a5c0d0e..b67365340b 100644
--- a/xen/include/public/xen-compat.h
+++ b/xen/include/public/xen-compat.h
@@ -27,7 +27,7 @@
#ifndef __XEN_PUBLIC_XEN_COMPAT_H__
#define
[Xen-devel] [PATCH v3] kexec: implement STATUS hypercall to check if image is loaded
The tools that use kexec are asynchronous in nature and do not keep
state changes. As such provide an hypercall to find out whether an
image has been loaded for either type.
Note: No need to modify XSM as it has one size fits all check and
does not check for subcommands.
Note: No need to check KEXEC_FLAG_IN_PROGRESS (and error out of
kexec_status()) as this flag is set only once by the first/only
cpu on the crash path.
Note: This is just the Xen side of the hypercall, kexec-tools patch
to come separately.
Signed-off-by: Konrad Rzeszutek Wilk
Signed-off-by: Eric DeVolder
---
CC: Andrew Cooper
CC: Elena Ufimtseva
CC: Daniel Kiper
v0: Internal version.
v1: Dropped Reviewed-by, posting on xen-devel.
v2: Incorporated xen-devel feedback, re-posted on xen-devel.
v3: Incorporated xen-devel feedback
---
tools/libxc/include/xenctrl.h | 10 ++
tools/libxc/xc_kexec.c| 24
xen/common/kexec.c| 19 +++
xen/include/public/kexec.h| 13 +
4 files changed, 66 insertions(+)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 4ab0f57..63c616f 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2574,6 +2574,16 @@ int xc_kexec_load(xc_interface *xch, uint8_t type,
uint16_t arch,
*/
int xc_kexec_unload(xc_interface *xch, int type);
+/*
+ * Find out whether the image has been succesfully loaded.
+ *
+ * The type can be either KEXEC_TYPE_DEFAULT or KEXEC_TYPE_CRASH.
+ * If zero is returned, that means no image is loaded for the type.
+ * If one is returned, that means an image is loaded for the type.
+ * Otherwise, negative return value indicates error.
+ */
+int xc_kexec_status(xc_interface *xch, int type);
+
typedef xenpf_resource_entry_t xc_resource_entry_t;
/*
diff --git a/tools/libxc/xc_kexec.c b/tools/libxc/xc_kexec.c
index 59e2f07..a4e8966 100644
--- a/tools/libxc/xc_kexec.c
+++ b/tools/libxc/xc_kexec.c
@@ -126,3 +126,27 @@ out:
return ret;
}
+
+int xc_kexec_status(xc_interface *xch, int type)
+{
+DECLARE_HYPERCALL_BUFFER(xen_kexec_status_t, status);
+int ret = -1;
+
+status = xc_hypercall_buffer_alloc(xch, status, sizeof(*status));
+if ( status == NULL )
+{
+PERROR("Could not alloc buffer for kexec status hypercall");
+goto out;
+}
+
+status->type = type;
+
+ret = xencall2(xch->xcall, __HYPERVISOR_kexec_op,
+ KEXEC_CMD_kexec_status,
+ HYPERCALL_BUFFER_AS_ARG(status));
+
+out:
+xc_hypercall_buffer_free(xch, status);
+
+return ret;
+}
diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index c83d48f..aa808cb 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -1169,6 +1169,22 @@ static int kexec_unload(XEN_GUEST_HANDLE_PARAM(void)
uarg)
return kexec_do_unload(&unload);
}
+static int kexec_status(XEN_GUEST_HANDLE_PARAM(void) uarg)
+{
+xen_kexec_status_t status;
+int base, bit;
+
+if ( unlikely(copy_from_guest(&status, uarg, 1)) )
+return -EFAULT;
+
+/* No need to check KEXEC_FLAG_IN_PROGRESS. */
+
+if ( kexec_load_get_bits(status.type, &base, &bit) )
+return -EINVAL;
+
+return test_bit(bit, &kexec_flags);
+}
+
static int do_kexec_op_internal(unsigned long op,
XEN_GUEST_HANDLE_PARAM(void) uarg,
bool_t compat)
@@ -1208,6 +1224,9 @@ static int do_kexec_op_internal(unsigned long op,
case KEXEC_CMD_kexec_unload:
ret = kexec_unload(uarg);
break;
+case KEXEC_CMD_kexec_status:
+ret = kexec_status(uarg);
+break;
}
return ret;
diff --git a/xen/include/public/kexec.h b/xen/include/public/kexec.h
index a6a0a88..c200e8c 100644
--- a/xen/include/public/kexec.h
+++ b/xen/include/public/kexec.h
@@ -227,6 +227,19 @@ typedef struct xen_kexec_unload {
} xen_kexec_unload_t;
DEFINE_XEN_GUEST_HANDLE(xen_kexec_unload_t);
+/*
+ * Figure out whether we have an image loaded. A return value of
+ * zero indicates no image loaded. A return value of one
+ * indicates an image is loaded. A negative return value
+ * indicates an error.
+ *
+ * Type must be one of KEXEC_TYPE_DEFAULT or KEXEC_TYPE_CRASH.
+ */
+#define KEXEC_CMD_kexec_status 6
+typedef struct xen_kexec_status {
+uint8_t type;
+} xen_kexec_status_t;
+DEFINE_XEN_GUEST_HANDLE(xen_kexec_status_t);
#else /* __XEN_INTERFACE_VERSION__ < 0x00040400 */
#define KEXEC_CMD_kexec_load KEXEC_CMD_kexec_load_v1
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4] kexec: implement STATUS hypercall to check if image is loaded
The tools that use kexec are asynchronous in nature and do not keep
state changes. As such provide an hypercall to find out whether an
image has been loaded for either type.
Note: No need to modify XSM as it has one size fits all check and
does not check for subcommands.
Note: No need to check KEXEC_FLAG_IN_PROGRESS (and error out of
kexec_status()) as this flag is set only once by the first/only
cpu on the crash path.
Note: In kexec_status(), the use of test_bit() can also return
EPERM, so the return value from test_bit() must be checked to
ensure that kexec_status() always returns 0, 1 or -1, per the
public header description.
Note: This is just the Xen side of the hypercall, kexec-tools patch
to come separately.
Signed-off-by: Konrad Rzeszutek Wilk
Signed-off-by: Eric DeVolder
---
CC: Andrew Cooper
CC: Elena Ufimtseva
CC: Daniel Kiper
v0: Internal version.
v1: Dropped Reviewed-by, posting on xen-devel.
v2: Incorporated xen-devel feedback, re-posted on xen-devel.
v3: Incorporated xen-devel feedback, re-posted on xen-devel.
v4: Incorporated xen-devel feedback
---
tools/libxc/include/xenctrl.h | 10 ++
tools/libxc/xc_kexec.c| 24
xen/common/kexec.c| 19 +++
xen/include/public/kexec.h| 14 ++
4 files changed, 67 insertions(+)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 4ab0f57..63c616f 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2574,6 +2574,16 @@ int xc_kexec_load(xc_interface *xch, uint8_t type,
uint16_t arch,
*/
int xc_kexec_unload(xc_interface *xch, int type);
+/*
+ * Find out whether the image has been succesfully loaded.
+ *
+ * The type can be either KEXEC_TYPE_DEFAULT or KEXEC_TYPE_CRASH.
+ * If zero is returned, that means no image is loaded for the type.
+ * If one is returned, that means an image is loaded for the type.
+ * Otherwise, negative return value indicates error.
+ */
+int xc_kexec_status(xc_interface *xch, int type);
+
typedef xenpf_resource_entry_t xc_resource_entry_t;
/*
diff --git a/tools/libxc/xc_kexec.c b/tools/libxc/xc_kexec.c
index 59e2f07..a4e8966 100644
--- a/tools/libxc/xc_kexec.c
+++ b/tools/libxc/xc_kexec.c
@@ -126,3 +126,27 @@ out:
return ret;
}
+
+int xc_kexec_status(xc_interface *xch, int type)
+{
+DECLARE_HYPERCALL_BUFFER(xen_kexec_status_t, status);
+int ret = -1;
+
+status = xc_hypercall_buffer_alloc(xch, status, sizeof(*status));
+if ( status == NULL )
+{
+PERROR("Could not alloc buffer for kexec status hypercall");
+goto out;
+}
+
+status->type = type;
+
+ret = xencall2(xch->xcall, __HYPERVISOR_kexec_op,
+ KEXEC_CMD_kexec_status,
+ HYPERCALL_BUFFER_AS_ARG(status));
+
+out:
+xc_hypercall_buffer_free(xch, status);
+
+return ret;
+}
diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index c83d48f..40b76d5 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -1169,6 +1169,22 @@ static int kexec_unload(XEN_GUEST_HANDLE_PARAM(void)
uarg)
return kexec_do_unload(&unload);
}
+static int kexec_status(XEN_GUEST_HANDLE_PARAM(void) uarg)
+{
+xen_kexec_status_t status;
+int base, bit;
+
+if ( unlikely(copy_from_guest(&status, uarg, 1)) )
+return -EFAULT;
+
+/* No need to check KEXEC_FLAG_IN_PROGRESS. */
+
+if ( kexec_load_get_bits(status.type, &base, &bit) )
+return -EINVAL;
+
+return (test_bit(bit, &kexec_flags) == 1);
+}
+
static int do_kexec_op_internal(unsigned long op,
XEN_GUEST_HANDLE_PARAM(void) uarg,
bool_t compat)
@@ -1208,6 +1224,9 @@ static int do_kexec_op_internal(unsigned long op,
case KEXEC_CMD_kexec_unload:
ret = kexec_unload(uarg);
break;
+case KEXEC_CMD_kexec_status:
+ret = kexec_status(uarg);
+break;
}
return ret;
diff --git a/xen/include/public/kexec.h b/xen/include/public/kexec.h
index a6a0a88..74ea981 100644
--- a/xen/include/public/kexec.h
+++ b/xen/include/public/kexec.h
@@ -227,6 +227,20 @@ typedef struct xen_kexec_unload {
} xen_kexec_unload_t;
DEFINE_XEN_GUEST_HANDLE(xen_kexec_unload_t);
+/*
+ * Figure out whether we have an image loaded. A return value of
+ * zero indicates no image loaded. A return value of one
+ * indicates an image is loaded. A negative return value
+ * indicates an error.
+ *
+ * Type must be one of KEXEC_TYPE_DEFAULT or KEXEC_TYPE_CRASH.
+ */
+#define KEXEC_CMD_kexec_status 6
+typedef struct xen_kexec_status {
+uint8_t type;
+} xen_kexec_status_t;
+DEFINE_XEN_GUEST_HANDLE(xen_kexec_status_t);
+
#else /* __XEN_INTERFACE_VERSION__ < 0x00040400 */
#define KEXEC_CMD_kexec_load KEXEC_CMD_kexec_load_v1
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3] kexec: implement STATUS hypercall to check if image is loaded
On 01/18/2017 04:47 AM, Wei Liu wrote:
On Wed, Jan 18, 2017 at 03:45:54AM -0700, Jan Beulich wrote:
On 18.01.17 at 11:37, wrote:
On Wed, Jan 18, 2017 at 03:19:49AM -0700, Jan Beulich wrote:
On 17.01.17 at 18:29, wrote:
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -1169,6 +1169,22 @@ static int kexec_unload(XEN_GUEST_HANDLE_PARAM(void)
uarg)
return kexec_do_unload(&unload);
}
+static int kexec_status(XEN_GUEST_HANDLE_PARAM(void) uarg)
+{
+xen_kexec_status_t status;
+int base, bit;
+
+if ( unlikely(copy_from_guest(&status, uarg, 1)) )
+return -EFAULT;
+
+/* No need to check KEXEC_FLAG_IN_PROGRESS. */
+
+if ( kexec_load_get_bits(status.type, &base, &bit) )
+return -EINVAL;
+
+return test_bit(bit, &kexec_flags);
In the public header you promise to return zero or one here (unless
an error occurs), which requires the use of !!. Please see x86's
implementation of the function for how/when there can actually be
other non-zero values returned here (in particular all ones, which
would resolve to -EPERM).
--- a/xen/include/public/kexec.h
+++ b/xen/include/public/kexec.h
@@ -227,6 +227,19 @@ typedef struct xen_kexec_unload {
} xen_kexec_unload_t;
DEFINE_XEN_GUEST_HANDLE(xen_kexec_unload_t);
+/*
+ * Figure out whether we have an image loaded. A return value of
+ * zero indicates no image loaded. A return value of one
+ * indicates an image is loaded. A negative return value
+ * indicates an error.
+ *
+ * Type must be one of KEXEC_TYPE_DEFAULT or KEXEC_TYPE_CRASH.
+ */
+#define KEXEC_CMD_kexec_status 6
+typedef struct xen_kexec_status {
+uint8_t type;
+} xen_kexec_status_t;
+DEFINE_XEN_GUEST_HANDLE(xen_kexec_status_t);
#else /* __XEN_INTERFACE_VERSION__ < 0x00040400 */
There was a blank line above here before your addition, and you
shouldn't eliminate it (making quickly scanning over the file harder).
I guess both items are simple enough to fix while committing.
Oops, I already committed this patch with Andrew's review. A follow-up
patch is appreciated. Thanks.
Well, I suppose that was directed at Eric ...
Yes. That was for Eric.
Wei.
Hi. I have made the two changes and recreated the entire patch.
It has been posted as v4.
If a delta patch against v3 was what was desired, then please
let me know and I'll provide.
Note my handling of the test_bit() scenario is to explicitly check
for return value of 1, so any value other than 1 returns 0.
Regards,
Eric
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] Followup corrections to kexec STATUS patch v3
This contains the two corrections pointed out by Jan Beulich for the kexec STATUS call just introduced. Note: In kexec_status(), the use of test_bit() can also return EPERM, so the return value from test_bit() must be checked to ensure that kexec_status() always returns 0, 1 or -1, per the public header description. Note: My handling of the test_bit() scenario is to explicitly check for return value of 1, so any value other than 1 causes kexec_status to return 0. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH] kexec: ensure kexec_status() always returns 0 or 1
The use of test_bit() can also return EPERM, so the return value from test_bit() must be checked to ensure that kexec_status() always returns 0, 1 or -1, per the public header description. --- xen/common/kexec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/common/kexec.c b/xen/common/kexec.c index aa808cb..40b76d5 100644 --- a/xen/common/kexec.c +++ b/xen/common/kexec.c @@ -1182,7 +1182,7 @@ static int kexec_status(XEN_GUEST_HANDLE_PARAM(void) uarg) if ( kexec_load_get_bits(status.type, &base, &bit) ) return -EINVAL; -return test_bit(bit, &kexec_flags); +return (test_bit(bit, &kexec_flags) == 1); } static int do_kexec_op_internal(unsigned long op, -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH] Put back blank line for readability purposes.
This blank line was accidentally removed during
the insertion of the kexec_status() declarations.
---
xen/include/public/kexec.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/xen/include/public/kexec.h b/xen/include/public/kexec.h
index c200e8c..74ea981 100644
--- a/xen/include/public/kexec.h
+++ b/xen/include/public/kexec.h
@@ -240,6 +240,7 @@ typedef struct xen_kexec_status {
uint8_t type;
} xen_kexec_status_t;
DEFINE_XEN_GUEST_HANDLE(xen_kexec_status_t);
+
#else /* __XEN_INTERFACE_VERSION__ < 0x00040400 */
#define KEXEC_CMD_kexec_load KEXEC_CMD_kexec_load_v1
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4] kexec: implement STATUS hypercall to check if image is loaded
On 01/18/2017 11:47 AM, Wei Liu wrote: On Wed, Jan 18, 2017 at 11:46:13AM -0600, Eric DeVolder wrote: The tools that use kexec are asynchronous in nature and do not keep state changes. As such provide an hypercall to find out whether an image has been loaded for either type. Note: No need to modify XSM as it has one size fits all check and does not check for subcommands. Note: No need to check KEXEC_FLAG_IN_PROGRESS (and error out of kexec_status()) as this flag is set only once by the first/only cpu on the crash path. Note: In kexec_status(), the use of test_bit() can also return EPERM, so the return value from test_bit() must be checked to ensure that kexec_status() always returns 0, 1 or -1, per the public header description. Note: This is just the Xen side of the hypercall, kexec-tools patch to come separately. Signed-off-by: Konrad Rzeszutek Wilk Signed-off-by: Eric DeVolder Your v3 is already applied to staging. Please submit a patch on top of staging branch. Wei. Wei, I just sent those, subject line 'Followup corrections to kexec STATUS' eric ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH] kexec: followup to STATUS patch v3 already in staging
This contains the two corrections pointed out by Jan Beulich for the kexec STATUS call just introduced. Note: In kexec_status(), the use of test_bit() can also return EPERM, so the return value from test_bit() must be checked to ensure that kexec_status() always returns 0, 1 or -1, per the public header description. Note: My handling of the test_bit() scenario is to explicitly check for return value of 1, so any value other than 1 causes kexec_status to return 0. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 2/2] kexec: ensure kexec_status() always returns 0 or 1
The use of test_bit() can also return EPERM, so the return value from test_bit() must be checked to ensure that kexec_status() always returns 0, 1 or -1, per the public header description. Signed-off-by: Eric DeVolder --- xen/common/kexec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/common/kexec.c b/xen/common/kexec.c index aa808cb..40b76d5 100644 --- a/xen/common/kexec.c +++ b/xen/common/kexec.c @@ -1182,7 +1182,7 @@ static int kexec_status(XEN_GUEST_HANDLE_PARAM(void) uarg) if ( kexec_load_get_bits(status.type, &base, &bit) ) return -EINVAL; -return test_bit(bit, &kexec_flags); +return (test_bit(bit, &kexec_flags) == 1); } static int do_kexec_op_internal(unsigned long op, -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 1/2] Put back blank line for readability purposes.
This blank line was accidentally removed during
the insertion of the kexec_status() declarations.
Signed-off-by: Eric DeVolder
---
xen/include/public/kexec.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/xen/include/public/kexec.h b/xen/include/public/kexec.h
index c200e8c..74ea981 100644
--- a/xen/include/public/kexec.h
+++ b/xen/include/public/kexec.h
@@ -240,6 +240,7 @@ typedef struct xen_kexec_status {
uint8_t type;
} xen_kexec_status_t;
DEFINE_XEN_GUEST_HANDLE(xen_kexec_status_t);
+
#else /* __XEN_INTERFACE_VERSION__ < 0x00040400 */
#define KEXEC_CMD_kexec_load KEXEC_CMD_kexec_load_v1
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 1/1] kexec: ensure kexec_status() return bit value of 0 or 1
When checking kexec_flags bit corresponding to the requested image, ensure that 0 or 1 is returned. Signed-off-by: Eric DeVolder --- xen/common/kexec.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/xen/common/kexec.c b/xen/common/kexec.c index aa808cb..3da27bf 100644 --- a/xen/common/kexec.c +++ b/xen/common/kexec.c @@ -1182,7 +1182,8 @@ static int kexec_status(XEN_GUEST_HANDLE_PARAM(void) uarg) if ( kexec_load_get_bits(status.type, &base, &bit) ) return -EINVAL; -return test_bit(bit, &kexec_flags); +/* Ensure return bit value of 0 or 1 */ +return !!test_bit(bit, &kexec_flags); } static int do_kexec_op_internal(unsigned long op, -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 0/1] Followup corrections for kexec_status()
Jan, This corrects the use of test_bit() in kexec_status(). Wei, This patch is against the kexec_status() recently applied to staging. Regards, Eric ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2] kexec: implemented XEN KEXEC STATUS to determine if an image is loaded
Instead of the scripts having to poke at various fields we can
provide that functionality via the -S parameter.
Returns 0 if the payload is loaded. Can be used in combination
with -l or -p to get the state of the proper kexec image.
Signed-off-by: Konrad Rzeszutek Wilk
Signed-off-by: Eric DeVolder
---
Note: The corresponding Xen changes have been committed
to the Xen staging branch. Follow this thread:
https://lists.xenproject.org/archives/html/xen-devel/2017-01/msg01570.html
CC: Andrew Cooper
CC: ke...@lists.infradead.org
CC: xen-de...@lists.xenproject.org
CC: Daniel Kiper
v0: First version (internal product).
v1: Posted on kexec mailing list. Changed -s to -S
v2: Incorporated feedback from kexec mailing list
---
configure.ac | 8 ++-
kexec/kexec-xen.c | 26 +++
kexec/kexec.8 | 6 ++
kexec/kexec.c | 62 ---
kexec/kexec.h | 5 -
5 files changed, 98 insertions(+), 9 deletions(-)
diff --git a/configure.ac b/configure.ac
index 3044185..c6e864b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -165,8 +165,14 @@ fi
dnl find Xen control stack libraries
if test "$with_xen" = yes ; then
AC_CHECK_HEADER(xenctrl.h,
- [AC_CHECK_LIB(xenctrl, xc_kexec_load, ,
+ [AC_CHECK_LIB(xenctrl, xc_kexec_load, [ have_xenctrl_h=yes ],
AC_MSG_NOTICE([Xen support disabled]))])
+if test "$have_xenctrl_h" = yes ; then
+ AC_CHECK_LIB(xenctrl, xc_kexec_status,
+ AC_DEFINE(HAVE_KEXEC_CMD_STATUS, 1,
+ [The kexec_status call is available]),
+ AC_MSG_NOTICE([The kexec_status call is not available]))
+fi
fi
dnl ---Sanity checks
diff --git a/kexec/kexec-xen.c b/kexec/kexec-xen.c
index 24a4191..2b448d3 100644
--- a/kexec/kexec-xen.c
+++ b/kexec/kexec-xen.c
@@ -105,6 +105,27 @@ int xen_kexec_unload(uint64_t kexec_flags)
return ret;
}
+int xen_kexec_status(uint64_t kexec_flags)
+{
+ xc_interface *xch;
+ uint8_t type;
+ int ret = -1;
+
+#ifdef HAVE_KEXEC_CMD_STATUS
+ xch = xc_interface_open(NULL, NULL, 0);
+ if (!xch)
+ return -1;
+
+ type = (kexec_flags & KEXEC_ON_CRASH) ? KEXEC_TYPE_CRASH :
KEXEC_TYPE_DEFAULT;
+
+ ret = xc_kexec_status(xch, type);
+
+ xc_interface_close(xch);
+#endif
+
+ return ret;
+}
+
void xen_kexec_exec(void)
{
xc_interface *xch;
@@ -130,6 +151,11 @@ int xen_kexec_unload(uint64_t kexec_flags)
return -1;
}
+int xen_kexec_status(uint64_t kexec_flags)
+{
+ return -1;
+}
+
void xen_kexec_exec(void)
{
}
diff --git a/kexec/kexec.8 b/kexec/kexec.8
index 4d0c1d1..f4b39a6 100644
--- a/kexec/kexec.8
+++ b/kexec/kexec.8
@@ -107,6 +107,12 @@ command:
.B \-d\ (\-\-debug)
Enable debugging messages.
.TP
+.B \-S\ (\-\-status)
+Return 0 if the type (by default crash) is loaded. Can be used in conjuction
+with -l or -p to toggle the type. Note this option supersedes other options
+and it will
+.BR not\ load\ or\ unload\ the\ kernel.
+.TP
.B \-e\ (\-\-exec)
Run the currently loaded kernel. Note that it will reboot into the loaded
kernel without calling shutdown(8).
.TP
diff --git a/kexec/kexec.c b/kexec/kexec.c
index 500e5a9..defbbe3 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -51,6 +51,9 @@
#include "kexec-lzma.h"
#include
+#define KEXEC_LOADED_PATH "/sys/kernel/kexec_loaded"
+#define KEXEC_CRASH_LOADED_PATH "/sys/kernel/kexec_crash_loaded"
+
unsigned long long mem_min = 0;
unsigned long long mem_max = ULONG_MAX;
static unsigned long kexec_flags = 0;
@@ -58,6 +61,8 @@ static unsigned long kexec_flags = 0;
static unsigned long kexec_file_flags = 0;
int kexec_debug = 0;
+static int kexec_loaded(const char *file);
+
void dbgprint_mem_range(const char *prefix, struct memory_range *mr, int nr_mr)
{
int i;
@@ -890,8 +895,6 @@ static int my_exec(void)
return -1;
}
-static int kexec_loaded(void);
-
static int load_jump_back_helper_image(unsigned long kexec_flags, void *entry)
{
int result;
@@ -909,7 +912,7 @@ static int my_load_jump_back_helper(unsigned long
kexec_flags, void *entry)
{
int result;
- if (kexec_loaded()) {
+ if (kexec_loaded(KEXEC_LOADED_PATH)) {
fprintf(stderr, "There is kexec kernel loaded, make sure "
"you are in kexeced kernel.\n");
return -1;
@@ -970,6 +973,7 @@ void usage(void)
" to original kernel.\n"
" -s, --kexec-file-syscall Use file based syscall for kexec
operation\n"
" -d, --debug Enable debugging to help spot a
failure.\n"
+ " -S, --status Return 0 if the type (by default crash)
is loaded.\n"
"\n"
&q
[Xen-devel] [PATCH v3] kexec: implemented XEN KEXEC STATUS to determine if an image is loaded
Instead of the scripts having to poke at various fields we can
provide that functionality via the -S parameter.
kexec_loaded/kexec_crash_loaded exposes Linux kernel kexec/crash
state. It does not say anything about Xen kexec/crash state. So,
we need a special approach to get the latter. Though for
compatibility we provide similar functionality in kexec-tools
for the former.
This change enables the --status or -S option to work either
with or without Xen.
Returns 0 if the payload is loaded. Can be used in combination
with -l or -p to get the state of the proper kexec image.
Signed-off-by: Konrad Rzeszutek Wilk
Signed-off-by: Eric DeVolder
---
Note: The corresponding Xen changes have been committed
to the Xen staging branch. Follow this thread:
https://lists.xenproject.org/archives/html/xen-devel/2017-01/msg01570.html
CC: Andrew Cooper
CC: ke...@lists.infradead.org
CC: xen-de...@lists.xenproject.org
CC: Daniel Kiper
v0: First version (internal product).
v1: Posted on kexec mailing list. Changed -s to -S
v2: Incorporated feedback from kexec mailing list, posted on kexec mailing list
v3: Incorporated feedback from kexec mailing list
---
configure.ac | 8 ++-
kexec/kexec-xen.c | 26 +++
kexec/kexec.8 | 6 ++
kexec/kexec.c | 62 ---
kexec/kexec.h | 5 -
5 files changed, 98 insertions(+), 9 deletions(-)
diff --git a/configure.ac b/configure.ac
index 3044185..c6e864b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -165,8 +165,14 @@ fi
dnl find Xen control stack libraries
if test "$with_xen" = yes ; then
AC_CHECK_HEADER(xenctrl.h,
- [AC_CHECK_LIB(xenctrl, xc_kexec_load, ,
+ [AC_CHECK_LIB(xenctrl, xc_kexec_load, [ have_xenctrl_h=yes ],
AC_MSG_NOTICE([Xen support disabled]))])
+if test "$have_xenctrl_h" = yes ; then
+ AC_CHECK_LIB(xenctrl, xc_kexec_status,
+ AC_DEFINE(HAVE_KEXEC_CMD_STATUS, 1,
+ [The kexec_status call is available]),
+ AC_MSG_NOTICE([The kexec_status call is not available]))
+fi
fi
dnl ---Sanity checks
diff --git a/kexec/kexec-xen.c b/kexec/kexec-xen.c
index 24a4191..2b448d3 100644
--- a/kexec/kexec-xen.c
+++ b/kexec/kexec-xen.c
@@ -105,6 +105,27 @@ int xen_kexec_unload(uint64_t kexec_flags)
return ret;
}
+int xen_kexec_status(uint64_t kexec_flags)
+{
+ xc_interface *xch;
+ uint8_t type;
+ int ret = -1;
+
+#ifdef HAVE_KEXEC_CMD_STATUS
+ xch = xc_interface_open(NULL, NULL, 0);
+ if (!xch)
+ return -1;
+
+ type = (kexec_flags & KEXEC_ON_CRASH) ? KEXEC_TYPE_CRASH :
KEXEC_TYPE_DEFAULT;
+
+ ret = xc_kexec_status(xch, type);
+
+ xc_interface_close(xch);
+#endif
+
+ return ret;
+}
+
void xen_kexec_exec(void)
{
xc_interface *xch;
@@ -130,6 +151,11 @@ int xen_kexec_unload(uint64_t kexec_flags)
return -1;
}
+int xen_kexec_status(uint64_t kexec_flags)
+{
+ return -1;
+}
+
void xen_kexec_exec(void)
{
}
diff --git a/kexec/kexec.8 b/kexec/kexec.8
index 4d0c1d1..f4b39a6 100644
--- a/kexec/kexec.8
+++ b/kexec/kexec.8
@@ -107,6 +107,12 @@ command:
.B \-d\ (\-\-debug)
Enable debugging messages.
.TP
+.B \-S\ (\-\-status)
+Return 0 if the type (by default crash) is loaded. Can be used in conjuction
+with -l or -p to toggle the type. Note this option supersedes other options
+and it will
+.BR not\ load\ or\ unload\ the\ kernel.
+.TP
.B \-e\ (\-\-exec)
Run the currently loaded kernel. Note that it will reboot into the loaded
kernel without calling shutdown(8).
.TP
diff --git a/kexec/kexec.c b/kexec/kexec.c
index 500e5a9..defbbe3 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -51,6 +51,9 @@
#include "kexec-lzma.h"
#include
+#define KEXEC_LOADED_PATH "/sys/kernel/kexec_loaded"
+#define KEXEC_CRASH_LOADED_PATH "/sys/kernel/kexec_crash_loaded"
+
unsigned long long mem_min = 0;
unsigned long long mem_max = ULONG_MAX;
static unsigned long kexec_flags = 0;
@@ -58,6 +61,8 @@ static unsigned long kexec_flags = 0;
static unsigned long kexec_file_flags = 0;
int kexec_debug = 0;
+static int kexec_loaded(const char *file);
+
void dbgprint_mem_range(const char *prefix, struct memory_range *mr, int nr_mr)
{
int i;
@@ -890,8 +895,6 @@ static int my_exec(void)
return -1;
}
-static int kexec_loaded(void);
-
static int load_jump_back_helper_image(unsigned long kexec_flags, void *entry)
{
int result;
@@ -909,7 +912,7 @@ static int my_load_jump_back_helper(unsigned long
kexec_flags, void *entry)
{
int result;
- if (kexec_loaded()) {
+ if (kexec_loaded(KEXEC_LOADED_PATH)) {
fprintf(stderr, "There is kexec kernel loaded, make sure "
"you are in kexeced kernel.\n");
return -1;
@@ -970,6 +973,7 @@ vo
Re: [Xen-devel] [PATCH v3] kexec: implemented XEN KEXEC STATUS to determine if an image is loaded
On 01/24/2017 01:16 PM, Daniel Kiper wrote:
On Tue, Jan 24, 2017 at 12:55:35PM -0600, Eric DeVolder wrote:
Instead of the scripts having to poke at various fields we can
provide that functionality via the -S parameter.
kexec_loaded/kexec_crash_loaded exposes Linux kernel kexec/crash
state. It does not say anything about Xen kexec/crash state. So,
we need a special approach to get the latter. Though for
compatibility we provide similar functionality in kexec-tools
for the former.
This change enables the --status or -S option to work either
with or without Xen.
Returns 0 if the payload is loaded. Can be used in combination
with -l or -p to get the state of the proper kexec image.
Signed-off-by: Konrad Rzeszutek Wilk
Signed-off-by: Eric DeVolder
---
Note: The corresponding Xen changes have been committed
to the Xen staging branch. Follow this thread:
https://lists.xenproject.org/archives/html/xen-devel/2017-01/msg01570.html
CC: Andrew Cooper
CC: ke...@lists.infradead.org
CC: xen-de...@lists.xenproject.org
CC: Daniel Kiper
v0: First version (internal product).
v1: Posted on kexec mailing list. Changed -s to -S
v2: Incorporated feedback from kexec mailing list, posted on kexec mailing list
v3: Incorporated feedback from kexec mailing list
---
configure.ac | 8 ++-
kexec/kexec-xen.c | 26 +++
kexec/kexec.8 | 6 ++
kexec/kexec.c | 62 ---
kexec/kexec.h | 5 -
5 files changed, 98 insertions(+), 9 deletions(-)
diff --git a/configure.ac b/configure.ac
index 3044185..c6e864b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -165,8 +165,14 @@ fi
dnl find Xen control stack libraries
if test "$with_xen" = yes ; then
AC_CHECK_HEADER(xenctrl.h,
- [AC_CHECK_LIB(xenctrl, xc_kexec_load, ,
+ [AC_CHECK_LIB(xenctrl, xc_kexec_load, [ have_xenctrl_h=yes ],
AC_MSG_NOTICE([Xen support disabled]))])
+if test "$have_xenctrl_h" = yes ; then
+ AC_CHECK_LIB(xenctrl, xc_kexec_status,
+ AC_DEFINE(HAVE_KEXEC_CMD_STATUS, 1,
+ [The kexec_status call is available]),
+ AC_MSG_NOTICE([The kexec_status call is not available]))
+fi
I have a feeling that you have missed my comment. Please add two TABs
starting from "+if test "$have_xenctrl_h" = yes ; then" up to "+fi".
So, it should look more or less like this:
AC_MSG_NOTICE([Xen support disabled]))])
+ if test "$have_xenctrl_h" = yes ; then
+ AC_CHECK_LIB(xenctrl, xc_kexec_status,
...
If it is not needed or something like that please drop me a line.
The tabs are not needed for the configure to work properly.
If tabs are needed for readability/style purposes, I will
add them in. There is not any precedent of nesting in
the configure.ac file, so I am unsure what convention is
for this package.
fi
dnl ---Sanity checks
diff --git a/kexec/kexec-xen.c b/kexec/kexec-xen.c
index 24a4191..2b448d3 100644
--- a/kexec/kexec-xen.c
+++ b/kexec/kexec-xen.c
@@ -105,6 +105,27 @@ int xen_kexec_unload(uint64_t kexec_flags)
return ret;
}
+int xen_kexec_status(uint64_t kexec_flags)
+{
+ xc_interface *xch;
+ uint8_t type;
+ int ret = -1;
+
+#ifdef HAVE_KEXEC_CMD_STATUS
+ xch = xc_interface_open(NULL, NULL, 0);
+ if (!xch)
+ return -1;
+
+ type = (kexec_flags & KEXEC_ON_CRASH) ? KEXEC_TYPE_CRASH :
KEXEC_TYPE_DEFAULT;
+
+ ret = xc_kexec_status(xch, type);
+
+ xc_interface_close(xch);
+#endif
+
+ return ret;
+}
+
void xen_kexec_exec(void)
{
xc_interface *xch;
@@ -130,6 +151,11 @@ int xen_kexec_unload(uint64_t kexec_flags)
return -1;
}
+int xen_kexec_status(uint64_t kexec_flags)
+{
+ return -1;
+}
+
void xen_kexec_exec(void)
{
}
diff --git a/kexec/kexec.8 b/kexec/kexec.8
index 4d0c1d1..f4b39a6 100644
--- a/kexec/kexec.8
+++ b/kexec/kexec.8
@@ -107,6 +107,12 @@ command:
.B \-d\ (\-\-debug)
Enable debugging messages.
.TP
+.B \-S\ (\-\-status)
+Return 0 if the type (by default crash) is loaded. Can be used in conjuction
+with -l or -p to toggle the type. Note this option supersedes other options
+and it will
+.BR not\ load\ or\ unload\ the\ kernel.
Same as above. I think that you have missed my earlier comments.
I suppose that you can join "+and it will" and "+.BR not\ load\ or\
unload\ the\ kernel." into one line.
In that file, all dot directives start with the dot in the
first column. I did the same for the .BR in this statement.
+.TP
.B \-e\ (\-\-exec)
Run the currently loaded kernel. Note that it will reboot into the loaded
kernel without calling shutdown(8).
.TP
diff --git a/kexec/kexec.c b/kexec/kexec.c
index 500e5a9..defbbe3 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -51,6 +51,9 @@
#include "
Re: [Xen-devel] [PATCH v3] kexec: implemented XEN KEXEC STATUS to determine if an image is loaded
On 01/24/2017 01:16 PM, Daniel Kiper wrote:
On Tue, Jan 24, 2017 at 12:55:35PM -0600, Eric DeVolder wrote:
Instead of the scripts having to poke at various fields we can
provide that functionality via the -S parameter.
kexec_loaded/kexec_crash_loaded exposes Linux kernel kexec/crash
state. It does not say anything about Xen kexec/crash state. So,
we need a special approach to get the latter. Though for
compatibility we provide similar functionality in kexec-tools
for the former.
This change enables the --status or -S option to work either
with or without Xen.
Returns 0 if the payload is loaded. Can be used in combination
with -l or -p to get the state of the proper kexec image.
Signed-off-by: Konrad Rzeszutek Wilk
Signed-off-by: Eric DeVolder
---
Note: The corresponding Xen changes have been committed
to the Xen staging branch. Follow this thread:
https://lists.xenproject.org/archives/html/xen-devel/2017-01/msg01570.html
CC: Andrew Cooper
CC: ke...@lists.infradead.org
CC: xen-de...@lists.xenproject.org
CC: Daniel Kiper
v0: First version (internal product).
v1: Posted on kexec mailing list. Changed -s to -S
v2: Incorporated feedback from kexec mailing list, posted on kexec mailing list
v3: Incorporated feedback from kexec mailing list
---
configure.ac | 8 ++-
kexec/kexec-xen.c | 26 +++
kexec/kexec.8 | 6 ++
kexec/kexec.c | 62 ---
kexec/kexec.h | 5 -
5 files changed, 98 insertions(+), 9 deletions(-)
diff --git a/configure.ac b/configure.ac
index 3044185..c6e864b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -165,8 +165,14 @@ fi
dnl find Xen control stack libraries
if test "$with_xen" = yes ; then
AC_CHECK_HEADER(xenctrl.h,
- [AC_CHECK_LIB(xenctrl, xc_kexec_load, ,
+ [AC_CHECK_LIB(xenctrl, xc_kexec_load, [ have_xenctrl_h=yes ],
AC_MSG_NOTICE([Xen support disabled]))])
+if test "$have_xenctrl_h" = yes ; then
+ AC_CHECK_LIB(xenctrl, xc_kexec_status,
+ AC_DEFINE(HAVE_KEXEC_CMD_STATUS, 1,
+ [The kexec_status call is available]),
+ AC_MSG_NOTICE([The kexec_status call is not available]))
+fi
I have a feeling that you have missed my comment. Please add two TABs
starting from "+if test "$have_xenctrl_h" = yes ; then" up to "+fi".
So, it should look more or less like this:
AC_MSG_NOTICE([Xen support disabled]))])
+ if test "$have_xenctrl_h" = yes ; then
+ AC_CHECK_LIB(xenctrl, xc_kexec_status,
...
If it is not needed or something like that please drop me a line.
fi
dnl ---Sanity checks
diff --git a/kexec/kexec-xen.c b/kexec/kexec-xen.c
index 24a4191..2b448d3 100644
--- a/kexec/kexec-xen.c
+++ b/kexec/kexec-xen.c
@@ -105,6 +105,27 @@ int xen_kexec_unload(uint64_t kexec_flags)
return ret;
}
+int xen_kexec_status(uint64_t kexec_flags)
+{
+ xc_interface *xch;
+ uint8_t type;
+ int ret = -1;
+
+#ifdef HAVE_KEXEC_CMD_STATUS
+ xch = xc_interface_open(NULL, NULL, 0);
+ if (!xch)
+ return -1;
+
+ type = (kexec_flags & KEXEC_ON_CRASH) ? KEXEC_TYPE_CRASH :
KEXEC_TYPE_DEFAULT;
+
+ ret = xc_kexec_status(xch, type);
+
+ xc_interface_close(xch);
+#endif
+
+ return ret;
+}
+
void xen_kexec_exec(void)
{
xc_interface *xch;
@@ -130,6 +151,11 @@ int xen_kexec_unload(uint64_t kexec_flags)
return -1;
}
+int xen_kexec_status(uint64_t kexec_flags)
+{
+ return -1;
+}
+
void xen_kexec_exec(void)
{
}
diff --git a/kexec/kexec.8 b/kexec/kexec.8
index 4d0c1d1..f4b39a6 100644
--- a/kexec/kexec.8
+++ b/kexec/kexec.8
@@ -107,6 +107,12 @@ command:
.B \-d\ (\-\-debug)
Enable debugging messages.
.TP
+.B \-S\ (\-\-status)
+Return 0 if the type (by default crash) is loaded. Can be used in conjuction
+with -l or -p to toggle the type. Note this option supersedes other options
+and it will
+.BR not\ load\ or\ unload\ the\ kernel.
Same as above. I think that you have missed my earlier comments.
I suppose that you can join "+and it will" and "+.BR not\ load\ or\
unload\ the\ kernel." into one line.
+.TP
.B \-e\ (\-\-exec)
Run the currently loaded kernel. Note that it will reboot into the loaded
kernel without calling shutdown(8).
.TP
diff --git a/kexec/kexec.c b/kexec/kexec.c
index 500e5a9..defbbe3 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -51,6 +51,9 @@
#include "kexec-lzma.h"
#include
+#define KEXEC_LOADED_PATH "/sys/kernel/kexec_loaded"
+#define KEXEC_CRASH_LOADED_PATH "/sys/kernel/kexec_crash_loaded"
+
unsigned long long mem_min = 0;
unsigned long long mem_max = ULONG_MAX;
static unsigned long kexec_flags = 0;
@@ -58,6 +61,8 @@ static unsigned long kexec_flags = 0;
static unsigned long kexec_file_flags
[Xen-devel] [PATCH v4] kexec: implemented XEN KEXEC STATUS to determine if an image is loaded
Instead of the scripts having to poke at various fields we can
provide that functionality via the -S parameter.
kexec_loaded/kexec_crash_loaded exposes Linux kernel kexec/crash
state. It does not say anything about Xen kexec/crash state. So,
we need a special approach to get the latter. Though for
compatibility we provide similar functionality in kexec-tools
for the former.
This change enables the --status or -S option to work either
with or without Xen.
Returns 0 if the payload is loaded. Can be used in combination
with -l or -p to get the state of the proper kexec image.
Signed-off-by: Konrad Rzeszutek Wilk
Signed-off-by: Eric DeVolder
---
CC: ke...@lists.infradead.org
CC: xen-de...@lists.xenproject.org
CC: Daniel Kiper
v0: First version (internal product).
v1: Posted on kexec mailing list. Changed -s to -S
v2: Incorporated feedback from kexec mailing list, posted on kexec mailing list
v3: Incorporated feedback from kexec mailing list, posted on kexec mailing list
v4: Incorporated feedback from kexec mailing list
---
configure.ac | 8 +++-
kexec/kexec-xen.c | 26 +
kexec/kexec.8 | 6 +++
kexec/kexec.c | 114 ++
kexec/kexec.h | 5 ++-
5 files changed, 123 insertions(+), 36 deletions(-)
diff --git a/configure.ac b/configure.ac
index 3044185..53fffc3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -165,8 +165,14 @@ fi
dnl find Xen control stack libraries
if test "$with_xen" = yes ; then
AC_CHECK_HEADER(xenctrl.h,
- [AC_CHECK_LIB(xenctrl, xc_kexec_load, ,
+ [AC_CHECK_LIB(xenctrl, xc_kexec_load, [ have_xenctrl_h=yes ],
AC_MSG_NOTICE([Xen support disabled]))])
+ if test "$have_xenctrl_h" = yes ; then
+ AC_CHECK_LIB(xenctrl, xc_kexec_status,
+ AC_DEFINE(HAVE_KEXEC_CMD_STATUS, 1,
+ [The kexec_status call is available]),
+ AC_MSG_NOTICE([The kexec_status call is not
available]))
+ fi
fi
dnl ---Sanity checks
diff --git a/kexec/kexec-xen.c b/kexec/kexec-xen.c
index 24a4191..2b448d3 100644
--- a/kexec/kexec-xen.c
+++ b/kexec/kexec-xen.c
@@ -105,6 +105,27 @@ int xen_kexec_unload(uint64_t kexec_flags)
return ret;
}
+int xen_kexec_status(uint64_t kexec_flags)
+{
+ xc_interface *xch;
+ uint8_t type;
+ int ret = -1;
+
+#ifdef HAVE_KEXEC_CMD_STATUS
+ xch = xc_interface_open(NULL, NULL, 0);
+ if (!xch)
+ return -1;
+
+ type = (kexec_flags & KEXEC_ON_CRASH) ? KEXEC_TYPE_CRASH :
KEXEC_TYPE_DEFAULT;
+
+ ret = xc_kexec_status(xch, type);
+
+ xc_interface_close(xch);
+#endif
+
+ return ret;
+}
+
void xen_kexec_exec(void)
{
xc_interface *xch;
@@ -130,6 +151,11 @@ int xen_kexec_unload(uint64_t kexec_flags)
return -1;
}
+int xen_kexec_status(uint64_t kexec_flags)
+{
+ return -1;
+}
+
void xen_kexec_exec(void)
{
}
diff --git a/kexec/kexec.8 b/kexec/kexec.8
index 4d0c1d1..f4b39a6 100644
--- a/kexec/kexec.8
+++ b/kexec/kexec.8
@@ -107,6 +107,12 @@ command:
.B \-d\ (\-\-debug)
Enable debugging messages.
.TP
+.B \-S\ (\-\-status)
+Return 0 if the type (by default crash) is loaded. Can be used in conjuction
+with -l or -p to toggle the type. Note this option supersedes other options
+and it will
+.BR not\ load\ or\ unload\ the\ kernel.
+.TP
.B \-e\ (\-\-exec)
Run the currently loaded kernel. Note that it will reboot into the loaded
kernel without calling shutdown(8).
.TP
diff --git a/kexec/kexec.c b/kexec/kexec.c
index 500e5a9..ec16247 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -51,6 +51,9 @@
#include "kexec-lzma.h"
#include
+#define KEXEC_LOADED_PATH "/sys/kernel/kexec_loaded"
+#define KEXEC_CRASH_LOADED_PATH "/sys/kernel/kexec_crash_loaded"
+
unsigned long long mem_min = 0;
unsigned long long mem_max = ULONG_MAX;
static unsigned long kexec_flags = 0;
@@ -890,8 +893,6 @@ static int my_exec(void)
return -1;
}
-static int kexec_loaded(void);
-
static int load_jump_back_helper_image(unsigned long kexec_flags, void *entry)
{
int result;
@@ -902,6 +903,40 @@ static int load_jump_back_helper_image(unsigned long
kexec_flags, void *entry)
return result;
}
+static int kexec_loaded(const char *file)
+{
+ long ret = -1;
+ FILE *fp;
+ char *p;
+ char line[3];
+
+ /* No way to tell if an image is loaded under Xen, assume it is. */
+ if (xen_present())
+ return 1;
+
+ fp = fopen(file, "r");
+ if (fp == NULL)
+ return -1;
+
+ p = fgets(line, sizeof(line), fp);
+ fclose(fp);
+
+ if (p == NULL)
+ return -1;
+
+ ret = strtol(line, &p, 10);
+
+ /* Too long */
+ if (ret > INT_MAX)
+
[Xen-devel] [XEN KEXEC PATCH] Corrected comment typo "count not" to "could not"
Fix cut-n-paste typo; changed the words "count not" to "could not".
No functional changes
Signed-off-by: Eric DeVolder
---
tools/libxc/xc_kexec.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/libxc/xc_kexec.c b/tools/libxc/xc_kexec.c
index 1cceb5d2d6..989e225192 100644
--- a/tools/libxc/xc_kexec.c
+++ b/tools/libxc/xc_kexec.c
@@ -20,7 +20,7 @@ int xc_kexec_exec(xc_interface *xch, int type)
exec = xc_hypercall_buffer_alloc(xch, exec, sizeof(*exec));
if ( exec == NULL )
{
-PERROR("Count not alloc bounce buffer for kexec_exec hypercall");
+PERROR("Could not alloc bounce buffer for kexec_exec hypercall");
goto out;
}
@@ -111,7 +111,7 @@ int xc_kexec_unload(xc_interface *xch, int type)
unload = xc_hypercall_buffer_alloc(xch, unload, sizeof(*unload));
if ( unload == NULL )
{
-PERROR("Count not alloc buffer for kexec unload hypercall");
+PERROR("Could not alloc buffer for kexec unload hypercall");
goto out;
}
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH] kexec: Add spinlock for the whole hypercall
When we concurrently try to unload and load crash
images we eventually get:
Xen call trace:
[] machine_kexec_add_page+0x3a0/0x3fa
[] machine_kexec_load+0xdb/0x107
[] kexec.c#kexec_load_slot+0x11/0x42
[] kexec.c#kexec_load+0x119/0x150
[] kexec.c#do_kexec_op_internal+0xab/0xcf
[] do_kexec_op+0xe/0x1e
[] pv_hypercall+0x20a/0x44a
[] cpufreq.c#test_all_events+0/0x30
Pagetable walk from 820040088320:
L4[0x104] = 0002979d1063
L3[0x001] = 0002979d0063
L2[0x000] = 0002979c7063
L1[0x088] = 80037a91ede97063
The interesting thing is that the page bits (063) look legit.
The operation on which we blow up is us trying to write
in the L1 and finding that the L2 entry points to some
bizzare MFN. It stinks of a race, and it looks like
the issue is due to no concurrency locks when dealing
with the crash kernel space.
Specifically we concurrently call kimage_alloc_crash_control_page
which iterates over the kexec_crash_area.start -> kexec_crash_area.size
and once found:
if ( page )
{
image->next_crash_page = hole_end;
clear_domain_page(_mfn(page_to_mfn(page)));
}
clears. Since the parameters of what MFN to use are provided
by the callers (and the area to search is bounded) the the 'page'
is probably the same. So #1 we concurrently clear the
'control_code_page'.
The next step is us passing this 'control_code_page' to
machine_kexec_add_page. This function requires the MFNs:
page_to_maddr(image->control_code_page).
And this would always return the same virtual address, as
the MFN of the control_code_page is inside of the
kexec_crash_area.start -> kexec_crash_area.size area.
Then machine_kexec_add_page updates the L1 .. which can be done
concurrently and on subsequent calls we mangle it up.
This is all a theory at this time, but testing reveals
that adding the spinlock at the kexec hypercall fixes
the crash.
An alternative solution would be to only add the spinlock
on paths that touch the CRASH region and not everything.
NOTE: The spinlock in kexec_swap_images() was removed as
this function is only reachable on the kexec call, which is
now entirely protected by a spinlock, thus the local spinlock
is no longer necessary.
NOTE: This patch follows 5c5216 (kexec: clear kexec_image slot
when unloading kexec image) to prevent crashes during
simultaneous load/unloads.
Signed-off-by: Konrad Rzeszutek Wilk
Signed-off-by: Eric DeVolder
Reviewed-by: Daniel Kiper
---
xen/common/kexec.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index 072cc8e..5bcc356 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -819,7 +819,6 @@ static int kexec_exec(XEN_GUEST_HANDLE_PARAM(void) uarg)
static int kexec_swap_images(int type, struct kexec_image *new,
struct kexec_image **old)
{
-static DEFINE_SPINLOCK(kexec_lock);
int base, bit, pos;
int new_slot, old_slot;
@@ -831,8 +830,6 @@ static int kexec_swap_images(int type, struct kexec_image
*new,
if ( kexec_load_get_bits(type, &base, &bit) )
return -EINVAL;
-spin_lock(&kexec_lock);
-
pos = (test_bit(bit, &kexec_flags) != 0);
old_slot = base + pos;
new_slot = base + !pos;
@@ -845,8 +842,6 @@ static int kexec_swap_images(int type, struct kexec_image
*new,
clear_bit(old_slot, &kexec_flags);
*old = kexec_image[old_slot];
-spin_unlock(&kexec_lock);
-
return 0;
}
@@ -1187,12 +1182,22 @@ static int do_kexec_op_internal(unsigned long op,
XEN_GUEST_HANDLE_PARAM(void) uarg,
bool_t compat)
{
+static DEFINE_SPINLOCK(kexec_lock);
int ret = -EINVAL;
ret = xsm_kexec(XSM_PRIV);
if ( ret )
return ret;
+/*
+ * Because we write directly to the reserved memory
+ * region when loading crash kernels we need a spinlock here to
+ * prevent multiple crash kernels from attempting to load
+ * simultaneously, and to prevent a crash kernel from loading
+ * over the top of a in use crash kernel.
+ */
+spin_lock(&kexec_lock);
+
switch ( op )
{
case KEXEC_CMD_kexec_get_range:
@@ -1227,6 +1232,8 @@ static int do_kexec_op_internal(unsigned long op,
break;
}
+spin_unlock(&kexec_lock);
+
return ret;
}
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 1/2] kexec: use hypercall_create_continuation to protect KEXEC ops
When we concurrently try to unload and load crash
images we eventually get:
Xen call trace:
[] machine_kexec_add_page+0x3a0/0x3fa
[] machine_kexec_load+0xdb/0x107
[] kexec.c#kexec_load_slot+0x11/0x42
[] kexec.c#kexec_load+0x119/0x150
[] kexec.c#do_kexec_op_internal+0xab/0xcf
[] do_kexec_op+0xe/0x1e
[] pv_hypercall+0x20a/0x44a
[] cpufreq.c#test_all_events+0/0x30
Pagetable walk from 820040088320:
L4[0x104] = 0002979d1063
L3[0x001] = 0002979d0063
L2[0x000] = 0002979c7063
L1[0x088] = 80037a91ede97063
The interesting thing is that the page bits (063) look legit.
The operation on which we blow up is us trying to write
in the L1 and finding that the L2 entry points to some
bizzare MFN. It stinks of a race, and it looks like
the issue is due to no concurrency locks when dealing
with the crash kernel space.
Specifically we concurrently call kimage_alloc_crash_control_page
which iterates over the kexec_crash_area.start -> kexec_crash_area.size
and once found:
if ( page )
{
image->next_crash_page = hole_end;
clear_domain_page(_mfn(page_to_mfn(page)));
}
clears. Since the parameters of what MFN to use are provided
by the callers (and the area to search is bounded) the the 'page'
is probably the same. So #1 we concurrently clear the
'control_code_page'.
The next step is us passing this 'control_code_page' to
machine_kexec_add_page. This function requires the MFNs:
page_to_maddr(image->control_code_page).
And this would always return the same virtual address, as
the MFN of the control_code_page is inside of the
kexec_crash_area.start -> kexec_crash_area.size area.
Then machine_kexec_add_page updates the L1 .. which can be done
concurrently and on subsequent calls we mangle it up.
This is all a theory at this time, but testing reveals
that adding the hypercall_create_continuation() at the
kexec hypercall fixes the crash.
NOTE: This patch follows 5c5216 (kexec: clear kexec_image slot
when unloading kexec image) to prevent crashes during
simultaneous load/unloads.
NOTE: Consideration was given to using the existing flag
KEXEC_FLAG_IN_PROGRESS to denote a kexec hypercall in
progress. This, however, overloads the original intent of
the flag which is to denote that we are about-to/have made
the jump to the crash path. The overloading would lead to
failures in existing checks on this flag as the flag would
always be set at the top level in do_kexec_op_internal().
For this reason, the new flag KEXEC_FLAG_HC_IN_PROGRESS
was introduced.
While at it, fixed the #define mismatched spacing
Signed-off-by: Eric DeVolder
Reviewed-by: Bhavesh Davda
Reviewed-by: Konrad Rzeszutek Wilk
---
v2: 04/17/2017
- Patch titled 'kexec: use hypercall_create_continuation to protect KEXEC ops'
- Jan Beulich directed me to use a continuation method instead
of spinlock.
- Incorporated feedback from Daniel Kiper
- Incorporated feedback from Konrad Wilk
v1: 04/10/2017
- Patch titled 'kexec: Add spinlock for the whole hypercall'
- Used spinlock in do_kexec_op_internal()
---
xen/common/kexec.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index 072cc8e..3f96eb2 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -50,9 +50,10 @@ static cpumask_t crash_saved_cpus;
static struct kexec_image *kexec_image[KEXEC_IMAGE_NR];
-#define KEXEC_FLAG_DEFAULT_POS (KEXEC_IMAGE_NR + 0)
-#define KEXEC_FLAG_CRASH_POS (KEXEC_IMAGE_NR + 1)
-#define KEXEC_FLAG_IN_PROGRESS (KEXEC_IMAGE_NR + 2)
+#define KEXEC_FLAG_DEFAULT_POS(KEXEC_IMAGE_NR + 0)
+#define KEXEC_FLAG_CRASH_POS (KEXEC_IMAGE_NR + 1)
+#define KEXEC_FLAG_IN_PROGRESS(KEXEC_IMAGE_NR + 2)
+#define KEXEC_FLAG_HC_IN_PROGRESS (KEXEC_IMAGE_NR + 3)
static unsigned long kexec_flags = 0; /* the lowest bits are for
KEXEC_IMAGE... */
@@ -1193,6 +1194,9 @@ static int do_kexec_op_internal(unsigned long op,
if ( ret )
return ret;
+if ( test_and_set_bit(KEXEC_FLAG_HC_IN_PROGRESS, &kexec_flags))
+return hypercall_create_continuation(__HYPERVISOR_kexec_op, "lh", op,
uarg);
+
switch ( op )
{
case KEXEC_CMD_kexec_get_range:
@@ -1227,6 +1231,8 @@ static int do_kexec_op_internal(unsigned long op,
break;
}
+clear_bit(KEXEC_FLAG_HC_IN_PROGRESS, &kexec_flags);
+
return ret;
}
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 0/2] kexec: Use hypercall_create_continuation to protect KEXEC ops
During testing (using the script below) we found that multiple
invocations of kexec of unload/load are not safe.
This does not exist in classic Xen kernels in which the kexec-tools
did the kexec via Linux kernel syscall (which in turn made the
hypercall), as the Linux code has a mutex_trylock which would
inhibit multiple concurrent calls.
But with the kexec-tools utilizing xc_kexec_* that is no longer
the case and we need to protect against multiple concurrent
invocations.
Please see the patches and review at your convenience!
try-crash.pl from bhavesh.da...@oracle.com
#!/usr/bin/perl -w
use strict;
use warnings;
use threads;
sub threaded_task {
threads->create(sub {
my $thr_id = threads->self->tid;
#print "Starting load thread $thr_id\n";
system("/sbin/kexec -p --command-line=\"placeholder
root=/dev/mapper/nimbula-root ro rhbg console=tty0 console=hvc0 earlyprintk=xen
nomodeset printk.time=1 irqpoll maxcpus=1 nr_cpus=1 reset_devices
cgroup_disable=memory mce=off selinux=0 console=ttyS1,115200n8\"
--initrd=/boot/initrd-4.1.12-61.1.9.el6uek.x86_64kdump.img
/boot/vmlinuz-4.1.12-61.1.9.el6uek.x86_64");
#print "Ending load thread $thr_id\n";
threads->detach(); #End thread.
});
threads->create(sub {
my $thr_id = threads->self->tid;
#print "Starting unload thread $thr_id\n";
system("/sbin/kexec -p -u");
#print "Ending unload thread $thr_id\n";
threads->detach(); #End thread.
});
}
for my $i (0..99)
{
threaded_task();
}
Eric DeVolder (2):
kexec: use hypercall_create_continuation to protect KEXEC ops
kexec: remove spinlock now that all KEXEC hypercall ops are protected
at the top-level
xen/common/kexec.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 2/2] kexec: remove spinlock now that all KEXEC hypercall ops are protected at the top-level
The spinlock in kexec_swap_images() was removed as
this function is only reachable on the kexec hypercall, which is
now protected at the top-level in do_kexec_op_internal(),
thus the local spinlock is no longer necessary.
Signed-off-by: Eric DeVolder
Reviewed-by: Bhavesh Davda
Reviewed-by: Konrad Rzeszutek Wilk
---
xen/common/kexec.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index 3f96eb2..efecf60 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -820,7 +820,6 @@ static int kexec_exec(XEN_GUEST_HANDLE_PARAM(void) uarg)
static int kexec_swap_images(int type, struct kexec_image *new,
struct kexec_image **old)
{
-static DEFINE_SPINLOCK(kexec_lock);
int base, bit, pos;
int new_slot, old_slot;
@@ -832,8 +831,6 @@ static int kexec_swap_images(int type, struct kexec_image
*new,
if ( kexec_load_get_bits(type, &base, &bit) )
return -EINVAL;
-spin_lock(&kexec_lock);
-
pos = (test_bit(bit, &kexec_flags) != 0);
old_slot = base + pos;
new_slot = base + !pos;
@@ -846,8 +843,6 @@ static int kexec_swap_images(int type, struct kexec_image
*new,
clear_bit(old_slot, &kexec_flags);
*old = kexec_image[old_slot];
-spin_unlock(&kexec_lock);
-
return 0;
}
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 2/2] kexec: remove spinlock now that all KEXEC hypercall ops are protected at the top-level
The spinlock in kexec_swap_images() was removed as
this function is only reachable on the kexec hypercall, which is
now protected at the top-level in do_kexec_op_internal(),
thus the local spinlock is no longer necessary.
Per recommendation from Jan Beulich and Andrew Cooper, I left
an ASSERT in place of the spin_lock().
Signed-off-by: Eric DeVolder
Reviewed-by: Bhavesh Davda
Reviewed-by: Konrad Rzeszutek Wilk
---
v3:
- Incorporated feedback from Jan Beulich and Andrew Cooper
to leave an ASSERT where spin_lock() once was.
v2: 04/17/2017
- Patch titled 'kexec: use hypercall_create_continuation to protect KEXEC ops'
- Separated removal of spinlock in kexec_swap_images() into its own patch.
v1: 04/10/2017
- Patch titled 'kexec: Add spinlock for the whole hypercall'
- Removal of spinlock in kexec_swap_images() was part of other patch.
---
xen/common/kexec.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index 253c204..af32e58 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -820,7 +820,6 @@ static int kexec_exec(XEN_GUEST_HANDLE_PARAM(void) uarg)
static int kexec_swap_images(int type, struct kexec_image *new,
struct kexec_image **old)
{
-static DEFINE_SPINLOCK(kexec_lock);
int base, bit, pos;
int new_slot, old_slot;
@@ -832,7 +831,7 @@ static int kexec_swap_images(int type, struct kexec_image
*new,
if ( kexec_load_get_bits(type, &base, &bit) )
return -EINVAL;
-spin_lock(&kexec_lock);
+ASSERT( !test_bit(KEXEC_FLAG_IN_HYPERCALL, &kexec_flags) );
pos = (test_bit(bit, &kexec_flags) != 0);
old_slot = base + pos;
@@ -846,8 +845,6 @@ static int kexec_swap_images(int type, struct kexec_image
*new,
clear_bit(old_slot, &kexec_flags);
*old = kexec_image[old_slot];
-spin_unlock(&kexec_lock);
-
return 0;
}
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 1/2] kexec: use hypercall_create_continuation to protect KEXEC ops
When we concurrently try to unload and load crash
images we eventually get:
Xen call trace:
[] machine_kexec_add_page+0x3a0/0x3fa
[] machine_kexec_load+0xdb/0x107
[] kexec.c#kexec_load_slot+0x11/0x42
[] kexec.c#kexec_load+0x119/0x150
[] kexec.c#do_kexec_op_internal+0xab/0xcf
[] do_kexec_op+0xe/0x1e
[] pv_hypercall+0x20a/0x44a
[] cpufreq.c#test_all_events+0/0x30
Pagetable walk from 820040088320:
L4[0x104] = 0002979d1063
L3[0x001] = 0002979d0063
L2[0x000] = 0002979c7063
L1[0x088] = 80037a91ede97063
The interesting thing is that the page bits (063) look legit.
The operation on which we blow up is us trying to write
in the L1 and finding that the L2 entry points to some
bizzare MFN. It stinks of a race, and it looks like
the issue is due to no concurrency locks when dealing
with the crash kernel space.
Specifically we concurrently call kimage_alloc_crash_control_page
which iterates over the kexec_crash_area.start -> kexec_crash_area.size
and once found:
if ( page )
{
image->next_crash_page = hole_end;
clear_domain_page(_mfn(page_to_mfn(page)));
}
clears. Since the parameters of what MFN to use are provided
by the callers (and the area to search is bounded) the the 'page'
is probably the same. So #1 we concurrently clear the
'control_code_page'.
The next step is us passing this 'control_code_page' to
machine_kexec_add_page. This function requires the MFNs:
page_to_maddr(image->control_code_page).
And this would always return the same virtual address, as
the MFN of the control_code_page is inside of the
kexec_crash_area.start -> kexec_crash_area.size area.
Then machine_kexec_add_page updates the L1 .. which can be done
concurrently and on subsequent calls we mangle it up.
This is all a theory at this time, but testing reveals
that adding the hypercall_create_continuation() at the
kexec hypercall fixes the crash.
NOTE: This patch follows 5c5216 (kexec: clear kexec_image slot
when unloading kexec image) to prevent crashes during
simultaneous load/unloads.
NOTE: Consideration was given to using the existing flag
KEXEC_FLAG_IN_PROGRESS to denote a kexec hypercall in
progress. This, however, overloads the original intent of
the flag which is to denote that we are about-to/have made
the jump to the crash path. The overloading would lead to
failures in existing checks on this flag as the flag would
always be set at the top level in do_kexec_op_internal().
For this reason, the new flag KEXEC_FLAG_HC_IN_PROGRESS
was introduced.
While at it, fixed the #define mismatched spacing
Signed-off-by: Eric DeVolder
Reviewed-by: Bhavesh Davda
Reviewed-by: Konrad Rzeszutek Wilk
Reviewed-by: Jan Beulich
Reviewed-by: Andrew Cooper
Reviewed-by: Daniel Kiper
---
v3:
- Incorporated feedback from Jan Beulich to change the
name of the new flag to KEXEC_FLAG_IN_HYPERCALL
v2: 04/17/2017
- Patch titled 'kexec: use hypercall_create_continuation to protect KEXEC ops'
- Jan Beulich directed me to use a continuation method instead
of spinlock.
- Incorporated feedback from Daniel Kiper
- Incorporated feedback from Konrad Wilk
v1: 04/10/2017
- Patch titled 'kexec: Add spinlock for the whole hypercall'
- Used spinlock in do_kexec_op_internal()
---
xen/common/kexec.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index 072cc8e..253c204 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -50,9 +50,10 @@ static cpumask_t crash_saved_cpus;
static struct kexec_image *kexec_image[KEXEC_IMAGE_NR];
-#define KEXEC_FLAG_DEFAULT_POS (KEXEC_IMAGE_NR + 0)
-#define KEXEC_FLAG_CRASH_POS (KEXEC_IMAGE_NR + 1)
-#define KEXEC_FLAG_IN_PROGRESS (KEXEC_IMAGE_NR + 2)
+#define KEXEC_FLAG_DEFAULT_POS (KEXEC_IMAGE_NR + 0)
+#define KEXEC_FLAG_CRASH_POS(KEXEC_IMAGE_NR + 1)
+#define KEXEC_FLAG_IN_PROGRESS (KEXEC_IMAGE_NR + 2)
+#define KEXEC_FLAG_IN_HYPERCALL (KEXEC_IMAGE_NR + 3)
static unsigned long kexec_flags = 0; /* the lowest bits are for
KEXEC_IMAGE... */
@@ -1193,6 +1194,9 @@ static int do_kexec_op_internal(unsigned long op,
if ( ret )
return ret;
+if ( test_and_set_bit(KEXEC_FLAG_IN_HYPERCALL, &kexec_flags) )
+return hypercall_create_continuation(__HYPERVISOR_kexec_op, "lh", op,
uarg);
+
switch ( op )
{
case KEXEC_CMD_kexec_get_range:
@@ -1227,6 +1231,8 @@ static int do_kexec_op_internal(unsigned long op,
break;
}
+clear_bit(KEXEC_FLAG_IN_HYPERCALL, &kexec_flags);
+
return ret;
}
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 0/2] kexec: Use hypercall_create_continuation to protect KEXEC ops
During testing (using the script below) we found that multiple
invocations of kexec of unload/load are not safe.
This does not exist in classic Xen kernels in which the kexec-tools
did the kexec via Linux kernel syscall (which in turn made the
hypercall), as the Linux code has a mutex_trylock which would
inhibit multiple concurrent calls.
But with the kexec-tools utilizing xc_kexec_* that is no longer
the case and we need to protect against multiple concurrent
invocations.
Please see the patches and review at your convenience!
try-crash.pl from bhavesh.da...@oracle.com
#!/usr/bin/perl -w
use strict;
use warnings;
use threads;
sub threaded_task {
threads->create(sub {
my $thr_id = threads->self->tid;
#print "Starting load thread $thr_id\n";
system("/sbin/kexec -p --command-line=\"placeholder
root=/dev/mapper/nimbula-root ro rhbg console=tty0 console=hvc0 earlyprintk=xen
nomodeset printk.time=1 irqpoll maxcpus=1 nr_cpus=1 reset_devices
cgroup_disable=memory mce=off selinux=0 console=ttyS1,115200n8\"
--initrd=/boot/initrd-4.1.12-61.1.9.el6uek.x86_64kdump.img
/boot/vmlinuz-4.1.12-61.1.9.el6uek.x86_64");
#print "Ending load thread $thr_id\n";
threads->detach(); #End thread.
});
threads->create(sub {
my $thr_id = threads->self->tid;
#print "Starting unload thread $thr_id\n";
system("/sbin/kexec -p -u");
#print "Ending unload thread $thr_id\n";
threads->detach(); #End thread.
});
}
for my $i (0..99)
{
threaded_task();
}
Eric DeVolder (2):
kexec: use hypercall_create_continuation to protect KEXEC ops
kexec: remove spinlock now that all KEXEC hypercall ops are protected
at the top-level
xen/common/kexec.c | 17 ++---
1 file changed, 10 insertions(+), 7 deletions(-)
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 2/2] kexec: remove spinlock now that all KEXEC hypercall ops are protected at the top-level
On 04/19/2017 08:37 AM, Jan Beulich wrote: On 19.04.17 at 14:13, wrote: On Wed, Apr 19, 2017 at 05:20:50AM -0600, Jan Beulich wrote: On 19.04.17 at 12:56, wrote: On Tue, Apr 18, 2017 at 04:49:48AM -0600, Jan Beulich wrote: On 17.04.17 at 21:09, wrote: The spinlock in kexec_swap_images() was removed as this function is only reachable on the kexec hypercall, which is now protected at the top-level in do_kexec_op_internal(), thus the local spinlock is no longer necessary. But perhaps leave an ASSERT() there, making sure the in-hypercall flag is set? I am not sure why but if at all I think that we should also consider other key kexec functions then. Or put ASSERT() into do_kexec_op_internal() just before "switch ( op )". The point of my placement suggestion was that the ASSERT() effectively replaces the lock acquire - the places you name didn't previously require any synchronization. After the first patch of this series kexec_swap_images() cannot be started twice in parallel. So, I do not see the point of ASSERT() here. Or let's say we wish to have it to double check that "the in-hypercall flag is set". AIUI, it is your original idea. However, then I think that we should have an ASSERT() at least in kexec_load_slot() because parallel loads make issues too. And we can go higher to feel more safe. That is why I suggested do_kexec_op_internal() as the final resting place for an ASSERT(). Simply it looks to me the safest approach. Am I missing something? The point you're missing is - why don't you then move the ASSERT() yet one more level up, right next to where the flag is being set? IOW what you suggest would imo rather mean not adding any assertion at all. I've just posted v3 of this patch with the ASSERT in lieu of the spin_lock(). Eric ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 1/2] kexec: use hypercall_create_continuation to protect KEXEC ops
On 04/19/2017 06:48 AM, Andrew Cooper wrote: On 19/04/17 12:00, Daniel Kiper wrote: On Tue, Apr 18, 2017 at 04:48:06AM -0600, Jan Beulich wrote: On 17.04.17 at 21:09, wrote: --- a/xen/common/kexec.c +++ b/xen/common/kexec.c @@ -50,9 +50,10 @@ static cpumask_t crash_saved_cpus; static struct kexec_image *kexec_image[KEXEC_IMAGE_NR]; -#define KEXEC_FLAG_DEFAULT_POS (KEXEC_IMAGE_NR + 0) -#define KEXEC_FLAG_CRASH_POS (KEXEC_IMAGE_NR + 1) -#define KEXEC_FLAG_IN_PROGRESS (KEXEC_IMAGE_NR + 2) +#define KEXEC_FLAG_DEFAULT_POS(KEXEC_IMAGE_NR + 0) +#define KEXEC_FLAG_CRASH_POS (KEXEC_IMAGE_NR + 1) +#define KEXEC_FLAG_IN_PROGRESS(KEXEC_IMAGE_NR + 2) +#define KEXEC_FLAG_HC_IN_PROGRESS (KEXEC_IMAGE_NR + 3) Perhaps KEXEC_FLAG_IN_HYPERCALL? Other than that (and this Make sense for me. Agreed. (I can fix on commit). clearly is subject to Andrew's opinion) Reviewed-by: Jan Beulich Otherwise Reviewed-by: Daniel Kiper Reviewed-by: Andrew Cooper CC'ing Julien. This is clearly a good bugfix for 4.9 I've just posted v3 of this patch with the flag name change. Eric ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 2/2] kexec: remove spinlock now that all KEXEC hypercall ops are protected at the top-level
On 04/19/2017 10:47 AM, Eric DeVolder wrote:
The spinlock in kexec_swap_images() was removed as
this function is only reachable on the kexec hypercall, which is
now protected at the top-level in do_kexec_op_internal(),
thus the local spinlock is no longer necessary.
Per recommendation from Jan Beulich and Andrew Cooper, I left
an ASSERT in place of the spin_lock().
Signed-off-by: Eric DeVolder
Reviewed-by: Bhavesh Davda
Reviewed-by: Konrad Rzeszutek Wilk
---
v3:
- Incorporated feedback from Jan Beulich and Andrew Cooper
to leave an ASSERT where spin_lock() once was.
v2: 04/17/2017
- Patch titled 'kexec: use hypercall_create_continuation to protect KEXEC ops'
- Separated removal of spinlock in kexec_swap_images() into its own patch.
v1: 04/10/2017
- Patch titled 'kexec: Add spinlock for the whole hypercall'
- Removal of spinlock in kexec_swap_images() was part of other patch.
---
xen/common/kexec.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index 253c204..af32e58 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -820,7 +820,6 @@ static int kexec_exec(XEN_GUEST_HANDLE_PARAM(void) uarg)
static int kexec_swap_images(int type, struct kexec_image *new,
struct kexec_image **old)
{
-static DEFINE_SPINLOCK(kexec_lock);
int base, bit, pos;
int new_slot, old_slot;
@@ -832,7 +831,7 @@ static int kexec_swap_images(int type, struct kexec_image
*new,
if ( kexec_load_get_bits(type, &base, &bit) )
return -EINVAL;
-spin_lock(&kexec_lock);
+ASSERT( !test_bit(KEXEC_FLAG_IN_HYPERCALL, &kexec_flags) );
There are two problems here: the spaces need to be eliminated, and more
importantly, the sense/polarity of the expression is wrong, the '!'
needs to be removed.
pos = (test_bit(bit, &kexec_flags) != 0);
old_slot = base + pos;
@@ -846,8 +845,6 @@ static int kexec_swap_images(int type, struct kexec_image
*new,
clear_bit(old_slot, &kexec_flags);
*old = kexec_image[old_slot];
-spin_unlock(&kexec_lock);
-
return 0;
}
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 2/2] kexec: remove spinlock now that all KEXEC hypercall ops are protected at the top-level
On 04/19/2017 11:21 AM, Jan Beulich wrote: On 19.04.17 at 17:47, wrote: @@ -832,7 +831,7 @@ static int kexec_swap_images(int type, struct kexec_image *new, if ( kexec_load_get_bits(type, &base, &bit) ) return -EINVAL; -spin_lock(&kexec_lock); +ASSERT( !test_bit(KEXEC_FLAG_IN_HYPERCALL, &kexec_flags) ); Did you test this (in a debug build)? I ask because it looks to me that you've inverted the condition. You are correct, I inverted the condition. I had found this shortly after posting, and have corrected and have tested with Config.mk:debug=y I will repost patch. Thanks, eric ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4 2/2] kexec: remove spinlock now that all KEXEC hypercall ops are protected at the top-level
The spinlock in kexec_swap_images() was removed as
this function is only reachable on the kexec hypercall, which is
now protected at the top-level in do_kexec_op_internal(),
thus the local spinlock is no longer necessary.
Per recommendation from Jan Beulich and Andrew Cooper, I left
an ASSERT in place of the spin_lock().
Signed-off-by: Eric DeVolder
Reviewed-by: Bhavesh Davda
Reviewed-by: Konrad Rzeszutek Wilk
---
v4: 04/19/2017
- Fix ASSERT to work properly. Tested with Config.mk:debug=y
v3: 04/19/2017
- Incorporated feedback from Jan Beulich and Andrew Cooper
to leave an ASSERT where spin_lock() once was.
v2: 04/17/2017
- Patch titled 'kexec: use hypercall_create_continuation to protect KEXEC ops'
- Separated removal of spinlock in kexec_swap_images() into its own patch.
v1: 04/10/2017
- Patch titled 'kexec: Add spinlock for the whole hypercall'
- Removal of spinlock in kexec_swap_images() was part of other patch.
---
xen/common/kexec.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index 253c204..96efa3b 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -820,7 +820,6 @@ static int kexec_exec(XEN_GUEST_HANDLE_PARAM(void) uarg)
static int kexec_swap_images(int type, struct kexec_image *new,
struct kexec_image **old)
{
-static DEFINE_SPINLOCK(kexec_lock);
int base, bit, pos;
int new_slot, old_slot;
@@ -832,7 +831,7 @@ static int kexec_swap_images(int type, struct kexec_image
*new,
if ( kexec_load_get_bits(type, &base, &bit) )
return -EINVAL;
-spin_lock(&kexec_lock);
+ASSERT(test_bit(KEXEC_FLAG_IN_HYPERCALL, &kexec_flags));
pos = (test_bit(bit, &kexec_flags) != 0);
old_slot = base + pos;
@@ -846,8 +845,6 @@ static int kexec_swap_images(int type, struct kexec_image
*new,
clear_bit(old_slot, &kexec_flags);
*old = kexec_image[old_slot];
-spin_unlock(&kexec_lock);
-
return 0;
}
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4 0/2] kexec: Use hypercall_create_continuation to protect KEXEC ops
During testing (using the script below) we found that multiple
invocations of kexec of unload/load are not safe.
This does not exist in classic Xen kernels in which the kexec-tools
did the kexec via Linux kernel syscall (which in turn made the
hypercall), as the Linux code has a mutex_trylock which would
inhibit multiple concurrent calls.
But with the kexec-tools utilizing xc_kexec_* that is no longer
the case and we need to protect against multiple concurrent
invocations.
Please see the patches and review at your convenience!
try-crash.pl from bhavesh.da...@oracle.com
#!/usr/bin/perl -w
use strict;
use warnings;
use threads;
sub threaded_task {
threads->create(sub {
my $thr_id = threads->self->tid;
#print "Starting load thread $thr_id\n";
system("/sbin/kexec -p --command-line=\"placeholder
root=/dev/mapper/nimbula-root ro rhbg console=tty0 console=hvc0 earlyprintk=xen
nomodeset printk.time=1 irqpoll maxcpus=1 nr_cpus=1 reset_devices
cgroup_disable=memory mce=off selinux=0 console=ttyS1,115200n8\"
--initrd=/boot/initrd-4.1.12-61.1.9.el6uek.x86_64kdump.img
/boot/vmlinuz-4.1.12-61.1.9.el6uek.x86_64");
#print "Ending load thread $thr_id\n";
threads->detach(); #End thread.
});
threads->create(sub {
my $thr_id = threads->self->tid;
#print "Starting unload thread $thr_id\n";
system("/sbin/kexec -p -u");
#print "Ending unload thread $thr_id\n";
threads->detach(); #End thread.
});
}
for my $i (0..99)
{
threaded_task();
}
Eric DeVolder (2):
kexec: use hypercall_create_continuation to protect KEXEC ops
kexec: remove spinlock now that all KEXEC hypercall ops are protected
at the top-level
xen/common/kexec.c | 17 ++---
1 file changed, 10 insertions(+), 7 deletions(-)
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4 1/2] kexec: use hypercall_create_continuation to protect KEXEC ops
When we concurrently try to unload and load crash
images we eventually get:
Xen call trace:
[] machine_kexec_add_page+0x3a0/0x3fa
[] machine_kexec_load+0xdb/0x107
[] kexec.c#kexec_load_slot+0x11/0x42
[] kexec.c#kexec_load+0x119/0x150
[] kexec.c#do_kexec_op_internal+0xab/0xcf
[] do_kexec_op+0xe/0x1e
[] pv_hypercall+0x20a/0x44a
[] cpufreq.c#test_all_events+0/0x30
Pagetable walk from 820040088320:
L4[0x104] = 0002979d1063
L3[0x001] = 0002979d0063
L2[0x000] = 0002979c7063
L1[0x088] = 80037a91ede97063
The interesting thing is that the page bits (063) look legit.
The operation on which we blow up is us trying to write
in the L1 and finding that the L2 entry points to some
bizzare MFN. It stinks of a race, and it looks like
the issue is due to no concurrency locks when dealing
with the crash kernel space.
Specifically we concurrently call kimage_alloc_crash_control_page
which iterates over the kexec_crash_area.start -> kexec_crash_area.size
and once found:
if ( page )
{
image->next_crash_page = hole_end;
clear_domain_page(_mfn(page_to_mfn(page)));
}
clears. Since the parameters of what MFN to use are provided
by the callers (and the area to search is bounded) the the 'page'
is probably the same. So #1 we concurrently clear the
'control_code_page'.
The next step is us passing this 'control_code_page' to
machine_kexec_add_page. This function requires the MFNs:
page_to_maddr(image->control_code_page).
And this would always return the same virtual address, as
the MFN of the control_code_page is inside of the
kexec_crash_area.start -> kexec_crash_area.size area.
Then machine_kexec_add_page updates the L1 .. which can be done
concurrently and on subsequent calls we mangle it up.
This is all a theory at this time, but testing reveals
that adding the hypercall_create_continuation() at the
kexec hypercall fixes the crash.
NOTE: This patch follows 5c5216 (kexec: clear kexec_image slot
when unloading kexec image) to prevent crashes during
simultaneous load/unloads.
NOTE: Consideration was given to using the existing flag
KEXEC_FLAG_IN_PROGRESS to denote a kexec hypercall in
progress. This, however, overloads the original intent of
the flag which is to denote that we are about-to/have made
the jump to the crash path. The overloading would lead to
failures in existing checks on this flag as the flag would
always be set at the top level in do_kexec_op_internal().
For this reason, the new flag KEXEC_FLAG_HC_IN_PROGRESS
was introduced.
While at it, fixed the #define mismatched spacing
Signed-off-by: Eric DeVolder
Reviewed-by: Bhavesh Davda
Reviewed-by: Konrad Rzeszutek Wilk
Reviewed-by: Jan Beulich
Reviewed-by: Andrew Cooper
Reviewed-by: Daniel Kiper
---
v4: 04/19/2017
- Unchanged
v3: 04/19/2017
- Incorporated feedback from Jan Beulich to change the
name of the new flag to KEXEC_FLAG_IN_HYPERCALL
v2: 04/17/2017
- Patch titled 'kexec: use hypercall_create_continuation to protect KEXEC ops'
- Jan Beulich directed me to use a continuation method instead
of spinlock.
- Incorporated feedback from Daniel Kiper
- Incorporated feedback from Konrad Wilk
v1: 04/10/2017
- Patch titled 'kexec: Add spinlock for the whole hypercall'
- Used spinlock in do_kexec_op_internal()
---
xen/common/kexec.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index 072cc8e..253c204 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -50,9 +50,10 @@ static cpumask_t crash_saved_cpus;
static struct kexec_image *kexec_image[KEXEC_IMAGE_NR];
-#define KEXEC_FLAG_DEFAULT_POS (KEXEC_IMAGE_NR + 0)
-#define KEXEC_FLAG_CRASH_POS (KEXEC_IMAGE_NR + 1)
-#define KEXEC_FLAG_IN_PROGRESS (KEXEC_IMAGE_NR + 2)
+#define KEXEC_FLAG_DEFAULT_POS (KEXEC_IMAGE_NR + 0)
+#define KEXEC_FLAG_CRASH_POS(KEXEC_IMAGE_NR + 1)
+#define KEXEC_FLAG_IN_PROGRESS (KEXEC_IMAGE_NR + 2)
+#define KEXEC_FLAG_IN_HYPERCALL (KEXEC_IMAGE_NR + 3)
static unsigned long kexec_flags = 0; /* the lowest bits are for
KEXEC_IMAGE... */
@@ -1193,6 +1194,9 @@ static int do_kexec_op_internal(unsigned long op,
if ( ret )
return ret;
+if ( test_and_set_bit(KEXEC_FLAG_IN_HYPERCALL, &kexec_flags) )
+return hypercall_create_continuation(__HYPERVISOR_kexec_op, "lh", op,
uarg);
+
switch ( op )
{
case KEXEC_CMD_kexec_get_range:
@@ -1227,6 +1231,8 @@ static int do_kexec_op_internal(unsigned long op,
break;
}
+clear_bit(KEXEC_FLAG_IN_HYPERCALL, &kexec_flags);
+
return ret;
}
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
