* [PATCH v7] KVM: arm64: Fix confusion in documentation for pKVM SME assert
@ 2025-02-12 0:44 Mark Brown
2025-02-12 11:11 ` Mark Rutland
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2025-02-12 0:44 UTC (permalink / raw)
To: linux-arm-kernel, kvmarm, linux-kernel
Cc: Fuad Tabba, Mark Brown, Marc Zyngier, Oliver Upton, James Morse,
Suzuki K Poulose, Catalin Marinas, Will Deacon, Mark Rutland
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));
}
/*
---
base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b
change-id: 20240604-kvm-arm64-sme-assert-5ad755d4e8a6
Best regards,
--
Mark Brown <broonie@kernel.org>
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v7] KVM: arm64: Fix confusion in documentation for pKVM SME assert
2025-02-12 0:44 [PATCH v7] KVM: arm64: Fix confusion in documentation for pKVM SME assert Mark Brown
@ 2025-02-12 11:11 ` Mark Rutland
2025-02-13 6:14 ` Oliver Upton
2025-02-13 8:55 ` Marc Zyngier
0 siblings, 2 replies; 6+ messages in thread
From: Mark Rutland @ 2025-02-12 11:11 UTC (permalink / raw)
To: Mark Brown, Marc Zyngier
Cc: linux-arm-kernel, kvmarm, linux-kernel, Fuad Tabba, Oliver Upton,
James Morse, Suzuki K Poulose, Catalin Marinas, Will Deacon
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v7] KVM: arm64: Fix confusion in documentation for pKVM SME assert
2025-02-12 11:11 ` Mark Rutland
@ 2025-02-13 6:14 ` Oliver Upton
2025-02-13 8:55 ` Marc Zyngier
1 sibling, 0 replies; 6+ messages in thread
From: Oliver Upton @ 2025-02-13 6:14 UTC (permalink / raw)
To: Mark Rutland
Cc: Mark Brown, Marc Zyngier, linux-arm-kernel, kvmarm, linux-kernel,
Fuad Tabba, James Morse, Suzuki K Poulose, Catalin Marinas,
Will Deacon
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.
--
Thanks,
Oliver
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v7] KVM: arm64: Fix confusion in documentation for pKVM SME assert
2025-02-12 11:11 ` Mark Rutland
2025-02-13 6:14 ` Oliver Upton
@ 2025-02-13 8:55 ` Marc Zyngier
2025-02-13 9:24 ` Mark Rutland
1 sibling, 1 reply; 6+ messages in thread
From: Marc Zyngier @ 2025-02-13 8:55 UTC (permalink / raw)
To: Mark Rutland
Cc: Mark Brown, linux-arm-kernel, kvmarm, linux-kernel, Fuad Tabba,
Oliver Upton, James Morse, Suzuki K Poulose, Catalin Marinas,
Will Deacon
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.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v7] KVM: arm64: Fix confusion in documentation for pKVM SME assert
2025-02-13 8:55 ` Marc Zyngier
@ 2025-02-13 9:24 ` Mark Rutland
2025-02-13 10:56 ` Marc Zyngier
0 siblings, 1 reply; 6+ messages in thread
From: Mark Rutland @ 2025-02-13 9:24 UTC (permalink / raw)
To: Marc Zyngier
Cc: Mark Brown, linux-arm-kernel, kvmarm, linux-kernel, Fuad Tabba,
Oliver Upton, James Morse, Suzuki K Poulose, Catalin Marinas,
Will Deacon
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));
}
/*
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v7] KVM: arm64: Fix confusion in documentation for pKVM SME assert
2025-02-13 9:24 ` Mark Rutland
@ 2025-02-13 10:56 ` Marc Zyngier
0 siblings, 0 replies; 6+ messages in thread
From: Marc Zyngier @ 2025-02-13 10:56 UTC (permalink / raw)
To: Mark Rutland
Cc: Mark Brown, linux-arm-kernel, kvmarm, linux-kernel, Fuad Tabba,
Oliver Upton, James Morse, Suzuki K Poulose, Catalin Marinas,
Will Deacon
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.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-02-13 10:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-12 0:44 [PATCH v7] KVM: arm64: Fix confusion in documentation for pKVM SME assert Mark Brown
2025-02-12 11:11 ` Mark Rutland
2025-02-13 6:14 ` Oliver Upton
2025-02-13 8:55 ` Marc Zyngier
2025-02-13 9:24 ` Mark Rutland
2025-02-13 10:56 ` 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).