From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1aacbj-0008Fz-F8 for mharc-grub-devel@gnu.org; Tue, 01 Mar 2016 00:12:27 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59221) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aacbg-0008Ff-0R for grub-devel@gnu.org; Tue, 01 Mar 2016 00:12:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aacbb-00061G-0T for grub-devel@gnu.org; Tue, 01 Mar 2016 00:12:23 -0500 Received: from mx2.suse.de ([195.135.220.15]:59316) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aacba-000616-Le for grub-devel@gnu.org; Tue, 01 Mar 2016 00:12:18 -0500 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 0C9A6AD16; Tue, 1 Mar 2016 05:12:17 +0000 (UTC) Subject: Re: [Xen-devel] [PATCH v4 10/11] xen: modify page table construction To: Andrei Borzenkov , The development of GNU GRUB , Daniel Kiper References: <1456120999-5639-1-git-send-email-jgross@suse.com> <1456120999-5639-11-git-send-email-jgross@suse.com> <20160222091748.GO3482@olila.local.net-space.pl> <56CAD4E0.80304@suse.com> <20160222121858.GQ3482@olila.local.net-space.pl> <56CAFF66.1080709@suse.com> <20160222124805.GS3482@olila.local.net-space.pl> <56CB09AE.3040301@suse.com> <56CF4903.6020304@gmail.com> <56D40BB6.6090904@suse.com> <56D4374F.5050209@suse.com> <56D511EA.4000503@gmail.com> From: Juergen Gross Message-ID: <56D524AD.4000107@suse.com> Date: Tue, 1 Mar 2016 06:12:13 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <56D511EA.4000503@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x (no timestamps) [generic] X-Received-From: 195.135.220.15 Cc: phcoder@gmail.com, mchang@suse.com, xen-devel@lists.xen.org X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 01 Mar 2016 05:12:25 -0000 On 01/03/16 04:52, Andrei Borzenkov wrote: > 29.02.2016 15:19, Juergen Gross =D0=BF=D0=B8=D1=88=D0=B5=D1=82: >> On 29/02/16 10:13, Juergen Gross wrote: >>> On 25/02/16 19:33, Andrei Borzenkov wrote: >>>> 22.02.2016 16:14, Juergen Gross =D0=BF=D0=B8=D1=88=D0=B5=D1=82: >>>>> On 22/02/16 13:48, Daniel Kiper wrote: >>>>>> On Mon, Feb 22, 2016 at 01:30:30PM +0100, Juergen Gross wrote: >>>>>>> On 22/02/16 13:18, Daniel Kiper wrote: >>>>>>>> On Mon, Feb 22, 2016 at 10:29:04AM +0100, Juergen Gross wrote: >>>>>>>>> On 22/02/16 10:17, Daniel Kiper wrote: >>>>>>>>>> On Mon, Feb 22, 2016 at 07:03:18AM +0100, Juergen Gross wrote: >>>>>>>>>>> diff --git a/grub-core/lib/xen/relocator.c b/grub-core/lib/xe= n/relocator.c >>>>>>>>>>> index 8f427d3..a05b253 100644 >>>>>>>>>>> --- a/grub-core/lib/xen/relocator.c >>>>>>>>>>> +++ b/grub-core/lib/xen/relocator.c >>>>>>>>>>> @@ -29,6 +29,11 @@ >>>>>>>>>>> >>>>>>>>>>> typedef grub_addr_t grub_xen_reg_t; >>>>>>>>>>> >>>>>>>>>>> +struct grub_relocator_xen_paging_area { >>>>>>>>>>> + grub_xen_reg_t start; >>>>>>>>>>> + grub_xen_reg_t size; >>>>>>>>>>> +}; >>>>>>>>>>> + >>>>>>>>>> >>>>>>>>>> ... this should have GRUB_PACKED because compiler may >>>>>>>>>> add padding to align size member. >>>>>>>>> >>>>>>>>> Why would the compiler add padding to a structure containing tw= o items >>>>>>>>> of the same type? I don't think the C standard would allow this= . >>>>>>>>> >>>>>>>>> grub_xen_reg_t is either unsigned (32 bit) or unsigned long (64= bit). >>>>>>>>> There is no way this could require any padding. >>>>>>>> >>>>>>>> You are right but we should add this here just in case. >>>>>>> >>>>>>> Sorry, I don't think this makes any sense. The C standard is very= clear >>>>>>> in this case: a type requiring a special alignment has always a l= ength >>>>>>> being a multiple of that alignment. Otherwise arrays wouldn't wor= k. >>>>>> >>>>>> Sorry, I am not sure what do you mean by that. >>>>> >>>>> The size of any C type (no matter whether it is an integral type li= ke >>>>> "int" or a structure) has always the same alignment restriction as = the >>>>> type itself. So a type requiring 8 byte alignment will always have = a >>>>> size of a multiple of 8 bytes. This is mandatory for arrays to work= , as >>>>> otherwise either the elements wouldn't be placed consecutively in m= emory >>>>> or the alignment restrictions wouldn't be obeyed for all elements. >>>>> >>>> >>>> I too not follow how it is relevant to this case. We talk about inte= rnal >>>> padding between structure members, not between array elements. >>>> >>>>> For our case it means that two structure elements of the same type = will >>>>> never require a padding between them, thus the annotation with "pac= ked" >>>>> can't serve any purpose. >>>>> >>>> >>>> Well, I am not aware of any requirement. Compiler may add arbitrary >>>> padding between structure elements; it is only prohibited to add pad= ding >>>> at the beginning. Sure, it would be unusual, but never say "never" .= .. >>>> also should Xen ever be ported to architecture where types are not >>>> self-aligned it will become an issue. >>> >>> So you are telling me that _all_ interfaces between e.g. Linux, grub2= , >>> Xen and all wire protocols not attributed with "packed" are just wron= g? >>> >>> Sorry, I don't think this is true. >> >> Okay, just found a reference: The x86 ABI states: >> >> Aggregates and Unions >> --------------------- >> Structures and unions assume the alignment of their most strictly >> aligned component. Each member is assigned to the lowest available >> offset with the appropriate alignment. The size of any object is alway= s >> a multiple of the object=E2=80=98s alignment. >> >> I don't think any x86 C-compiler will violate the x86 ABI. >> >=20 > Thank you! I was not really objecting, more thinking loud, because I > missed such explicit statemrnt. Could you post link? http://www.x86-64.org/documentation/abi.pdf Chapter 3.1.2, page 13 Juergen