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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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.

 
> >
> >>  int arch_domain_create(struct domain *d, unsigned int domcr_flags,
> >>                         struct xen_arch_domainconfig *config)
> >>  {
> >> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> >> index de59e5f..3abacc0 100644
> >> --- a/xen/arch/arm/domain_build.c
> >> +++ b/xen/arch/arm/domain_build.c
> >> @@ -2148,6 +2148,10 @@ int construct_dom0(struct domain *d)
> >>          return -EINVAL;
> >>      }
> >>      d->arch.type = kinfo.type;
> >> +
> >> +    if ( is_64bit_domain(d) )
> >> +        vcpu_switch_to_aarch64_mode(v);
> >> +
> >>  #endif
> >>
> >>      allocate_memory(d, &kinfo);
> >> @@ -2240,6 +2244,9 @@ int construct_dom0(struct domain *d)
> >>              printk("Failed to allocate dom0 vcpu %d on pcpu %d\n", i, 
> >> cpu);
> >>              break;
> >>          }
> >> +
> >> +        if ( is_64bit_domain(d) )
> >> +            vcpu_switch_to_aarch64_mode(d->vcpu[i]);
> >>      }
> >>
> >>      return 0;
> >> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> >> index c49bfa6..1cba0d0 100644
> >> --- a/xen/arch/arm/p2m.c
> >> +++ b/xen/arch/arm/p2m.c
> >> @@ -136,11 +136,6 @@ void p2m_restore_state(struct vcpu *n)
> >>      WRITE_SYSREG64(p2m->vttbr, VTTBR_EL2);
> >>      isb();
> >>
> >> -    if ( is_32bit_domain(n->domain) )
> >> -        n->arch.hcr_el2 &= ~HCR_RW;
> >> -    else
> >> -        n->arch.hcr_el2 |= HCR_RW;
> >> -
> >>      WRITE_SYSREG(n->arch.sctlr, SCTLR_EL1);
> >>      isb();
> >>
> >> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
> >> index 7b1dacc..68185e2 100644
> >> --- a/xen/include/asm-arm/domain.h
> >> +++ b/xen/include/asm-arm/domain.h
> >> @@ -268,6 +268,7 @@ struct arch_vcpu
> >>
> >>  void vcpu_show_execution_state(struct vcpu *);
> >>  void vcpu_show_registers(const struct vcpu *);
> >> +void vcpu_switch_to_aarch64_mode(struct vcpu *);
> >>
> >>  unsigned int domain_max_vcpus(const struct domain *);


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