[RFC PATCH 10/16] KVM: arm64: Add a vcpu flag to control SVE visibility for the guest
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
---
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() &&
+ (vcpu_arch->flags & KVM_ARM64_GUEST_HAS_SVE);
+
+}
#define vcpu_gp_regs(v)(&(v)->arch.ctxt.gp_regs)
--
2.1.4
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 10/16] KVM: arm64: Add a vcpu flag to control SVE visibility for the guest
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
> ---
> 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.
> + (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()?
Thanks,
drew
> +
> +}
>
> #define vcpu_gp_regs(v) (&(v)->arch.ctxt.gp_regs)
>
> --
> 2.1.4
>
> ___
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 10/16] KVM: arm64: Add a vcpu flag to control SVE visibility for the guest
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
> ---
> 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)
Shouldn't this vcpu function take a vcpu instead of a vcpu_arch?
Thanks,
drew
> +{
> + return system_supports_sve() &&
> + (vcpu_arch->flags & KVM_ARM64_GUEST_HAS_SVE);
> +
> +}
>
> #define vcpu_gp_regs(v) (&(v)->arch.ctxt.gp_regs)
>
> --
> 2.1.4
>
> ___
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 10/16] KVM: arm64: Add a vcpu flag to control SVE visibility for the guest
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
> > ---
> > 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.
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.
Cheers
---Dave
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 10/16] KVM: arm64: Add a vcpu flag to control SVE visibility for the guest
On Thu, Jul 19, 2018 at 05:02:44PM +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
> > ---
> > 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)
>
> Shouldn't this vcpu function take a vcpu instead of a vcpu_arch?
Logically it could. There was some circular include issue that made it
tricky to get the definition of struct kvm_vcpu here, but I may have
another go at it.
Cheers
---Dave
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 10/16] KVM: arm64: Add a vcpu flag to control SVE visibility for the guest
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
> > > ---
> > > 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@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 10/16] KVM: arm64: Add a vcpu flag to control SVE visibility for the guest
On Wed, Jul 25, 2018 at 03:43:59PM +0200, Andrew Jones wrote: > On Wed, Jul 25, 2018 at 12:41:06PM +0100, Dave Martin wrote: [...] > > 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. Sure, I'll add something in the respin. Cheers ---Dave ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
