* [PATCH 1/5] libxl: introduce cpuid interface to domain build
@ 2010-09-16 13:05 Andre Przywara
2010-09-16 15:34 ` Ian Campbell
0 siblings, 1 reply; 6+ messages in thread
From: Andre Przywara @ 2010-09-16 13:05 UTC (permalink / raw)
To: Stefano Stabellini, Ian Campbell; +Cc: xen-devel, Keir Fraser
[-- Attachment #1: Type: text/plain, Size: 355 bytes --]
Add a cpuid parameter into libxl_domain_build_info and use
it's content while setting up the domain. This is a only paving the way,
the real functionality is implemented in the later patches.
Signed-off-by: Andre Przywara <andre.przywara@amd.com>
--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448-3567-12
[-- Attachment #2: 0001-xl-introduce-cpuid-interface-to-domain-build.patch --]
[-- Type: text/x-patch, Size: 5694 bytes --]
>From 8045ab8f2e25c5c755d884f30da18ece1a4cf44f Mon Sep 17 00:00:00 2001
From: Andre Przywara <andre.przywara@amd.com>
Date: Tue, 24 Aug 2010 09:35:51 +0200
Subject: [PATCH 1/5] xl: introduce cpuid interface to domain build
this one adds a cpuid parameter into libxl_domain_build_info and uses
it's content while setting up the domain. This is a placeholder for
now, since the parsing is only implemented in the next patch.
Signed-off-by: Andre Przywara <andre.przywara@amd.com>
---
tools/libxl/libxl.c | 15 +++++++++++++++
tools/libxl/libxl.h | 8 ++++++++
tools/libxl/libxl.idl | 2 ++
tools/libxl/libxl_dom.c | 6 ++++++
tools/libxl/libxl_internal.h | 11 +++++++++++
tools/libxl/xl_cmdimpl.c | 1 +
6 files changed, 43 insertions(+), 0 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 5e5c482..fafb1a6 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -102,6 +102,21 @@ void libxl_key_value_list_destroy(libxl_key_value_list *pkvl)
free(kvl);
}
+void libxl_cpuid_destroy(libxl_cpuid_policy_list *p_cpuid_list)
+{
+ int i, j;
+ libxl_cpuid_policy_list cpuid_list = *p_cpuid_list;
+
+ if (cpuid_list == NULL)
+ return;
+ for (i = 0; cpuid_list[i].input[0] != XEN_CPUID_INPUT_UNUSED; i++) {
+ for (j = 0; j < 4; j++)
+ if (cpuid_list[i].policy[j] != NULL)
+ free(cpuid_list[i].policy[j]);
+ }
+ return;
+}
+
/******************************************************************************/
int libxl_domain_make(libxl_ctx *ctx, libxl_domain_create_info *info,
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index b7e6c94..728baf2 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -172,6 +172,13 @@ typedef enum {
NICTYPE_VIF,
} libxl_nic_type;
+/* libxl_cpuid_policy_list is a dynamic array storing CPUID policies
+ * for multiple leafs. It is terminated with an entry holding
+ * XEN_CPUID_INPUT_UNUSED in input[0]
+ */
+typedef struct libxl__cpuid_policy libxl_cpuid_policy;
+typedef libxl_cpuid_policy * libxl_cpuid_policy_list;
+
#define LIBXL_PCI_FUNC_ALL (~0U)
#include "_libxl_types.h"
@@ -231,6 +238,7 @@ int libxl_domain_preserve(libxl_ctx *ctx, uint32_t domid, libxl_domain_create_in
void libxl_string_list_destroy(libxl_string_list *sl);
void libxl_key_value_list_destroy(libxl_key_value_list *kvl);
void libxl_file_reference_destroy(libxl_file_reference *f);
+void libxl_cpuid_destroy(libxl_cpuid_policy_list *cpuid_list);
/*
* Run the configured bootloader for a PV domain and update
diff --git a/tools/libxl/libxl.idl b/tools/libxl/libxl.idl
index 1e36926..da7fc7a 100644
--- a/tools/libxl/libxl.idl
+++ b/tools/libxl/libxl.idl
@@ -11,6 +11,7 @@ libxl_console_consback = Builtin("console_consback")
libxl_console_constype = Builtin("console_constype")
libxl_disk_phystype = Builtin("disk_phystype")
libxl_nic_type = Builtin("nic_type")
+libxl_cpuid_policy_list = Builtin("cpuid_policy_list", destructor_fn="libxl_cpuid_destroy", passby=PASS_BY_REFERENCE)
libxl_string_list = Builtin("string_list", destructor_fn="libxl_string_list_destroy", passby=PASS_BY_REFERENCE)
libxl_key_value_list = Builtin("key_value_list", destructor_fn="libxl_key_value_list_destroy", passby=PASS_BY_REFERENCE)
@@ -97,6 +98,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
("shadow_memkb", uint32),
("disable_migrate", bool),
("kernel", libxl_file_reference),
+ ("cpuid", libxl_cpuid_policy_list),
("hvm", integer),
("u", KeyedUnion(None, "hvm",
[("hvm", "%s", Struct(None,
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 5dafa1d..9f5d985 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -95,9 +95,15 @@ int libxl__build_post(libxl_ctx *ctx, uint32_t domid,
xs_transaction_t t;
char **ents;
int i;
+ char *cpuid_res[4];
#if defined(__i386__) || defined(__x86_64__)
xc_cpuid_apply_policy(ctx->xch, domid);
+ if (info->cpuid != NULL) {
+ for (i = 0; info->cpuid[i].input[0] != XEN_CPUID_INPUT_UNUSED; i++)
+ xc_cpuid_set(ctx->xch, domid, info->cpuid[i].input,
+ (const char**)(info->cpuid[i].policy), cpuid_res);
+ }
#endif
ents = libxl__calloc(&gc, 12 + (info->max_vcpus * 2) + 2, sizeof(char *));
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 7ddb980..42e4e86 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -235,6 +235,17 @@ _hidden char *libxl__abs_path(libxl__gc *gc, char *s, const char *path);
_hidden char *libxl__domid_to_name(libxl__gc *gc, uint32_t domid);
_hidden char *libxl__poolid_to_name(libxl__gc *gc, uint32_t poolid);
+
+ /* holds the CPUID response for a single CPUID leaf
+ * input contains the value of the EAX and ECX register,
+ * and each policy string contains a filter to apply to
+ * the host given values for that particular leaf.
+ */
+struct libxl__cpuid_policy {
+ uint32_t input[2];
+ char *policy[4];
+};
+
/*
* blktap2 support
*/
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 12be40e..9d73ac7 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -271,6 +271,7 @@ static void init_build_info(libxl_domain_build_info *b_info, libxl_domain_create
b_info->max_memkb = 32 * 1024;
b_info->target_memkb = b_info->max_memkb;
b_info->disable_migrate = 0;
+ b_info->cpuid = NULL;
if (c_info->hvm) {
b_info->shadow_memkb = 0; /* Set later */
b_info->video_memkb = 8 * 1024;
--
1.6.4
[-- Attachment #3: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 1/5] libxl: introduce cpuid interface to domain build
@ 2010-09-08 9:19 Andre Przywara
2010-09-08 9:47 ` Ian Campbell
0 siblings, 1 reply; 6+ messages in thread
From: Andre Przywara @ 2010-09-08 9:19 UTC (permalink / raw)
To: Stefano Stabellini, Keir Fraser; +Cc: xen-devel@lists.xensource.com
[-- Attachment #1: Type: text/plain, Size: 352 bytes --]
Add a cpuid parameter into libxl_domain_build_info and use
it's content while setting up the domain. This is a only paving the way,
the real functionality is implemented in a later patch.
Signed-off-by: Andre Przywara <andre.przywara@amd.com>
--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448-3567-12
[-- Attachment #2: 0001-libxl-introduce-cpuid-interface-to-domain-build.patch --]
[-- Type: text/x-patch, Size: 5068 bytes --]
>From 6b0ac1f7fd35699da181322bcaddd1d78ef931be Mon Sep 17 00:00:00 2001
From: Andre Przywara <andre.przywara@amd.com>
Date: Tue, 24 Aug 2010 09:35:51 +0200
Subject: [PATCH 1/5] libxl: introduce cpuid interface to domain build
this one adds a cpuid parameter into libxl_domain_build_info and uses
it's content while setting up the domain. This is a placeholder for
now, since the parsing is only implemented in the next patch.
Signed-off-by: Andre Przywara <andre.przywara@amd.com>
---
tools/libxl/libxl.c | 15 +++++++++++++++
tools/libxl/libxl.h | 17 +++++++++++++++++
tools/libxl/libxl.idl | 2 ++
tools/libxl/libxl_dom.c | 6 ++++++
tools/libxl/xl_cmdimpl.c | 1 +
5 files changed, 41 insertions(+), 0 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 03d9a93..ef0e617 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -102,6 +102,21 @@ void libxl_key_value_list_destroy(libxl_key_value_list *pkvl)
free(kvl);
}
+void libxl_cpuid_destroy(libxl_cpuid_policy_list cpuid_list)
+{
+ int i, j;
+
+ if (cpuid_list == NULL)
+ return;
+ for (i = 0; cpuid_list[i].input[0] != XEN_CPUID_INPUT_UNUSED; i++) {
+ for (j = 0; j < 4; j++)
+ if (cpuid_list[i].policy[j] != NULL)
+ free(cpuid_list[i].policy[j]);
+ }
+ free(cpuid_list);
+ return;
+}
+
/******************************************************************************/
int libxl_domain_make(libxl_ctx *ctx, libxl_domain_create_info *info,
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index d989f10..92dedcf 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -172,6 +172,22 @@ typedef enum {
NICTYPE_VIF,
} libxl_nic_type;
+/* holds the CPUID response for a single CPUID leaf
+ * input contains the value of the EAX and ECX register,
+ * and each policy string contains a filter to apply to
+ * the host given values for that particular leaf.
+ */
+struct libxl_cpuid_policy {
+ uint32_t input[2];
+ char *policy[4];
+};
+
+/* libxl_cpuid_policy_list is a dynamic array storing CPUID policies
+ * for multiple leafs. It is terminated with an entry holding
+ * XEN_CPUID_INPUT_UNUSED in input[0]
+ */
+typedef struct libxl_cpuid_policy * libxl_cpuid_policy_list;
+
#define LIBXL_PCI_FUNC_ALL (~0U)
#include "_libxl_types.h"
@@ -231,6 +247,7 @@ int libxl_domain_preserve(libxl_ctx *ctx, uint32_t domid, libxl_domain_create_in
void libxl_string_list_destroy(libxl_string_list *sl);
void libxl_key_value_list_destroy(libxl_key_value_list *kvl);
void libxl_file_reference_destroy(libxl_file_reference *f);
+void libxl_cpuid_destroy(libxl_cpuid_policy_list cpuid_list);
/*
* Run the configured bootloader for a PV domain and update
diff --git a/tools/libxl/libxl.idl b/tools/libxl/libxl.idl
index 1e36926..9ad8a7e 100644
--- a/tools/libxl/libxl.idl
+++ b/tools/libxl/libxl.idl
@@ -11,6 +11,7 @@ libxl_console_consback = Builtin("console_consback")
libxl_console_constype = Builtin("console_constype")
libxl_disk_phystype = Builtin("disk_phystype")
libxl_nic_type = Builtin("nic_type")
+libxl_cpuid_policy_list = Builtin("cpuid_policy_list", destructor_fn="libxl_cpuid_destroy")
libxl_string_list = Builtin("string_list", destructor_fn="libxl_string_list_destroy", passby=PASS_BY_REFERENCE)
libxl_key_value_list = Builtin("key_value_list", destructor_fn="libxl_key_value_list_destroy", passby=PASS_BY_REFERENCE)
@@ -97,6 +98,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
("shadow_memkb", uint32),
("disable_migrate", bool),
("kernel", libxl_file_reference),
+ ("cpuid", libxl_cpuid_policy_list),
("hvm", integer),
("u", KeyedUnion(None, "hvm",
[("hvm", "%s", Struct(None,
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index e83bbbf..e8c0bda 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -91,9 +91,15 @@ int build_post(libxl_ctx *ctx, uint32_t domid,
xs_transaction_t t;
char **ents;
int i;
+ char *cpuid_res[4];
#if defined(__i386__) || defined(__x86_64__)
xc_cpuid_apply_policy(ctx->xch, domid);
+ if (info->cpuid != NULL) {
+ for (i = 0; info->cpuid[i].input[0] != XEN_CPUID_INPUT_UNUSED; i++)
+ xc_cpuid_set(ctx->xch, domid, info->cpuid[i].input,
+ (const char**)(info->cpuid[i].policy), cpuid_res);
+ }
#endif
ents = libxl_calloc(&gc, 12 + (info->max_vcpus * 2) + 2, sizeof(char *));
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 3f6219b..c6b6d85 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -268,6 +268,7 @@ static void init_build_info(libxl_domain_build_info *b_info, libxl_domain_create
b_info->max_memkb = 32 * 1024;
b_info->target_memkb = b_info->max_memkb;
b_info->disable_migrate = 0;
+ b_info->cpuid = NULL;
if (c_info->hvm) {
b_info->shadow_memkb = 0; /* Set later */
b_info->video_memkb = 8 * 1024;
--
1.6.4
[-- Attachment #3: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/5] libxl: introduce cpuid interface to domain build
2010-09-08 9:19 Andre Przywara
@ 2010-09-08 9:47 ` Ian Campbell
2010-09-09 19:16 ` Andre Przywara
0 siblings, 1 reply; 6+ messages in thread
From: Ian Campbell @ 2010-09-08 9:47 UTC (permalink / raw)
To: Andre Przywara
Cc: xen-devel@lists.xensource.com, Keir Fraser, Stefano Stabellini
On Wed, 2010-09-08 at 10:19 +0100, Andre Przywara wrote:
> Add a cpuid parameter into libxl_domain_build_info and use
> it's content while setting up the domain. This is a only paving the way,
> the real functionality is implemented in a later patch.
>
> Signed-off-by: Andre Przywara <andre.przywara@amd.com>
The destructor function should free the type but not the reference to
it, so:
> @@ -102,6 +102,21 @@ void
> libxl_key_value_list_destroy(libxl_key_value_list *pkvl)
> free(kvl);
> }
>
> +void libxl_cpuid_destroy(libxl_cpuid_policy_list cpuid_list)
This should be *cpuid_list and the function should be adjusted to free
the elements of *cpuid_list but not cpuid_list itself.
> --- a/tools/libxl/libxl.idl
> +++ b/tools/libxl/libxl.idl
> @@ -11,6 +11,7 @@ libxl_console_consback = Builtin("console_consback")
> libxl_console_constype = Builtin("console_constype")
> libxl_disk_phystype = Builtin("disk_phystype")
> libxl_nic_type = Builtin("nic_type")
> +libxl_cpuid_policy_list = Builtin("cpuid_policy_list", destructor_fn="libxl_cpuid_destroy")
And this should have passby=PASS_BY_REFERENCE.
See 22078:573ddf5cc145 for a similar change to the libxl_string_list and
libxl_key_value_list destructor functions.
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -172,6 +172,22 @@ typedef enum {
> NICTYPE_VIF,
> } libxl_nic_type;
>
> +/* holds the CPUID response for a single CPUID leaf
> + * input contains the value of the EAX and ECX register,
> + * and each policy string contains a filter to apply to
> + * the host given values for that particular leaf.
> + */
> +struct libxl_cpuid_policy {
> + uint32_t input[2];
> + char *policy[4];
> +};
I realise (at least I think I do) that this is just exposing the
existing hypervisor/libxc interface warts and all but would this be more
obvious to users if it was:
struct libxl_cpuid_policy {
uint32_t eax;
uint32_t ecx;
char *eax_filter;
char *ebx_filter;
char *ecx_filter;
char *edx_filter;
}
could either translate in libxl or push the change down into libxc.
Alternatively
#define LIBXL_CPUID_INPUT_EAX 0
#define LIBXL_CPUID_INPUT_ECX 1
#define LIBXL_CPUID_FILTER_EAX 0
#define LIBXL_CPUID_FILTER_EBX 1
...
would at least make the code (or at least the data structure) a bit more
obvious.
Ian.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/5] libxl: introduce cpuid interface to domain build
2010-09-08 9:47 ` Ian Campbell
@ 2010-09-09 19:16 ` Andre Przywara
2010-09-10 8:06 ` Ian Campbell
0 siblings, 1 reply; 6+ messages in thread
From: Andre Przywara @ 2010-09-09 19:16 UTC (permalink / raw)
To: Ian Campbell
Cc: xen-devel@lists.xensource.com, Keir Fraser, Stefano Stabellini
[-- Attachment #1: Type: text/plain, Size: 3919 bytes --]
Ian Campbell wrote:
> On Wed, 2010-09-08 at 10:19 +0100, Andre Przywara wrote:
>> Add a cpuid parameter into libxl_domain_build_info and use
>> it's content while setting up the domain. This is a only paving the way,
>> the real functionality is implemented in a later patch.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@amd.com>
>
> The destructor function should free the type but not the reference to
> it, so:
>
>> @@ -102,6 +102,21 @@ void
>> libxl_key_value_list_destroy(libxl_key_value_list *pkvl)
>> free(kvl);
>> }
>>
>> +void libxl_cpuid_destroy(libxl_cpuid_policy_list cpuid_list)
>
> This should be *cpuid_list and the function should be adjusted to free
> the elements of *cpuid_list but not cpuid_list itself.
>
>> --- a/tools/libxl/libxl.idl
>> +++ b/tools/libxl/libxl.idl
>> @@ -11,6 +11,7 @@ libxl_console_consback = Builtin("console_consback")
>> libxl_console_constype = Builtin("console_constype")
>> libxl_disk_phystype = Builtin("disk_phystype")
>> libxl_nic_type = Builtin("nic_type")
>> +libxl_cpuid_policy_list = Builtin("cpuid_policy_list", destructor_fn="libxl_cpuid_destroy")
>
> And this should have passby=PASS_BY_REFERENCE.
>
> See 22078:573ddf5cc145 for a similar change to the libxl_string_list and
> libxl_key_value_list destructor functions.
Do you mean like in the attached patch?
>
>> --- a/tools/libxl/libxl.h
>> +++ b/tools/libxl/libxl.h
>> @@ -172,6 +172,22 @@ typedef enum {
>> NICTYPE_VIF,
>> } libxl_nic_type;
>>
>> +/* holds the CPUID response for a single CPUID leaf
>> + * input contains the value of the EAX and ECX register,
>> + * and each policy string contains a filter to apply to
>> + * the host given values for that particular leaf.
>> + */
>> +struct libxl_cpuid_policy {
>> + uint32_t input[2];
>> + char *policy[4];
>> +};
>
> I realise (at least I think I do) that this is just exposing the
> existing hypervisor/libxc interface warts and all but would this be more
> obvious to users if it was:
> struct libxl_cpuid_policy {
> uint32_t eax;
> uint32_t ecx;
>
> char *eax_filter;
> char *ebx_filter;
> char *ecx_filter;
> char *edx_filter;
> }
>
> could either translate in libxl or push the change down into libxc.
Actually I consider this structure not a real interface, but more an
opaque kludge to transport the data from the configuration parsing to
domain creation. If you want to change the data here, I'd like to see
the parse functions used. Actually I designed them such that one can
alter the policy at any time by chaining calls to these functions. This
is my plan to use in the upcoming multi-core patch, it would simply call
libxl_cpuid_parse_config(&b_info->cpuid, "proccount=4");
to make it ultimately readable.
Beside that I'd rather hide the dynamic array nature of it.
> Alternatively
> #define LIBXL_CPUID_INPUT_EAX 0
> #define LIBXL_CPUID_INPUT_ECX 1
>
> #define LIBXL_CPUID_FILTER_EAX 0
> #define LIBXL_CPUID_FILTER_EBX 1
> ...
> would at least make the code (or at least the data structure) a bit more
> obvious.
I am not sure whether that would help. The interface is too error-prone
to be directly used by other functions than those parsers, so I'd like
not to promote it with defining macros (which I probably wouldn't use in
my own code, since I mostly either propagate the reg number or enumerate
over all registers).
If you like I could introduce a kind of low-level function, like:
libxl_cpuid_set_policy(libxl_cpuid_policy_list *list, uint32_t leaf,
int bit, char policy);
That could be used by other parts of libxl as well and would care about
the string fiddling and allocation.
Do we need this function?
Shall I state the opaque nature of this structure in a comment?
Regards,
Andre.
--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448-3567-12
[-- Attachment #2: 0001-libxl-introduce-cpuid-interface-to-domain-build_v2.patch --]
[-- Type: text/plain, Size: 4839 bytes --]
commit 350ce61c3c96a582a6f5d641f4ffc20e0806182a
Author: Andre Przywara <andre.przywara@amd.com>
Date: Tue Aug 24 09:35:51 2010 +0200
xl: introduce cpuid interface to domain build
this one adds a cpuid parameter into libxl_domain_build_info and uses
it's content while setting up the domain. This is a placeholder for
now, since the parsing is only implemented in the next patch.
Signed-off-by: Andre Przywara <andre.przywara@amd.com>
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 03d9a93..564afc6 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -102,6 +102,21 @@ void libxl_key_value_list_destroy(libxl_key_value_list *pkvl)
free(kvl);
}
+void libxl_cpuid_destroy(libxl_cpuid_policy_list *p_cpuid_list)
+{
+ int i, j;
+ libxl_cpuid_policy_list cpuid_list = *p_cpuid_list;
+
+ if (cpuid_list == NULL)
+ return;
+ for (i = 0; cpuid_list[i].input[0] != XEN_CPUID_INPUT_UNUSED; i++) {
+ for (j = 0; j < 4; j++)
+ if (cpuid_list[i].policy[j] != NULL)
+ free(cpuid_list[i].policy[j]);
+ }
+ return;
+}
+
/******************************************************************************/
int libxl_domain_make(libxl_ctx *ctx, libxl_domain_create_info *info,
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index d989f10..50c9e3d 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -172,6 +172,22 @@ typedef enum {
NICTYPE_VIF,
} libxl_nic_type;
+/* holds the CPUID response for a single CPUID leaf
+ * input contains the value of the EAX and ECX register,
+ * and each policy string contains a filter to apply to
+ * the host given values for that particular leaf.
+ */
+struct libxl_cpuid_policy {
+ uint32_t input[2];
+ char *policy[4];
+};
+
+/* libxl_cpuid_policy_list is a dynamic array storing CPUID policies
+ * for multiple leafs. It is terminated with an entry holding
+ * XEN_CPUID_INPUT_UNUSED in input[0]
+ */
+typedef struct libxl_cpuid_policy * libxl_cpuid_policy_list;
+
#define LIBXL_PCI_FUNC_ALL (~0U)
#include "_libxl_types.h"
@@ -231,6 +247,7 @@ int libxl_domain_preserve(libxl_ctx *ctx, uint32_t domid, libxl_domain_create_in
void libxl_string_list_destroy(libxl_string_list *sl);
void libxl_key_value_list_destroy(libxl_key_value_list *kvl);
void libxl_file_reference_destroy(libxl_file_reference *f);
+void libxl_cpuid_destroy(libxl_cpuid_policy_list *cpuid_list);
/*
* Run the configured bootloader for a PV domain and update
diff --git a/tools/libxl/libxl.idl b/tools/libxl/libxl.idl
index 1e36926..da7fc7a 100644
--- a/tools/libxl/libxl.idl
+++ b/tools/libxl/libxl.idl
@@ -11,6 +11,7 @@ libxl_console_consback = Builtin("console_consback")
libxl_console_constype = Builtin("console_constype")
libxl_disk_phystype = Builtin("disk_phystype")
libxl_nic_type = Builtin("nic_type")
+libxl_cpuid_policy_list = Builtin("cpuid_policy_list", destructor_fn="libxl_cpuid_destroy", passby=PASS_BY_REFERENCE)
libxl_string_list = Builtin("string_list", destructor_fn="libxl_string_list_destroy", passby=PASS_BY_REFERENCE)
libxl_key_value_list = Builtin("key_value_list", destructor_fn="libxl_key_value_list_destroy", passby=PASS_BY_REFERENCE)
@@ -97,6 +98,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
("shadow_memkb", uint32),
("disable_migrate", bool),
("kernel", libxl_file_reference),
+ ("cpuid", libxl_cpuid_policy_list),
("hvm", integer),
("u", KeyedUnion(None, "hvm",
[("hvm", "%s", Struct(None,
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index e83bbbf..e8c0bda 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -91,9 +91,15 @@ int build_post(libxl_ctx *ctx, uint32_t domid,
xs_transaction_t t;
char **ents;
int i;
+ char *cpuid_res[4];
#if defined(__i386__) || defined(__x86_64__)
xc_cpuid_apply_policy(ctx->xch, domid);
+ if (info->cpuid != NULL) {
+ for (i = 0; info->cpuid[i].input[0] != XEN_CPUID_INPUT_UNUSED; i++)
+ xc_cpuid_set(ctx->xch, domid, info->cpuid[i].input,
+ (const char**)(info->cpuid[i].policy), cpuid_res);
+ }
#endif
ents = libxl_calloc(&gc, 12 + (info->max_vcpus * 2) + 2, sizeof(char *));
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 3f6219b..c6b6d85 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -268,6 +268,7 @@ static void init_build_info(libxl_domain_build_info *b_info, libxl_domain_create
b_info->max_memkb = 32 * 1024;
b_info->target_memkb = b_info->max_memkb;
b_info->disable_migrate = 0;
+ b_info->cpuid = NULL;
if (c_info->hvm) {
b_info->shadow_memkb = 0; /* Set later */
b_info->video_memkb = 8 * 1024;
[-- Attachment #3: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/5] libxl: introduce cpuid interface to domain build
2010-09-09 19:16 ` Andre Przywara
@ 2010-09-10 8:06 ` Ian Campbell
0 siblings, 0 replies; 6+ messages in thread
From: Ian Campbell @ 2010-09-10 8:06 UTC (permalink / raw)
To: Andre Przywara
Cc: xen-devel@lists.xensource.com, Keir Fraser, Stefano Stabellini
On Thu, 2010-09-09 at 20:16 +0100, Andre Przywara wrote:
> Ian Campbell wrote:
> > On Wed, 2010-09-08 at 10:19 +0100, Andre Przywara wrote:
> >> Add a cpuid parameter into libxl_domain_build_info and use
> >> it's content while setting up the domain. This is a only paving the way,
> >> the real functionality is implemented in a later patch.
> >>
> >> Signed-off-by: Andre Przywara <andre.przywara@amd.com>
> >
> > The destructor function should free the type but not the reference to
> > it, so:
> >
> >> @@ -102,6 +102,21 @@ void
> >> libxl_key_value_list_destroy(libxl_key_value_list *pkvl)
> >> free(kvl);
> >> }
> >>
> >> +void libxl_cpuid_destroy(libxl_cpuid_policy_list cpuid_list)
> >
> > This should be *cpuid_list and the function should be adjusted to free
> > the elements of *cpuid_list but not cpuid_list itself.
> >
> >> --- a/tools/libxl/libxl.idl
> >> +++ b/tools/libxl/libxl.idl
> >> @@ -11,6 +11,7 @@ libxl_console_consback = Builtin("console_consback")
> >> libxl_console_constype = Builtin("console_constype")
> >> libxl_disk_phystype = Builtin("disk_phystype")
> >> libxl_nic_type = Builtin("nic_type")
> >> +libxl_cpuid_policy_list = Builtin("cpuid_policy_list", destructor_fn="libxl_cpuid_destroy")
> >
> > And this should have passby=PASS_BY_REFERENCE.
> >
> > See 22078:573ddf5cc145 for a similar change to the libxl_string_list and
> > libxl_key_value_list destructor functions.
> Do you mean like in the attached patch?
Yes it looks good from the IDL perspective.
> >
> >> --- a/tools/libxl/libxl.h
> >> +++ b/tools/libxl/libxl.h
> >> @@ -172,6 +172,22 @@ typedef enum {
> >> NICTYPE_VIF,
> >> } libxl_nic_type;
> >>
> >> +/* holds the CPUID response for a single CPUID leaf
> >> + * input contains the value of the EAX and ECX register,
> >> + * and each policy string contains a filter to apply to
> >> + * the host given values for that particular leaf.
> >> + */
> >> +struct libxl_cpuid_policy {
> >> + uint32_t input[2];
> >> + char *policy[4];
> >> +};
> >
> > I realise (at least I think I do) that this is just exposing the
> > existing hypervisor/libxc interface warts and all but would this be more
> > obvious to users if it was:
> > struct libxl_cpuid_policy {
> > uint32_t eax;
> > uint32_t ecx;
> >
> > char *eax_filter;
> > char *ebx_filter;
> > char *ecx_filter;
> > char *edx_filter;
> > }
> >
> > could either translate in libxl or push the change down into libxc.
> Actually I consider this structure not a real interface, but more an
> opaque kludge to transport the data from the configuration parsing to
> domain creation.
> If you want to change the data here, I'd like to see
> the parse functions used. Actually I designed them such that one can
> alter the policy at any time by chaining calls to these functions. This
> is my plan to use in the upcoming multi-core patch, it would simply call
> libxl_cpuid_parse_config(&b_info->cpuid, "proccount=4");
> to make it ultimately readable.
> Beside that I'd rather hide the dynamic array nature of it.
>
> > Alternatively
> > #define LIBXL_CPUID_INPUT_EAX 0
> > #define LIBXL_CPUID_INPUT_ECX 1
> >
> > #define LIBXL_CPUID_FILTER_EAX 0
> > #define LIBXL_CPUID_FILTER_EBX 1
> > ...
> > would at least make the code (or at least the data structure) a bit more
> > obvious.
> I am not sure whether that would help. The interface is too error-prone
> to be directly used by other functions than those parsers, so I'd like
> not to promote it with defining macros (which I probably wouldn't use in
> my own code, since I mostly either propagate the reg number or enumerate
> over all registers).
OK, I think that's all very reasonable.
> If you like I could introduce a kind of low-level function, like:
> libxl_cpuid_set_policy(libxl_cpuid_policy_list *list, uint32_t leaf,
> int bit, char policy);
> That could be used by other parts of libxl as well and would care about
> the string fiddling and allocation.
> Do we need this function?
I don't think we need to do this unless/until we have a user which
specifically requires it and we can always add it when such a thing
shows up.
> Shall I state the opaque nature of this structure in a comment?
If it is truly opaque to the users of libxl (and from your patches it
seems that it is) then even better would be to move struct
libxl_cpuid_policy to libxl_internal.h and rename it to struct
libxl__cpuid_policy. Then libxl.h simply contains public typedefs
typedef struct libxl__cpuid_policy libxl_cpuid_policy;
typedef libxl_cpuid_policy * libxl_cpuid_policy_list;
This is similar to how the libxl__device_model_starting type is handled.
Ian.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-09-16 15:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-16 13:05 [PATCH 1/5] libxl: introduce cpuid interface to domain build Andre Przywara
2010-09-16 15:34 ` Ian Campbell
-- strict thread matches above, loose matches on Subject: below --
2010-09-08 9:19 Andre Przywara
2010-09-08 9:47 ` Ian Campbell
2010-09-09 19:16 ` Andre Przywara
2010-09-10 8:06 ` Ian Campbell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.