The hyp_adr/ldr_this_cpu helpers were introduced for use in hyp code
because they always needed to use TPIDR_EL2 for base, while
adr/ldr_this_cpu from kernel proper would select between TPIDR_EL2 and
_EL1 based on VHE/nVHE.
Simplify this now that the hyp mode case can be handled using the
__KVM_VHE/NVHE_HYPERVISOR__ macros.
Acked-by: Andrew Scull <[email protected]>
Signed-off-by: David Brazdil <[email protected]>
---
arch/arm64/include/asm/assembler.h | 27 +++++++++++++++++----------
arch/arm64/include/asm/kvm_asm.h | 14 +-------------
arch/arm64/kvm/hyp/hyp-entry.S | 2 +-
3 files changed, 19 insertions(+), 24 deletions(-)
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index 54d181177656..f79231a0f949 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -218,6 +218,21 @@ lr .req x30 // link register
str \src, [\tmp, :lo12:\sym]
.endm
+ /*
+ * @dst: destination register (32 or 64 bit wide)
+ */
+ .macro this_cpu_offset, dst
+#if defined(__KVM_NVHE_HYPERVISOR__) || defined(__KVM_VHE_HYPERVISOR__)
+ mrs \dst, tpidr_el2
+#else
+alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
+ mrs \dst, tpidr_el1
+alternative_else
+ mrs \dst, tpidr_el2
+alternative_endif
+#endif
+ .endm
+
/*
* @dst: Result of per_cpu(sym, smp_processor_id()) (can be SP)
* @sym: The name of the per-cpu variable
@@ -226,11 +241,7 @@ lr .req x30 // link register
.macro adr_this_cpu, dst, sym, tmp
adrp \tmp, \sym
add \dst, \tmp, #:lo12:\sym
-alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
- mrs \tmp, tpidr_el1
-alternative_else
- mrs \tmp, tpidr_el2
-alternative_endif
+ this_cpu_offset \tmp
add \dst, \dst, \tmp
.endm
@@ -241,11 +252,7 @@ alternative_endif
*/
.macro ldr_this_cpu dst, sym, tmp
adr_l \dst, \sym
-alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
- mrs \tmp, tpidr_el1
-alternative_else
- mrs \tmp, tpidr_el2
-alternative_endif
+ this_cpu_offset \tmp
ldr \dst, [\dst, \tmp]
.endm
diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
index c196eec25498..cf9456663289 100644
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -173,20 +173,8 @@ extern char __smccc_workaround_1_smc[__SMCCC_WORKAROUND_1_SMC_SZ];
#else /* __ASSEMBLY__ */
-.macro hyp_adr_this_cpu reg, sym, tmp
- adr_l \reg, \sym
- mrs \tmp, tpidr_el2
- add \reg, \reg, \tmp
-.endm
-
-.macro hyp_ldr_this_cpu reg, sym, tmp
- adr_l \reg, \sym
- mrs \tmp, tpidr_el2
- ldr \reg, [\reg, \tmp]
-.endm
-
.macro get_host_ctxt reg, tmp
- hyp_adr_this_cpu \reg, kvm_host_data, \tmp
+ adr_this_cpu \reg, kvm_host_data, \tmp
add \reg, \reg, #HOST_DATA_CONTEXT
.endm
diff --git a/arch/arm64/kvm/hyp/hyp-entry.S b/arch/arm64/kvm/hyp/hyp-entry.S
index 46b4dab933d0..fba91c2ab410 100644
--- a/arch/arm64/kvm/hyp/hyp-entry.S
+++ b/arch/arm64/kvm/hyp/hyp-entry.S
@@ -132,7 +132,7 @@ alternative_cb_end
str x0, [x2, #VCPU_WORKAROUND_FLAGS]
/* Check that we actually need to perform the call */
- hyp_ldr_this_cpu x0, arm64_ssbd_callback_required, x2
+ ldr_this_cpu x0, arm64_ssbd_callback_required, x2
cbz x0, wa2_end
mov w0, #ARM_SMCCC_ARCH_WORKAROUND_2
--
2.28.0.618.gf4bc123cb7-goog
On Wed, Sep 16, 2020 at 06:34:33PM +0100, David Brazdil wrote:
> The hyp_adr/ldr_this_cpu helpers were introduced for use in hyp code
> because they always needed to use TPIDR_EL2 for base, while
> adr/ldr_this_cpu from kernel proper would select between TPIDR_EL2 and
> _EL1 based on VHE/nVHE.
>
> Simplify this now that the hyp mode case can be handled using the
> __KVM_VHE/NVHE_HYPERVISOR__ macros.
>
> Acked-by: Andrew Scull <[email protected]>
> Signed-off-by: David Brazdil <[email protected]>
> ---
> arch/arm64/include/asm/assembler.h | 27 +++++++++++++++++----------
> arch/arm64/include/asm/kvm_asm.h | 14 +-------------
> arch/arm64/kvm/hyp/hyp-entry.S | 2 +-
> 3 files changed, 19 insertions(+), 24 deletions(-)
>
> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
> index 54d181177656..f79231a0f949 100644
> --- a/arch/arm64/include/asm/assembler.h
> +++ b/arch/arm64/include/asm/assembler.h
> @@ -218,6 +218,21 @@ lr .req x30 // link register
> str \src, [\tmp, :lo12:\sym]
> .endm
>
> + /*
> + * @dst: destination register (32 or 64 bit wide)
> + */
> + .macro this_cpu_offset, dst
> +#if defined(__KVM_NVHE_HYPERVISOR__) || defined(__KVM_VHE_HYPERVISOR__)
> + mrs \dst, tpidr_el2
> +#else
> +alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
> + mrs \dst, tpidr_el1
> +alternative_else
> + mrs \dst, tpidr_el2
> +alternative_endif
> +#endif
Cosmetic, but I think it would be cleaner just to define two variants of the
macro here:
#if defined(__KVM_NVHE_HYPERVISOR__) || defined(__KVM_VHE_HYPERVISOR__)
.macro this_cpu_offset, dst
mrs \dst, tpidr_el2
.endm
#else
.macro this_cpu_offset, dst
alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
mrs \dst, tpidr_el1
alternative_else
mrs \dst, tpidr_el2
alternative_endif
.endm
#endif
(and should we have a shorthand __HYPERVISOR define to avoid the NVHE || VHE
logic?)
With that:
Acked-by: Will Deacon <[email protected]>
Will
Hi Will,
> Cosmetic, but I think it would be cleaner just to define two variants of the
> macro here:
>
> #if defined(__KVM_NVHE_HYPERVISOR__) || defined(__KVM_VHE_HYPERVISOR__)
> .macro this_cpu_offset, dst
> mrs \dst, tpidr_el2
> .endm
> #else
> .macro this_cpu_offset, dst
> alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
> mrs \dst, tpidr_el1
> alternative_else
> mrs \dst, tpidr_el2
> alternative_endif
> .endm
> #endif
Sure.
> (and should we have a shorthand __HYPERVISOR define to avoid the NVHE || VHE
> logic?)
Happy to add this but let's agree on the details.
* name: just __HYPERVISOR or __KVM_HYPERVISOR__?
* defined where? I'm wary of defining it in a header file because then sombody
will forget to add it and 'ifdef HYP' will be skipped. So I'd put this as
another '-D__HYPERVISOR' in the build rules. Do you agree?
David
On Mon, Sep 21, 2020 at 03:53:20PM +0100, David Brazdil wrote:
> > Cosmetic, but I think it would be cleaner just to define two variants of the
> > macro here:
> >
> > #if defined(__KVM_NVHE_HYPERVISOR__) || defined(__KVM_VHE_HYPERVISOR__)
> > .macro this_cpu_offset, dst
> > mrs \dst, tpidr_el2
> > .endm
> > #else
> > .macro this_cpu_offset, dst
> > alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
> > mrs \dst, tpidr_el1
> > alternative_else
> > mrs \dst, tpidr_el2
> > alternative_endif
> > .endm
> > #endif
>
> Sure.
Ta.
> > (and should we have a shorthand __HYPERVISOR define to avoid the NVHE || VHE
> > logic?)
>
> Happy to add this but let's agree on the details.
> * name: just __HYPERVISOR or __KVM_HYPERVISOR__?
Hey, I leave the hard decisions to others!
> * defined where? I'm wary of defining it in a header file because then sombody
> will forget to add it and 'ifdef HYP' will be skipped. So I'd put this as
> another '-D__HYPERVISOR' in the build rules. Do you agree?
Hmm, that's a good point. Leave that part as-is for now then and we can
tackle it separately if it comes up again.
Will