[Xen-devel] [PATCH 0/6] stash domain create flags and then use them

[Xen-devel] [PATCH 0/6] stash domain create flags and then use them

[Paul Durrant]

This series starts with a patch to stash the domain create flags in struct
domain then then follows up with various clean-up patches that this
enables.

Paul Durrant (6):
  domain: stash xen_domctl_createdomain flags in struct domain
  domain: remove 'guest_type' field (and enum guest_type)
  x86/hvm/domain: remove the 'hap_enabled' flag
  x86/domain: remove the 'oos_off' flag
  domain: remove the 'is_xenstore' flag
  x86/domain: remove the 's3_integrity' flag

 xen/arch/x86/domain.c            |  9 +--------
 xen/arch/x86/mm/paging.c         |  4 ++--
 xen/arch/x86/mm/shadow/common.c  |  5 ++---
 xen/arch/x86/tboot.c             |  2 +-
 xen/common/domain.c              | 11 +++--------
 xen/common/domctl.c              | 12 ++----------
 xen/common/kernel.c              |  9 ++-------
 xen/include/asm-x86/domain.h     |  3 ---
 xen/include/asm-x86/hvm/domain.h |  9 +++------
 xen/include/asm-x86/paging.h     |  2 +-
 xen/include/asm-x86/shadow.h     |  2 +-
 xen/include/xen/sched.h          | 22 ++++++++++------------
 xen/include/xsm/dummy.h          |  2 +-
 13 files changed, 29 insertions(+), 63 deletions(-)
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Cc: Gang Wei <gang.wei@intel.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: Shane Wang <shane.wang@intel.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wl@xen.org>
-- 
2.20.1.2.gb21ebb671


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 1/6] domain: stash xen_domctl_createdomain flags in struct domain

[Paul Durrant]

These are canonical source of data used to set various other flags. If
they are available directly in struct domain then the other flags are no
longer needed.

