From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH v12 for-xen-4.5 01/20] common/symbols: Export hypervisor symbols to privileged guest Date: Mon, 29 Sep 2014 09:29:06 -0400 Message-ID: <54295EA2.1030800@oracle.com> References: <1411673336-32736-1-git-send-email-boris.ostrovsky@oracle.com> <1411673336-32736-2-git-send-email-boris.ostrovsky@oracle.com> <20140926145812.GB14378@laptop.dumpdata.com> <54259DF10200007800039C83@mail.emea.novell.com> <20140926164903.GA3927@laptop.dumpdata.com> <54291BAE020000780003A3AD@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <54291BAE020000780003A3AD@mail.emea.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 , Konrad Rzeszutek Wilk Cc: kevin.tian@intel.com, keir@xen.org, suravee.suthikulpanit@amd.com, andrew.cooper3@citrix.com, tim@xen.org, dietmar.hahn@ts.fujitsu.com, xen-devel@lists.xen.org, Aravind.Gopalakrishnan@amd.com, jun.nakajima@intel.com, dgdegra@tycho.nsa.gov List-Id: xen-devel@lists.xenproject.org On 09/29/2014 02:43 AM, Jan Beulich wrote: >>>> On 26.09.14 at 18:49, wrote: >> On Fri, Sep 26, 2014 at 04:10:09PM +0100, Jan Beulich wrote: >>>>>> On 26.09.14 at 16:58, wrote: >>>> If I move them just a bit: >>>> >>>> >>>> diff --git a/xen/include/public/platform.h b/xen/include/public/platform.h >>>> index 4f21b17..b97e476 100644 >>>> --- a/xen/include/public/platform.h >>>> +++ b/xen/include/public/platform.h >>>> @@ -538,9 +538,9 @@ struct xenpf_symdata { >>>> /* we reached the end */ >>>> >>>> /* OUT variables */ >>>> - char type; >>>> - XEN_GUEST_HANDLE(char) name; >>>> uint64_t address; >>>> + XEN_GUEST_HANDLE(char) name; >>>> + char type; >>>> }; >>>> typedef struct xenpf_symdata xenpf_symdata_t; >>>> DEFINE_XEN_GUEST_HANDLE(xenpf_symdata_t); >>>> >>>> >>>> 'pahole' is satisfied: >>>> >>>> struct xenpf_symdata { >> >>>> uint32_t namelen; /* 0 4 */ >> >>>> uint32_t symnum; /* 4 4 */ >> >>>> uint64_t address; /* 8 8 */ >> >>>> __guest_handle_char name; /* 16 8 */ >> >>>> char type; /* 24 1 */ >> >>>> >> >>>> /* size: 32, cachelines: 1, members: 5 */ >> >>>> /* padding: 7 */ >> >>>> /* last cacheline: 32 bytes */ >> >>>> }; >>>> >>>> >>>> With that change, Reviewed-by: Konrad Rzeszutek Wilk >>> This change buys us exactly nothing: Structure size doesn't change, >>> and 7 bytes of padding are still there. >> It does allow us to put more parameters (if we want to) at the end of the >> structure instead of fitting them in between. > Regardless of where the gap is, adding further fields in the future > would work only if the code now checked that this field is zero > (which first of all would require it being given a name). I keep > pointing out that this should be done for all padding fields, but I'm > afraid I may have missed doing so on this occasion. I am not sure I understand how setting fields to zero would help with figuring out whether a new fields has been added. I can see how it can in some cases but not in general. -boris