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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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?

Jan


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