kvmarm.lists.cs.columbia.edu archive mirror
 help / color / mirror / Atom feed
* [PATCHv2 0/3]  arm64: kvm: avoid referencing cpu_hwcaps from hyp
@ 2020-10-26 13:49 Mark Rutland
  2020-10-26 13:49 ` [PATCHv2 1/3] arm64: kvm: factor out is_{vhe,nvhe}_hyp_code() Mark Rutland
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Mark Rutland @ 2020-10-26 13:49 UTC (permalink / raw)
  To: kvmarm, linux-arm-kernel; +Cc: maz, will

In a few places we use cpus_have_const_cap() in hyp code, usually
because this is hidden within a helper that's also used in regular
kernel context. As cpus_have_const_cap() generates code to read the
cpu_hwcaps array before capabilities are finalized, this means we
generate some potentially-unsound references to regular kernel VAs, but
this these are redundant as capabilities are finalized before we
initialize the kvm hyp code.

This series gets rid of the redundant code by automatically upgrading
cpust_have_const_cap() to cpus_have_final_cap() when used in hyp code.
This allows us to avoid creating an NVHE alias for the cpu_hwcaps array,
so we can catch if we accidentally introduce an runtime reference to
this (e.g. via cpus_have_cap()).

Since v1 [1]:
* Trivial rebase to v5.10-rc1

[1] https://lore.kernel.org/r/20201007125211.30043-1-mark.rutland@arm.com

Mark Rutland (3):
  arm64: kvm: factor out is_{vhe,nvhe}_hyp_code()
  arm64: cpufeature: reorder cpus_have_{const,final}_cap()
  arm64: cpufeature: upgrade hyp caps to final

 arch/arm64/include/asm/cpufeature.h | 40 ++++++++++++++++++++++++++++---------
 arch/arm64/include/asm/virt.h       |  9 ++++-----
 arch/arm64/kernel/image-vars.h      |  1 -
 3 files changed, 35 insertions(+), 15 deletions(-)

-- 
2.11.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCHv2 1/3] arm64: kvm: factor out is_{vhe,nvhe}_hyp_code()
  2020-10-26 13:49 [PATCHv2 0/3] arm64: kvm: avoid referencing cpu_hwcaps from hyp Mark Rutland
@ 2020-10-26 13:49 ` Mark Rutland
  2020-10-30  8:21   ` Will Deacon
  2020-10-26 13:49 ` [PATCHv2 2/3] arm64: cpufeature: reorder cpus_have_{const, final}_cap() Mark Rutland
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Mark Rutland @ 2020-10-26 13:49 UTC (permalink / raw)
  To: kvmarm, linux-arm-kernel; +Cc: maz, will

Currently has_vhe() detects whether it is being compiled for VHE/NVHE
hyp code based on preprocessor definitions, and uses this knowledge to
avoid redundant runtime checks.

There are other cases where we'd like to use this knowledge, so let's
factor the preprocessor checks out into separate helpers.

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: David Brazdil <dbrazdil@google.com>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/include/asm/virt.h | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h
index 09977acc007d1..300be14ba77b2 100644
--- a/arch/arm64/include/asm/virt.h
+++ b/arch/arm64/include/asm/virt.h
@@ -83,16 +83,27 @@ static inline bool is_kernel_in_hyp_mode(void)
 	return read_sysreg(CurrentEL) == CurrentEL_EL2;
 }
 
