All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] kvm: arm64: correctly align nVHE percpu data
@ 2020-11-13  0:31 Jamie Iles
  2020-11-13 10:12 ` David Brazdil
  0 siblings, 1 reply; 5+ messages in thread
From: Jamie Iles @ 2020-11-13  0:31 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: Marc Zyngier, Jamie Iles, David Brazdil, Will Deacon

The nVHE percpu data is partially linked but the nVHE linker script did
not align the percpu section.  The PERCPU_INPUT macro would then align
the data to a page boundary:

  #define PERCPU_INPUT(cacheline)					\
  	__per_cpu_start = .;						\
  	*(.data..percpu..first)						\
  	. = ALIGN(PAGE_SIZE);						\
  	*(.data..percpu..page_aligned)					\
  	. = ALIGN(cacheline);						\
  	*(.data..percpu..read_mostly)					\
  	. = ALIGN(cacheline);						\
  	*(.data..percpu)						\
  	*(.data..percpu..shared_aligned)				\
  	PERCPU_DECRYPTED_SECTION					\
  	__per_cpu_end = .;

but then when the final vmlinux linking happens the hypervisor percpu
data is included after page alignment and so the offsets potentially
don't match.  This manifests as one of the CPUs getting lost when
running kvm-unit-tests on EL2 entry and subsequent soft lockup.

Fixes: 30c953911c43 ("kvm: arm64: Set up hyp percpu data for nVHE")
Cc: David Brazdil <dbrazdil@google.com>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Will Deacon <will@kernel.org>
Signed-off-by: Jamie Iles <jamie@nuviainc.com>
---
 arch/arm64/kvm/hyp/nvhe/hyp.lds.S | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/kvm/hyp/nvhe/hyp.lds.S b/arch/arm64/kvm/hyp/nvhe/hyp.lds.S
index bb2d986ff696..84edca959893 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp.lds.S
+++ b/arch/arm64/kvm/hyp/nvhe/hyp.lds.S
@@ -13,6 +13,7 @@
 
 SECTIONS {
 	HYP_SECTION(.text)
+	. = ALIGN(PAGE_SIZE);
 	HYP_SECTION_NAME(.data..percpu) : {
 		PERCPU_INPUT(L1_CACHE_BYTES)
 	}
-- 
2.27.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] kvm: arm64: correctly align nVHE percpu data
  2020-11-13  0:31 [PATCH] kvm: arm64: correctly align nVHE percpu data Jamie Iles
@ 2020-11-13 10:12 ` David Brazdil
  2020-11-13 15:04   ` Jamie Iles
  0 siblings, 1 reply; 5+ messages in thread
From: David Brazdil @ 2020-11-13 10:12 UTC (permalink / raw)
  To: Jamie Iles; +Cc: Marc Zyngier, Will Deacon, linux-arm-kernel

Hi Jamie,

On Fri, Nov 13, 2020 at 12:31:43AM +0000, Jamie Iles wrote:
> The nVHE percpu data is partially linked but the nVHE linker script did
> not align the percpu section.  The PERCPU_INPUT macro would then align
> the data to a page boundary:
> 
>   #define PERCPU_INPUT(cacheline)					\
>   	__per_cpu_start = .;						\
>   	*(.data..percpu..first)						\
>   	. = ALIGN(PAGE_SIZE);						\
>   	*(.data..percpu..page_aligned)					\
>   	. = ALIGN(cacheline);						\
>   	*(.data..percpu..read_mostly)					\
>   	. = ALIGN(cacheline);						\
>   	*(.data..percpu)						\
>   	*(.data..percpu..shared_aligned)				\
>   	PERCPU_DECRYPTED_SECTION					\
>   	__per_cpu_end = .;
> 
> but then when the final vmlinux linking happens the hypervisor percpu
> data is included after page alignment and so the offsets potentially
> don't match.  This manifests as one of the CPUs getting lost when
> running kvm-unit-tests on EL2 entry and subsequent soft lockup.

You're right - the internal alignment of these subsections can be off after
linking into vmlinux. Could you just elaborate on how you managed to trigger
this? Which offsets don't match?

> 
> Fixes: 30c953911c43 ("kvm: arm64: Set up hyp percpu data for nVHE")
> Cc: David Brazdil <dbrazdil@google.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Will Deacon <will@kernel.org>
> Signed-off-by: Jamie Iles <jamie@nuviainc.com>
> ---
>  arch/arm64/kvm/hyp/nvhe/hyp.lds.S | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp.lds.S b/arch/arm64/kvm/hyp/nvhe/hyp.lds.S
> index bb2d986ff696..84edca959893 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp.lds.S
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp.lds.S
> @@ -13,6 +13,7 @@
>  
>  SECTIONS {
>  	HYP_SECTION(.text)
> +	. = ALIGN(PAGE_SIZE);

Could you add a comment so that we don't forget why it was needed?

Thanks,
David

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH] kvm: arm64: correctly align nVHE percpu data
  2020-11-13 10:12 ` David Brazdil
