| Message ID | 20190725133920.40673-4-paul.durrant@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | stash domain create flags and then use them | expand |
On Thu, Jul 25, 2019 at 02:39:17PM +0100, Paul Durrant wrote: > The hap_enabled() macro can determine whether the feature is available > using the domain 'options'; there is no need for a separate flag. > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Thanks, Roger.
On 25.07.2019 15:39, Paul Durrant wrote: > @@ -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->options & XEN_DOMCTL_CDF_hap)) Rather than adding yet another && here I think you want to reject XEN_DOMCTL_CDF_hap in arch_sanitise_domain_config() when !hvm_hap_supported(). Similarly the is_hvm_domain() could then also be eliminated by checking that CDF_hap is set only together with CDF_hvm (or by clearing CDF_hap if CDF_hvm is clear - depends on what compatibility needs there are. This would then also eliminate the double evaluation of "d". Jan
> -----Original Message----- > From: Jan Beulich <JBeulich@suse.com> > Sent: 29 July 2019 15:36 > To: Paul Durrant <Paul.Durrant@citrix.com> > Cc: xen-devel@lists.xenproject.org; Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monne > <roger.pau@citrix.com>; Wei Liu <wl@xen.org> > Subject: Re: [PATCH v2 3/6] x86/hvm/domain: remove the 'hap_enabled' flag > > On 25.07.2019 15:39, Paul Durrant wrote: > > @@ -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->options & XEN_DOMCTL_CDF_hap)) > > Rather than adding yet another && here I think you want to > reject XEN_DOMCTL_CDF_hap in arch_sanitise_domain_config() > when !hvm_hap_supported(). Similarly the is_hvm_domain() > could then also be eliminated by checking that CDF_hap is > set only together with CDF_hvm (or by clearing CDF_hap if > CDF_hvm is clear - depends on what compatibility needs > there are. > > This would then also eliminate the double evaluation of > "d". Oh yes, that's neater. I'll do that, and also fix up patch #2. Paul > > Jan
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..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/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h index 6c7c4f5aa6..7897e36b94 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->options & 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
The hap_enabled() macro can determine whether the feature is available using the domain 'options'; 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> v2: - Defer changes to shadow_domain_init() to patch #4 --- xen/arch/x86/domain.c | 7 +------ xen/arch/x86/mm/paging.c | 4 ++-- xen/include/asm-x86/hvm/domain.h | 9 +++------ xen/include/asm-x86/paging.h | 2 +- 4 files changed, 7 insertions(+), 15 deletions(-)