[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



Hi Wei,

On 03/17/2017 06:51 AM, Wei Chen wrote:
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.

Nack from my side. We are getting ride of macro in Xen in favor of static inline in the whole. I see no point here to do the macro, except saving 4 lines....

I don't want to see us in this game of trying to get the fewer number of lines just to claim we are small. Clarity and typesafety first.

In this case, a macro is not clarity nor safe.


Cheers,

--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.