From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v2 15/30] xen/x86: Improvements to in-hypervisor cpuid sanity checks Date: Thu, 18 Feb 2016 12:17:22 +0000 Message-ID: <56C5B652.9050501@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> <56C44EBA.3090906@citrix.com> <56C45FBA02000078000D30D9@prv-mh.provo.novell.com> <56C47D7A.3070406@citrix.com> <56C4958002000078000D3361@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <56C4958002000078000D3361@prv-mh.provo.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Xen-devel List-Id: xen-devel@lists.xenproject.org On 17/02/16 14:45, Jan Beulich wrote: >>>> On 17.02.16 at 15:02, wrote: >> On 17/02/16 10:55, Jan Beulich wrote: >>>>>> On 17.02.16 at 11:43, wrote: >>>> On 16/02/16 10:06, Jan Beulich wrote: >>>>>>>> On 15.02.16 at 18:12, wrote: >>>>>> On 15/02/16 15:43, Jan Beulich wrote: >>>>>>>>>> On 05.02.16 at 14:42, 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= 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. 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