From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH v2 1/4] x86/compat: Test whether guest has 32b shinfo instead of being a PV 32b domain Date: Tue, 07 Jul 2015 13:13:17 -0400 Message-ID: <559C08AD.106@oracle.com> References: <1435609282-1383-1-git-send-email-boris.ostrovsky@oracle.com> <1435609282-1383-2-git-send-email-boris.ostrovsky@oracle.com> <559BB3DF020000780008D3A6@mail.emea.novell.com> <559BF471.7080904@oracle.com> <559C173F020000780008DBAD@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <559C173F020000780008DBAD@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: elena.ufimtseva@oracle.com, tim@xen.org, wei.liu2@citrix.com, ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org, roger.pau@citrix.com List-Id: xen-devel@lists.xenproject.org On 07/07/2015 12:15 PM, Jan Beulich wrote: >>>> On 07.07.15 at 17:46, wrote: >> On 07/07/2015 05:11 AM, Jan Beulich wrote: >>>>>> On 29.06.15 at 22:21, 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 >> right). >> >> 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 > hvm_guest_x86_mode(). 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) u_domctl) >>>> break; >>>> >>>> #ifdef CONFIG_COMPAT >>>> - if ( !is_pv_32on64_domain(d) ) >>>> + if ( !has_32bit_shinfo(d) ) >>>> ret = copy_from_guest(c.nat, op->u.vcpucontext.ctxt, 1); >>>> else >>>> ret = copy_from_guest(c.cmp, >>>> @@ -902,7 +902,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) >>>> vcpu_unpause(v); >>>> >>>> #ifdef CONFIG_COMPAT >>>> - if ( !is_pv_32on64_domain(d) ) >>>> + if ( !has_32bit_shinfo(d) ) >>>> ret = copy_to_guest(op->u.vcpucontext.ctxt, c.nat, 1); >>>> else >>>> 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 >>> mode? >> 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). -boris