On Wed, Jul 25, 2018 at 12:41:06PM +0100, Dave Martin wrote:
> On Thu, Jul 19, 2018 at 01:08:10PM +0200, Andrew Jones wrote:
> > On Thu, Jun 21, 2018 at 03:57:34PM +0100, Dave Martin wrote:
> > > Since SVE will be enabled or disabled on a per-vcpu basis, a flag
> > > is needed in order to track which vcpus have it enabled.
> > >
> > > This patch adds a suitable flag and a helper for checking it.
> > >
> > > Signed-off-by: Dave Martin <Dave.Martin@xxxxxxx>
> > > ---
> > > arch/arm64/include/asm/kvm_host.h | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > >
> > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > > index 9671ddd..609d08b 100644
> > > --- a/arch/arm64/include/asm/kvm_host.h
> > > +++ b/arch/arm64/include/asm/kvm_host.h
> > > @@ -308,6 +308,14 @@ struct kvm_vcpu_arch {
> > > #define KVM_ARM64_FP_HOST (1 << 2) /* host FP regs loaded */
> > > #define KVM_ARM64_HOST_SVE_IN_USE (1 << 3) /* backup for host TIF_SVE */
> > > #define KVM_ARM64_HOST_SVE_ENABLED (1 << 4) /* SVE enabled for EL0 */
> > > +#define KVM_ARM64_GUEST_HAS_SVE (1 << 5) /* SVE exposed to guest */
> > > +
> > > +static inline bool vcpu_has_sve(struct kvm_vcpu_arch const *vcpu_arch)
> > > +{
> > > + return system_supports_sve() &&
> >
> > system_supports_sve() checks cpus_have_const_cap(), not
> > this_cpu_has_cap(), so, iiuc, the result of this check won't
> > change, regardless of which cpu it's run on at the time.
>
> That's correct: this is intentional.
>
> If any physical cpu doesn't have SVE, we treat it is absent from the
> whole system, and we don't permit its use. This ensures that any task
> or vcpu can always be migrated to any physical cpu.
> >
> > > + (vcpu_arch->flags & KVM_ARM64_GUEST_HAS_SVE);
> >
> > Since this flag can only be set if system_supports_sve() is
> > true at vcpu init time, then it isn't necessary to always check
> > system_supports_sve() in this function. Or, should
> > system_supports_sve() be changed to use this_cpu_has_cap()?
>
> The main purpose of system_supports_sve() here is to shadow the check on
> vcpu_arch->flags with a static branch. If the system doesn't support
> SVE, we don't pay the runtime cost of the dynamic check on
> vcpu_arch->flags.
>
> If the kernel is built with CONFIG_ARM64_SVE=n, the dynamic check should
> be entirely optimised away by the compiler.
Ah, that makes sense. Thanks for clarifying it.
>
> I'd rather not add an explicit comment for this because the same
> convention is followed elsewhere -- thus for consistency the comment
> would need to be added in a lot of places.
Agreed that we don't need a comment. A note in the commit message might
have been nice though.
Thanks,
drew
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
