From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH RFC 1/3] xen/save: pass a size paramter to the HVM compat functions Date: Thu, 15 Oct 2015 11:31:07 +0100 Message-ID: <561F806B.3010202@citrix.com> References: <1444839880-15260-1-git-send-email-roger.pau@citrix.com> <1444839880-15260-2-git-send-email-roger.pau@citrix.com> <561F7BF702000078000AB3D4@prv-mh.provo.novell.com> <561F76AC.9090905@citrix.com> <561F952002000078000AB4D6@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 1ZmfpP-0000we-TV for xen-devel@lists.xenproject.org; Thu, 15 Oct 2015 10:32:08 +0000 In-Reply-To: <561F952002000078000AB4D6@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 , =?windows-1252?Q?Roger_Pau_Monn=E9?= Cc: xen-devel@lists.xenproject.org, Ian Jackson , Ian Campbell , Tim Deegan List-Id: xen-devel@lists.xenproject.org On 15/10/15 10:59, Jan Beulich wrote: >>>> On 15.10.15 at 11:49, wrote: >> El 15/10/15 a les 10.12, Jan Beulich ha escrit: >>>>>> On 14.10.15 at 18:24, wrote: >>>> In order to cope with types having multiple compat versions pass a parameter >>>> to the fixup function so we can identify which compat version Xen is dealing >>>> with. >>> Having peeked at patch 2, this won't help once another bit gets added >>> to the tail of that structure. Also it doesn't seem logical that the >>> previous compat handling got around without being passed the size >>> explicitly. I.e. while perhaps more involved, I think the compat >>> handling needs to be extended to allow for multiple versions. >> Yes, patch #2 needs to be fixed by making fpu_initialised an uint32_t. >> Adding versions would be the best option IMHO, but I don't think I have >> enough time to do that myself right now. > Understood. An I don't think we really need versioning here. > >>> Or, since we have this under control going forward, don't even declare >>> all the various compat structures in the public header (and only ever >>> add to the tail). Then staying with the passing of size probably makes >>> sense, but the fixup function then should use offsetof() instead of >>> sizeof() (and validate unused tail bits are zero, so they can be used for >>> something later on). >> Since we use hvm_load_entry_zeroextend I think we can assert that all >> new tail bits are going to be 0, but I'm not sure I follow you regarding >> the usage of offsetof instead of sizeof. What are we going to compare >> with offsetof? > Instead of introducing compat1 and compat2 structures, just add > the new field to the existing structure, and use offsetof() to compare > the passed in size with the that of the structure in its original state. > >> Also, we already have a compat version of hvm_hw_cpu that didn't add the >> new field to the end. > Right, but we can avoid making the same mistake again. Please be aware that, in due course, I will be replacing all of this, migration v2 style, as a prerequisite of getting cpuid policies functioning correctly during migrate. While we shouldn't paint ourselves into a corner, I wouldn't worry too much about their longevity. ~Andrew