Re: [Xen-devel] [PATCH 18/18] PVH xen: introduce vmx_pvh.c

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.07.13 at 04:00, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> wrote:
> On Fri, 28 Jun 2013 10:44:08 +0100
> "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
> 
>> >>> On 28.06.13 at 04:28, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>
>> >>> wrote:
>> > On Tue, 25 Jun 2013 11:49:57 +0100 "Jan Beulich"
>> > <JBeulich@xxxxxxxx> wrote:
>> >> > +int vmx_pvh_set_vcpu_info(struct vcpu *v, struct
>> >> > vcpu_guest_context *ctxtp) +{
>> >> > +    if ( v->vcpu_id == 0 )
>> >> > +        return 0;
>> >> > +
>> >> > +    vmx_vmcs_enter(v);
>> >> > +    __vmwrite(GUEST_GDTR_BASE, ctxtp->gdt.pvh.addr);
>> >> > +    __vmwrite(GUEST_GDTR_LIMIT, ctxtp->gdt.pvh.limit);
>> >> > +    __vmwrite(GUEST_GS_BASE, ctxtp->gs_base_user);
>> >> > +
>> >> > +    __vmwrite(GUEST_CS_SELECTOR, ctxtp->user_regs.cs);
>> >> > +    __vmwrite(GUEST_DS_SELECTOR, ctxtp->user_regs.ds);
>> >> > +    __vmwrite(GUEST_ES_SELECTOR, ctxtp->user_regs.es);
>> >> > +    __vmwrite(GUEST_SS_SELECTOR, ctxtp->user_regs.ss);
>> >> > +    __vmwrite(GUEST_GS_SELECTOR, ctxtp->user_regs.gs);
>> >> 
>> >> How does this work without also writing the "hidden" register
>> >> fields?
>> > 
>> > This is for bringing up SMP CPUs by the guest, which already has
>> > set GDT up so it just needs selectors to be loaded to start the
>> > target vcpu.
>> 
>> That makes no sense to me: Once you VMLAUNCH that vCPU, it'll
>> get the hidden register fields loaded from the VMCS, without
>> accessing the GDT. If that understanding of mine is wrong, please
>> explain how you see things working in more detail.
> 
> Re-reading this I realize I misunderstood your question. Sorry. The hidden
> fields are set to the default values in vmcs create. That call comes from
> the tool stack during domain creation via the do_domctl ---> vcpu_initialise
> hcall. That happens during guest creation. Here, the guest is booting and 
> bringing up secondary CPU, and wants to set certain fields via 
> the do_vcpu_op() -> VCPUOP_initialise hcall.
> 
> Hope that clarifies.

Yes, that clarifies that my complaint was right. Either you add a
comment to the code stating why you don't load these fields with
the guest specified values (from the GDT/LDT), or you set them
to the specified values rather than the defaults (or, of course, a
mixture of both - it may be reasonable to require the descriptors
referenced by CS, SS, DS, and ES to use defaults, but extending
this to FS, GS, and LDT would likely be too much of a requirement).
And even if implying defaults, verifying the descriptors to match
those requirements at least in debug builds may be a good idea.

Jan


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