Re: [Xen-devel] [PATCH 1/2] x86/xstate: Use the CPUID policy in valid_xcr0(), rather than allowing all features

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.07.18 at 18:05, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 18/07/18 15:48, Jan Beulich wrote:
>>>>> On 18.07.18 at 14:30, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> Backporting notes: This is safe in the restore case, but only back as far as
>>> the introduction of cpuid_policy infrastructure.  Before then, a restore
>>> boolean needs to be pumbed down as well, and used to select between the
>>> hardware maximum value and calls to {hvm,pv}_cpuid() to find the
>>> toolstack-chosen level.
>> So can I take this to mean that for ...
>>
>>> --- a/xen/arch/x86/domctl.c
>>> +++ b/xen/arch/x86/domctl.c
>>> @@ -1170,7 +1170,7 @@ long arch_do_domctl(
>>>              if ( _xcr0_accum )
>>>              {
>>>                  if ( evc->size >= PV_XSAVE_HDR_SIZE + XSTATE_AREA_MIN_SIZE 
>>> )
>>> -                    ret = validate_xstate(_xcr0, _xcr0_accum,
>>> +                    ret = validate_xstate(d, _xcr0, _xcr0_accum,
>> ... this and ...
>>
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -1254,7 +1254,7 @@ static int hvm_load_cpu_xsave_states(struct domain 
>>> *d, 
> hvm_domain_context_t *h)
>>>      ctxt = (struct hvm_hw_cpu_xsave *)&h->data[h->cur];
>>>      h->cur += desc->length;
>>>  
>>> -    err = validate_xstate(ctxt->xcr0, ctxt->xcr0_accum,
>>> +    err = validate_xstate(d, ctxt->xcr0, ctxt->xcr0_accum,
>> ... this there's no ordering issue on master (i.e. policy arrives before
>> register state)?
> 
> You are correct.  In the case that the policy hasn't arrived on migrate,
> d->arch.cpuid is the applicable max policy.

Hmm, you say max policy, but the plan is to have max >= default.
Isn't it the default policy that's active before the respective migration
data arrives? In which case - won't this go wrong for domains with
a policy allowing more than default? Or are you/we assuming that
for the particular features here there's not going to be any way to
expose more than the default (IOW default == max for all of
XSTATE) for the foreseeable future (in which case this assumption
should be spelled out in the description I think)?

Jan



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