+static __always_inline bool is_vhe_hyp_code(void)
+{
+	/* Only defined for code run in VHE hyp context */
+	return __is_defined(__KVM_VHE_HYPERVISOR__);
+}
+
+static __always_inline bool is_nvhe_hyp_code(void)
+{
+	/* Only defined for code run in NVHE hyp context */
+	return __is_defined(__KVM_NVHE_HYPERVISOR__);
+}
+
 static __always_inline bool has_vhe(void)
 {
 	/*
-	 * The following macros are defined for code specic to VHE/nVHE.
-	 * If has_vhe() is inlined into those compilation units, it can
-	 * be determined statically. Otherwise fall back to caps.
+	 * Code only run in VHE/NVHE hyp context can assume VHE is present or
+	 * absent. Otherwise fall back to caps.
 	 */
-	if (__is_defined(__KVM_VHE_HYPERVISOR__))
+	if (is_vhe_hyp_code())
 		return true;
-	else if (__is_defined(__KVM_NVHE_HYPERVISOR__))
+	else if (is_nvhe_hyp_code())
 		return false;
 	else
 		return cpus_have_final_cap(ARM64_HAS_VIRT_HOST_EXTN);
-- 
2.11.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCHv2 2/3] arm64: cpufeature: reorder cpus_have_{const, final}_cap()
  2020-10-26 13:49 [PATCHv2 0/3] arm64: kvm: avoid referencing cpu_hwcaps from hyp Mark Rutland
  2020-10-26 13:49 ` [PATCHv2 1/3] arm64: kvm: factor out is_{vhe,nvhe}_hyp_code() Mark Rutland
@ 2020-10-26 13:49 ` Mark Rutland
  2020-10-30  8:18   ` [PATCHv2 2/3] arm64: cpufeature: reorder cpus_have_{const,final}_cap() Will Deacon
  2020-10-26 13:49 ` [PATCHv2 3/3] arm64: cpufeature: upgrade hyp caps to final Mark Rutland
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Mark Rutland @ 2020-10-26 13:49 UTC (permalink / raw)
  To: kvmarm, linux-arm-kernel; +Cc: maz, will

In a subsequent patch we'll modify cpus_have_const_cap() to call
cpus_have_final_cap(), and hence we need to define cpus_have_final_cap()
first.

To make subsequent changes easier to follow, this patch reorders the two
without making any other changes.

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: David Brazdil <dbrazdil@google.com>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/include/asm/cpufeature.h | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index f7e7144af174c..5d18c54507e6a 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -428,35 +428,35 @@ static __always_inline bool __cpus_have_const_cap(int num)
 }
 
 /*
- * Test for a capability, possibly with a runtime check.
+ * Test for a capability without a runtime check.
  *
- * Before capabilities are finalized, this behaves as cpus_have_cap().
+ * Before capabilities are finalized, this will BUG().
  * After capabilities are finalized, this is patched to avoid a runtime check.
  *
  * @num must be a compile-time constant.
  */
-static __always_inline bool cpus_have_const_cap(int num)
+static __always_inline bool cpus_have_final_cap(int num)
 {
 	if (system_capabilities_finalized())
 		return __cpus_have_const_cap(num);
 	else
-		return cpus_have_cap(num);
+		BUG();
 }
 
 /*
- * Test for a capability without a runtime check.
+ * Test for a capability, possibly with a runtime check.
  *
- * Before capabilities are finalized, this will BUG().
+ * Before capabilities are finalized, this behaves as cpus_have_cap().
  * After capabilities are finalized, this is patched to avoid a runtime check.
  *
  * @num must be a compile-time constant.
  */
-static __always_inline bool cpus_have_final_cap(int num)
+static __always_inline bool cpus_have_const_cap(int num)
 {
 	if (system_capabilities_finalized())
 		return __cpus_have_const_cap(num);
 	else
-		BUG();
+		return cpus_have_cap(num);
 }
 
 static inline void cpus_set_cap(unsigned int num)
-- 
2.11.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCHv2 3/3] arm64: cpufeature: upgrade hyp caps to final
  2020-10-26 13:49 [PATCHv2 0/3] arm64: kvm: avoid referencing cpu_hwcaps from hyp Mark Rutland
  2020-10-26 13:49 ` [PATCHv2 1/3] arm64: kvm: factor out is_{vhe,nvhe}_hyp_code() Mark Rutland
  2020-10-26 13:49 ` [PATCHv2 2/3] arm64: cpufeature: reorder cpus_have_{const, final}_cap() Mark Rutland
@ 2020-10-26 13:49 ` Mark Rutland
  2020-10-30  8:24   ` Will Deacon
  2020-10-29 19:48 ` [PATCHv2 0/3] arm64: kvm: avoid referencing cpu_hwcaps from hyp Marc Zyngier
  2020-10-30 10:59 ` Marc Zyngier
  4 siblings, 1 reply; 11+ messages in thread
