diff mbox series

[v7] KVM: arm64: Fix confusion in documentation for pKVM SME assert

Message ID 20250212-kvm-arm64-sme-assert-v7-1-0f786db838d3@kernel.org (mailing list archive)
State New
Headers show
Series [v7] KVM: arm64: Fix confusion in documentation for pKVM SME assert | expand

Commit Message

Mark Brown Feb. 12, 2025, 12:44 a.m. UTC
As raised in the review comments for the original patch the assert and
comment added in afb91f5f8ad7 ("KVM: arm64: Ensure that SME controls are
disabled in protected mode") are bogus. The comments says that we check
that we do not have SME enabled for a pKVM guest but the assert actually
checks to see if the host has anything set in SVCR which is unrelated to
the guest features or state, regardless of if those guests are protected
or not. This check is also made in the hypervisor, it will refuse to run
a guest if the check fails, so it appears that the assert here is
intended to improve diagnostics.

Update the comment to reflect the check in the code, and to clarify that
we do actually enforce this in the hypervisor. While we're here also
update to use a WARN_ON_ONCE() to avoid log spam if this triggers.

Fixes: afb91f5f8ad7 ("KVM: arm64: Ensure that SME controls are disabled in protected mode")
Reviewed-by: Fuad Tabba <tabba@google.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
This has been sent with v6.10 with only positive review comments after
the first revision, if there is some issue with the change please share
it.

To: Marc Zyngier <maz@kernel.org>
To: Oliver Upton <oliver.upton@linux.dev>
To: James Morse <james.morse@arm.com>
To: Suzuki K Poulose <suzuki.poulose@arm.com>
To: Catalin Marinas <catalin.marinas@arm.com>
To: Will Deacon <will@kernel.org>
To: Fuad Tabba <tabba@google.com>
Cc: Mark Rutland <mark.rutland@arm.com>
---
Changes in v7:
- Reword the comment.
- Link to v6: https://lore.kernel.org/r/20250210-kvm-arm64-sme-assert-v6-1-cc26c46d1b43@kernel.org

Changes in v6:
- Rebase onto v6.14-rc1.
- Link to v5: https://lore.kernel.org/r/20241210-kvm-arm64-sme-assert-v5-1-995c8dd1025b@kernel.org

Changes in v5:
- Rebase onto v6.13-rc1.
- Link to v4: https://lore.kernel.org/r/20240930-kvm-arm64-sme-assert-v4-1-3c9df71db688@kernel.org

Changes in v4:
- Rebase onto v6.12-rc1
- Link to v3: https://lore.kernel.org/r/20240730-kvm-arm64-sme-assert-v3-1-8699454e5cb8@kernel.org

Changes in v3:
- Rebase onto v6.11-rc1.
- Link to v2: https://lore.kernel.org/r/20240605-kvm-arm64-sme-assert-v2-1-54391b0032f4@kernel.org

Changes in v2:
- Commit message tweaks.
- Change the assert to WARN_ON_ONCE().
- Link to v1: https://lore.kernel.org/r/20240604-kvm-arm64-sme-assert-v1-1-5d98348d00f8@kernel.org
---
 arch/arm64/kvm/fpsimd.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)


---
base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b
change-id: 20240604-kvm-arm64-sme-assert-5ad755d4e8a6

Best regards,

Comments

