All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel <xen-devel@lists.xenproject.org>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Jan Beulich" <JBeulich@suse.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>
Subject: [PATCH] libs/guest: Don't hide the indirection on xc_cpu_policy_t
Date: Tue, 4 May 2021 19:53:22 +0100	[thread overview]
Message-ID: <20210504185322.19306-1-andrew.cooper3@citrix.com> (raw)

It is bad form in C, perhaps best demonstrated by trying to read
xc_cpu_policy_destroy(), and causes const qualification to have
less-than-obvious behaviour (the hidden pointer becomes const, not the thing
it points at).

xc_cpu_policy_set_domain() needs to drop its (now normal) const qualification,
as the policy object is modified by the serialisation operation.

This also shows up a problem with the x86_cpu_policies_are_compatible(), where
the intermediate pointers are non-const.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>

Discovered while trying to start the integration into XenServer.  This wants
fixing ASAP, before futher uses get added.

Unsure what to do about x86_cpu_policies_are_compatible().  It would be nice
to have xc_cpu_policy_is_compatible() sensibly const'd, but maybe that means
we need a struct const_cpu_policy and that smells like it is spiralling out of
control.
---
 tools/include/xenctrl.h             | 22 +++++++++++-----------
 tools/libs/guest/xg_cpuid_x86.c     | 22 +++++++++++-----------
 tools/libs/guest/xg_sr_common_x86.c |  2 +-
 tools/misc/xen-cpuid.c              |  2 +-
 4 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index 0fdb2e8885..58d3377d6a 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -2590,33 +2590,33 @@ int xc_psr_get_domain_data(xc_interface *xch, uint32_t domid,
 int xc_psr_get_hw_info(xc_interface *xch, uint32_t socket,
                        xc_psr_feat_type type, xc_psr_hw_info *hw_info);
 
-typedef struct xc_cpu_policy *xc_cpu_policy_t;
+typedef struct xc_cpu_policy xc_cpu_policy_t;
 
 /* Create and free a xc_cpu_policy object. */
-xc_cpu_policy_t xc_cpu_policy_init(void);
-void xc_cpu_policy_destroy(xc_cpu_policy_t policy);
+xc_cpu_policy_t *xc_cpu_policy_init(void);
+void xc_cpu_policy_destroy(xc_cpu_policy_t *policy);
 
 /* Retrieve a system policy, or get/set a domains policy. */
 int xc_cpu_policy_get_system(xc_interface *xch, unsigned int policy_idx,
-                             xc_cpu_policy_t policy);
+                             xc_cpu_policy_t *policy);
 int xc_cpu_policy_get_domain(xc_interface *xch, uint32_t domid,
-                             xc_cpu_policy_t policy);
+                             xc_cpu_policy_t *policy);
 int xc_cpu_policy_set_domain(xc_interface *xch, uint32_t domid,
-                             const xc_cpu_policy_t policy);
+                             xc_cpu_policy_t *policy);
 
 /* Manipulate a policy via architectural representations. */
