From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel De Graaf Subject: Re: [PATCH 1/6] xen: use domid check in is_hardware_domain Date: Wed, 05 Mar 2014 16:23:17 -0500 Message-ID: <531795C5.10805@tycho.nsa.gov> References: <1393973494-29411-1-git-send-email-dgdegra@tycho.nsa.gov> <1393973494-29411-2-git-send-email-dgdegra@tycho.nsa.gov> <5316FB0E02000078001211A9@nat28.tlf.novell.com> <531741FF.1000509@tycho.nsa.gov> <531754A802000078001213CE@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <531754A802000078001213CE@nat28.tlf.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: Keir Fraser , Ian Campbell , Tim Deegan , xen-devel@lists.xen.org, Stefano Stabellini , Suravee Suthikulpanit , Xiantao Zhang List-Id: xen-devel@lists.xenproject.org On 03/05/2014 10:45 AM, Jan Beulich wrote: >>>> On 05.03.14 at 16:25, Daniel De Graaf wrote: >> On 03/05/2014 04:23 AM, Jan Beulich wrote: >>>>>> On 04.03.14 at 23:51, Daniel De Graaf wrote: >>>> --- a/xen/arch/x86/domain.c >>>> +++ b/xen/arch/x86/domain.c >>>> @@ -1505,7 +1505,7 @@ void context_switch(struct vcpu *prev, struct vcpu *next) >>>> } >>>> >>>> set_cpuid_faulting(is_pv_vcpu(next) && >>>> - (next->domain->domain_id != 0)); >>>> + !is_control_domain(next->domain)); >>> >>> I can't see why the hardware domain (which can't be migrated) >>> should be prevented from seeing the real CPUID values. It's >>> less obvious whether a control domain should always see real >>> values. Hence if you think the latter should be the case (as the >>> change above shows), I think you should check for both cases >>> here. >> >> Agreed, the hardware domain also needs to be checked for here. The >> reason the control domain is present is that it needs to see real >> CPUID values in order to set CPUID policy for guests based on the >> real hardware values. > > So don't envision a staged system where the root domain hides > some features from creation-capable ones, and those may hide > further features from their descendants? Up to where even the > controlling domains might become migratable? Perhaps is_control_domain should be renamed to is_root_control_domain; it is not necessary for every control domain to have is_control_domain return true. In fact, the domain builder I posted does not set the is_privileged field on any guest it creates, and so once it shuts down, there are no domains that the hypervisor considers control domains. The XSM policy governs which domains are permitted to create, pause, and do all the usual "control" operations. A quick grep actually seems to point out that is_control_domain could easily be removed, as the only references that remain are these CPUID fields. This would end up simplifying the disaggregation model a bit. >> Using is_hardware_domain here avoids that problem (as the hardware domain >> may never shut down or be destroyed), so that may be the simplest >> solution until a better model of who is responsible for profiling is >> presented. > > Then please do so, with a short note to that effect in either the > description or a code comment. > > Jan Right, this change and a comment will be in the v2 when I post it. -- Daniel De Graaf National Security Agency