From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?Q?Roger_Pau_Monn=c3=a9?= Subject: Re: [PATCH RFC 1/3] xen/save: pass a size paramter to the HVM compat functions Date: Thu, 15 Oct 2015 11:49:32 +0200 Message-ID: <561F76AC.9090905@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> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZmfAJ-0005zf-GM for xen-devel@lists.xenproject.org; Thu, 15 Oct 2015 09:49:39 +0000 In-Reply-To: <561F7BF702000078000AB3D4@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@lists.xenproject.org, Ian Jackson , Ian Campbell , Tim Deegan List-Id: xen-devel@lists.xenproject.org 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. > 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? Also, we already have a compat version of hvm_hw_cpu that didn't add the new field to the end. Roger.