* [PATCH] KVM: arm64: Only default to enabling SVE when present
@ 2023-09-13 18:34 Mark Brown
2023-09-14 0:04 ` Oliver Upton
0 siblings, 1 reply; 3+ messages in thread
From: Mark Brown @ 2023-09-13 18:34 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, James Morse, Suzuki K Poulose,
Catalin Marinas, Will Deacon
Cc: linux-arm-kernel, kvmarm, linux-kernel, Mark Brown
For unclear reasons our handling of SVE and SME when setting the default
value of CPTR_EL2 for VHE mode is inconsistent. For normal VHE we
unconditionally set CPTR_EL2.ZEN to 0b01 but only set the equivalent
field CPTR_EL2.SMEN to 0b01 if SME is present, for hVHE we will always
set the field 0b11 if SVE is not supported. Given the similarities
between the two extensions it would generally be expected that the code
handling SVE and SME would be very similar.
Since CPTR_ELx.ZEN is RES0 when SVE is not implemented it is probably not
harmful to try to set the bits but it is better practice to not set
unimplemented bits so resolve the inconsistency in favour of checking if
SVE is present too.
FPSIMD is also in theory optional though there's probably much more work to
handle the case where it is not implemented properly and that is not
something we see in practical systems.
Signed-off-by: Mark Brown <broonie@kernel.org>
---
arch/arm64/include/asm/kvm_emulate.h | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index 3d6725ff0bf6..4cf53b4aa226 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -584,15 +584,17 @@ static __always_inline u64 kvm_get_reset_cptr_el2(struct kvm_vcpu *vcpu)
u64 val;
if (has_vhe()) {
- val = (CPACR_EL1_FPEN_EL0EN | CPACR_EL1_FPEN_EL1EN |
- CPACR_EL1_ZEN_EL1EN);
+ val = (CPACR_EL1_FPEN_EL0EN | CPACR_EL1_FPEN_EL1EN);
+ if (cpus_have_final_cap(ARM64_SVE))
+ val |= CPACR_EL1_ZEN_EL1EN;
if (cpus_have_final_cap(ARM64_SME))
val |= CPACR_EL1_SMEN_EL1EN;
} else if (has_hvhe()) {
val = (CPACR_EL1_FPEN_EL0EN | CPACR_EL1_FPEN_EL1EN);
- if (!vcpu_has_sve(vcpu) ||
- (vcpu->arch.fp_state != FP_STATE_GUEST_OWNED))
+ if (cpus_have_final_cap(ARM64_SVE) &&
+ (!vcpu_has_sve(vcpu) ||
+ (vcpu->arch.fp_state != FP_STATE_GUEST_OWNED)))
val |= CPACR_EL1_ZEN_EL1EN | CPACR_EL1_ZEN_EL0EN;
if (cpus_have_final_cap(ARM64_SME))
val |= CPACR_EL1_SMEN_EL1EN | CPACR_EL1_SMEN_EL0EN;
---
base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d
change-id: 20230908-kvm-arm64-fp-init-8948a8d55e44
Best regards,
--
Mark Brown <broonie@kernel.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] KVM: arm64: Only default to enabling SVE when present
2023-09-13 18:34 [PATCH] KVM: arm64: Only default to enabling SVE when present Mark Brown
@ 2023-09-14 0:04 ` Oliver Upton
2023-09-14 10:32 ` Mark Brown
0 siblings, 1 reply; 3+ messages in thread
From: Oliver Upton @ 2023-09-14 0:04 UTC (permalink / raw)
To: Mark Brown
Cc: Marc Zyngier, James Morse, Suzuki K Poulose, Catalin Marinas,
Will Deacon, linux-arm-kernel, kvmarm, linux-kernel
Hi Mark,
On Wed, Sep 13, 2023 at 07:34:16PM +0100, Mark Brown wrote:
> For unclear reasons our handling of SVE and SME when setting the default
> value of CPTR_EL2 for VHE mode is inconsistent. For normal VHE we
> unconditionally set CPTR_EL2.ZEN to 0b01 but only set the equivalent
> field CPTR_EL2.SMEN to 0b01 if SME is present, for hVHE we will always
> set the field 0b11 if SVE is not supported. Given the similarities
> between the two extensions it would generally be expected that the code
> handling SVE and SME would be very similar.
>
> Since CPTR_ELx.ZEN is RES0 when SVE is not implemented it is probably not
> harmful to try to set the bits but it is better practice to not set
> unimplemented bits so resolve the inconsistency in favour of checking if
> SVE is present too.
It is entirely possible that implementers 'disable' a feature by hiding
it from the ID register, leaving the control bits completely functional.
These systems are at odds with the architecture, though we probably
shouldn't engage in _deliberately_ hostile programming patterns in the
kernel :)
> FPSIMD is also in theory optional though there's probably much more work to
> handle the case where it is not implemented properly and that is not
> something we see in practical systems.
>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
> arch/arm64/include/asm/kvm_emulate.h | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index 3d6725ff0bf6..4cf53b4aa226 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -584,15 +584,17 @@ static __always_inline u64 kvm_get_reset_cptr_el2(struct kvm_vcpu *vcpu)
> u64 val;
>
> if (has_vhe()) {
> - val = (CPACR_EL1_FPEN_EL0EN | CPACR_EL1_FPEN_EL1EN |
> - CPACR_EL1_ZEN_EL1EN);
> + val = (CPACR_EL1_FPEN_EL0EN | CPACR_EL1_FPEN_EL1EN);
> + if (cpus_have_final_cap(ARM64_SVE))
> + val |= CPACR_EL1_ZEN_EL1EN;
> if (cpus_have_final_cap(ARM64_SME))
> val |= CPACR_EL1_SMEN_EL1EN;
> } else if (has_hvhe()) {
> val = (CPACR_EL1_FPEN_EL0EN | CPACR_EL1_FPEN_EL1EN);
>
> - if (!vcpu_has_sve(vcpu) ||
> - (vcpu->arch.fp_state != FP_STATE_GUEST_OWNED))
> + if (cpus_have_final_cap(ARM64_SVE) &&
> + (!vcpu_has_sve(vcpu) ||
> + (vcpu->arch.fp_state != FP_STATE_GUEST_OWNED)))
vcpu_has_sve() already tests system_supports_sve(), so I don't believe
this hunk is necessary.
--
Thanks,
Oliver
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] KVM: arm64: Only default to enabling SVE when present
2023-09-14 0:04 ` Oliver Upton
@ 2023-09-14 10:32 ` Mark Brown
0 siblings, 0 replies; 3+ messages in thread
From: Mark Brown @ 2023-09-14 10:32 UTC (permalink / raw)
To: Oliver Upton
Cc: Marc Zyngier, James Morse, Suzuki K Poulose, Catalin Marinas,
Will Deacon, linux-arm-kernel, kvmarm, linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 634 bytes --]
On Thu, Sep 14, 2023 at 12:04:29AM +0000, Oliver Upton wrote:
> On Wed, Sep 13, 2023 at 07:34:16PM +0100, Mark Brown wrote:
> > - if (!vcpu_has_sve(vcpu) ||
> > - (vcpu->arch.fp_state != FP_STATE_GUEST_OWNED))
> > + if (cpus_have_final_cap(ARM64_SVE) &&
> > + (!vcpu_has_sve(vcpu) ||
> > + (vcpu->arch.fp_state != FP_STATE_GUEST_OWNED)))
> vcpu_has_sve() already tests system_supports_sve(), so I don't believe
> this hunk is necessary.
That'll mean that the first branch of the || should always return true
on systems without SVE so we'll set CPTR_EL2_TZ, and there's also the
check for guest ownership as well.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-09-14 10:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-13 18:34 [PATCH] KVM: arm64: Only default to enabling SVE when present Mark Brown
2023-09-14 0:04 ` Oliver Upton
2023-09-14 10:32 ` Mark Brown
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).