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

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

On Mon, 01 Jul 2013 09:54:30 +0100
"Jan Beulich" <JBeulich@suse.com> wrote:

> >>> On 29.06.13 at 05:04, Mukesh Rathor <mukesh.rathor@oracle.com>
> >>> wrote:
> > On Fri, 28 Jun 2013 10:44:08 +0100 "Jan Beulich"
> > <JBeulich@suse.com> wrote:
> >> >>> On 28.06.13 at 04:28, Mukesh Rathor <mukesh.rathor@oracle.com>
> >> >>> wrote:
> >> > On Tue, 25 Jun 2013 11:49:57 +0100 "Jan Beulich"
> >> > <JBeulich@suse.com> 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.
> > 
> > I see things same as you do, that it'll get hidden fields from
> > vmcs. The dilemma here is what to do about the VCPUOP_initialise
> > hcall. I am currently checking to see if the new vcpu can just set
> > the context itself first thing, without the VCPUOP_initialise hcall
> > completely.
> 
> I don't follow: What's the dilemma here? Why can't you just put
> _all_ of the values specified by VCPUOP_initialise into the VMCS?

Well, OK, whatever is relevant for PVH, like ldt. Other things like
trap_ctxt, *callback*... are not applicable to PVH.

> > Correct, the guest I am dealing with is Linux. And correct again,
> > that these are linux focussed. I believe it was someone working on
> > BSD who had inquired about PVH, and looking at my patches, and
> > probably would be working with us when they are ready.
> > 
> > These patches are to allow a 64bit Linux PV domU to boot in PVH
> > mode on xen. Since no new hypercalls are created, nor are there any
> > major changes to any, it doesn't seem to me a huge deal that they
> > are linux focussed right now. To me, it's a step in the direction
> > of someday having PVH support for all OSs.
> 
> As said before - I would consider this sort of acceptable if the
> omissions were spelled out clearly (i.e. easily grep-able for and
> easily recognizable when reading the code).

Ok, I'll expand the function prolog.

thanks
mukesh