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/
>
>> 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/
>
>> 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.

One other thing.  Why is this eval_nospec()?

~Andrew

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

Reply via email to