@ 2020-11-13 15:04   ` Jamie Iles
  2020-11-16  9:24     ` David Brazdil
  2020-11-16  9:42     ` Marc Zyngier
  0 siblings, 2 replies; 5+ messages in thread
From: Jamie Iles @ 2020-11-13 15:04 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: Marc Zyngier, Jamie Iles, David Brazdil, Will Deacon

The nVHE percpu data is partially linked but the nVHE linker script did
not align the percpu section.  The PERCPU_INPUT macro would then align
the data to a page boundary:

  #define PERCPU_INPUT(cacheline)					\
  	__per_cpu_start = .;						\
  	*(.data..percpu..first)						\
  	. = ALIGN(PAGE_SIZE);						\
  	*(.data..percpu..page_aligned)					\
  	. = ALIGN(cacheline);						\
  	*(.data..percpu..read_mostly)					\
  	. = ALIGN(cacheline);						\
  	*(.data..percpu)						\
  	*(.data..percpu..shared_aligned)				\
  	PERCPU_DECRYPTED_SECTION					\
  	__per_cpu_end = .;

but then when the final vmlinux linking happens the hypervisor percpu
data is included after page alignment and so the offsets potentially
don't match.  On my build I saw that the .hyp.data..percpu section was
at address 0x20 and then the percpu data would begin at 0x1000 (because
of the page alignment in PERCPU_INPUT), but when linked into vmlinux,
everything would be shifted down by 0x20 bytes.

This manifests as one of the CPUs getting lost when running
kvm-unit-tests or starting any VM and subsequent soft lockup on a Cortex
A72 device.

Fixes: 30c953911c43 ("kvm: arm64: Set up hyp percpu data for nVHE")
Cc: David Brazdil <dbrazdil@google.com>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Will Deacon <will@kernel.org>
Signed-off-by: Jamie Iles <jamie@nuviainc.com>
---
v2: add clarifying commentary

 arch/arm64/kvm/hyp/nvhe/hyp.lds.S | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/kvm/hyp/nvhe/hyp.lds.S b/arch/arm64/kvm/hyp/nvhe/hyp.lds.S
index bb2d986ff696..a797abace13f 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp.lds.S
+++ b/arch/arm64/kvm/hyp/nvhe/hyp.lds.S
@@ -13,6 +13,11 @@
 
 SECTIONS {
 	HYP_SECTION(.text)
+	/*
+	 * .hyp..data..percpu needs to be page aligned to maintain the same
+	 * alignment for when linking into vmlinux.
+	 */
+	. = ALIGN(PAGE_SIZE);
 	HYP_SECTION_NAME(.data..percpu) : {
 		PERCPU_INPUT(L1_CACHE_BYTES)
 	}
-- 
2.27.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] kvm: arm64: correctly align nVHE percpu data
  2020-11-13 15:04   ` Jamie Iles
@ 2020-11-16  9:24     ` David Brazdil
  2020-11-16  9:42     ` Marc Zyngier
  1 sibling, 0 replies; 5+ messages in thread
From: David Brazdil @ 2020-11-16  9:24 UTC (permalink / raw)
  To: Jamie Iles; +Cc: Marc Zyngier, Will Deacon, linux-arm-kernel

Thanks Jamie

