| Message ID | 20190730093904.1794-3-paul.durrant@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | use stashed domain create flags | expand |
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
> -----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
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);
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(-)