From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH v2 15/30] xen/x86: Improvements to
 in-hypervisor cpuid sanity checks
Date: Wed, 17 Feb 2016 10:43:06 +0000
Message-ID: <56C44EBA.3090906@citrix.com>
References: <1454679743-18133-1-git-send-email-andrew.cooper3@citrix.com>
	<1454679743-18133-16-git-send-email-andrew.cooper3@citrix.com>
	<56C2003902000078000D23AC@prv-mh.provo.novell.com>
	<56C2070D.10803@citrix.com>
	<56C302C402000078000D2852@prv-mh.provo.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <56C302C402000078000D2852@prv-mh.provo.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: Xen-devel <xen-devel@lists.xen.org>
List-Id: xen-devel@lists.xenproject.org

On 16/02/16 10:06, Jan Beulich wrote:
>>>> On 15.02.16 at 18:12, <andrew.cooper3@citrix.com> wrote:
>> On 15/02/16 15:43, Jan Beulich wrote:
>>>>>> On 05.02.16 at 14:42, <andrew.cooper3@citrix.com> 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.

>>>> +    case 0x80000007:
>>>> +        d &= pv_featureset[FEATURESET_e7d];
>>>> +        break;
>>> By not clearing eax and ebx (not sure about ecx) here you would
>>> again expose flags to guests without proper white listing.
>>>
>>>> +    case 0x80000008:
>>>> +        b &= pv_featureset[FEATURESET_e8b];
>>>>          break;
>>> Same here for ecx and edx and perhaps the upper 8 bits of eax.
>> Both of these would be changes to how these things are currently
>> handled, whereby a guest gets to see whatever the toolstack managed to
>> find in said leaf.  I was hoping to put off some of these decisions, but
>> they probably need making now.  On the PV side they definitely can't be
>> fully hidden, as these leaves are not maskable.
> Right, but many are meaningful primarily to kernels, and there we
> can hide them.
>
> Since you're switching from black to white listing here, I also think
> we need a default label alongside the "unsupported" one here.
> Similarly I would think XSTATE sub-leaves beyond 63 need hiding
> now.

I would prefer not to do that now.  It is conflated with the future
work, deliberately deferred to make this work a manageable size.

At the moment, the toolstack won't ever generate subleaves beyond 63.

~Andrew