* [PATCH v1 0/5] Fix setting SVE and SME traps in (h)VHE
@ 2023-07-19 15:06 Fuad Tabba
2023-07-19 15:06 ` [PATCH v1 1/5] KVM: arm64: Use the appropriate feature trap register for SVE at EL2 setup Fuad Tabba
` (5 more replies)
0 siblings, 6 replies; 13+ messages in thread
From: Fuad Tabba @ 2023-07-19 15:06 UTC (permalink / raw)
To: kvmarm
Cc: maz, oliver.upton, catalin.marinas, james.morse, suzuki.poulose,
yuzenghui, will, tabba
The (re)setting and disabling of SVE/SME trap handling (mostly)
done for the hVHE work [*] misses a couple of cases.
This patch series ensures that these traps are disabled on setup
and reset. Moreover, it makes the code consistent in using
CPACR_EL1 or CPTR_EL2, depending on the mode.
Based on Linux 6.5-rc2.
Cheers,
/fuad
[*] https://lore.kernel.org/all/20230609162200.2024064-1-maz@kernel.org/
Fuad Tabba (5):
KVM: arm64: Use the appropriate feature trap register for SVE at EL2
setup
KVM: arm64: Disable SME Traps for (h)VHE at setup
KVM: arm64: Use the appropriate feature trap register when activating
traps
KVM: arm64: Fix resetting SVE trap values on reset for hVHE
KVM: arm64: Fix resetting SME trap values on reset for (h)VHE
arch/arm64/include/asm/el2_setup.h | 27 +++++++++++++++++++++------
arch/arm64/include/asm/kvm_emulate.h | 8 ++++++++
arch/arm64/kvm/hyp/nvhe/switch.c | 6 +++++-
3 files changed, 34 insertions(+), 7 deletions(-)
base-commit: fdf0eaf11452d72945af31804e2a1048ee1b574c
--
2.41.0.255.g8b1d071c50-goog
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v1 1/5] KVM: arm64: Use the appropriate feature trap register for SVE at EL2 setup
2023-07-19 15:06 [PATCH v1 0/5] Fix setting SVE and SME traps in (h)VHE Fuad Tabba
@ 2023-07-19 15:06 ` Fuad Tabba
2023-07-19 15:06 ` [PATCH v1 2/5] KVM: arm64: Disable SME Traps for (h)VHE at setup Fuad Tabba
` (4 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Fuad Tabba @ 2023-07-19 15:06 UTC (permalink / raw)
To: kvmarm
Cc: maz, oliver.upton, catalin.marinas, james.morse, suzuki.poulose,
yuzenghui, will, tabba
Use the architectural feature trap/control register that
corresponds to the current KVM mode, i.e., CPTR_EL2 or CPACR_EL1,
when setting up SVE feature traps.
Signed-off-by: Fuad Tabba <tabba@google.com>
---
arch/arm64/include/asm/el2_setup.h | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h
index 8e5ffb58f83e..c106b31d776c 100644
--- a/arch/arm64/include/asm/el2_setup.h
+++ b/arch/arm64/include/asm/el2_setup.h
@@ -143,11 +143,12 @@
and x1, x1, #HCR_E2H
cbz x1, .LnVHE_\@
mov x0, #(CPACR_EL1_FPEN_EL1EN | CPACR_EL1_FPEN_EL0EN)
- b .Lset_cptr_\@
+ msr cpacr_el1, x0
+ b .Lskip_set_cptr_\@
.LnVHE_\@:
mov x0, #0x33ff
-.Lset_cptr_\@:
msr cptr_el2, x0 // Disable copro. traps to EL2
+.Lskip_set_cptr_\@:
.endm
/* Disable any fine grained traps */
@@ -268,19 +269,21 @@
check_override id_aa64pfr0, ID_AA64PFR0_EL1_SVE_SHIFT, .Linit_sve_\@, .Lskip_sve_\@, x1, x2
.Linit_sve_\@: /* SVE register access */
- mrs x0, cptr_el2 // Disable SVE traps
mrs x1, hcr_el2
and x1, x1, #HCR_E2H
cbz x1, .Lcptr_nvhe_\@
- // VHE case
+ // (h)VHE case
+ mrs x0, cpacr_el1 // Disable SVE traps
orr x0, x0, #(CPACR_EL1_ZEN_EL1EN | CPACR_EL1_ZEN_EL0EN)
- b .Lset_cptr_\@
+ msr cpacr_el1, x0
+ b .Lskip_set_cptr_\@
.Lcptr_nvhe_\@: // nVHE case
+ mrs x0, cptr_el2 // Disable SVE traps
bic x0, x0, #CPTR_EL2_TZ
-.Lset_cptr_\@:
msr cptr_el2, x0
+.Lskip_set_cptr_\@:
isb
mov x1, #ZCR_ELx_LEN_MASK // SVE: Enable full vector
msr_s SYS_ZCR_EL2, x1 // length for EL1.
--
2.41.0.255.g8b1d071c50-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v1 2/5] KVM: arm64: Disable SME Traps for (h)VHE at setup
2023-07-19 15:06 [PATCH v1 0/5] Fix setting SVE and SME traps in (h)VHE Fuad Tabba
2023-07-19 15:06 ` [PATCH v1 1/5] KVM: arm64: Use the appropriate feature trap register for SVE at EL2 setup Fuad Tabba
@ 2023-07-19 15:06 ` Fuad Tabba
2023-07-19 17:34 ` Oliver Upton
2023-07-19 15:06 ` [PATCH v1 3/5] KVM: arm64: Use the appropriate feature trap register when activating traps Fuad Tabba
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Fuad Tabba @ 2023-07-19 15:06 UTC (permalink / raw)
To: kvmarm
Cc: maz, oliver.upton, catalin.marinas, james.morse, suzuki.poulose,
yuzenghui, will, tabba
Ensure that SME traps are disabled for (h)VHE when setting up
EL2, as they are for nVHE.
Signed-off-by: Fuad Tabba <tabba@google.com>
---
arch/arm64/include/asm/el2_setup.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h
index c106b31d776c..7001b6db3ccc 100644
--- a/arch/arm64/include/asm/el2_setup.h
+++ b/arch/arm64/include/asm/el2_setup.h
@@ -292,9 +292,21 @@
check_override id_aa64pfr1, ID_AA64PFR1_EL1_SME_SHIFT, .Linit_sme_\@, .Lskip_sme_\@, x1, x2
.Linit_sme_\@: /* SME register access and priority mapping */
+ mrs x1, hcr_el2
+ and x1, x1, #HCR_E2H
+ cbz x1, .Lcptr_nvhe_sme_\@
+
+ // (h)VHE case
+ mrs x0, cpacr_el1 // Disable SME traps
+ orr x0, x0, #(CPACR_EL1_SMEN_EL0EN | CPACR_EL1_SMEN_EL1EN)
+ msr cpacr_el1, x0
+ b .Lskip_set_cptr_sme_\@
+
+.Lcptr_nvhe_sme_\@: // nVHE case
mrs x0, cptr_el2 // Disable SME traps
bic x0, x0, #CPTR_EL2_TSM
msr cptr_el2, x0
+.Lskip_set_cptr_sme_\@:
isb
mrs x1, sctlr_el2
--
2.41.0.255.g8b1d071c50-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v1 3/5] KVM: arm64: Use the appropriate feature trap register when activating traps
2023-07-19 15:06 [PATCH v1 0/5] Fix setting SVE and SME traps in (h)VHE Fuad Tabba
2023-07-19 15:06 ` [PATCH v1 1/5] KVM: arm64: Use the appropriate feature trap register for SVE at EL2 setup Fuad Tabba
2023-07-19 15:06 ` [PATCH v1 2/5] KVM: arm64: Disable SME Traps for (h)VHE at setup Fuad Tabba
@ 2023-07-19 15:06 ` Fuad Tabba
2023-07-19 17:48 ` Oliver Upton
2023-07-19 15:06 ` [PATCH v1 4/5] KVM: arm64: Fix resetting SVE trap values on reset for hVHE Fuad Tabba
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Fuad Tabba @ 2023-07-19 15:06 UTC (permalink / raw)
To: kvmarm
Cc: maz, oliver.upton, catalin.marinas, james.morse, suzuki.poulose,
yuzenghui, will, tabba
Use the architectural feature trap/control register that
corresponds to the current KVM mode, i.e., CPTR_EL2 or CPACR_EL1,
when activating traps.
Signed-off-by: Fuad Tabba <tabba@google.com>
---
arch/arm64/kvm/hyp/nvhe/switch.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
index 0a6271052def..e5ea517bab9c 100644
--- a/arch/arm64/kvm/hyp/nvhe/switch.c
+++ b/arch/arm64/kvm/hyp/nvhe/switch.c
@@ -63,7 +63,11 @@ static void __activate_traps(struct kvm_vcpu *vcpu)
__activate_traps_fpsimd32(vcpu);
}
- write_sysreg(val, cptr_el2);
+ if (has_hvhe())
+ write_sysreg(val, cpacr_el1);
+ else
+ write_sysreg(val, cptr_el2);
+
write_sysreg(__this_cpu_read(kvm_hyp_vector), vbar_el2);
if (cpus_have_final_cap(ARM64_WORKAROUND_SPECULATIVE_AT)) {
--
2.41.0.255.g8b1d071c50-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v1 4/5] KVM: arm64: Fix resetting SVE trap values on reset for hVHE
2023-07-19 15:06 [PATCH v1 0/5] Fix setting SVE and SME traps in (h)VHE Fuad Tabba
` (2 preceding siblings ...)
2023-07-19 15:06 ` [PATCH v1 3/5] KVM: arm64: Use the appropriate feature trap register when activating traps Fuad Tabba
@ 2023-07-19 15:06 ` Fuad Tabba
2023-07-19 15:06 ` [PATCH v1 5/5] KVM: arm64: Fix resetting SME trap values on reset for (h)VHE Fuad Tabba
2023-07-19 15:20 ` [PATCH v1 0/5] Fix setting SVE and SME traps in (h)VHE Marc Zyngier
5 siblings, 0 replies; 13+ messages in thread
From: Fuad Tabba @ 2023-07-19 15:06 UTC (permalink / raw)
To: kvmarm
Cc: maz, oliver.upton, catalin.marinas, james.morse, suzuki.poulose,
yuzenghui, will, tabba
Ensure that SVE traps are disabled for hVHE, if the FPSIMD state
isn't owned by the guest, when getting the reset value for the
architectural feature control register.
Fixes: 75c76ab5a641 ("KVM: arm64: Rework CPTR_EL2 programming for HVHE configuration")
Signed-off-by: Fuad Tabba <tabba@google.com>
---
arch/arm64/include/asm/kvm_emulate.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index efc0b45d79c3..997c0ae72204 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -580,6 +580,10 @@ static __always_inline u64 kvm_get_reset_cptr_el2(struct kvm_vcpu *vcpu)
CPACR_EL1_ZEN_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))
+ val |= CPACR_EL1_ZEN_EL1EN | CPACR_EL1_ZEN_EL0EN;
} else {
val = CPTR_NVHE_EL2_RES1;
--
2.41.0.255.g8b1d071c50-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v1 5/5] KVM: arm64: Fix resetting SME trap values on reset for (h)VHE
2023-07-19 15:06 [PATCH v1 0/5] Fix setting SVE and SME traps in (h)VHE Fuad Tabba
` (3 preceding siblings ...)
2023-07-19 15:06 ` [PATCH v1 4/5] KVM: arm64: Fix resetting SVE trap values on reset for hVHE Fuad Tabba
@ 2023-07-19 15:06 ` Fuad Tabba
2023-07-19 15:20 ` [PATCH v1 0/5] Fix setting SVE and SME traps in (h)VHE Marc Zyngier
5 siblings, 0 replies; 13+ messages in thread
From: Fuad Tabba @ 2023-07-19 15:06 UTC (permalink / raw)
To: kvmarm
Cc: maz, oliver.upton, catalin.marinas, james.morse, suzuki.poulose,
yuzenghui, will, tabba
Ensure that SME traps are disabled for (h)VHE when getting the
reset value for the architectural feature control register.
Fixes: 75c76ab5a641 ("KVM: arm64: Rework CPTR_EL2 programming for HVHE configuration")
Signed-off-by: Fuad Tabba <tabba@google.com>
---
arch/arm64/include/asm/kvm_emulate.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index 997c0ae72204..c37584f77f4f 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -578,12 +578,16 @@ static __always_inline u64 kvm_get_reset_cptr_el2(struct kvm_vcpu *vcpu)
if (has_vhe()) {
val = (CPACR_EL1_FPEN_EL0EN | CPACR_EL1_FPEN_EL1EN |
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))
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;
} else {
val = CPTR_NVHE_EL2_RES1;
--
2.41.0.255.g8b1d071c50-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v1 0/5] Fix setting SVE and SME traps in (h)VHE
2023-07-19 15:06 [PATCH v1 0/5] Fix setting SVE and SME traps in (h)VHE Fuad Tabba
` (4 preceding siblings ...)
2023-07-19 15:06 ` [PATCH v1 5/5] KVM: arm64: Fix resetting SME trap values on reset for (h)VHE Fuad Tabba
@ 2023-07-19 15:20 ` Marc Zyngier
2023-07-20 8:42 ` Fuad Tabba
5 siblings, 1 reply; 13+ messages in thread
From: Marc Zyngier @ 2023-07-19 15:20 UTC (permalink / raw)
To: Fuad Tabba
Cc: kvmarm, oliver.upton, catalin.marinas, james.morse,
suzuki.poulose, yuzenghui, will
Hey Fuad,
Many thanks for looking into this, much appreciated. A passing comment
below.
On Wed, 19 Jul 2023 16:06:34 +0100,
Fuad Tabba <tabba@google.com> wrote:
>
> The (re)setting and disabling of SVE/SME trap handling (mostly)
> done for the hVHE work [*] misses a couple of cases.
>
> This patch series ensures that these traps are disabled on setup
> and reset. Moreover, it makes the code consistent in using
> CPACR_EL1 or CPTR_EL2, depending on the mode.
To provide context, this last point isn't really a correctness issue.
It only ensures that:
- we don't need to issue a synchronisation when alternating one or the
other accessor,
- when running hVHE under NV, we don't trap unnecessarily on accessing
CPTR_EL2, while CPACR_EL1 can be used directly without any trap.
I'll have a look at the actual patches shortly.
Thanks again,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 2/5] KVM: arm64: Disable SME Traps for (h)VHE at setup
2023-07-19 15:06 ` [PATCH v1 2/5] KVM: arm64: Disable SME Traps for (h)VHE at setup Fuad Tabba
@ 2023-07-19 17:34 ` Oliver Upton
2023-07-20 8:03 ` Fuad Tabba
0 siblings, 1 reply; 13+ messages in thread
From: Oliver Upton @ 2023-07-19 17:34 UTC (permalink / raw)
To: Fuad Tabba
Cc: kvmarm, maz, catalin.marinas, james.morse, suzuki.poulose,
yuzenghui, will
On Wed, Jul 19, 2023 at 04:06:36PM +0100, Fuad Tabba wrote:
> Ensure that SME traps are disabled for (h)VHE when setting up
> EL2, as they are for nVHE.
>
> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
> arch/arm64/include/asm/el2_setup.h | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h
> index c106b31d776c..7001b6db3ccc 100644
> --- a/arch/arm64/include/asm/el2_setup.h
> +++ b/arch/arm64/include/asm/el2_setup.h
> @@ -292,9 +292,21 @@
> check_override id_aa64pfr1, ID_AA64PFR1_EL1_SME_SHIFT, .Linit_sme_\@, .Lskip_sme_\@, x1, x2
>
> .Linit_sme_\@: /* SME register access and priority mapping */
> + mrs x1, hcr_el2
> + and x1, x1, #HCR_E2H
> + cbz x1, .Lcptr_nvhe_sme_\@
Hmm.. Our test for hVHE is repeated enough times, it might make sense
for readability to wrap it up in a macro:
.macro check_hvhe pass, fail, tmp
mrs \tmp, hcr_el2
and \tmp, \tmp, #HCR_E2H
cbnz \tmp, \pass
b \fail
.endm
So long as we tolerate the additional word of bloat for the extra branch
:)
> + // (h)VHE case
> + mrs x0, cpacr_el1 // Disable SME traps
> + orr x0, x0, #(CPACR_EL1_SMEN_EL0EN | CPACR_EL1_SMEN_EL1EN)
> + msr cpacr_el1, x0
> + b .Lskip_set_cptr_sme_\@
> +
> +.Lcptr_nvhe_sme_\@: // nVHE case
> mrs x0, cptr_el2 // Disable SME traps
> bic x0, x0, #CPTR_EL2_TSM
> msr cptr_el2, x0
> +.Lskip_set_cptr_sme_\@:
> isb
>
> mrs x1, sctlr_el2
> --
> 2.41.0.255.g8b1d071c50-goog
>
--
Thanks,
Oliver
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 3/5] KVM: arm64: Use the appropriate feature trap register when activating traps
2023-07-19 15:06 ` [PATCH v1 3/5] KVM: arm64: Use the appropriate feature trap register when activating traps Fuad Tabba
@ 2023-07-19 17:48 ` Oliver Upton
2023-07-19 18:40 ` Fuad Tabba
0 siblings, 1 reply; 13+ messages in thread
From: Oliver Upton @ 2023-07-19 17:48 UTC (permalink / raw)
To: Fuad Tabba
Cc: kvmarm, maz, catalin.marinas, james.morse, suzuki.poulose,
yuzenghui, will
On Wed, Jul 19, 2023 at 04:06:37PM +0100, Fuad Tabba wrote:
> Use the architectural feature trap/control register that
> corresponds to the current KVM mode, i.e., CPTR_EL2 or CPACR_EL1,
> when activating traps.
>
> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
> arch/arm64/kvm/hyp/nvhe/switch.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
> index 0a6271052def..e5ea517bab9c 100644
> --- a/arch/arm64/kvm/hyp/nvhe/switch.c
> +++ b/arch/arm64/kvm/hyp/nvhe/switch.c
> @@ -63,7 +63,11 @@ static void __activate_traps(struct kvm_vcpu *vcpu)
> __activate_traps_fpsimd32(vcpu);
> }
>
> - write_sysreg(val, cptr_el2);
> + if (has_hvhe())
> + write_sysreg(val, cpacr_el1);
> + else
> + write_sysreg(val, cptr_el2);
> +
Does it make sense to have an accessor that's shared with
kvm_reset_cptr_el2()? Due to the lack of synchronization between the
aliases we really want to avoid inconsistently using one or the other.
--
Thanks,
Oliver
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 3/5] KVM: arm64: Use the appropriate feature trap register when activating traps
2023-07-19 17:48 ` Oliver Upton
@ 2023-07-19 18:40 ` Fuad Tabba
0 siblings, 0 replies; 13+ messages in thread
From: Fuad Tabba @ 2023-07-19 18:40 UTC (permalink / raw)
To: Oliver Upton
Cc: kvmarm, maz, catalin.marinas, james.morse, suzuki.poulose,
yuzenghui, will
Hi Oliver,
On Wed, Jul 19, 2023 at 6:48 PM Oliver Upton <oliver.upton@linux.dev> wrote:
>
> On Wed, Jul 19, 2023 at 04:06:37PM +0100, Fuad Tabba wrote:
> > Use the architectural feature trap/control register that
> > corresponds to the current KVM mode, i.e., CPTR_EL2 or CPACR_EL1,
> > when activating traps.
> >
> > Signed-off-by: Fuad Tabba <tabba@google.com>
> > ---
> > arch/arm64/kvm/hyp/nvhe/switch.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
> > index 0a6271052def..e5ea517bab9c 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/switch.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/switch.c
> > @@ -63,7 +63,11 @@ static void __activate_traps(struct kvm_vcpu *vcpu)
> > __activate_traps_fpsimd32(vcpu);
> > }
> >
> > - write_sysreg(val, cptr_el2);
> > + if (has_hvhe())
> > + write_sysreg(val, cpacr_el1);
> > + else
> > + write_sysreg(val, cptr_el2);
> > +
>
> Does it make sense to have an accessor that's shared with
> kvm_reset_cptr_el2()? Due to the lack of synchronization between the
> aliases we really want to avoid inconsistently using one or the other.
I think that's a good idea. I'll add one when I respin.
Cheers,
/fuad
> --
> Thanks,
> Oliver
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 2/5] KVM: arm64: Disable SME Traps for (h)VHE at setup
2023-07-19 17:34 ` Oliver Upton
@ 2023-07-20 8:03 ` Fuad Tabba
2023-07-20 8:36 ` Fuad Tabba
0 siblings, 1 reply; 13+ messages in thread
From: Fuad Tabba @ 2023-07-20 8:03 UTC (permalink / raw)
To: Oliver Upton
Cc: kvmarm, maz, catalin.marinas, james.morse, suzuki.poulose,
yuzenghui, will
Hi Oliver,
On Wed, Jul 19, 2023 at 6:34 PM Oliver Upton <oliver.upton@linux.dev> wrote:
>
> On Wed, Jul 19, 2023 at 04:06:36PM +0100, Fuad Tabba wrote:
> > Ensure that SME traps are disabled for (h)VHE when setting up
> > EL2, as they are for nVHE.
> >
> > Signed-off-by: Fuad Tabba <tabba@google.com>
> > ---
> > arch/arm64/include/asm/el2_setup.h | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h
> > index c106b31d776c..7001b6db3ccc 100644
> > --- a/arch/arm64/include/asm/el2_setup.h
> > +++ b/arch/arm64/include/asm/el2_setup.h
> > @@ -292,9 +292,21 @@
> > check_override id_aa64pfr1, ID_AA64PFR1_EL1_SME_SHIFT, .Linit_sme_\@, .Lskip_sme_\@, x1, x2
> >
> > .Linit_sme_\@: /* SME register access and priority mapping */
> > + mrs x1, hcr_el2
> > + and x1, x1, #HCR_E2H
> > + cbz x1, .Lcptr_nvhe_sme_\@
>
> Hmm.. Our test for hVHE is repeated enough times, it might make sense
> for readability to wrap it up in a macro:
>
> .macro check_hvhe pass, fail, tmp
> mrs \tmp, hcr_el2
> and \tmp, \tmp, #HCR_E2H
> cbnz \tmp, \pass
> b \fail
> .endm
>
> So long as we tolerate the additional word of bloat for the extra branch
> :)
I did consider that, but went for less bloat. I'll wrap it in a macro
for the respin though, and leave it up to the maintainers to decide
which one to pick :D
Cheers,
/fuad
>
> > + // (h)VHE case
> > + mrs x0, cpacr_el1 // Disable SME traps
> > + orr x0, x0, #(CPACR_EL1_SMEN_EL0EN | CPACR_EL1_SMEN_EL1EN)
> > + msr cpacr_el1, x0
> > + b .Lskip_set_cptr_sme_\@
> > +
> > +.Lcptr_nvhe_sme_\@: // nVHE case
> > mrs x0, cptr_el2 // Disable SME traps
> > bic x0, x0, #CPTR_EL2_TSM
> > msr cptr_el2, x0
> > +.Lskip_set_cptr_sme_\@:
> > isb
> >
> > mrs x1, sctlr_el2
> > --
> > 2.41.0.255.g8b1d071c50-goog
> >
>
> --
> Thanks,
> Oliver
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 2/5] KVM: arm64: Disable SME Traps for (h)VHE at setup
2023-07-20 8:03 ` Fuad Tabba
@ 2023-07-20 8:36 ` Fuad Tabba
0 siblings, 0 replies; 13+ messages in thread
From: Fuad Tabba @ 2023-07-20 8:36 UTC (permalink / raw)
To: Oliver Upton
Cc: kvmarm, maz, catalin.marinas, james.morse, suzuki.poulose,
yuzenghui, will
Hi Oliver,
On Thu, Jul 20, 2023 at 9:03 AM Fuad Tabba <tabba@google.com> wrote:
>
> Hi Oliver,
>
>
> On Wed, Jul 19, 2023 at 6:34 PM Oliver Upton <oliver.upton@linux.dev> wrote:
> >
> > On Wed, Jul 19, 2023 at 04:06:36PM +0100, Fuad Tabba wrote:
> > > Ensure that SME traps are disabled for (h)VHE when setting up
> > > EL2, as they are for nVHE.
> > >
> > > Signed-off-by: Fuad Tabba <tabba@google.com>
> > > ---
> > > arch/arm64/include/asm/el2_setup.h | 12 ++++++++++++
> > > 1 file changed, 12 insertions(+)
> > >
> > > diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h
> > > index c106b31d776c..7001b6db3ccc 100644
> > > --- a/arch/arm64/include/asm/el2_setup.h
> > > +++ b/arch/arm64/include/asm/el2_setup.h
> > > @@ -292,9 +292,21 @@
> > > check_override id_aa64pfr1, ID_AA64PFR1_EL1_SME_SHIFT, .Linit_sme_\@, .Lskip_sme_\@, x1, x2
> > >
> > > .Linit_sme_\@: /* SME register access and priority mapping */
> > > + mrs x1, hcr_el2
> > > + and x1, x1, #HCR_E2H
> > > + cbz x1, .Lcptr_nvhe_sme_\@
> >
> > Hmm.. Our test for hVHE is repeated enough times, it might make sense
> > for readability to wrap it up in a macro:
> >
> > .macro check_hvhe pass, fail, tmp
> > mrs \tmp, hcr_el2
> > and \tmp, \tmp, #HCR_E2H
> > cbnz \tmp, \pass
> > b \fail
> > .endm
> >
> > So long as we tolerate the additional word of bloat for the extra branch
> > :)
>
> I did consider that, but went for less bloat. I'll wrap it in a macro
> for the respin though, and leave it up to the maintainers to decide
> which one to pick :D
Actually, with this macro I get the same desired effect without
affecting the emitted code:
/* Check if running in host at EL2 mode, i.e., (h)VHE. Jumps to fail if not. */
.macro __check_e2h fail, tmp
mrs \tmp, hcr_el2
and \tmp, \tmp, #HCR_E2H
cbz \tmp, \fail
.endm
Unless someone disagrees, I will use this for the respin.
Cheers,
/fuad
>
> Cheers,
> /fuad
>
> >
> > > + // (h)VHE case
> > > + mrs x0, cpacr_el1 // Disable SME traps
> > > + orr x0, x0, #(CPACR_EL1_SMEN_EL0EN | CPACR_EL1_SMEN_EL1EN)
> > > + msr cpacr_el1, x0
> > > + b .Lskip_set_cptr_sme_\@
> > > +
> > > +.Lcptr_nvhe_sme_\@: // nVHE case
> > > mrs x0, cptr_el2 // Disable SME traps
> > > bic x0, x0, #CPTR_EL2_TSM
> > > msr cptr_el2, x0
> > > +.Lskip_set_cptr_sme_\@:
> > > isb
> > >
> > > mrs x1, sctlr_el2
> > > --
> > > 2.41.0.255.g8b1d071c50-goog
> > >
> >
> > --
> > Thanks,
> > Oliver
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 0/5] Fix setting SVE and SME traps in (h)VHE
2023-07-19 15:20 ` [PATCH v1 0/5] Fix setting SVE and SME traps in (h)VHE Marc Zyngier
@ 2023-07-20 8:42 ` Fuad Tabba
0 siblings, 0 replies; 13+ messages in thread
From: Fuad Tabba @ 2023-07-20 8:42 UTC (permalink / raw)
To: Marc Zyngier
Cc: kvmarm, oliver.upton, catalin.marinas, james.morse,
suzuki.poulose, yuzenghui, will
Hi Marc,
On Wed, Jul 19, 2023 at 4:20 PM Marc Zyngier <maz@kernel.org> wrote:
>
> Hey Fuad,
>
> Many thanks for looking into this, much appreciated. A passing comment
> below.
>
> On Wed, 19 Jul 2023 16:06:34 +0100,
> Fuad Tabba <tabba@google.com> wrote:
> >
> > The (re)setting and disabling of SVE/SME trap handling (mostly)
> > done for the hVHE work [*] misses a couple of cases.
> >
> > This patch series ensures that these traps are disabled on setup
> > and reset. Moreover, it makes the code consistent in using
> > CPACR_EL1 or CPTR_EL2, depending on the mode.
>
> To provide context, this last point isn't really a correctness issue.
> It only ensures that:
>
> - we don't need to issue a synchronisation when alternating one or the
> other accessor,
>
> - when running hVHE under NV, we don't trap unnecessarily on accessing
> CPTR_EL2, while CPACR_EL1 can be used directly without any trap.
Thanks for that. I'll include this in the cover letter when I respin
to give more context.
Cheers,
/fuad
>
> I'll have a look at the actual patches shortly.
>
> Thanks again,
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2023-07-20 8:42 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-19 15:06 [PATCH v1 0/5] Fix setting SVE and SME traps in (h)VHE Fuad Tabba
2023-07-19 15:06 ` [PATCH v1 1/5] KVM: arm64: Use the appropriate feature trap register for SVE at EL2 setup Fuad Tabba
2023-07-19 15:06 ` [PATCH v1 2/5] KVM: arm64: Disable SME Traps for (h)VHE at setup Fuad Tabba
2023-07-19 17:34 ` Oliver Upton
2023-07-20 8:03 ` Fuad Tabba
2023-07-20 8:36 ` Fuad Tabba
2023-07-19 15:06 ` [PATCH v1 3/5] KVM: arm64: Use the appropriate feature trap register when activating traps Fuad Tabba
2023-07-19 17:48 ` Oliver Upton
2023-07-19 18:40 ` Fuad Tabba
2023-07-19 15:06 ` [PATCH v1 4/5] KVM: arm64: Fix resetting SVE trap values on reset for hVHE Fuad Tabba
2023-07-19 15:06 ` [PATCH v1 5/5] KVM: arm64: Fix resetting SME trap values on reset for (h)VHE Fuad Tabba
2023-07-19 15:20 ` [PATCH v1 0/5] Fix setting SVE and SME traps in (h)VHE Marc Zyngier
2023-07-20 8:42 ` Fuad Tabba
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.