* [Xen-devel] [PATCH v2 01/17] tools/libxl: Remove libxl_cpuid_{set, apply_policy}() from the API
2020-01-27 14:34 [Xen-devel] [PATCH v2 00/17] Support CPUID/MSR data in migration streams Andrew Cooper
@ 2020-01-27 14:34 ` Andrew Cooper
2020-02-24 16:35 ` Andrew Cooper
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 02/17] tools/libxl: Simplify callback handling in libxl-save-helper Andrew Cooper
` (15 subsequent siblings)
16 siblings, 1 reply; 47+ messages in thread
From: Andrew Cooper @ 2020-01-27 14:34 UTC (permalink / raw)
To: Xen-devel; +Cc: Anthony PERARD, Andrew Cooper, Wei Liu, Ian Jackson
These functions should never have been exposed. They don't have external
users, and can't usefully be used for several reasons.
Move libxl_cpuid_{set,apply_policy}() to being internal functions, and leave
an equivalent of the nop stubs in the API for caller compatibility.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <Ian.Jackson@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Anthony PERARD <anthony.perard@citrix.com>
RFC for obvious reasons. An alternative would be to #if 0 them, which would
result in a compile failure rather than silent stubbing. I'm not sure which
is least bad, but I don't think either are going to cause a problem in
practice.
---
tools/libxl/libxl.h | 26 ++++++++++++++++++++++----
tools/libxl/libxl_cpuid.c | 6 +++---
tools/libxl/libxl_dom.c | 4 ++--
tools/libxl/libxl_internal.h | 4 ++++
tools/libxl/libxl_nocpuid.c | 6 +++---
5 files changed, 34 insertions(+), 12 deletions(-)
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 54abb9db1f..a02548f595 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -665,7 +665,7 @@ typedef struct libxl__ctx libxl_ctx;
#if LIBXL_API_VERSION != 0x040200 && LIBXL_API_VERSION != 0x040300 && \
LIBXL_API_VERSION != 0x040400 && LIBXL_API_VERSION != 0x040500 && \
LIBXL_API_VERSION != 0x040700 && LIBXL_API_VERSION != 0x040800 && \
- LIBXL_API_VERSION != 0x041300
+ LIBXL_API_VERSION != 0x041300 && LIBXL_API_VERSION != 0x041400
#error Unknown LIBXL_API_VERSION
#endif
#endif
@@ -2323,9 +2323,27 @@ libxl_device_pci *libxl_device_pci_assignable_list(libxl_ctx *ctx, int *num);
int libxl_cpuid_parse_config(libxl_cpuid_policy_list *cpuid, const char* str);
int libxl_cpuid_parse_config_xend(libxl_cpuid_policy_list *cpuid,
const char* str);
-void libxl_cpuid_apply_policy(libxl_ctx *ctx, uint32_t domid);
-void libxl_cpuid_set(libxl_ctx *ctx, uint32_t domid,
- libxl_cpuid_policy_list cpuid);
+#if LIBXL_API_VERSION < 0x041400
+/*
+ * Dropped from the API in Xen 4.14. At the time of writing, these functions
+ * don't appear to ever have had external callers.
+ *
+ * These have always been used internally during domain construction, and
+ * can't easily be used externally because of their implicit parameters in
+ * other pieces of global state.
+ *
+ * Furthermore, an API user can't usefully determine whether they get
+ * libxl_cpuid (the real implementation) or libxl_nocpuid (no-op stubs).
+ *
+ * The internal behaviour of these functions also needs to change. Therefore
+ * for simplicitly, provide the no-op stubs. Yes technically this is an API
+ * change in some cases for existing software, but there is 0 of that in
+ * practice.
+ */
+static inline void libxl_cpuid_apply_policy(libxl_ctx *ctx, uint32_t domid) {}
+static inline void libxl_cpuid_set(libxl_ctx *ctx, uint32_t domid,
+ libxl_cpuid_policy_list cpuid) {}
+#endif
/*
* Functions for allowing users of libxl to store private data
diff --git a/tools/libxl/libxl_cpuid.c b/tools/libxl/libxl_cpuid.c
index 5c52cbe0f9..505ec1b048 100644
--- a/tools/libxl/libxl_cpuid.c
+++ b/tools/libxl/libxl_cpuid.c
@@ -410,13 +410,13 @@ int libxl_cpuid_parse_config_xend(libxl_cpuid_policy_list *cpuid,
return 0;
}
-void libxl_cpuid_apply_policy(libxl_ctx *ctx, uint32_t domid)
+void libxl__cpuid_apply_policy(libxl_ctx *ctx, uint32_t domid)
{
xc_cpuid_apply_policy(ctx->xch, domid, NULL, 0);
}
-void libxl_cpuid_set(libxl_ctx *ctx, uint32_t domid,
- libxl_cpuid_policy_list cpuid)
+void libxl__cpuid_set(libxl_ctx *ctx, uint32_t domid,
+ libxl_cpuid_policy_list cpuid)
{
int i;
char *cpuid_res[4];
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 573c63692b..b730365f47 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -454,9 +454,9 @@ int libxl__build_post(libxl__gc *gc, uint32_t domid,
if (rc)
return rc;
- libxl_cpuid_apply_policy(ctx, domid);
+ libxl__cpuid_apply_policy(ctx, domid);
if (info->cpuid != NULL)
- libxl_cpuid_set(ctx, domid, info->cpuid);
+ libxl__cpuid_set(ctx, domid, info->cpuid);
if (info->type == LIBXL_DOMAIN_TYPE_HVM
&& !libxl_ms_vm_genid_is_zero(&info->u.hvm.ms_vm_genid)) {
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 64f6fdada8..50856ca703 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2042,6 +2042,10 @@ struct libxl__cpuid_policy {
char *policy[4];
};
+_hidden void libxl__cpuid_apply_policy(libxl_ctx *ctx, uint32_t domid);
+_hidden void libxl__cpuid_set(libxl_ctx *ctx, uint32_t domid,
+ libxl_cpuid_policy_list cpuid);
+
/* Calls poll() again - useful to check whether a signaled condition
* is still true. Cannot fail. Returns currently-true revents. */
_hidden short libxl__fd_poll_recheck(libxl__egc *egc, int fd, short events);
diff --git a/tools/libxl/libxl_nocpuid.c b/tools/libxl/libxl_nocpuid.c
index ef1161c434..a39babe754 100644
--- a/tools/libxl/libxl_nocpuid.c
+++ b/tools/libxl/libxl_nocpuid.c
@@ -34,12 +34,12 @@ int libxl_cpuid_parse_config_xend(libxl_cpuid_policy_list *cpuid,
return 0;
}
-void libxl_cpuid_apply_policy(libxl_ctx *ctx, uint32_t domid)
+void libxl__cpuid_apply_policy(libxl_ctx *ctx, uint32_t domid)
{
}
-void libxl_cpuid_set(libxl_ctx *ctx, uint32_t domid,
- libxl_cpuid_policy_list cpuid)
+void libxl__cpuid_set(libxl_ctx *ctx, uint32_t domid,
+ libxl_cpuid_policy_list cpuid)
{
}
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Xen-devel] [PATCH v2 01/17] tools/libxl: Remove libxl_cpuid_{set, apply_policy}() from the API
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 01/17] tools/libxl: Remove libxl_cpuid_{set, apply_policy}() from the API Andrew Cooper
@ 2020-02-24 16:35 ` Andrew Cooper
0 siblings, 0 replies; 47+ messages in thread
From: Andrew Cooper @ 2020-02-24 16:35 UTC (permalink / raw)
To: Xen-devel; +Cc: Anthony PERARD, Ian Jackson, Wei Liu
On 27/01/2020 14:34, Andrew Cooper wrote:
> These functions should never have been exposed. They don't have external
> users, and can't usefully be used for several reasons.
>
> Move libxl_cpuid_{set,apply_policy}() to being internal functions, and leave
> an equivalent of the nop stubs in the API for caller compatibility.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
This has already been committed (c/s
dacb80f9757c011161cec6609f39837c9ea8caa8) as part of a smaller series
developed subsequent to posting this series, which made sense to go in
ahead.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Xen-devel] [PATCH v2 02/17] tools/libxl: Simplify callback handling in libxl-save-helper
2020-01-27 14:34 [Xen-devel] [PATCH v2 00/17] Support CPUID/MSR data in migration streams Andrew Cooper
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 01/17] tools/libxl: Remove libxl_cpuid_{set, apply_policy}() from the API Andrew Cooper
@ 2020-01-27 14:34 ` Andrew Cooper
2020-02-24 17:21 ` Ian Jackson
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 03/17] tools/migration: Drop IHDR_VERSION constant from libxc and python Andrew Cooper
` (14 subsequent siblings)
16 siblings, 1 reply; 47+ messages in thread
From: Andrew Cooper @ 2020-01-27 14:34 UTC (permalink / raw)
To: Xen-devel; +Cc: Anthony PERARD, Andrew Cooper, Wei Liu, Ian Jackson
The {save,restore}_callback helpers can have their scope reduced vastly, and
helper_setcallbacks_{save,restore}() doesn't need to use a ternary operator to
write 0 (meaning NULL) into an already zeroed structure.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <Ian.Jackson@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Anthony PERARD <anthony.perard@citrix.com>
---
tools/libxl/libxl_save_helper.c | 16 ++++++----------
tools/libxl/libxl_save_msgs_gen.pl | 2 +-
2 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/tools/libxl/libxl_save_helper.c b/tools/libxl/libxl_save_helper.c
index 017c7cd988..65dff389bf 100644
--- a/tools/libxl/libxl_save_helper.c
+++ b/tools/libxl/libxl_save_helper.c
@@ -217,8 +217,6 @@ int helper_getreply(void *user)
/*----- other callbacks -----*/
-static struct save_callbacks helper_save_callbacks;
-
static void startup(const char *op) {
xtl_log(&logger,XTL_DEBUG,0,program,"starting %s",op);
@@ -234,8 +232,6 @@ static void complete(int retval) {
exit(0);
}
-static struct restore_callbacks helper_restore_callbacks;
-
int main(int argc, char **argv)
{
int r;
@@ -247,6 +243,7 @@ int main(int argc, char **argv)
assert(mode);
if (!strcmp(mode,"--save-domain")) {
+ static struct save_callbacks cb;
io_fd = atoi(NEXTARG);
recv_fd = atoi(NEXTARG);
@@ -256,16 +253,16 @@ int main(int argc, char **argv)
xc_stream_type_t stream_type = strtoul(NEXTARG,0,10);
assert(!*++argv);
- helper_setcallbacks_save(&helper_save_callbacks, cbflags);
+ helper_setcallbacks_save(&cb, cbflags);
startup("save");
setup_signals(save_signal_handler);
- r = xc_domain_save(xch, io_fd, dom, flags, &helper_save_callbacks,
- stream_type, recv_fd);
+ r = xc_domain_save(xch, io_fd, dom, flags, &cb, stream_type, recv_fd);
complete(r);
} else if (!strcmp(mode,"--restore-domain")) {
+ static struct restore_callbacks cb;
io_fd = atoi(NEXTARG);
send_back_fd = atoi(NEXTARG);
@@ -278,7 +275,7 @@ int main(int argc, char **argv)
xc_stream_type_t stream_type = strtoul(NEXTARG,0,10);
assert(!*++argv);
- helper_setcallbacks_restore(&helper_restore_callbacks, cbflags);
+ helper_setcallbacks_restore(&cb, cbflags);
unsigned long store_mfn = 0;
unsigned long console_mfn = 0;
@@ -288,8 +285,7 @@ int main(int argc, char **argv)
r = xc_domain_restore(xch, io_fd, dom, store_evtchn, &store_mfn,
store_domid, console_evtchn, &console_mfn,
- console_domid, stream_type,
- &helper_restore_callbacks, send_back_fd);
+ console_domid, stream_type, &cb, send_back_fd);
helper_stub_restore_results(store_mfn,console_mfn,0);
complete(r);
diff --git a/tools/libxl/libxl_save_msgs_gen.pl b/tools/libxl/libxl_save_msgs_gen.pl
index 6f1d79f821..831a15e0bb 100755
--- a/tools/libxl/libxl_save_msgs_gen.pl
+++ b/tools/libxl/libxl_save_msgs_gen.pl
@@ -333,7 +333,7 @@ END_ALWAYS
my $c_v = "(1u<<$msgnum)";
my $c_cb = "cbs->$name";
$f_more_sr->(" if ($c_cb) cbflags |= $c_v;\n", $enumcallbacks);
- $f_more_sr->(" $c_cb = (cbflags & $c_v) ? ${encode}_${name} : 0;\n",
+ $f_more_sr->(" if (cbflags & $c_v) $c_cb = ${encode}_${name};\n",
$setcallbacks);
}
$f_more_sr->(" return 1;\n }\n\n");
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Xen-devel] [PATCH v2 03/17] tools/migration: Drop IHDR_VERSION constant from libxc and python
2020-01-27 14:34 [Xen-devel] [PATCH v2 00/17] Support CPUID/MSR data in migration streams Andrew Cooper
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 01/17] tools/libxl: Remove libxl_cpuid_{set, apply_policy}() from the API Andrew Cooper
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 02/17] tools/libxl: Simplify callback handling in libxl-save-helper Andrew Cooper
@ 2020-01-27 14:34 ` Andrew Cooper
2020-02-24 17:25 ` Ian Jackson
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 04/17] docs/migration Specify migration v3 and STATIC_DATA_END Andrew Cooper
` (13 subsequent siblings)
16 siblings, 1 reply; 47+ messages in thread
From: Andrew Cooper @ 2020-01-27 14:34 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Marek Marczykowski-Górecki, Wei Liu, Ian Jackson
Migration v3 is in the process of being introduced, meaning that the code has
to cope with both versions. Use an explicit 2 for now.
For the verify-stream-v2 and convert-legacy-stream scripts, update text to say
"v2 (or later)". What matters is the distinction vs legacy streams.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <Ian.Jackson@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
tools/libxc/xc_sr_restore.c | 6 +++---
tools/libxc/xc_sr_save.c | 2 +-
tools/libxc/xc_sr_stream_format.h | 1 -
tools/python/scripts/convert-legacy-stream | 6 +++---
tools/python/scripts/verify-stream-v2 | 2 +-
tools/python/xen/migration/libxc.py | 9 ++++-----
6 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c
index 5e31908ca8..dc2ffcf855 100644
--- a/tools/libxc/xc_sr_restore.c
+++ b/tools/libxc/xc_sr_restore.c
@@ -35,10 +35,10 @@ static int read_headers(struct xc_sr_context *ctx)
return -1;
}
- if ( ihdr.version != IHDR_VERSION )
+ if ( ihdr.version != 2 )
{
- ERROR("Invalid Version: Expected %d, Got %d",
- IHDR_VERSION, ihdr.version);
+ ERROR("Invalid Version: Expected 2, Got %d",
+ ihdr.version);
return -1;
}
diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
index fa736a311f..5c5ce18ac3 100644
--- a/tools/libxc/xc_sr_save.c
+++ b/tools/libxc/xc_sr_save.c
@@ -13,7 +13,7 @@ static int write_headers(struct xc_sr_context *ctx, uint16_t guest_type)
struct xc_sr_ihdr ihdr = {
.marker = IHDR_MARKER,
.id = htonl(IHDR_ID),
- .version = htonl(IHDR_VERSION),
+ .version = htonl(2),
.options = htons(IHDR_OPT_LITTLE_ENDIAN),
};
struct xc_sr_dhdr dhdr = {
diff --git a/tools/libxc/xc_sr_stream_format.h b/tools/libxc/xc_sr_stream_format.h
index 37a7da6eab..ae7c0de393 100644
--- a/tools/libxc/xc_sr_stream_format.h
+++ b/tools/libxc/xc_sr_stream_format.h
@@ -23,7 +23,6 @@ struct xc_sr_ihdr
#define IHDR_MARKER 0xffffffffffffffffULL
#define IHDR_ID 0x58454E46U
-#define IHDR_VERSION 2
#define _IHDR_OPT_ENDIAN 0
#define IHDR_OPT_LITTLE_ENDIAN (0 << _IHDR_OPT_ENDIAN)
diff --git a/tools/python/scripts/convert-legacy-stream b/tools/python/scripts/convert-legacy-stream
index 2922fb3185..02a194178f 100755
--- a/tools/python/scripts/convert-legacy-stream
+++ b/tools/python/scripts/convert-legacy-stream
@@ -79,7 +79,7 @@ def write_libxc_ihdr():
stream_write(pack(libxc.IHDR_FORMAT,
libxc.IHDR_MARKER, # Marker
libxc.IHDR_IDENT, # Ident
- libxc.IHDR_VERSION, # Version
+ 2, # Version
libxc.IHDR_OPT_LE, # Options
0, 0)) # Reserved
@@ -632,13 +632,13 @@ def main():
usage = ("%prog [options] -i INPUT -o OUTPUT"
" -w WIDTH -g GUEST"),
description =
- "Convert a legacy stream to a v2 stream")
+ "Convert a legacy stream to a v2 (or later) stream")
# Required options
parser.add_option("-i", "--in", dest = "fin", metavar = "<FD or FILE>",
help = "Legacy input to convert")
parser.add_option("-o", "--out", dest = "fout", metavar = "<FD or FILE>",
- help = "v2 format output")
+ help = "v2 (or later) format output")
parser.add_option("-w", "--width", dest = "twidth",
metavar = "<32/64>", choices = ["32", "64"],
help = "Legacy toolstack bitness")
diff --git a/tools/python/scripts/verify-stream-v2 b/tools/python/scripts/verify-stream-v2
index 8bac04d566..fe82b86c11 100755
--- a/tools/python/scripts/verify-stream-v2
+++ b/tools/python/scripts/verify-stream-v2
@@ -108,7 +108,7 @@ def main():
parser = OptionParser(usage = "%prog [options]",
description =
- "Verify a stream according to the v2 spec")
+ "Verify a stream according to the v2 (or later) spec")
# Optional options
parser.add_option("-i", "--in", dest = "fin", metavar = "<FD or FILE>",
diff --git a/tools/python/xen/migration/libxc.py b/tools/python/xen/migration/libxc.py
index 8a800df980..63b3558029 100644
--- a/tools/python/xen/migration/libxc.py
+++ b/tools/python/xen/migration/libxc.py
@@ -19,7 +19,6 @@
IHDR_MARKER = 0xffffffffffffffff
IHDR_IDENT = 0x58454E46 # "XENF" in ASCII
-IHDR_VERSION = 2
IHDR_OPT_BIT_ENDIAN = 0
IHDR_OPT_LE = (0 << IHDR_OPT_BIT_ENDIAN)
@@ -113,7 +112,7 @@
HVM_PARAMS_FORMAT = "II"
class VerifyLibxc(VerifyBase):
- """ Verify a Libxc v2 stream """
+ """ Verify a Libxc v2 (or later) stream """
def __init__(self, info, read):
VerifyBase.__init__(self, info, read)
@@ -144,9 +143,9 @@ def verify_ihdr(self):
raise StreamError("Bad image id: Expected 0x%x, got 0x%x" %
(IHDR_IDENT, ident))
- if version != IHDR_VERSION:
- raise StreamError("Unknown image version: Expected %d, got %d" %
- (IHDR_VERSION, version))
+ if version != 2:
+ raise StreamError("Unknown image version: Expected 2, got %d" %
+ (version, ))
if options & IHDR_OPT_RESZ_MASK:
raise StreamError("Reserved bits set in image options field: 0x%x" %
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Xen-devel] [PATCH v2 03/17] tools/migration: Drop IHDR_VERSION constant from libxc and python
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 03/17] tools/migration: Drop IHDR_VERSION constant from libxc and python Andrew Cooper
@ 2020-02-24 17:25 ` Ian Jackson
2020-02-24 17:32 ` Andrew Cooper
0 siblings, 1 reply; 47+ messages in thread
From: Ian Jackson @ 2020-02-24 17:25 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Marek Marczykowski-Górecki, Wei Liu
Andrew Cooper writes ("[PATCH v2 03/17] tools/migration: Drop IHDR_VERSION constant from libxc and python"):
> Migration v3 is in the process of being introduced, meaning that the code has
> to cope with both versions. Use an explicit 2 for now.
>
> For the verify-stream-v2 and convert-legacy-stream scripts, update
> text to say "v2 (or later)". What matters is the distinction vs
> legacy streams.
Can I request that you use a manifest constant for `2', rather than
sprinkling literal `2's everywhere ? Something like IHDR_VERSION_2 ?
This may seem pointless, but it will mean that it is possible to grep
the code much more easily for things which are relevant to v2 or v3 or
whatever.
(I don't it's necessary to go to any great lengths to substitute the
value of IHDR_VERSION_2 into error messages; a literal 2 in the string
is OK I think.)
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [Xen-devel] [PATCH v2 03/17] tools/migration: Drop IHDR_VERSION constant from libxc and python
2020-02-24 17:25 ` Ian Jackson
@ 2020-02-24 17:32 ` Andrew Cooper
2020-03-05 17:11 ` Ian Jackson
0 siblings, 1 reply; 47+ messages in thread
From: Andrew Cooper @ 2020-02-24 17:32 UTC (permalink / raw)
To: Ian Jackson; +Cc: Xen-devel, Marek Marczykowski-Górecki, Wei Liu
On 24/02/2020 17:25, Ian Jackson wrote:
> Andrew Cooper writes ("[PATCH v2 03/17] tools/migration: Drop IHDR_VERSION constant from libxc and python"):
>> Migration v3 is in the process of being introduced, meaning that the code has
>> to cope with both versions. Use an explicit 2 for now.
>>
>> For the verify-stream-v2 and convert-legacy-stream scripts, update
>> text to say "v2 (or later)". What matters is the distinction vs
>> legacy streams.
> Can I request that you use a manifest constant for `2', rather than
> sprinkling literal `2's everywhere ? Something like IHDR_VERSION_2 ?
> This may seem pointless, but it will mean that it is possible to grep
> the code much more easily for things which are relevant to v2 or v3 or
> whatever.
>
> (I don't it's necessary to go to any great lengths to substitute the
> value of IHDR_VERSION_2 into error messages; a literal 2 in the string
> is OK I think.)
As I reply previously... The value comes out of a pipe, and is checked
exactly once.
You can already grep for everything, using format_version which is where
this number is stashed for the duration of restore.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [Xen-devel] [PATCH v2 03/17] tools/migration: Drop IHDR_VERSION constant from libxc and python
2020-02-24 17:32 ` Andrew Cooper
@ 2020-03-05 17:11 ` Ian Jackson
2020-04-14 18:16 ` Andrew Cooper
0 siblings, 1 reply; 47+ messages in thread
From: Ian Jackson @ 2020-03-05 17:11 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Marek Marczykowski-Górecki, Wei Liu
Andrew Cooper writes ("Re: [PATCH v2 03/17] tools/migration: Drop IHDR_VERSION constant from libxc and python"):
> On 24/02/2020 17:25, Ian Jackson wrote:
> > Andrew Cooper writes ("[PATCH v2 03/17] tools/migration: Drop IHDR_VERSION constant from libxc and python"):
> >> Migration v3 is in the process of being introduced, meaning that the code has
> >> to cope with both versions. Use an explicit 2 for now.
> >>
> >> For the verify-stream-v2 and convert-legacy-stream scripts, update
> >> text to say "v2 (or later)". What matters is the distinction vs
> >> legacy streams.
> > Can I request that you use a manifest constant for `2', rather than
> > sprinkling literal `2's everywhere ? Something like IHDR_VERSION_2 ?
> > This may seem pointless, but it will mean that it is possible to grep
> > the code much more easily for things which are relevant to v2 or v3 or
> > whatever.
> >
> > (I don't it's necessary to go to any great lengths to substitute the
> > value of IHDR_VERSION_2 into error messages; a literal 2 in the string
> > is OK I think.)
>
> As I reply previously... The value comes out of a pipe, and is checked
> exactly once.
I think we are talking at cross purposes.
I am suggesting that you replace every instance of a numeric literal
`2' in the code with IHDR_VERSION_2 (which would be a #define or enum
for 2).
I count 4 such literals.
> You can already grep for everything, using format_version which is where
> this number is stashed for the duration of restore.
None of the things I am talking about have "format_version" nearby.
They tend to have variants on "version" but that is a poor thing to
grep for.
Am I making any more sense now ?
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v2 03/17] tools/migration: Drop IHDR_VERSION constant from libxc and python
2020-03-05 17:11 ` Ian Jackson
@ 2020-04-14 18:16 ` Andrew Cooper
2020-04-27 16:07 ` [PATCH v2 03/17] tools/migration: Drop IHDR_VERSION constant from libxc and python [and 1 more messages] Ian Jackson
0 siblings, 1 reply; 47+ messages in thread
From: Andrew Cooper @ 2020-04-14 18:16 UTC (permalink / raw)
To: Ian Jackson; +Cc: Xen-devel, Marek Marczykowski-Górecki, Wei Liu
On 05/03/2020 17:11, Ian Jackson wrote:
> Andrew Cooper writes ("Re: [PATCH v2 03/17] tools/migration: Drop IHDR_VERSION constant from libxc and python"):
>> On 24/02/2020 17:25, Ian Jackson wrote:
>>> Andrew Cooper writes ("[PATCH v2 03/17] tools/migration: Drop IHDR_VERSION constant from libxc and python"):
>>>> Migration v3 is in the process of being introduced, meaning that the code has
>>>> to cope with both versions. Use an explicit 2 for now.
>>>>
>>>> For the verify-stream-v2 and convert-legacy-stream scripts, update
>>>> text to say "v2 (or later)". What matters is the distinction vs
>>>> legacy streams.
>>> Can I request that you use a manifest constant for `2', rather than
>>> sprinkling literal `2's everywhere ? Something like IHDR_VERSION_2 ?
>>> This may seem pointless, but it will mean that it is possible to grep
>>> the code much more easily for things which are relevant to v2 or v3 or
>>> whatever.
>>>
>>> (I don't it's necessary to go to any great lengths to substitute the
>>> value of IHDR_VERSION_2 into error messages; a literal 2 in the string
>>> is OK I think.)
>> As I reply previously... The value comes out of a pipe, and is checked
>> exactly once.
> I think we are talking at cross purposes.
>
> I am suggesting that you replace every instance of a numeric literal
> `2' in the code with IHDR_VERSION_2 (which would be a #define or enum
> for 2).
>
> I count 4 such literals.
I presume you mean here 2x send IHDR and 2x receive IHDR, one C and one
Python in each case.
I understand what you're suggesting. I completely disagree with it, as
it obfuscates the logic and introduces a source of bugs for zero gain.
IHDR_VERSION_* isn't grepable. You've got to find the only instance of
it in code to figure out what to search for.
>> You can already grep for everything, using format_version which is where
>> this number is stashed for the duration of restore.
> None of the things I am talking about have "format_version" nearby.
> They tend to have variants on "version" but that is a poor thing to
> grep for.
"ctx->restore.format_version = ihdr.version;" (the only instance in the
codebase at this point in the series) is immediately after ihdr is
sanity checked, which is sole time where idhr.version has its value
checked (in the restore path. There is also exactly one place in the
save side, and any more than this would be buggy code).
The only thing IHDR_VERSION_* usefully gets you is the ability to get
the defines accidentally wrong, and introduce bugs that way.
~Andrew
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v2 03/17] tools/migration: Drop IHDR_VERSION constant from libxc and python [and 1 more messages]
2020-04-14 18:16 ` Andrew Cooper
@ 2020-04-27 16:07 ` Ian Jackson
0 siblings, 0 replies; 47+ messages in thread
From: Ian Jackson @ 2020-04-27 16:07 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Marek Marczykowski-Górecki, Wei Liu
Andrew Cooper writes ("Re: [PATCH v2 03/17] tools/migration: Drop IHDR_VERSION constant from libxc and python"):
> I presume you mean here 2x send IHDR and 2x receive IHDR, one C and one
> Python in each case.
>
> I understand what you're suggesting. I completely disagree with it, as
> it obfuscates the logic and introduces a source of bugs for zero gain.
...
> The only thing IHDR_VERSION_* usefully gets you is the ability to get
> the defines accidentally wrong, and introduce bugs that way.
Andrew Cooper writes ("Re: [PATCH v2 07/17] libxc/restore: STATIC_DATA_END inference for v2 compatibility"):
> On 05/03/2020 16:24, Ian Jackson wrote:
> > You could handle that with a small bit of code around one of the call
> > sites to adjust the error handling. (Also, what a mess, but I guess
> > we're here now...)
>
> ... which is the majority the code you're trying to abstract away.
...
> tl;dr I could put an #ifdef x86'd static inline in the root common
> header (xc_sr_common.h), but the overall complexity is greater than what
> is presented here.
I still don't agree with you I'm afraid. I went back through our
messages, and looked at the code again, and I think we are probably
still not communicating well. However, I thought about how best to
deal with this disagreement. As the actual author of much of this
code, and certainly the person putting a lot of effort in, you should
get quite a bit of leeway.
I considered taking your branch and showing you what I meant in code
terms. But ultimately I think this is a waste of our time and I don't
want us to get into a pointless argument. I don't think these issues
matter enough to be worth the debate. I don't think there are actual
bugs here - we're talking about matters of style and taste.
So,
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
It would probably have been better for me to have got to this point
earlier.
Sorry,
Ian.
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Xen-devel] [PATCH v2 04/17] docs/migration Specify migration v3 and STATIC_DATA_END
2020-01-27 14:34 [Xen-devel] [PATCH v2 00/17] Support CPUID/MSR data in migration streams Andrew Cooper
` (2 preceding siblings ...)
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 03/17] tools/migration: Drop IHDR_VERSION constant from libxc and python Andrew Cooper
@ 2020-01-27 14:34 ` Andrew Cooper
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 05/17] python/migration: Update validation logic to understand a v3 stream Andrew Cooper
` (12 subsequent siblings)
16 siblings, 0 replies; 47+ messages in thread
From: Andrew Cooper @ 2020-01-27 14:34 UTC (permalink / raw)
To: Xen-devel
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Marek Marczykowski-Górecki,
Ian Jackson
Migration data can be split into two parts - that which is invariant of
guest execution, and that which is not. Separate these two with the
STATIC_DATA_END record.
The short term, we want to move the x86 CPU Policy data into the stream.
In the longer term, we want to provisionally send the static data only
to the destination as a more robust compatibility check. In both cases,
we will want a callback into the higher level toolstack.
Mandate the presence of the STATIC_DATA_END record, and declare this v3,
along with instructions for how to compatibly interpret a v2 stream.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Jan Beulich <JBeulich@suse.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Wei Liu <wl@xen.org>
CC: Julien Grall <julien@xen.org>
CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
docs/specs/libxc-migration-stream.pandoc | 39 +++++++++++++++++++++++++++++---
tools/libxc/xc_sr_common.c | 1 +
tools/libxc/xc_sr_stream_format.h | 1 +
tools/python/xen/migration/libxc.py | 2 ++
4 files changed, 40 insertions(+), 3 deletions(-)
diff --git a/docs/specs/libxc-migration-stream.pandoc b/docs/specs/libxc-migration-stream.pandoc
index a7a8a08936..22ff306e0b 100644
--- a/docs/specs/libxc-migration-stream.pandoc
+++ b/docs/specs/libxc-migration-stream.pandoc
@@ -127,7 +127,7 @@ marker 0xFFFFFFFFFFFFFFFF.
id 0x58454E46 ("XENF" in ASCII).
-version 0x00000002. The version of this specification.
+version 0x00000003. The version of this specification.
options bit 0: Endianness. 0 = little-endian, 1 = big-endian.
@@ -620,6 +620,21 @@ The count of pfns is: record->length/sizeof(uint64_t).
\clearpage
+STATIC_DATA_END
+---------------
+
+A static data end record marks the end of the static state. I.e. state which
+is invariant of guest execution.
+
+
+ 0 1 2 3 4 5 6 7 octet
+ +-------------------------------------------------+
+
+The end record contains no fields; its body_length is 0.
+
+\clearpage
+
+
Layout
======
@@ -639,7 +654,9 @@ A typical save record for an x86 PV guest image would look like:
* Image header
* Domain header
-* X86_PV_INFO record
+* Static data records:
+ * X86_PV_INFO record
+ * STATIC_DATA_END
* X86_PV_P2M_FRAMES record
* Many PAGE_DATA records
* X86_TSC_INFO
@@ -667,6 +684,8 @@ A typical save record for an x86 HVM guest image would look like:
* Image header
* Domain header
+* Static data records:
+ * STATIC_DATA_END
* Many PAGE_DATA records
* X86_TSC_INFO
* HVM_PARAMS
@@ -675,9 +694,23 @@ A typical save record for an x86 HVM guest image would look like:
HVM_PARAMS must precede HVM_CONTEXT, as certain parameters can affect
the validity of architectural state in the context.
+Compatibility with older versions
+=================================
+
+v3 compat with v2
+-----------------
+
+A v3 stream is compatible with a v2 stream, but mandates the presense of a
+STATIC_DATA_END record ahead of any memory/register content. This is to ease
+the introduction of new static configuration records over time.
+
+A v3-compatible reciever interpreting a v2 stream should infer the position of
+STATIC_DATA_END based on finding the first X86_PV_P2M_FRAMES record (for PV
+guests), or PAGE_DATA record (for HVM guests) and behave as if STATIC_DATA_END
+had been sent.
Legacy Images (x86 only)
-========================
+------------------------
Restoring legacy images from older tools shall be handled by
translating the legacy format image into this new format.
diff --git a/tools/libxc/xc_sr_common.c b/tools/libxc/xc_sr_common.c
index dd9a11b4b5..7f22cf0365 100644
--- a/tools/libxc/xc_sr_common.c
+++ b/tools/libxc/xc_sr_common.c
@@ -36,6 +36,7 @@ static const char *const mandatory_rec_types[] =
[REC_TYPE_VERIFY] = "Verify",
[REC_TYPE_CHECKPOINT] = "Checkpoint",
[REC_TYPE_CHECKPOINT_DIRTY_PFN_LIST] = "Checkpoint dirty pfn list",
+ [REC_TYPE_STATIC_DATA_END] = "Static data end",
};
const char *rec_type_to_str(uint32_t type)
diff --git a/tools/libxc/xc_sr_stream_format.h b/tools/libxc/xc_sr_stream_format.h
index ae7c0de393..81c9765b0a 100644
--- a/tools/libxc/xc_sr_stream_format.h
+++ b/tools/libxc/xc_sr_stream_format.h
@@ -73,6 +73,7 @@ struct xc_sr_rhdr
#define REC_TYPE_VERIFY 0x0000000dU
#define REC_TYPE_CHECKPOINT 0x0000000eU
#define REC_TYPE_CHECKPOINT_DIRTY_PFN_LIST 0x0000000fU
+#define REC_TYPE_STATIC_DATA_END 0x00000010U
#define REC_TYPE_OPTIONAL 0x80000000U
diff --git a/tools/python/xen/migration/libxc.py b/tools/python/xen/migration/libxc.py
index 63b3558029..d0c4f3527d 100644
--- a/tools/python/xen/migration/libxc.py
+++ b/tools/python/xen/migration/libxc.py
@@ -56,6 +56,7 @@
REC_TYPE_verify = 0x0000000d
REC_TYPE_checkpoint = 0x0000000e
REC_TYPE_checkpoint_dirty_pfn_list = 0x0000000f
+REC_TYPE_static_data_end = 0x00000010
rec_type_to_str = {
REC_TYPE_end : "End",
@@ -74,6 +75,7 @@
REC_TYPE_verify : "Verify",
REC_TYPE_checkpoint : "Checkpoint",
REC_TYPE_checkpoint_dirty_pfn_list : "Checkpoint dirty pfn list",
+ REC_TYPE_static_data_end : "Static data end",
}
# page_data
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Xen-devel] [PATCH v2 05/17] python/migration: Update validation logic to understand a v3 stream
2020-01-27 14:34 [Xen-devel] [PATCH v2 00/17] Support CPUID/MSR data in migration streams Andrew Cooper
` (3 preceding siblings ...)
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 04/17] docs/migration Specify migration v3 and STATIC_DATA_END Andrew Cooper
@ 2020-01-27 14:34 ` Andrew Cooper
2020-02-24 17:28 ` Ian Jackson
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 06/17] libxc/restore: Support v3 streams and handle STATIC_DATA_END Andrew Cooper
` (11 subsequent siblings)
16 siblings, 1 reply; 47+ messages in thread
From: Andrew Cooper @ 2020-01-27 14:34 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Marek Marczykowski-Górecki, Wei Liu, Ian Jackson
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <Ian.Jackson@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
tools/python/xen/migration/libxc.py | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/tools/python/xen/migration/libxc.py b/tools/python/xen/migration/libxc.py
index d0c4f3527d..5fb51b56ac 100644
--- a/tools/python/xen/migration/libxc.py
+++ b/tools/python/xen/migration/libxc.py
@@ -119,6 +119,7 @@ class VerifyLibxc(VerifyBase):
def __init__(self, info, read):
VerifyBase.__init__(self, info, read)
+ self.version = 0
self.squashed_pagedata_records = 0
@@ -145,9 +146,12 @@ def verify_ihdr(self):
raise StreamError("Bad image id: Expected 0x%x, got 0x%x" %
(IHDR_IDENT, ident))
- if version != 2:
- raise StreamError("Unknown image version: Expected 2, got %d" %
- (version, ))
+ if not (2 <= version <= 3):
+ raise StreamError(
+ "Unknown image version: Expected 2 <= ver <= 3, got %d" %
+ (version, ))
+
+ self.version = version
if options & IHDR_OPT_RESZ_MASK:
raise StreamError("Reserved bits set in image options field: 0x%x" %
@@ -164,7 +168,8 @@ def verify_ihdr(self):
"Stream is not native endianess - unable to validate")
endian = ["little", "big"][options & IHDR_OPT_LE]
- self.info("Libxc Image Header: %s endian" % (endian, ))
+ self.info("Libxc Image Header: Version %d, %s endian" %
+ (version, endian))
def verify_dhdr(self):
@@ -424,6 +429,16 @@ def verify_record_checkpoint_dirty_pfn_list(self, content):
raise RecordError("Found checkpoint dirty pfn list record in stream")
+ def verify_record_static_data_end(self, content):
+ """ static data end record """
+
+ if len(content) != 0:
+ raise RecordError("End record with non-zero length")
+
+ if self.version < 3:
+ raise RecordError("Static data end record found in v2 stream")
+
+
record_verifiers = {
REC_TYPE_end:
VerifyLibxc.verify_record_end,
@@ -465,4 +480,7 @@ def verify_record_checkpoint_dirty_pfn_list(self, content):
VerifyLibxc.verify_record_checkpoint,
REC_TYPE_checkpoint_dirty_pfn_list:
VerifyLibxc.verify_record_checkpoint_dirty_pfn_list,
+
+ REC_TYPE_static_data_end:
+ VerifyLibxc.verify_record_static_data_end,
}
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Xen-devel] [PATCH v2 06/17] libxc/restore: Support v3 streams and handle STATIC_DATA_END
2020-01-27 14:34 [Xen-devel] [PATCH v2 00/17] Support CPUID/MSR data in migration streams Andrew Cooper
` (4 preceding siblings ...)
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 05/17] python/migration: Update validation logic to understand a v3 stream Andrew Cooper
@ 2020-01-27 14:34 ` Andrew Cooper
2020-02-24 17:29 ` Ian Jackson
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 07/17] libxc/restore: STATIC_DATA_END inference for v2 compatibility Andrew Cooper
` (10 subsequent siblings)
16 siblings, 1 reply; 47+ messages in thread
From: Andrew Cooper @ 2020-01-27 14:34 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Ian Jackson
Higher level toolstacks may wish to know when the static data is complete, so
introduce a restore_callback for the purpose.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <Ian.Jackson@citrix.com>
CC: Wei Liu <wl@xen.org>
v2:
* Split/rearranged from v1
---
tools/libxc/include/xenguest.h | 3 +++
tools/libxc/xc_sr_common.h | 3 +++
tools/libxc/xc_sr_restore.c | 29 +++++++++++++++++++++++++++--
3 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h
index 19d828a7f2..efd90b0d42 100644
--- a/tools/libxc/include/xenguest.h
+++ b/tools/libxc/include/xenguest.h
@@ -139,6 +139,9 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom,
/* callbacks provided by xc_domain_restore */
struct restore_callbacks {
+ /* Called once the STATIC_DATA_END record has been received. */
+ int (*static_data_done)(void *data);
+
/* Called after a new checkpoint to suspend the guest. */
int (*suspend)(void *data);
diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
index 5dd51ccb15..ae0ab70f76 100644
--- a/tools/libxc/xc_sr_common.h
+++ b/tools/libxc/xc_sr_common.h
@@ -253,6 +253,9 @@ struct xc_sr_context
/* Currently buffering records between a checkpoint */
bool buffer_all_records;
+ /* Whether a STATIC_DATA_END record has been seen. */
+ bool seen_static_data_end;
+
/*
* With Remus/COLO, we buffer the records sent by the primary at checkpoint,
* in case the primary will fail, we can recover from the last
diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c
index dc2ffcf855..9c924387ae 100644
--- a/tools/libxc/xc_sr_restore.c
+++ b/tools/libxc/xc_sr_restore.c
@@ -35,9 +35,9 @@ static int read_headers(struct xc_sr_context *ctx)
return -1;
}
- if ( ihdr.version != 2 )
+ if ( ihdr.version < 2 || ihdr.version > 3 )
{
- ERROR("Invalid Version: Expected 2, Got %d",
+ ERROR("Invalid Version: Expected 2 <= ver <= 3, Got %d",
ihdr.version);
return -1;
}
@@ -631,6 +631,27 @@ static int buffer_record(struct xc_sr_context *ctx, struct xc_sr_record *rec)
return 0;
}
+static int handle_static_data_end(struct xc_sr_context *ctx)
+{
+ xc_interface *xch = ctx->xch;
+ int rc = 0;
+
+ if ( ctx->restore.seen_static_data_end )
+ {
+ ERROR("Multiple STATIC_DATA_END records found");
+ return -1;
+ }
+
+ ctx->restore.seen_static_data_end = true;
+
+ if ( ctx->restore.callbacks->static_data_done &&
+ (rc = ctx->restore.callbacks->static_data_done(
+ ctx->restore.callbacks->data) != 0) )
+ ERROR("static_data_done() callback failed: %d\n", rc);
+
+ return rc;
+}
+
static int process_record(struct xc_sr_context *ctx, struct xc_sr_record *rec)
{
xc_interface *xch = ctx->xch;
@@ -654,6 +675,10 @@ static int process_record(struct xc_sr_context *ctx, struct xc_sr_record *rec)
rc = handle_checkpoint(ctx);
break;
+ case REC_TYPE_STATIC_DATA_END:
+ rc = handle_static_data_end(ctx);
+ break;
+
default:
rc = ctx->restore.ops.process_record(ctx, rec);
break;
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Xen-devel] [PATCH v2 07/17] libxc/restore: STATIC_DATA_END inference for v2 compatibility
2020-01-27 14:34 [Xen-devel] [PATCH v2 00/17] Support CPUID/MSR data in migration streams Andrew Cooper
` (5 preceding siblings ...)
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 06/17] libxc/restore: Support v3 streams and handle STATIC_DATA_END Andrew Cooper
@ 2020-01-27 14:34 ` Andrew Cooper
2020-02-24 17:32 ` Ian Jackson
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 08/17] libxc/save: Write a v3 stream Andrew Cooper
` (9 subsequent siblings)
16 siblings, 1 reply; 47+ messages in thread
From: Andrew Cooper @ 2020-01-27 14:34 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Ian Jackson
A v3 stream can compatibly read a v2 stream by inferring the position of the
STATIC_DATA_END record.
v2 compatibility is only needed for x86. No other architectures exist yet,
but they will have a minimum of v3 when introduced.
The x86 HVM compatibility point being in handle_page_data() (which is common
code) is a bit awkward. However, as the two compatibility points are subtly
different, and it is (intentionally) not possible to call into arch specific
code from common code (except via the ops hooks), use some #ifdef-ary and
opencode the check, rather than make handle_page_data() a per-arch helper.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <Ian.Jackson@citrix.com>
CC: Wei Liu <wl@xen.org>
v2:
* Split/rearranged from v1
* Rewrite the commit message to explain why compatibility is done this way.
---
tools/libxc/include/xenguest.h | 2 +-
tools/libxc/xc_sr_common.h | 5 ++++-
tools/libxc/xc_sr_restore.c | 27 ++++++++++++++++++++++++++-
tools/libxc/xc_sr_restore_x86_pv.c | 17 +++++++++++++++++
4 files changed, 48 insertions(+), 3 deletions(-)
diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h
index efd90b0d42..b4df8d0ffe 100644
--- a/tools/libxc/include/xenguest.h
+++ b/tools/libxc/include/xenguest.h
@@ -139,7 +139,7 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom,
/* callbacks provided by xc_domain_restore */
struct restore_callbacks {
- /* Called once the STATIC_DATA_END record has been received. */
+ /* Called once the STATIC_DATA_END record has been received/inferred. */
int (*static_data_done)(void *data);
/* Called after a new checkpoint to suspend the guest. */
diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
index ae0ab70f76..51e3d3ee3b 100644
--- a/tools/libxc/xc_sr_common.h
+++ b/tools/libxc/xc_sr_common.h
@@ -253,7 +253,7 @@ struct xc_sr_context
/* Currently buffering records between a checkpoint */
bool buffer_all_records;
- /* Whether a STATIC_DATA_END record has been seen. */
+ /* Whether a STATIC_DATA_END record has been seen/inferred. */
bool seen_static_data_end;
/*
@@ -428,6 +428,9 @@ int read_record(struct xc_sr_context *ctx, int fd, struct xc_sr_record *rec);
int populate_pfns(struct xc_sr_context *ctx, unsigned int count,
const xen_pfn_t *original_pfns, const uint32_t *types);
+/* Handle a STATIC_DATA_END record. */
+int handle_static_data_end(struct xc_sr_context *ctx);
+
#endif
/*
* Local variables:
diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c
index 9c924387ae..bb94cd879d 100644
--- a/tools/libxc/xc_sr_restore.c
+++ b/tools/libxc/xc_sr_restore.c
@@ -342,6 +342,31 @@ static int handle_page_data(struct xc_sr_context *ctx, struct xc_sr_record *rec)
xen_pfn_t *pfns = NULL, pfn;
uint32_t *types = NULL, type;
+ /*
+ * v2 compatibility only exists for x86 streams. This is a bit of a
+ * bodge, but it is less bad than duplicating handle_page_data() between
+ * different architectures.
+ */
+#if defined(__i386__) || defined(__x86_64__)
+ /* v2 compat. Infer the position of STATIC_DATA_END. */
+ if ( ctx->restore.format_version < 3 && !ctx->restore.seen_static_data_end )
+ {
+ rc = handle_static_data_end(ctx);
+ if ( rc )
+ {
+ ERROR("Inferred STATIC_DATA_END record failed");
+ goto err;
+ }
+ rc = -1;
+ }
+
+ if ( !ctx->restore.seen_static_data_end )
+ {
+ ERROR("No STATIC_DATA_END seen");
+ goto err;
+ }
+#endif
+
if ( rec->length < sizeof(*pages) )
{
ERROR("PAGE_DATA record truncated: length %u, min %zu",
@@ -631,7 +656,7 @@ static int buffer_record(struct xc_sr_context *ctx, struct xc_sr_record *rec)
return 0;
}
-static int handle_static_data_end(struct xc_sr_context *ctx)
+int handle_static_data_end(struct xc_sr_context *ctx)
{
xc_interface *xch = ctx->xch;
int rc = 0;
diff --git a/tools/libxc/xc_sr_restore_x86_pv.c b/tools/libxc/xc_sr_restore_x86_pv.c
index 16e738884e..3756225be6 100644
--- a/tools/libxc/xc_sr_restore_x86_pv.c
+++ b/tools/libxc/xc_sr_restore_x86_pv.c
@@ -679,6 +679,23 @@ static int handle_x86_pv_p2m_frames(struct xc_sr_context *ctx,
unsigned int start, end, x, fpp = PAGE_SIZE / ctx->x86.pv.width;
int rc;
+ /* v2 compat. Infer the position of STATIC_DATA_END. */
+ if ( ctx->restore.format_version < 3 && !ctx->restore.seen_static_data_end )
+ {
+ rc = handle_static_data_end(ctx);
+ if ( rc )
+ {
+ ERROR("Inferred STATIC_DATA_END record failed");
+ return rc;
+ }
+ }
+
+ if ( !ctx->restore.seen_static_data_end )
+ {
+ ERROR("No STATIC_DATA_END seen");
+ return -1;
+ }
+
if ( !ctx->x86.pv.restore.seen_pv_info )
{
ERROR("Not yet received X86_PV_INFO record");
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Xen-devel] [PATCH v2 07/17] libxc/restore: STATIC_DATA_END inference for v2 compatibility
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 07/17] libxc/restore: STATIC_DATA_END inference for v2 compatibility Andrew Cooper
@ 2020-02-24 17:32 ` Ian Jackson
2020-02-28 14:51 ` Andrew Cooper
0 siblings, 1 reply; 47+ messages in thread
From: Ian Jackson @ 2020-02-24 17:32 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Wei Liu
Andrew Cooper writes ("[PATCH v2 07/17] libxc/restore: STATIC_DATA_END inference for v2 compatibility"):
> A v3 stream can compatibly read a v2 stream by inferring the position of the
> STATIC_DATA_END record.
>
> v2 compatibility is only needed for x86. No other architectures exist yet,
> but they will have a minimum of v3 when introduced.
>
> The x86 HVM compatibility point being in handle_page_data() (which is common
> code) is a bit awkward. However, as the two compatibility points are subtly
> different, and it is (intentionally) not possible to call into arch specific
> code from common code (except via the ops hooks), use some #ifdef-ary and
> opencode the check, rather than make handle_page_data() a per-arch helper.
...
> +#if defined(__i386__) || defined(__x86_64__)
> + /* v2 compat. Infer the position of STATIC_DATA_END. */
> + if ( ctx->restore.format_version < 3 && !ctx->restore.seen_static_data_end )
> + {
> + rc = handle_static_data_end(ctx);
> + if ( rc )
These 17 lines appears twice, in basically identical form. Could it
be refactored ?
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [Xen-devel] [PATCH v2 07/17] libxc/restore: STATIC_DATA_END inference for v2 compatibility
2020-02-24 17:32 ` Ian Jackson
@ 2020-02-28 14:51 ` Andrew Cooper
2020-03-05 16:24 ` Ian Jackson
0 siblings, 1 reply; 47+ messages in thread
From: Andrew Cooper @ 2020-02-28 14:51 UTC (permalink / raw)
To: Ian Jackson; +Cc: Xen-devel, Wei Liu
On 24/02/2020 17:32, Ian Jackson wrote:
> Andrew Cooper writes ("[PATCH v2 07/17] libxc/restore: STATIC_DATA_END inference for v2 compatibility"):
>> A v3 stream can compatibly read a v2 stream by inferring the position of the
>> STATIC_DATA_END record.
>>
>> v2 compatibility is only needed for x86. No other architectures exist yet,
>> but they will have a minimum of v3 when introduced.
>>
>> The x86 HVM compatibility point being in handle_page_data() (which is common
>> code) is a bit awkward. However, as the two compatibility points are subtly
>> different, and it is (intentionally) not possible to call into arch specific
>> code from common code (except via the ops hooks), use some #ifdef-ary and
>> opencode the check, rather than make handle_page_data() a per-arch helper.
> ...
>> +#if defined(__i386__) || defined(__x86_64__)
>> + /* v2 compat. Infer the position of STATIC_DATA_END. */
>> + if ( ctx->restore.format_version < 3 && !ctx->restore.seen_static_data_end )
>> + {
>> + rc = handle_static_data_end(ctx);
>> + if ( rc )
> These 17 lines appears twice, in basically identical form. Could it
> be refactored ?
Not really, no.
The error handling (i.e. half of those 17 lines) is different, making it
somewhat awkward to fit into a static inline.
More importantly however, by design, common code can't call
arch-specific code without a restore_ops hook. Deduping these would
require breaking the restriction which is currently doing a decent job
of keeping x86-isms out of common code.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [Xen-devel] [PATCH v2 07/17] libxc/restore: STATIC_DATA_END inference for v2 compatibility
2020-02-28 14:51 ` Andrew Cooper
@ 2020-03-05 16:24 ` Ian Jackson
2020-04-14 18:43 ` Andrew Cooper
0 siblings, 1 reply; 47+ messages in thread
From: Ian Jackson @ 2020-03-05 16:24 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Wei Liu
Andrew Cooper writes ("Re: [PATCH v2 07/17] libxc/restore: STATIC_DATA_END inference for v2 compatibility"):
> On 24/02/2020 17:32, Ian Jackson wrote:
> > These 17 lines appears twice, in basically identical form. Could it
> > be refactored ?
>
> Not really, no.
>
> The error handling (i.e. half of those 17 lines) is different, making it
> somewhat awkward to fit into a static inline.
You could handle that with a small bit of code around one of the call
sites to adjust the error handling. (Also, what a mess, but I guess
we're here now...)
> More importantly however, by design, common code can't call
> arch-specific code without a restore_ops hook. Deduping these would
> require breaking the restriction which is currently doing a decent job
> of keeping x86-isms out of common code.
I'm afraid you're going to have to explain that to me at a bit greater
length. The biggest thing that is confusing me about your statement
here is that your patch is indeed adding x86-specific code to a common
file. But also I don't understand the comment about restore_ops
hooks - do you mean something in restore_callbacks ?
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v2 07/17] libxc/restore: STATIC_DATA_END inference for v2 compatibility
2020-03-05 16:24 ` Ian Jackson
@ 2020-04-14 18:43 ` Andrew Cooper
2020-05-29 15:53 ` Ian Jackson
0 siblings, 1 reply; 47+ messages in thread
From: Andrew Cooper @ 2020-04-14 18:43 UTC (permalink / raw)
To: Ian Jackson; +Cc: Xen-devel, Wei Liu
On 05/03/2020 16:24, Ian Jackson wrote:
> Andrew Cooper writes ("Re: [PATCH v2 07/17] libxc/restore: STATIC_DATA_END inference for v2 compatibility"):
>> On 24/02/2020 17:32, Ian Jackson wrote:
>>> These 17 lines appears twice, in basically identical form. Could it
>>> be refactored ?
>> Not really, no.
>>
>> The error handling (i.e. half of those 17 lines) is different, making it
>> somewhat awkward to fit into a static inline.
> You could handle that with a small bit of code around one of the call
> sites to adjust the error handling. (Also, what a mess, but I guess
> we're here now...)
... which is the majority the code you're trying to abstract away.
>
>> More importantly however, by design, common code can't call
>> arch-specific code without a restore_ops hook. Deduping these would
>> require breaking the restriction which is currently doing a decent job
>> of keeping x86-isms out of common code.
> I'm afraid you're going to have to explain that to me at a bit greater
> length. The biggest thing that is confusing me about your statement
> here is that your patch is indeed adding x86-specific code to a common
> file. But also I don't understand the comment about restore_ops
> hooks - do you mean something in restore_callbacks ?
No. restore_callbacks are plumbing between libxl-save-helper and libxc.
restore_ops are internal to the restore code, and come in x86_pv and
x86_hvm flavours, with ARM existing in some theoretical future. The
design of the common save/restore code had deliberate measures put in
place to make it hard to get arch-specific details slip into common
code, so porting to different architectures didn't have to start by
doing a bunch of cleanup.
tl;dr I could put an #ifdef x86'd static inline in the root common
header (xc_sr_common.h), but the overall complexity is greater than what
is presented here.
~Andrew
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v2 07/17] libxc/restore: STATIC_DATA_END inference for v2 compatibility
2020-04-14 18:43 ` Andrew Cooper
@ 2020-05-29 15:53 ` Ian Jackson
0 siblings, 0 replies; 47+ messages in thread
From: Ian Jackson @ 2020-05-29 15:53 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Wei Liu
Andrew Cooper writes ("Re: [PATCH v2 07/17] libxc/restore: STATIC_DATA_END inference for v2 compatibility"):
> On 05/03/2020 16:24, Ian Jackson wrote:
> > Andrew Cooper writes ("Re: [PATCH v2 07/17] libxc/restore: STATIC_DATA_END inference for v2 compatibility"):
> >> More importantly however, by design, common code can't call
> >> arch-specific code without a restore_ops hook. Deduping these would
> >> require breaking the restriction which is currently doing a decent job
> >> of keeping x86-isms out of common code.
> > I'm afraid you're going to have to explain that to me at a bit greater
> > length. The biggest thing that is confusing me about your statement
> > here is that your patch is indeed adding x86-specific code to a common
> > file. But also I don't understand the comment about restore_ops
> > hooks - do you mean something in restore_callbacks ?
>
> No. restore_callbacks are plumbing between libxl-save-helper and libxc.
>
> restore_ops are internal to the restore code, and come in x86_pv and
> x86_hvm flavours, with ARM existing in some theoretical future. The
> design of the common save/restore code had deliberate measures put in
> place to make it hard to get arch-specific details slip into common
> code, so porting to different architectures didn't have to start by
> doing a bunch of cleanup.
>
> tl;dr I could put an #ifdef x86'd static inline in the root common
> header (xc_sr_common.h), but the overall complexity is greater than what
> is presented here.
Well, I still don't quite follow but as you point out on irc I haven't
replied for too long. I don't think I should withhold my ack at this
stage.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Thanks,
Ian.
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Xen-devel] [PATCH v2 08/17] libxc/save: Write a v3 stream
2020-01-27 14:34 [Xen-devel] [PATCH v2 00/17] Support CPUID/MSR data in migration streams Andrew Cooper
` (6 preceding siblings ...)
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 07/17] libxc/restore: STATIC_DATA_END inference for v2 compatibility Andrew Cooper
@ 2020-01-27 14:34 ` Andrew Cooper
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 09/17] tools/libxl: Provide a static_data_done callback for domain restore Andrew Cooper
` (8 subsequent siblings)
16 siblings, 0 replies; 47+ messages in thread
From: Andrew Cooper @ 2020-01-27 14:34 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Marek Marczykowski-Górecki, Wei Liu, Ian Jackson
Introduce a new static_data() hook which is responsible for writing out
any static data records. The HVM side continues to be a no-op, while
the PV side moves write_x86_pv_info() into this earlier hook. The the
common code writes out a STATIC_DATA_END record, and the stream version
is bumped to 3.
Update convert-legacy-stream to write a v3 stream, because this will
bypass the compatibly logic in libxc.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
CC: Ian Jackson <Ian.Jackson@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
tools/libxc/xc_sr_common.h | 10 ++++++++--
tools/libxc/xc_sr_save.c | 20 +++++++++++++++++++-
tools/libxc/xc_sr_save_x86_hvm.c | 6 ++++++
tools/libxc/xc_sr_save_x86_pv.c | 10 ++++++----
tools/python/scripts/convert-legacy-stream | 9 ++++++++-
5 files changed, 47 insertions(+), 8 deletions(-)
diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
index 51e3d3ee3b..fd7fb67305 100644
--- a/tools/libxc/xc_sr_common.h
+++ b/tools/libxc/xc_sr_common.h
@@ -63,8 +63,14 @@ struct xc_sr_save_ops
int (*setup)(struct xc_sr_context *ctx);
/**
- * Send records which need to be at the start of the stream. This is
- * called once, after the Image and Domain headers are written.
+ * Send static records at the head of the stream. This is called once,
+ * after the Image and Domain headers are written.
+ */
+ int (*static_data)(struct xc_sr_context *ctx);
+
+ /**
+ * Send dynamic records which need to be at the start of the stream. This
+ * is called after the STATIC_DATA_END record is written.
*/
int (*start_of_stream)(struct xc_sr_context *ctx);
diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
index 5c5ce18ac3..2b6c55af2a 100644
--- a/tools/libxc/xc_sr_save.c
+++ b/tools/libxc/xc_sr_save.c
@@ -13,7 +13,7 @@ static int write_headers(struct xc_sr_context *ctx, uint16_t guest_type)
struct xc_sr_ihdr ihdr = {
.marker = IHDR_MARKER,
.id = htonl(IHDR_ID),
- .version = htonl(2),
+ .version = htonl(3),
.options = htons(IHDR_OPT_LITTLE_ENDIAN),
};
struct xc_sr_dhdr dhdr = {
@@ -55,6 +55,16 @@ static int write_end_record(struct xc_sr_context *ctx)
}
/*
+ * Writes a STATIC_DATA_END record into the stream.
+ */
+static int write_static_data_end_record(struct xc_sr_context *ctx)
+{
+ struct xc_sr_record end = { .type = REC_TYPE_STATIC_DATA_END };
+
+ return write_record(ctx, &end);
+}
+
+/*
* Writes a CHECKPOINT record into the stream.
*/
static int write_checkpoint_record(struct xc_sr_context *ctx)
@@ -849,6 +859,14 @@ static int save(struct xc_sr_context *ctx, uint16_t guest_type)
if ( rc )
goto err;
+ rc = ctx->save.ops.static_data(ctx);
+ if ( rc )
+ goto err;
+
+ rc = write_static_data_end_record(ctx);
+ if ( rc )
+ goto err;
+
rc = ctx->save.ops.start_of_stream(ctx);
if ( rc )
goto err;
diff --git a/tools/libxc/xc_sr_save_x86_hvm.c b/tools/libxc/xc_sr_save_x86_hvm.c
index d99efe65e5..93bcc1c273 100644
--- a/tools/libxc/xc_sr_save_x86_hvm.c
+++ b/tools/libxc/xc_sr_save_x86_hvm.c
@@ -170,6 +170,11 @@ static int x86_hvm_setup(struct xc_sr_context *ctx)
return 0;
}
+static int x86_hvm_static_data(struct xc_sr_context *ctx)
+{
+ return 0;
+}
+
static int x86_hvm_start_of_stream(struct xc_sr_context *ctx)
{
return 0;
@@ -228,6 +233,7 @@ struct xc_sr_save_ops save_ops_x86_hvm =
.pfn_to_gfn = x86_hvm_pfn_to_gfn,
.normalise_page = x86_hvm_normalise_page,
.setup = x86_hvm_setup,
+ .static_data = x86_hvm_static_data,
.start_of_stream = x86_hvm_start_of_stream,
.start_of_checkpoint = x86_hvm_start_of_checkpoint,
.end_of_checkpoint = x86_hvm_end_of_checkpoint,
diff --git a/tools/libxc/xc_sr_save_x86_pv.c b/tools/libxc/xc_sr_save_x86_pv.c
index f3ccf5bb4b..46019d962d 100644
--- a/tools/libxc/xc_sr_save_x86_pv.c
+++ b/tools/libxc/xc_sr_save_x86_pv.c
@@ -1052,14 +1052,15 @@ static int x86_pv_setup(struct xc_sr_context *ctx)
return 0;
}
+static int x86_pv_static_data(struct xc_sr_context *ctx)
+{
+ return write_x86_pv_info(ctx);
+}
+
static int x86_pv_start_of_stream(struct xc_sr_context *ctx)
{
int rc;
- rc = write_x86_pv_info(ctx);
- if ( rc )
- return rc;
-
/*
* Ideally should be able to change during migration. Currently
* corruption will occur if the contents or location of the P2M changes
@@ -1126,6 +1127,7 @@ struct xc_sr_save_ops save_ops_x86_pv =
.pfn_to_gfn = x86_pv_pfn_to_gfn,
.normalise_page = x86_pv_normalise_page,
.setup = x86_pv_setup,
+ .static_data = x86_pv_static_data,
.start_of_stream = x86_pv_start_of_stream,
.start_of_checkpoint = x86_pv_start_of_checkpoint,
.end_of_checkpoint = x86_pv_end_of_checkpoint,
diff --git a/tools/python/scripts/convert-legacy-stream b/tools/python/scripts/convert-legacy-stream
index 02a194178f..ca93a93848 100755
--- a/tools/python/scripts/convert-legacy-stream
+++ b/tools/python/scripts/convert-legacy-stream
@@ -79,7 +79,7 @@ def write_libxc_ihdr():
stream_write(pack(libxc.IHDR_FORMAT,
libxc.IHDR_MARKER, # Marker
libxc.IHDR_IDENT, # Ident
- 2, # Version
+ 3, # Version
libxc.IHDR_OPT_LE, # Options
0, 0)) # Reserved
@@ -166,6 +166,9 @@ def write_libxc_hvm_params(params):
pack(libxc.HVM_PARAMS_FORMAT, len(params) / 2, 0),
pack("Q" * len(params), *params))
+def write_libxc_static_data_end():
+ write_record(libxc.REC_TYPE_static_data_end)
+
def write_libxl_end():
write_record(libxl.REC_TYPE_end)
@@ -590,6 +593,10 @@ def read_legacy_stream(vm):
if pv:
read_pv_extended_info(vm)
+
+ write_libxc_static_data_end()
+
+ if pv:
read_pv_p2m_frames(vm)
read_chunks(vm)
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Xen-devel] [PATCH v2 09/17] tools/libxl: Provide a static_data_done callback for domain restore
2020-01-27 14:34 [Xen-devel] [PATCH v2 00/17] Support CPUID/MSR data in migration streams Andrew Cooper
` (7 preceding siblings ...)
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 08/17] libxc/save: Write a v3 stream Andrew Cooper
@ 2020-01-27 14:34 ` Andrew Cooper
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 10/17] tools/libxl: Plumb a restore boolean down into libxl__build_pre() Andrew Cooper
` (7 subsequent siblings)
16 siblings, 0 replies; 47+ messages in thread
From: Andrew Cooper @ 2020-01-27 14:34 UTC (permalink / raw)
To: Xen-devel; +Cc: Anthony PERARD, Andrew Cooper, Wei Liu, Ian Jackson
This will be needed shortly to provide backwards compatiblity for migration
streams which do not have CPUID information contained within them.
No functional change yet.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <Ian.Jackson@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Anthony PERARD <anthony.perard@citrix.com>
v2:
* Split/rearranged from v1
---
tools/libxl/libxl_create.c | 12 ++++++++++++
tools/libxl/libxl_save_msgs_gen.pl | 1 +
2 files changed, 13 insertions(+)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 32d45dcef0..12113185ac 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1227,6 +1227,7 @@ static void domcreate_bootloader_done(libxl__egc *egc,
dcs->srs.dcs = dcs;
/* Restore */
+ callbacks->static_data_done = libxl__srm_callout_callback_static_data_done;
callbacks->restore_results = libxl__srm_callout_callback_restore_results;
/* COLO only supports HVM now because it does not work very
@@ -1296,6 +1297,17 @@ static void libxl__colo_restore_setup_done(libxl__egc *egc,
libxl__stream_read_start(egc, &dcs->srs);
}
+int libxl__srm_callout_callback_static_data_done(void *user)
+{
+ libxl__save_helper_state *shs = user;
+ libxl__domain_create_state *dcs = shs->caller_state;
+ STATE_AO_GC(dcs->ao);
+
+ /* Nothing to do (yet). */
+
+ return 0;
+}
+
void libxl__srm_callout_callback_restore_results(xen_pfn_t store_mfn,
xen_pfn_t console_mfn, void *user)
{
diff --git a/tools/libxl/libxl_save_msgs_gen.pl b/tools/libxl/libxl_save_msgs_gen.pl
index 831a15e0bb..93dc252370 100755
--- a/tools/libxl/libxl_save_msgs_gen.pl
+++ b/tools/libxl/libxl_save_msgs_gen.pl
@@ -29,6 +29,7 @@ our @msgs = (
[ 'srcxA', "wait_checkpoint", [] ],
[ 'scxA', "switch_qemu_logdirty", [qw(uint32_t domid
unsigned enable)] ],
+ [ 'rcxW', "static_data_done", [] ],
[ 'rcx', "restore_results", ['xen_pfn_t', 'store_gfn',
'xen_pfn_t', 'console_gfn'] ],
[ 'srW', "complete", [qw(int retval
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Xen-devel] [PATCH v2 10/17] tools/libxl: Plumb a restore boolean down into libxl__build_pre()
2020-01-27 14:34 [Xen-devel] [PATCH v2 00/17] Support CPUID/MSR data in migration streams Andrew Cooper
` (8 preceding siblings ...)
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 09/17] tools/libxl: Provide a static_data_done callback for domain restore Andrew Cooper
@ 2020-01-27 14:34 ` Andrew Cooper
2020-02-24 17:39 ` Ian Jackson
2020-04-14 20:23 ` [PATCH v3 10/17] tools/libxl: Plumb a restore boolean into libxl__domain_build_state Andrew Cooper
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 11/17] tools/libxl: Re-position CPUID handling during domain construction Andrew Cooper
` (6 subsequent siblings)
16 siblings, 2 replies; 47+ messages in thread
From: Andrew Cooper @ 2020-01-27 14:34 UTC (permalink / raw)
To: Xen-devel; +Cc: Anthony PERARD, Andrew Cooper, Wei Liu, Ian Jackson
To fix CPUID handling, libxl__build_pre() is going to have to distinguish
between a brand new VM vs one which is being migrated-in/resumed.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <Ian.Jackson@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Anthony PERARD <anthony.perard@citrix.com>
v2:
* New. This is c/s aacc1430064 "tools/libxl: Plumb domain_create_state down
into libxl__build_pre()" take-2, without any collateral damage to stubdoms.
---
tools/libxl/libxl_create.c | 9 +++++----
tools/libxl/libxl_dm.c | 2 +-
tools/libxl/libxl_dom.c | 4 +++-
tools/libxl/libxl_internal.h | 6 ++++--
4 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 12113185ac..1d2e193509 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -434,14 +434,15 @@ static void init_console_info(libxl__gc *gc,
int libxl__domain_build(libxl__gc *gc,
libxl_domain_config *d_config,
uint32_t domid,
- libxl__domain_build_state *state)
+ libxl__domain_build_state *state,
+ bool restore)
{
libxl_domain_build_info *const info = &d_config->b_info;
char **vments = NULL, **localents = NULL;
struct timeval start_time;
int i, ret;
- ret = libxl__build_pre(gc, domid, d_config, state);
+ ret = libxl__build_pre(gc, domid, d_config, state, restore);
if (ret)
goto out;
@@ -1218,7 +1219,7 @@ static void domcreate_bootloader_done(libxl__egc *egc,
dcs->sdss.callback = domcreate_devmodel_started;
if (restore_fd < 0 && dcs->domid_soft_reset == INVALID_DOMID) {
- rc = libxl__domain_build(gc, d_config, domid, state);
+ rc = libxl__domain_build(gc, d_config, domid, state, false);
domcreate_rebuild_done(egc, dcs, rc);
return;
}
@@ -1245,7 +1246,7 @@ static void domcreate_bootloader_done(libxl__egc *egc,
goto out;
}
- rc = libxl__build_pre(gc, domid, d_config, state);
+ rc = libxl__build_pre(gc, domid, d_config, state, restore_fd >= 0);
if (rc)
goto out;
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index e92e412c1b..d3dfa8751c 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2197,7 +2197,7 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
if (ret)
goto out;
uint32_t dm_domid = sdss->pvqemu.guest_domid;
- ret = libxl__domain_build(gc, dm_config, dm_domid, stubdom_state);
+ ret = libxl__domain_build(gc, dm_config, dm_domid, stubdom_state, false);
if (ret)
goto out;
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index b730365f47..1bac277351 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -244,7 +244,9 @@ static int numa_place_domain(libxl__gc *gc, uint32_t domid,
}
int libxl__build_pre(libxl__gc *gc, uint32_t domid,
- libxl_domain_config *d_config, libxl__domain_build_state *state)
+ libxl_domain_config *d_config,
+ libxl__domain_build_state *state,
+ bool restore)
{
libxl_domain_build_info *const info = &d_config->b_info;
libxl_ctx *ctx = libxl__gc_owner(gc);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 50856ca703..e66b068d16 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1390,7 +1390,8 @@ _hidden void libxl__domain_build_state_dispose(libxl__domain_build_state *s);
_hidden int libxl__build_pre(libxl__gc *gc, uint32_t domid,
libxl_domain_config * const d_config,
- libxl__domain_build_state *state);
+ libxl__domain_build_state *state,
+ bool restore);
_hidden int libxl__build_post(libxl__gc *gc, uint32_t domid,
libxl_domain_build_info *info, libxl__domain_build_state *state,
char **vms_ents, char **local_ents);
@@ -1963,7 +1964,8 @@ _hidden int libxl__domain_make(libxl__gc *gc,
_hidden int libxl__domain_build(libxl__gc *gc,
libxl_domain_config *d_config,
uint32_t domid,
- libxl__domain_build_state *state);
+ libxl__domain_build_state *state,
+ bool restore);
/* for device model creation */
_hidden const char *libxl__domain_device_model(libxl__gc *gc,
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Xen-devel] [PATCH v2 10/17] tools/libxl: Plumb a restore boolean down into libxl__build_pre()
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 10/17] tools/libxl: Plumb a restore boolean down into libxl__build_pre() Andrew Cooper
@ 2020-02-24 17:39 ` Ian Jackson
2020-02-28 18:50 ` Andrew Cooper
2020-04-14 20:23 ` [PATCH v3 10/17] tools/libxl: Plumb a restore boolean into libxl__domain_build_state Andrew Cooper
1 sibling, 1 reply; 47+ messages in thread
From: Ian Jackson @ 2020-02-24 17:39 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Anthony Perard, Xen-devel, Wei Liu
Andrew Cooper writes ("[PATCH v2 10/17] tools/libxl: Plumb a restore boolean down into libxl__build_pre()"):
> To fix CPUID handling, libxl__build_pre() is going to have to distinguish
> between a brand new VM vs one which is being migrated-in/resumed.
The distinction between libxl__build_pre and these other functions is
not particularly principled. It is a legacy from an old API (prior to
the existince of *create) where libxl callers were expected to "build"
a domain first and then do other things to it.
Maybe it would be better to pass this via libxl__domain_build_state
rather than as an additional parameter ?
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [Xen-devel] [PATCH v2 10/17] tools/libxl: Plumb a restore boolean down into libxl__build_pre()
2020-02-24 17:39 ` Ian Jackson
@ 2020-02-28 18:50 ` Andrew Cooper
2020-03-05 17:07 ` Ian Jackson
0 siblings, 1 reply; 47+ messages in thread
From: Andrew Cooper @ 2020-02-28 18:50 UTC (permalink / raw)
To: Ian Jackson; +Cc: Anthony Perard, Xen-devel, Wei Liu
On 24/02/2020 17:39, Ian Jackson wrote:
> Andrew Cooper writes ("[PATCH v2 10/17] tools/libxl: Plumb a restore boolean down into libxl__build_pre()"):
>> To fix CPUID handling, libxl__build_pre() is going to have to distinguish
>> between a brand new VM vs one which is being migrated-in/resumed.
> The distinction between libxl__build_pre and these other functions is
> not particularly principled. It is a legacy from an old API (prior to
> the existince of *create) where libxl callers were expected to "build"
> a domain first and then do other things to it.
>
> Maybe it would be better to pass this via libxl__domain_build_state
> rather than as an additional parameter ?
Well - I tried a similar approach the first time around, and it broke
stubdoms so badly it needed reverting.
(Untrim the commit details)
> v2:
> * New. This is c/s aacc1430064 "tools/libxl: Plumb domain_create_state down
> into libxl__build_pre()" take-2, without any collateral damage to stubdoms.
The actual information we want is in libxl__domain_create_state
(specifically, restore_fd >= -1). I first tried plumbing dcs down, to
avoid stashing the same information in two different structures at
different times.
Sadly, plumbing dcs didn't work because it is common between the real
domain and the stubdom (and this lead to the stubdom getting no settings
at all). What we want to do is only influence the CPUID construction of
the main domain (which may be migrating in), whereas the stubdom always
wants fresh settings.
I could duplicate it into dbs, and at a guess that would probably work,
but isn't it taking a bad problem and making it worse?
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [Xen-devel] [PATCH v2 10/17] tools/libxl: Plumb a restore boolean down into libxl__build_pre()
2020-02-28 18:50 ` Andrew Cooper
@ 2020-03-05 17:07 ` Ian Jackson
0 siblings, 0 replies; 47+ messages in thread
From: Ian Jackson @ 2020-03-05 17:07 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Anthony Perard, Ian Jackson, Wei Liu, Xen-devel
Andrew Cooper writes ("Re: [PATCH v2 10/17] tools/libxl: Plumb a restore boolean down into libxl__build_pre()"):
> On 24/02/2020 17:39, Ian Jackson wrote:
> > Maybe it would be better to pass this via libxl__domain_build_state
> > rather than as an additional parameter ?
>
> Well - I tried a similar approach the first time around, and it broke
> stubdoms so badly it needed reverting.
>
> (Untrim the commit details)
>
> > v2:
> > * New. This is c/s aacc1430064 "tools/libxl: Plumb domain_create_state down
> > into libxl__build_pre()" take-2, without any collateral damage to stubdoms.
>
> The actual information we want is in libxl__domain_create_state
> (specifically, restore_fd >= -1). I first tried plumbing dcs down, to
> avoid stashing the same information in two different structures at
> different times.
>
> Sadly, plumbing dcs didn't work because it is common between the real
> domain and the stubdom (and this lead to the stubdom getting no settings
> at all). What we want to do is only influence the CPUID construction of
> the main domain (which may be migrating in), whereas the stubdom always
> wants fresh settings.
Right. Thanks for the explanation, which make sense to me.
> I could duplicate it into dbs, and at a guess that would probably work,
> but isn't it taking a bad problem and making it worse?
I think that is fine.
Conceptually I think it's like this:
These structs take place of huge lists of parameters. When we create
a service domain, we need to pass a new list of parameters
(_build_state), we also pass some of the original ones
(_create_state). If we are talking about parameters that need to be
different for a service domain, they should be in _build_state; even
if the main domain's version is effectively a copy of something in
_create_state.
So I think adding a "restoring" or "restore" boolean to dbs is
probably right.
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v3 10/17] tools/libxl: Plumb a restore boolean into libxl__domain_build_state
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 10/17] tools/libxl: Plumb a restore boolean down into libxl__build_pre() Andrew Cooper
2020-02-24 17:39 ` Ian Jackson
@ 2020-04-14 20:23 ` Andrew Cooper
2020-04-27 16:02 ` Ian Jackson
1 sibling, 1 reply; 47+ messages in thread
From: Andrew Cooper @ 2020-04-14 20:23 UTC (permalink / raw)
To: Xen-devel; +Cc: Anthony PERARD, Andrew Cooper, Wei Liu, Ian Jackson
To fix CPUID handling, libxl__build_pre() is going to have to distinguish
between a brand new VM vs one which is being migrated-in/resumed.
Transcribe dcs->restore_fd into dbs->restore in initiate_domain_create()
only (specifically avoiding the stubdom state in libxl__spawn_stub_dm()).
While tweaking initiate_domain_create(), make a new dbs alias and simplify
later code, and drop the local restore_fd alias as the new dbs->restore is
more intuitive in context.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <Ian.Jackson@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Anthony PERARD <anthony.perard@citrix.com>
v2:
* New. This is c/s aacc1430064 "tools/libxl: Plumb domain_create_state down
into libxl__build_pre()" take-2, without any collateral damage to stubdoms.
v3:
* Extend libxl__domain_build_state instead of adding a new parameter to
libxl__build_pre().
---
tools/libxl/libxl_create.c | 14 +++++++-------
tools/libxl/libxl_internal.h | 4 ++++
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index e7cb2dbc2b..5a043df15f 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1181,18 +1181,18 @@ static void initiate_domain_create(libxl__egc *egc,
/* convenience aliases */
libxl_domain_config *const d_config = dcs->guest_config;
- const int restore_fd = dcs->restore_fd;
+ libxl__domain_build_state *dbs = &dcs->build_state;
libxl__xswait_init(&dcs->console_xswait);
domid = dcs->domid;
- libxl__domain_build_state_init(&dcs->build_state);
+ libxl__domain_build_state_init(dbs);
+ dbs->restore = dcs->restore_fd >= 0;
ret = libxl__domain_config_setdefault(gc,d_config,domid);
if (ret) goto error_out;
- ret = libxl__domain_make(gc, d_config, &dcs->build_state, &domid,
- dcs->soft_reset);
+ ret = libxl__domain_make(gc, d_config, dbs, &domid, dcs->soft_reset);
if (ret) {
LOGD(ERROR, domid, "cannot make domain: %d", ret);
dcs->guest_domid = domid;
@@ -1236,7 +1236,7 @@ static void initiate_domain_create(libxl__egc *egc,
if (ret)
goto error_out;
- if (restore_fd >= 0 || dcs->soft_reset) {
+ if (dbs->restore || dcs->soft_reset) {
LOGD(DEBUG, domid, "restoring, not running bootloader");
domcreate_bootloader_done(egc, &dcs->bl, 0);
} else {
@@ -1247,8 +1247,8 @@ static void initiate_domain_create(libxl__egc *egc,
dcs->bl.disk = bootdisk;
dcs->bl.domid = dcs->guest_domid;
- dcs->bl.kernel = &dcs->build_state.pv_kernel;
- dcs->bl.ramdisk = &dcs->build_state.pv_ramdisk;
+ dcs->bl.kernel = &dbs->pv_kernel;
+ dcs->bl.ramdisk = &dbs->pv_ramdisk;
libxl__bootloader_run(egc, &dcs->bl);
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 5f39e44cb9..e5effd2ad1 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1397,6 +1397,10 @@ typedef struct {
/* ARM only to deal with broken firmware */
uint32_t clock_frequency;
+
+ /* Whether this domain is being migrated/restored, or booting fresh. Only
+ * applicable to the primary domain, not support domains (e.g. stub QEMU). */
+ bool restore;
} libxl__domain_build_state;
_hidden void libxl__domain_build_state_init(libxl__domain_build_state *s);
--
2.11.0
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH v3 10/17] tools/libxl: Plumb a restore boolean into libxl__domain_build_state
2020-04-14 20:23 ` [PATCH v3 10/17] tools/libxl: Plumb a restore boolean into libxl__domain_build_state Andrew Cooper
@ 2020-04-27 16:02 ` Ian Jackson
0 siblings, 0 replies; 47+ messages in thread
From: Ian Jackson @ 2020-04-27 16:02 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Anthony Perard, Xen-devel, Wei Liu
Andrew Cooper writes ("[PATCH v3 10/17] tools/libxl: Plumb a restore boolean into libxl__domain_build_state"):
> To fix CPUID handling, libxl__build_pre() is going to have to distinguish
> between a brand new VM vs one which is being migrated-in/resumed.
>
> Transcribe dcs->restore_fd into dbs->restore in initiate_domain_create()
> only (specifically avoiding the stubdom state in libxl__spawn_stub_dm()).
>
> While tweaking initiate_domain_create(), make a new dbs alias and simplify
> later code, and drop the local restore_fd alias as the new dbs->restore is
> more intuitive in context.
>
> No functional change.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Xen-devel] [PATCH v2 11/17] tools/libxl: Re-position CPUID handling during domain construction
2020-01-27 14:34 [Xen-devel] [PATCH v2 00/17] Support CPUID/MSR data in migration streams Andrew Cooper
` (9 preceding siblings ...)
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 10/17] tools/libxl: Plumb a restore boolean down into libxl__build_pre() Andrew Cooper
@ 2020-01-27 14:34 ` Andrew Cooper
2020-02-24 16:46 ` [Xen-devel] [PATCH v2.1 " Andrew Cooper
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 12/17] docs/migration: Specify X86_{CPUID, MSR}_POLICY records Andrew Cooper
` (5 subsequent siblings)
16 siblings, 1 reply; 47+ messages in thread
From: Andrew Cooper @ 2020-01-27 14:34 UTC (permalink / raw)
To: Xen-devel; +Cc: Anthony PERARD, Andrew Cooper, Wei Liu, Ian Jackson
CPUID handling needs to be earlier in construction. Move it from its current
position in libxl__build_post() to libxl__build_pre() for fresh builds, and
libxl__srm_callout_callback_static_data_done() for the migration/resume case.
Later changes will make the migration/resume case conditional on whether CPUID
data was present in the migration stream, and the libxc layer took care of
restoring it.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
CC: Ian Jackson <Ian.Jackson@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Anthony PERARD <anthony.perard@citrix.com>
---
tools/libxl/libxl_create.c | 8 +++++++-
tools/libxl/libxl_dom.c | 16 ++++++++++++----
2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 1d2e193509..09f84858d5 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1303,8 +1303,14 @@ int libxl__srm_callout_callback_static_data_done(void *user)
libxl__save_helper_state *shs = user;
libxl__domain_create_state *dcs = shs->caller_state;
STATE_AO_GC(dcs->ao);
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+
+ const libxl_domain_config *d_config = dcs->guest_config;
+ const libxl_domain_build_info *info = &d_config->b_info;
- /* Nothing to do (yet). */
+ libxl__cpuid_apply_policy(ctx, dcs->guest_domid);
+ if (info->cpuid != NULL)
+ libxl__cpuid_set(ctx, dcs->guest_domid, info->cpuid);
return 0;
}
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 1bac277351..5dc8369eda 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -389,6 +389,18 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
rc = libxl__arch_domain_create(gc, d_config, domid);
+ /* Construct a CPUID policy, but only for brand new domains. Domains
+ * being migrated-in/restored have CPUID handled during the
+ * static_data_done() callback. */
+ if (!restore) {
+ /* For x86 at least, libxl_cpuid_apply_policy() takes an implicit
+ * parameter, HVM_PARAM_PAE_ENABLED, which is only set up in
+ * libxl__arch_domain_create(). */
+ libxl_cpuid_apply_policy(ctx, domid);
+ if (info->cpuid != NULL)
+ libxl_cpuid_set(ctx, domid, info->cpuid);
+ }
+
return rc;
}
@@ -456,10 +468,6 @@ int libxl__build_post(libxl__gc *gc, uint32_t domid,
if (rc)
return rc;
- libxl__cpuid_apply_policy(ctx, domid);
- if (info->cpuid != NULL)
- libxl__cpuid_set(ctx, domid, info->cpuid);
-
if (info->type == LIBXL_DOMAIN_TYPE_HVM
&& !libxl_ms_vm_genid_is_zero(&info->u.hvm.ms_vm_genid)) {
rc = libxl__ms_vm_genid_set(gc, domid,
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Xen-devel] [PATCH v2.1 11/17] tools/libxl: Re-position CPUID handling during domain construction
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 11/17] tools/libxl: Re-position CPUID handling during domain construction Andrew Cooper
@ 2020-02-24 16:46 ` Andrew Cooper
0 siblings, 0 replies; 47+ messages in thread
From: Andrew Cooper @ 2020-02-24 16:46 UTC (permalink / raw)
To: Xen-devel; +Cc: Anthony PERARD, Andrew Cooper, Wei Liu, Ian Jackson
CPUID handling needs to be earlier in construction. Move it from its current
position in libxl__build_post() to libxl__build_pre() for fresh builds, and
libxl__srm_callout_callback_static_data_done() for the migration/resume case.
Later changes will make the migration/resume case conditional on whether CPUID
data was present in the migration stream, and the libxc layer took care of
restoring it.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
CC: Ian Jackson <Ian.Jackson@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Anthony PERARD <anthony.perard@citrix.com>
v2.1:
* Rebase over libxl__cpuid_legacy() changes
---
tools/libxl/libxl_create.c | 6 +++++-
tools/libxl/libxl_dom.c | 8 ++++++--
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index cb665a44cc..2c1cbdfb2a 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1313,8 +1313,12 @@ int libxl__srm_callout_callback_static_data_done(void *user)
libxl__save_helper_state *shs = user;
libxl__domain_create_state *dcs = shs->caller_state;
STATE_AO_GC(dcs->ao);
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+
+ libxl_domain_config *d_config = dcs->guest_config;
+ libxl_domain_build_info *info = &d_config->b_info;
- /* Nothing to do (yet). */
+ libxl__cpuid_legacy(ctx, dcs->guest_domid, info);
return 0;
}
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index d3731e5b8f..1da23de5b9 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -389,6 +389,12 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
rc = libxl__arch_domain_create(gc, d_config, domid);
+ /* Construct a CPUID policy, but only for brand new domains. Domains
+ * being migrated-in/restored have CPUID handled during the
+ * static_data_done() callback. */
+ if (!restore)
+ libxl__cpuid_legacy(ctx, domid, info);
+
return rc;
}
@@ -456,8 +462,6 @@ int libxl__build_post(libxl__gc *gc, uint32_t domid,
if (rc)
return rc;
- libxl__cpuid_legacy(ctx, domid, info);
-
if (info->type == LIBXL_DOMAIN_TYPE_HVM
&& !libxl_ms_vm_genid_is_zero(&info->u.hvm.ms_vm_genid)) {
rc = libxl__ms_vm_genid_set(gc, domid,
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [Xen-devel] [PATCH v2 12/17] docs/migration: Specify X86_{CPUID, MSR}_POLICY records
2020-01-27 14:34 [Xen-devel] [PATCH v2 00/17] Support CPUID/MSR data in migration streams Andrew Cooper
` (10 preceding siblings ...)
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 11/17] tools/libxl: Re-position CPUID handling during domain construction Andrew Cooper
@ 2020-01-27 14:34 ` Andrew Cooper
2020-02-24 17:41 ` Ian Jackson
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 13/17] libxc/restore: Handle X86_{CPUID, MSR}_DATA records Andrew Cooper
` (4 subsequent siblings)
16 siblings, 1 reply; 47+ messages in thread
From: Andrew Cooper @ 2020-01-27 14:34 UTC (permalink / raw)
To: Xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Tim Deegan,
Marek Marczykowski-Górecki, Julien Grall, Ian Jackson
These two records move blobs from the XEN_DOMCTL_{get,set}_cpu_policy
hypercall.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Jan Beulich <JBeulich@suse.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Tim Deegan <tim@xen.org>
CC: Wei Liu <wl@xen.org>
CC: Julien Grall <julien.grall@arm.com>
CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
docs/specs/libxc-migration-stream.pandoc | 42 +++++++++++++++++++++++++++++++
tools/libxc/xc_sr_common.c | 2 ++
tools/libxc/xc_sr_stream_format.h | 2 ++
tools/python/xen/migration/libxc.py | 43 ++++++++++++++++++++++++++++++++
4 files changed, 89 insertions(+)
diff --git a/docs/specs/libxc-migration-stream.pandoc b/docs/specs/libxc-migration-stream.pandoc
index 22ff306e0b..3a0915b795 100644
--- a/docs/specs/libxc-migration-stream.pandoc
+++ b/docs/specs/libxc-migration-stream.pandoc
@@ -634,6 +634,46 @@ The end record contains no fields; its body_length is 0.
\clearpage
+X86_CPUID_POLICY
+----------------
+
+CPUID policy content, as accessed by the XEN_DOMCTL_{get,set}_cpu_policy
+hypercall sub-ops.
+
+ 0 1 2 3 4 5 6 7 octet
+ +-------------------------------------------------+
+ | CPUID_policy |
+ ...
+ +-------------------------------------------------+
+
+--------------------------------------------------------------------
+Field Description
+------------ ---------------------------------------------------
+CPUID_policy Array of xen_cpuid_leaf_t[]'s
+--------------------------------------------------------------------
+
+\clearpage
+
+X86_MSR_POLICY
+--------------
+
+MSR policy content, as accessed by the XEN_DOMCTL_{get,set}_cpu_policy
+hypercall sub-ops.
+
+ 0 1 2 3 4 5 6 7 octet
+ +-------------------------------------------------+
+ | MSR_policy |
+ ...
+ +-------------------------------------------------+
+
+--------------------------------------------------------------------
+Field Description
+---------- ---------------------------------------------------
+MSR_policy Array of xen_msr_entry_t[]'s
+--------------------------------------------------------------------
+
+\clearpage
+
Layout
======
@@ -656,6 +696,7 @@ A typical save record for an x86 PV guest image would look like:
* Domain header
* Static data records:
* X86_PV_INFO record
+ * X86_{CPUID,MSR}_POLICY
* STATIC_DATA_END
* X86_PV_P2M_FRAMES record
* Many PAGE_DATA records
@@ -685,6 +726,7 @@ A typical save record for an x86 HVM guest image would look like:
* Image header
* Domain header
* Static data records:
+ * X86_{CPUID,MSR}_POLICY
* STATIC_DATA_END
* Many PAGE_DATA records
* X86_TSC_INFO
diff --git a/tools/libxc/xc_sr_common.c b/tools/libxc/xc_sr_common.c
index 7f22cf0365..7c54b03414 100644
--- a/tools/libxc/xc_sr_common.c
+++ b/tools/libxc/xc_sr_common.c
@@ -37,6 +37,8 @@ static const char *const mandatory_rec_types[] =
[REC_TYPE_CHECKPOINT] = "Checkpoint",
[REC_TYPE_CHECKPOINT_DIRTY_PFN_LIST] = "Checkpoint dirty pfn list",
[REC_TYPE_STATIC_DATA_END] = "Static data end",
+ [REC_TYPE_X86_CPUID_POLICY] = "x86 CPUID policy",
+ [REC_TYPE_X86_MSR_POLICY] = "x86 MSR policy",
};
const char *rec_type_to_str(uint32_t type)
diff --git a/tools/libxc/xc_sr_stream_format.h b/tools/libxc/xc_sr_stream_format.h
index 81c9765b0a..8a0da26f75 100644
--- a/tools/libxc/xc_sr_stream_format.h
+++ b/tools/libxc/xc_sr_stream_format.h
@@ -74,6 +74,8 @@ struct xc_sr_rhdr
#define REC_TYPE_CHECKPOINT 0x0000000eU
#define REC_TYPE_CHECKPOINT_DIRTY_PFN_LIST 0x0000000fU
#define REC_TYPE_STATIC_DATA_END 0x00000010U
+#define REC_TYPE_X86_CPUID_POLICY 0x00000011U
+#define REC_TYPE_X86_MSR_POLICY 0x00000012U
#define REC_TYPE_OPTIONAL 0x80000000U
diff --git a/tools/python/xen/migration/libxc.py b/tools/python/xen/migration/libxc.py
index 5fb51b56ac..9881f5ced4 100644
--- a/tools/python/xen/migration/libxc.py
+++ b/tools/python/xen/migration/libxc.py
@@ -57,6 +57,8 @@
REC_TYPE_checkpoint = 0x0000000e
REC_TYPE_checkpoint_dirty_pfn_list = 0x0000000f
REC_TYPE_static_data_end = 0x00000010
+REC_TYPE_x86_cpuid_policy = 0x00000011
+REC_TYPE_x86_msr_policy = 0x00000012
rec_type_to_str = {
REC_TYPE_end : "End",
@@ -76,6 +78,8 @@
REC_TYPE_checkpoint : "Checkpoint",
REC_TYPE_checkpoint_dirty_pfn_list : "Checkpoint dirty pfn list",
REC_TYPE_static_data_end : "Static data end",
+ REC_TYPE_x86_cpuid_policy : "x86 CPUID policy",
+ REC_TYPE_x86_msr_policy : "x86 MSR policy",
}
# page_data
@@ -113,6 +117,12 @@
HVM_PARAMS_ENTRY_FORMAT = "QQ"
HVM_PARAMS_FORMAT = "II"
+# x86_cpuid_policy => xen_cpuid_leaf_t[]
+X86_CPUID_POLICY_FORMAT = "IIIIII"
+
+# x86_msr_policy => xen_msr_entry_t[]
+X86_MSR_POLICY_FORMAT = "QII"
+
class VerifyLibxc(VerifyBase):
""" Verify a Libxc v2 (or later) stream """
@@ -439,6 +449,34 @@ def verify_record_static_data_end(self, content):
raise RecordError("Static data end record found in v2 stream")
+ def verify_record_x86_cpuid_policy(self, content):
+ """ x86 CPUID policy record """
+
+ if self.version < 3:
+ raise RecordError("x86 CPUID policy record found in v2 stream")
+
+ sz = calcsize(X86_CPUID_POLICY_FORMAT)
+ contentsz = len(content)
+
+ if contentsz < sz or (contentsz % sz) != 0:
+ raise RecordError("Record length %u, expected multiple of %u" %
+ (contentsz, sz))
+
+
+ def verify_record_x86_msr_policy(self, content):
+ """ x86 MSR policy record """
+
+ if self.version < 3:
+ raise RecordError("x86 MSR policy record found in v2 stream")
+
+ sz = calcsize(X86_MSR_POLICY_FORMAT)
+ contentsz = len(content)
+
+ if contentsz < sz or (contentsz % sz) != 0:
+ raise RecordError("Record length %u, expected multiple of %u" %
+ (contentsz, sz))
+
+
record_verifiers = {
REC_TYPE_end:
VerifyLibxc.verify_record_end,
@@ -483,4 +521,9 @@ def verify_record_static_data_end(self, content):
REC_TYPE_static_data_end:
VerifyLibxc.verify_record_static_data_end,
+
+ REC_TYPE_x86_cpuid_policy:
+ VerifyLibxc.verify_record_x86_cpuid_policy,
+ REC_TYPE_x86_msr_policy:
+ VerifyLibxc.verify_record_x86_msr_policy,
}
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Xen-devel] [PATCH v2 12/17] docs/migration: Specify X86_{CPUID, MSR}_POLICY records
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 12/17] docs/migration: Specify X86_{CPUID, MSR}_POLICY records Andrew Cooper
@ 2020-02-24 17:41 ` Ian Jackson
0 siblings, 0 replies; 47+ messages in thread
From: Ian Jackson @ 2020-02-24 17:41 UTC (permalink / raw)
To: Andrew Cooper
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk, Tim (Xen.org),
George Dunlap, Marek Marczykowski-Górecki, Julien Grall,
Jan Beulich, Xen-devel
Andrew Cooper writes ("[PATCH v2 12/17] docs/migration: Specify X86_{CPUID,MSR}_POLICY records"):
> These two records move blobs from the XEN_DOMCTL_{get,set}_cpu_policy
> hypercall.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Xen-devel] [PATCH v2 13/17] libxc/restore: Handle X86_{CPUID, MSR}_DATA records
2020-01-27 14:34 [Xen-devel] [PATCH v2 00/17] Support CPUID/MSR data in migration streams Andrew Cooper
` (11 preceding siblings ...)
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 12/17] docs/migration: Specify X86_{CPUID, MSR}_POLICY records Andrew Cooper
@ 2020-01-27 14:34 ` Andrew Cooper
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 14/17] libxc/save: Write " Andrew Cooper
` (3 subsequent siblings)
16 siblings, 0 replies; 47+ messages in thread
From: Andrew Cooper @ 2020-01-27 14:34 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Ian Jackson
For now, the data are just stashed, and discarded at the end.
A future change will restore the data, once libxl has been adjusted to avoid
clobbering the data.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
CC: Ian Jackson <Ian.Jackson@citrix.com>
CC: Wei Liu <wl@xen.org>
---
tools/libxc/xc_sr_common.h | 10 ++++++++++
tools/libxc/xc_sr_common_x86.c | 40 +++++++++++++++++++++++++++++++++++++
tools/libxc/xc_sr_common_x86.h | 14 +++++++++++++
tools/libxc/xc_sr_restore_x86_hvm.c | 9 +++++++++
tools/libxc/xc_sr_restore_x86_pv.c | 9 +++++++++
5 files changed, 82 insertions(+)
diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
index fd7fb67305..7742260690 100644
--- a/tools/libxc/xc_sr_common.h
+++ b/tools/libxc/xc_sr_common.h
@@ -296,6 +296,16 @@ struct xc_sr_context
{
struct /* x86 */
{
+ /* Common save/restore data. */
+ union
+ {
+ struct
+ {
+ /* X86_{CPUID,MSR}_DATA blobs for CPU Policy. */
+ struct xc_sr_blob cpuid, msr;
+ } restore;
+ };
+
struct /* x86 PV guest. */
{
/* 4 or 8; 32 or 64 bit domain */
diff --git a/tools/libxc/xc_sr_common_x86.c b/tools/libxc/xc_sr_common_x86.c
index 011684df97..8980299e9a 100644
--- a/tools/libxc/xc_sr_common_x86.c
+++ b/tools/libxc/xc_sr_common_x86.c
@@ -42,6 +42,46 @@ int handle_x86_tsc_info(struct xc_sr_context *ctx, struct xc_sr_record *rec)
return 0;
}
+int handle_x86_cpuid_policy(struct xc_sr_context *ctx, struct xc_sr_record *rec)
+{
+ xc_interface *xch = ctx->xch;
+ int rc;
+
+ if ( rec->length == 0 ||
+ rec->length % sizeof(xen_cpuid_leaf_t) != 0 )
+ {
+ ERROR("X86_CPUID_POLICY size %u should be multiple of %zu",
+ rec->length, sizeof(xen_cpuid_leaf_t));
+ return -1;
+ }
+
+ rc = update_blob(&ctx->x86.restore.cpuid, rec->data, rec->length);
+ if ( rc )
+ ERROR("Unable to allocate %u bytes for X86_CPUID_POLICY", rec->length);
+
+ return rc;
+}
+
+int handle_x86_msr_policy(struct xc_sr_context *ctx, struct xc_sr_record *rec)
+{
+ xc_interface *xch = ctx->xch;
+ int rc;
+
+ if ( rec->length == 0 ||
+ rec->length % sizeof(xen_msr_entry_t) != 0 )
+ {
+ ERROR("X86_MSR_POLICY size %u should be multiple of %zu",
+ rec->length, sizeof(xen_cpuid_leaf_t));
+ return -1;
+ }
+
+ rc = update_blob(&ctx->x86.restore.msr, rec->data, rec->length);
+ if ( rc )
+ ERROR("Unable to allocate %u bytes for X86_MSR_POLICY", rec->length);
+
+ return rc;
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/libxc/xc_sr_common_x86.h b/tools/libxc/xc_sr_common_x86.h
index ebc4355bd1..c458c1aa37 100644
--- a/tools/libxc/xc_sr_common_x86.h
+++ b/tools/libxc/xc_sr_common_x86.h
@@ -14,6 +14,20 @@ int write_x86_tsc_info(struct xc_sr_context *ctx);
*/
int handle_x86_tsc_info(struct xc_sr_context *ctx, struct xc_sr_record *rec);
+/*
+ * Parses an X86_CPUID_POLICY record and stashes the content for application
+ * when a STATIC_DATA_END record is encountered.
+ */
+int handle_x86_cpuid_policy(struct xc_sr_context *ctx,
+ struct xc_sr_record *rec);
+
+/*
+ * Parses an X86_MSR_POLICY record and stashes the content for application
+ * when a STATIC_DATA_END record is encountered.
+ */
+int handle_x86_msr_policy(struct xc_sr_context *ctx,
+ struct xc_sr_record *rec);
+
#endif
/*
* Local variables:
diff --git a/tools/libxc/xc_sr_restore_x86_hvm.c b/tools/libxc/xc_sr_restore_x86_hvm.c
index 3f78248f32..cef99e0397 100644
--- a/tools/libxc/xc_sr_restore_x86_hvm.c
+++ b/tools/libxc/xc_sr_restore_x86_hvm.c
@@ -171,6 +171,12 @@ static int x86_hvm_process_record(struct xc_sr_context *ctx,
case REC_TYPE_HVM_PARAMS:
return handle_hvm_params(ctx, rec);
+ case REC_TYPE_X86_CPUID_POLICY:
+ return handle_x86_cpuid_policy(ctx, rec);
+
+ case REC_TYPE_X86_MSR_POLICY:
+ return handle_x86_msr_policy(ctx, rec);
+
default:
return RECORD_NOT_PROCESSED;
}
@@ -227,6 +233,9 @@ static int x86_hvm_cleanup(struct xc_sr_context *ctx)
{
free(ctx->x86.hvm.restore.context.ptr);
+ free(ctx->x86.restore.cpuid.ptr);
+ free(ctx->x86.restore.msr.ptr);
+
return 0;
}
diff --git a/tools/libxc/xc_sr_restore_x86_pv.c b/tools/libxc/xc_sr_restore_x86_pv.c
index 3756225be6..3aac4bd502 100644
--- a/tools/libxc/xc_sr_restore_x86_pv.c
+++ b/tools/libxc/xc_sr_restore_x86_pv.c
@@ -1102,6 +1102,12 @@ static int x86_pv_process_record(struct xc_sr_context *ctx,
case REC_TYPE_X86_TSC_INFO:
return handle_x86_tsc_info(ctx, rec);
+ case REC_TYPE_X86_CPUID_POLICY:
+ return handle_x86_cpuid_policy(ctx, rec);
+
+ case REC_TYPE_X86_MSR_POLICY:
+ return handle_x86_msr_policy(ctx, rec);
+
default:
return RECORD_NOT_PROCESSED;
}
@@ -1173,6 +1179,9 @@ static int x86_pv_cleanup(struct xc_sr_context *ctx)
if ( ctx->x86.pv.m2p )
munmap(ctx->x86.pv.m2p, ctx->x86.pv.nr_m2p_frames * PAGE_SIZE);
+ free(ctx->x86.restore.cpuid.ptr);
+ free(ctx->x86.restore.msr.ptr);
+
return 0;
}
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Xen-devel] [PATCH v2 14/17] libxc/save: Write X86_{CPUID, MSR}_DATA records
2020-01-27 14:34 [Xen-devel] [PATCH v2 00/17] Support CPUID/MSR data in migration streams Andrew Cooper
` (12 preceding siblings ...)
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 13/17] libxc/restore: Handle X86_{CPUID, MSR}_DATA records Andrew Cooper
@ 2020-01-27 14:34 ` Andrew Cooper
2020-04-14 20:24 ` Ping [PATCH v2 14/17] libxc/save: Write X86_{CPUID,MSR}_DATA records Andrew Cooper
2020-04-27 16:12 ` Ian Jackson
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 15/17] tools/libx[cl]: Plumb 'missing' through static_data_done() up into libxl Andrew Cooper
` (2 subsequent siblings)
16 siblings, 2 replies; 47+ messages in thread
From: Andrew Cooper @ 2020-01-27 14:34 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Ian Jackson
With all other plumbing in place, obtain the CPU Policy from Xen and
write it into the migration stream.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <Ian.Jackson@citrix.com>
CC: Wei Liu <wl@xen.org>
---
tools/libxc/xc_sr_common_x86.c | 50 ++++++++++++++++++++++++++++++++++++++++
tools/libxc/xc_sr_common_x86.h | 6 +++++
tools/libxc/xc_sr_save_x86_hvm.c | 2 +-
tools/libxc/xc_sr_save_x86_pv.c | 12 +++++++++-
4 files changed, 68 insertions(+), 2 deletions(-)
diff --git a/tools/libxc/xc_sr_common_x86.c b/tools/libxc/xc_sr_common_x86.c
index 8980299e9a..6267655dab 100644
--- a/tools/libxc/xc_sr_common_x86.c
+++ b/tools/libxc/xc_sr_common_x86.c
@@ -42,6 +42,56 @@ int handle_x86_tsc_info(struct xc_sr_context *ctx, struct xc_sr_record *rec)
return 0;
}
+int write_x86_cpu_policy_records(struct xc_sr_context *ctx)
+{
+ xc_interface *xch = ctx->xch;
+ struct xc_sr_record cpuid = { .type = REC_TYPE_X86_CPUID_POLICY, };
+ struct xc_sr_record msrs = { .type = REC_TYPE_X86_MSR_POLICY, };
+ uint32_t nr_leaves = 0, nr_msrs = 0;
+ int rc;
+
+ if ( xc_get_cpu_policy_size(xch, &nr_leaves, &nr_msrs) < 0 )
+ {
+ PERROR("Unable to get CPU Policy size");
+ return -1;
+ }
+
+ cpuid.data = malloc(nr_leaves * sizeof(xen_cpuid_leaf_t));
+ msrs.data = malloc(nr_msrs * sizeof(xen_msr_entry_t));
+ if ( !cpuid.data || !msrs.data )
+ {
+ ERROR("Cannot allocate memory for CPU Policy");
+ rc = -1;
+ goto out;
+ }
+
+ if ( xc_get_domain_cpu_policy(xch, ctx->domid, &nr_leaves, cpuid.data,
+ &nr_msrs, msrs.data) )
+ {
+ PERROR("Unable to get d%d CPU Policy", ctx->domid);
+ rc = -1;
+ goto out;
+ }
+
+ cpuid.length = nr_leaves * sizeof(xen_cpuid_leaf_t);
+ if ( cpuid.length )
+ {
+ rc = write_record(ctx, &cpuid);
+ if ( rc )
+ goto out;
+ }
+
+ msrs.length = nr_msrs * sizeof(xen_msr_entry_t);
+ if ( msrs.length )
+ rc = write_record(ctx, &msrs);
+
+ out:
+ free(cpuid.data);
+ free(msrs.data);
+
+ return rc;
+}
+
int handle_x86_cpuid_policy(struct xc_sr_context *ctx, struct xc_sr_record *rec)
{
xc_interface *xch = ctx->xch;
diff --git a/tools/libxc/xc_sr_common_x86.h b/tools/libxc/xc_sr_common_x86.h
index c458c1aa37..d1050981dd 100644
--- a/tools/libxc/xc_sr_common_x86.h
+++ b/tools/libxc/xc_sr_common_x86.h
@@ -15,6 +15,12 @@ int write_x86_tsc_info(struct xc_sr_context *ctx);
int handle_x86_tsc_info(struct xc_sr_context *ctx, struct xc_sr_record *rec);
/*
+ * Obtains a domains CPU Policy from Xen, and writes X86_{CPUID,MSR}_POLICY
+ * records into the stream.
+ */
+int write_x86_cpu_policy_records(struct xc_sr_context *ctx);
+
+/*
* Parses an X86_CPUID_POLICY record and stashes the content for application
* when a STATIC_DATA_END record is encountered.
*/
diff --git a/tools/libxc/xc_sr_save_x86_hvm.c b/tools/libxc/xc_sr_save_x86_hvm.c
index 93bcc1c273..acf9264dec 100644
--- a/tools/libxc/xc_sr_save_x86_hvm.c
+++ b/tools/libxc/xc_sr_save_x86_hvm.c
@@ -172,7 +172,7 @@ static int x86_hvm_setup(struct xc_sr_context *ctx)
static int x86_hvm_static_data(struct xc_sr_context *ctx)
{
- return 0;
+ return write_x86_cpu_policy_records(ctx);
}
static int x86_hvm_start_of_stream(struct xc_sr_context *ctx)
diff --git a/tools/libxc/xc_sr_save_x86_pv.c b/tools/libxc/xc_sr_save_x86_pv.c
index 46019d962d..c7e246ef4f 100644
--- a/tools/libxc/xc_sr_save_x86_pv.c
+++ b/tools/libxc/xc_sr_save_x86_pv.c
@@ -1054,7 +1054,17 @@ static int x86_pv_setup(struct xc_sr_context *ctx)
static int x86_pv_static_data(struct xc_sr_context *ctx)
{
- return write_x86_pv_info(ctx);
+ int rc;
+
+ rc = write_x86_pv_info(ctx);
+ if ( rc )
+ return rc;
+
+ rc = write_x86_cpu_policy_records(ctx);
+ if ( rc )
+ return rc;
+
+ return 0;
}
static int x86_pv_start_of_stream(struct xc_sr_context *ctx)
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: Ping [PATCH v2 14/17] libxc/save: Write X86_{CPUID,MSR}_DATA records
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 14/17] libxc/save: Write " Andrew Cooper
@ 2020-04-14 20:24 ` Andrew Cooper
2020-04-27 16:12 ` Ian Jackson
1 sibling, 0 replies; 47+ messages in thread
From: Andrew Cooper @ 2020-04-14 20:24 UTC (permalink / raw)
To: Xen-devel; +Cc: Ian Jackson, Wei Liu
On 27/01/2020 14:34, Andrew Cooper wrote:
> With all other plumbing in place, obtain the CPU Policy from Xen and
> write it into the migration stream.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Ian Jackson <Ian.Jackson@citrix.com>
> CC: Wei Liu <wl@xen.org>
> ---
> tools/libxc/xc_sr_common_x86.c | 50 ++++++++++++++++++++++++++++++++++++++++
> tools/libxc/xc_sr_common_x86.h | 6 +++++
> tools/libxc/xc_sr_save_x86_hvm.c | 2 +-
> tools/libxc/xc_sr_save_x86_pv.c | 12 +++++++++-
> 4 files changed, 68 insertions(+), 2 deletions(-)
>
> diff --git a/tools/libxc/xc_sr_common_x86.c b/tools/libxc/xc_sr_common_x86.c
> index 8980299e9a..6267655dab 100644
> --- a/tools/libxc/xc_sr_common_x86.c
> +++ b/tools/libxc/xc_sr_common_x86.c
> @@ -42,6 +42,56 @@ int handle_x86_tsc_info(struct xc_sr_context *ctx, struct xc_sr_record *rec)
> return 0;
> }
>
> +int write_x86_cpu_policy_records(struct xc_sr_context *ctx)
> +{
> + xc_interface *xch = ctx->xch;
> + struct xc_sr_record cpuid = { .type = REC_TYPE_X86_CPUID_POLICY, };
> + struct xc_sr_record msrs = { .type = REC_TYPE_X86_MSR_POLICY, };
> + uint32_t nr_leaves = 0, nr_msrs = 0;
> + int rc;
> +
> + if ( xc_get_cpu_policy_size(xch, &nr_leaves, &nr_msrs) < 0 )
> + {
> + PERROR("Unable to get CPU Policy size");
> + return -1;
> + }
> +
> + cpuid.data = malloc(nr_leaves * sizeof(xen_cpuid_leaf_t));
> + msrs.data = malloc(nr_msrs * sizeof(xen_msr_entry_t));
> + if ( !cpuid.data || !msrs.data )
> + {
> + ERROR("Cannot allocate memory for CPU Policy");
> + rc = -1;
> + goto out;
> + }
> +
> + if ( xc_get_domain_cpu_policy(xch, ctx->domid, &nr_leaves, cpuid.data,
> + &nr_msrs, msrs.data) )
> + {
> + PERROR("Unable to get d%d CPU Policy", ctx->domid);
> + rc = -1;
> + goto out;
> + }
> +
> + cpuid.length = nr_leaves * sizeof(xen_cpuid_leaf_t);
> + if ( cpuid.length )
> + {
> + rc = write_record(ctx, &cpuid);
> + if ( rc )
> + goto out;
> + }
> +
> + msrs.length = nr_msrs * sizeof(xen_msr_entry_t);
> + if ( msrs.length )
> + rc = write_record(ctx, &msrs);
> +
> + out:
> + free(cpuid.data);
> + free(msrs.data);
> +
> + return rc;
> +}
> +
> int handle_x86_cpuid_policy(struct xc_sr_context *ctx, struct xc_sr_record *rec)
> {
> xc_interface *xch = ctx->xch;
> diff --git a/tools/libxc/xc_sr_common_x86.h b/tools/libxc/xc_sr_common_x86.h
> index c458c1aa37..d1050981dd 100644
> --- a/tools/libxc/xc_sr_common_x86.h
> +++ b/tools/libxc/xc_sr_common_x86.h
> @@ -15,6 +15,12 @@ int write_x86_tsc_info(struct xc_sr_context *ctx);
> int handle_x86_tsc_info(struct xc_sr_context *ctx, struct xc_sr_record *rec);
>
> /*
> + * Obtains a domains CPU Policy from Xen, and writes X86_{CPUID,MSR}_POLICY
> + * records into the stream.
> + */
> +int write_x86_cpu_policy_records(struct xc_sr_context *ctx);
> +
> +/*
> * Parses an X86_CPUID_POLICY record and stashes the content for application
> * when a STATIC_DATA_END record is encountered.
> */
> diff --git a/tools/libxc/xc_sr_save_x86_hvm.c b/tools/libxc/xc_sr_save_x86_hvm.c
> index 93bcc1c273..acf9264dec 100644
> --- a/tools/libxc/xc_sr_save_x86_hvm.c
> +++ b/tools/libxc/xc_sr_save_x86_hvm.c
> @@ -172,7 +172,7 @@ static int x86_hvm_setup(struct xc_sr_context *ctx)
>
> static int x86_hvm_static_data(struct xc_sr_context *ctx)
> {
> - return 0;
> + return write_x86_cpu_policy_records(ctx);
> }
>
> static int x86_hvm_start_of_stream(struct xc_sr_context *ctx)
> diff --git a/tools/libxc/xc_sr_save_x86_pv.c b/tools/libxc/xc_sr_save_x86_pv.c
> index 46019d962d..c7e246ef4f 100644
> --- a/tools/libxc/xc_sr_save_x86_pv.c
> +++ b/tools/libxc/xc_sr_save_x86_pv.c
> @@ -1054,7 +1054,17 @@ static int x86_pv_setup(struct xc_sr_context *ctx)
>
> static int x86_pv_static_data(struct xc_sr_context *ctx)
> {
> - return write_x86_pv_info(ctx);
> + int rc;
> +
> + rc = write_x86_pv_info(ctx);
> + if ( rc )
> + return rc;
> +
> + rc = write_x86_cpu_policy_records(ctx);
> + if ( rc )
> + return rc;
> +
> + return 0;
> }
>
> static int x86_pv_start_of_stream(struct xc_sr_context *ctx)
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v2 14/17] libxc/save: Write X86_{CPUID,MSR}_DATA records
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 14/17] libxc/save: Write " Andrew Cooper
2020-04-14 20:24 ` Ping [PATCH v2 14/17] libxc/save: Write X86_{CPUID,MSR}_DATA records Andrew Cooper
@ 2020-04-27 16:12 ` Ian Jackson
1 sibling, 0 replies; 47+ messages in thread
From: Ian Jackson @ 2020-04-27 16:12 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Wei Liu
Andrew Cooper writes ("[PATCH v2 14/17] libxc/save: Write X86_{CPUID,MSR}_DATA records"):
> With all other plumbing in place, obtain the CPU Policy from Xen and
> write it into the migration stream.
Thanks for your earlier explanation in the thread:
In all cases with migration development, the receive side logic
(previous patch) has to come before the save side logic (this patch), or
the result will break bisection with the receive side choking on an
unknown record type.
From the "whole series" point of view, compatibility is also the
destination side discarding the data because libxl still needs its order
of CPUID handling shuffling to cope.
I still would have some comments about the compatibility implications
*in the commit message*; maybe an abbreviated version the text above.
Nevertheless,
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Xen-devel] [PATCH v2 15/17] tools/libx[cl]: Plumb 'missing' through static_data_done() up into libxl
2020-01-27 14:34 [Xen-devel] [PATCH v2 00/17] Support CPUID/MSR data in migration streams Andrew Cooper
` (13 preceding siblings ...)
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 14/17] libxc/save: Write " Andrew Cooper
@ 2020-01-27 14:34 ` Andrew Cooper
2020-02-24 16:50 ` [Xen-devel] [PATCH v2.1 " Andrew Cooper
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 16/17] tools/libxc: Restore CPUID/MSR data found in the migration stream Andrew Cooper
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 17/17] docs/xl.cfg: Rewrite cpuid= section Andrew Cooper
16 siblings, 1 reply; 47+ messages in thread
From: Andrew Cooper @ 2020-01-27 14:34 UTC (permalink / raw)
To: Xen-devel; +Cc: Anthony PERARD, Andrew Cooper, Wei Liu, Ian Jackson
Pre Xen-4.14 streams will not contain any CPUID/MSR information. There is
nothing libxc can do about this, and will have to rely on the higher level
toolstack to provide backwards compatibility.
To facilitate this, extend the static_data_done() callback, highlighting the
missing information, and modify libxl to use it. At the libxc level, this
requires an arch-specific hook which, for now, always reports CPUID and MSR as
missing. This will be adjusted in a later change.
No overall functional change - this is just plumbing.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <Ian.Jackson@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Anthony PERARD <anthony.perard@citrix.com>
v2:
* Split/rearrange from v1
* Don't re-evalute 'k' on migrate
---
tools/libxc/include/xenguest.h | 12 ++++++++++--
tools/libxc/xc_sr_common.h | 9 +++++++++
tools/libxc/xc_sr_common_x86.c | 8 ++++++++
tools/libxc/xc_sr_common_x86.h | 5 +++++
tools/libxc/xc_sr_restore.c | 7 ++++++-
tools/libxc/xc_sr_restore_x86_hvm.c | 1 +
tools/libxc/xc_sr_restore_x86_pv.c | 1 +
tools/libxl/libxl_create.c | 18 ++++++++++++++----
tools/libxl/libxl_save_msgs_gen.pl | 2 +-
9 files changed, 55 insertions(+), 8 deletions(-)
diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h
index b4df8d0ffe..7a12d21ff2 100644
--- a/tools/libxc/include/xenguest.h
+++ b/tools/libxc/include/xenguest.h
@@ -139,8 +139,16 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom,
/* callbacks provided by xc_domain_restore */
struct restore_callbacks {
- /* Called once the STATIC_DATA_END record has been received/inferred. */
- int (*static_data_done)(void *data);
+ /*
+ * Called once the STATIC_DATA_END record has been received/inferred.
+ *
+ * For compatibility with older streams, provides a list of static data
+ * expected to be found in the stream, which was missing. A higher level
+ * toolstack is responsible for providing any necessary compatibiltiy.
+ */
+#define XGR_SDD_MISSING_CPUID (1 << 0)
+#define XGR_SDD_MISSING_MSR (1 << 1)
+ int (*static_data_done)(unsigned int missing, void *data);
/* Called after a new checkpoint to suspend the guest. */
int (*suspend)(void *data);
diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
index 7742260690..f3bdea8006 100644
--- a/tools/libxc/xc_sr_common.h
+++ b/tools/libxc/xc_sr_common.h
@@ -159,6 +159,15 @@ struct xc_sr_restore_ops
int (*process_record)(struct xc_sr_context *ctx, struct xc_sr_record *rec);
/**
+ * Perform any actions required after the static data has arrived. Called
+ * when the STATIC_DATA_COMPLETE record has been recieved/inferred.
+ * 'missing' should be filled in for any data item the higher level
+ * toolstack needs to provide compatiblity for.
+ */
+ int (*static_data_complete)(struct xc_sr_context *ctx,
+ unsigned int *missing);
+
+ /**
* Perform any actions required after the stream has been finished. Called
* after the END record has been received.
*/
diff --git a/tools/libxc/xc_sr_common_x86.c b/tools/libxc/xc_sr_common_x86.c
index 6267655dab..a849891634 100644
--- a/tools/libxc/xc_sr_common_x86.c
+++ b/tools/libxc/xc_sr_common_x86.c
@@ -132,6 +132,14 @@ int handle_x86_msr_policy(struct xc_sr_context *ctx, struct xc_sr_record *rec)
return rc;
}
+int x86_static_data_complete(struct xc_sr_context *ctx, unsigned int *missing)
+{
+ /* TODO: Become conditional on there being no data in the stream. */
+ *missing = XGR_SDD_MISSING_MSR | XGR_SDD_MISSING_CPUID;
+
+ return 0;
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/libxc/xc_sr_common_x86.h b/tools/libxc/xc_sr_common_x86.h
index d1050981dd..e08d81e0e7 100644
--- a/tools/libxc/xc_sr_common_x86.h
+++ b/tools/libxc/xc_sr_common_x86.h
@@ -34,6 +34,11 @@ int handle_x86_cpuid_policy(struct xc_sr_context *ctx,
int handle_x86_msr_policy(struct xc_sr_context *ctx,
struct xc_sr_record *rec);
+/*
+ * Perform common x86 actions required after the static data has arrived.
+ */
+int x86_static_data_complete(struct xc_sr_context *ctx, unsigned int *missing);
+
#endif
/*
* Local variables:
diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c
index bb94cd879d..bc811e6e3a 100644
--- a/tools/libxc/xc_sr_restore.c
+++ b/tools/libxc/xc_sr_restore.c
@@ -659,6 +659,7 @@ static int buffer_record(struct xc_sr_context *ctx, struct xc_sr_record *rec)
int handle_static_data_end(struct xc_sr_context *ctx)
{
xc_interface *xch = ctx->xch;
+ unsigned int missing = 0;
int rc = 0;
if ( ctx->restore.seen_static_data_end )
@@ -669,9 +670,13 @@ int handle_static_data_end(struct xc_sr_context *ctx)
ctx->restore.seen_static_data_end = true;
+ rc = ctx->restore.ops.static_data_complete(ctx, &missing);
+ if ( rc )
+ return rc;
+
if ( ctx->restore.callbacks->static_data_done &&
(rc = ctx->restore.callbacks->static_data_done(
- ctx->restore.callbacks->data) != 0) )
+ missing, ctx->restore.callbacks->data) != 0) )
ERROR("static_data_done() callback failed: %d\n", rc);
return rc;
diff --git a/tools/libxc/xc_sr_restore_x86_hvm.c b/tools/libxc/xc_sr_restore_x86_hvm.c
index cef99e0397..9190edaee7 100644
--- a/tools/libxc/xc_sr_restore_x86_hvm.c
+++ b/tools/libxc/xc_sr_restore_x86_hvm.c
@@ -248,6 +248,7 @@ struct xc_sr_restore_ops restore_ops_x86_hvm =
.localise_page = x86_hvm_localise_page,
.setup = x86_hvm_setup,
.process_record = x86_hvm_process_record,
+ .static_data_complete = x86_static_data_complete,
.stream_complete = x86_hvm_stream_complete,
.cleanup = x86_hvm_cleanup,
};
diff --git a/tools/libxc/xc_sr_restore_x86_pv.c b/tools/libxc/xc_sr_restore_x86_pv.c
index 3aac4bd502..1252cd1310 100644
--- a/tools/libxc/xc_sr_restore_x86_pv.c
+++ b/tools/libxc/xc_sr_restore_x86_pv.c
@@ -1194,6 +1194,7 @@ struct xc_sr_restore_ops restore_ops_x86_pv =
.localise_page = x86_pv_localise_page,
.setup = x86_pv_setup,
.process_record = x86_pv_process_record,
+ .static_data_complete = x86_static_data_complete,
.stream_complete = x86_pv_stream_complete,
.cleanup = x86_pv_cleanup,
};
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 09f84858d5..1d54cdc429 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1298,7 +1298,8 @@ static void libxl__colo_restore_setup_done(libxl__egc *egc,
libxl__stream_read_start(egc, &dcs->srs);
}
-int libxl__srm_callout_callback_static_data_done(void *user)
+int libxl__srm_callout_callback_static_data_done(unsigned int missing,
+ void *user)
{
libxl__save_helper_state *shs = user;
libxl__domain_create_state *dcs = shs->caller_state;
@@ -1308,9 +1309,18 @@ int libxl__srm_callout_callback_static_data_done(void *user)
const libxl_domain_config *d_config = dcs->guest_config;
const libxl_domain_build_info *info = &d_config->b_info;
- libxl__cpuid_apply_policy(ctx, dcs->guest_domid);
- if (info->cpuid != NULL)
- libxl__cpuid_set(ctx, dcs->guest_domid, info->cpuid);
+ /*
+ * CPUID/MSR information is not present in pre Xen-4.14 streams.
+ *
+ * Libxl used to always regenerate the CPUID policy from first principles
+ * on migrate. Continue to do so for backwards compatibility when the
+ * stream doesn't contain any CPUID data.
+ */
+ if (missing & XGR_SDD_MISSING_CPUID) {
+ libxl__cpuid_apply_policy(ctx, dcs->guest_domid);
+ if (info->cpuid != NULL)
+ libxl__cpuid_set(ctx, dcs->guest_domid, info->cpuid);
+ }
return 0;
}
diff --git a/tools/libxl/libxl_save_msgs_gen.pl b/tools/libxl/libxl_save_msgs_gen.pl
index 93dc252370..5bfbd4fd10 100755
--- a/tools/libxl/libxl_save_msgs_gen.pl
+++ b/tools/libxl/libxl_save_msgs_gen.pl
@@ -29,7 +29,7 @@ our @msgs = (
[ 'srcxA', "wait_checkpoint", [] ],
[ 'scxA', "switch_qemu_logdirty", [qw(uint32_t domid
unsigned enable)] ],
- [ 'rcxW', "static_data_done", [] ],
+ [ 'rcxW', "static_data_done", [qw(unsigned missing)] ],
[ 'rcx', "restore_results", ['xen_pfn_t', 'store_gfn',
'xen_pfn_t', 'console_gfn'] ],
[ 'srW', "complete", [qw(int retval
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Xen-devel] [PATCH v2.1 15/17] tools/libx[cl]: Plumb 'missing' through static_data_done() up into libxl
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 15/17] tools/libx[cl]: Plumb 'missing' through static_data_done() up into libxl Andrew Cooper
@ 2020-02-24 16:50 ` Andrew Cooper
2020-02-26 12:02 ` Ian Jackson
0 siblings, 1 reply; 47+ messages in thread
From: Andrew Cooper @ 2020-02-24 16:50 UTC (permalink / raw)
To: Xen-devel; +Cc: Anthony PERARD, Andrew Cooper, Wei Liu, Ian Jackson
Pre Xen-4.14 streams will not contain any CPUID/MSR information. There is
nothing libxc can do about this, and will have to rely on the higher level
toolstack to provide backwards compatibility.
To facilitate this, extend the static_data_done() callback, highlighting the
missing information, and modify libxl to use it. At the libxc level, this
requires an arch-specific hook which, for now, always reports CPUID and MSR as
missing. This will be adjusted in a later change.
No overall functional change - this is just plumbing.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <Ian.Jackson@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Anthony PERARD <anthony.perard@citrix.com>
v2:
* Split/rearrange from v1
* Don't re-evalute 'k' on migrate
v2.1:
* Rebase over libxl__cpuid_legacy() changes
---
tools/libxc/include/xenguest.h | 12 ++++++++++--
tools/libxc/xc_sr_common.h | 9 +++++++++
tools/libxc/xc_sr_common_x86.c | 8 ++++++++
tools/libxc/xc_sr_common_x86.h | 5 +++++
tools/libxc/xc_sr_restore.c | 7 ++++++-
tools/libxc/xc_sr_restore_x86_hvm.c | 1 +
tools/libxc/xc_sr_restore_x86_pv.c | 1 +
tools/libxl/libxl_create.c | 13 +++++++++++--
tools/libxl/libxl_save_msgs_gen.pl | 2 +-
9 files changed, 52 insertions(+), 6 deletions(-)
diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h
index b4df8d0ffe..7a12d21ff2 100644
--- a/tools/libxc/include/xenguest.h
+++ b/tools/libxc/include/xenguest.h
@@ -139,8 +139,16 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom,
/* callbacks provided by xc_domain_restore */
struct restore_callbacks {
- /* Called once the STATIC_DATA_END record has been received/inferred. */
- int (*static_data_done)(void *data);
+ /*
+ * Called once the STATIC_DATA_END record has been received/inferred.
+ *
+ * For compatibility with older streams, provides a list of static data
+ * expected to be found in the stream, which was missing. A higher level
+ * toolstack is responsible for providing any necessary compatibiltiy.
+ */
+#define XGR_SDD_MISSING_CPUID (1 << 0)
+#define XGR_SDD_MISSING_MSR (1 << 1)
+ int (*static_data_done)(unsigned int missing, void *data);
/* Called after a new checkpoint to suspend the guest. */
int (*suspend)(void *data);
diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
index 7742260690..f3bdea8006 100644
--- a/tools/libxc/xc_sr_common.h
+++ b/tools/libxc/xc_sr_common.h
@@ -159,6 +159,15 @@ struct xc_sr_restore_ops
int (*process_record)(struct xc_sr_context *ctx, struct xc_sr_record *rec);
/**
+ * Perform any actions required after the static data has arrived. Called
+ * when the STATIC_DATA_COMPLETE record has been recieved/inferred.
+ * 'missing' should be filled in for any data item the higher level
+ * toolstack needs to provide compatiblity for.
+ */
+ int (*static_data_complete)(struct xc_sr_context *ctx,
+ unsigned int *missing);
+
+ /**
* Perform any actions required after the stream has been finished. Called
* after the END record has been received.
*/
diff --git a/tools/libxc/xc_sr_common_x86.c b/tools/libxc/xc_sr_common_x86.c
index 6267655dab..a849891634 100644
--- a/tools/libxc/xc_sr_common_x86.c
+++ b/tools/libxc/xc_sr_common_x86.c
@@ -132,6 +132,14 @@ int handle_x86_msr_policy(struct xc_sr_context *ctx, struct xc_sr_record *rec)
return rc;
}
+int x86_static_data_complete(struct xc_sr_context *ctx, unsigned int *missing)
+{
+ /* TODO: Become conditional on there being no data in the stream. */
+ *missing = XGR_SDD_MISSING_MSR | XGR_SDD_MISSING_CPUID;
+
+ return 0;
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/libxc/xc_sr_common_x86.h b/tools/libxc/xc_sr_common_x86.h
index d1050981dd..e08d81e0e7 100644
--- a/tools/libxc/xc_sr_common_x86.h
+++ b/tools/libxc/xc_sr_common_x86.h
@@ -34,6 +34,11 @@ int handle_x86_cpuid_policy(struct xc_sr_context *ctx,
int handle_x86_msr_policy(struct xc_sr_context *ctx,
struct xc_sr_record *rec);
+/*
+ * Perform common x86 actions required after the static data has arrived.
+ */
+int x86_static_data_complete(struct xc_sr_context *ctx, unsigned int *missing);
+
#endif
/*
* Local variables:
diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c
index bb94cd879d..bc811e6e3a 100644
--- a/tools/libxc/xc_sr_restore.c
+++ b/tools/libxc/xc_sr_restore.c
@@ -659,6 +659,7 @@ static int buffer_record(struct xc_sr_context *ctx, struct xc_sr_record *rec)
int handle_static_data_end(struct xc_sr_context *ctx)
{
xc_interface *xch = ctx->xch;
+ unsigned int missing = 0;
int rc = 0;
if ( ctx->restore.seen_static_data_end )
@@ -669,9 +670,13 @@ int handle_static_data_end(struct xc_sr_context *ctx)
ctx->restore.seen_static_data_end = true;
+ rc = ctx->restore.ops.static_data_complete(ctx, &missing);
+ if ( rc )
+ return rc;
+
if ( ctx->restore.callbacks->static_data_done &&
(rc = ctx->restore.callbacks->static_data_done(
- ctx->restore.callbacks->data) != 0) )
+ missing, ctx->restore.callbacks->data) != 0) )
ERROR("static_data_done() callback failed: %d\n", rc);
return rc;
diff --git a/tools/libxc/xc_sr_restore_x86_hvm.c b/tools/libxc/xc_sr_restore_x86_hvm.c
index 1704d524b4..a77624cc9d 100644
--- a/tools/libxc/xc_sr_restore_x86_hvm.c
+++ b/tools/libxc/xc_sr_restore_x86_hvm.c
@@ -258,6 +258,7 @@ struct xc_sr_restore_ops restore_ops_x86_hvm =
.localise_page = x86_hvm_localise_page,
.setup = x86_hvm_setup,
.process_record = x86_hvm_process_record,
+ .static_data_complete = x86_static_data_complete,
.stream_complete = x86_hvm_stream_complete,
.cleanup = x86_hvm_cleanup,
};
diff --git a/tools/libxc/xc_sr_restore_x86_pv.c b/tools/libxc/xc_sr_restore_x86_pv.c
index a3d85d517d..d086271efb 100644
--- a/tools/libxc/xc_sr_restore_x86_pv.c
+++ b/tools/libxc/xc_sr_restore_x86_pv.c
@@ -1194,6 +1194,7 @@ struct xc_sr_restore_ops restore_ops_x86_pv =
.localise_page = x86_pv_localise_page,
.setup = x86_pv_setup,
.process_record = x86_pv_process_record,
+ .static_data_complete = x86_static_data_complete,
.stream_complete = x86_pv_stream_complete,
.cleanup = x86_pv_cleanup,
};
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 2c1cbdfb2a..ad28c527ee 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1308,7 +1308,8 @@ static void libxl__colo_restore_setup_done(libxl__egc *egc,
libxl__stream_read_start(egc, &dcs->srs);
}
-int libxl__srm_callout_callback_static_data_done(void *user)
+int libxl__srm_callout_callback_static_data_done(unsigned int missing,
+ void *user)
{
libxl__save_helper_state *shs = user;
libxl__domain_create_state *dcs = shs->caller_state;
@@ -1318,7 +1319,15 @@ int libxl__srm_callout_callback_static_data_done(void *user)
libxl_domain_config *d_config = dcs->guest_config;
libxl_domain_build_info *info = &d_config->b_info;
- libxl__cpuid_legacy(ctx, dcs->guest_domid, info);
+ /*
+ * CPUID/MSR information is not present in pre Xen-4.14 streams.
+ *
+ * Libxl used to always regenerate the CPUID policy from first principles
+ * on migrate. Continue to do so for backwards compatibility when the
+ * stream doesn't contain any CPUID data.
+ */
+ if (missing & XGR_SDD_MISSING_CPUID)
+ libxl__cpuid_legacy(ctx, dcs->guest_domid, info);
return 0;
}
diff --git a/tools/libxl/libxl_save_msgs_gen.pl b/tools/libxl/libxl_save_msgs_gen.pl
index 93dc252370..5bfbd4fd10 100755
--- a/tools/libxl/libxl_save_msgs_gen.pl
+++ b/tools/libxl/libxl_save_msgs_gen.pl
@@ -29,7 +29,7 @@ our @msgs = (
[ 'srcxA', "wait_checkpoint", [] ],
[ 'scxA', "switch_qemu_logdirty", [qw(uint32_t domid
unsigned enable)] ],
- [ 'rcxW', "static_data_done", [] ],
+ [ 'rcxW', "static_data_done", [qw(unsigned missing)] ],
[ 'rcx', "restore_results", ['xen_pfn_t', 'store_gfn',
'xen_pfn_t', 'console_gfn'] ],
[ 'srW', "complete", [qw(int retval
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Xen-devel] [PATCH v2.1 15/17] tools/libx[cl]: Plumb 'missing' through static_data_done() up into libxl
2020-02-24 16:50 ` [Xen-devel] [PATCH v2.1 " Andrew Cooper
@ 2020-02-26 12:02 ` Ian Jackson
0 siblings, 0 replies; 47+ messages in thread
From: Ian Jackson @ 2020-02-26 12:02 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Anthony Perard, Xen-devel, Wei Liu
Andrew Cooper writes ("[PATCH v2.1 15/17] tools/libx[cl]: Plumb 'missing' through static_data_done() up into libxl"):
> Pre Xen-4.14 streams will not contain any CPUID/MSR information. There is
> nothing libxc can do about this, and will have to rely on the higher level
> toolstack to provide backwards compatibility.
>
> To facilitate this, extend the static_data_done() callback, highlighting the
> missing information, and modify libxl to use it. At the libxc level, this
> requires an arch-specific hook which, for now, always reports CPUID and MSR as
> missing. This will be adjusted in a later change.
>
> No overall functional change - this is just plumbing.
Thanks for the additional explanation and the explanation on IRC.
I'm now convinced that this is the best way to do it.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Xen-devel] [PATCH v2 16/17] tools/libxc: Restore CPUID/MSR data found in the migration stream
2020-01-27 14:34 [Xen-devel] [PATCH v2 00/17] Support CPUID/MSR data in migration streams Andrew Cooper
` (14 preceding siblings ...)
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 15/17] tools/libx[cl]: Plumb 'missing' through static_data_done() up into libxl Andrew Cooper
@ 2020-01-27 14:34 ` Andrew Cooper
2020-01-27 14:51 ` Jan Beulich
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 17/17] docs/xl.cfg: Rewrite cpuid= section Andrew Cooper
16 siblings, 1 reply; 47+ messages in thread
From: Andrew Cooper @ 2020-01-27 14:34 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Ian Jackson, Wei Liu, Roger Pau Monné
With all other pieces in place, it is now safe to restore the CPUID and MSR
data in the migration stream, rather than discarding them and using the higher
level toolstacks compatibility logic.
While this is a small patch, it has large implications for migrated/resumed
domains. Most obviously, the CPU family/model/stepping data,
cache/tlb/etc. will no longer change behind the guests back.
Another change is the interpretation of the Xend cpuid strings. The 'k'
option is not a sensible thing to have ever supported, and 's' is how how the
stream will end up behaving.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wl@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Ian Jackson <Ian.Jackson@citrix.com>
CC: Wei Liu <wl@xen.org>
---
tools/libxc/xc_cpuid_x86.c | 8 ++++----
tools/libxc/xc_sr_common_x86.c | 26 ++++++++++++++++++++++++--
2 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index ac38c1406e..c78d00bbc3 100644
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -291,10 +291,9 @@ int xc_set_domain_cpu_policy(xc_interface *xch, uint32_t domid,
* '0' -> force to 0
* 'x' -> we don't care (use default)
* 'k' -> pass through host value
- * 's' -> pass through the first time and then keep the same value
- * across save/restore and migration.
+ * 's' -> legacy alias for 'k'
*
- * For 's' and 'x' the configuration is overwritten with the value applied.
+ * In all cases, the returned string consists of just '0' and '1'.
*/
int xc_cpuid_set(
xc_interface *xch, uint32_t domid, const unsigned int *input,
@@ -420,7 +419,8 @@ int xc_cpuid_set(
clear_feature(31 - j, regs[i]);
config_transformed[i][j] = config[i][j];
- if ( config[i][j] == 's' )
+ /* All non 0/1 values get overwritten. */
+ if ( (config[i][j] & ~1) != '0' )
config_transformed[i][j] = '0' + val;
}
}
diff --git a/tools/libxc/xc_sr_common_x86.c b/tools/libxc/xc_sr_common_x86.c
index a849891634..77ea044a74 100644
--- a/tools/libxc/xc_sr_common_x86.c
+++ b/tools/libxc/xc_sr_common_x86.c
@@ -134,8 +134,30 @@ int handle_x86_msr_policy(struct xc_sr_context *ctx, struct xc_sr_record *rec)
int x86_static_data_complete(struct xc_sr_context *ctx, unsigned int *missing)
{
- /* TODO: Become conditional on there being no data in the stream. */
- *missing = XGR_SDD_MISSING_MSR | XGR_SDD_MISSING_CPUID;
+ xc_interface *xch = ctx->xch;
+ uint32_t nr_leaves = 0, nr_msrs = 0;
+ uint32_t err_l = ~0, err_s = ~0, err_m = ~0;
+
+ if ( ctx->x86.restore.cpuid.ptr )
+ nr_leaves = ctx->x86.restore.cpuid.size / sizeof(xen_cpuid_leaf_t);
+ else
+ *missing |= XGR_SDD_MISSING_CPUID;
+
+ if ( ctx->x86.restore.msr.ptr )
+ nr_msrs = ctx->x86.restore.msr.size / sizeof(xen_msr_entry_t);
+ else
+ *missing |= XGR_SDD_MISSING_MSR;
+
+ if ( (nr_leaves || nr_msrs) &&
+ xc_set_domain_cpu_policy(xch, ctx->domid,
+ nr_leaves, ctx->x86.restore.cpuid.ptr,
+ nr_msrs, ctx->x86.restore.msr.ptr,
+ &err_l, &err_s, &err_m) )
+ {
+ PERROR("Failed to set CPUID policy: leaf %08x, subleaf %08x, msr %08x",
+ err_l, err_s, err_m);
+ return -1;
+ }
return 0;
}
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Xen-devel] [PATCH v2 16/17] tools/libxc: Restore CPUID/MSR data found in the migration stream
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 16/17] tools/libxc: Restore CPUID/MSR data found in the migration stream Andrew Cooper
@ 2020-01-27 14:51 ` Jan Beulich
2020-01-27 15:52 ` Andrew Cooper
0 siblings, 1 reply; 47+ messages in thread
From: Jan Beulich @ 2020-01-27 14:51 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Ian Jackson, Wei Liu, Roger Pau Monné
On 27.01.2020 15:34, Andrew Cooper wrote:
> With all other pieces in place, it is now safe to restore the CPUID and MSR
> data in the migration stream, rather than discarding them and using the higher
> level toolstacks compatibility logic.
>
> While this is a small patch, it has large implications for migrated/resumed
> domains. Most obviously, the CPU family/model/stepping data,
> cache/tlb/etc. will no longer change behind the guests back.
>
> Another change is the interpretation of the Xend cpuid strings. The 'k'
> option is not a sensible thing to have ever supported, and 's' is how how the
> stream will end up behaving.
I'm a little confused I guess - I'd have expected such a description
to mean that ...
> --- a/tools/libxc/xc_cpuid_x86.c
> +++ b/tools/libxc/xc_cpuid_x86.c
> @@ -291,10 +291,9 @@ int xc_set_domain_cpu_policy(xc_interface *xch, uint32_t domid,
> * '0' -> force to 0
> * 'x' -> we don't care (use default)
> * 'k' -> pass through host value
> - * 's' -> pass through the first time and then keep the same value
> - * across save/restore and migration.
> + * 's' -> legacy alias for 'k'
... 's' remains documented as is, and 'k' to become a legacy alias.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Xen-devel] [PATCH v2 16/17] tools/libxc: Restore CPUID/MSR data found in the migration stream
2020-01-27 14:51 ` Jan Beulich
@ 2020-01-27 15:52 ` Andrew Cooper
0 siblings, 0 replies; 47+ messages in thread
From: Andrew Cooper @ 2020-01-27 15:52 UTC (permalink / raw)
To: Jan Beulich; +Cc: Xen-devel, Ian Jackson, Wei Liu, Roger Pau Monné
On 27/01/2020 14:51, Jan Beulich wrote:
> On 27.01.2020 15:34, Andrew Cooper wrote:
>> With all other pieces in place, it is now safe to restore the CPUID and MSR
>> data in the migration stream, rather than discarding them and using the higher
>> level toolstacks compatibility logic.
>>
>> While this is a small patch, it has large implications for migrated/resumed
>> domains. Most obviously, the CPU family/model/stepping data,
>> cache/tlb/etc. will no longer change behind the guests back.
>>
>> Another change is the interpretation of the Xend cpuid strings. The 'k'
>> option is not a sensible thing to have ever supported, and 's' is how how the
>> stream will end up behaving.
> I'm a little confused I guess - I'd have expected such a description
> to mean that ...
>
>> --- a/tools/libxc/xc_cpuid_x86.c
>> +++ b/tools/libxc/xc_cpuid_x86.c
>> @@ -291,10 +291,9 @@ int xc_set_domain_cpu_policy(xc_interface *xch, uint32_t domid,
>> * '0' -> force to 0
>> * 'x' -> we don't care (use default)
>> * 'k' -> pass through host value
>> - * 's' -> pass through the first time and then keep the same value
>> - * across save/restore and migration.
>> + * 's' -> legacy alias for 'k'
> ... 's' remains documented as is, and 'k' to become a legacy alias.
Given that s has never worked in the past, k is the only plausible one
used by people.
As they mean the same now, we could specify it either way around, but
this way round gives users less work to do.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Xen-devel] [PATCH v2 17/17] docs/xl.cfg: Rewrite cpuid= section
2020-01-27 14:34 [Xen-devel] [PATCH v2 00/17] Support CPUID/MSR data in migration streams Andrew Cooper
` (15 preceding siblings ...)
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 16/17] tools/libxc: Restore CPUID/MSR data found in the migration stream Andrew Cooper
@ 2020-01-27 14:34 ` Andrew Cooper
2020-04-14 20:25 ` Ping " Andrew Cooper
2020-05-29 15:47 ` Ian Jackson
16 siblings, 2 replies; 47+ messages in thread
From: Andrew Cooper @ 2020-01-27 14:34 UTC (permalink / raw)
To: Xen-devel; +Cc: Anthony PERARD, Andrew Cooper, Wei Liu, Ian Jackson
This is partly to adjust the description of 'k' and 's' seeing as they have
changed, but mostly restructuring the information for clarity.
In particular, use indentation to clearly separate the areas discussing libxl
format from xend format. In addition, extend the xend format section to
discuss subleaf notation.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <Ian.Jackson@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Anthony PERARD <anthony.perard@citrix.com>
v2:
* New
---
docs/man/xl.cfg.5.pod.in | 74 ++++++++++++++++++++++++++++++++++--------------
1 file changed, 53 insertions(+), 21 deletions(-)
diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index 245d3f9472..1da68c4a07 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -1964,26 +1964,42 @@ This option is disabled by default.
Configure the value returned when a guest executes the CPUID instruction.
Two versions of config syntax are recognized: libxl and xend.
-The libxl syntax is a comma separated list of key=value pairs, preceded by the
-word "host". A few keys take a numerical value, all others take a single
-character which describes what to do with the feature bit.
-
-Possible values for a single feature bit:
+Both formats use a common notation for specifying a single feature bit.
+Possible values are:
'1' -> force the corresponding bit to 1
'0' -> force to 0
'x' -> Get a safe value (pass through and mask with the default policy)
- 'k' -> pass through the host bit value
- 's' -> as 'k' but preserve across save/restore and migration (not implemented)
+ 'k' -> pass through the host bit value (at boot only - value preserved on migrate)
+ 's' -> legacy alias for 'k'
-Note: when specifying B<cpuid> for hypervisor leaves (0x4000xxxx major group)
-only the lowest 8 bits of leaf's 0x4000xx00 EAX register are processed, the
-rest are ignored (these 8 bits signify maximum number of hypervisor leaves).
+B<Libxl format>:
+
+=over 4
+
+The libxl format is a single string, starting with the word "host", and
+followed by a comma separated list of key=value pairs. A few keys take a
+numerical value, all others take a single character which describes what to do
+with the feature bit. e.g.:
+
+=over 4
+
+cpuid="host,tm=0,sse3=0"
+
+=back
List of keys taking a value:
+
+=over 4
+
apicidsize brandid clflush family localapicid maxleaf maxhvleaf model nc
proccount procpkg stepping
+=back
+
List of keys taking a character:
+
+=over 4
+
3dnow 3dnowext 3dnowprefetch abm acpi adx aes altmovcr8 apic arat avx avx2
avx512-4fmaps avx512-4vnniw avx512bw avx512cd avx512dq avx512er avx512f
avx512ifma avx512pf avx512vbmi avx512vl bmi1 bmi2 clflushopt clfsh clwb cmov
@@ -1997,21 +2013,37 @@ ssse3 svm svm_decode svm_lbrv svm_npt svm_nrips svm_pausefilt svm_tscrate
svm_vmcbclean syscall sysenter tbm tm tm2 topoext tsc tsc-deadline tsc_adjust
umip vme vmx wdt x2apic xop xsave xtpr
+=back
+
+=back
+
+B<Xend format>:
-The xend syntax is a list of values in the form of
-'leafnum:register=bitstring,register=bitstring'
- "leafnum" is the requested function,
- "register" is the response register to modify
- "bitstring" represents all bits in the register, its length must be 32 chars.
- Each successive character represent a lesser-significant bit, possible values
- are listed above in the libxl section.
+=over 4
-Example to hide two features from the guest: 'tm', which is bit #29 in EDX, and
-'pni' (SSE3), which is bit #0 in ECX:
+Xend format consists of an array of one or more strings of the form
+"leaf:reg=bitstring,...". e.g. (matching the libxl example above):
-xend: [ "1:ecx=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx0,edx=xx0xxxxxxxxxxxxxxxxxxxxxxxxxxxxx" ]
+=over 4
-libxl: "host,tm=0,sse3=0"
+cpuid=["1:ecx=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx0,edx=xx0xxxxxxxxxxxxxxxxxxxxxxxxxxxxx", ...]
+
+=back
+
+"leaf" is an integer, either decimal or hex with a "0x" prefix. e.g. to
+specify something in the AMD feature leaves, use "0x80000001:ecx=...".
+
+Some leaves have subleaves which can be specified as "leaf,subleaf". e.g. for
+the Intel structured feature leaf, use "7,0:ebx=..."
+
+The bitstring represents all bits in the register, its length must be 32
+chars. Each successive character represent a lesser-significant bit.
+
+=back
+
+Note: when specifying B<cpuid> for hypervisor leaves (0x4000xxxx major group)
+only the lowest 8 bits of leaf's 0x4000xx00 EAX register are processed, the
+rest are ignored (these 8 bits signify maximum number of hypervisor leaves).
More info about the CPUID instruction can be found in the processor manuals,
and on Wikipedia: L<https://en.wikipedia.org/wiki/CPUID>
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: Ping [PATCH v2 17/17] docs/xl.cfg: Rewrite cpuid= section
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 17/17] docs/xl.cfg: Rewrite cpuid= section Andrew Cooper
@ 2020-04-14 20:25 ` Andrew Cooper
2020-05-29 15:47 ` Ian Jackson
1 sibling, 0 replies; 47+ messages in thread
From: Andrew Cooper @ 2020-04-14 20:25 UTC (permalink / raw)
To: Xen-devel; +Cc: Anthony PERARD, Ian Jackson, Wei Liu
On 27/01/2020 14:34, Andrew Cooper wrote:
> This is partly to adjust the description of 'k' and 's' seeing as they have
> changed, but mostly restructuring the information for clarity.
>
> In particular, use indentation to clearly separate the areas discussing libxl
> format from xend format. In addition, extend the xend format section to
> discuss subleaf notation.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Ian Jackson <Ian.Jackson@citrix.com>
> CC: Wei Liu <wl@xen.org>
> CC: Anthony PERARD <anthony.perard@citrix.com>
>
> v2:
> * New
> ---
> docs/man/xl.cfg.5.pod.in | 74 ++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 53 insertions(+), 21 deletions(-)
>
> diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
> index 245d3f9472..1da68c4a07 100644
> --- a/docs/man/xl.cfg.5.pod.in
> +++ b/docs/man/xl.cfg.5.pod.in
> @@ -1964,26 +1964,42 @@ This option is disabled by default.
> Configure the value returned when a guest executes the CPUID instruction.
> Two versions of config syntax are recognized: libxl and xend.
>
> -The libxl syntax is a comma separated list of key=value pairs, preceded by the
> -word "host". A few keys take a numerical value, all others take a single
> -character which describes what to do with the feature bit.
> -
> -Possible values for a single feature bit:
> +Both formats use a common notation for specifying a single feature bit.
> +Possible values are:
> '1' -> force the corresponding bit to 1
> '0' -> force to 0
> 'x' -> Get a safe value (pass through and mask with the default policy)
> - 'k' -> pass through the host bit value
> - 's' -> as 'k' but preserve across save/restore and migration (not implemented)
> + 'k' -> pass through the host bit value (at boot only - value preserved on migrate)
> + 's' -> legacy alias for 'k'
>
> -Note: when specifying B<cpuid> for hypervisor leaves (0x4000xxxx major group)
> -only the lowest 8 bits of leaf's 0x4000xx00 EAX register are processed, the
> -rest are ignored (these 8 bits signify maximum number of hypervisor leaves).
> +B<Libxl format>:
> +
> +=over 4
> +
> +The libxl format is a single string, starting with the word "host", and
> +followed by a comma separated list of key=value pairs. A few keys take a
> +numerical value, all others take a single character which describes what to do
> +with the feature bit. e.g.:
> +
> +=over 4
> +
> +cpuid="host,tm=0,sse3=0"
> +
> +=back
>
> List of keys taking a value:
> +
> +=over 4
> +
> apicidsize brandid clflush family localapicid maxleaf maxhvleaf model nc
> proccount procpkg stepping
>
> +=back
> +
> List of keys taking a character:
> +
> +=over 4
> +
> 3dnow 3dnowext 3dnowprefetch abm acpi adx aes altmovcr8 apic arat avx avx2
> avx512-4fmaps avx512-4vnniw avx512bw avx512cd avx512dq avx512er avx512f
> avx512ifma avx512pf avx512vbmi avx512vl bmi1 bmi2 clflushopt clfsh clwb cmov
> @@ -1997,21 +2013,37 @@ ssse3 svm svm_decode svm_lbrv svm_npt svm_nrips svm_pausefilt svm_tscrate
> svm_vmcbclean syscall sysenter tbm tm tm2 topoext tsc tsc-deadline tsc_adjust
> umip vme vmx wdt x2apic xop xsave xtpr
>
> +=back
> +
> +=back
> +
> +B<Xend format>:
>
> -The xend syntax is a list of values in the form of
> -'leafnum:register=bitstring,register=bitstring'
> - "leafnum" is the requested function,
> - "register" is the response register to modify
> - "bitstring" represents all bits in the register, its length must be 32 chars.
> - Each successive character represent a lesser-significant bit, possible values
> - are listed above in the libxl section.
> +=over 4
>
> -Example to hide two features from the guest: 'tm', which is bit #29 in EDX, and
> -'pni' (SSE3), which is bit #0 in ECX:
> +Xend format consists of an array of one or more strings of the form
> +"leaf:reg=bitstring,...". e.g. (matching the libxl example above):
>
> -xend: [ "1:ecx=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx0,edx=xx0xxxxxxxxxxxxxxxxxxxxxxxxxxxxx" ]
> +=over 4
>
> -libxl: "host,tm=0,sse3=0"
> +cpuid=["1:ecx=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx0,edx=xx0xxxxxxxxxxxxxxxxxxxxxxxxxxxxx", ...]
> +
> +=back
> +
> +"leaf" is an integer, either decimal or hex with a "0x" prefix. e.g. to
> +specify something in the AMD feature leaves, use "0x80000001:ecx=...".
> +
> +Some leaves have subleaves which can be specified as "leaf,subleaf". e.g. for
> +the Intel structured feature leaf, use "7,0:ebx=..."
> +
> +The bitstring represents all bits in the register, its length must be 32
> +chars. Each successive character represent a lesser-significant bit.
> +
> +=back
> +
> +Note: when specifying B<cpuid> for hypervisor leaves (0x4000xxxx major group)
> +only the lowest 8 bits of leaf's 0x4000xx00 EAX register are processed, the
> +rest are ignored (these 8 bits signify maximum number of hypervisor leaves).
>
> More info about the CPUID instruction can be found in the processor manuals,
> and on Wikipedia: L<https://en.wikipedia.org/wiki/CPUID>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v2 17/17] docs/xl.cfg: Rewrite cpuid= section
2020-01-27 14:34 ` [Xen-devel] [PATCH v2 17/17] docs/xl.cfg: Rewrite cpuid= section Andrew Cooper
2020-04-14 20:25 ` Ping " Andrew Cooper
@ 2020-05-29 15:47 ` Ian Jackson
1 sibling, 0 replies; 47+ messages in thread
From: Ian Jackson @ 2020-05-29 15:47 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Anthony Perard, Xen-devel, Wei Liu
Andrew Cooper writes ("[PATCH v2 17/17] docs/xl.cfg: Rewrite cpuid= section"):
> This is partly to adjust the description of 'k' and 's' seeing as they have
> changed, but mostly restructuring the information for clarity.
>
> In particular, use indentation to clearly separate the areas discussing libxl
> format from xend format. In addition, extend the xend format section to
> discuss subleaf notation.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
^ permalink raw reply [flat|nested] 47+ messages in thread