Re: [PATCH v7 12/27] KVM: arm64/sve: System register context switch and access support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux KVM]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux