> -----Original Message-----
> From: Andrew Cooper <andrew.coop...@citrix.com>
> Sent: 23 August 2019 13:26
> To: Paul Durrant <paul.durr...@citrix.com>; xen-devel@lists.xenproject.org
> Cc: Stefano Stabellini <sstabell...@kernel.org>; Wei Liu <w...@xen.org>;
> Konrad Rzeszutek Wilk
> <konrad.w...@oracle.com>; George Dunlap <george.dun...@citrix.com>; Tim
> (Xen.org) <t...@xen.org>; Ian
> Jackson <ian.jack...@citrix.com>; Julien Grall <julien.gr...@arm.com>; Jan
> Beulich
> <jbeul...@suse.com>; Roger Pau Monne <roger....@citrix.com>
> Subject: Re: [Xen-devel] [PATCH v6 02/10] x86/hvm/domain: remove the
> 'hap_enabled' flag
>
> On 23/08/2019 13:23, Andrew Cooper wrote:
> > On 16/08/2019 18:19, 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.
> >>
> >> NOTE: Furthermore, by extending sanitiziing of the domain 'options', the
> > s/ii/i/
Oh yes.
> >
> >> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> >> index 9a6eb89ddc..bc0db03387 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");
> > s/enabled/requested/
> >
I'm not fussed... I just went with the incumbent flag name.
> >> diff --git a/xen/common/domain.c b/xen/common/domain.c
> >> index 744b572195..6109623730 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");
> > Again, I think 'requested' would be better here.
> >
> >> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> >> index 2e6e0d3488..07a64947ed 100644
> >> --- a/xen/include/xen/sched.h
> >> +++ b/xen/include/xen/sched.h
> >> @@ -954,6 +954,12 @@ static inline bool is_hvm_vcpu(const struct vcpu *v)
> >> return is_hvm_domain(v->domain);
> >> }
> >>
> >> +static inline bool hap_enabled(const struct domain *d)
> >> +{
> >> + return IS_ENABLED(CONFIG_HVM) && /* necessary for pv shim build */
> >> + evaluate_nospec(d->options & XEN_DOMCTL_CDF_hap);
> > I'm not sure how helpful this comment is. What should be here however
> > is a note saying that this logic depends on domain_create() rejecting
> > !HVM and HAP.
> >
> > All can be adjusted on commit if there are no other concerns.
>
Ok.
> One other thing. Why is this eval_nospec()?
>
General paranoia about what might happen in speculation if the inline evaluates
false and we wander into e.g. shadow code.
Paul
> ~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
