Re: [Xen-devel] [PATCH v2 15/30] xen/x86: Improvements to in-hypervisor cpuid sanity checks

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.02.16 at 13:17, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 17/02/16 14:45, Jan Beulich wrote:
>>>>> On 17.02.16 at 15:02, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 17/02/16 10:55, Jan Beulich wrote:
>>>>>>> On 17.02.16 at 11:43, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>>> On 16/02/16 10:06, Jan Beulich wrote:
>>>>>>>>> On 15.02.16 at 18:12, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>>>>> On 15/02/16 15:43, Jan Beulich wrote:
>>>>>>>>>>> On 05.02.16 at 14:42, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>>>>>>> @@ -4617,50 +4618,39 @@ void hvm_cpuid(unsigned int input, unsigned 
>>>>>>>>> int *eax, 
> unsigned int *ebx,
>>>>>>>>>          /* Fix up VLAPIC details. */
>>>>>>>>>          *ebx &= 0x00FFFFFFu;
>>>>>>>>>          *ebx |= (v->vcpu_id * 2) << 24;
>>>>>>>>> +
>>>>>>>>> +        *ecx &= hvm_featureset[FEATURESET_1c];
>>>>>>>>> +        *edx &= hvm_featureset[FEATURESET_1d];
>>>>>>>> Looks like I've overlooked an issue in patch 11, which becomes
>>>>>>>> apparent here: How can you use a domain-independent featureset
>>>>>>>> here, when features vary between HAP and shadow mode guests?
>>>>>>>> I.e. in the earlier patch I suppose you need to calculate two
>>>>>>>> hvm_*_featureset[]s, with the HAP one perhaps empty when
>>>>>>>> !hvm_funcs.hap_supported.
>>>>>>> Their use here is a halfway house between nothing and the planned full
>>>>>>> per-domain policies.
>>>>>>>
>>>>>>> In this case, the "don't expose $X to a non-hap domain" checks have been
>>>>>>> retained, to cover the difference.
>>>>>> Well, doesn't it seem to you that doing only half of the HAP/shadow
>>>>>> separation is odd/confusing? I.e. could I talk you into not doing any
>>>>>> such separation (enforcing the non-HAP overrides as is done now)
>>>>>> or finishing the separation to become visible/usable here?
>>>>> The HAP/shadow distinction is needed in the toolstack to account for the
>>>>> hap=<bool> option.
>>>>>
>>>>> The distinction will disappear when per-domain policies are introduced. 
>>>>> If you notice, the distinction is private to the data generated by the
>>>>> autogen script, and does not form a part of any API/ABI.  The sysctl
>>>>> only has a single hvm featureset.
>>>> I don't see this as being in line with
>>>>
>>>>     hvm_featuremask = hvm_funcs.hap_supported ?
>>>>         hvm_hap_featuremask : hvm_shadow_featuremask;
>>>>
>>>> in patch 11. A shadow mode guest should see exactly the same
>>>> set of features, regardless of whether HAP was available (and
>>>> enabled) on a host.
>>> A shadow mode guest will see the same features, independently of whether
>>> HAP was available.
>> I'm afraid I'm being dense: Either the guest sees the same features,
>> which to me implies both of hvm_{hap,shadow}_featuremask are
>> identical, or the two masks are different, resulting in different guest
>> feature masks (and hence different guest features) depending on
>> HAP availability. What am I missing?
> 
> A guest booted with hap and a guest booted with shadow will see
> different features when booted on the same host.  Hap includes 1GB
> superpages, PCID, etc.

Okay, now I (think I) at least understand the background. What
I then still don't understand is why you don't - in the code still
visible above - use a HAP/shadow dependent mask, thus taking
care of those differences instead of elsewhere doing dynamic
adjustments.

Jan

> The problem comes with a shadow guest booted on a hap-capable host. 
> Such a guest can safely be migrated to a non-hap capable host, but only
> if the toolstack knows that the guest saw a reduced featureset.
> 
> As there is still no interface to query what a guest can actually see
> (depends on full per-domain policys and no dynamic hiding), the shadow
> featuremask is used by the toolstack as a promise of what the Xen
> dynamic checks will do.
> 
> ~Andrew




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