Mark Rutland Feb. 12, 2025, 11:11 a.m. UTC | #1
On Wed, Feb 12, 2025 at 12:44:57AM +0000, Mark Brown wrote:
> As raised in the review comments for the original patch the assert and
> comment added in afb91f5f8ad7 ("KVM: arm64: Ensure that SME controls are
> disabled in protected mode") are bogus. The comments says that we check
> that we do not have SME enabled for a pKVM guest but the assert actually
> checks to see if the host has anything set in SVCR which is unrelated to
> the guest features or state, regardless of if those guests are protected
> or not. This check is also made in the hypervisor, it will refuse to run
> a guest if the check fails, so it appears that the assert here is
> intended to improve diagnostics.
> 
> Update the comment to reflect the check in the code, and to clarify that
> we do actually enforce this in the hypervisor. While we're here also
> update to use a WARN_ON_ONCE() to avoid log spam if this triggers.
> 
> Fixes: afb91f5f8ad7 ("KVM: arm64: Ensure that SME controls are disabled in protected mode")
> Reviewed-by: Fuad Tabba <tabba@google.com>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
> This has been sent with v6.10 with only positive review comments after
> the first revision, if there is some issue with the change please share
> it.
> 
> To: Marc Zyngier <maz@kernel.org>
> To: Oliver Upton <oliver.upton@linux.dev>
> To: James Morse <james.morse@arm.com>
> To: Suzuki K Poulose <suzuki.poulose@arm.com>
> To: Catalin Marinas <catalin.marinas@arm.com>
> To: Will Deacon <will@kernel.org>
> To: Fuad Tabba <tabba@google.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> ---
> Changes in v7:
> - Reword the comment.
> - Link to v6: https://lore.kernel.org/r/20250210-kvm-arm64-sme-assert-v6-1-cc26c46d1b43@kernel.org
> 
> Changes in v6:
> - Rebase onto v6.14-rc1.
> - Link to v5: https://lore.kernel.org/r/20241210-kvm-arm64-sme-assert-v5-1-995c8dd1025b@kernel.org
> 
> Changes in v5:
> - Rebase onto v6.13-rc1.
> - Link to v4: https://lore.kernel.org/r/20240930-kvm-arm64-sme-assert-v4-1-3c9df71db688@kernel.org
> 
> Changes in v4:
> - Rebase onto v6.12-rc1
> - Link to v3: https://lore.kernel.org/r/20240730-kvm-arm64-sme-assert-v3-1-8699454e5cb8@kernel.org
> 
> Changes in v3:
> - Rebase onto v6.11-rc1.
> - Link to v2: https://lore.kernel.org/r/20240605-kvm-arm64-sme-assert-v2-1-54391b0032f4@kernel.org
> 
> Changes in v2:
> - Commit message tweaks.
> - Change the assert to WARN_ON_ONCE().
> - Link to v1: https://lore.kernel.org/r/20240604-kvm-arm64-sme-assert-v1-1-5d98348d00f8@kernel.org
> ---
>  arch/arm64/kvm/fpsimd.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
> index 4d3d1a2eb157047b4b2488e9c4ffaabc6f5a0818..e37e53883c357093ff4455f5afdaec90e662d744 100644
> --- a/arch/arm64/kvm/fpsimd.c
> +++ b/arch/arm64/kvm/fpsimd.c
> @@ -93,11 +93,14 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
>  	}
>  
>  	/*
> -	 * If normal guests gain SME support, maintain this behavior for pKVM
> -	 * guests, which don't support SME.
> +	 * Protected and non-protected KVM modes require that
> +	 * SVCR.{SM,ZA} == {0,0} when entering a guest so that no
> +	 * host/guest SME state needs to be saved/restored by hyp code.
> +	 *
> +	 * In protected mode, hyp code will verify this later.
>  	 */
> -	WARN_ON(is_protected_kvm_enabled() && system_supports_sme() &&
> -		read_sysreg_s(SYS_SVCR));
> +	WARN_ON_ONCE(is_protected_kvm_enabled() && system_supports_sme() &&
> +		     read_sysreg_s(SYS_SVCR));

As I mentioned on the last round, we can drop the is_protected_kvm_enabled()
check, i.e. have:

	/*
	 * Protected and non-protected KVM modes require that
	 * SVCR.{SM,ZA} == {0,0} when entering a guest so that no
	 * host/guest SME state needs to be saved/restored by hyp code.
	 *
	 * In protected mode, hyp code will verify this later.
	 */
	WARN_ON_ONCE(system_supports_sme() && read_sysreg_s(SYS_SVCR));

Either way:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Marc, are you happy to queue this atop the recent fixes from me? Those
try to ensure SVCR.{SM,ZA} == {0,0} regardless of whether KVM is in
protected mode.

