Re: [PATCH 10/17] PVH xen: introduce vmx_pvh.c and pvh.c

[Mukesh Rathor <mukesh.rathor@oracle.com>]

On Mon, 06 May 2013 07:44:33 +0100
"Jan Beulich" <JBeulich@suse.com> wrote:

> >>> On 04.05.13 at 03:40, Mukesh Rathor <mukesh.rathor@oracle.com>
> >>> wrote:
> > On Fri, 03 May 2013 07:33:50 +0100 "Jan Beulich"
 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. 
> 
> I'm afraid you're still not understanding what I'm trying to explain:
> Whether base and limit are ignored (and default to 0/~0) depends
> on whether the guest executes in 64-bit mode, and this you can
> know only by looking at CS.L, no matter what selector register
> you're reading.
> 
> Maybe part of the confusion stems from you mixing two things
> here - reading of a descriptor from a descriptor table (which is
> what read_descriptor() does, as that's all you can do for PV
> guests) vs reading of the hidden portions of a selector register
> (which is what hvm_get_segment_register() does, thanks to
> VMX/SVM providing access).

read_descriptor() confuses me a lot. The way I understood it: it reads
the full desc from LDT/GDT indexed by the selector, which is where the hidden 
portions get loaded from, so it has full info like we get from vmcs.
It can look at the "Type" bits 8-11 in the upper half and figure if it's
code segment also.  Following check in it adds to confusiong:

        if ( !(vm86attr & _SEGMENT_CODE) )
            desc.b &= ~_SEGMENT_L;


Anyways, I think (and hope), I finally have it:

{
    struct segment_register seg;
    unsigned int long_mode = 0;

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

    hvm_get_segment_register(v, x86_seg_cs, &seg);
    long_mode = seg.attr.fields.l;

    if ( which_sel != x86_seg_cs )
        hvm_get_segment_register(v, which_sel, &seg);

    /* ar is returned packed as in segment_attributes_t. Fix it up */
    *ar = (unsigned int)seg.attr.bytes;
    *ar = (*ar & 0xff ) | ((*ar & 0xf00) << 4);
    *ar = *ar << 8;

    if ( long_mode )
    {
        *limit = ~0UL;

        if ( which_sel < x86_seg_fs )
        {
            *base = 0UL;
            return 1;
        }
   }
   else
       *limit = (unsigned long)seg.limit;

   *base = (unsigned long)seg.base;
    return 1;
}


Thanks for your time and help.
Mukesh