Received-SPF: pass (google.com: domain of linux-kernel+bounces-203052-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240605-kvm-arm64-sme-assert-v2-1-54391b0032f4@kernel.org>
In-Reply-To: <20240605-kvm-arm64-sme-assert-v2-1-54391b0032f4@kernel.org>
From: Fuad Tabba <tabba@google.com>
Date: Wed, 5 Jun 2024 19:22:04 +0100
Message-ID: <CA+EHjTywH4BQNMpJokWgEiaD2zyc4rNisxSRDszogaxRBDnCdg@mail.gmail.com>
Subject: Re: [PATCH v2] KVM: arm64: Fix confusion in documentation for pKVM
 SME assert
To: Mark Brown <broonie@kernel.org>
Cc: Marc Zyngier <maz@kernel.org>, Oliver Upton <oliver.upton@linux.dev>, 
	James Morse <james.morse@arm.com>, Suzuki K Poulose <suzuki.poulose@arm.com>, 
	Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will@kernel.org>, 
	linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Mark,

On Wed, Jun 5, 2024 at 7:11=E2=80=AFPM Mark Brown <broonie@kernel.org> wrot=
e:
>
> 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 i=
n 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>
>