On Fri, Nov 13, 2020 at 03:04:06PM +0000, Jamie Iles wrote:
> The nVHE percpu data is partially linked but the nVHE linker script did
> not align the percpu section.  The PERCPU_INPUT macro would then align
> the data to a page boundary:
> 
>   #define PERCPU_INPUT(cacheline)					\
>   	__per_cpu_start = .;						\
>   	*(.data..percpu..first)						\
>   	. = ALIGN(PAGE_SIZE);						\
>   	*(.data..percpu..page_aligned)					\
>   	. = ALIGN(cacheline);						\
>   	*(.data..percpu..read_mostly)					\
>   	. = ALIGN(cacheline);						\
>   	*(.data..percpu)						\
>   	*(.data..percpu..shared_aligned)				\
>   	PERCPU_DECRYPTED_SECTION					\
>   	__per_cpu_end = .;
> 
> but then when the final vmlinux linking happens the hypervisor percpu
> data is included after page alignment and so the offsets potentially
> don't match.  On my build I saw that the .hyp.data..percpu section was
> at address 0x20 and then the percpu data would begin at 0x1000 (because
> of the page alignment in PERCPU_INPUT), but when linked into vmlinux,
> everything would be shifted down by 0x20 bytes.
> 
> This manifests as one of the CPUs getting lost when running
> kvm-unit-tests or starting any VM and subsequent soft lockup on a Cortex
> A72 device.
> 
> Fixes: 30c953911c43 ("kvm: arm64: Set up hyp percpu data for nVHE")
> Cc: David Brazdil <dbrazdil@google.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Will Deacon <will@kernel.org>

Acked-by: David Brazdil <dbrazdil@google.com>

> Signed-off-by: Jamie Iles <jamie@nuviainc.com>
> ---
> v2: add clarifying commentary
> 
>  arch/arm64/kvm/hyp/nvhe/hyp.lds.S | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp.lds.S b/arch/arm64/kvm/hyp/nvhe/hyp.lds.S
> index bb2d986ff696..a797abace13f 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp.lds.S
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp.lds.S
> @@ -13,6 +13,11 @@
>  
>  SECTIONS {
>  	HYP_SECTION(.text)
> +	/*
> +	 * .hyp..data..percpu needs to be page aligned to maintain the same
> +	 * alignment for when linking into vmlinux.
> +	 */
> +	. = ALIGN(PAGE_SIZE);
>  	HYP_SECTION_NAME(.data..percpu) : {
>  		PERCPU_INPUT(L1_CACHE_BYTES)
>  	}
> -- 
> 2.27.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] kvm: arm64: correctly align nVHE percpu data
  2020-11-13 15:04   ` Jamie Iles
  2020-11-16  9:24     ` David Brazdil
@ 2020-11-16  9:42     ` Marc Zyngier
  1 sibling, 0 replies; 5+ messages in thread
From: Marc Zyngier @ 2020-11-16  9:42 UTC (permalink / raw)
  To: Jamie Iles, linux-arm-kernel; +Cc: David Brazdil, Will Deacon

On Fri, 13 Nov 2020 15:04:06 +0000, Jamie Iles wrote:
> The nVHE percpu data is partially linked but the nVHE linker script did
> not align the percpu section.  The PERCPU_INPUT macro would then align
> the data to a page boundary:
> 
>   #define PERCPU_INPUT(cacheline)					\
>   	__per_cpu_start = .;						\
>   	*(.data..percpu..first)						\
>   	. = ALIGN(PAGE_SIZE);						\
>   	*(.data..percpu..page_aligned)					\
>   	. = ALIGN(cacheline);						\
>   	*(.data..percpu..read_mostly)					\
>   	. = ALIGN(cacheline);						\
>   	*(.data..percpu)						\
>   	*(.data..percpu..shared_aligned)				\
>   	PERCPU_DECRYPTED_SECTION					\
>   	__per_cpu_end = .;
> 
> [...]

Applied to kvm-arm64/fixes-5.10, thanks!

[1/1] KVM: arm64: Correctly align nVHE percpu data
      commit: 7bab16a6075b7b94999666355ab532c3dabb94f9

Cheers,

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



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, other threads:[~2020-11-16  9:42 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-13  0:31 [PATCH] kvm: arm64: correctly align nVHE percpu data Jamie Iles
2020-11-13 10:12 ` David Brazdil
2020-11-13 15:04   ` Jamie Iles
2020-11-16  9:24     ` David Brazdil
2020-11-16  9:42     ` Marc Zyngier

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.