[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2 1/4] x86/compat: Test whether guest has 32b shinfo instead of being a PV 32b domain

On 07/07/2015 12:15 PM, Jan Beulich wrote:
On 07.07.15 at 17:46, <boris.ostrovsky@xxxxxxxxxx> wrote:
On 07/07/2015 05:11 AM, Jan Beulich wrote:
On 29.06.15 at 22:21, <boris.ostrovsky@xxxxxxxxxx> wrote:
@@ -737,7 +737,7 @@ int arch_set_info_guest(
/* The context is a compat-mode one if the target domain is compat-mode;
        * we expect the tools to DTRT even in compat-mode callers. */
-    compat = is_pv_32on64_domain(d);
+    compat = has_32bit_shinfo(d);
Furthermore, looking at uses like this, tying such decisions to the
shared info layout looks kind of odd. I think for documentation
purposes we may need a differently named alias.
Yes, it does look odd, which is why I was asking in another thread about
having another field in domain structure (well, I was asking about
replacing has_32bit_shinfo but I think I can see now that wouldn't be

Are you suggesting a new macro, e.g.
#define is_32b_mode(d)    ((d)->arch.has_32bit_shinfo)

or would it better to add new field? Or get_mode() hvm op, similar to
set_mode(), which can look, say, at EFER?
If looking at EFER (plus perhaps CS) is right in all the cases you
care about, then yes. And remember we already have

Can't use hvm_guest_x86_mode(), it asserts on 'v != current'. But adding new op just because of that seems to be an overkill since it would essentially do what .guest_x86_mode() does. How about hvm_guest_x86_mode_unsafe() (with a better name) and wrap hvm_guest_x86_mode() with the ASSERT around it?

--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -496,7 +496,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
-        if ( !is_pv_32on64_domain(d) )
+        if ( !has_32bit_shinfo(d) )
               ret = copy_from_guest(c.nat, op->u.vcpucontext.ctxt, 1);
               ret = copy_from_guest(c.cmp,
@@ -902,7 +902,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
-        if ( !is_pv_32on64_domain(d) )
+        if ( !has_32bit_shinfo(d) )
               ret = copy_to_guest(op->u.vcpucontext.ctxt, c.nat, 1);
               ret = copy_to_guest(guest_handle_cast(op->u.vcpucontext.ctxt,
Where is it written down what format 32-bit PVH guests' vCPU
contexts get passed in? It would seem to me that it would be
rather more natural for them to use the 64-bit layout. Or else
how do you intend to suppress them being able to enter 64-bit
So why do we use the 'else' clause for 32b PV guests when they also use
the same vcpu_guest_context_x86_32_t in libxc/xc_dom_x86.c:vcpu_x86_32()?
32bit PV guests use the if() branch afaict (as they use the 32-bit
shared info layout).

No, they use the 'else' part, I just confirmed it. 'd' in is_pv_32on64_domain() is domain for which domctl is being called, not domain that is making the call (which is what I suspect the original intent was).


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.