From mboxrd@z Thu Jan  1 00:00:00 1970
From: Mukesh Rathor <mukesh.rathor@oracle.com>
Subject: Re: [PATCH 10/17] PVH xen: introduce vmx_pvh.c and pvh.c
Date: Fri, 3 May 2013 18:40:12 -0700
Message-ID: <20130503184012.0826a62d@mantra.us.oracle.com>
References: <1366752366-16594-1-git-send-email-mukesh.rathor@oracle.com>
	<1366752366-16594-11-git-send-email-mukesh.rathor@oracle.com>
	<5177B85B02000078000D03CA@nat28.tlf.novell.com>
	<20130501181704.7619ede4@mantra.us.oracle.com>
	<5182297E02000078000D26E3@nat28.tlf.novell.com>
	<20130502174010.067b551d@mantra.us.oracle.com>
	<5183766E02000078000D2EF5@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: <5183766E02000078000D2EF5@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 Fri, 03 May 2013 07:33:50 +0100
"Jan Beulich" <JBeulich@suse.com> wrote:

> >>> On 03.05.13 at 02:40, Mukesh Rathor <mukesh.rathor@oracle.com>
> >>> wrote:
> > On Thu, 02 May 2013 07:53:18 +0100
> > "Jan Beulich" <JBeulich@suse.com> wrote:
> > 
> >     if ( (tmp_ar & X86_SEG_AR_CS_LM_ACTIVE) && selector <
> > x86_seg_fs  )
> 
> This is still wrong. As said before you need to look as the _CS_
> access rights, not the ones of the selector register you read.

Hmm... unless I'm reading the SDM wrong, it says "for non-code segments
bit 21 is reserved and should always be set to 0". But its prob clearer
to check for _CS_ only. 

> But as also hinted at - do you really need the override at all?

Yes, because of the following check in insn_fetch macro:

     "(eip) > (limit) - (sizeof(_x) - 1)" in the if statment:

   if ( (limit) < sizeof(_x) - 1 || (eip) > (limit) - (sizeof(_x) - 1) )   \
       goto fail;                                                          \

Reading vmcs would return 32bit limit of 0xffffffff. BTW, same override
exists in read_descriptor() (it seems to do the override for FS and
GS also, which I don't understand).


Anyways, thanks to hvm_get_segment_register(), I got rid of the function 
vmx_pvh_read_descriptor():

static int read_descriptor_sel(unsigned int 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)
{
    if ( is_pvh_vcpu(v) )
    {
        struct segment_register seg;

        hvm_get_segment_register(v, which_sel, &seg);
        *ar = (unsigned int)seg.attr.bytes;

        /* ar is returned packed as in segment_attributes_t. fix it up */
        *ar = (*ar & 0xff ) | ((*ar & 0xf00) << 4);
        *ar = *ar << 8;

        if ( (vm86attr & _SEGMENT_CODE) && (*ar & _SEGMENT_L) &&
             (which_sel < x86_seg_fs) )
        {
            *base = 0UL;
            *limit = ~0UL;
        }
        else
        {
            *base = (unsigned long)seg.base;
            *limit = (unsigned long)seg.limit;
        }

        return 1;
    }

    return read_descriptor(sel, v, regs, base, limit, ar, vm86attr);

}