[PATCH v5 0/2] KVM: arm64: Support for per-guest fine grained traps configuration

[PATCH v5 0/2] KVM: arm64: Support for per-guest fine grained traps configuration

[Mark Brown]

A number of upcoming features will require per guest configuration of
the fine grained traps enabled in the guest.  At present the only
management we do is for SME where we don't yet have guest support so we
unconditionally enable the traps while guests are running but this will
change going forward and at present the code isn't particularly
scalable.  This series aims to make it easier to configure which traps
are enabled for the guest by making the value set for the guest into
configuration in the vCPU data which we can set up based on the chosen
features rather than requiring conditional code in the trap enable and
disable paths.

This will have no benefit until one of the features that requires
configuration of these traps is merged but since there's a number of
such features it seems useful to decide on an approach to handling traps
for them which can be shared.

Changes in v5:
- Rebase onto v6.5-rc1, this required a bit of rework.
- Link to v4: https://lore.kernel.org/r/20230301-kvm-arm64-fgt-v4-0-1bf8d235ac1f@kernel.org
Changes in v4:
- Rebase onto v6.4-rc3.
- Link to v3: https://lore.kernel.org/r/20230301-kvm-arm64-fgt-v3-0-8f59543bb683@kernel.org
Changes in v3:
- Move the HFGxTR_EL2 storage from the sys_regs array to a member
  variable directly in struct kvm_vcpu_context.
- Link to v2: https://lore.kernel.org/r/20230301-kvm-arm64-fgt-v2-0-c11c0dcf810a@kernel.org
Changes in v2:
- List all the traps enabled by setting the registers to 0 by default.
- Save the HWFGxTR_EL2 valus in the vCPU/host sysreg structures.
- Link to v1: https://lore.kernel.org/r/20230301-kvm-arm64-fgt-v1-0-cfdf71ac67dc@kernel.org

---
Mark Brown (2):
      arm64: Add feature detection for fine grained traps
      KVM: arm64: Move FGT value configuration to vCPU state

 arch/arm64/include/asm/kvm_emulate.h    | 21 +++++++++++
 arch/arm64/include/asm/kvm_host.h       |  6 ++++
 arch/arm64/kernel/cpufeature.c          | 11 ++++++
 arch/arm64/kvm/arm.c                    |  1 +
 arch/arm64/kvm/hyp/include/hyp/switch.h | 62 +++++++++------------------------
 arch/arm64/tools/cpucaps                |  1 +
 6 files changed, 56 insertions(+), 46 deletions(-)
---
base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
change-id: 20230301-kvm-arm64-fgt-e5dd12746f67

Best regards,
-- 
Mark Brown <broonie@kernel.org>

[PATCH v5 0/2] KVM: arm64: Support for per-guest fine grained traps configuration

[Mark Brown]

A number of upcoming features will require per guest configuration of
the fine grained traps enabled in the guest.  At present the only
management we do is for SME where we don't yet have guest support so we
unconditionally enable the traps while guests are running but this will
change going forward and at present the code isn't particularly
scalable.  This series aims to make it easier to configure which traps
are enabled for the guest by making the value set for the guest into
configuration in the vCPU data which we can set up based on the chosen
features rather than requiring conditional code in the trap enable and
disable paths.

This will have no benefit until one of the features that requires
configuration of these traps is merged but since there's a number of
such features it seems useful to decide on an approach to handling traps
for them which can be shared.

Changes in v5:
- Rebase onto v6.5-rc1, this required a bit of rework.
- Link to v4: https://lore.kernel.org/r/20230301-kvm-arm64-fgt-v4-0-1bf8d235ac1f@kernel.org
Changes in v4:
- Rebase onto v6.4-rc3.
- Link to v3: https://lore.kernel.org/r/20230301-kvm-arm64-fgt-v3-0-8f59543bb683@kernel.org
Changes in v3:
- Move the HFGxTR_EL2 storage from the sys_regs array to a member
  variable directly in struct kvm_vcpu_context.
- Link to v2: https://lore.kernel.org/r/20230301-kvm-arm64-fgt-v2-0-c11c0dcf810a@kernel.org
Changes in v2:
- List all the traps enabled by setting the registers to 0 by default.
- Save the HWFGxTR_EL2 valus in the vCPU/host sysreg structures.
- Link to v1: https://lore.kernel.org/r/20230301-kvm-arm64-fgt-v1-0-cfdf71ac67dc@kernel.org

---
Mark Brown (2):
      arm64: Add feature detection for fine grained traps
      KVM: arm64: Move FGT value configuration to vCPU state

 arch/arm64/include/asm/kvm_emulate.h    | 21 +++++++++++
 arch/arm64/include/asm/kvm_host.h       |  6 ++++
 arch/arm64/kernel/cpufeature.c          | 11 ++++++
 arch/arm64/kvm/arm.c                    |  1 +
 arch/arm64/kvm/hyp/include/hyp/switch.h | 62 +++++++++------------------------
 arch/arm64/tools/cpucaps                |  1 +
 6 files changed, 56 insertions(+), 46 deletions(-)
---
base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
change-id: 20230301-kvm-arm64-fgt-e5dd12746f67

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

[PATCH v5 1/2] arm64: Add feature detection for fine grained traps

[Mark Brown]

In order to allow us to have shared code for managing fine grained traps
for KVM guests add it as a detected feature rather than relying on it
being a dependency of other features.

Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/arm64/kernel/cpufeature.c | 11 +++++++++++
 arch/arm64/tools/cpucaps       |  1 +
 2 files changed, 12 insertions(+)

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index f9d456fe132d..0768f98c49cc 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -2627,6 +2627,17 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 		.matches = has_cpuid_feature,
 		ARM64_CPUID_FIELDS(ID_AA64ISAR1_EL1, LRCPC, IMP)
 	},
+	{
+		.desc = "Fine Grained Traps",
+		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
+		.capability = ARM64_HAS_FGT,
+		.sys_reg = SYS_ID_AA64MMFR0_EL1,
+		.sign = FTR_UNSIGNED,
+		.field_pos = ID_AA64MMFR0_EL1_FGT_SHIFT,
+		.field_width = 4,
+		.min_field_value = 1,
+		.matches = has_cpuid_feature,
+	},
 #ifdef CONFIG_ARM64_SME
 	{
 		.desc = "Scalable Matrix Extension",
diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
index c80ed4f3cbce..c3f06fdef609 100644
--- a/arch/arm64/tools/cpucaps
+++ b/arch/arm64/tools/cpucaps
@@ -26,6 +26,7 @@ HAS_ECV
 HAS_ECV_CNTPOFF
 HAS_EPAN
 HAS_EVT
+HAS_FGT
 HAS_GENERIC_AUTH
 HAS_GENERIC_AUTH_ARCH_QARMA3
 HAS_GENERIC_AUTH_ARCH_QARMA5

-- 
2.30.2

[PATCH v5 1/2] arm64: Add feature detection for fine grained traps

[Mark Brown]

In order to allow us to have shared code for managing fine grained traps
for KVM guests add it as a detected feature rather than relying on it
being a dependency of other features.

Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/arm64/kernel/cpufeature.c | 11 +++++++++++
 arch/arm64/tools/cpucaps       |  1 +
 2 files changed, 12 insertions(+)

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index f9d456fe132d..0768f98c49cc 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -2627,6 +2627,17 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 		.matches = has_cpuid_feature,
 		ARM64_CPUID_FIELDS(ID_AA64ISAR1_EL1, LRCPC, IMP)
 	},
+	{
+		.desc = "Fine Grained Traps",
+		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
+		.capability = ARM64_HAS_FGT,
+		.sys_reg = SYS_ID_AA64MMFR0_EL1,
+		.sign = FTR_UNSIGNED,
+		.field_pos = ID_AA64MMFR0_EL1_FGT_SHIFT,
+		.field_width = 4,
+		.min_field_value = 1,
+		.matches = has_cpuid_feature,
+	},
 #ifdef CONFIG_ARM64_SME
 	{
 		.desc = "Scalable Matrix Extension",
diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
index c80ed4f3cbce..c3f06fdef609 100644
--- a/arch/arm64/tools/cpucaps
+++ b/arch/arm64/tools/cpucaps
@@ -26,6 +26,7 @@ HAS_ECV
 HAS_ECV_CNTPOFF
 HAS_EPAN
 HAS_EVT
+HAS_FGT
 HAS_GENERIC_AUTH
 HAS_GENERIC_AUTH_ARCH_QARMA3
 HAS_GENERIC_AUTH_ARCH_QARMA5

-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 1/2] arm64: Add feature detection for fine grained traps

[Zenghui Yu]

On 2023/7/12 20:50, Mark Brown wrote:
> In order to allow us to have shared code for managing fine grained traps
> for KVM guests add it as a detected feature rather than relying on it
> being a dependency of other features.
> 
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/kernel/cpufeature.c | 11 +++++++++++
>  arch/arm64/tools/cpucaps       |  1 +
>  2 files changed, 12 insertions(+)
> 
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index f9d456fe132d..0768f98c49cc 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -2627,6 +2627,17 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
>  		.matches = has_cpuid_feature,
>  		ARM64_CPUID_FIELDS(ID_AA64ISAR1_EL1, LRCPC, IMP)
>  	},
> +	{
> +		.desc = "Fine Grained Traps",
> +		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
> +		.capability = ARM64_HAS_FGT,
> +		.sys_reg = SYS_ID_AA64MMFR0_EL1,
> +		.sign = FTR_UNSIGNED,
> +		.field_pos = ID_AA64MMFR0_EL1_FGT_SHIFT,
> +		.field_width = 4,
> +		.min_field_value = 1,
> +		.matches = has_cpuid_feature,
> +	},

Any reason not to use the ARM64_CPUID_FIELDS() helper?

Zenghui

Re: [PATCH v5 1/2] arm64: Add feature detection for fine grained traps

[Zenghui Yu]

