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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 17.07.13 at 02:47, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> wrote:
> On Tue, 16 Jul 2013 07:50:49 +0100
> "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
> 
>> >>> 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
> ....
>> >> >> 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.
> 
> I see your point. I had it in the back of my mind that loading a selector
> will cause the hidden fields to be loaded, SDM Vol 3A 3.4.3, but I see 
> that it doesn't apply to VMCS. I didn't realize this because the default
> values are the same as the initial guest GDT. So, options: 
> 
>  o Add comment indicating that we don't need to load hidden parts because
>    they are same as default values.
> 
>       - This makes it linux dependent.

Not if documented to be this way. There are restrictions on what a
PV guest may do, so reasonable restrictions - properly documented -
can also be placed on PVH.

>  o Load hidden parts also for any non null CS/SS/DS/ES selector being
>    loaded.
> 
>       - This is better if the guest GDT doesn't match defaults.

If going that route, null selectors also need taking care of (you
would have to invalidate register, CS only for a 64-bit guest, but
all of them for a 32-bit one). And of course, a NULL selector in
CS (and for a 32-bit guest in SS) would need to be treated as
illegal (and would probably also cause vmlaunch to fail).

>  o Don't worry about the selectors in xen, just let the guest load it itself
>    first thing. I tested this and works fine. The ABI is flexible at this
>    point :).

I don't like this option. PV startup code doesn't need to care about
setting up selector registers that aren't used for special purposes,
so I'd prefer PVH to not start placing such a requirement onto the
guest. Early startup code - aiui - should be possible to be the same
for PV and PVH modes. Am I mistaken here?

Bottom line, considering that you probably want to save yourself
the hassle of implementing the second of the options above, I'd
see the first one as the better choice over the last one.

Jan


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