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

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

On Mon, 12 Aug 2013 08:54:34 +0100
"Jan Beulich" <JBeulich@suse.com> wrote:

> >>> On 10.08.13 at 01:41, Mukesh Rathor <mukesh.rathor@oracle.com>
> >>> wrote:
> > On Thu, 08 Aug 2013 07:56:41 +0100
> > "Jan Beulich" <JBeulich@suse.com> wrote:
> > 
......
> > Ugh, to find the G bit, I need to walk the GDT to find the LDT
> > descriptor. Walking the GDT to look for system descriptor means
> > mapping guest gdt pages as I go thru the table, and also the system
> > descriptor sizes are different for 32bit vs IA-32e modes adding
> > extra code... All that just doesn't seem worth it to me for
> > supporting LDT during vcpu bringup.
> 
> Which is why I suggested requiring the LDT to be empty.

I don't recall you doing that, wish I had noted that.  OK, I'll just 
require ldt base/limit to be null.

> > Keir, do you have any thoughts? Basically, I'm trying to support 
> > VCPUOP_initialise here, which is used by a PV guest boot vcpu to 
> > set context of another vcpu it's trying to bring up. In retrospect,
> > I should have just created VCPUOP_initialise_pvh with limited fields
> > needed for PVH. (We already ignore bunch of stuff for PVH from 
> > VCPUOP_initialise like trap_ctxt, event_callback*,
> > syscall_callback*, etc...). But anyways, can't we just document
> > VCPUOP_initialise that only following fields are relevant and
> > honored for PVH:
> > 
> >    gdt.pvh.addr/limit, and ctxtp->user_regs.cs/ds/ss
> > 
> > (And others used in arch_set_info_guest like user_regs, flags,...)
> > 
> > Since we are loading gdtr and selectors cs/ds/ss, we should also
> > load the hidden fields for cs/ds/ss. That IMO is plenty enough
> > support for the vcpu to come up, and the vcpu itself can then load
> > ldtr, fs base, gs base, etc first thing in it's HVM container. What
> > do you all think?
> 
> If you implement loading the hidden fields of CS, then doing the
> same for the LDT shouldn't be that much more code (and if you
> permit a non-null LDT selector, then having it in place would even
> be a requirement before validating the CS selector). But again,
> I had already indicated that I'd be fine with requiring the state to
> be truly minimal: CS -> flat 64-bit code descriptor, SS, DS, ES, FS
> and GS holding null selectors. And CS descriptor validation done
> only in debug mode.

It seems so unlikely that any guest would *require* LDT table set on
boot, that I'll just check for NULL like you said above. We can always
enhance in future. CS->flat is an option, but it would require special
code in linux. It would be nice to keep that code same for both PV and
PVH in linux.

> Talking of the LDT selector: Iirc you modify struct
> vcpu_guest_context's GDT to match PVH needs, but if I'm not
> mistaken you don't do the same for the LDT - PVH would require
> merely a selector here, not a base/ents pair.

I think that's way overkill, may make sense for PV, but PVH can
just do it first thing in it's boot code.

Let me crank out some code and propose it. I'd like PVH to make
4.4 if possible.

thanks
mukesh