diff mbox series

[5/8] x86/hvm: Obsolete the use of HVM_PARAM_NESTEDHVM

Message ID 20200930134248.4918-6-andrew.cooper3@citrix.com (mailing list archive)
State New, archived
Headers show
Series x86: Untangle Nested virt and CPUID interactions | expand

Commit Message

Andrew Cooper Sept. 30, 2020, 1:42 p.m. UTC
With XEN_DOMCTL_CDF_nested_virt now passed properly to domain_create(),
reimplement nestedhvm_enabled() to use the property which is fixed for the
lifetime of the domain.

This makes the call to nestedhvm_vcpu_initialise() from hvm_vcpu_initialise()
no longer dead.  It became logically dead with the Xend => XL transition, as
they initialise HVM_PARAM_NESTEDHVM in opposite orders with respect to
XEN_DOMCTL_max_vcpus.

There is one opencoded user of nestedhvm_enabled() in HVM_PARAM_ALTP2M's
safety check.

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>
CC: Ian Jackson <iwj@xenproject.org>
CC: Anthony PERARD <anthony.perard@citrix.com>

The redunction in complexity of nestedhvm_enabled() has a substantial effect:

  add/remove: 1/0 grow/shrink: 2/37 up/down: 669/-2095 (-1426)
---
 tools/libxl/libxl_x86.c             |  5 -----
 xen/arch/x86/hvm/hvm.c              | 33 ++++-----------------------------
 xen/include/asm-x86/hvm/nestedhvm.h |  5 ++---
 xen/include/public/hvm/params.h     |  4 +---
 4 files changed, 7 insertions(+), 40 deletions(-)

Comments

Roger Pau Monné Oct. 1, 2020, 10:53 a.m. UTC | #1
On Wed, Sep 30, 2020 at 02:42:45PM +0100, Andrew Cooper wrote:
> With XEN_DOMCTL_CDF_nested_virt now passed properly to domain_create(),
> reimplement nestedhvm_enabled() to use the property which is fixed for the
> lifetime of the domain.
> 
> This makes the call to nestedhvm_vcpu_initialise() from hvm_vcpu_initialise()
> no longer dead.  It became logically dead with the Xend => XL transition, as
> they initialise HVM_PARAM_NESTEDHVM in opposite orders with respect to
> XEN_DOMCTL_max_vcpus.
> 
> There is one opencoded user of nestedhvm_enabled() in HVM_PARAM_ALTP2M's
> safety check.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

It's nice that the logic is already present in hvm_vcpu_initialise :).

Thanks, Roger.
Wei Liu Oct. 1, 2020, 10:57 a.m. UTC | #2
On Wed, Sep 30, 2020 at 02:42:45PM +0100, Andrew Cooper wrote:
> With XEN_DOMCTL_CDF_nested_virt now passed properly to domain_create(),
> reimplement nestedhvm_enabled() to use the property which is fixed for the
> lifetime of the domain.
> 
> This makes the call to nestedhvm_vcpu_initialise() from hvm_vcpu_initialise()
> no longer dead.  It became logically dead with the Xend => XL transition, as
> they initialise HVM_PARAM_NESTEDHVM in opposite orders with respect to
> XEN_DOMCTL_max_vcpus.
> 
> There is one opencoded user of nestedhvm_enabled() in HVM_PARAM_ALTP2M's
> safety check.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Wei Liu <wl@xen.org>
diff mbox series

Patch

diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
index 7d95506e00..72777fcaad 100644
--- a/tools/libxl/libxl_x86.c
+++ b/tools/libxl/libxl_x86.c
@@ -428,11 +428,6 @@  static int hvm_set_conf_params(libxl__gc *gc, uint32_t domid,
             LOG(ERROR, "Couldn't set HVM_PARAM_TIMER_MODE");
             goto out;
         }
