kexec: implement STATUS hypercall to check if image is loaded

PATCH Superseded Eric DeVolder 3 participants 9 messages Jan 12, 2017 – Jan 13, 2017

Share & tools

Eric DeVolder

author

2017-01-12 21:59 UTC #0 lore raw

PATCH v2 1/1 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 <[email protected]>

Signed-off-by Eric DeVolder <[email protected]>

Reviewed-by Daniel Kiper <[email protected]>

---
CC: Elena Ufimtseva <[email protected]>
CC: Daniel Kiper <[email protected]>

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(-)

tools/libxc/include/xenctrl.h +10 -0

index 4ab0f57117..63c616ff6a 100644
@@ -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;
 
 /*

tools/libxc/xc_kexec.c +24 -0

index 989e225192..a0a9fd3841 100644
@@ -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;
+}

xen/common/kexec.c +19 -0

index c83d48fc79..aa808cb2f2 100644
@@ -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;

xen/include/public/kexec.h +13 -0

index a6a0a88f4f..c200e8ceee 100644
@@ -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

xen/include/public/xen-compat.h +1 -1

index dd8a5c0d0e..b67365340b 100644
@@ -27,7 +27,7 @@
 #ifndef __XEN_PUBLIC_XEN_COMPAT_H__
 #define __XEN_PUBLIC_XEN_COMPAT_H__
 
-#define __XEN_LATEST_INTERFACE_VERSION__ 0x00040800
+#define __XEN_LATEST_INTERFACE_VERSION__ 0x00040900
 
 #if defined(__XEN__) || defined(__XEN_TOOLS__)
 /* Xen is built with matching headers and implements the latest interface. */

Signature · 7 lines

2.11.0


_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Jan Beulich

2017-01-13 11:16 UTC #1 lore raw

Quoted 3 lines

Quoted 1 line
Quoted 1 line · On 12.01.17 at 22:59, <[email protected]> wrote:
CC Elena Ufimtseva <[email protected]>
CC Daniel Kiper <[email protected]>

Cc: Andrew Cooper <[email protected]>
(as the kexec maintainer)

Quoted 2 lines

Note: The __XEN_LATEST_INTERFACE_VERSION__ has been bumped to 0x00040900 due to the introduction of a new hypervisor call.

This bumping is required only if something changes in the interface such that e.g. source code would need adjustment to not fail to build.

Jan

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Daniel Kiper

2017-01-13 11:40 UTC #2 lore raw

Quoted 7 lines · Jan Beulich

Quoted 3 lines
Quoted 1 line
Quoted 1 line · On 12.01.17 at 22:59, <[email protected]> wrote:
CC Elena Ufimtseva <[email protected]>
CC Daniel Kiper <[email protected]>
Cc: Andrew Cooper <[email protected]>
(as the kexec maintainer)

Quoted 6 lines

Quoted 2 lines
Note: The __XEN_LATEST_INTERFACE_VERSION__ has been bumped to 0x00040900 due to the introduction of a new hypervisor call.
This bumping is required only if something changes in the interface such that e.g. source code would need adjustment to not fail to build.

We need a check in kexec-tools to verify that STATUS hypercall is available. Otherwise we can disable relevant functionality. I thought that we can do that because __XEN_LATEST_INTERFACE_VERSION__ is exposed via public headers.

Daniel

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Jan Beulich

2017-01-13 11:56 UTC #3 lore raw

Quoted 12 lines

Quoted 1 line
Quoted 1 line · On 13.01.17 at 12:40, <[email protected]> wrote:
Quoted 10 lines · Jan Beulich
Quoted 3 lines
Quoted 1 line
Quoted 1 line · On 12.01.17 at 22:59, <[email protected]> wrote:
CC Elena Ufimtseva <[email protected]>
CC Daniel Kiper <[email protected]>
Cc: Andrew Cooper <[email protected]>
(as the kexec maintainer)
Quoted 2 lines
Note: The __XEN_LATEST_INTERFACE_VERSION__ has been bumped to 0x00040900 due to the introduction of a new hypervisor call.

Quoted 8 lines

Quoted 3 lines
This bumping is required only if something changes in the interface such that e.g. source code would need adjustment to not fail to build.
We need a check in kexec-tools to verify that STATUS hypercall is available. Otherwise we can disable relevant functionality. I thought that we can do that because __XEN_LATEST_INTERFACE_VERSION__ is exposed via public headers.

No, you should check for KEXEC_CMD_kexec_status being defined instead. That'll also cover someone backporting the change to their packages.

Jan

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Daniel Kiper

2017-01-13 12:14 UTC #4 lore raw

Quoted 16 lines · Jan Beulich

