Re: [Xen-devel] [PATCH 2/4] x86/cpuid: Store the toolstacks choice of hypervisor max leaf

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 13/01/17 12:00, Jan Beulich wrote:
>>>> On 13.01.17 at 12:45, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 13/01/17 11:32, Jan Beulich wrote:
>>>>>> On 11.01.17 at 18:44, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>> --- a/xen/arch/x86/domctl.c
>>>> +++ b/xen/arch/x86/domctl.c
>>>> @@ -136,6 +136,14 @@ static int update_domain_cpuid_info(struct domain *d,
>>>>              p->basic.raw[ctl->input[0]] = leaf;
>>>>          break;
>>>>  
>>>> +    case 0x40000000:
>>>> +        p->hv_limit = ctl->eax;
>>>> +        break;
>>>> +
>>>> +    case 0x40000100:
>>>> +        p->hv2_limit = ctl->eax;
>>>> +        break;
>>> Generally the patch is fine, but do we really need to store two limits
>>> here?
>> Currently yes, because that is the ABI the toolstack expects.
>>
>>> Can't we rely on the Viridian flag to be set first, so we could
>>> use it here?
>> You objected to implicit assumptions like this before.  I think it might
>> be safe given the current ordering in libxl and xenguest, but...
> Indeed I did, but there are things which clearly ought to be set
> very early, and others where the ordering can be any. It's those
> latter cases where I don't think we should expect certain tool
> stack behavior, whereas for the former I think we should not
> only require it, but even aim at enforcing it (by rejecting the
> operation if it is being done too late, because it would alter
> decisions taken in the past).

By the time I am done with the toolstack side, setting a CPUID policy
will have to come before allocating any RAM, so we have a suitable
maxphysaddr to audit against.

>
>>> And if indeed we can't right now, should it not be made so as a first step?
>> .. I plan to rework this all differently when altering the toolstack
>> half of CPUID.
>>
>> Several Viridian and Xen fields work like plain feature words, and I
>> want the toolstack to control those in the same manner as other CPUID
>> information, even down to selecting which APIs/ABIs to make visible by
>> choosing the entirety of leaf 0x40000x00.  In particular, this gives us
>> a debugging mechanism previously lacking to actually hide the Xen leaves
>> entirely (We are the only hypervisor which doesn't have this option).
>>
>> For now, I'd prefer to avoid making unnecessary toolstack changes.
> Fair enough.
>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks.

~Andrew

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