From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH 1/3] x86: drop is_pv_32on64_vcpu() Date: Wed, 24 Jun 2015 21:08:59 -0400 Message-ID: <558B54AB.1010804@oracle.com> References: <558993E902000078000886B3@mail.emea.novell.com> <558994ED02000078000886DB@mail.emea.novell.com> <558B228D.80404@oracle.com> <558B4211.80207@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Z7vfB-0004dI-MM for xen-devel@lists.xenproject.org; Thu, 25 Jun 2015 01:09:09 +0000 In-Reply-To: <558B4211.80207@citrix.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: Andrew Cooper , Jan Beulich , xen-devel Cc: Keir Fraser List-Id: xen-devel@lists.xenproject.org On 06/24/2015 07:49 PM, Andrew Cooper wrote: > On 24/06/2015 22:35, Boris Ostrovsky wrote: >> On 06/23/2015 11:18 AM, Jan Beulich wrote: >>> ... as being identical to is_pv_32bit_vcpu() after the x86-32 removal. >>> >>> In a few cases this includes an additional is_pv_32bit_vcpu() -> >>> is_pv_32bit_domain() conversion. >>> >>> Signed-off-by: Jan Beulich >> We have >> struct arch_domain >> { >> ... >> /* Is a 32-bit PV (non-HVM) guest? */ >> bool_t is_32bit_pv; >> /* Is shared-info page in 32-bit format? */ >> bool_t has_32bit_shinfo; >> ... >> } >> >> and currently both of these fields are set/unset together (except for >> one HVM case --- hvm_latch_shinfo_size()). Why not have a single 'bool >> is_32bit' and then replace macros at the top of >> include/asm-x86/domain.h with is_32bit_vcpu/domain()? >> >> I think in majority of places when we test for >> is_pv_32bit_vcpu/domain() we already know that we are PV so it >> shouldn't add any additional tests. > For the PV case, the two are equivalent. For HVM, they are not. > > HVM domains have shared info, but may be latched as either 32 or 64bit, > depending on the mode they were running in when they most recently wrote > a hypercall page. Sadly, the shared info layout is width-dependent > which is why such hacks need to exist. Why can't we latch the mode into is_32bit field? I am essentially suggesting to drop is_32bit_pv and rename has_32bit_shinfo to is_32bit. Then is_pv_32bit_vcpu() becomes '(is_pv_vcpu() && domain->is_32bit)' (or simply domain->is_32bit, depending on context) and has_32bit_shinfo() becomes domain->is_32bit. The reason I am asking is because for the 32b PVH I will need to switch a few places from using is_pv_32bit_vcpu() to has_32bit_shinfo() and that would look strange: asking whether the guest is 32-bit looks more natural than asking whether its shared info is 32-bit. At least it's more natural to my eye. -boris