-int xc_cpu_policy_serialise(xc_interface *xch, const xc_cpu_policy_t policy,
+int xc_cpu_policy_serialise(xc_interface *xch, const xc_cpu_policy_t *policy,
                             xen_cpuid_leaf_t *leaves, uint32_t *nr_leaves,
                             xen_msr_entry_t *msrs, uint32_t *nr_msrs);
-int xc_cpu_policy_update_cpuid(xc_interface *xch, xc_cpu_policy_t policy,
+int xc_cpu_policy_update_cpuid(xc_interface *xch, xc_cpu_policy_t *policy,
                                const xen_cpuid_leaf_t *leaves,
                                uint32_t nr);
-int xc_cpu_policy_update_msrs(xc_interface *xch, xc_cpu_policy_t policy,
+int xc_cpu_policy_update_msrs(xc_interface *xch, xc_cpu_policy_t *policy,
                               const xen_msr_entry_t *msrs, uint32_t nr);
 
 /* Compatibility calculations. */
-bool xc_cpu_policy_is_compatible(xc_interface *xch, const xc_cpu_policy_t host,
-                                 const xc_cpu_policy_t guest);
+bool xc_cpu_policy_is_compatible(xc_interface *xch, xc_cpu_policy_t *host,
+                                 xc_cpu_policy_t *guest);
 
 int xc_get_cpu_levelling_caps(xc_interface *xch, uint32_t *caps);
 int xc_get_cpu_featureset(xc_interface *xch, uint32_t index,
diff --git a/tools/libs/guest/xg_cpuid_x86.c b/tools/libs/guest/xg_cpuid_x86.c
index d4e02cecb1..1ebc108213 100644
--- a/tools/libs/guest/xg_cpuid_x86.c
+++ b/tools/libs/guest/xg_cpuid_x86.c
@@ -672,18 +672,18 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid, bool restore,
     return rc;
 }
 
-xc_cpu_policy_t xc_cpu_policy_init(void)
+xc_cpu_policy_t *xc_cpu_policy_init(void)
 {
     return calloc(1, sizeof(struct xc_cpu_policy));
 }
 
-void xc_cpu_policy_destroy(xc_cpu_policy_t policy)
+void xc_cpu_policy_destroy(xc_cpu_policy_t *policy)
 {
     if ( policy )
         free(policy);
 }
 
-static int deserialize_policy(xc_interface *xch, xc_cpu_policy_t policy,
+static int deserialize_policy(xc_interface *xch, xc_cpu_policy_t *policy,
                               unsigned int nr_leaves, unsigned int nr_entries)
 {
     uint32_t err_leaf = -1, err_subleaf = -1, err_msr = -1;
@@ -713,7 +713,7 @@ static int deserialize_policy(xc_interface *xch, xc_cpu_policy_t policy,
 }
 
 int xc_cpu_policy_get_system(xc_interface *xch, unsigned int policy_idx,
-                             xc_cpu_policy_t policy)
+                             xc_cpu_policy_t *policy)
 {
     unsigned int nr_leaves = ARRAY_SIZE(policy->leaves);
     unsigned int nr_entries = ARRAY_SIZE(policy->entries);
@@ -738,7 +738,7 @@ int xc_cpu_policy_get_system(xc_interface *xch, unsigned int policy_idx,
 }
 
 int xc_cpu_policy_get_domain(xc_interface *xch, uint32_t domid,
-                             xc_cpu_policy_t policy)
+                             xc_cpu_policy_t *policy)
 {
     unsigned int nr_leaves = ARRAY_SIZE(policy->leaves);
     unsigned int nr_entries = ARRAY_SIZE(policy->entries);
@@ -763,7 +763,7 @@ int xc_cpu_policy_get_domain(xc_interface *xch, uint32_t domid,
 }
 
 int xc_cpu_policy_set_domain(xc_interface *xch, uint32_t domid,
-                             const xc_cpu_policy_t policy)
+                             xc_cpu_policy_t *policy)
 {
     uint32_t err_leaf = -1, err_subleaf = -1, err_msr = -1;
     unsigned int nr_leaves = ARRAY_SIZE(policy->leaves);
@@ -791,7 +791,7 @@ int xc_cpu_policy_set_domain(xc_interface *xch, uint32_t domid,
     return rc;
 }
 
-int xc_cpu_policy_serialise(xc_interface *xch, const xc_cpu_policy_t p,
+int xc_cpu_policy_serialise(xc_interface *xch, const xc_cpu_policy_t *p,
                             xen_cpuid_leaf_t *leaves, uint32_t *nr_leaves,
                             xen_msr_entry_t *msrs, uint32_t *nr_msrs)
 {
@@ -823,7 +823,7 @@ int xc_cpu_policy_serialise(xc_interface *xch, const xc_cpu_policy_t p,
     return 0;
 }
 
-int xc_cpu_policy_update_cpuid(xc_interface *xch, xc_cpu_policy_t policy,
+int xc_cpu_policy_update_cpuid(xc_interface *xch, xc_cpu_policy_t *policy,
                                const xen_cpuid_leaf_t *leaves,
                                uint32_t nr)
 {
@@ -843,7 +843,7 @@ int xc_cpu_policy_update_cpuid(xc_interface *xch, xc_cpu_policy_t policy,
     return rc;
 }
 
-int xc_cpu_policy_update_msrs(xc_interface *xch, xc_cpu_policy_t policy,
+int xc_cpu_policy_update_msrs(xc_interface *xch, xc_cpu_policy_t *policy,
                               const xen_msr_entry_t *msrs, uint32_t nr)
 {
     unsigned int err_msr = -1;
@@ -861,8 +861,8 @@ int xc_cpu_policy_update_msrs(xc_interface *xch, xc_cpu_policy_t policy,
     return rc;
 }
 
-bool xc_cpu_policy_is_compatible(xc_interface *xch, const xc_cpu_policy_t host,
-                                 const xc_cpu_policy_t guest)
+bool xc_cpu_policy_is_compatible(xc_interface *xch, xc_cpu_policy_t *host,
+                                 xc_cpu_policy_t *guest)
 {
     struct cpu_policy_errors err = INIT_CPU_POLICY_ERRORS;
     struct cpu_policy h = { &host->cpuid, &host->msr };
diff --git a/tools/libs/guest/xg_sr_common_x86.c b/tools/libs/guest/xg_sr_common_x86.c
index 15265e7a33..563b4f0168 100644
--- a/tools/libs/guest/xg_sr_common_x86.c
+++ b/tools/libs/guest/xg_sr_common_x86.c
@@ -48,7 +48,7 @@ int write_x86_cpu_policy_records(struct xc_sr_context *ctx)
     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;
-    xc_cpu_policy_t policy = NULL;
+    xc_cpu_policy_t *policy = NULL;
     int rc;
 
     if ( xc_cpu_policy_get_size(xch, &nr_leaves, &nr_msrs) < 0 )
diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
index b2a36deacc..d4bc83d8c9 100644
--- a/tools/misc/xen-cpuid.c
+++ b/tools/misc/xen-cpuid.c
@@ -468,7 +468,7 @@ int main(int argc, char **argv)
         uint32_t i, max_leaves, max_msrs;
 
         xc_interface *xch = xc_interface_open(0, 0, 0);
-        xc_cpu_policy_t policy = xc_cpu_policy_init();
+        xc_cpu_policy_t *policy = xc_cpu_policy_init();
 
         if ( !xch )
             err(1, "xc_interface_open");
-- 
2.11.0



             reply	other threads:[~2021-05-04 18:54 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-04 18:53 Andrew Cooper [this message]
2021-05-05  6:27 ` [PATCH] libs/guest: Don't hide the indirection on xc_cpu_policy_t Jan Beulich
2021-05-05  9:16 ` Roger Pau Monné
2021-05-05 12:48   ` Andrew Cooper

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210504185322.19306-1-andrew.cooper3@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=JBeulich@suse.com \
    --cc=roger.pau@citrix.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
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.