Re: [Xen-devel] [PATCH 03/18] xen/arm: Avoid setting/clearing HCR_RW at every context switch

[Julien Grall <julien.grall@xxxxxxx>]

On 03/16/2017 10:40 PM, Stefano Stabellini wrote:
> On Wed, 15 Mar 2017, Wei Chen wrote:
> > Hi Stefano,
> >
> > On 2017/3/15 8:25, Stefano Stabellini wrote:
> > > On Mon, 13 Mar 2017, Wei Chen wrote:
> > > > The HCR_EL2 flags for 64-bit and 32-bit domains are different. But
> > > > when we initialized the HCR_EL2 for vcpu0 of Dom0 and all vcpus of
> > > > DomU in vcpu_initialise, we didn't know the domain's address size
> > > > information. We had to use compatible flags to initialize HCR_EL2,
> > > > and set HCR_RW for 64-bit domain or clear HCR_RW for 32-bit domain
> > > > at every context switch.
> > > >
> > > > But, after we added the HCR_EL2 to vcpu's context, this behaviour
> > > > seems a little fussy. We can update the HCR_RW bit in vcpu's context
> > > > as soon as we get the domain's address size to avoid setting/clearing
> > > > HCR_RW at every context switch.
> > > >
> > > > Signed-off-by: Wei Chen <Wei.Chen@xxxxxxx>
> > > > ---
> > > >  xen/arch/arm/arm64/domctl.c  | 6 ++++++
> > > >  xen/arch/arm/domain.c        | 5 +++++
> > > >  xen/arch/arm/domain_build.c  | 7 +++++++
> > > >  xen/arch/arm/p2m.c           | 5 -----
> > > >  xen/include/asm-arm/domain.h | 1 +
> > > >  5 files changed, 19 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/xen/arch/arm/arm64/domctl.c b/xen/arch/arm/arm64/domctl.c
> > > > index 44e1e7b..ab8781f 100644
> > > > --- a/xen/arch/arm/arm64/domctl.c
> > > > +++ b/xen/arch/arm/arm64/domctl.c
> > > > @@ -14,6 +14,8 @@
> > > >
> > > >  static long switch_mode(struct domain *d, enum domain_type type)
> > > >  {
> > > > +    struct vcpu *v;
> > > > +
> > > >      if ( d == NULL )
> > > >          return -EINVAL;
> > > >      if ( d->tot_pages != 0 )
> > > > @@ -23,6 +25,10 @@ static long switch_mode(struct domain *d, enum domain_type 
> > > > type)
> > > >
> > > >      d->arch.type = type;
> > > >
> > > > +    if ( is_64bit_domain(d) )
> > > > +        for_each_vcpu(d, v)
> > > > +            vcpu_switch_to_aarch64_mode(v);
> > > > +
> > > >      return 0;
> > > >  }
> > > >
> > > > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> > > > index 5d18bb0..69c2854 100644
> > > > --- a/xen/arch/arm/domain.c
> > > > +++ b/xen/arch/arm/domain.c
> > > > @@ -537,6 +537,11 @@ void vcpu_destroy(struct vcpu *v)
> > > >      free_xenheap_pages(v->arch.stack, STACK_ORDER);
> > > >  }
> > > >
> > > > +void vcpu_switch_to_aarch64_mode(struct vcpu *v)
> > > > +{
> > > > +    v->arch.hcr_el2 |= HCR_RW;
> > > > +}
> > > if possible, I would write it as a static inline function
> > I had tried to write it as a static inline function in asm/domain.h
> > But while the first source file (arm64/asm-offsets.c) include
> > xen/sched.h wanted to compile this inline function it could not find
> > 'struct vcpu'. Because the xen/sched.h included the asm/domain.h
> > but defined the vcpu type later. Even though we had included the
> > xen/sched.h in asm/domain.h already.
> It might be too complex to disentangle. In this case, there isn't much
> type safety to be gained by using a static inline over a macro, so it
> would be OK to use a macro for this.
It is not like vCPU will be switch to AArch64 mode often? Only once at 
vCPU creation time.
So what do we gain to switch to static inline or even macro?

Cheers,

--
Julien Grall

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