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: Wed, 08 Jul 2015 10:40:34 -0400 Message-ID: <559D3662.4070803@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> <559C08AD.106@oracle.com> <559CE3DD020000780008DEAA@mail.emea.novell.com> <559D2CD8.7050807@oracle.com> <559D4AF2020000780008E463@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: <559D4AF2020000780008E463@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/08/2015 10:08 AM, Jan Beulich wrote: >>>> On 08.07.15 at 15:59, wrote: >> On 07/08/2015 02:48 AM, Jan Beulich wrote: >>>>>> On 07.07.15 at 19:13, wrote: >>>> 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? >>> svm_guest_x86_mode() doesn't depend on v == current, but >>> vmx_guest_x86_mode() would first need to be made safe (or >>> get an "unsafe" sibling implementation). With that, the ASSERT() >>> could then check for current or non-running vCPU. >> By checking for non-running you mean v->is_running? I am not sure it's >> safe to do since is_running is set in context switch before VMCS is >> loaded later, in vmx_do_resume(). > No, I rather thought about making sure the vCPU is paused (i.e. > can't become running under your feet). What would prevent it from becoming running if it is paused, right after the ASSERT? -boris > >> OTOH, current itself is set before VMCS is loaded so I am not sure >> whether the ASSERT in hvm_guest_x86_mode() is completely effective in >> catching "bad" invocations anyway. >> >> I think we need vmx_vmcs_enter/exit in vmx_guest_x86_mode() regardless >> of what current is. And drop the ASSERT. > That's an option, but the current uses don't require that (and hence > a change like that may be considered harming performance if some > caller sits on a hot path).