[PATCH] KVM: arm64: Add missing memory barriers when switching to pKVM's hyp pgd

[PATCH] KVM: arm64: Add missing memory barriers when switching to pKVM's hyp pgd

[Will Deacon]

In commit f320bc742bc23 ("KVM: arm64: Prepare the creation of s1
mappings at EL2"), pKVM switches from a temporary host-provided
page-table to its own page-table at EL2. Since there is only a single
TTBR for the nVHE hypervisor, this involves disabling and re-enabling
the MMU in __pkvm_init_switch_pgd().

Unfortunately, the memory barriers here are not quite correct.
Specifically:

  - A DSB is required to complete the TLB invalidation executed while
    the MMU is disabled.

  - An ISB is required to make the new TTBR value visible to the
    page-table walker before the MMU is enabled in the SCTLR.

An earlier version of the patch actually got this correct:

  https://lore.kernel.org/lkml/20210304184717.GB21795@willie-the-truck/

but thanks to some badly worded review comments from yours truly, these
were dropped for the version that was eventually merged.

Bring back the barriers and fix the potential issue (but note that this
was found by code inspection).

Cc: Quentin Perret <qperret@google.com>
Fixes: f320bc742bc23 ("KVM: arm64: Prepare the creation of s1 mappings at EL2")
Signed-off-by: Will Deacon <will@kernel.org>
---
 arch/arm64/kvm/hyp/nvhe/hyp-init.S | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-init.S b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
index 1cc06e6797bd..9205e95f8529 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-init.S
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
@@ -292,6 +292,8 @@ alternative_else_nop_endif
 	mov	sp, x0
 
 	/* And turn the MMU back on! */
+	dsb	nsh
+	isb
 	set_sctlr_el2	x2
 	ret	x1
 SYM_FUNC_END(__pkvm_init_switch_pgd)
-- 
2.43.0.472.g3155946c3a-goog


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

Re: [PATCH] KVM: arm64: Add missing memory barriers when switching to pKVM's hyp pgd

[Marc Zyngier]

On Thu, 04 Jan 2024 16:42:20 +0000,
Will Deacon <will@kernel.org> wrote:
> 
> In commit f320bc742bc23 ("KVM: arm64: Prepare the creation of s1
> mappings at EL2"), pKVM switches from a temporary host-provided
> page-table to its own page-table at EL2. Since there is only a single
> TTBR for the nVHE hypervisor, this involves disabling and re-enabling
> the MMU in __pkvm_init_switch_pgd().
> 
> Unfortunately, the memory barriers here are not quite correct.
> Specifically:
> 
>   - A DSB is required to complete the TLB invalidation executed while
>     the MMU is disabled.
> 
>   - An ISB is required to make the new TTBR value visible to the
>     page-table walker before the MMU is enabled in the SCTLR.
> 
> An earlier version of the patch actually got this correct:
> 
>   https://lore.kernel.org/lkml/20210304184717.GB21795@willie-the-truck/
> 
> but thanks to some badly worded review comments from yours truly, these
> were dropped for the version that was eventually merged.
> 
> Bring back the barriers and fix the potential issue (but note that this
> was found by code inspection).
> 
> Cc: Quentin Perret <qperret@google.com>
> Fixes: f320bc742bc23 ("KVM: arm64: Prepare the creation of s1 mappings at EL2")
> Signed-off-by: Will Deacon <will@kernel.org>
> ---
>  arch/arm64/kvm/hyp/nvhe/hyp-init.S | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-init.S b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> index 1cc06e6797bd..9205e95f8529 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> @@ -292,6 +292,8 @@ alternative_else_nop_endif
>  	mov	sp, x0
>  
>  	/* And turn the MMU back on! */
> +	dsb	nsh
> +	isb
>  	set_sctlr_el2	x2
>  	ret	x1
>  SYM_FUNC_END(__pkvm_init_switch_pgd)

I'm happy to take the patch as is, but maybe we consider moving these
barriers into the set_sctlr helper as a future cleanup?

Thanks,

	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

Re: [PATCH] KVM: arm64: Add missing memory barriers when switching to pKVM's hyp pgd

[Oliver Upton]

On Thu, Jan 04, 2024 at 04:42:20PM +0000, Will Deacon wrote:
> In commit f320bc742bc23 ("KVM: arm64: Prepare the creation of s1
> mappings at EL2"), pKVM switches from a temporary host-provided
> page-table to its own page-table at EL2. Since there is only a single
> TTBR for the nVHE hypervisor, this involves disabling and re-enabling
> the MMU in __pkvm_init_switch_pgd().
> 
> Unfortunately, the memory barriers here are not quite correct.
> Specifically:
> 
>   - A DSB is required to complete the TLB invalidation executed while
>     the MMU is disabled.
> 
>   - An ISB is required to make the new TTBR value visible to the
>     page-table walker before the MMU is enabled in the SCTLR.
> 
> An earlier version of the patch actually got this correct:
> 
>   https://lore.kernel.org/lkml/20210304184717.GB21795@willie-the-truck/
> 
> but thanks to some badly worded review comments from yours truly, these
> were dropped for the version that was eventually merged.
> 
> Bring back the barriers and fix the potential issue (but note that this
> was found by code inspection).
> 
> Cc: Quentin Perret <qperret@google.com>
> Fixes: f320bc742bc23 ("KVM: arm64: Prepare the creation of s1 mappings at EL2")
> Signed-off-by: Will Deacon <will@kernel.org>

+1 to Marc's suggestion about folding the appropriate barriers into the
macro, but fine with this as is:

Reviewed-by: Oliver Upton <oliver.upton@linux.dev>

-- 
Thanks,
Oliver

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

Re: [PATCH] KVM: arm64: Add missing memory barriers when switching to pKVM's hyp pgd

[Marc Zyngier]

On Thu, 4 Jan 2024 16:42:20 +0000, Will Deacon wrote:
> In commit f320bc742bc23 ("KVM: arm64: Prepare the creation of s1
> mappings at EL2"), pKVM switches from a temporary host-provided
> page-table to its own page-table at EL2. Since there is only a single
> TTBR for the nVHE hypervisor, this involves disabling and re-enabling
> the MMU in __pkvm_init_switch_pgd().
> 
> Unfortunately, the memory barriers here are not quite correct.
> Specifically:
> 
> [...]

Applied to next, thanks!

[1/1] KVM: arm64: Add missing memory barriers when switching to pKVM's hyp pgd
      commit: 040113fa32f27096f531c377001936e0d7964597

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