This patch simply copies the flags into a new 'createflags' field in
struct domain. Subsequent patches will do the related clean-up work.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien.grall@arm.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>
---
 xen/common/domain.c     | 6 ++++--
 xen/include/xen/sched.h | 1 +
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index 55aa759b75..d559c8898e 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -333,6 +333,8 @@ struct domain *domain_create(domid_t domid,
     if ( (d = alloc_domain_struct()) == NULL )
         return ERR_PTR(-ENOMEM);
 
+    d->createflags = config ? config->flags : 0;
+
     /* Sort out our idea of is_system_domain(). */
     d->domain_id = domid;
 
@@ -354,7 +356,7 @@ struct domain *domain_create(domid_t domid,
     }
 
     /* Sort out our idea of is_{pv,hvm}_domain().  All system domains are PV. */
-    d->guest_type = ((config && (config->flags & XEN_DOMCTL_CDF_hvm_guest))
+    d->guest_type = ((d->createflags & XEN_DOMCTL_CDF_hvm_guest)
                      ? guest_type_hvm : guest_type_pv);
 
     TRACE_1D(TRC_DOM0_DOM_ADD, d->domain_id);
@@ -431,7 +433,7 @@ struct domain *domain_create(domid_t domid,
         watchdog_domain_init(d);
         init_status |= INIT_watchdog;
 
-        if ( config->flags & XEN_DOMCTL_CDF_xs_domain )
+        if ( d->createflags & XEN_DOMCTL_CDF_xs_domain )
         {
             d->is_xenstore = 1;
             d->disable_migrate = 1;
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index b40c8fd138..edae372c2b 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -308,6 +308,7 @@ enum guest_type {
 
 struct domain
 {
+    unsigned int     createflags;
     domid_t          domain_id;
 
     unsigned int     max_vcpus;
-- 
2.20.1.2.gb21ebb671


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 2/6] domain: remove 'guest_type' field (and enum guest_type)

[Paul Durrant]

The enum guest_type was introduced in commit 6c6492780ea "pvh prep:
introduce pv guest type and has_hvm_container macros" to allow a new guest
type, distinct from either PV or HVM guest types, to be added in commit
8271d6522c6 "pvh: introduce PVH guest type". Subsequently, commit
33e5c32559e "x86: remove PVHv1 code" removed this third guest type.

This patch removes the struct domain field and enumeration as the guest
type can now be trivially determined from the 'createflags' field.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien.grall@arm.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>
---
 xen/common/domain.c     |  4 ----
 xen/common/domctl.c     | 10 +---------
 xen/common/kernel.c     |  9 ++-------
 xen/include/xen/sched.h | 14 ++++----------
 4 files changed, 7 insertions(+), 30 deletions(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index d559c8898e..6f405d2541 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -355,10 +355,6 @@ struct domain *domain_create(domid_t domid,
         hardware_domain = d;
     }
 
-    /* Sort out our idea of is_{pv,hvm}_domain().  All system domains are PV. */
-    d->guest_type = ((d->createflags & XEN_DOMCTL_CDF_hvm_guest)
-                     ? guest_type_hvm : guest_type_pv);
-
     TRACE_1D(TRC_DOM0_DOM_ADD, d->domain_id);
 
     /*
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 72a44953d0..ef6714c0aa 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -187,17 +187,9 @@ void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info)
         (d->controller_pause_count > 0  ? XEN_DOMINF_paused    : 0) |
         (d->debugger_attached           ? XEN_DOMINF_debugged  : 0) |
         (d->is_xenstore                 ? XEN_DOMINF_xs_domain : 0) |
+        (is_hvm_domain(d)               ? XEN_DOMINF_hvm_guest : 0) |
         d->shutdown_code << XEN_DOMINF_shutdownshift;
 
-    switch ( d->guest_type )
-    {
-    case guest_type_hvm:
-        info->flags |= XEN_DOMINF_hvm_guest;
-        break;
-    default:
-        break;
-    }
-
     xsm_security_domaininfo(d, info);
 
     info->tot_pages         = d->tot_pages;
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 612575430f..f7628d73ce 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -474,19 +474,14 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
             fi.submap |= (1U << XENFEAT_ARM_SMCCC_supported);
 #endif
 #ifdef CONFIG_X86
-            switch ( d->guest_type )
-            {
-            case guest_type_pv:
+            if ( is_pv_domain(d) )
                 fi.submap |= (1U << XENFEAT_mmu_pt_update_preserve_ad) |
                              (1U << XENFEAT_highmem_assist) |
                              (1U << XENFEAT_gnttab_map_avail_bits);
-                break;
-            case guest_type_hvm:
+            else
                 fi.submap |= (1U << XENFEAT_hvm_safe_pvclock) |
                              (1U << XENFEAT_hvm_callback_vector) |
                              (has_pirq(d) ? (1U << XENFEAT_hvm_pirqs) : 0);
-                break;
-            }
 #endif
             break;
         default:
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index edae372c2b..9a98857237 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -302,10 +302,6 @@ struct vm_event_domain
 
 struct evtchn_port_ops;
 
-enum guest_type {
-    guest_type_pv, guest_type_hvm
-};
-
 struct domain
 {
     unsigned int     createflags;
@@ -357,8 +353,6 @@ struct domain
     struct radix_tree_root pirq_tree;
     unsigned int     nr_pirqs;
 
-    enum guest_type guest_type;
-
     /* Is this guest dying (i.e., a zombie)? */
     enum { DOMDYING_alive, DOMDYING_dying, DOMDYING_dead } is_dying;
 
@@ -918,8 +912,8 @@ void watchdog_domain_destroy(struct domain *d);
 
 static inline bool is_pv_domain(const struct domain *d)
 {
-    return IS_ENABLED(CONFIG_PV)
-           ? evaluate_nospec(d->guest_type == guest_type_pv) : false;
+    return IS_ENABLED(CONFIG_PV) &&
+        !evaluate_nospec(d->createflags & XEN_DOMCTL_CDF_hvm_guest);
 }
 
 static inline bool is_pv_vcpu(const struct vcpu *v)
@@ -950,8 +944,8 @@ static inline bool is_pv_64bit_vcpu(const struct vcpu *v)
 #endif
 static inline bool is_hvm_domain(const struct domain *d)
 {
-    return IS_ENABLED(CONFIG_HVM)
-           ? evaluate_nospec(d->guest_type == guest_type_hvm) : false;
+    return IS_ENABLED(CONFIG_HVM) &&
+        evaluate_nospec(d->createflags & XEN_DOMCTL_CDF_hvm_guest);
 }
 
 static inline bool is_hvm_vcpu(const struct vcpu *v)
-- 
2.20.1.2.gb21ebb671


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 3/6] x86/hvm/domain: remove the 'hap_enabled' flag

[Paul Durrant]

The hap_enabled() macro can determine whether the feature is available
using the domain 'createflags'; there is no need for a separate flag.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wl@xen.org>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
---
 xen/arch/x86/domain.c            | 7 +------
 xen/arch/x86/mm/paging.c         | 4 ++--
 xen/arch/x86/mm/shadow/common.c  | 4 ++--
 xen/include/asm-x86/hvm/domain.h | 9 +++------
 xen/include/asm-x86/paging.h     | 2 +-
 xen/include/asm-x86/shadow.h     | 2 +-
 6 files changed, 10 insertions(+), 18 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index ea55160887..65f47a7627 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -564,12 +564,7 @@ int arch_domain_create(struct domain *d,
     HYPERVISOR_COMPAT_VIRT_START(d) =
         is_pv_domain(d) ? __HYPERVISOR_COMPAT_VIRT_START : ~0u;
 
-    /* Need to determine if HAP is enabled before initialising paging */
-    if ( is_hvm_domain(d) )
-        d->arch.hvm.hap_enabled =
-            hvm_hap_supported() && (config->flags & XEN_DOMCTL_CDF_hap);
-
-    if ( (rc = paging_domain_init(d, config->flags)) != 0 )
+    if ( (rc = paging_domain_init(d)) != 0 )
         goto fail;
     paging_initialised = true;
 
diff --git a/xen/arch/x86/mm/paging.c b/xen/arch/x86/mm/paging.c
index 011089368a..69aa228e46 100644
--- a/xen/arch/x86/mm/paging.c
+++ b/xen/arch/x86/mm/paging.c
@@ -632,7 +632,7 @@ void paging_log_dirty_init(struct domain *d, const struct log_dirty_ops *ops)
 /*           CODE FOR PAGING SUPPORT            */
 /************************************************/
 /* Domain paging struct initialization. */
-int paging_domain_init(struct domain *d, unsigned int domcr_flags)
+int paging_domain_init(struct domain *d)
 {
     int rc;
 
@@ -653,7 +653,7 @@ int paging_domain_init(struct domain *d, unsigned int domcr_flags)
     if ( hap_enabled(d) )
         hap_domain_init(d);
     else
-        rc = shadow_domain_init(d, domcr_flags);
+        rc = shadow_domain_init(d);
 
     return rc;
 }
diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
index fa18de0bb6..320ea0db21 100644
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -46,7 +46,7 @@ static void sh_clean_dirty_bitmap(struct domain *);
 
 /* Set up the shadow-specific parts of a domain struct at start of day.
  * Called for every domain from arch_domain_create() */
-int shadow_domain_init(struct domain *d, unsigned int domcr_flags)
+int shadow_domain_init(struct domain *d)
 {
     static const struct log_dirty_ops sh_ops = {
         .enable  = sh_enable_log_dirty,
@@ -62,7 +62,7 @@ int shadow_domain_init(struct domain *d, unsigned int domcr_flags)
 
 #if (SHADOW_OPTIMIZATIONS & SHOPT_OUT_OF_SYNC)
     d->arch.paging.shadow.oos_active = 0;
-    d->arch.paging.shadow.oos_off = domcr_flags & XEN_DOMCTL_CDF_oos_off;
+    d->arch.paging.shadow.oos_off = d->createflags & XEN_DOMCTL_CDF_oos_off;
 #endif
     d->arch.paging.shadow.pagetable_dying_op = 0;
 
diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
index 6c7c4f5aa6..b5292696d2 100644
--- a/xen/include/asm-x86/hvm/domain.h
+++ b/xen/include/asm-x86/hvm/domain.h
@@ -156,7 +156,6 @@ struct hvm_domain {
 
     struct viridian_domain *viridian;
 
-    bool_t                 hap_enabled;
     bool_t                 mem_sharing_enabled;
     bool_t                 qemu_mapcache_invalidate;
     bool_t                 is_s3_suspended;
@@ -195,11 +194,9 @@ struct hvm_domain {
     };
 };
 
-#ifdef CONFIG_HVM
-#define hap_enabled(d)  (is_hvm_domain(d) && (d)->arch.hvm.hap_enabled)
-#else
-#define hap_enabled(d)  ({(void)(d); false;})
-#endif
+#define hap_enabled(d) \
+    (hvm_hap_supported() && is_hvm_domain(d) && \
+     evaluate_nospec(d->createflags & XEN_DOMCTL_CDF_hap))
 
 #endif /* __ASM_X86_HVM_DOMAIN_H__ */
 
diff --git a/xen/include/asm-x86/paging.h b/xen/include/asm-x86/paging.h
index cf57ca708d..ab7887f23c 100644
--- a/xen/include/asm-x86/paging.h
+++ b/xen/include/asm-x86/paging.h
@@ -207,7 +207,7 @@ void paging_vcpu_init(struct vcpu *v);
 
 /* Set up the paging-assistance-specific parts of a domain struct at
  * start of day.  Called for every domain from arch_domain_create() */
-int paging_domain_init(struct domain *d, unsigned int domcr_flags);
+int paging_domain_init(struct domain *d);
 
 /* Handler for paging-control ops: operations from user-space to enable
  * and disable ephemeral shadow modes (test mode and log-dirty mode) and
diff --git a/xen/include/asm-x86/shadow.h b/xen/include/asm-x86/shadow.h
index f29f0f652b..8ebb89c027 100644
--- a/xen/include/asm-x86/shadow.h
+++ b/xen/include/asm-x86/shadow.h
@@ -49,7 +49,7 @@
 
 /* Set up the shadow-specific parts of a domain struct at start of day.
  * Called from paging_domain_init(). */
-int shadow_domain_init(struct domain *d, unsigned int domcr_flags);
+int shadow_domain_init(struct domain *d);
 
 /* Setup the shadow-specific parts of a vcpu struct. It is called by
  * paging_vcpu_init() in paging.c */
-- 
2.20.1.2.gb21ebb671


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 4/6] x86/domain: remove the 'oos_off' flag

[Paul Durrant]

The flag is not needed since the domain 'createflags' can now be tested
directly.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Tim Deegan <tim@xen.org>
Cc: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wl@xen.org>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
---
 xen/arch/x86/mm/shadow/common.c | 3 +--
 xen/include/asm-x86/domain.h    | 1 -
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
index 320ea0db21..2c7fafa4fb 100644
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -62,7 +62,6 @@ int shadow_domain_init(struct domain *d)
 
 #if (SHADOW_OPTIMIZATIONS & SHOPT_OUT_OF_SYNC)
     d->arch.paging.shadow.oos_active = 0;
-    d->arch.paging.shadow.oos_off = d->createflags & XEN_DOMCTL_CDF_oos_off;
 #endif
     d->arch.paging.shadow.pagetable_dying_op = 0;
 
@@ -2523,7 +2522,7 @@ static void sh_update_paging_modes(struct vcpu *v)
 #if (SHADOW_OPTIMIZATIONS & SHOPT_OUT_OF_SYNC)
     /* We need to check that all the vcpus have paging enabled to
      * unsync PTs. */
-    if ( is_hvm_domain(d) && !d->arch.paging.shadow.oos_off )
+    if ( is_hvm_domain(d) && !(d->createflags & XEN_DOMCTL_CDF_oos_off) )
     {
         int pe = 1;
         struct vcpu *vptr;
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 933b85901f..5f9899469c 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -115,7 +115,6 @@ struct shadow_domain {
 
     /* OOS */
     bool_t oos_active;
-    bool_t oos_off;
 
     /* Has this domain ever used HVMOP_pagetable_dying? */
     bool_t pagetable_dying_op;
-- 
2.20.1.2.gb21ebb671


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 5/6] domain: remove the 'is_xenstore' flag

[Paul Durrant]

This patch introduces a convenience macro, is_xenstore_domain(), which
tests the domain 'createflags' directly and then uses that in place of
the 'is_xenstore' flag.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien.grall@arm.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: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 xen/common/domain.c     | 5 +----
 xen/common/domctl.c     | 2 +-
 xen/include/xen/sched.h | 7 +++++--
 xen/include/xsm/dummy.h | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index 6f405d2541..5703da357f 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -429,11 +429,8 @@ struct domain *domain_create(domid_t domid,
         watchdog_domain_init(d);
         init_status |= INIT_watchdog;
 
-        if ( d->createflags & XEN_DOMCTL_CDF_xs_domain )
-        {
-            d->is_xenstore = 1;
+        if ( is_xenstore_domain(d) )
             d->disable_migrate = 1;
-        }
 
         d->iomem_caps = rangeset_new(d, "I/O Memory", RANGESETF_prettyprint_hex);
         d->irq_caps   = rangeset_new(d, "Interrupts", 0);
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index ef6714c0aa..f2b582812c 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -186,7 +186,7 @@ void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info)
         (d->is_shut_down                ? XEN_DOMINF_shutdown  : 0) |
         (d->controller_pause_count > 0  ? XEN_DOMINF_paused    : 0) |
         (d->debugger_attached           ? XEN_DOMINF_debugged  : 0) |
-        (d->is_xenstore                 ? XEN_DOMINF_xs_domain : 0) |
+        (is_xenstore_domain(d)          ? XEN_DOMINF_xs_domain : 0) |
         (is_hvm_domain(d)               ? XEN_DOMINF_hvm_guest : 0) |
         d->shutdown_code << XEN_DOMINF_shutdownshift;
 
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 9a98857237..1033ca0e8c 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -374,8 +374,6 @@ struct domain
     bool             is_privileged;
     /* Can this guest access the Xen console? */
     bool             is_console;
-    /* Is this a xenstore domain (not dom0)? */
-    bool             is_xenstore;
     /* Non-migratable and non-restoreable? */
     bool             disable_migrate;
     /* Is this guest being debugged by dom0? */
@@ -972,6 +970,11 @@ static inline bool is_vcpu_online(const struct vcpu *v)
     return !test_bit(_VPF_down, &v->pause_flags);
 }
 
+static inline bool is_xenstore_domain(const struct domain *d)
+{
+    return d->createflags & XEN_DOMCTL_CDF_xs_domain;
+}
+
 extern bool sched_smt_power_savings;
 
 extern enum cpufreq_controller {
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index ef52bb1764..b8e185e6fa 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -79,7 +79,7 @@ static always_inline int xsm_default_action(
         {
             return 0;
     case XSM_XS_PRIV:
-            if ( src->is_xenstore )
+            if ( is_xenstore_domain(src) )
                 return 0;
         }
         /* fall through */
-- 
2.20.1.2.gb21ebb671


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 6/6] x86/domain: remove the 's3_integrity' flag

[Paul Durrant]

The flag is not needed since the domain 'createflags' can now be tested
directly.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wl@xen.org>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: Gang Wei <gang.wei@intel.com>
Cc: Shane Wang <shane.wang@intel.com>
---
 xen/arch/x86/domain.c        | 2 --
 xen/arch/x86/tboot.c         | 2 +-
 xen/include/asm-x86/domain.h | 2 --
 3 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 65f47a7627..2203882445 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -538,8 +538,6 @@ int arch_domain_create(struct domain *d,
                d->domain_id);
     }
 
-    d->arch.s3_integrity = config->flags & XEN_DOMCTL_CDF_s3_integrity;
-
     emflags = config->arch.emulation_flags;
 
     if ( is_hardware_domain(d) && is_pv_domain(d) )
diff --git a/xen/arch/x86/tboot.c b/xen/arch/x86/tboot.c
index f3fdee4d39..1a7e44a8a9 100644
--- a/xen/arch/x86/tboot.c
+++ b/xen/arch/x86/tboot.c
@@ -212,7 +212,7 @@ static void tboot_gen_domain_integrity(const uint8_t key[TB_KEY_SIZE],
     vmac_set_key((uint8_t *)key, &ctx);
     for_each_domain( d )
     {
-        if ( !d->arch.s3_integrity )
+        if ( !(d->createflags & XEN_DOMCTL_CDF_s3_integrity) )
             continue;
         printk("MACing Domain %u\n", d->domain_id);
 
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 5f9899469c..5c038a1065 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -295,8 +295,6 @@ struct arch_domain
     uint32_t pci_cf8;
     uint8_t cmos_idx;
 
-    bool_t s3_integrity;
-
     union {
         struct pv_domain pv;
         struct hvm_domain hvm;
-- 
2.20.1.2.gb21ebb671


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 4/6] x86/domain: remove the 'oos_off' flag

[Tim Deegan]

At 17:06 +0100 on 23 Jul (1563901567), Paul Durrant wrote:
> The flag is not needed since the domain 'createflags' can now be tested
> directly.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

Acked-by: Tim Deegan <tim@xen.org>

though some of this change seems to have got into patch 3, maybe they
were reordered at some point?

Cheers,

Tim.


> ---
> Cc: George Dunlap <george.dunlap@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Wei Liu <wl@xen.org>
> Cc: "Roger Pau Monné" <roger.pau@citrix.com>
> ---
>  xen/arch/x86/mm/shadow/common.c | 3 +--
>  xen/include/asm-x86/domain.h    | 1 -
>  2 files changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
> index 320ea0db21..2c7fafa4fb 100644
> --- a/xen/arch/x86/mm/shadow/common.c
> +++ b/xen/arch/x86/mm/shadow/common.c
> @@ -62,7 +62,6 @@ int shadow_domain_init(struct domain *d)
>  
>  #if (SHADOW_OPTIMIZATIONS & SHOPT_OUT_OF_SYNC)
>      d->arch.paging.shadow.oos_active = 0;
> -    d->arch.paging.shadow.oos_off = d->createflags & XEN_DOMCTL_CDF_oos_off;
>  #endif
>      d->arch.paging.shadow.pagetable_dying_op = 0;
>  
> @@ -2523,7 +2522,7 @@ static void sh_update_paging_modes(struct vcpu *v)
>  #if (SHADOW_OPTIMIZATIONS & SHOPT_OUT_OF_SYNC)
>      /* We need to check that all the vcpus have paging enabled to
>       * unsync PTs. */
> -    if ( is_hvm_domain(d) && !d->arch.paging.shadow.oos_off )
> +    if ( is_hvm_domain(d) && !(d->createflags & XEN_DOMCTL_CDF_oos_off) )
>      {
>          int pe = 1;
>          struct vcpu *vptr;
> diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
> index 933b85901f..5f9899469c 100644
> --- a/xen/include/asm-x86/domain.h
> +++ b/xen/include/asm-x86/domain.h
> @@ -115,7 +115,6 @@ struct shadow_domain {
>  
>      /* OOS */
>      bool_t oos_active;
> -    bool_t oos_off;
>  
>      /* Has this domain ever used HVMOP_pagetable_dying? */
>      bool_t pagetable_dying_op;
> -- 
> 2.20.1.2.gb21ebb671
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/6] domain: stash xen_domctl_createdomain flags in struct domain

[Roger Pau Monné]

On Tue, Jul 23, 2019 at 05:06:04PM +0100, Paul Durrant wrote:
> These are canonical source of data used to set various other flags. If
> they are available directly in struct domain then the other flags are no
> longer needed.
> 
> This patch simply copies the flags into a new 'createflags' field in
> struct domain. Subsequent patches will do the related clean-up work.

Thanks!

Just one naming comment (which is subject to taste I guess).

> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index b40c8fd138..edae372c2b 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -308,6 +308,7 @@ enum guest_type {
>  
>  struct domain
>  {
> +    unsigned int     createflags;

Can you name this just flags or options or some such (without the
create prefix). IMO adding the create prefix makes it look like a
field only used during domain creation, while it's not the case.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 2/6] domain: remove 'guest_type' field (and enum guest_type)

[Roger Pau Monné]

On Tue, Jul 23, 2019 at 05:06:05PM +0100, Paul Durrant wrote:
> The enum guest_type was introduced in commit 6c6492780ea "pvh prep:
> introduce pv guest type and has_hvm_container macros" to allow a new guest
> type, distinct from either PV or HVM guest types, to be added in commit
> 8271d6522c6 "pvh: introduce PVH guest type". Subsequently, commit
> 33e5c32559e "x86: remove PVHv1 code" removed this third guest type.
> 
> This patch removes the struct domain field and enumeration as the guest
> type can now be trivially determined from the 'createflags' field.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

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

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 3/6] x86/hvm/domain: remove the 'hap_enabled' flag

[Roger Pau Monné]

On Tue, Jul 23, 2019 at 05:06:06PM +0100, Paul Durrant wrote:
> -#ifdef CONFIG_HVM
> -#define hap_enabled(d)  (is_hvm_domain(d) && (d)->arch.hvm.hap_enabled)
> -#else
> -#define hap_enabled(d)  ({(void)(d); false;})
> -#endif
> +#define hap_enabled(d) \
> +    (hvm_hap_supported() && is_hvm_domain(d) && \
> +     evaluate_nospec(d->createflags & XEN_DOMCTL_CDF_hap))

You could make this an inline function while at it AFAICT.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 5/6] domain: remove the 'is_xenstore' flag

[Roger Pau Monné]

On Tue, Jul 23, 2019 at 05:06:08PM +0100, Paul Durrant wrote:
> This patch introduces a convenience macro, is_xenstore_domain(), which
> tests the domain 'createflags' directly and then uses that in place of
> the 'is_xenstore' flag.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

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

Just one request below (which is not related to your code actually).

> ---
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Julien Grall <julien.grall@arm.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: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> ---
>  xen/common/domain.c     | 5 +----
>  xen/common/domctl.c     | 2 +-
>  xen/include/xen/sched.h | 7 +++++--
>  xen/include/xsm/dummy.h | 2 +-
>  4 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 6f405d2541..5703da357f 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -429,11 +429,8 @@ struct domain *domain_create(domid_t domid,
>          watchdog_domain_init(d);
>          init_status |= INIT_watchdog;
>  
> -        if ( d->createflags & XEN_DOMCTL_CDF_xs_domain )
> -        {
> -            d->is_xenstore = 1;
> +        if ( is_xenstore_domain(d) )
>              d->disable_migrate = 1;

While here can you also fix this line to use true instead of 1?

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 6/6] x86/domain: remove the 's3_integrity' flag

[Roger Pau Monné]

On Tue, Jul 23, 2019 at 05:06:09PM +0100, Paul Durrant wrote:
> The flag is not needed since the domain 'createflags' can now be tested
> directly.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 3/6] x86/hvm/domain: remove the 'hap_enabled' flag

[Paul Durrant]

> -----Original Message-----
> From: Roger Pau Monne <roger.pau@citrix.com>
> Sent: 25 July 2019 10:44
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; Jan Beulich <jbeulich@suse.com>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; Wei Liu <wl@xen.org>
> Subject: Re: [PATCH 3/6] x86/hvm/domain: remove the 'hap_enabled' flag
> 
> On Tue, Jul 23, 2019 at 05:06:06PM +0100, Paul Durrant wrote:
> > -#ifdef CONFIG_HVM
> > -#define hap_enabled(d)  (is_hvm_domain(d) && (d)->arch.hvm.hap_enabled)
> > -#else
> > -#define hap_enabled(d)  ({(void)(d); false;})
> > -#endif
> > +#define hap_enabled(d) \
> > +    (hvm_hap_supported() && is_hvm_domain(d) && \
> > +     evaluate_nospec(d->createflags & XEN_DOMCTL_CDF_hap))
> 
> You could make this an inline function while at it AFAICT.

Yeah, that's not a bad idea.

  Paul

> 
> Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 5/6] domain: remove the 'is_xenstore' flag

[Paul Durrant]

> -----Original Message-----
> From: Roger Pau Monne <roger.pau@citrix.com>
> Sent: 25 July 2019 10:48
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; Stefano Stabellini <sstabellini@kernel.org>; Wei Liu <wl@xen.org>;
> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; George Dunlap <George.Dunlap@citrix.com>; Andrew
> Cooper <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Tim (Xen.org) <tim@xen.org>;
> Julien Grall <julien.grall@arm.com>; Jan Beulich <jbeulich@suse.com>; Daniel De Graaf
> <dgdegra@tycho.nsa.gov>
> Subject: Re: [Xen-devel] [PATCH 5/6] domain: remove the 'is_xenstore' flag
> 
> On Tue, Jul 23, 2019 at 05:06:08PM +0100, Paul Durrant wrote:
> > This patch introduces a convenience macro, is_xenstore_domain(), which
> > tests the domain 'createflags' directly and then uses that in place of
> > the 'is_xenstore' flag.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> 
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
> 

Thanks.

> Just one request below (which is not related to your code actually).
> 
> > ---
> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> > Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Cc: Julien Grall <julien.grall@arm.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: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> > ---
> >  xen/common/domain.c     | 5 +----
> >  xen/common/domctl.c     | 2 +-
> >  xen/include/xen/sched.h | 7 +++++--
> >  xen/include/xsm/dummy.h | 2 +-
> >  4 files changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/xen/common/domain.c b/xen/common/domain.c
> > index 6f405d2541..5703da357f 100644
> > --- a/xen/common/domain.c
> > +++ b/xen/common/domain.c
> > @@ -429,11 +429,8 @@ struct domain *domain_create(domid_t domid,
> >          watchdog_domain_init(d);
> >          init_status |= INIT_watchdog;
> >
> > -        if ( d->createflags & XEN_DOMCTL_CDF_xs_domain )
> > -        {
> > -            d->is_xenstore = 1;
> > +        if ( is_xenstore_domain(d) )
> >              d->disable_migrate = 1;
> 
> While here can you also fix this line to use true instead of 1?
> 

Sure.

  Paul

> Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/6] domain: stash xen_domctl_createdomain flags in struct domain

[Paul Durrant]

> -----Original Message-----
> From: Roger Pau Monne <roger.pau@citrix.com>
> Sent: 25 July 2019 10:23
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; Stefano Stabellini <sstabellini@kernel.org>; Wei Liu <wl@xen.org>;
> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; George Dunlap <George.Dunlap@citrix.com>; Andrew
> Cooper <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Tim (Xen.org) <tim@xen.org>;
> Julien Grall <julien.grall@arm.com>; Jan Beulich <jbeulich@suse.com>
> Subject: Re: [Xen-devel] [PATCH 1/6] domain: stash xen_domctl_createdomain flags in struct domain
> 
> On Tue, Jul 23, 2019 at 05:06:04PM +0100, Paul Durrant wrote:
> > These are canonical source of data used to set various other flags. If
> > they are available directly in struct domain then the other flags are no
> > longer needed.
> >
> > This patch simply copies the flags into a new 'createflags' field in
> > struct domain. Subsequent patches will do the related clean-up work.
> 
> Thanks!
> 
> Just one naming comment (which is subject to taste I guess).
> 
> > diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> > index b40c8fd138..edae372c2b 100644
> > --- a/xen/include/xen/sched.h
> > +++ b/xen/include/xen/sched.h
> > @@ -308,6 +308,7 @@ enum guest_type {
> >
> >  struct domain
> >  {
> > +    unsigned int     createflags;
> 
> Can you name this just flags or options or some such (without the
> create prefix). IMO adding the create prefix makes it look like a
> field only used during domain creation, while it's not the case.

I guess naming it simply 'flags' would be ok coupled with a comment in the header stating that the field is merely a copy of the domain create flags. Anyone else got opinions on this?

  Paul

> 
> Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 4/6] x86/domain: remove the 'oos_off' flag

[Paul Durrant]

> -----Original Message-----
> From: Tim Deegan <tim@xen.org>
> Sent: 24 July 2019 18:45
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; George Dunlap <George.Dunlap@citrix.com>; Jan Beulich
> <jbeulich@suse.com>; Andrew Cooper <Andrew.Cooper3@citrix.com>; Wei Liu <wl@xen.org>; Roger Pau Monne
> <roger.pau@citrix.com>
> Subject: Re: [PATCH 4/6] x86/domain: remove the 'oos_off' flag
> 
> At 17:06 +0100 on 23 Jul (1563901567), Paul Durrant wrote:
> > The flag is not needed since the domain 'createflags' can now be tested
> > directly.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> 
> Acked-by: Tim Deegan <tim@xen.org>
> 
> though some of this change seems to have got into patch 3, maybe they
> were reordered at some point?
>

Oh, yes, they were actually combined when I originally made the change. The removal of domcr_flags for shadow_domain_init() would indeed be more logically placed in this patch. It's a pretty trivial change so I'll move it in v2 and assume your A-b still stands.

  Paul
 

> Cheers,
> 
> Tim.
> 
> 
> > ---
> > Cc: George Dunlap <george.dunlap@eu.citrix.com>
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> > Cc: Wei Liu <wl@xen.org>
> > Cc: "Roger Pau Monné" <roger.pau@citrix.com>
> > ---
> >  xen/arch/x86/mm/shadow/common.c | 3 +--
> >  xen/include/asm-x86/domain.h    | 1 -
> >  2 files changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
> > index 320ea0db21..2c7fafa4fb 100644
> > --- a/xen/arch/x86/mm/shadow/common.c
> > +++ b/xen/arch/x86/mm/shadow/common.c
> > @@ -62,7 +62,6 @@ int shadow_domain_init(struct domain *d)
> >
> >  #if (SHADOW_OPTIMIZATIONS & SHOPT_OUT_OF_SYNC)
> >      d->arch.paging.shadow.oos_active = 0;
> > -    d->arch.paging.shadow.oos_off = d->createflags & XEN_DOMCTL_CDF_oos_off;
> >  #endif
> >      d->arch.paging.shadow.pagetable_dying_op = 0;
> >
> > @@ -2523,7 +2522,7 @@ static void sh_update_paging_modes(struct vcpu *v)
> >  #if (SHADOW_OPTIMIZATIONS & SHOPT_OUT_OF_SYNC)
> >      /* We need to check that all the vcpus have paging enabled to
> >       * unsync PTs. */
> > -    if ( is_hvm_domain(d) && !d->arch.paging.shadow.oos_off )
> > +    if ( is_hvm_domain(d) && !(d->createflags & XEN_DOMCTL_CDF_oos_off) )
> >      {
> >          int pe = 1;
> >          struct vcpu *vptr;
> > diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
> > index 933b85901f..5f9899469c 100644
> > --- a/xen/include/asm-x86/domain.h
> > +++ b/xen/include/asm-x86/domain.h
> > @@ -115,7 +115,6 @@ struct shadow_domain {
> >
> >      /* OOS */
> >      bool_t oos_active;
> > -    bool_t oos_off;
> >
> >      /* Has this domain ever used HVMOP_pagetable_dying? */
> >      bool_t pagetable_dying_op;
> > --
> > 2.20.1.2.gb21ebb671
> >

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/6] domain: stash xen_domctl_createdomain flags in struct domain

[Jan Beulich]

On 25.07.2019 12:11, Paul Durrant wrote:
>> -----Original Message-----
>> From: Roger Pau Monne <roger.pau@citrix.com>
>> Sent: 25 July 2019 10:23
>> To: Paul Durrant <Paul.Durrant@citrix.com>
>> Cc: xen-devel@lists.xenproject.org; Stefano Stabellini <sstabellini@kernel.org>; Wei Liu <wl@xen.org>;
>> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; George Dunlap <George.Dunlap@citrix.com>; Andrew
>> Cooper <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Tim (Xen.org) <tim@xen.org>;
>> Julien Grall <julien.grall@arm.com>; Jan Beulich <jbeulich@suse.com>
>> Subject: Re: [Xen-devel] [PATCH 1/6] domain: stash xen_domctl_createdomain flags in struct domain
>>
>> On Tue, Jul 23, 2019 at 05:06:04PM +0100, Paul Durrant wrote:
>>> These are canonical source of data used to set various other flags. If
>>> they are available directly in struct domain then the other flags are no
>>> longer needed.
>>>
>>> This patch simply copies the flags into a new 'createflags' field in
>>> struct domain. Subsequent patches will do the related clean-up work.
>>
>> Thanks!
>>
>> Just one naming comment (which is subject to taste I guess).
>>
>>> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
>>> index b40c8fd138..edae372c2b 100644
>>> --- a/xen/include/xen/sched.h
>>> +++ b/xen/include/xen/sched.h
>>> @@ -308,6 +308,7 @@ enum guest_type {
>>>
>>>   struct domain
>>>   {
>>> +    unsigned int     createflags;
>>
>> Can you name this just flags or options or some such (without the
>> create prefix). IMO adding the create prefix makes it look like a
>> field only used during domain creation, while it's not the case.
> 
> I guess naming it simply 'flags' would be ok coupled with a comment
> in the header stating that the field is merely a copy of the domain
> create flags. Anyone else got opinions on this?

We use "flags" too often imo. What about "options" as suggested by
Roger, or "settings"?

Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/6] domain: stash xen_domctl_createdomain flags in struct domain

[Paul Durrant]

> -----Original Message-----
> From: Jan Beulich <JBeulich@suse.com>
> Sent: 25 July 2019 11:25
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: Roger Pau Monne <roger.pau@citrix.com>; Julien Grall <julien.grall@arm.com>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; George Dunlap <George.Dunlap@citrix.com>; Ian Jackson
> <Ian.Jackson@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; xen-devel@lists.xenproject.org;
> KonradRzeszutek Wilk <konrad.wilk@oracle.com>; Tim (Xen.org) <tim@xen.org>; Wei Liu <wl@xen.org>
> Subject: Re: [Xen-devel] [PATCH 1/6] domain: stash xen_domctl_createdomain flags in struct domain
> 
> On 25.07.2019 12:11, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Roger Pau Monne <roger.pau@citrix.com>
> >> Sent: 25 July 2019 10:23
> >> To: Paul Durrant <Paul.Durrant@citrix.com>
> >> Cc: xen-devel@lists.xenproject.org; Stefano Stabellini <sstabellini@kernel.org>; Wei Liu
> <wl@xen.org>;
> >> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; George Dunlap <George.Dunlap@citrix.com>; Andrew
> >> Cooper <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Tim (Xen.org)
> <tim@xen.org>;
> >> Julien Grall <julien.grall@arm.com>; Jan Beulich <jbeulich@suse.com>
> >> Subject: Re: [Xen-devel] [PATCH 1/6] domain: stash xen_domctl_createdomain flags in struct domain
> >>
> >> On Tue, Jul 23, 2019 at 05:06:04PM +0100, Paul Durrant wrote:
> >>> These are canonical source of data used to set various other flags. If
> >>> they are available directly in struct domain then the other flags are no
> >>> longer needed.
> >>>
> >>> This patch simply copies the flags into a new 'createflags' field in
> >>> struct domain. Subsequent patches will do the related clean-up work.
> >>
> >> Thanks!
> >>
> >> Just one naming comment (which is subject to taste I guess).
> >>
> >>> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> >>> index b40c8fd138..edae372c2b 100644
> >>> --- a/xen/include/xen/sched.h
> >>> +++ b/xen/include/xen/sched.h
> >>> @@ -308,6 +308,7 @@ enum guest_type {
> >>>
> >>>   struct domain
> >>>   {
> >>> +    unsigned int     createflags;
> >>
> >> Can you name this just flags or options or some such (without the
> >> create prefix). IMO adding the create prefix makes it look like a
> >> field only used during domain creation, while it's not the case.
> >
> > I guess naming it simply 'flags' would be ok coupled with a comment
> > in the header stating that the field is merely a copy of the domain
> > create flags. Anyone else got opinions on this?
> 
> We use "flags" too often imo. What about "options" as suggested by
> Roger, or "settings"?

Alright, let's go with 'options' then.

  Paul

> 
> Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 3/6] x86/hvm/domain: remove the 'hap_enabled' flag

[Paul Durrant]

> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Paul Durrant
> Sent: 25 July 2019 11:08
> To: Roger Pau Monne <roger.pau@citrix.com>
> Cc: xen-devel@lists.xenproject.org; Wei Liu <wl@xen.org>; Jan Beulich <jbeulich@suse.com>; Andrew
> Cooper <Andrew.Cooper3@citrix.com>
> Subject: Re: [Xen-devel] [PATCH 3/6] x86/hvm/domain: remove the 'hap_enabled' flag
> 
> > -----Original Message-----
> > From: Roger Pau Monne <roger.pau@citrix.com>
> > Sent: 25 July 2019 10:44
> > To: Paul Durrant <Paul.Durrant@citrix.com>
> > Cc: xen-devel@lists.xenproject.org; Jan Beulich <jbeulich@suse.com>; Andrew Cooper
> > <Andrew.Cooper3@citrix.com>; Wei Liu <wl@xen.org>
> > Subject: Re: [PATCH 3/6] x86/hvm/domain: remove the 'hap_enabled' flag
> >
> > On Tue, Jul 23, 2019 at 05:06:06PM +0100, Paul Durrant wrote:
> > > -#ifdef CONFIG_HVM
> > > -#define hap_enabled(d)  (is_hvm_domain(d) && (d)->arch.hvm.hap_enabled)
> > > -#else
> > > -#define hap_enabled(d)  ({(void)(d); false;})
> > > -#endif
> > > +#define hap_enabled(d) \
> > > +    (hvm_hap_supported() && is_hvm_domain(d) && \
> > > +     evaluate_nospec(d->createflags & XEN_DOMCTL_CDF_hap))
> >
> > You could make this an inline function while at it AFAICT.
> 
> Yeah, that's not a bad idea.
> 

Alas this turns out to be a can of worms. As an inline function I cannot call is_hvm_domain() from here which, I guess, is why it’s a macro at the moment. Also I cannot dereference d->createflags (a.k.a. options) as the definition of struct domain is not available.

  Paul

>   Paul
> 
> >
> > Thanks, Roger.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel