From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v3 4/4] x86/PV: enable the emulated PIT Date: Mon, 18 Jan 2016 09:29:11 +0000 Message-ID: <569CB067.7070905@citrix.com> References: <5699358D02000078000C7800@prv-mh.provo.novell.com> <1452879951-76391-1-git-send-email-roger.pau@citrix.com> <569CA5CB02000078000C7C43@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1aL67j-0005bD-1m for xen-devel@lists.xenproject.org; Mon, 18 Jan 2016 09:29:19 +0000 In-Reply-To: <569CA5CB02000078000C7C43@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 , Roger Pau Monne Cc: Ian Jackson , Wei Liu , Ian Campbell , xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org On 18/01/2016 07:43, Jan Beulich wrote: >>>> On 15.01.16 at 18:45, wrote: >> Changes since v2: >> - Change 'if ( (a && b) || (!a && c) )' into 'if ( a ? b : c )'. > Thanks, but after some more thinking about it I'm afraid there are > a few more aspects to consider here: > >> --- a/xen/arch/x86/domain.c >> +++ b/xen/arch/x86/domain.c >> @@ -542,8 +542,9 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags, >> d->domain_id, config->emulation_flags); >> return -EINVAL; >> } >> - if ( config->emulation_flags != 0 && >> - (!is_hvm_domain(d) || config->emulation_flags != XEN_X86_EMU_ALL) ) >> + if ( is_hvm_domain(d) ? (config->emulation_flags != XEN_X86_EMU_ALL && >> + config->emulation_flags != 0) : >> + (config->emulation_flags != XEN_X86_EMU_PIT) ) >> { > For one I think it would be a good idea to allow zero for PV domains, > and perhaps even default new DomU-s to have the PIT flag clear. > (Also - indentation.) > > Which gets us to the second, broader issue: These flags shouldn't > be forced to a particular value during migration, but instead they > should be part of the state getting migrated. Incoming domains > then would - if the field is missing due to coming from an older > hypervisor - have the flag default to 1. There is sadly another ratsnest here. These values are needed for domain creation, which means that putting them anywhere in the migration stream is already too late, as the domain has been created before the stream header is read. In principle, the best which could occur is that a value gets stashed in the stream and used as a sanity check. That will at least catch the case when they are different. I was planning to do exactly the same for the vcpu count etc. as part of my further cpuid work. However, there is a substantial quantity of development work required to make this function in a rational way. For now, I wouldn't worry too much. It is just one of a very large number of things which should be moved on migrate, but isn't. (Most notably, the cpuid policy.) ~Andrew