On 2023/7/12 20:50, Mark Brown wrote:
> In order to allow us to have shared code for managing fine grained traps
> for KVM guests add it as a detected feature rather than relying on it
> being a dependency of other features.
> 
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/kernel/cpufeature.c | 11 +++++++++++
>  arch/arm64/tools/cpucaps       |  1 +
>  2 files changed, 12 insertions(+)
> 
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index f9d456fe132d..0768f98c49cc 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -2627,6 +2627,17 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
>  		.matches = has_cpuid_feature,
>  		ARM64_CPUID_FIELDS(ID_AA64ISAR1_EL1, LRCPC, IMP)
>  	},
> +	{
> +		.desc = "Fine Grained Traps",
> +		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
> +		.capability = ARM64_HAS_FGT,
> +		.sys_reg = SYS_ID_AA64MMFR0_EL1,
> +		.sign = FTR_UNSIGNED,
> +		.field_pos = ID_AA64MMFR0_EL1_FGT_SHIFT,
> +		.field_width = 4,
> +		.min_field_value = 1,
> +		.matches = has_cpuid_feature,
> +	},

Any reason not to use the ARM64_CPUID_FIELDS() helper?

Zenghui

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 1/2] arm64: Add feature detection for fine grained traps

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 564 bytes --]

On Sun, Jul 16, 2023 at 10:44:01PM +0800, Zenghui Yu wrote:
> On 2023/7/12 20:50, Mark Brown wrote:

> > +	{
> > +		.desc = "Fine Grained Traps",
> > +		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
> > +		.capability = ARM64_HAS_FGT,
> > +		.sys_reg = SYS_ID_AA64MMFR0_EL1,
> > +		.sign = FTR_UNSIGNED,
> > +		.field_pos = ID_AA64MMFR0_EL1_FGT_SHIFT,
> > +		.field_width = 4,
> > +		.min_field_value = 1,
> > +		.matches = has_cpuid_feature,
> > +	},

> Any reason not to use the ARM64_CPUID_FIELDS() helper?

It should - the original was sent out before that was merged.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v5 1/2] arm64: Add feature detection for fine grained traps

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 564 bytes --]

On Sun, Jul 16, 2023 at 10:44:01PM +0800, Zenghui Yu wrote:
> On 2023/7/12 20:50, Mark Brown wrote:

> > +	{
> > +		.desc = "Fine Grained Traps",
> > +		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
> > +		.capability = ARM64_HAS_FGT,
> > +		.sys_reg = SYS_ID_AA64MMFR0_EL1,
> > +		.sign = FTR_UNSIGNED,
> > +		.field_pos = ID_AA64MMFR0_EL1_FGT_SHIFT,
> > +		.field_width = 4,
> > +		.min_field_value = 1,
> > +		.matches = has_cpuid_feature,
> > +	},

> Any reason not to use the ARM64_CPUID_FIELDS() helper?

It should - the original was sent out before that was merged.

[-- 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

[PATCH v5 2/2] KVM: arm64: Move FGT value configuration to vCPU state

[Mark Brown]

Currently the only fine grained traps we use are the SME ones and we decide
which to enable based on the presence of that feature. In order to support
SME, GCS and other features where we need fine grained traps we will need to
select per guest which traps are enabled. Move to storing the traps to
enable in the vCPU data, updating the registers if fine grained traps are
supported and any are enabled.

The code assumes that we never change the set of fine grained traps for the
host after boot.

No functional change, though there will be a small overhead on systems with
fine grained traps supported.  We could optimise slightly by assuming that
host and guest always set the same pattern for read and write (they do
currently) but this seemed more likely to cause surprises in future than
it was worth.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/arm64/include/asm/kvm_emulate.h    | 21 +++++++++++
 arch/arm64/include/asm/kvm_host.h       |  6 ++++
 arch/arm64/kvm/arm.c                    |  1 +
 arch/arm64/kvm/hyp/include/hyp/switch.h | 62 +++++++++------------------------
 4 files changed, 44 insertions(+), 46 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index efc0b45d79c3..0d6c8d7e2aaa 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -108,6 +108,27 @@ static inline unsigned long *vcpu_hcr(struct kvm_vcpu *vcpu)
 	return (unsigned long *)&vcpu->arch.hcr_el2;
 }
 
+static inline void vcpu_reset_fgt(struct kvm_vcpu *vcpu)
+{
+	if (!cpus_have_const_cap(ARM64_HAS_FGT))
+		return;
+
+	vcpu->arch.hfgrtr_el2 = 0;
+	vcpu->arch.hfgwtr_el2 = 0;
+
+	/*
+	 * Trap guest writes to TCR_EL1 to prevent it from enabling HA or HD.
+	 */
+	if (cpus_have_final_cap(ARM64_WORKAROUND_AMPERE_AC03_CPU_38)) {
+		vcpu->arch.hfgrtr_el2 |= HFGxTR_EL2_TCR_EL1_MASK;
+		vcpu->arch.hfgwtr_el2 |= HFGxTR_EL2_TCR_EL1_MASK;
+	}
+
+	/* We currently assume the host configuration never changes */
+	vcpu->arch.hfgrtr_el2_host = read_sysreg_s(SYS_HFGRTR_EL2);
+	vcpu->arch.hfgwtr_el2_host = read_sysreg_s(SYS_HFGWTR_EL2);
+}
+
 static inline void vcpu_clear_wfx_traps(struct kvm_vcpu *vcpu)
 {
 	vcpu->arch.hcr_el2 &= ~HCR_TWE;
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 8b6096753740..a7f558ba0406 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -574,6 +574,12 @@ struct kvm_vcpu_arch {
 	/* Virtual SError ESR to restore when HCR_EL2.VSE is set */
 	u64 vsesr_el2;
 
+	/* Fine grained traps values for the guest and host */
+	u64 hfgrtr_el2;
+	u64 hfgwtr_el2;
+	u64 hfgrtr_el2_host;
+	u64 hfgwtr_el2_host;
+
 	/* Additional reset state */
 	struct vcpu_reset_state	reset_state;
 
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index c2c14059f6a8..86866d05c6d5 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1306,6 +1306,7 @@ static int kvm_arch_vcpu_ioctl_vcpu_init(struct kvm_vcpu *vcpu,
 	}
 
 	vcpu_reset_hcr(vcpu);
+	vcpu_reset_fgt(vcpu);
 	vcpu->arch.cptr_el2 = kvm_get_reset_cptr_el2(vcpu);
 
 	/*
diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index 4bddb8541bec..f35f8fb6a489 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -70,54 +70,26 @@ static inline void __activate_traps_fpsimd32(struct kvm_vcpu *vcpu)
 	}
 }
 
-static inline bool __hfgxtr_traps_required(void)
+static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu)
 {
-	if (cpus_have_final_cap(ARM64_SME))
-		return true;
-
-	if (cpus_have_final_cap(ARM64_WORKAROUND_AMPERE_AC03_CPU_38))
-		return true;
-
-	return false;
-}
-
-static inline void __activate_traps_hfgxtr(void)
-{
-	u64 r_clr = 0, w_clr = 0, r_set = 0, w_set = 0, tmp;
-
-	if (cpus_have_final_cap(ARM64_SME)) {
-		tmp = HFGxTR_EL2_nSMPRI_EL1_MASK | HFGxTR_EL2_nTPIDR2_EL0_MASK;
-
-		r_clr |= tmp;
-		w_clr |= tmp;
-	}
-
-	/*
-	 * Trap guest writes to TCR_EL1 to prevent it from enabling HA or HD.
-	 */
-	if (cpus_have_final_cap(ARM64_WORKAROUND_AMPERE_AC03_CPU_38))
-		w_set |= HFGxTR_EL2_TCR_EL1_MASK;
+	if (!cpus_have_final_cap(ARM64_HAS_FGT))
+		return;
 
-	sysreg_clear_set_s(SYS_HFGRTR_EL2, r_clr, r_set);
-	sysreg_clear_set_s(SYS_HFGWTR_EL2, w_clr, w_set);
+	if (vcpu->arch.hfgrtr_el2_host != vcpu->arch.hfgrtr_el2)
+		write_sysreg_s(vcpu->arch.hfgrtr_el2, SYS_HFGRTR_EL2);
+	if (vcpu->arch.hfgwtr_el2_host != vcpu->arch.hfgwtr_el2)
+		write_sysreg_s(vcpu->arch.hfgwtr_el2,  SYS_HFGWTR_EL2);
 }
 
-static inline void __deactivate_traps_hfgxtr(void)
+static inline void __deactivate_traps_hfgxtr(struct kvm_vcpu *vcpu)
 {
-	u64 r_clr = 0, w_clr = 0, r_set = 0, w_set = 0, tmp;
-
-	if (cpus_have_final_cap(ARM64_SME)) {
-		tmp = HFGxTR_EL2_nSMPRI_EL1_MASK | HFGxTR_EL2_nTPIDR2_EL0_MASK;
-
-		r_set |= tmp;
-		w_set |= tmp;
-	}
-
-	if (cpus_have_final_cap(ARM64_WORKAROUND_AMPERE_AC03_CPU_38))
-		w_clr |= HFGxTR_EL2_TCR_EL1_MASK;
+	if (!cpus_have_final_cap(ARM64_HAS_FGT))
+		return;
 
-	sysreg_clear_set_s(SYS_HFGRTR_EL2, r_clr, r_set);
-	sysreg_clear_set_s(SYS_HFGWTR_EL2, w_clr, w_set);
+	if (vcpu->arch.hfgrtr_el2_host != vcpu->arch.hfgrtr_el2)
+		write_sysreg_s(vcpu->arch.hfgrtr_el2, SYS_HFGRTR_EL2);
+	if (vcpu->arch.hfgwtr_el2_host != vcpu->arch.hfgwtr_el2)
+		write_sysreg_s(vcpu->arch.hfgwtr_el2,  SYS_HFGWTR_EL2);
 }
 
 static inline void __activate_traps_common(struct kvm_vcpu *vcpu)
@@ -145,8 +117,7 @@ static inline void __activate_traps_common(struct kvm_vcpu *vcpu)
 	vcpu->arch.mdcr_el2_host = read_sysreg(mdcr_el2);
 	write_sysreg(vcpu->arch.mdcr_el2, mdcr_el2);
 
-	if (__hfgxtr_traps_required())
-		__activate_traps_hfgxtr();
+	__activate_traps_hfgxtr(vcpu);
 }
 
 static inline void __deactivate_traps_common(struct kvm_vcpu *vcpu)
@@ -162,8 +133,7 @@ static inline void __deactivate_traps_common(struct kvm_vcpu *vcpu)
 		vcpu_clear_flag(vcpu, PMUSERENR_ON_CPU);
 	}
 
-	if (__hfgxtr_traps_required())
-		__deactivate_traps_hfgxtr();
+	__deactivate_traps_hfgxtr(vcpu);
 }
 
 static inline void ___activate_traps(struct kvm_vcpu *vcpu)

-- 
2.30.2

[PATCH v5 2/2] KVM: arm64: Move FGT value configuration to vCPU state

[Mark Brown]

Currently the only fine grained traps we use are the SME ones and we decide
which to enable based on the presence of that feature. In order to support
SME, GCS and other features where we need fine grained traps we will need to
select per guest which traps are enabled. Move to storing the traps to
enable in the vCPU data, updating the registers if fine grained traps are
supported and any are enabled.

The code assumes that we never change the set of fine grained traps for the
host after boot.

No functional change, though there will be a small overhead on systems with
fine grained traps supported.  We could optimise slightly by assuming that
host and guest always set the same pattern for read and write (they do
currently) but this seemed more likely to cause surprises in future than
it was worth.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/arm64/include/asm/kvm_emulate.h    | 21 +++++++++++
 arch/arm64/include/asm/kvm_host.h       |  6 ++++
 arch/arm64/kvm/arm.c                    |  1 +
 arch/arm64/kvm/hyp/include/hyp/switch.h | 62 +++++++++------------------------
 4 files changed, 44 insertions(+), 46 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index efc0b45d79c3..0d6c8d7e2aaa 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -108,6 +108,27 @@ static inline unsigned long *vcpu_hcr(struct kvm_vcpu *vcpu)
 	return (unsigned long *)&vcpu->arch.hcr_el2;
 }
 
+static inline void vcpu_reset_fgt(struct kvm_vcpu *vcpu)
+{
+	if (!cpus_have_const_cap(ARM64_HAS_FGT))
+		return;
+
+	vcpu->arch.hfgrtr_el2 = 0;
+	vcpu->arch.hfgwtr_el2 = 0;
+
+	/*
+	 * Trap guest writes to TCR_EL1 to prevent it from enabling HA or HD.
+	 */
+	if (cpus_have_final_cap(ARM64_WORKAROUND_AMPERE_AC03_CPU_38)) {
+		vcpu->arch.hfgrtr_el2 |= HFGxTR_EL2_TCR_EL1_MASK;
+		vcpu->arch.hfgwtr_el2 |= HFGxTR_EL2_TCR_EL1_MASK;
+	}
+
+	/* We currently assume the host configuration never changes */
+	vcpu->arch.hfgrtr_el2_host = read_sysreg_s(SYS_HFGRTR_EL2);
+	vcpu->arch.hfgwtr_el2_host = read_sysreg_s(SYS_HFGWTR_EL2);
+}
+
 static inline void vcpu_clear_wfx_traps(struct kvm_vcpu *vcpu)
 {
 	vcpu->arch.hcr_el2 &= ~HCR_TWE;
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 8b6096753740..a7f558ba0406 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -574,6 +574,12 @@ struct kvm_vcpu_arch {
 	/* Virtual SError ESR to restore when HCR_EL2.VSE is set */
 	u64 vsesr_el2;
 
+	/* Fine grained traps values for the guest and host */
+	u64 hfgrtr_el2;
+	u64 hfgwtr_el2;
+	u64 hfgrtr_el2_host;
+	u64 hfgwtr_el2_host;
+
 	/* Additional reset state */
 	struct vcpu_reset_state	reset_state;
 
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index c2c14059f6a8..86866d05c6d5 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1306,6 +1306,7 @@ static int kvm_arch_vcpu_ioctl_vcpu_init(struct kvm_vcpu *vcpu,
 	}
 
 	vcpu_reset_hcr(vcpu);
+	vcpu_reset_fgt(vcpu);
 	vcpu->arch.cptr_el2 = kvm_get_reset_cptr_el2(vcpu);
 
 	/*
diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index 4bddb8541bec..f35f8fb6a489 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -70,54 +70,26 @@ static inline void __activate_traps_fpsimd32(struct kvm_vcpu *vcpu)
 	}
 }
 
-static inline bool __hfgxtr_traps_required(void)
+static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu)
 {
-	if (cpus_have_final_cap(ARM64_SME))
-		return true;
-
-	if (cpus_have_final_cap(ARM64_WORKAROUND_AMPERE_AC03_CPU_38))
-		return true;
-
-	return false;
-}
-
-static inline void __activate_traps_hfgxtr(void)
-{
-	u64 r_clr = 0, w_clr = 0, r_set = 0, w_set = 0, tmp;
-
-	if (cpus_have_final_cap(ARM64_SME)) {
-		tmp = HFGxTR_EL2_nSMPRI_EL1_MASK | HFGxTR_EL2_nTPIDR2_EL0_MASK;
-
-		r_clr |= tmp;
-		w_clr |= tmp;
-	}
-
-	/*
-	 * Trap guest writes to TCR_EL1 to prevent it from enabling HA or HD.
-	 */
-	if (cpus_have_final_cap(ARM64_WORKAROUND_AMPERE_AC03_CPU_38))
-		w_set |= HFGxTR_EL2_TCR_EL1_MASK;
+	if (!cpus_have_final_cap(ARM64_HAS_FGT))
+		return;
 
-	sysreg_clear_set_s(SYS_HFGRTR_EL2, r_clr, r_set);
-	sysreg_clear_set_s(SYS_HFGWTR_EL2, w_clr, w_set);
+	if (vcpu->arch.hfgrtr_el2_host != vcpu->arch.hfgrtr_el2)
+		write_sysreg_s(vcpu->arch.hfgrtr_el2, SYS_HFGRTR_EL2);
+	if (vcpu->arch.hfgwtr_el2_host != vcpu->arch.hfgwtr_el2)
+		write_sysreg_s(vcpu->arch.hfgwtr_el2,  SYS_HFGWTR_EL2);
 }
 
-static inline void __deactivate_traps_hfgxtr(void)
+static inline void __deactivate_traps_hfgxtr(struct kvm_vcpu *vcpu)
 {
-	u64 r_clr = 0, w_clr = 0, r_set = 0, w_set = 0, tmp;
-
-	if (cpus_have_final_cap(ARM64_SME)) {
-		tmp = HFGxTR_EL2_nSMPRI_EL1_MASK | HFGxTR_EL2_nTPIDR2_EL0_MASK;
-
-		r_set |= tmp;
-		w_set |= tmp;
-	}
-
-	if (cpus_have_final_cap(ARM64_WORKAROUND_AMPERE_AC03_CPU_38))
-		w_clr |= HFGxTR_EL2_TCR_EL1_MASK;
+	if (!cpus_have_final_cap(ARM64_HAS_FGT))
+		return;
 
-	sysreg_clear_set_s(SYS_HFGRTR_EL2, r_clr, r_set);
-	sysreg_clear_set_s(SYS_HFGWTR_EL2, w_clr, w_set);
+	if (vcpu->arch.hfgrtr_el2_host != vcpu->arch.hfgrtr_el2)
+		write_sysreg_s(vcpu->arch.hfgrtr_el2, SYS_HFGRTR_EL2);
+	if (vcpu->arch.hfgwtr_el2_host != vcpu->arch.hfgwtr_el2)
+		write_sysreg_s(vcpu->arch.hfgwtr_el2,  SYS_HFGWTR_EL2);
 }
 
 static inline void __activate_traps_common(struct kvm_vcpu *vcpu)
@@ -145,8 +117,7 @@ static inline void __activate_traps_common(struct kvm_vcpu *vcpu)
 	vcpu->arch.mdcr_el2_host = read_sysreg(mdcr_el2);
 	write_sysreg(vcpu->arch.mdcr_el2, mdcr_el2);
 
-	if (__hfgxtr_traps_required())
-		__activate_traps_hfgxtr();
+	__activate_traps_hfgxtr(vcpu);
 }
 
 static inline void __deactivate_traps_common(struct kvm_vcpu *vcpu)
@@ -162,8 +133,7 @@ static inline void __deactivate_traps_common(struct kvm_vcpu *vcpu)
 		vcpu_clear_flag(vcpu, PMUSERENR_ON_CPU);
 	}
 
-	if (__hfgxtr_traps_required())
-		__deactivate_traps_hfgxtr();
+	__deactivate_traps_hfgxtr(vcpu);
 }
 
 static inline void ___activate_traps(struct kvm_vcpu *vcpu)

-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 2/2] KVM: arm64: Move FGT value configuration to vCPU state

[Suzuki K Poulose]

On 12/07/2023 13:50, Mark Brown wrote:
> Currently the only fine grained traps we use are the SME ones and we decide
> which to enable based on the presence of that feature. In order to support
> SME, GCS and other features where we need fine grained traps we will need to
> select per guest which traps are enabled. Move to storing the traps to
> enable in the vCPU data, updating the registers if fine grained traps are
> supported and any are enabled.
> 
> The code assumes that we never change the set of fine grained traps for the
> host after boot.
> 
> No functional change, though there will be a small overhead on systems with
> fine grained traps supported.  We could optimise slightly by assuming that
> host and guest always set the same pattern for read and write (they do
> currently) but this seemed more likely to cause surprises in future than
> it was worth.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>   arch/arm64/include/asm/kvm_emulate.h    | 21 +++++++++++
>   arch/arm64/include/asm/kvm_host.h       |  6 ++++
>   arch/arm64/kvm/arm.c                    |  1 +
>   arch/arm64/kvm/hyp/include/hyp/switch.h | 62 +++++++++------------------------
>   4 files changed, 44 insertions(+), 46 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index efc0b45d79c3..0d6c8d7e2aaa 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -108,6 +108,27 @@ static inline unsigned long *vcpu_hcr(struct kvm_vcpu *vcpu)
>   	return (unsigned long *)&vcpu->arch.hcr_el2;
>   }
>   
> +static inline void vcpu_reset_fgt(struct kvm_vcpu *vcpu)
> +{
> +	if (!cpus_have_const_cap(ARM64_HAS_FGT))
> +		return;
> +
> +	vcpu->arch.hfgrtr_el2 = 0;
> +	vcpu->arch.hfgwtr_el2 = 0;
> +
> +	/*
> +	 * Trap guest writes to TCR_EL1 to prevent it from enabling HA or HD.
> +	 */
> +	if (cpus_have_final_cap(ARM64_WORKAROUND_AMPERE_AC03_CPU_38)) {
> +		vcpu->arch.hfgrtr_el2 |= HFGxTR_EL2_TCR_EL1_MASK;
> +		vcpu->arch.hfgwtr_el2 |= HFGxTR_EL2_TCR_EL1_MASK;
> +	}
> +
> +	/* We currently assume the host configuration never changes */
> +	vcpu->arch.hfgrtr_el2_host = read_sysreg_s(SYS_HFGRTR_EL2);
> +	vcpu->arch.hfgwtr_el2_host = read_sysreg_s(SYS_HFGWTR_EL2);

Doesn't this crash an nVHE host ?

Suzuki

Re: [PATCH v5 2/2] KVM: arm64: Move FGT value configuration to vCPU state

[Suzuki K Poulose]

On 12/07/2023 13:50, Mark Brown wrote:
> Currently the only fine grained traps we use are the SME ones and we decide
> which to enable based on the presence of that feature. In order to support
> SME, GCS and other features where we need fine grained traps we will need to
> select per guest which traps are enabled. Move to storing the traps to
> enable in the vCPU data, updating the registers if fine grained traps are
> supported and any are enabled.
> 
> The code assumes that we never change the set of fine grained traps for the
> host after boot.
> 
> No functional change, though there will be a small overhead on systems with
> fine grained traps supported.  We could optimise slightly by assuming that
> host and guest always set the same pattern for read and write (they do
> currently) but this seemed more likely to cause surprises in future than
> it was worth.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>   arch/arm64/include/asm/kvm_emulate.h    | 21 +++++++++++
>   arch/arm64/include/asm/kvm_host.h       |  6 ++++
>   arch/arm64/kvm/arm.c                    |  1 +
>   arch/arm64/kvm/hyp/include/hyp/switch.h | 62 +++++++++------------------------
>   4 files changed, 44 insertions(+), 46 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index efc0b45d79c3..0d6c8d7e2aaa 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -108,6 +108,27 @@ static inline unsigned long *vcpu_hcr(struct kvm_vcpu *vcpu)
>   	return (unsigned long *)&vcpu->arch.hcr_el2;
>   }
>   
> +static inline void vcpu_reset_fgt(struct kvm_vcpu *vcpu)
> +{
> +	if (!cpus_have_const_cap(ARM64_HAS_FGT))
> +		return;
> +
> +	vcpu->arch.hfgrtr_el2 = 0;
> +	vcpu->arch.hfgwtr_el2 = 0;
> +
> +	/*
> +	 * Trap guest writes to TCR_EL1 to prevent it from enabling HA or HD.
> +	 */
> +	if (cpus_have_final_cap(ARM64_WORKAROUND_AMPERE_AC03_CPU_38)) {
> +		vcpu->arch.hfgrtr_el2 |= HFGxTR_EL2_TCR_EL1_MASK;
> +		vcpu->arch.hfgwtr_el2 |= HFGxTR_EL2_TCR_EL1_MASK;
> +	}
> +
> +	/* We currently assume the host configuration never changes */
> +	vcpu->arch.hfgrtr_el2_host = read_sysreg_s(SYS_HFGRTR_EL2);
> +	vcpu->arch.hfgwtr_el2_host = read_sysreg_s(SYS_HFGWTR_EL2);

Doesn't this crash an nVHE host ?

Suzuki


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 2/2] KVM: arm64: Move FGT value configuration to vCPU state

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 490 bytes --]

On Thu, Jul 13, 2023 at 10:48:54AM +0100, Suzuki K Poulose wrote:
> On 12/07/2023 13:50, Mark Brown wrote:

> > +	/* We currently assume the host configuration never changes */
> > +	vcpu->arch.hfgrtr_el2_host = read_sysreg_s(SYS_HFGRTR_EL2);
> > +	vcpu->arch.hfgwtr_el2_host = read_sysreg_s(SYS_HFGWTR_EL2);

> Doesn't this crash an nVHE host ?

This series is probably moot given Marc's nv series (or at least needs
another rework) but could you expand on the issue you're seeing please?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v5 2/2] KVM: arm64: Move FGT value configuration to vCPU state

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 490 bytes --]

On Thu, Jul 13, 2023 at 10:48:54AM +0100, Suzuki K Poulose wrote:
> On 12/07/2023 13:50, Mark Brown wrote:

> > +	/* We currently assume the host configuration never changes */
> > +	vcpu->arch.hfgrtr_el2_host = read_sysreg_s(SYS_HFGRTR_EL2);
> > +	vcpu->arch.hfgwtr_el2_host = read_sysreg_s(SYS_HFGWTR_EL2);

> Doesn't this crash an nVHE host ?

This series is probably moot given Marc's nv series (or at least needs
another rework) but could you expand on the issue you're seeing please?

