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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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.

>> > +    if ( vmx_add_guest_msr(MSR_SHADOW_GS_BASE) )
>> > +    {
>> > +        vmx_vmcs_exit(v);
>> > +        return -EINVAL;
>> > +    }
>> > +    vmx_write_guest_msr(MSR_SHADOW_GS_BASE, ctxtp->gs_base_kernel);
>> 
>> So you write both GS bases, but not the base of FS (and above
>> its selector is being skipped too)?
> 
> Right, for 32bit PVH we'd need to do that. It needs PVH 32bitfixme tag.
> Or I can just do it for both, 64bit would be null anyways.

This again is a Linux assumption. Please stop this building in of
Linux-isms into the hypervisor.

>> And there are other parts of struct vcpu_guest_context that
>> I don't see getting mirrored - are all of them getting handled
>> elsewhere?
> 
> The call comes from VCPUOP_initialise -> arch_set_info_guest() which handles 
> some of the other fields. There's lot less to load for PVH compared to
> PV.

So just as an example - where would non-zero ldt_base/ldt_ents
get dealt with? Just to repeat the above - don't make any
implications on which fields may be unused by your Linux patch set
(or add fixme notes identifying those places).

And btw., looking at that patch again I'm also getting the
impression that the GS base handling in that function is lacking
consideration of VGCF_in_kernel.

Jan


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