From mboxrd@z Thu Jan  1 00:00:00 1970
From: Mukesh Rathor <mukesh.rathor@oracle.com>
Subject: Re: [V10 PATCH 12/23] PVH xen: Support privileged op
 emulation for PVH
Date: Thu, 8 Aug 2013 18:32:49 -0700
Message-ID: <20130808183249.17340c18@mantra.us.oracle.com>
References: <1374631171-15224-1-git-send-email-mukesh.rathor@oracle.com>
	<1374631171-15224-13-git-send-email-mukesh.rathor@oracle.com>
	<CAFLBxZaVJg6HO+JOeLy1o=my+QXkzYGqehhC3TS+VcSYpMCOJQ@mail.gmail.com>
	<20130807185941.56d3613d@mantra.us.oracle.com>
	<CAFLBxZbF1NTZe=Gx8BU0Oqe4eBckts5kZ7bT_9Y5Af69HK2yFw@mail.gmail.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: <CAFLBxZbF1NTZe=Gx8BU0Oqe4eBckts5kZ7bT_9Y5Af69HK2yFw@mail.gmail.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: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: "xen-devel@lists.xensource.com" <Xen-devel@lists.xensource.com>, "keir.xen@gmail.com" <keir.xen@gmail.com>, Jan Beulich <JBeulich@suse.com>
List-Id: xen-devel@lists.xenproject.org

On Thu, 8 Aug 2013 15:18:56 +0100
George Dunlap <George.Dunlap@eu.citrix.com> wrote:

> On Thu, Aug 8, 2013 at 2:59 AM, Mukesh Rathor
> <mukesh.rathor@oracle.com> wrote:
> > On Wed, 7 Aug 2013 14:49:50 +0100
> > George Dunlap <dunlapg@umich.edu> wrote:
> >
> >> On Wed, Jul 24, 2013 at 2:59 AM, Mukesh Rathor
> >> <mukesh.rathor@oracle.com> wrote:
.......
> The problem I have is that you still pass in *both* the value of
> regs->$SEGMENT_REGISTER, *and* an enum of a segment register, and use
> one in one case, and another in a different case.  That's just a
> really ugly interface.
> 
> What I'd like to see is for read_descriptor_sel() to *just* take
> which_sel (perhaps renamed sreg or something, since it's referring to
> a segment register), and in the PV case, read the appropriate segment
> register, then calling read_descriptor().  Then you don't have this
> crazy thing where you set two variables (sel and which_cs) all over
> the place.


Hmm... lemme make sure I understand precisely, what you mean is
something like:

static int read_descriptor_sel(enum x86_segment which_sel,
                               struct vcpu *v,
                               const struct cpu_user_regs *regs,
                               unsigned long *base,
                               unsigned long *limit,
                               unsigned int *ar,
                               unsigned int vm86attr)

{
    uint sel;
    if (!pvh)
    {
        sel = read_pv_segreg(which_sel)
        return read_descriptor(sel, v, regs, base, limit, ar, vm86attr);
    }
}

where read_pv_segreg() has one long case statment:
   case x86_seg_cs
       return read_segment_register(v, regs, cs);
   case x86_seg_cs
       return read_segment_register(v, regs, ds);
       .....


Then emulate_privileged_op() will not be setting data_sel, but 
only which_sel, except for one place:

....
            if ( lm_ovr == lm_seg_none || data_sel < 4 )
            {
                switch ( lm_ovr )
                {
                case lm_seg_none:
...

That sounds like a good change to me. Jan, you OK with this?


-mukesh