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

[PATCH v2 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 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       | 16 ++++++++++++++
 arch/arm64/include/asm/kvm_host.h          |  2 ++
 arch/arm64/kernel/cpufeature.c             | 11 ++++++++++
 arch/arm64/kvm/arm.c                       |  1 +
 arch/arm64/kvm/hyp/include/hyp/switch.h    | 35 ++++++++++++++++--------------
 arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h |  9 ++++++++
 arch/arm64/tools/cpucaps                   |  1 +
 7 files changed, 59 insertions(+), 16 deletions(-)
---
base-commit: e8d018dd0257f744ca50a729e3d042cf2ec9da65
change-id: 20230301-kvm-arm64-fgt-e5dd12746f67

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

[PATCH v2 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 2e3e55139777..6d1cda522f69 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -2698,6 +2698,17 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 		.matches = has_cpuid_feature,
 		.min_field_value = 1,
 	},
+	{
+		.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 37b1340e9646..b63cd342add2 100644
--- a/arch/arm64/tools/cpucaps
+++ b/arch/arm64/tools/cpucaps
@@ -24,6 +24,7 @@ HAS_DIT
 HAS_E0PD
 HAS_ECV
 HAS_EPAN
+HAS_FGT
 HAS_GENERIC_AUTH
 HAS_GENERIC_AUTH_ARCH_QARMA3
 HAS_GENERIC_AUTH_ARCH_QARMA5

-- 
2.30.2

[PATCH v2 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
if we want to manage fine grained traps for the guest and which to
enable based on the presence of that feature. In order to support SME,
PIE 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. In order to ensure that the fine
grained traps are restored along with other traps there is a bit of
asymmetry with where the registers are restored on guest exit.

Currently we always set this register to 0 when running the guest so
unconditionally use that value for guests, future patches will configure
this.

No functional change, though we will do additional saves of the guest
FGT register configurations and will save and restore even if the host
and guest states are identical.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/arm64/include/asm/kvm_emulate.h       | 16 ++++++++++++++
 arch/arm64/include/asm/kvm_host.h          |  2 ++
 arch/arm64/kvm/arm.c                       |  1 +
 arch/arm64/kvm/hyp/include/hyp/switch.h    | 35 ++++++++++++++++--------------
 arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h |  9 ++++++++
 5 files changed, 47 insertions(+), 16 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index b31b32ecbe2d..9f88bcfdff70 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -107,6 +107,22 @@ 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;
+
+	/*
+	 * Enable traps for the guest by default:
+	 *
+	 * ACCDATA_EL1, GCSPR_EL0, GCSCRE0_EL1, GCSPR_EL1, GCSCR_EL1,
+	 * SMPRI_EL1, TPIDR2_EL0, RCWMASK_EL1, PIRE0_EL1, PIR_EL1,
+	 * POR_EL0, POR_EL1, S2POR_EL1, MAIR2_EL1, and AMAIR_EL1,
+	 */
+	__vcpu_sys_reg(vcpu, HFGRTR_EL2) = 0;
+	__vcpu_sys_reg(vcpu, HFGWTR_EL2) = 0;
+}
+
 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 bcd774d74f34..d81831e36443 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -365,6 +365,8 @@ enum vcpu_sysreg {
 	TPIDR_EL2,	/* EL2 Software Thread ID Register */
 	CNTHCTL_EL2,	/* Counter-timer Hypervisor Control register */
 	SP_EL2,		/* EL2 Stack Pointer */
+	HFGRTR_EL2,	/* Fine Grained Read Traps */
+	HFGWTR_EL2,	/* Fine Grained Write Traps */
 
 	NR_SYS_REGS	/* Nothing after this line! */
 };
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 3bd732eaf087..baa8d1a089bd 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1205,6 +1205,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 = CPTR_EL2_DEFAULT;
 
 	/*
diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index 07d37ff88a3f..bf0183a3a82d 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -88,33 +88,36 @@ 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 (cpus_have_final_cap(ARM64_SME)) {
-		sysreg_clear_set_s(SYS_HFGRTR_EL2,
-				   HFGxTR_EL2_nSMPRI_EL1_MASK |
-				   HFGxTR_EL2_nTPIDR2_EL0_MASK,
-				   0);
-		sysreg_clear_set_s(SYS_HFGWTR_EL2,
-				   HFGxTR_EL2_nSMPRI_EL1_MASK |
-				   HFGxTR_EL2_nTPIDR2_EL0_MASK,
-				   0);
+	if (cpus_have_final_cap(ARM64_HAS_FGT)) {
+		write_sysreg_s(__vcpu_sys_reg(vcpu, HFGRTR_EL2),
+			       SYS_HFGRTR_EL2);
+
+		write_sysreg_s(__vcpu_sys_reg(vcpu, HFGWTR_EL2),
+			       SYS_HFGWTR_EL2);
 	}
 }
 
 static inline void __deactivate_traps_common(struct kvm_vcpu *vcpu)
 {
+	struct kvm_cpu_context *host_ctxt;
+
 	write_sysreg(vcpu->arch.mdcr_el2_host, mdcr_el2);
 
 	write_sysreg(0, hstr_el2);
 	if (kvm_arm_support_pmu_v3())
 		write_sysreg(0, pmuserenr_el0);
 
-	if (cpus_have_final_cap(ARM64_SME)) {
-		sysreg_clear_set_s(SYS_HFGRTR_EL2, 0,
-				   HFGxTR_EL2_nSMPRI_EL1_MASK |
-				   HFGxTR_EL2_nTPIDR2_EL0_MASK);
-		sysreg_clear_set_s(SYS_HFGWTR_EL2, 0,
-				   HFGxTR_EL2_nSMPRI_EL1_MASK |
-				   HFGxTR_EL2_nTPIDR2_EL0_MASK);
+	/*
+	 * Restore the host FGT configuration here since it's managing
+	 * traps.
+	 */
+	if (cpus_have_final_cap(ARM64_HAS_FGT)) {
+		host_ctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt;
+
+		write_sysreg_s(__vcpu_sys_reg(vcpu, HFGRTR_EL2),
+			       SYS_HFGRTR_EL2);
+		write_sysreg_s(__vcpu_sys_reg(vcpu, HFGWTR_EL2),
+			       SYS_HFGWTR_EL2);
 	}
 }
 
diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
index 699ea1f8d409..7e67a3e27749 100644
--- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
+++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
@@ -19,6 +19,15 @@
 static inline void __sysreg_save_common_state(struct kvm_cpu_context *ctxt)
 {
 	ctxt_sys_reg(ctxt, MDSCR_EL1)	= read_sysreg(mdscr_el1);
+
+	/*
+	 * These are restored as part of trap disablement rather than
+	 * in __sysreg_restore_common_state().
+	 */
+	if (cpus_have_final_cap(ARM64_HAS_FGT)) {
+		ctxt_sys_reg(ctxt, HFGRTR_EL2) = read_sysreg_s(SYS_HFGRTR_EL2);
+		ctxt_sys_reg(ctxt, HFGWTR_EL2) = read_sysreg_s(SYS_HFGWTR_EL2);
+	}
 }
 
 static inline void __sysreg_save_user_state(struct kvm_cpu_context *ctxt)

-- 
2.30.2

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

[Marc Zyngier]

On Thu, 23 Mar 2023 15:48:36 +0000,
Mark Brown <broonie@kernel.org> wrote:
> 
> Currently the only fine grained traps we use are the SME ones and we decide
> if we want to manage fine grained traps for the guest and which to
> enable based on the presence of that feature. In order to support SME,
> PIE 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. In order to ensure that the fine
> grained traps are restored along with other traps there is a bit of
> asymmetry with where the registers are restored on guest exit.
> 
> Currently we always set this register to 0 when running the guest so
> unconditionally use that value for guests, future patches will configure
> this.
> 
> No functional change, though we will do additional saves of the guest
> FGT register configurations and will save and restore even if the host
> and guest states are identical.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/include/asm/kvm_emulate.h       | 16 ++++++++++++++
>  arch/arm64/include/asm/kvm_host.h          |  2 ++
>  arch/arm64/kvm/arm.c                       |  1 +
>  arch/arm64/kvm/hyp/include/hyp/switch.h    | 35 ++++++++++++++++--------------
>  arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h |  9 ++++++++
>  5 files changed, 47 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index b31b32ecbe2d..9f88bcfdff70 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -107,6 +107,22 @@ 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;
> +
> +	/*
> +	 * Enable traps for the guest by default:
> +	 *
> +	 * ACCDATA_EL1, GCSPR_EL0, GCSCRE0_EL1, GCSPR_EL1, GCSCR_EL1,
> +	 * SMPRI_EL1, TPIDR2_EL0, RCWMASK_EL1, PIRE0_EL1, PIR_EL1,
> +	 * POR_EL0, POR_EL1, S2POR_EL1, MAIR2_EL1, and AMAIR_EL1,
> +	 */
> +	__vcpu_sys_reg(vcpu, HFGRTR_EL2) = 0;
> +	__vcpu_sys_reg(vcpu, HFGWTR_EL2) = 0;
> +}
> +
>  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 bcd774d74f34..d81831e36443 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -365,6 +365,8 @@ enum vcpu_sysreg {
>  	TPIDR_EL2,	/* EL2 Software Thread ID Register */
>  	CNTHCTL_EL2,	/* Counter-timer Hypervisor Control register */
>  	SP_EL2,		/* EL2 Stack Pointer */
> +	HFGRTR_EL2,	/* Fine Grained Read Traps */
> +	HFGWTR_EL2,	/* Fine Grained Write Traps */

No, this is the wrong spot. These registers describe the *guest*
state. Not the state that KVM sets for its own use. These registers
would be used by a guest hypervisor to manage traps it uses for its
own guests.

See HCR_EL2, for example, which exists both in this register file for
the EL2 guests, and kvm_vcpu_arch for KVM to manage the guest.

>  
>  	NR_SYS_REGS	/* Nothing after this line! */
>  };
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 3bd732eaf087..baa8d1a089bd 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -1205,6 +1205,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 = CPTR_EL2_DEFAULT;
>  
>  	/*
> diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
> index 07d37ff88a3f..bf0183a3a82d 100644
> --- a/arch/arm64/kvm/hyp/include/hyp/switch.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
> @@ -88,33 +88,36 @@ 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 (cpus_have_final_cap(ARM64_SME)) {
> -		sysreg_clear_set_s(SYS_HFGRTR_EL2,
> -				   HFGxTR_EL2_nSMPRI_EL1_MASK |
> -				   HFGxTR_EL2_nTPIDR2_EL0_MASK,
> -				   0);
> -		sysreg_clear_set_s(SYS_HFGWTR_EL2,
> -				   HFGxTR_EL2_nSMPRI_EL1_MASK |
> -				   HFGxTR_EL2_nTPIDR2_EL0_MASK,
> -				   0);
> +	if (cpus_have_final_cap(ARM64_HAS_FGT)) {
> +		write_sysreg_s(__vcpu_sys_reg(vcpu, HFGRTR_EL2),
> +			       SYS_HFGRTR_EL2);
> +
> +		write_sysreg_s(__vcpu_sys_reg(vcpu, HFGWTR_EL2),
> +			       SYS_HFGWTR_EL2);
>  	}
>  }
>  
>  static inline void __deactivate_traps_common(struct kvm_vcpu *vcpu)
>  {
> +	struct kvm_cpu_context *host_ctxt;
> +
>  	write_sysreg(vcpu->arch.mdcr_el2_host, mdcr_el2);
>  
>  	write_sysreg(0, hstr_el2);
>  	if (kvm_arm_support_pmu_v3())
>  		write_sysreg(0, pmuserenr_el0);
>  
> -	if (cpus_have_final_cap(ARM64_SME)) {
> -		sysreg_clear_set_s(SYS_HFGRTR_EL2, 0,
> -				   HFGxTR_EL2_nSMPRI_EL1_MASK |
> -				   HFGxTR_EL2_nTPIDR2_EL0_MASK);
> -		sysreg_clear_set_s(SYS_HFGWTR_EL2, 0,
> -				   HFGxTR_EL2_nSMPRI_EL1_MASK |
> -				   HFGxTR_EL2_nTPIDR2_EL0_MASK);
> +	/*
> +	 * Restore the host FGT configuration here since it's managing
> +	 * traps.
> +	 */
> +	if (cpus_have_final_cap(ARM64_HAS_FGT)) {
> +		host_ctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt;
> +
> +		write_sysreg_s(__vcpu_sys_reg(vcpu, HFGRTR_EL2),
> +			       SYS_HFGRTR_EL2);
> +		write_sysreg_s(__vcpu_sys_reg(vcpu, HFGWTR_EL2),
> +			       SYS_HFGWTR_EL2);
>  	}
>  }
>  
> diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> index 699ea1f8d409..7e67a3e27749 100644
> --- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> @@ -19,6 +19,15 @@
>  static inline void __sysreg_save_common_state(struct kvm_cpu_context *ctxt)
>  {
>  	ctxt_sys_reg(ctxt, MDSCR_EL1)	= read_sysreg(mdscr_el1);
> +
> +	/*
> +	 * These are restored as part of trap disablement rather than
> +	 * in __sysreg_restore_common_state().
> +	 */
> +	if (cpus_have_final_cap(ARM64_HAS_FGT)) {
> +		ctxt_sys_reg(ctxt, HFGRTR_EL2) = read_sysreg_s(SYS_HFGRTR_EL2);
> +		ctxt_sys_reg(ctxt, HFGWTR_EL2) = read_sysreg_s(SYS_HFGWTR_EL2);
> +	}

I understand why this gets saved for the host, as we need to restore
it. But why does it need to be saved for the guest? Nothing changes
it, and certainly not the guest itself.

This looks pretty wrong to me.

	M.

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

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

[Mark Brown]

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

On Thu, Mar 23, 2023 at 07:34:08PM +0000, Marc Zyngier wrote:
> Mark Brown <broonie@kernel.org> wrote:

> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -365,6 +365,8 @@ enum vcpu_sysreg {
> >  	TPIDR_EL2,	/* EL2 Software Thread ID Register */
> >  	CNTHCTL_EL2,	/* Counter-timer Hypervisor Control register */
> >  	SP_EL2,		/* EL2 Stack Pointer */
> > +	HFGRTR_EL2,	/* Fine Grained Read Traps */
> > +	HFGWTR_EL2,	/* Fine Grained Write Traps */

> No, this is the wrong spot. These registers describe the *guest*
> state. Not the state that KVM sets for its own use. These registers
> would be used by a guest hypervisor to manage traps it uses for its
> own guests.

Ah, sorry, I misinterpreted what you meant when you said to put them in
kvm_vcpu_context - the sys_regs array in there seemed to be the place
where data for sysregs is stored.  The large set of EL2 registers in
there already stopped alarm bells going off, as did the fact that with
the patch above we were loading a copy from there for the guest.

To clarify, place just regular member variables in kvm_vcpu_context then
save host state to the percpu host context (I guess while activating
traps) and load the guest state from vcpu->arch.ctxt.hfgxtr_el2?  I've
got that testing locally just now.

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

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

[Will Deacon]

On Thu, Mar 23, 2023 at 03:48:36PM +0000, Mark Brown wrote:
> Currently the only fine grained traps we use are the SME ones and we decide
> if we want to manage fine grained traps for the guest and which to
> enable based on the presence of that feature. In order to support SME,
> PIE 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. In order to ensure that the fine
> grained traps are restored along with other traps there is a bit of
> asymmetry with where the registers are restored on guest exit.
> 
> Currently we always set this register to 0 when running the guest so
> unconditionally use that value for guests, future patches will configure
> this.
> 
> No functional change, though we will do additional saves of the guest
> FGT register configurations and will save and restore even if the host
> and guest states are identical.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/include/asm/kvm_emulate.h       | 16 ++++++++++++++
>  arch/arm64/include/asm/kvm_host.h          |  2 ++
>  arch/arm64/kvm/arm.c                       |  1 +
>  arch/arm64/kvm/hyp/include/hyp/switch.h    | 35 ++++++++++++++++--------------
>  arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h |  9 ++++++++
>  5 files changed, 47 insertions(+), 16 deletions(-)

[...]

> diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
> index 07d37ff88a3f..bf0183a3a82d 100644
> --- a/arch/arm64/kvm/hyp/include/hyp/switch.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
> @@ -88,33 +88,36 @@ 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 (cpus_have_final_cap(ARM64_SME)) {
> -		sysreg_clear_set_s(SYS_HFGRTR_EL2,
> -				   HFGxTR_EL2_nSMPRI_EL1_MASK |
> -				   HFGxTR_EL2_nTPIDR2_EL0_MASK,
> -				   0);
> -		sysreg_clear_set_s(SYS_HFGWTR_EL2,
> -				   HFGxTR_EL2_nSMPRI_EL1_MASK |
> -				   HFGxTR_EL2_nTPIDR2_EL0_MASK,
> -				   0);
> +	if (cpus_have_final_cap(ARM64_HAS_FGT)) {
> +		write_sysreg_s(__vcpu_sys_reg(vcpu, HFGRTR_EL2),
> +			       SYS_HFGRTR_EL2);
> +
> +		write_sysreg_s(__vcpu_sys_reg(vcpu, HFGWTR_EL2),
> +			       SYS_HFGWTR_EL2);
>  	}
>  }
>  
>  static inline void __deactivate_traps_common(struct kvm_vcpu *vcpu)
>  {
> +	struct kvm_cpu_context *host_ctxt;
> +
>  	write_sysreg(vcpu->arch.mdcr_el2_host, mdcr_el2);
>  
>  	write_sysreg(0, hstr_el2);
>  	if (kvm_arm_support_pmu_v3())
>  		write_sysreg(0, pmuserenr_el0);
>  
> -	if (cpus_have_final_cap(ARM64_SME)) {
> -		sysreg_clear_set_s(SYS_HFGRTR_EL2, 0,
> -				   HFGxTR_EL2_nSMPRI_EL1_MASK |
> -				   HFGxTR_EL2_nTPIDR2_EL0_MASK);
> -		sysreg_clear_set_s(SYS_HFGWTR_EL2, 0,
> -				   HFGxTR_EL2_nSMPRI_EL1_MASK |
> -				   HFGxTR_EL2_nTPIDR2_EL0_MASK);
> +	/*
> +	 * Restore the host FGT configuration here since it's managing
> +	 * traps.
> +	 */
> +	if (cpus_have_final_cap(ARM64_HAS_FGT)) {
> +		host_ctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt;
> +
> +		write_sysreg_s(__vcpu_sys_reg(vcpu, HFGRTR_EL2),
> +			       SYS_HFGRTR_EL2);
> +		write_sysreg_s(__vcpu_sys_reg(vcpu, HFGWTR_EL2),
> +			       SYS_HFGWTR_EL2);

I don't understand this hunk. Where is `host_ctxt` being used?

Will

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

[Mark Brown]

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

On Tue, Mar 28, 2023 at 04:27:45PM +0100, Will Deacon wrote:
> On Thu, Mar 23, 2023 at 03:48:36PM +0000, Mark Brown wrote:

> > +	if (cpus_have_final_cap(ARM64_HAS_FGT)) {
> > +		host_ctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt;
> > +
> > +		write_sysreg_s(__vcpu_sys_reg(vcpu, HFGRTR_EL2),
> > +			       SYS_HFGRTR_EL2);
> > +		write_sysreg_s(__vcpu_sys_reg(vcpu, HFGWTR_EL2),
> > +			       SYS_HFGWTR_EL2);

> I don't understand this hunk. Where is `host_ctxt` being used?

That's bitrot from a previous version, should be removed.

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