| Message ID | 20180215210332.8648-35-christoffer.dall@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 15 Feb 2018 21:03:26 +0000, Christoffer Dall wrote: > > To make the code more readable and to avoid the overhead of a function > call, let's get rid of a pair of the alternative function selectors and > explicitly call the VHE and non-VHE functions using the has_vhe() static > key based selector instead, telling the compiler to try to inline the > static function if it can. > > Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org> > --- > arch/arm64/kvm/hyp/switch.c | 30 ++++++++++++++---------------- > 1 file changed, 14 insertions(+), 16 deletions(-) > > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c > index 5e94955b89ea..0e54fe2aab1c 100644 > --- a/arch/arm64/kvm/hyp/switch.c > +++ b/arch/arm64/kvm/hyp/switch.c > @@ -85,7 +85,7 @@ static void __hyp_text __deactivate_traps_common(void) > write_sysreg(0, pmuserenr_el0); > } > > -static void __hyp_text __activate_traps_vhe(struct kvm_vcpu *vcpu) > +static inline void activate_traps_vhe(struct kvm_vcpu *vcpu) > { > u64 val; > > @@ -97,7 +97,7 @@ static void __hyp_text __activate_traps_vhe(struct kvm_vcpu *vcpu) > write_sysreg(kvm_get_hyp_vector(), vbar_el1); > } > > -static void __hyp_text __activate_traps_nvhe(struct kvm_vcpu *vcpu) > +static inline void __hyp_text __activate_traps_nvhe(struct kvm_vcpu *vcpu) Having both inline and __hyp_text does not make much sense (the function will be emitted in the section defined by the caller). I suggest you drop the inline and let the compiler do its magic. > { > u64 val; > > @@ -108,11 +108,7 @@ static void __hyp_text __activate_traps_nvhe(struct kvm_vcpu *vcpu) > write_sysreg(val, cptr_el2); > } > > -static hyp_alternate_select(__activate_traps_arch, > - __activate_traps_nvhe, __activate_traps_vhe, > - ARM64_HAS_VIRT_HOST_EXTN); > - > -static void __hyp_text __activate_traps(struct kvm_vcpu *vcpu) > +static inline void __hyp_text __activate_traps(struct kvm_vcpu *vcpu) Same here, and in other spots in this file. > { > u64 hcr = vcpu->arch.hcr_el2; > > @@ -122,10 +118,13 @@ static void __hyp_text __activate_traps(struct kvm_vcpu *vcpu) > write_sysreg_s(vcpu->arch.vsesr_el2, SYS_VSESR_EL2); > > __activate_traps_fpsimd32(vcpu); > - __activate_traps_arch()(vcpu); > + if (has_vhe()) > + activate_traps_vhe(vcpu); > + else > + __activate_traps_nvhe(vcpu); > } > > -static void __hyp_text __deactivate_traps_vhe(void) > +static inline void deactivate_traps_vhe(void) > { > extern char vectors[]; /* kernel exception vectors */ > write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2); > @@ -133,7 +132,7 @@ static void __hyp_text __deactivate_traps_vhe(void) > write_sysreg(vectors, vbar_el1); > } > > -static void __hyp_text __deactivate_traps_nvhe(void) > +static inline void __hyp_text __deactivate_traps_nvhe(void) > { > u64 mdcr_el2 = read_sysreg(mdcr_el2); > > @@ -147,11 +146,7 @@ static void __hyp_text __deactivate_traps_nvhe(void) > write_sysreg(CPTR_EL2_DEFAULT, cptr_el2); > } > > -static hyp_alternate_select(__deactivate_traps_arch, > - __deactivate_traps_nvhe, __deactivate_traps_vhe, > - ARM64_HAS_VIRT_HOST_EXTN); > - > -static void __hyp_text __deactivate_traps(struct kvm_vcpu *vcpu) > +static inline void __hyp_text __deactivate_traps(struct kvm_vcpu *vcpu) > { > /* > * If we pended a virtual abort, preserve it until it gets > @@ -162,7 +157,10 @@ static void __hyp_text __deactivate_traps(struct kvm_vcpu *vcpu) > if (vcpu->arch.hcr_el2 & HCR_VSE) > vcpu->arch.hcr_el2 = read_sysreg(hcr_el2); > > - __deactivate_traps_arch()(); > + if (has_vhe()) > + deactivate_traps_vhe(); > + else > + __deactivate_traps_nvhe(); > } > > void activate_traps_vhe_load(struct kvm_vcpu *vcpu) > -- > 2.14.2 > Thanks, M.
On Thu, Feb 15, 2018 at 10:03:26PM +0100, Christoffer Dall wrote: > To make the code more readable and to avoid the overhead of a function > call, let's get rid of a pair of the alternative function selectors and > explicitly call the VHE and non-VHE functions using the has_vhe() static > key based selector instead, telling the compiler to try to inline the > static function if it can. > > Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org> > --- > arch/arm64/kvm/hyp/switch.c | 30 ++++++++++++++---------------- > 1 file changed, 14 insertions(+), 16 deletions(-) > Reviewed-by: Andrew Jones <drjones@redhat.com>
On Wed, Feb 21, 2018 at 06:26:39PM +0000, Marc Zyngier wrote: > On Thu, 15 Feb 2018 21:03:26 +0000, > Christoffer Dall wrote: > > > > To make the code more readable and to avoid the overhead of a function > > call, let's get rid of a pair of the alternative function selectors and > > explicitly call the VHE and non-VHE functions using the has_vhe() static > > key based selector instead, telling the compiler to try to inline the > > static function if it can. > > > > Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org> > > --- > > arch/arm64/kvm/hyp/switch.c | 30 ++++++++++++++---------------- > > 1 file changed, 14 insertions(+), 16 deletions(-) > > > > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c > > index 5e94955b89ea..0e54fe2aab1c 100644 > > --- a/arch/arm64/kvm/hyp/switch.c > > +++ b/arch/arm64/kvm/hyp/switch.c > > @@ -85,7 +85,7 @@ static void __hyp_text __deactivate_traps_common(void) > > write_sysreg(0, pmuserenr_el0); > > } > > > > -static void __hyp_text __activate_traps_vhe(struct kvm_vcpu *vcpu) > > +static inline void activate_traps_vhe(struct kvm_vcpu *vcpu) > > { > > u64 val; > > > > @@ -97,7 +97,7 @@ static void __hyp_text __activate_traps_vhe(struct kvm_vcpu *vcpu) > > write_sysreg(kvm_get_hyp_vector(), vbar_el1); > > } > > > > -static void __hyp_text __activate_traps_nvhe(struct kvm_vcpu *vcpu) > > +static inline void __hyp_text __activate_traps_nvhe(struct kvm_vcpu *vcpu) > > Having both inline and __hyp_text does not make much sense (the > function will be emitted in the section defined by the caller). Is that true even if the function is not inlined? > I > suggest you drop the inline and let the compiler do its magic. > We were using an older compiler (I believe GCC 4.8) when doing some of the initial experiements, and saw that some of these small functions weren't inlined without the inline hint. However, this doesn't seem to be an issue on any of the compilers I'm using today. So I'll remove the inline. Thanks, -Christoffer > > { > > u64 val; > > > > @@ -108,11 +108,7 @@ static void __hyp_text __activate_traps_nvhe(struct kvm_vcpu *vcpu) > > write_sysreg(val, cptr_el2); > > } > > > > -static hyp_alternate_select(__activate_traps_arch, > > - __activate_traps_nvhe, __activate_traps_vhe, > > - ARM64_HAS_VIRT_HOST_EXTN); > > - > > -static void __hyp_text __activate_traps(struct kvm_vcpu *vcpu) > > +static inline void __hyp_text __activate_traps(struct kvm_vcpu *vcpu) > > Same here, and in other spots in this file. > > > { > > u64 hcr = vcpu->arch.hcr_el2; > > > > @@ -122,10 +118,13 @@ static void __hyp_text __activate_traps(struct kvm_vcpu *vcpu) > > write_sysreg_s(vcpu->arch.vsesr_el2, SYS_VSESR_EL2); > > > > __activate_traps_fpsimd32(vcpu); > > - __activate_traps_arch()(vcpu); > > + if (has_vhe()) > > + activate_traps_vhe(vcpu); > > + else > > + __activate_traps_nvhe(vcpu); > > } > > > > -static void __hyp_text __deactivate_traps_vhe(void) > > +static inline void deactivate_traps_vhe(void) > > { > > extern char vectors[]; /* kernel exception vectors */ > > write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2); > > @@ -133,7 +132,7 @@ static void __hyp_text __deactivate_traps_vhe(void) > > write_sysreg(vectors, vbar_el1); > > } > > > > -static void __hyp_text __deactivate_traps_nvhe(void) > > +static inline void __hyp_text __deactivate_traps_nvhe(void) > > { > > u64 mdcr_el2 = read_sysreg(mdcr_el2); > > > > @@ -147,11 +146,7 @@ static void __hyp_text __deactivate_traps_nvhe(void) > > write_sysreg(CPTR_EL2_DEFAULT, cptr_el2); > > } > > > > -static hyp_alternate_select(__deactivate_traps_arch, > > - __deactivate_traps_nvhe, __deactivate_traps_vhe, > > - ARM64_HAS_VIRT_HOST_EXTN); > > - > > -static void __hyp_text __deactivate_traps(struct kvm_vcpu *vcpu) > > +static inline void __hyp_text __deactivate_traps(struct kvm_vcpu *vcpu) > > { > > /* > > * If we pended a virtual abort, preserve it until it gets > > @@ -162,7 +157,10 @@ static void __hyp_text __deactivate_traps(struct kvm_vcpu *vcpu) > > if (vcpu->arch.hcr_el2 & HCR_VSE) > > vcpu->arch.hcr_el2 = read_sysreg(hcr_el2); > > > > - __deactivate_traps_arch()(); > > + if (has_vhe()) > > + deactivate_traps_vhe(); > > + else > > + __deactivate_traps_nvhe(); > > } > > > > void activate_traps_vhe_load(struct kvm_vcpu *vcpu) > > -- > > 2.14.2 > > > > Thanks, > > M. > > -- > Jazz is not dead, it just smell funny.
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c index 5e94955b89ea..0e54fe2aab1c 100644 --- a/arch/arm64/kvm/hyp/switch.c +++ b/arch/arm64/kvm/hyp/switch.c @@ -85,7 +85,7 @@ static void __hyp_text __deactivate_traps_common(void) write_sysreg(0, pmuserenr_el0); } -static void __hyp_text __activate_traps_vhe(struct kvm_vcpu *vcpu) +static inline void activate_traps_vhe(struct kvm_vcpu *vcpu) { u64 val; @@ -97,7 +97,7 @@ static void __hyp_text __activate_traps_vhe(struct kvm_vcpu *vcpu) write_sysreg(kvm_get_hyp_vector(), vbar_el1); } -static void __hyp_text __activate_traps_nvhe(struct kvm_vcpu *vcpu) +static inline void __hyp_text __activate_traps_nvhe(struct kvm_vcpu *vcpu) { u64 val; @@ -108,11 +108,7 @@ static void __hyp_text __activate_traps_nvhe(struct kvm_vcpu *vcpu) write_sysreg(val, cptr_el2); } -static hyp_alternate_select(__activate_traps_arch, - __activate_traps_nvhe, __activate_traps_vhe, - ARM64_HAS_VIRT_HOST_EXTN); - -static void __hyp_text __activate_traps(struct kvm_vcpu *vcpu) +static inline void __hyp_text __activate_traps(struct kvm_vcpu *vcpu) { u64 hcr = vcpu->arch.hcr_el2; @@ -122,10 +118,13 @@ static void __hyp_text __activate_traps(struct kvm_vcpu *vcpu) write_sysreg_s(vcpu->arch.vsesr_el2, SYS_VSESR_EL2); __activate_traps_fpsimd32(vcpu); - __activate_traps_arch()(vcpu); + if (has_vhe()) + activate_traps_vhe(vcpu); + else + __activate_traps_nvhe(vcpu); } -static void __hyp_text __deactivate_traps_vhe(void) +static inline void deactivate_traps_vhe(void) { extern char vectors[]; /* kernel exception vectors */ write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2); @@ -133,7 +132,7 @@ static void __hyp_text __deactivate_traps_vhe(void) write_sysreg(vectors, vbar_el1); } -static void __hyp_text __deactivate_traps_nvhe(void) +static inline void __hyp_text __deactivate_traps_nvhe(void) { u64 mdcr_el2 = read_sysreg(mdcr_el2); @@ -147,11 +146,7 @@ static void __hyp_text __deactivate_traps_nvhe(void) write_sysreg(CPTR_EL2_DEFAULT, cptr_el2); } -static hyp_alternate_select(__deactivate_traps_arch, - __deactivate_traps_nvhe, __deactivate_traps_vhe, - ARM64_HAS_VIRT_HOST_EXTN); - -static void __hyp_text __deactivate_traps(struct kvm_vcpu *vcpu) +static inline void __hyp_text __deactivate_traps(struct kvm_vcpu *vcpu) { /* * If we pended a virtual abort, preserve it until it gets @@ -162,7 +157,10 @@ static void __hyp_text __deactivate_traps(struct kvm_vcpu *vcpu) if (vcpu->arch.hcr_el2 & HCR_VSE) vcpu->arch.hcr_el2 = read_sysreg(hcr_el2); - __deactivate_traps_arch()(); + if (has_vhe()) + deactivate_traps_vhe(); + else + __deactivate_traps_nvhe(); } void activate_traps_vhe_load(struct kvm_vcpu *vcpu)
To make the code more readable and to avoid the overhead of a function call, let's get rid of a pair of the alternative function selectors and explicitly call the VHE and non-VHE functions using the has_vhe() static key based selector instead, telling the compiler to try to inline the static function if it can. Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org> --- arch/arm64/kvm/hyp/switch.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-)