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

NOTE: Furthermore, by extending sanitiziing of the domain 'options', the
      macro can be transformed into an inline function and re-located to
      xen/sched.h. This also makes hap_enabled() common, thus allowing
      removal of an ugly ifdef CONFIG_X86 from the common iommu code.

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: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.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: George Dunlap <george.dunlap@eu.citrix.com>

v3:
 - Re-worked as suggested by Jan
 - Not adding Roger's R-b as the patch has changed substantially

v2:
 - Defer changes to shadow_domain_init() to patch #4
---
 xen/arch/x86/domain.c            | 13 +++++++------
 xen/arch/x86/mm/paging.c         |  4 ++--
 xen/common/domain.c              |  7 +++++++
 xen/drivers/passthrough/iommu.c  |  2 --
 xen/include/asm-x86/hvm/domain.h |  7 -------
 xen/include/asm-x86/paging.h     |  2 +-
 xen/include/xen/sched.h          |  5 +++++
 7 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 5933b3f51b..0f43da3b6a 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -460,6 +460,12 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
         return -EINVAL;
     }
 
+    if ( config->flags & XEN_DOMCTL_CDF_hap && !hvm_hap_supported() )
+    {
+        dprintk(XENLOG_INFO, "HAP enabled but not supported\n");
+        return -EINVAL;
+    }
+
     return 0;
 }
 
@@ -564,12 +570,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..097a27f608 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, d->options);
 
     return rc;
 }
diff --git a/xen/common/domain.c b/xen/common/domain.c
index df523c9ce4..53d0a62f82 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -313,6 +313,13 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
         return -EINVAL;
     }
 
+    if ( !(config->flags & XEN_DOMCTL_CDF_hvm_guest) &&
+         config->flags & XEN_DOMCTL_CDF_hap )
+    {
+        dprintk(XENLOG_INFO, "HAP enabled for non-HVM guest\n");
+        return -EINVAL;
+    }
+
     return arch_sanitise_domain_config(config);
 }
 
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index f8c3bf53bd..37eb0f7d01 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -573,9 +573,7 @@ int iommu_do_domctl(
 
 void iommu_share_p2m_table(struct domain* d)
 {
-#ifdef CONFIG_X86
     ASSERT(hap_enabled(d));
-#endif
     /*
      * iommu_use_hap_pt(d) cannot be used here because during domain
      * construction need_iommu(d) will always return false here.
diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
index 6c7c4f5aa6..bcc5621797 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,12 +194,6 @@ 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
-
 #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/xen/sched.h b/xen/include/xen/sched.h
index 79eff418c4..e14540966f 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -948,6 +948,11 @@ static inline bool is_hvm_domain(const struct domain *d)
         evaluate_nospec(d->options & XEN_DOMCTL_CDF_hvm_guest);
 }
 
+static inline bool hap_enabled(const struct domain *d)
+{
+    return evaluate_nospec(d->options & XEN_DOMCTL_CDF_hap);
+}
+
 static inline bool is_hvm_vcpu(const struct vcpu *v)
 {
     return is_hvm_domain(v->domain);
-- 
2.20.1.2.gb21ebb671


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

On 30.07.2019 11:39, Paul Durrant wrote:
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -460,6 +460,12 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
>           return -EINVAL;
>       }
>   
> +    if ( config->flags & XEN_DOMCTL_CDF_hap && !hvm_hap_supported() )

The & wants to be parenthesized here.

> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -313,6 +313,13 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
>           return -EINVAL;
>       }
>   
> +    if ( !(config->flags & XEN_DOMCTL_CDF_hvm_guest) &&
> +         config->flags & XEN_DOMCTL_CDF_hap )

Same here.

> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -948,6 +948,11 @@ static inline bool is_hvm_domain(const struct domain *d)
>           evaluate_nospec(d->options & XEN_DOMCTL_CDF_hvm_guest);
>   }
>   
> +static inline bool hap_enabled(const struct domain *d)
> +{
> +    return evaluate_nospec(d->options & XEN_DOMCTL_CDF_hap);
> +}
> +
>   static inline bool is_hvm_vcpu(const struct vcpu *v)

Could this please not be placed between two closely related functions?

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

> -----Original Message-----
> From: Jan Beulich <JBeulich@suse.com>
> Sent: 30 July 2019 12:56
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; Julien Grall <julien.grall@arm.com>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; Roger Pau Monne <roger.pau@citrix.com>; George Dunlap
> <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Stefano Stabellini
> <sstabellini@kernel.org>; Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; Tim (Xen.org) <tim@xen.org>;
> Wei Liu <wl@xen.org>
> Subject: Re: [PATCH v3 2/5] x86/hvm/domain: remove the 'hap_enabled' flag
> 
> On 30.07.2019 11:39, Paul Durrant wrote:
> > --- a/xen/arch/x86/domain.c
> > +++ b/xen/arch/x86/domain.c
> > @@ -460,6 +460,12 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
> >           return -EINVAL;
> >       }
> >
> > +    if ( config->flags & XEN_DOMCTL_CDF_hap && !hvm_hap_supported() )
> 
> The & wants to be parenthesized here.
> 
> > --- a/xen/common/domain.c
> > +++ b/xen/common/domain.c
> > @@ -313,6 +313,13 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
> >           return -EINVAL;
> >       }
> >
> > +    if ( !(config->flags & XEN_DOMCTL_CDF_hvm_guest) &&
> > +         config->flags & XEN_DOMCTL_CDF_hap )
> 
> Same here.

Oh yes. Not sure how I missed those.

> 
> > --- a/xen/include/xen/sched.h
> > +++ b/xen/include/xen/sched.h
> > @@ -948,6 +948,11 @@ static inline bool is_hvm_domain(const struct domain *d)
> >           evaluate_nospec(d->options & XEN_DOMCTL_CDF_hvm_guest);
> >   }
> >
> > +static inline bool hap_enabled(const struct domain *d)
> > +{
> > +    return evaluate_nospec(d->options & XEN_DOMCTL_CDF_hap);
> > +}
> > +
> >   static inline bool is_hvm_vcpu(const struct vcpu *v)
> 
> Could this please not be placed between two closely related functions?
> 

Ok.

  Paul

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