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

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Stefano,

On 2017/3/17 7:17, Stefano Stabellini wrote:
> On Thu, 16 Mar 2017, Julien Grall wrote:
>> 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?
>
> To turn 5 lines of code into 1. Obviously it's no big deal.
>

Ok, I think switch to macro is easier then static inline. I will switch
it to a macro in next version.


-- 
Regards,
Wei Chen

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