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

[Marc Zyngier <maz@kernel.org>]

On Mon, 06 Mar 2023 16:08:52 +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.
> 
> Currently we always set this register to 0 when running the guest and
> unused bits in the registers are RES0 so unconditionally use that value
> for guests, future patches will configure this.

This doesn't quite tell the story.

> 
> The configuration for the host is saved at guest reset under the
> assumption that the traps are not dynamically managed for the host at
> runtime.  This is currently the case, if this changes then we will need
> to save each time we enter the guest.
> 
> No functional change.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/include/asm/kvm_emulate.h    | 12 ++++++++++++
>  arch/arm64/include/asm/kvm_host.h       |  6 ++++++
>  arch/arm64/kvm/arm.c                    |  1 +
>  arch/arm64/kvm/hyp/include/hyp/switch.h | 31 +++++++++++++++----------------
>  4 files changed, 34 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index b31b32ecbe2d..20f2faae12d4 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -107,6 +107,18 @@ 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_host = read_sysreg_s(SYS_HFGRTR_EL2);
> +	vcpu->arch.hfgwtr_el2_host = read_sysreg_s(SYS_HFGWTR_EL2);
> +
> +	vcpu->arch.hfgrtr_el2 = 0;
> +	vcpu->arch.hfgwtr_el2 = 0;

Although this looks completely innocent, this actually have the effect
of trapping the SMPRI_EL1 and TPIDR2_EL0 registers, something that is
self documented in the current code, and that completely disappears
with this patch.

This needs documenting by enumerating the sysregs that get trapped.

> +}
> +
>  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 a1892a8f6032..09b223635764 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -530,6 +530,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;

Why do we have both host and guest? This is the vcpu structure, and
the host state doesn't belong here. If you want to save some host
state, place the hfgxtr_el2 fields in kvm_cpu_context, and use the
per-CPU instance of this structure to save the host state.

	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