From: Mark Rutland @ 2020-10-26 13:49 UTC (permalink / raw)
  To: kvmarm, linux-arm-kernel; +Cc: maz, will

We finalize caps before initializing kvm hyp code, and any use of
cpus_have_const_cap() in kvm hyp code generates redundant and
potentially unsound code to read the cpu_hwcaps array.

A number of helper functions used in both hyp context and regular kernel
context use cpus_have_const_cap(), as some regular kernel code runs
before the capabilities are finalized. It's tedious and error-prone to
write separate copies of these for hyp and non-hyp code.

So that we can avoid the redundant code, let's automatically upgrade
cpus_have_const_cap() to cpus_have_final_cap() when used in hyp context.
With this change, there's never a reason to access to cpu_hwcaps array
from hyp code, and we don't need to create an NVHE alias for this.

This should have no effect on non-hyp code.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: David Brazdil <dbrazdil@google.com>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/include/asm/cpufeature.h | 26 ++++++++++++++++++++++++--
 arch/arm64/include/asm/virt.h       | 12 ------------
 arch/arm64/kernel/image-vars.h      |  1 -
 3 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 5d18c54507e6a..97244d4feca9c 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -375,6 +375,23 @@ cpucap_multi_entry_cap_matches(const struct arm64_cpu_capabilities *entry,
 	return false;
 }
 
+static __always_inline bool is_vhe_hyp_code(void)
+{
+	/* Only defined for code run in VHE hyp context */
+	return __is_defined(__KVM_VHE_HYPERVISOR__);
+}
+
+static __always_inline bool is_nvhe_hyp_code(void)
+{
+	/* Only defined for code run in NVHE hyp context */
+	return __is_defined(__KVM_NVHE_HYPERVISOR__);
+}
+
+static __always_inline bool is_hyp_code(void)
+{
+	return is_vhe_hyp_code() || is_nvhe_hyp_code();
+}
+
 extern DECLARE_BITMAP(cpu_hwcaps, ARM64_NCAPS);
 extern struct static_key_false cpu_hwcap_keys[ARM64_NCAPS];
 extern struct static_key_false arm64_const_caps_ready;
@@ -444,8 +461,11 @@ static __always_inline bool cpus_have_final_cap(int num)
 }
 
 /*
- * Test for a capability, possibly with a runtime check.
+ * Test for a capability, possibly with a runtime check for non-hyp code.
  *
+ * For hyp code, this behaves the same as cpus_have_final_cap().
+ *
+ * For non-hyp code:
  * Before capabilities are finalized, this behaves as cpus_have_cap().
  * After capabilities are finalized, this is patched to avoid a runtime check.
  *
@@ -453,7 +473,9 @@ static __always_inline bool cpus_have_final_cap(int num)
  */
 static __always_inline bool cpus_have_const_cap(int num)
 {
-	if (system_capabilities_finalized())
+	if (is_hyp_code())
+		return cpus_have_final_cap(num);
+	else if (system_capabilities_finalized())
 		return __cpus_have_const_cap(num);
 	else
 		return cpus_have_cap(num);
diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h
index 300be14ba77b2..6069be50baf9f 100644
--- a/arch/arm64/include/asm/virt.h
+++ b/arch/arm64/include/asm/virt.h
@@ -83,18 +83,6 @@ static inline bool is_kernel_in_hyp_mode(void)
 	return read_sysreg(CurrentEL) == CurrentEL_EL2;
 }
 
-static __always_inline bool is_vhe_hyp_code(void)
-{
-	/* Only defined for code run in VHE hyp context */
-	return __is_defined(__KVM_VHE_HYPERVISOR__);
-}
-
-static __always_inline bool is_nvhe_hyp_code(void)
-{
-	/* Only defined for code run in NVHE hyp context */
-	return __is_defined(__KVM_NVHE_HYPERVISOR__);
-}
-
 static __always_inline bool has_vhe(void)
 {
 	/*
diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index 61684a5009148..c615b285ff5b3 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -87,7 +87,6 @@ KVM_NVHE_ALIAS(__icache_flags);
 /* Kernel symbols needed for cpus_have_final/const_caps checks. */
 KVM_NVHE_ALIAS(arm64_const_caps_ready);
 KVM_NVHE_ALIAS(cpu_hwcap_keys);
-KVM_NVHE_ALIAS(cpu_hwcaps);
 
 /* Static keys which are set if a vGIC trap should be handled in hyp. */
 KVM_NVHE_ALIAS(vgic_v2_cpuif_trap);
-- 
2.11.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCHv2 0/3]  arm64: kvm: avoid referencing cpu_hwcaps from hyp
  2020-10-26 13:49 [PATCHv2 0/3] arm64: kvm: avoid referencing cpu_hwcaps from hyp Mark Rutland
                   ` (2 preceding siblings ...)
  2020-10-26 13:49 ` [PATCHv2 3/3] arm64: cpufeature: upgrade hyp caps to final Mark Rutland
@ 2020-10-29 19:48 ` Marc Zyngier
  2020-10-30 10:59 ` Marc Zyngier
  4 siblings, 0 replies; 11+ messages in thread
From: Marc Zyngier @ 2020-10-29 19:48 UTC (permalink / raw)
  To: Mark Rutland, will, Catalin Marinas; +Cc: kvmarm, linux-arm-kernel

[+ Catalin]

On Mon, 26 Oct 2020 13:49:28 +0000,
Mark Rutland <mark.rutland@arm.com> wrote:
> 
> In a few places we use cpus_have_const_cap() in hyp code, usually
> because this is hidden within a helper that's also used in regular
> kernel context. As cpus_have_const_cap() generates code to read the
> cpu_hwcaps array before capabilities are finalized, this means we
> generate some potentially-unsound references to regular kernel VAs, but
> this these are redundant as capabilities are finalized before we
> initialize the kvm hyp code.
> 
> This series gets rid of the redundant code by automatically upgrading
> cpust_have_const_cap() to cpus_have_final_cap() when used in hyp code.
> This allows us to avoid creating an NVHE alias for the cpu_hwcaps array,
> so we can catch if we accidentally introduce an runtime reference to
> this (e.g. via cpus_have_cap()).
> 
> Since v1 [1]:
> * Trivial rebase to v5.10-rc1
> 
> [1] https://lore.kernel.org/r/20201007125211.30043-1-mark.rutland@arm.com
> 
> Mark Rutland (3):
>   arm64: kvm: factor out is_{vhe,nvhe}_hyp_code()
>   arm64: cpufeature: reorder cpus_have_{const,final}_cap()
>   arm64: cpufeature: upgrade hyp caps to final
> 
>  arch/arm64/include/asm/cpufeature.h | 40 ++++++++++++++++++++++++++++---------
>  arch/arm64/include/asm/virt.h       |  9 ++++-----
>  arch/arm64/kernel/image-vars.h      |  1 -
>  3 files changed, 35 insertions(+), 15 deletions(-)

Catalin, Will: can I get an Ack for patches 2 and 3? I'd be quite
happy to drop yet another reference from the nVHE object...

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCHv2 2/3] arm64: cpufeature: reorder cpus_have_{const,final}_cap()
  2020-10-26 13:49 ` [PATCHv2 2/3] arm64: cpufeature: reorder cpus_have_{const, final}_cap() Mark Rutland
@ 2020-10-30  8:18   ` Will Deacon
  2020-10-30  8:20     ` Will Deacon
  0 siblings, 1 reply; 11+ messages in thread
From: Will Deacon @ 2020-10-30  8:18 UTC (permalink / raw)
  To: Mark Rutland; +Cc: maz, kvmarm, linux-arm-kernel

On Mon, Oct 26, 2020 at 01:49:30PM +0000, Mark Rutland wrote:
> In a subsequent patch we'll modify cpus_have_const_cap() to call
> cpus_have_final_cap(), and hence we need to define cpus_have_final_cap()
> first.
> 
> To make subsequent changes easier to follow, this patch reorders the two
> without making any other changes.
> 
> There should be no functional change as a result of this patch.

You say this...

> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: David Brazdil <dbrazdil@google.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Will Deacon <will@kernel.org>
> ---
>  arch/arm64/include/asm/cpufeature.h | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index f7e7144af174c..5d18c54507e6a 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -428,35 +428,35 @@ static __always_inline bool __cpus_have_const_cap(int num)
>  }
>  
>  /*
> - * Test for a capability, possibly with a runtime check.
> + * Test for a capability without a runtime check.
>   *
> - * Before capabilities are finalized, this behaves as cpus_have_cap().
> + * Before capabilities are finalized, this will BUG().
>   * After capabilities are finalized, this is patched to avoid a runtime check.
>   *
>   * @num must be a compile-time constant.
>   */
> -static __always_inline bool cpus_have_const_cap(int num)
> +static __always_inline bool cpus_have_final_cap(int num)
>  {
>  	if (system_capabilities_finalized())
>  		return __cpus_have_const_cap(num);
>  	else
> -		return cpus_have_cap(num);
> +		BUG();

... but isn't the failure case of calling cpus_have_final_cap() early now
different? What does BUG() do at EL2 w/ nVHE?

Will
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCHv2 2/3] arm64: cpufeature: reorder cpus_have_{const,final}_cap()
  2020-10-30  8:18   ` [PATCHv2 2/3] arm64: cpufeature: reorder cpus_have_{const,final}_cap() Will Deacon
@ 2020-10-30  8:20     ` Will Deacon
  2020-10-30 14:18       ` Mark Rutland
  0 siblings, 1 reply; 11+ messages in thread
From: Will Deacon @ 2020-10-30  8:20 UTC (permalink / raw)
  To: Mark Rutland; +Cc: maz, kvmarm, linux-arm-kernel

On Fri, Oct 30, 2020 at 08:18:48AM +0000, Will Deacon wrote:
> On Mon, Oct 26, 2020 at 01:49:30PM +0000, Mark Rutland wrote:
> > In a subsequent patch we'll modify cpus_have_const_cap() to call
> > cpus_have_final_cap(), and hence we need to define cpus_have_final_cap()
> > first.
> > 
> > To make subsequent changes easier to follow, this patch reorders the two
> > without making any other changes.
> > 
> > There should be no functional change as a result of this patch.
> 
> You say this...
> 
> > 
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Cc: David Brazdil <dbrazdil@google.com>
> > Cc: Marc Zyngier <maz@kernel.org>
> > Cc: Will Deacon <will@kernel.org>
> > ---
> >  arch/arm64/include/asm/cpufeature.h | 16 ++++++++--------
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> > index f7e7144af174c..5d18c54507e6a 100644
> > --- a/arch/arm64/include/asm/cpufeature.h
> > +++ b/arch/arm64/include/asm/cpufeature.h
> > @@ -428,35 +428,35 @@ static __always_inline bool __cpus_have_const_cap(int num)
> >  }
> >  
> >  /*
> > - * Test for a capability, possibly with a runtime check.
> > + * Test for a capability without a runtime check.
> >   *
> > - * Before capabilities are finalized, this behaves as cpus_have_cap().
> > + * Before capabilities are finalized, this will BUG().
> >   * After capabilities are finalized, this is patched to avoid a runtime check.
> >   *
> >   * @num must be a compile-time constant.
> >   */
> > -static __always_inline bool cpus_have_const_cap(int num)
> > +static __always_inline bool cpus_have_final_cap(int num)
> >  {
> >  	if (system_capabilities_finalized())
> >  		return __cpus_have_const_cap(num);
> >  	else
> > -		return cpus_have_cap(num);
> > +		BUG();
> 
> ... but isn't the failure case of calling cpus_have_final_cap() early now
> different? What does BUG() do at EL2 w/ nVHE?

Ah no, sorry, I see you're just moving things around and the diff makes it
look confusing (that and I've been up since 5:30 for KVM Forum).

So on closer inspection:

Acked-by: Will Deacon <will@kernel.org>

Will
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCHv2 1/3] arm64: kvm: factor out is_{vhe,nvhe}_hyp_code()
  2020-10-26 13:49 ` [PATCHv2 1/3] arm64: kvm: factor out is_{vhe,nvhe}_hyp_code() Mark Rutland
@ 2020-10-30  8:21   ` Will Deacon
  0 siblings, 0 replies; 11+ messages in thread
From: Will Deacon @ 2020-10-30  8:21 UTC (permalink / raw)
  To: Mark Rutland; +Cc: maz, kvmarm, linux-arm-kernel

On Mon, Oct 26, 2020 at 01:49:29PM +0000, Mark Rutland wrote:
> Currently has_vhe() detects whether it is being compiled for VHE/NVHE
> hyp code based on preprocessor definitions, and uses this knowledge to
> avoid redundant runtime checks.
> 
> There are other cases where we'd like to use this knowledge, so let's
> factor the preprocessor checks out into separate helpers.
> 
> There should be no functional change as a result of this patch.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: David Brazdil <dbrazdil@google.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Will Deacon <will@kernel.org>
> ---
>  arch/arm64/include/asm/virt.h | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)

