Re: [Xen-devel] [PATCH 07/27] x86/cpuid: Recalculate a domains CPUID policy when appropriate

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.01.17 at 15:42, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 05/01/17 08:24, Jan Beulich wrote:
>>>>> On 04.01.17 at 18:37, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 04/01/17 16:04, Jan Beulich wrote:
>>>>>>> On 04.01.17 at 16:33, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>>> On 04/01/17 15:01, Jan Beulich wrote:
>>>>>>>>> On 04.01.17 at 13:39, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>>>>>  static void update_domain_cpuid_info(struct domain *d,
>>>>>>>                                       const xen_domctl_cpuid_t *ctl)
>>>>>>>  {
>>>>>>> +    struct cpuid_policy *p = d->arch.cpuid;
>>>>>>> +    struct cpuid_leaf leaf = { ctl->eax, ctl->ebx, ctl->ecx, ctl->edx 
>>>>>>> };
>>>>>>> +
>>>>>>> +    if ( ctl->input[0] < ARRAY_SIZE(p->basic.raw) )
>>>>>>> +    {
>>>>>>> +        if ( ctl->input[0] == 7 )
>>>>>>> +        {
>>>>>>> +            if ( ctl->input[1] < ARRAY_SIZE(p->feat.raw) )
>>>>>>> +                p->feat.raw[ctl->input[1]] = leaf;
>>>>>>> +        }
>>>>>>> +        else if ( ctl->input[0] == 0xd )
>>>>>>> +        {
>>>>>>> +            if ( ctl->input[1] < ARRAY_SIZE(p->xstate.raw) )
>>>>>>> +                p->xstate.raw[ctl->input[1]] = leaf;
>>>>>>> +        }
>>>>>>> +        else
>>>>>>> +            p->basic.raw[ctl->input[0]] = leaf;
>>>>>>> +    }
>>>>>>> +    else if ( (ctl->input[0] - 0x80000000) < ARRAY_SIZE(p->extd.raw) )
>>>>>>> +        p->extd.raw[ctl->input[0] - 0x80000000] = leaf;
>>>>>> These checks against ARRAY_SIZE() worry me - wouldn't we better
>>>>>> refuse any attempts to set values not representable in the policy?
>>>>> We can't do that yet, without toolstack side changes.  Currently the
>>>>> toolstack can lodge any values it wishes, and all we do is ignore them,
>>>>> which can be arbitrary information from a cpuid= clause.
>>>> Hmm, do we really _ignore_ them in all cases (rather than handing
>>>> them through to guests)? If so, that should indeed be good enough
>>>> for now.
>>> Any arbitrary values get can get inserted into the cpuids[] array but,
>>> given your fairly-recent change to check max_leaf, we don't guarantee to
>>> hand the values to a guest.
>> "we don't guarantee" != "we guarantee not to"
>>
>> But my main point here is that a domain's cpuid= may specify a
>> higher than default max leaf, and I think going forward we ought
>> to still return all zero for those leaves in that case, or else the
>> overall spirit of white listing would get violated.
> 
> Does this concern still stand in light of max_leaf handling in patches
> 21 and 22?

Indeed, now that I've seen the full series, this should be fine.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel