All of lore.kernel.org
 help / color / mirror / Atom feed
* [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.