Acked-by: Will Deacon <will@kernel.org>

Will
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCHv2 3/3] arm64: cpufeature: upgrade hyp caps to final
  2020-10-26 13:49 ` [PATCHv2 3/3] arm64: cpufeature: upgrade hyp caps to final Mark Rutland
@ 2020-10-30  8:24   ` Will Deacon
  0 siblings, 0 replies; 11+ messages in thread
From: Will Deacon @ 2020-10-30  8:24 UTC (permalink / raw)
  To: Mark Rutland; +Cc: maz, kvmarm, linux-arm-kernel

On Mon, Oct 26, 2020 at 01:49:31PM +0000, Mark Rutland wrote:
> We finalize caps before initializing kvm hyp code, and any use of
> cpus_have_const_cap() in kvm hyp code generates redundant and
> potentially unsound code to read the cpu_hwcaps array.
> 
> A number of helper functions used in both hyp context and regular kernel
> context use cpus_have_const_cap(), as some regular kernel code runs
> before the capabilities are finalized. It's tedious and error-prone to
> write separate copies of these for hyp and non-hyp code.
> 
> So that we can avoid the redundant code, let's automatically upgrade
> cpus_have_const_cap() to cpus_have_final_cap() when used in hyp context.
> With this change, there's never a reason to access to cpu_hwcaps array
> from hyp code, and we don't need to create an NVHE alias for this.
> 
> This should have no effect on non-hyp code.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: David Brazdil <dbrazdil@google.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Will Deacon <will@kernel.org>
> ---
>  arch/arm64/include/asm/cpufeature.h | 26 ++++++++++++++++++++++++--
>  arch/arm64/include/asm/virt.h       | 12 ------------
>  arch/arm64/kernel/image-vars.h      |  1 -
>  3 files changed, 24 insertions(+), 15 deletions(-)

[...]

> diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
> index 61684a5009148..c615b285ff5b3 100644
> --- a/arch/arm64/kernel/image-vars.h
> +++ b/arch/arm64/kernel/image-vars.h
> @@ -87,7 +87,6 @@ KVM_NVHE_ALIAS(__icache_flags);
>  /* Kernel symbols needed for cpus_have_final/const_caps checks. */
>  KVM_NVHE_ALIAS(arm64_const_caps_ready);
>  KVM_NVHE_ALIAS(cpu_hwcap_keys);
> -KVM_NVHE_ALIAS(cpu_hwcaps);

Nice!

Acked-by: Will Deacon <will@kernel.org>

Will
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCHv2 0/3] arm64: kvm: avoid referencing cpu_hwcaps from hyp
  2020-10-26 13:49 [PATCHv2 0/3] arm64: kvm: avoid referencing cpu_hwcaps from hyp Mark Rutland
                   ` (3 preceding siblings ...)
  2020-10-29 19:48 ` [PATCHv2 0/3] arm64: kvm: avoid referencing cpu_hwcaps from hyp Marc Zyngier
