From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Jan Beulich" <jbeulich@suse.com>
Subject: Re: Is: PVH + ARM new hypercalls. Was: Re: [PATCH]:
 PVH: specify xen features strings cleany for PVH
Date: Fri, 01 Feb 2013 16:24:57 +0000
Message-ID: <510BEC590200007800094177@nat28.tlf.novell.com>
References: <50FD3D6202000078000B7CE4@nat28.tlf.novell.com>
	<20130122151241.7ed034f4@mantra.us.oracle.com>
	<50FFABE702000078000B88E1@nat28.tlf.novell.com>
	<20130123144347.78ba0a3f@mantra.us.oracle.com>
	<51010A1802000078000B9027@nat28.tlf.novell.com>
	<20130124151336.311c3456@mantra.us.oracle.com>
	<51024A2602000078000B9786@nat28.tlf.novell.com>
	<1359108703.10051.5.camel@zakaz.uk.xensource.com>
	<51026C3602000078000B9911@nat28.tlf.novell.com>
	<1359110621.10051.25.camel@zakaz.uk.xensource.com>
	<20130128162607.GA7223@konrad-lan.dumpdata.com>
	<20130128185737.55ec65f8@mantra.us.oracle.com>
	<5107B6FC02000078000BA589@nat28.tlf.novell.com>
	<20130131182329.2cb3a225@mantra.us.oracle.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <20130131182329.2cb3a225@mantra.us.oracle.com>
Content-Disposition: inline
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: mukesh.rathor@oracle.com
Cc: konrad@kernel.org, konrad.wilk@oracle.com, Ian.Campbell@citrix.com, xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

>>> Mukesh Rathor <mukesh.rathor@oracle.com> 02/01/13 3:23 AM >>>
>On Tue, 29 Jan 2013 10:48:12 +0000 "Jan Beulich" <JBeulich@suse.com> wrote:
>> >>> On 29.01.13 at 03:57, Mukesh Rathor <mukesh.rathor@oracle.com>
>> >>> wrote:
>> > On xen side I added the ifdef:
>> > 
>> > #if __XEN_INTERFACE_VERSION__ < 0x00040300
>> >     unsigned long gdt_frames[16], gdt_ents; /* GDT (machine frames,
>> > # ents) */
>> > #else
>> >     union {
>> >         struct {
>> >             /* GDT (machine frames, # ents) */
>> >             unsigned long gdt_frames[16], gdt_ents;
>> >         } pv;
>> >         struct {
>> >             /* PVH: GDTR addr and size */
>> >             unsigned long gdtaddr, gdtsz;
>> >         } pvh;
>> >     } u;
>> > #endif
>> > 
>> > but it doesn't matter on linux side, so up to you.
>> 
>> But I'd still prefer for this to go away again - you could simply use
>> gdt_frames[0] for gdtaddr and gdt_ents for the (normalized)
>> gdtsz.
>
>That was my patch version 1 during linux patch review. Then the reviewer
>suggested to make it a union.
>
>> And if you nevertheless go the union route, call it "gdt" instead
>> of "u" and drop the gdt/gdt_ prefixes from the member names
>> (yes, I know, grepping and cscoping for such member is more
>> difficult, but I continue to see more advantage in avoiding the
>> redundancy).
>
>That was my patch version 2, where I called it gdt and another reviewer
>suggested to change to u. So I changed it to u.
>
>It's gone thru enough iterations that I'd like to leave as is. Thank
>you in advance for your compromise in helping us mortals grep/cscope 
>to learn code.

That's part of the reason why I said from the beginning that doing the Linux
side first is wrong.

Jan