-        if (xc_hvm_param_set(xch, domid, HVM_PARAM_NESTEDHVM,
-                             libxl_defbool_val(info->nested_hvm))) {
-            LOG(ERROR, "Couldn't set HVM_PARAM_NESTEDHVM");
-            goto out;
-        }
         if (xc_hvm_param_set(xch, domid, HVM_PARAM_ALTP2M, altp2m)) {
             LOG(ERROR, "Couldn't set HVM_PARAM_ALTP2M");
             goto out;
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 2dfda93e09..101a739952 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4086,6 +4086,7 @@  static int hvm_allow_set_param(struct domain *d,
     case HVM_PARAM_MEMORY_EVENT_CR3:
     case HVM_PARAM_MEMORY_EVENT_CR4:
     case HVM_PARAM_MEMORY_EVENT_INT3:
+    case HVM_PARAM_NESTEDHVM:
     case HVM_PARAM_MEMORY_EVENT_SINGLE_STEP:
     case HVM_PARAM_BUFIOREQ_EVTCHN:
     case HVM_PARAM_MEMORY_EVENT_MSR:
@@ -4204,39 +4205,12 @@  static int hvm_set_param(struct domain *d, uint32_t index, uint64_t value)
     case HVM_PARAM_ACPI_IOPORTS_LOCATION:
         rc = pmtimer_change_ioport(d, value);
         break;
-    case HVM_PARAM_NESTEDHVM:
-        rc = xsm_hvm_param_nested(XSM_PRIV, d);
-        if ( rc )
-            break;
-        if ( value > 1 )
-            rc = -EINVAL;
-        /*
-         * Remove the check below once we have
-         * shadow-on-shadow.
-         */
-        if ( !paging_mode_hap(d) && value )
-            rc = -EINVAL;
-        if ( value &&
-             d->arch.hvm.params[HVM_PARAM_ALTP2M] )
-            rc = -EINVAL;
-        /* Set up NHVM state for any vcpus that are already up. */
-        if ( value &&
-             !d->arch.hvm.params[HVM_PARAM_NESTEDHVM] )
-            for_each_vcpu(d, v)
-                if ( rc == 0 )
-                    rc = nestedhvm_vcpu_initialise(v);
-        if ( !value || rc )
-            for_each_vcpu(d, v)
-                nestedhvm_vcpu_destroy(v);
-        break;
     case HVM_PARAM_ALTP2M:
         rc = xsm_hvm_param_altp2mhvm(XSM_PRIV, d);
         if ( rc )
             break;
-        if ( value > XEN_ALTP2M_limited )
-            rc = -EINVAL;
-        if ( value &&
-             d->arch.hvm.params[HVM_PARAM_NESTEDHVM] )
+        if ( (value > XEN_ALTP2M_limited) ||
+             (value && nestedhvm_enabled(d)) )
             rc = -EINVAL;
         break;
     case HVM_PARAM_TRIPLE_FAULT_REASON:
@@ -4390,6 +4364,7 @@  static int hvm_allow_get_param(struct domain *d,
     case HVM_PARAM_MEMORY_EVENT_CR3:
     case HVM_PARAM_MEMORY_EVENT_CR4:
     case HVM_PARAM_MEMORY_EVENT_INT3:
+    case HVM_PARAM_NESTEDHVM:
     case HVM_PARAM_MEMORY_EVENT_SINGLE_STEP:
     case HVM_PARAM_BUFIOREQ_EVTCHN:
     case HVM_PARAM_MEMORY_EVENT_MSR:
diff --git a/xen/include/asm-x86/hvm/nestedhvm.h b/xen/include/asm-x86/hvm/nestedhvm.h
index d9784a2e0b..385cb708a3 100644
--- a/xen/include/asm-x86/hvm/nestedhvm.h
+++ b/xen/include/asm-x86/hvm/nestedhvm.h
@@ -34,10 +34,9 @@  enum nestedhvm_vmexits {
 };
 
 /* Nested HVM on/off per domain */
-static always_inline bool nestedhvm_enabled(const struct domain *d)
+static inline bool nestedhvm_enabled(const struct domain *d)
 {
-    return is_hvm_domain(d) && d->arch.hvm.params &&
-        d->arch.hvm.params[HVM_PARAM_NESTEDHVM];
+    return d->options & XEN_DOMCTL_CDF_nested_virt;
 }
 
 /* Nested VCPU */
diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
index 0a91bfa749..0e3fdca096 100644
--- a/xen/include/public/hvm/params.h
+++ b/xen/include/public/hvm/params.h
@@ -34,6 +34,7 @@ 
 #define HVM_PARAM_MEMORY_EVENT_CR3          21
 #define HVM_PARAM_MEMORY_EVENT_CR4          22
 #define HVM_PARAM_MEMORY_EVENT_INT3         23
+#define HVM_PARAM_NESTEDHVM                 24
 #define HVM_PARAM_MEMORY_EVENT_SINGLE_STEP  25
 #define HVM_PARAM_BUFIOREQ_EVTCHN           26
 #define HVM_PARAM_MEMORY_EVENT_MSR          30
@@ -232,9 +233,6 @@ 
  */
 #define HVM_PARAM_ACPI_IOPORTS_LOCATION 19
 
-/* Boolean: Enable nestedhvm (hvm only) */
-#define HVM_PARAM_NESTEDHVM    24
-
 /* Params for the mem event rings */
 #define HVM_PARAM_PAGING_RING_PFN   27
 #define HVM_PARAM_MONITOR_RING_PFN  28