@ 2020-10-30 10:59 ` Marc Zyngier
  4 siblings, 0 replies; 11+ messages in thread
From: Marc Zyngier @ 2020-10-30 10:59 UTC (permalink / raw)
  To: kvmarm, linux-arm-kernel, Mark Rutland; +Cc: will

On Mon, 26 Oct 2020 13:49:28 +0000, Mark Rutland wrote:
> In a few places we use cpus_have_const_cap() in hyp code, usually
> because this is hidden within a helper that's also used in regular
> kernel context. As cpus_have_const_cap() generates code to read the
> cpu_hwcaps array before capabilities are finalized, this means we
> generate some potentially-unsound references to regular kernel VAs, but
> this these are redundant as capabilities are finalized before we
> initialize the kvm hyp code.
> 
> [...]

Applied to next, thanks!

[1/3] KVM: arm64: Factor out is_{vhe,nvhe}_hyp_code()
      commit: e9a33caec90e05673e2f7fb7c80f172031964d25
[2/3] arm64: cpufeature: reorder cpus_have_{const, final}_cap()
      commit: dfc4e3f08903ed8fe0b66cc25b64524a82654166
[3/3] arm64: cpufeature: upgrade hyp caps to final
      commit: d86de40decaa14e6613af1b2783bf4d589d0f38b

Cheers,

	M.
-- 
Without deviation from the norm, progress is not possible.


_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCHv2 2/3] arm64: cpufeature: reorder cpus_have_{const,final}_cap()
  2020-10-30  8:20     ` Will Deacon
@ 2020-10-30 14:18       ` Mark Rutland
  0 siblings, 0 replies; 11+ messages in thread
From: Mark Rutland @ 2020-10-30 14:18 UTC (permalink / raw)
  To: Will Deacon; +Cc: maz, kvmarm, linux-arm-kernel

On Fri, Oct 30, 2020 at 08:20:14AM +0000, Will Deacon wrote:
> On Fri, Oct 30, 2020 at 08:18:48AM +0000, Will Deacon wrote:
> > On Mon, Oct 26, 2020 at 01:49:30PM +0000, Mark Rutland wrote:
> > > In a subsequent patch we'll modify cpus_have_const_cap() to call
> > > cpus_have_final_cap(), and hence we need to define cpus_have_final_cap()
> > > first.
> > > 
> > > To make subsequent changes easier to follow, this patch reorders the two
> > > without making any other changes.
> > > 
> > > There should be no functional change as a result of this patch.
> > 
> > You say this...

[...]

> > > -static __always_inline bool cpus_have_const_cap(int num)
> > > +static __always_inline bool cpus_have_final_cap(int num)
> > >  {
> > >  	if (system_capabilities_finalized())
> > >  		return __cpus_have_const_cap(num);
> > >  	else
> > > -		return cpus_have_cap(num);
> > > +		BUG();
> > 
> > ... but isn't the failure case of calling cpus_have_final_cap() early now
> > different? What does BUG() do at EL2 w/ nVHE?
> 
> Ah no, sorry, I see you're just moving things around and the diff makes it
> look confusing (that and I've been up since 5:30 for KVM Forum).

Indeed; the diff was even more confusing before I split this from the
changes in the next patch!

> So on closer inspection:
> 
> Acked-by: Will Deacon <will@kernel.org>

Cheers!

Mark.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-10-30 14:19 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-26 13:49 [PATCHv2 0/3] arm64: kvm: avoid referencing cpu_hwcaps from hyp Mark Rutland
2020-10-26 13:49 ` [PATCHv2 1/3] arm64: kvm: factor out is_{vhe,nvhe}_hyp_code() Mark Rutland
2020-10-30  8:21   ` Will Deacon
2020-10-26 13:49 ` [PATCHv2 2/3] arm64: cpufeature: reorder cpus_have_{const, final}_cap() Mark Rutland
2020-10-30  8:18   ` [PATCHv2 2/3] arm64: cpufeature: reorder cpus_have_{const,final}_cap() Will Deacon
2020-10-30  8:20     ` Will Deacon
2020-10-30 14:18       ` Mark Rutland
2020-10-26 13:49 ` [PATCHv2 3/3] arm64: cpufeature: upgrade hyp caps to final Mark Rutland
2020-10-30  8:24   ` Will Deacon
2020-10-29 19:48 ` [PATCHv2 0/3] arm64: kvm: avoid referencing cpu_hwcaps from hyp Marc Zyngier
2020-10-30 10:59 ` Marc Zyngier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).