Mark.
Oliver Upton Feb. 13, 2025, 6:14 a.m. UTC | #2
On Wed, Feb 12, 2025 at 11:11:04AM +0000, Mark Rutland wrote:
> On Wed, Feb 12, 2025 at 12:44:57AM +0000, Mark Brown wrote:
> > As raised in the review comments for the original patch the assert and
> > comment added in afb91f5f8ad7 ("KVM: arm64: Ensure that SME controls are
> > disabled in protected mode") are bogus. The comments says that we check
> > that we do not have SME enabled for a pKVM guest but the assert actually
> > checks to see if the host has anything set in SVCR which is unrelated to
> > the guest features or state, regardless of if those guests are protected
> > or not. This check is also made in the hypervisor, it will refuse to run
> > a guest if the check fails, so it appears that the assert here is
> > intended to improve diagnostics.
> > 
> > Update the comment to reflect the check in the code, and to clarify that
> > we do actually enforce this in the hypervisor. While we're here also
> > update to use a WARN_ON_ONCE() to avoid log spam if this triggers.
> > 
> > Fixes: afb91f5f8ad7 ("KVM: arm64: Ensure that SME controls are disabled in protected mode")
> > Reviewed-by: Fuad Tabba <tabba@google.com>
> > Signed-off-by: Mark Brown <broonie@kernel.org>

I don't think a Fixes tag is warranted here, this doesn't fix any
functional issue.

> > diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
> > index 4d3d1a2eb157047b4b2488e9c4ffaabc6f5a0818..e37e53883c357093ff4455f5afdaec90e662d744 100644
> > --- a/arch/arm64/kvm/fpsimd.c
> > +++ b/arch/arm64/kvm/fpsimd.c
> > @@ -93,11 +93,14 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
> >  	}
> >  
> >  	/*
> > -	 * If normal guests gain SME support, maintain this behavior for pKVM
> > -	 * guests, which don't support SME.
> > +	 * Protected and non-protected KVM modes require that
> > +	 * SVCR.{SM,ZA} == {0,0} when entering a guest so that no
> > +	 * host/guest SME state needs to be saved/restored by hyp code.
> > +	 *
> > +	 * In protected mode, hyp code will verify this later.
> >  	 */
> > -	WARN_ON(is_protected_kvm_enabled() && system_supports_sme() &&
> > -		read_sysreg_s(SYS_SVCR));
> > +	WARN_ON_ONCE(is_protected_kvm_enabled() && system_supports_sme() &&
> > +		     read_sysreg_s(SYS_SVCR));
> 
> As I mentioned on the last round, we can drop the is_protected_kvm_enabled()
> check, i.e. have:
> 
> 	/*
> 	 * Protected and non-protected KVM modes require that
> 	 * SVCR.{SM,ZA} == {0,0} when entering a guest so that no
> 	 * host/guest SME state needs to be saved/restored by hyp code.
> 	 *
> 	 * In protected mode, hyp code will verify this later.
> 	 */
> 	WARN_ON_ONCE(system_supports_sme() && read_sysreg_s(SYS_SVCR));
> 
> Either way:
> 
> Acked-by: Mark Rutland <mark.rutland@arm.com>
> 
> Marc, are you happy to queue this atop the recent fixes from me? Those
> try to ensure SVCR.{SM,ZA} == {0,0} regardless of whether KVM is in
> protected mode.

I'll pick it up for 6.15 if Marc doesn't grab it as a fix.
Marc Zyngier Feb. 13, 2025, 8:55 a.m. UTC | #3
On Wed, 12 Feb 2025 11:11:04 +0000,
Mark Rutland <mark.rutland@arm.com> wrote:
> 
> On Wed, Feb 12, 2025 at 12:44:57AM +0000, Mark Brown wrote:
> > As raised in the review comments for the original patch the assert and
> > comment added in afb91f5f8ad7 ("KVM: arm64: Ensure that SME controls are
> > disabled in protected mode") are bogus. The comments says that we check
> > that we do not have SME enabled for a pKVM guest but the assert actually
> > checks to see if the host has anything set in SVCR which is unrelated to
> > the guest features or state, regardless of if those guests are protected
> > or not. This check is also made in the hypervisor, it will refuse to run
> > a guest if the check fails, so it appears that the assert here is
> > intended to improve diagnostics.
> > 
> > Update the comment to reflect the check in the code, and to clarify that
> > we do actually enforce this in the hypervisor. While we're here also
> > update to use a WARN_ON_ONCE() to avoid log spam if this triggers.
> > 
> > Fixes: afb91f5f8ad7 ("KVM: arm64: Ensure that SME controls are disabled in protected mode")
> > Reviewed-by: Fuad Tabba <tabba@google.com>
> > Signed-off-by: Mark Brown <broonie@kernel.org>
> > ---
> > This has been sent with v6.10 with only positive review comments after
> > the first revision, if there is some issue with the change please share
> > it.
> > 
> > To: Marc Zyngier <maz@kernel.org>
> > To: Oliver Upton <oliver.upton@linux.dev>
> > To: James Morse <james.morse@arm.com>
> > To: Suzuki K Poulose <suzuki.poulose@arm.com>
> > To: Catalin Marinas <catalin.marinas@arm.com>
> > To: Will Deacon <will@kernel.org>
> > To: Fuad Tabba <tabba@google.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > ---
> > Changes in v7:
> > - Reword the comment.
> > - Link to v6: https://lore.kernel.org/r/20250210-kvm-arm64-sme-assert-v6-1-cc26c46d1b43@kernel.org
> > 
> > Changes in v6:
> > - Rebase onto v6.14-rc1.
> > - Link to v5: https://lore.kernel.org/r/20241210-kvm-arm64-sme-assert-v5-1-995c8dd1025b@kernel.org
> > 
> > Changes in v5:
> > - Rebase onto v6.13-rc1.
> > - Link to v4: https://lore.kernel.org/r/20240930-kvm-arm64-sme-assert-v4-1-3c9df71db688@kernel.org
> > 
> > Changes in v4:
> > - Rebase onto v6.12-rc1
> > - Link to v3: https://lore.kernel.org/r/20240730-kvm-arm64-sme-assert-v3-1-8699454e5cb8@kernel.org
> > 
> > Changes in v3:
> > - Rebase onto v6.11-rc1.
> > - Link to v2: https://lore.kernel.org/r/20240605-kvm-arm64-sme-assert-v2-1-54391b0032f4@kernel.org
> > 
> > Changes in v2:
> > - Commit message tweaks.
> > - Change the assert to WARN_ON_ONCE().
> > - Link to v1: https://lore.kernel.org/r/20240604-kvm-arm64-sme-assert-v1-1-5d98348d00f8@kernel.org
> > ---
> >  arch/arm64/kvm/fpsimd.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
> > index 4d3d1a2eb157047b4b2488e9c4ffaabc6f5a0818..e37e53883c357093ff4455f5afdaec90e662d744 100644
> > --- a/arch/arm64/kvm/fpsimd.c
> > +++ b/arch/arm64/kvm/fpsimd.c
> > @@ -93,11 +93,14 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
> >  	}
> >  
> >  	/*
> > -	 * If normal guests gain SME support, maintain this behavior for pKVM
> > -	 * guests, which don't support SME.
> > +	 * Protected and non-protected KVM modes require that
> > +	 * SVCR.{SM,ZA} == {0,0} when entering a guest so that no
> > +	 * host/guest SME state needs to be saved/restored by hyp code.
> > +	 *
> > +	 * In protected mode, hyp code will verify this later.
> >  	 */
> > -	WARN_ON(is_protected_kvm_enabled() && system_supports_sme() &&
> > -		read_sysreg_s(SYS_SVCR));
> > +	WARN_ON_ONCE(is_protected_kvm_enabled() && system_supports_sme() &&
> > +		     read_sysreg_s(SYS_SVCR));
> 
> As I mentioned on the last round, we can drop the is_protected_kvm_enabled()
> check, i.e. have:
> 
> 	/*
> 	 * Protected and non-protected KVM modes require that
> 	 * SVCR.{SM,ZA} == {0,0} when entering a guest so that no
> 	 * host/guest SME state needs to be saved/restored by hyp code.
> 	 *
> 	 * In protected mode, hyp code will verify this later.
> 	 */
> 	WARN_ON_ONCE(system_supports_sme() && read_sysreg_s(SYS_SVCR));
> 
> Either way:
> 
> Acked-by: Mark Rutland <mark.rutland@arm.com>
> 
> Marc, are you happy to queue this atop the recent fixes from me? Those
> try to ensure SVCR.{SM,ZA} == {0,0} regardless of whether KVM is in
> protected mode.

In all honesty, I find that at this stage, the comment just gets in
the way and is over-describing what is at stake here.

The

 	WARN_ON_ONCE(system_supports_sme() && read_sysreg_s(SYS_SVCR));

is really the only thing that matters. It perfectly shows what we are
checking for, and doesn't need an exegesis.

As for the Fixes: tag, and given the magnitude of the actual fixes
that are already queued, I don't think we need it.

Thanks,

	M.
