From mboxrd@z Thu Jan  1 00:00:00 1970
From: Mukesh Rathor <mukesh.rathor@oracle.com>
Subject: Re: [PATCH 09/18] PVH xen: Support privileged op
 emulation for PVH
Date: Fri, 5 Jul 2013 18:43:18 -0700
Message-ID: <20130705184318.611dc5ae@mantra.us.oracle.com>
References: <1372118507-16864-1-git-send-email-mukesh.rathor@oracle.com>
	<1372118507-16864-10-git-send-email-mukesh.rathor@oracle.com>
	<51C980C902000078000E0445@nat28.tlf.novell.com>
	<20130626154130.6c50b347@mantra.us.oracle.com>
	<51CC046202000078000E106E@nat28.tlf.novell.com>
	<20130627164339.476d2ef3@mantra.us.oracle.com>
	<51CD718F02000078000E17B3@nat28.tlf.novell.com>
	<20130702183815.621e95f8@mantra.us.oracle.com>
	<51D4174002000078000E26E3@nat28.tlf.novell.com>
	<20130703190016.1a20e418@mantra.us.oracle.com>
	<51D548C002000078000E2A3A@nat28.tlf.novell.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: <51D548C002000078000E2A3A@nat28.tlf.novell.com>
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: Jan Beulich <JBeulich@suse.com>
Cc: xen-devel <xen-devel@lists.xen.org>
List-Id: xen-devel@lists.xenproject.org

On Thu, 04 Jul 2013 09:04:48 +0100
"Jan Beulich" <JBeulich@suse.com> wrote:

> >>> On 04.07.13 at 04:00, Mukesh Rathor <mukesh.rathor@oracle.com>
> >>> wrote:
> > On Wed, 03 Jul 2013 11:21:20 +0100 "Jan Beulich"
> > <JBeulich@suse.com> wrote:
> >> Even if it really is (which I doubt), you still would make PVH
> >> different from both PV and HVM, which both don't populate the
> >> selector fields of the frame (PV obviously has ->cs and ->ss
> >> populated [by the CPU], but HVM avoids even that).
> > 
> > And what's wrong with PVH being little different?
> 
> There's nothing wrong with this as long as it's for a useful purpose,
> and without introducing hidden dependencies (the latter is what is
> happening here). Being different just for the purpose of being
> different is not desirable (and likely not even acceptable, as in any
> case this makes code more difficult to understand).
> 
> >> We'll have to see - at the first glance I don't follow...
> > 
> > Here's what I am talking about:
> > 
> > --- a/xen/arch/x86/domain.c
> > +++ b/xen/arch/x86/domain.c
> > @@ -1241,10 +1241,10 @@ static void save_segments(struct vcpu *v)
> >      struct cpu_user_regs *regs = &v->arch.user_regs;
> >      unsigned int dirty_segment_mask = 0;
> >  
> > -    regs->ds = read_segment_register(v, regs, ds);
> > -    regs->es = read_segment_register(v, regs, es);
> > -    regs->fs = read_segment_register(v, regs, fs);
> > -    regs->gs = read_segment_register(v, regs, gs);
> > +    regs->ds = read_segment_register(v, regs, x86_seg_ds);
> > +    regs->es = read_segment_register(v, regs, x86_seg_es);
> > +    regs->fs = read_segment_register(v, regs, x86_seg_fs);
> > +    regs->gs = read_segment_register(v, regs, x86_seg_gs);
> 
> This I think is completely pointless a change if you keep the thing
> being a macro (using token concatenation):

I find the ## to be disgusting because it makes code much harder to
read/understand as one cannot use cscope/grep to find things. But,
I'll make the change as suggested.

thanks
Mukesh