On Mon, 08 Jul 2024 18:34:36 +0100,
Mark Brown <broonie@xxxxxxxxxx> wrote:
>
> [1 <text/plain; us-ascii (7bit)>]
> On Mon, Jul 08, 2024 at 04:44:34PM +0100, Marc Zyngier wrote:
> > Just like the rest of the FP/SIMD state, FPMR needs to be context
> > switched.
>
> > The only interesting thing here is that we need to treat the pKVM
> > part a bit differently, as the host FP state is never written back
> > to the vcpu thread, but instead stored locally and eagerly restored.
>
> > Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
> > ---
> > arch/arm64/include/asm/kvm_host.h | 10 ++++++++++
> > arch/arm64/kvm/fpsimd.c | 1 +
> > arch/arm64/kvm/hyp/nvhe/hyp-main.c | 4 ++++
> > arch/arm64/kvm/hyp/nvhe/switch.c | 10 ++++++++++
> > arch/arm64/kvm/hyp/vhe/switch.c | 4 ++++
> > 5 files changed, 29 insertions(+)
>
> I'm possibly missing something here but I'm not seeing where we load the
> state for the guest, especially in the VHE case. I would expect to see
> a change in kvm_hyp_handle_fpsimd() to load FPMR for guests with the
> feature (it needs to be in there to keep in sync with the ownership
> tracking for the rest of the FP state, and to avoid loading needlessly
> in cases where the guest never touches FP).
>
> Saving for the guest was handled in the previous patch.
>
> > diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
> > index 77010b76c150f..a307c1d5ac874 100644
> > --- a/arch/arm64/kvm/hyp/vhe/switch.c
> > +++ b/arch/arm64/kvm/hyp/vhe/switch.c
> > @@ -312,6 +312,10 @@ static bool kvm_hyp_handle_eret(struct kvm_vcpu *vcpu, u64 *exit_code)
> > static void kvm_hyp_save_fpsimd_host(struct kvm_vcpu *vcpu)
> > {
> > __fpsimd_save_state(*host_data_ptr(fpsimd_state));
> > +
> > + if (system_supports_fpmr() &&
> > + kvm_has_feat(vcpu->kvm, ID_AA64PFR2_EL1, FPMR, IMP))
> > + **host_data_ptr(fpmr_ptr) = read_sysreg_s(SYS_FPMR);
> > }
>
> That's only saving the host state, it doesn't load the guest state.
Ah, I forgot to cherry-pick the fixes. Fsck knows what else I forgot.
Thanks for reminding me.
M.
diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index f59ccfe11ab9a..52c7dc8446f16 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -404,6 +404,10 @@ static bool kvm_hyp_handle_fpsimd(struct kvm_vcpu *vcpu, u64 *exit_code)
else
__fpsimd_restore_state(&vcpu->arch.ctxt.fp_regs);
+ if (system_supports_fpmr() &&
+ kvm_has_feat(kern_hyp_va(vcpu->kvm), ID_AA64PFR2_EL1, FPMR, IMP))
+ write_sysreg_s(__vcpu_sys_reg(vcpu, FPMR), SYS_FPMR);
+
/* Skip restoring fpexc32 for AArch64 guests */
if (!(read_sysreg(hcr_el2) & HCR_RW))
write_sysreg(__vcpu_sys_reg(vcpu, FPEXC32_EL2), fpexc32_el2);
--
Without deviation from the norm, progress is not possible.