Mark Rutland Feb. 13, 2025, 9:24 a.m. UTC | #4
On Thu, Feb 13, 2025 at 08:55:52AM +0000, Marc Zyngier wrote:
> On Wed, 12 Feb 2025 11:11:04 +0000,
> Mark Rutland <mark.rutland@arm.com> wrote:
> > On Wed, Feb 12, 2025 at 12:44:57AM +0000, Mark Brown wrote:
> > > diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
> > > index 4d3d1a2eb157047b4b2488e9c4ffaabc6f5a0818..e37e53883c357093ff4455f5afdaec90e662d744 100644
> > > --- a/arch/arm64/kvm/fpsimd.c
> > > +++ b/arch/arm64/kvm/fpsimd.c
> > > @@ -93,11 +93,14 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
> > >  	}
> > >  
> > >  	/*
> > > -	 * If normal guests gain SME support, maintain this behavior for pKVM
> > > -	 * guests, which don't support SME.
> > > +	 * Protected and non-protected KVM modes require that
> > > +	 * SVCR.{SM,ZA} == {0,0} when entering a guest so that no
> > > +	 * host/guest SME state needs to be saved/restored by hyp code.
> > > +	 *
> > > +	 * In protected mode, hyp code will verify this later.
> > >  	 */
> > > -	WARN_ON(is_protected_kvm_enabled() && system_supports_sme() &&
> > > -		read_sysreg_s(SYS_SVCR));
> > > +	WARN_ON_ONCE(is_protected_kvm_enabled() && system_supports_sme() &&
> > > +		     read_sysreg_s(SYS_SVCR));
> > 
> > As I mentioned on the last round, we can drop the is_protected_kvm_enabled()
> > check, i.e. have:
> > 
> > 	/*
> > 	 * Protected and non-protected KVM modes require that
> > 	 * SVCR.{SM,ZA} == {0,0} when entering a guest so that no
> > 	 * host/guest SME state needs to be saved/restored by hyp code.
> > 	 *
> > 	 * In protected mode, hyp code will verify this later.
> > 	 */
> > 	WARN_ON_ONCE(system_supports_sme() && read_sysreg_s(SYS_SVCR));
> > 
> > Either way:
> > 
> > Acked-by: Mark Rutland <mark.rutland@arm.com>
> > 
> > Marc, are you happy to queue this atop the recent fixes from me? Those
> > try to ensure SVCR.{SM,ZA} == {0,0} regardless of whether KVM is in
> > protected mode.
> 
> In all honesty, I find that at this stage, the comment just gets in
> the way and is over-describing what is at stake here.
> 
> The
> 
>  	WARN_ON_ONCE(system_supports_sme() && read_sysreg_s(SYS_SVCR));
> 
> is really the only thing that matters. It perfectly shows what we are
> checking for, and doesn't need an exegesis.
> 
> As for the Fixes: tag, and given the magnitude of the actual fixes
> that are already queued, I don't think we need it.

That's fair; if you haven't spun a patch for that already, I guess we're
after the following?

Mark.

---->8----
From 4d05f6dd6d39c747c175782b7b44daa775251994 Mon Sep 17 00:00:00 2001
From: Mark Rutland <mark.rutland@arm.com>
Date: Thu, 13 Feb 2025 09:15:31 +0000
Subject: [PATCH] KVM: arm64: Simplify warning in kvm_arch_vcpu_load_fp()

At the end of kvm_arch_vcpu_load_fp() we check that no bits are set in
SVCR. We only check this for protected mode despite this mattering
equally for non-protected mode, and the comment above this is confusing.

Remove the comment and simplify the check, moving from WARN_ON() to
WARN_ON_ONCE() to avoid spamming the log.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm64/kvm/fpsimd.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
index 3cbb999419af7..7f6e43d256915 100644
--- a/arch/arm64/kvm/fpsimd.c
+++ b/arch/arm64/kvm/fpsimd.c
@@ -65,12 +65,7 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
 	fpsimd_save_and_flush_cpu_state();
 	*host_data_ptr(fp_owner) = FP_STATE_FREE;
 
