Re: [PATCH v3 07/32] xen/x86: fix arch_set_info_guest for HVM guests

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 24/07/15 17:54, Roger Pau Monné wrote:
>
>>> struct vcpu_hvm_context {
>>> /* 16bit fields of the strucutre will be used. */
>>> #define _VCPUHVM_MODE_16B              0
>>> #define VCPUHVM_MODE_16B               (1<<_VCPUHVM_MODE_16B)
>>> /* 32bit fields of the structure will be used. */
>>> #define _VCPUHVM_MODE_32B              1
>>> #define VCPUHVM_MODE_32B               (1<<_VCPUHVM_MODE_32B)
>>> /* 64bit fields of the structure will be used. */
>>> #define _VCPUHVM_MODE_64B              2
>>> #define VCPUHVM_MODE_64B               (1<<_VCPUHVM_MODE_64B)
>> As soon as we have three values here, a boolean flag approach
>> doesn't make sense anymore.
> Yes, taking into account that only _one_ of them can be used at the same
> time.
>
> I have the feeling that we are over engineering this interface. IMHO we
> should only allow the user to set the control registers, efer (on amd64)
> and the GP registers. Segment selectors would be set by Xen to point to
> a flat segment suitable for the mode the guest has selected. I think
> that should be enough to get the guest OS into it's entry point, and
> then it can do whatever it wants.
>
> If that's not suitable, then my second option would be to allow the
> guest to set the base, limit and AR bytes of the selectors, but not load
> a GDT.

In an ideal world, it would be nice to pass enough state to be able to
point eip at the idle loop and have everything JustWork.  However, this
makes a lot of assumptions about exactly which state the vcpu wants to
set up, which might include MSRs as well.

Therefore, an appropriate compromise is to be able to point eip at
startup_cpu, running in the correct operating mode to avoid bouncing
through trampolines.

>From that point of view, the minimum quantity of state required is:

CR{0,3,4}, EFER, cs/eip, ds, ss/esp, gdt{base,limit}.  It would be
natural to extend this to all the 8 base GPRs and the user segment
selectors.  (Note that EFER is needed even for 32bit paged entries if NX
is set in the pagetables.)

In addition, a hint to Xen as to the eventual mode so it can widen its
reads to start with rather than attempting to reverse engineer the
operating mode and rereading half the structure later. 

Specifying the gdt{base,limit} will require Xen to read the GDT to set
up the entry attributes for the segment, as simply setting the selector
is insufficient.  A complicating factor is that VT-x and SVM have
different representations for access rights.  (For migration, VT-x's
representation is mutated to match SVM.)

I really undecided between suggesting a gdt{base,limit} and Xen reading
the access rights in an unambiguous fashon, vs specifying the internals
and avoiding any peeking into domain memory.

It would be nice if the new cpus gs base could be set by the caller,
which allows the new cpu straight access to its per-cpu data area (if gs
is in use).  This is not representable using the gdt method and would
involve casing the GSBASE MSR if we were to go down the strict
architectural route.

Therefore, I am leaning slightly towards the specify the internals side
of things, which removes some complexity from the Xen side of the hypercall.

~Andrew