linux-kernel - Re: [PATCH v2] KVM: arm64: Fix confusion in documentation for pKVM SME assert

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Wed, 5 Jun 2024 19:22:04 +0100
From: Fuad Tabba <tabba@...gle.com>
To: Mark Brown <broonie@...nel.org>
Cc: Marc Zyngier <maz@...nel.org>, Oliver Upton <oliver.upton@...ux.dev>, 
	James Morse <james.morse@....com>, Suzuki K Poulose <suzuki.poulose@....com>, 
	Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>, 
	linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.linux.dev, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] KVM: arm64: Fix confusion in documentation for pKVM
 SME assert

Hi Mark,

On Wed, Jun 5, 2024 at 7:11 PM Mark Brown <broonie@...nel.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@...gle.com

Cheers,
/fuad
>
> Fixes: afb91f5f8ad7 ("KVM: arm64: Ensure that SME controls are disabled in protected mode")
> Signed-off-by: Mark Brown <broonie@...nel.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@...nel.org>
>