Re: [Xen-devel] [PATCH 1/3] x86: drop is_pv_32on64_vcpu()

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

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 <jbeulich@xxxxxxxx>
> > 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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel