Re: [Xen-devel] [V10 PATCH 09/23] PVH xen: introduce pvh_set_vcpu_info() and vmx_pvh_set_vcpu_info()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 08.08.13 at 03:05, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> wrote:
> On Mon, 05 Aug 2013 12:10:15 +0100
> "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
> 
>> >>> On 24.07.13 at 03:59, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>
>> >>> 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.
However - now that you're intending to require trivial state (64-bit
CS, all other selectors zero), it would only be logical to also require
a zero LDT selector (and hence base and entry count to be zero).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel