On Wed, Apr 24, 2019 at 04:21:22PM +0100, Alex Bennée wrote:
>
> Dave Martin <Dave.Martin@xxxxxxx> writes:
>
> > This patch adds the necessary support for context switching ZCR_EL1
> > for each vcpu.
> >
> > ZCR_EL1 is trapped alongside the FPSIMD/SVE registers, so it makes
> > sense for it to be handled as part of the guest FPSIMD/SVE context
> > for context switch purposes instead of handling it as a general
> > system register. This means that it can be switched in lazily at
> > the appropriate time. No effort is made to track host context for
> > this register, since SVE requires VHE: thus the hosts's value for
> > this register lives permanently in ZCR_EL2 and does not alias the
> > guest's value at any time.
> >
> > The Hyp switch and fpsimd context handling code is extended
> > appropriately.
> >
> > Accessors are added in sys_regs.c to expose the SVE system
> > registers and ID register fields. Because these need to be
> > conditionally visible based on the guest configuration, they are
> > implemented separately for now rather than by use of the generic
> > system register helpers. This may be abstracted better later on
> > when/if there are more features requiring this model.
> >
> > ID_AA64ZFR0_EL1 is RO-RAZ for MRS/MSR when SVE is disabled for the
> > guest, but for compatibility with non-SVE aware KVM implementations
> > the register should not be enumerated at all for KVM_GET_REG_LIST
> > in this case. For consistency we also reject ioctl access to the
> > register. This ensures that a non-SVE-enabled guest looks the same
> > to userspace, irrespective of whether the kernel KVM implementation
> > supports SVE.
> >
> > Signed-off-by: Dave Martin <Dave.Martin@xxxxxxx>
> > Reviewed-by: Julien Thierry <julien.thierry@xxxxxxx>
> > Tested-by: zhang.lei <zhang.lei@xxxxxxxxxxxxxx>
> >
> > ---
> >
[...]
> > diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
> > index 1cf4f02..7053bf4 100644
> > --- a/arch/arm64/kvm/fpsimd.c
> > +++ b/arch/arm64/kvm/fpsimd.c
> > @@ -103,14 +103,21 @@ void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu)
> > void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu)
> > {
> > unsigned long flags;
> > + bool host_has_sve = system_supports_sve();
> > + bool guest_has_sve = vcpu_has_sve(vcpu);
> >
> > local_irq_save(flags);
> >
> > if (vcpu->arch.flags & KVM_ARM64_FP_ENABLED) {
> > + u64 *guest_zcr = &vcpu->arch.ctxt.sys_regs[ZCR_EL1];
> > +
>
> Is this just to avoid:
>
> vcpu->arch.ctxt.sys_regs[ZCR_EL1] = read_sysreg_s(SYS_ZCR_EL12);
No, it's just done to shorten the line. Otherwise a trailing = is hard
to avoid (which Marc didn't like) or the line has to be over 80 chars
(which I didn't like).
> in fact wouldn't:
>
> __vcpu_sys_reg(vcpu,ZCR_EL1) = read_sysreg_s(SYS_ZCR_EL12);
We could use __vcpu_sys_reg() yes, I missed that.
I could spin a patch for this, but it doesn't feel like a high priority
at this stage.
[...]
> Reviewed-by: Alex Bennée <alex.bennee@xxxxxxxxxx>
Thanks
---Dave
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