-	/*
-	 * If normal guests gain SME support, maintain this behavior for pKVM
-	 * guests, which don't support SME.
-	 */
-	WARN_ON(is_protected_kvm_enabled() && system_supports_sme() &&
-		read_sysreg_s(SYS_SVCR));
+	WARN_ON_ONCE(system_supports_sme() && read_sysreg_s(SYS_SVCR));
 }
 
 /*
Marc Zyngier Feb. 13, 2025, 10:56 a.m. UTC | #5
On Thu, 13 Feb 2025 09:24:22 +0000,
Mark Rutland <mark.rutland@arm.com> wrote:
> 
> On Thu, Feb 13, 2025 at 08:55:52AM +0000, Marc Zyngier wrote:
> > On Wed, 12 Feb 2025 11:11:04 +0000,
> > Mark Rutland <mark.rutland@arm.com> wrote:
> > > On Wed, Feb 12, 2025 at 12:44:57AM +0000, Mark Brown wrote:
> > > > diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
> > > > index 4d3d1a2eb157047b4b2488e9c4ffaabc6f5a0818..e37e53883c357093ff4455f5afdaec90e662d744 100644
> > > > --- a/arch/arm64/kvm/fpsimd.c
> > > > +++ b/arch/arm64/kvm/fpsimd.c
> > > > @@ -93,11 +93,14 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
> > > >  	}
> > > >  
> > > >  	/*
> > > > -	 * If normal guests gain SME support, maintain this behavior for pKVM
> > > > -	 * guests, which don't support SME.
> > > > +	 * Protected and non-protected KVM modes require that
> > > > +	 * SVCR.{SM,ZA} == {0,0} when entering a guest so that no
> > > > +	 * host/guest SME state needs to be saved/restored by hyp code.
> > > > +	 *
> > > > +	 * In protected mode, hyp code will verify this later.
> > > >  	 */
> > > > -	WARN_ON(is_protected_kvm_enabled() && system_supports_sme() &&
> > > > -		read_sysreg_s(SYS_SVCR));
> > > > +	WARN_ON_ONCE(is_protected_kvm_enabled() && system_supports_sme() &&
> > > > +		     read_sysreg_s(SYS_SVCR));
> > > 
> > > As I mentioned on the last round, we can drop the is_protected_kvm_enabled()
> > > check, i.e. have:
> > > 
> > > 	/*
> > > 	 * Protected and non-protected KVM modes require that
> > > 	 * SVCR.{SM,ZA} == {0,0} when entering a guest so that no
> > > 	 * host/guest SME state needs to be saved/restored by hyp code.
> > > 	 *
> > > 	 * In protected mode, hyp code will verify this later.
> > > 	 */
> > > 	WARN_ON_ONCE(system_supports_sme() && read_sysreg_s(SYS_SVCR));
> > > 
> > > Either way:
> > > 
> > > Acked-by: Mark Rutland <mark.rutland@arm.com>
> > > 
> > > Marc, are you happy to queue this atop the recent fixes from me? Those
> > > try to ensure SVCR.{SM,ZA} == {0,0} regardless of whether KVM is in
> > > protected mode.
> > 
> > In all honesty, I find that at this stage, the comment just gets in
> > the way and is over-describing what is at stake here.
> > 
> > The
> > 
> >  	WARN_ON_ONCE(system_supports_sme() && read_sysreg_s(SYS_SVCR));
> > 
> > is really the only thing that matters. It perfectly shows what we are
> > checking for, and doesn't need an exegesis.
> > 
> > As for the Fixes: tag, and given the magnitude of the actual fixes
> > that are already queued, I don't think we need it.
> 
> That's fair; if you haven't spun a patch for that already, I guess we're
> after the following?

Yup. Applied to fixes.

Thanks,

	M.
diff mbox series

Patch

diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
index 4d3d1a2eb157047b4b2488e9c4ffaabc6f5a0818..e37e53883c357093ff4455f5afdaec90e662d744 100644
--- a/arch/arm64/kvm/fpsimd.c
+++ b/arch/arm64/kvm/fpsimd.c
@@ -93,11 +93,14 @@  void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
 	}
 
 	/*
-	 * If normal guests gain SME support, maintain this behavior for pKVM
-	 * guests, which don't support SME.
+	 * Protected and non-protected KVM modes require that
+	 * SVCR.{SM,ZA} == {0,0} when entering a guest so that no
+	 * host/guest SME state needs to be saved/restored by hyp code.
+	 *
+	 * In protected mode, hyp code will verify this later.
 	 */
-	WARN_ON(is_protected_kvm_enabled() && system_supports_sme() &&
-		read_sysreg_s(SYS_SVCR));
+	WARN_ON_ONCE(is_protected_kvm_enabled() && system_supports_sme() &&
+		     read_sysreg_s(SYS_SVCR));
 }
 
 /*