From mboxrd@z Thu Jan  1 00:00:00 1970
From: Tim Deegan <tim@xen.org>
Subject: Re: [V10 PATCH 09/23] PVH xen: introduce
	pvh_set_vcpu_info() and vmx_pvh_set_vcpu_info()
Date: Mon, 12 Aug 2013 10:00:36 +0100
Message-ID: <20130812090036.GA23354@ocelot.phlegethon.org>
References: <1374631171-15224-1-git-send-email-mukesh.rathor@oracle.com>
	<1374631171-15224-10-git-send-email-mukesh.rathor@oracle.com>
	<51FFA43702000078000E936E@nat28.tlf.novell.com>
	<20130807180549.46941f85@mantra.us.oracle.com>
	<52035D4902000078000EA1C6@nat28.tlf.novell.com>
	<20130809164138.38aa9b06@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>
Received: from mail6.bemta14.messagelabs.com ([193.109.254.103])
	by lists.xen.org with esmtp (Exim 4.72) (envelope-from <tim@xen.org>)
	id 1V8nzb-0005nK-IH
	for xen-devel@lists.xenproject.org; Mon, 12 Aug 2013 09:00:47 +0000
Content-Disposition: inline
In-Reply-To: <20130809164138.38aa9b06@mantra.us.oracle.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: Mukesh Rathor <mukesh.rathor@oracle.com>
Cc: xen-devel <xen-devel@lists.xenproject.org>, Keir Fraser <keir.xen@gmail.com>, Jan Beulich <JBeulich@suse.com>
List-Id: xen-devel@lists.xenproject.org

At 16:41 -0700 on 09 Aug (1376066498), Mukesh Rathor wrote:
> On Thu, 08 Aug 2013 07:56:41 +0100
> "Jan Beulich" <JBeulich@suse.com> wrote:
> 
> > >>> On 08.08.13 at 03:05, Mukesh Rathor <mukesh.rathor@oracle.com>
> > >>> wrote:
> > > On Mon, 05 Aug 2013 12:10:15 +0100
> > > "Jan Beulich" <JBeulich@suse.com> wrote:
> > > 
> > >> >>> On 24.07.13 at 03:59, Mukesh Rathor <mukesh.rathor@oracle.com>
> > >> >>> wrote:
> > >> > +int vmx_pvh_set_vcpu_info(struct vcpu *v, struct
> > >> > vcpu_guest_context *ctxtp) +{
> > >> > +    if ( v->vcpu_id == 0 )
> > >> > +        return 0;
> > >> > +
> > >> > +    if ( !(ctxtp->flags & VGCF_in_kernel) )
> > >> > +        return -EINVAL;
> > >> > +
> > >> > +    vmx_vmcs_enter(v);
> > >> > +    __vmwrite(GUEST_GDTR_BASE, ctxtp->gdt.pvh.addr);
> > >> > +    __vmwrite(GUEST_GDTR_LIMIT, ctxtp->gdt.pvh.limit);
> > >> > +    __vmwrite(GUEST_LDTR_BASE, ctxtp->ldt_base);
> > >> > +    __vmwrite(GUEST_LDTR_LIMIT, ctxtp->ldt_ents);
> > >> 
> > >> Just noticed: Aren't you mixing up entries and bytes here?
> > > 
> > > Right:
> > > 
> > > __vmwrite(GUEST_LDTR_LIMIT, (ctxtp->ldt_ents * 8 - 1) );
> > > 
> > > Any formatting issues here? I don't see in coding style, and see
> > > both code where there is a space around '*' and not.
> > 
> > The inner parentheses are superfluous.
> > 
> > CODING_STYLE is pretty explicit about there needing to be white
> > space around operators: "Spaces are placed [...], and around
> > binary operators (except the structure access operators, '.' and
> > '->')."
> > 
> > > Also, when setting the limit, do we need to worry about the G flag?
> > > or for that matter, D/B whether segment is growing up or down?
> > > It appears we don't need to worry about that for LDT, but not sure
> > > reading the SDMs..
> > 
> > The D/B bit doesn't matter for LDT (and TSS), but the G bit would.
> 
> Ugh, to find the G bit, I need to walk the GDT to find the LDT descriptor.

Why so?  The caller supplies you with the LDT base and range, not a
segment selector.  I don't think you could find the right LDT selector
by scanning the GDT anyway -- what if there were two that matched?

Tim.