[v2,5/6] tools/libx[cl]: Don't use HVM_PARAM_PAE_ENABLED as a function parameter
diff mbox series

Message ID 20200217175731.14828-1-andrew.cooper3@citrix.com
State New
Headers show
Series
  • Untitled series #242765
Related show

Commit Message

Andrew Cooper Feb. 17, 2020, 5:57 p.m. UTC
HVM_PARAM_PAE_ENABLED is set and consumed by the toolstack only.  It is in
practice a complicated and non-standard way of passing a boolean parameter
into xc_cpuid_apply_policy().

This is silly.  Pass PAE as a regular parameter instead.

In libxl__cpuid_legacy(), leave a rather better explaination of why only HVM
guests have a choice in PAE setting.

No change in how a guest is constructed.

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:
 * Rewrite commit message and comments.
---
 tools/libxc/include/xenctrl.h |  2 +-
 tools/libxc/xc_cpuid_x86.c    | 15 +++------------
 tools/libxl/libxl_cpuid.c     | 16 +++++++++++++++-
 3 files changed, 19 insertions(+), 14 deletions(-)

Comments

Ian Jackson Feb. 17, 2020, 5:59 p.m. UTC | #1
Andrew Cooper writes ("[PATCH v2 5/6] tools/libx[cl]: Don't use HVM_PARAM_PAE_ENABLED as a function parameter"):
> HVM_PARAM_PAE_ENABLED is set and consumed by the toolstack only.  It is in
> practice a complicated and non-standard way of passing a boolean parameter
> into xc_cpuid_apply_policy().
> 
> This is silly.  Pass PAE as a regular parameter instead.
> 
> In libxl__cpuid_legacy(), leave a rather better explaination of why only HVM
> guests have a choice in PAE setting.
> 
> No change in how a guest is constructed.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Ian.

Patch
diff mbox series

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 311df1ef0f..4eb4f4c2c6 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1807,7 +1807,7 @@  int xc_cpuid_set(xc_interface *xch,
 int xc_cpuid_apply_policy(xc_interface *xch,
                           uint32_t domid,
                           const uint32_t *featureset,
-                          unsigned int nr_features);
+                          unsigned int nr_features, bool pae);
 int xc_mca_op(xc_interface *xch, struct xen_mc *mc);
 int xc_mca_op_inject_v2(xc_interface *xch, unsigned int flags,
                         xc_cpumap_t cpumap, unsigned int nr_cpus);
diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index 2540aa1e1c..21b15b86ec 100644
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -455,7 +455,8 @@  int xc_cpuid_set(
 }
 
 int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid,
-                          const uint32_t *featureset, unsigned int nr_features)
+                          const uint32_t *featureset, unsigned int nr_features,
+                          bool pae)
 {
     int rc;
     xc_dominfo_t di;
@@ -579,8 +580,6 @@  int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid,
     }
     else
     {
-        uint64_t val;
-
         /*
          * Topology for HVM guests is entirely controlled by Xen.  For now, we
          * hardcode APIC_ID = vcpu_id * 2 to give the illusion of no SMT.
@@ -634,15 +633,7 @@  int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid,
             break;
         }
 
-        /*
-         * HVM_PARAM_PAE_ENABLED is a parameter to this function, stashed in
-         * Xen.  Nothing else has ever taken notice of the value.
-         */
-        rc = xc_hvm_param_get(xch, domid, HVM_PARAM_PAE_ENABLED, &val);
-        if ( rc )
-            goto out;
-
-        p->basic.pae = val;
+        p->basic.pae = pae;
 
         /*
          * These settings are necessary to cause earlier HVM_PARAM_NESTEDHVM /
diff --git a/tools/libxl/libxl_cpuid.c b/tools/libxl/libxl_cpuid.c
index 49d3ca5b26..062750102e 100644
--- a/tools/libxl/libxl_cpuid.c
+++ b/tools/libxl/libxl_cpuid.c
@@ -416,8 +416,22 @@  void libxl__cpuid_legacy(libxl_ctx *ctx, uint32_t domid,
     libxl_cpuid_policy_list cpuid = info->cpuid;
     int i;
     char *cpuid_res[4];
+    bool pae = true;
+
+    /*
+     * For PV guests, PAE is Xen-controlled (it is the 'p' that differentiates
+     * the xen-3.0-x86_32 and xen-3.0-x86_32p ABIs).  It is mandatory as Xen
+     * is 64bit only these days.
+     *
+     * For PVH guests, there is no top-level PAE control in the domain config,
+     * so is treated as always available.
+     *
+     * HVM guests get a top-level choice of whether PAE is available.
+     */
+    if (info->type == LIBXL_DOMAIN_TYPE_HVM)
+        pae = libxl_defbool_val(info->u.hvm.pae);
 
-    xc_cpuid_apply_policy(ctx->xch, domid, NULL, 0);
+    xc_cpuid_apply_policy(ctx->xch, domid, NULL, 0, pae);
 
     if (!cpuid)
         return;