| Message ID | 20240605-kvm-arm64-sme-assert-v2-1-54391b0032f4@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] KVM: arm64: Fix confusion in documentation for pKVM SME assert | expand |
Hi Mark, On Wed, Jun 5, 2024 at 7:11 PM Mark Brown <broonie@kernel.org> 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. Reviewed-by: Fuad Tabba <tabba@google.com Cheers, /fuad > > Fixes: afb91f5f8ad7 ("KVM: arm64: Ensure that SME controls are disabled in protected mode") > Signed-off-by: Mark Brown <broonie@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 521b32868d0d..820769567080 100644 > --- a/arch/arm64/kvm/fpsimd.c > +++ b/arch/arm64/kvm/fpsimd.c > @@ -92,11 +92,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. > + * The pKVM hypervisor does not yet understand how to save or > + * restore SME state for the host so double check that if we > + * are running with pKVM we have disabled SME. The hypervisor > + * enforces this when the guest is run, this check is for > + * clearer diagnostics. > */ > - 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: afb91f5f8ad7af172d993a34fde1947892408f53 > change-id: 20240604-kvm-arm64-sme-assert-5ad755d4e8a6 > > Best regards, > -- > Mark Brown <broonie@kernel.org> >
diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c index 521b32868d0d..820769567080 100644 --- a/arch/arm64/kvm/fpsimd.c +++ b/arch/arm64/kvm/fpsimd.c @@ -92,11 +92,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. + * The pKVM hypervisor does not yet understand how to save or + * restore SME state for the host so double check that if we + * are running with pKVM we have disabled SME. The hypervisor + * enforces this when the guest is run, this check is for + * clearer diagnostics. */ - 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 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") Signed-off-by: Mark Brown <broonie@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: afb91f5f8ad7af172d993a34fde1947892408f53 change-id: 20240604-kvm-arm64-sme-assert-5ad755d4e8a6 Best regards,