[01/13] arm/arm64: Add new is_kernel_in_hyp_mode predicate
diff mbox

Message ID 1436372356-30410-2-git-send-email-marc.zyngier@arm.com
State New
Headers show

Commit Message

Marc Zyngier July 8, 2015, 4:19 p.m. UTC
With ARMv8.1 VHE extension, it will be possible to run the kernel
at EL2 (aka HYP mode). In order for the kernel to easily find out
where it is running, add a new predicate that returns whether or
not the kernel is in HYP mode.

For completeness, the 32bit code also get such a predicate (always
returning false) so that code common to both architecture (timers,
KVM) can use it transparently.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/include/asm/virt.h   |  5 +++++
 arch/arm64/include/asm/virt.h | 10 ++++++++++
 2 files changed, 15 insertions(+)

Comments

Mark Rutland July 9, 2015, 9:42 a.m. UTC | #1
Hi,

> +static inline bool is_kernel_in_hyp_mode(void)
> +{
> +	u64 el;
> +
> +	asm("mrs %0, CurrentEL" : "=r" (el));
> +	return el == CurrentEL_EL2;
> +}

If you can include cputype.h, I think this can be:

static inline bool is_kernel_in_hyp_mode(void)
{
	return read_cpuid(CurrentEL) == CurrentEL_EL2;
}

Mark.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marc Zyngier July 9, 2015, 10:05 a.m. UTC | #2
On 09/07/15 10:42, Mark Rutland wrote:
> Hi,
> 
>> +static inline bool is_kernel_in_hyp_mode(void)
>> +{
>> +	u64 el;
>> +
>> +	asm("mrs %0, CurrentEL" : "=r" (el));
>> +	return el == CurrentEL_EL2;
>> +}
> 
> If you can include cputype.h, I think this can be:
> 
> static inline bool is_kernel_in_hyp_mode(void)
> {
> 	return read_cpuid(CurrentEL) == CurrentEL_EL2;
> }

This would indeed work, but CurrentEL is hardly an ID register. I feel
slightly uncomfortable using read_cpuid (which might return a cached
version at some point) for random system registers.

Thoughts?

	M.
Mark Rutland July 9, 2015, 10:12 a.m. UTC | #3
On Thu, Jul 09, 2015 at 11:05:34AM +0100, Marc Zyngier wrote:
> On 09/07/15 10:42, Mark Rutland wrote:
> > Hi,
> > 
> >> +static inline bool is_kernel_in_hyp_mode(void)
> >> +{
> >> +	u64 el;
> >> +
> >> +	asm("mrs %0, CurrentEL" : "=r" (el));
> >> +	return el == CurrentEL_EL2;
> >> +}
> > 
> > If you can include cputype.h, I think this can be:
> > 
> > static inline bool is_kernel_in_hyp_mode(void)
> > {
> > 	return read_cpuid(CurrentEL) == CurrentEL_EL2;
> > }
> 
> This would indeed work, but CurrentEL is hardly an ID register. I feel
> slightly uncomfortable using read_cpuid (which might return a cached
> version at some point) for random system registers.
> 
> Thoughts?

I have no strong feelings either way, but I agree with the general
uneasiness w.r.t. what read_cpuid can be expected to do.

Elsewhere we just use inline asm to read non CPUID system registers, so
let's leave your patch as it was.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Will Deacon July 16, 2015, 6:08 p.m. UTC | #4
On Wed, Jul 08, 2015 at 05:19:04PM +0100, Marc Zyngier wrote:
> With ARMv8.1 VHE extension, it will be possible to run the kernel
> at EL2 (aka HYP mode). In order for the kernel to easily find out
> where it is running, add a new predicate that returns whether or
> not the kernel is in HYP mode.
> 
> For completeness, the 32bit code also get such a predicate (always
> returning false) so that code common to both architecture (timers,
> KVM) can use it transparently.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/include/asm/virt.h   |  5 +++++
>  arch/arm64/include/asm/virt.h | 10 ++++++++++
>  2 files changed, 15 insertions(+)

[...]

> +static inline bool is_kernel_in_hyp_mode(void)
> +{
> +	u64 el;
> +
> +	asm("mrs %0, CurrentEL" : "=r" (el));
> +	return el == CurrentEL_EL2;

Missing mask?

Will
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/arch/arm/include/asm/virt.h b/arch/arm/include/asm/virt.h
index 4371f45..b6a3cef 100644
--- a/arch/arm/include/asm/virt.h
+++ b/arch/arm/include/asm/virt.h
@@ -74,6 +74,11 @@  static inline bool is_hyp_mode_mismatched(void)
 {
 	return !!(__boot_cpu_mode & BOOT_CPU_MODE_MISMATCH);
 }
+
+static inline bool is_kernel_in_hyp_mode(void)
+{
+	return false;
+}
 #endif
 
 #endif /* __ASSEMBLY__ */
diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h
index 7a5df52..9f22dd6 100644
--- a/arch/arm64/include/asm/virt.h
+++ b/arch/arm64/include/asm/virt.h
@@ -23,6 +23,8 @@ 
 
 #ifndef __ASSEMBLY__
 
+#include <asm/ptrace.h>
+
 /*
  * __boot_cpu_mode records what mode CPUs were booted in.
  * A correctly-implemented bootloader must start all CPUs in the same mode:
@@ -50,6 +52,14 @@  static inline bool is_hyp_mode_mismatched(void)
 	return __boot_cpu_mode[0] != __boot_cpu_mode[1];
 }
 
+static inline bool is_kernel_in_hyp_mode(void)
+{
+	u64 el;
+
+	asm("mrs %0, CurrentEL" : "=r" (el));
+	return el == CurrentEL_EL2;
+}
+
 /* The section containing the hypervisor text */
 extern char __hyp_text_start[];
 extern char __hyp_text_end[];