[-- 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

Re: [PATCH v5 2/2] KVM: arm64: Move FGT value configuration to vCPU state

[Suzuki K Poulose]

On 13/07/2023 16:15, Mark Brown wrote:
> On Thu, Jul 13, 2023 at 10:48:54AM +0100, Suzuki K Poulose wrote:
>> On 12/07/2023 13:50, Mark Brown wrote:
> 
>>> +	/* We currently assume the host configuration never changes */
>>> +	vcpu->arch.hfgrtr_el2_host = read_sysreg_s(SYS_HFGRTR_EL2);
>>> +	vcpu->arch.hfgwtr_el2_host = read_sysreg_s(SYS_HFGWTR_EL2);
> 
>> Doesn't this crash an nVHE host ?
> 
> This series is probably moot given Marc's nv series (or at least needs
> another rework) but could you expand on the issue you're seeing please?

I haven't tried it. But we are reading a EL2 register and this could be
executed from EL1 in nVHE/protected mode and could crash the host ?

Suzuki

Re: [PATCH v5 2/2] KVM: arm64: Move FGT value configuration to vCPU state

[Suzuki K Poulose]

On 13/07/2023 16:15, Mark Brown wrote:
> On Thu, Jul 13, 2023 at 10:48:54AM +0100, Suzuki K Poulose wrote:
>> On 12/07/2023 13:50, Mark Brown wrote:
> 
>>> +	/* We currently assume the host configuration never changes */
>>> +	vcpu->arch.hfgrtr_el2_host = read_sysreg_s(SYS_HFGRTR_EL2);
>>> +	vcpu->arch.hfgwtr_el2_host = read_sysreg_s(SYS_HFGWTR_EL2);
> 
>> Doesn't this crash an nVHE host ?
> 
> This series is probably moot given Marc's nv series (or at least needs
> another rework) but could you expand on the issue you're seeing please?

I haven't tried it. But we are reading a EL2 register and this could be
executed from EL1 in nVHE/protected mode and could crash the host ?

Suzuki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 2/2] KVM: arm64: Move FGT value configuration to vCPU state

[Marc Zyngier]

On Thu, 13 Jul 2023 16:15:36 +0100,
Mark Brown <broonie@kernel.org> wrote:
> 
> [1  <text/plain; us-ascii (7bit)>]
> On Thu, Jul 13, 2023 at 10:48:54AM +0100, Suzuki K Poulose wrote:
> > On 12/07/2023 13:50, Mark Brown wrote:
> 
> > > +	/* We currently assume the host configuration never changes */
> > > +	vcpu->arch.hfgrtr_el2_host = read_sysreg_s(SYS_HFGRTR_EL2);
> > > +	vcpu->arch.hfgwtr_el2_host = read_sysreg_s(SYS_HFGWTR_EL2);
> 
> > Doesn't this crash an nVHE host ?
> 
> This series is probably moot given Marc's nv series (or at least needs
> another rework) but could you expand on the issue you're seeing please?

You're reading EL2 registers from EL1. What could possibly go wrong?

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v5 2/2] KVM: arm64: Move FGT value configuration to vCPU state

[Marc Zyngier]

On Thu, 13 Jul 2023 16:15:36 +0100,
Mark Brown <broonie@kernel.org> wrote:
> 
> [1  <text/plain; us-ascii (7bit)>]
> On Thu, Jul 13, 2023 at 10:48:54AM +0100, Suzuki K Poulose wrote:
> > On 12/07/2023 13:50, Mark Brown wrote:
> 
> > > +	/* We currently assume the host configuration never changes */
> > > +	vcpu->arch.hfgrtr_el2_host = read_sysreg_s(SYS_HFGRTR_EL2);
> > > +	vcpu->arch.hfgwtr_el2_host = read_sysreg_s(SYS_HFGWTR_EL2);
> 
> > Doesn't this crash an nVHE host ?
> 
> This series is probably moot given Marc's nv series (or at least needs
> another rework) but could you expand on the issue you're seeing please?

You're reading EL2 registers from EL1. What could possibly go wrong?

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 2/2] KVM: arm64: Move FGT value configuration to vCPU state

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 449 bytes --]

On Thu, Jul 13, 2023 at 04:22:41PM +0100, Marc Zyngier wrote:
> Mark Brown <broonie@kernel.org> wrote:

> > This series is probably moot given Marc's nv series (or at least needs
> > another rework) but could you expand on the issue you're seeing please?

> You're reading EL2 registers from EL1. What could possibly go wrong?

Ah, I see - I lost track of what's run in what context here and also
hate tests that need kernel command line arguments.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v5 2/2] KVM: arm64: Move FGT value configuration to vCPU state

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 449 bytes --]

On Thu, Jul 13, 2023 at 04:22:41PM +0100, Marc Zyngier wrote:
> Mark Brown <broonie@kernel.org> wrote:

> > This series is probably moot given Marc's nv series (or at least needs
> > another rework) but could you expand on the issue you're seeing please?

> You're reading EL2 registers from EL1. What could possibly go wrong?

Ah, I see - I lost track of what's run in what context here and also
hate tests that need kernel command line arguments.

[-- 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