Quoted 16 lines
Quoted 1 line
Quoted 1 line · On 13.01.17 at 12:40, <[email protected]> wrote:
Quoted 13 lines · Jan Beulich
Quoted 3 lines
Quoted 1 line
Quoted 1 line · On 12.01.17 at 22:59, <[email protected]> wrote:
CC Elena Ufimtseva <[email protected]>
CC Daniel Kiper <[email protected]>
Cc: Andrew Cooper <[email protected]>
(as the kexec maintainer)
Quoted 2 lines
Note: The __XEN_LATEST_INTERFACE_VERSION__ has been bumped to 0x00040900 due to the introduction of a new hypervisor call.
This bumping is required only if something changes in the interface such that e.g. source code would need adjustment to not fail to build.

Quoted 8 lines

Quoted 4 lines
We need a check in kexec-tools to verify that STATUS hypercall is available. Otherwise we can disable relevant functionality. I thought that we can do that because __XEN_LATEST_INTERFACE_VERSION__ is exposed via public headers.
No, you should check for KEXEC_CMD_kexec_status being defined instead. That'll also cover someone backporting the change to their packages.

OK, this also works for us. Eric, please drop __XEN_LATEST_INTERFACE_VERSION__ change and add KEXEC_CMD_kexec_status check to kexec-tools configure script. Sorry for confusion.

Jan, so, __XEN_LATEST_INTERFACE_VERSION__ is used only for Xen internal purposes (eg. for xl build, etc.) and should not be used as a reference point externally, e.g. in kexec-tools?

Daniel

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Jan Beulich

2017-01-13 12:35 UTC #5 lore raw

Quoted 4 lines

Quoted 1 line
Quoted 1 line · On 13.01.17 at 13:14, <[email protected]> wrote:
Jan, so, __XEN_LATEST_INTERFACE_VERSION__ is used only for Xen internal purposes (eg. for xl build, etc.) and should not be used as a reference point externally, e.g. in kexec-tools?

It's certainly meant for external consumption (see my original reply to Eric), but if at all possible you should not use it for (build time, obviously) feature detection.

Jan

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Daniel Kiper

2017-01-13 12:49 UTC #6 lore raw

Quoted 8 lines · Jan Beulich

Quoted 4 lines
Quoted 1 line
Quoted 1 line · On 13.01.17 at 13:14, <[email protected]> wrote:
Jan, so, __XEN_LATEST_INTERFACE_VERSION__ is used only for Xen internal purposes (eg. for xl build, etc.) and should not be used as a reference point externally, e.g. in kexec-tools?
It's certainly meant for external consumption (see my original reply to Eric), but if at all possible you should not use it for (build time, obviously) feature detection.

So, AIUI, __XEN_LATEST_INTERFACE_VERSION__ should be used when e.g. structure layout passed to a given hypercall changes and it is not possible to detect such change in other way. Right?

Daniel

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Jan Beulich

2017-01-13 13:19 UTC #7 lore raw

Quoted 7 lines

Quoted 1 line
Quoted 1 line · On 13.01.17 at 13:49, <[email protected]> wrote:
Quoted 5 lines · Jan Beulich
Quoted 4 lines
Quoted 1 line
Quoted 1 line · On 13.01.17 at 13:14, <[email protected]> wrote:
Jan, so, __XEN_LATEST_INTERFACE_VERSION__ is used only for Xen internal purposes (eg. for xl build, etc.) and should not be used as a reference point externally, e.g. in kexec-tools?

Quoted 7 lines

Quoted 3 lines
It's certainly meant for external consumption (see my original reply to Eric), but if at all possible you should not use it for (build time, obviously) feature detection.
So, AIUI, __XEN_LATEST_INTERFACE_VERSION__ should be used when e.g. structure layout passed to a given hypercall changes and it is not possible to detect such change in other way. Right?

No, structure _layout_ must not change (other than in tools only interfaces). But structure field naming occasionally changes. While that retains binary compatibility, source code requires changes in such a case, and that's what the symbol is meant to help with.

Jan

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Daniel Kiper

2017-01-13 13:34 UTC #8 lore raw

Quoted 11 lines · Jan Beulich

Quoted 11 lines
Quoted 1 line
Quoted 1 line · On 13.01.17 at 13:49, <[email protected]> wrote:
Quoted 8 lines · Jan Beulich
Quoted 4 lines
Quoted 1 line
Quoted 1 line · On 13.01.17 at 13:14, <[email protected]> wrote:
Jan, so, __XEN_LATEST_INTERFACE_VERSION__ is used only for Xen internal purposes (eg. for xl build, etc.) and should not be used as a reference point externally, e.g. in kexec-tools?
It's certainly meant for external consumption (see my original reply to Eric), but if at all possible you should not use it for (build time, obviously) feature detection.

Quoted 8 lines

Quoted 3 lines
So, AIUI, __XEN_LATEST_INTERFACE_VERSION__ should be used when e.g. structure layout passed to a given hypercall changes and it is not possible to detect such change in other way. Right?
No, structure _layout_ must not change (other than in tools only interfaces). But structure field naming occasionally changes. While that retains binary compatibility, source code requires changes in such a case, and that's what the symbol is meant to help with.

Thanks, make sense.

Daniel

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel