Introduce a dummy XEN_SYSCTL_LIVEPATCH_TEST hypercall to be used in order to
test livepatch functionality.  The hypercall fills a value in the passed
structure, which is returned to the caller.

The xen-livepatch utility is expanded to allow calling that hypercall, and
printing the returned value on stdout.

Finally, add dummy patch that changes the returned value of the hypercall from
1 to 2.  Such patch can be used with livepatch-build-tools in order to generate
a livepatch payload, that when applied to the hypervisor change the printed
value of `xen-livepatch test`.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
The whole logic is very simple now.  I think it's enough to have a skeleton we
can later expand.

Unsure whether we should do some kind of test (with `patch -F0`) that the patch
still applies cleanly as part of Xen build.
---
 tools/include/xenctrl.h                |  3 +++
 tools/libs/ctrl/xc_misc.c              | 14 ++++++++++++++
 tools/misc/xen-livepatch.c             | 25 +++++++++++++++++++++++++
 xen/common/Makefile                    |  2 +-
 xen/common/livepatch-test.c            | 20 ++++++++++++++++++++
 xen/common/livepatch.c                 |  4 ++++
 xen/include/public/sysctl.h            |  7 +++++++
 xen/include/xen/livepatch.h            |  4 ++++
 xen/test/livepatch/patches/test1.patch | 13 +++++++++++++
 9 files changed, 91 insertions(+), 1 deletion(-)
 create mode 100644 xen/common/livepatch-test.c
 create mode 100644 xen/test/livepatch/patches/test1.patch

diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index 2ef8b4e05422..83a00d4974dd 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -2645,6 +2645,9 @@ int xc_livepatch_revert(xc_interface *xch, char *name, uint32_t timeout, uint32_
 int xc_livepatch_unload(xc_interface *xch, char *name, uint32_t timeout, uint32_t flags);
 int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout, uint32_t flags);
 
+/* Dummy hypercall to test livepatch functionality. */
+int xc_livepatch_test(xc_interface *xch, uint32_t *result);
+
 /*
  * Ensure cache coherency after memory modifications. A call to this function
  * is only required on ARM as the x86 architecture provides cache coherency
diff --git a/tools/libs/ctrl/xc_misc.c b/tools/libs/ctrl/xc_misc.c
index 5ecdfa2c7934..0ca86a53d097 100644
--- a/tools/libs/ctrl/xc_misc.c
+++ b/tools/libs/ctrl/xc_misc.c
@@ -1021,6 +1021,20 @@ int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout, uint32
     return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_REPLACE, timeout, flags);
 }
 
+int xc_livepatch_test(xc_interface *xch, uint32_t *result)
+{
+    struct xen_sysctl sysctl = {
+        .cmd = XEN_SYSCTL_livepatch_op,
+        .u.livepatch.cmd = XEN_SYSCTL_LIVEPATCH_TEST,
+    };
+    int rc = do_sysctl(xch, &sysctl);
+
+    if ( !rc )
+        *result = sysctl.u.livepatch.u.test.result;
+
+    return rc;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
index 5bf9d9a32b65..5f6fd20d8814 100644
--- a/tools/misc/xen-livepatch.c
+++ b/tools/misc/xen-livepatch.c
@@ -37,6 +37,7 @@ void show_help(void)
             "  replace <name>         apply <name> patch and revert all others.\n"
             "  unload <name>          unload name <name> patch.\n"
             "  load <file> [flags]    upload and apply <file> with name as the <file> name\n"
+            "  test                   print the result of the test hypercall (for testing purposes only)\n"
             "    Supported flags:\n"
             "      --nodeps           Disable inter-module buildid dependency check.\n"
             "                         Check only against hypervisor buildid.\n",
@@ -542,6 +543,29 @@ error:
     return rc;
 }
 
+static int test_func(int argc, char *argv[])
+{
+    int rc;
+    uint32_t result = 0;
+
+    if ( argc != 0 )
+    {
+        show_help();
+        return -1;
+    }
+
+    rc = xc_livepatch_test(xch, &result);
+    if ( rc )
+    {
+        fprintf(stderr, "test operation failed: %s\n", strerror(errno));
+        return -1;
+    }
+
+    printf("%u\n", result);
+
+    return 0;
+}
+
 /*
  * These are also functions in action_options that are called in case
  * none of the ones in main_options match.
@@ -554,6 +578,7 @@ struct {
     { "list", list_func },
     { "upload", upload_func },
     { "load", load_func },
+    { "test", test_func },
 };
 
 int main(int argc, char *argv[])
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 69d6aa626c7f..ab073d41f1d2 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -23,7 +23,7 @@ obj-y += kernel.o
 obj-y += keyhandler.o
 obj-$(CONFIG_KEXEC) += kexec.o
 obj-$(CONFIG_KEXEC) += kimage.o
-obj-$(CONFIG_LIVEPATCH) += livepatch.o livepatch_elf.o
+obj-$(CONFIG_LIVEPATCH) += livepatch.o livepatch_elf.o livepatch-test.o
 obj-$(CONFIG_MEM_ACCESS) += mem_access.o
 obj-y += memory.o
 obj-y += multicall.o
diff --git a/xen/common/livepatch-test.c b/xen/common/livepatch-test.c
new file mode 100644
index 000000000000..05b638b2ac67
--- /dev/null
+++ b/xen/common/livepatch-test.c
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/* Dummy file for testing livepatch functionality. */
+#include <xen/livepatch.h>
+
+int livepatch_test(struct xen_sysctl_livepatch_test *test)
+{
+    test->result = 1;
+    return 0;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 1209fea2566c..e8894db1cc93 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -2116,6 +2116,10 @@ int livepatch_op(struct xen_sysctl_livepatch_op *livepatch)
         rc = livepatch_action(&livepatch->u.action);
         break;
 
+    case XEN_SYSCTL_LIVEPATCH_TEST:
+        rc = livepatch_test(&livepatch->u.test);
+        break;
+
     default:
         rc = -EOPNOTSUPP;
         break;
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 9b19679caeb1..9c13a7fdb22c 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -1137,6 +1137,12 @@ struct xen_sysctl_livepatch_action {
     uint32_t pad;                           /* IN: Always zero. */
 };
 
+/* Dummy hypercall for testing live patches. */
+#define XEN_SYSCTL_LIVEPATCH_TEST 4
+struct xen_sysctl_livepatch_test {
+    uint32_t result;                        /* OUT: dummy result for testing. */
+};
+
 struct xen_sysctl_livepatch_op {
     uint32_t cmd;                           /* IN: XEN_SYSCTL_LIVEPATCH_*. */
     uint32_t pad;                           /* IN: Always zero. */
@@ -1145,6 +1151,7 @@ struct xen_sysctl_livepatch_op {
         struct xen_sysctl_livepatch_list list;
         struct xen_sysctl_livepatch_get get;
         struct xen_sysctl_livepatch_action action;
+        struct xen_sysctl_livepatch_test test;
     } u;
 };
 
diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
index df339a134e40..60d11d037dfb 100644
--- a/xen/include/xen/livepatch.h
+++ b/xen/include/xen/livepatch.h
@@ -11,6 +11,8 @@ struct livepatch_elf_sec;
 struct livepatch_elf_sym;
 struct xen_sysctl_livepatch_op;
 
+#include <xen/types.h> /* For elfstructs.h */
+
 #include <xen/elfstructs.h>
 #include <xen/errno.h> /* For -ENOSYS or -EOVERFLOW */
 
@@ -165,6 +167,8 @@ static inline void common_livepatch_revert(const struct livepatch_func *func,
     arch_livepatch_revert(func, state);
     state->applied = LIVEPATCH_FUNC_NOT_APPLIED;
 }
+
+int livepatch_test(struct xen_sysctl_livepatch_test *test);
 #else
 
 /*
diff --git a/xen/test/livepatch/patches/test1.patch b/xen/test/livepatch/patches/test1.patch
new file mode 100644
index 000000000000..c07d697cc8de
--- /dev/null
+++ b/xen/test/livepatch/patches/test1.patch
@@ -0,0 +1,13 @@
+diff --git a/xen/common/livepatch-test.c b/xen/common/livepatch-test.c
+index 05b638b2ac..876173ab6f 100644
+--- a/xen/common/livepatch-test.c
++++ b/xen/common/livepatch-test.c
+@@ -5,7 +5,7 @@
+ 
+ int livepatch_test(struct xen_sysctl_livepatch_test *test)
+ {
+-    test->result = 1;
++    test->result = 2;
+     return 0;
+ }
+ 
-- 
2.43.0

On 23/11/2023 11:23 am, Roger Pau Monne wrote:
> Introduce a dummy XEN_SYSCTL_LIVEPATCH_TEST hypercall to be used in order to
> test livepatch functionality.  The hypercall fills a value in the passed
> structure, which is returned to the caller.
>
> The xen-livepatch utility is expanded to allow calling that hypercall, and
> printing the returned value on stdout.
>
> Finally, add dummy patch that changes the returned value of the hypercall from
> 1 to 2.  Such patch can be used with livepatch-build-tools in order to generate
> a livepatch payload, that when applied to the hypervisor change the printed
> value of `xen-livepatch test`.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> The whole logic is very simple now.  I think it's enough to have a skeleton we
> can later expand.
>
> Unsure whether we should do some kind of test (with `patch -F0`) that the patch
> still applies cleanly as part of Xen build.

Thanks for looking into this.  I think one tweak towards the larger plan
might help here.

When I envisaged this originally, it was something along the lines of
test_self_modify_code() which would be called on boot after alternatives
were called, which would sanity check the result of certain simple cases.

Then, for livepatching, I was thinking of something like this:

obj-y += test_smc.o
targets-y += test_smc_alt.o

i.e. we have test_smc.c and test_smc_alt.c which are two slightly
different copies of the same thing, and we compile both but don't link
the second one in.

Then, we can diff the two C files in order to make the patch to build as
a livepatch.  This way we're not maintaining a patch committed into the
tree, which I suspect will make everyone's lives easier.  I suspect in
practice that we'll want test_smc_asm.S pairs too.

Not necessarily for now, but I was also thinking we'd have a test stage
where we know exactly what the livepatch ought to look like, so we audit
the eventual livepatch elf that it has all the expected differences
encoded, and no extraneous ones.

Finally, I was thinking that the hypercall would (re)run
test_self_modify_code().  I'm on the fence about making it part of the
livepatch infrastructure, given that the nature of the test is wider,
but I can't think of any case that we'd be wanting runtime self
modifying code (e.g. rerun alternatives after ucode load) which isn't
linked to a new livepatch to begin with.

Thoughts?

~Andrew