[PATCH kvmtool v3 0/9] arm64: Pointer Authentication and SVE support

[PATCH kvmtool v3 0/9] arm64: Pointer Authentication and SVE support

[Dave Martin]

This series, based on kvmtool master [1], implements basic support for
pointer authentication and SVE for guests.

A git tree is also available [2].

For pointer auth, I include Amit's v10 patch [3], with some additional
refactoring to sit nicely alongside SVE, and some cosmetic / diagnostic
tidyups discussed during review on-list.  (I've kept the extra changes
separate for easier review, but they could be folded if desired.)

[Maintainer note: I'd like Amit to comment on my changes on top of his
pointer auth patch, but the first 4 patches just re-sync headers and
could be pulled earlier if you feel like it.]


This series has been tested against Linux v5.2-rc1.

If people have a strong view on the --sve-vls parameter, I'd be
interested to discuss what that should look like.  Since this is
primarily a debug/experimentation option, the current implementation is
probably good enough though.

[1] 
git://git.kernel.org/pub/scm/linux/kernel/git/will/kvmtool.git master
https://git.kernel.org/pub/scm/linux/kernel/git/will/kvmtool.git/log/
eaeaf60808d6 ("virtio/blk: Avoid taking pointer to packed struct")

[2] [PATCH v10 3/5] KVM: arm64: Add userspace flag to enable pointer authentication
https://lore.kernel.org/linux-arm-kernel/1555994558-26349-6-git-send-email-amit.kachhap@arm.com/

[3]
git://linux-arm.org/kvmtool-dm.git sve/v3/head
http://linux-arm.org/git?p=kvmtool-dm.git;a=shortlog;h=refs/heads/sve/v3/head


Amit Daniel Kachhap (1):
  KVM: arm/arm64: Add a vcpu feature for pointer authentication

Dave Martin (8):
  update_headers.sh: Add missing shell quoting
  update_headers.sh: Cleanly report failure on error
  update_headers.sh: arm64: Copy sve_context.h if available
  update_headers: Sync kvm UAPI headers with linux v5.1-rc1
  arm/arm64: Factor out ptrauth vcpu feature setup
  arm64: Make ptrauth enable/disable diagnostics more user-friendly
  arm64: Add SVE support
  arm64: Select SVE vector lengths via the command line

 arm/aarch32/include/kvm/kvm-cpu-arch.h    |   7 ++
 arm/aarch64/include/asm/kvm.h             |  43 +++++++++
 arm/aarch64/include/asm/sve_context.h     |  53 +++++++++++
 arm/aarch64/include/kvm/kvm-config-arch.h |  16 +++-
 arm/aarch64/include/kvm/kvm-cpu-arch.h    |   3 +
 arm/aarch64/kvm-cpu.c                     | 148 ++++++++++++++++++++++++++++++
 arm/include/arm-common/kvm-config-arch.h  |   5 +
 arm/kvm-cpu.c                             |   5 +
 include/linux/kvm.h                       |  15 ++-
 powerpc/include/asm/kvm.h                 |  48 ++++++++++
 util/update_headers.sh                    |  25 +++--
 x86/include/asm/kvm.h                     |   1 +
 12 files changed, 360 insertions(+), 9 deletions(-)
 create mode 100644 arm/aarch64/include/asm/sve_context.h

-- 
2.1.4

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH kvmtool v3 1/9] update_headers.sh: Add missing shell quoting

[Dave Martin]

update_headers.sh can break if the current working directory has a
funny name or if something odd is passed for LINUX_ROOT.

In the interest of cleanliness, quote where appropriate.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 util/update_headers.sh | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/util/update_headers.sh b/util/update_headers.sh
index 2d93646..4ba1b9f 100755
--- a/util/update_headers.sh
+++ b/util/update_headers.sh
@@ -11,17 +11,17 @@ if [ "$#" -ge 1 ]
 then
 	LINUX_ROOT="$1"
 else
-	LINUX_ROOT=/lib/modules/$(uname -r)/source
+	LINUX_ROOT="/lib/modules/$(uname -r)/source"
 fi
 
-if [ ! -d $LINUX_ROOT/include/uapi/linux ]
+if [ ! -d "$LINUX_ROOT/include/uapi/linux" ]
 then
 	echo "$LINUX_ROOT does not seem to be valid Linux source tree."
 	echo "usage: $0 [path-to-Linux-source-tree]"
 	exit 1
 fi
 
-cp $LINUX_ROOT/include/uapi/linux/kvm.h include/linux
+cp -- "$LINUX_ROOT/include/uapi/linux/kvm.h" include/linux
 
 for arch in arm arm64 mips powerpc x86
 do
@@ -30,6 +30,6 @@ do
 		arm64) KVMTOOL_PATH=arm/aarch64 ;;
 		*) KVMTOOL_PATH=$arch ;;
 	esac
-	cp $LINUX_ROOT/arch/$arch/include/uapi/asm/kvm.h \
-		$KVMTOOL_PATH/include/asm
+	cp -- "$LINUX_ROOT/arch/$arch/include/uapi/asm/kvm.h" \
+		"$KVMTOOL_PATH/include/asm"
 done
-- 
2.1.4

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH kvmtool v3 1/9] update_headers.sh: Add missing shell quoting

[Andre Przywara]

On Thu, 30 May 2019 16:13:06 +0100
Dave Martin <Dave.Martin@arm.com> wrote:

> update_headers.sh can break if the current working directory has a
> funny name or if something odd is passed for LINUX_ROOT.

Do you actually have spaces in your Linux path? ;-)
 
> In the interest of cleanliness, quote where appropriate.
> 
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>

Looks alright to me:

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre.

> ---
>  util/update_headers.sh | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/util/update_headers.sh b/util/update_headers.sh
> index 2d93646..4ba1b9f 100755
> --- a/util/update_headers.sh
> +++ b/util/update_headers.sh
> @@ -11,17 +11,17 @@ if [ "$#" -ge 1 ]
>  then
>  	LINUX_ROOT="$1"
>  else
> -	LINUX_ROOT=/lib/modules/$(uname -r)/source
> +	LINUX_ROOT="/lib/modules/$(uname -r)/source"
>  fi
>  
> -if [ ! -d $LINUX_ROOT/include/uapi/linux ]
> +if [ ! -d "$LINUX_ROOT/include/uapi/linux" ]
>  then
>  	echo "$LINUX_ROOT does not seem to be valid Linux source tree."
>  	echo "usage: $0 [path-to-Linux-source-tree]"
>  	exit 1
>  fi
>  
> -cp $LINUX_ROOT/include/uapi/linux/kvm.h include/linux
> +cp -- "$LINUX_ROOT/include/uapi/linux/kvm.h" include/linux
>  
>  for arch in arm arm64 mips powerpc x86
>  do
> @@ -30,6 +30,6 @@ do
>  		arm64) KVMTOOL_PATH=arm/aarch64 ;;
>  		*) KVMTOOL_PATH=$arch ;;
>  	esac
> -	cp $LINUX_ROOT/arch/$arch/include/uapi/asm/kvm.h \
> -		$KVMTOOL_PATH/include/asm
> +	cp -- "$LINUX_ROOT/arch/$arch/include/uapi/asm/kvm.h" \
> +		"$KVMTOOL_PATH/include/asm"
>  done

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH kvmtool v3 1/9] update_headers.sh: Add missing shell quoting

[Dave Martin]

On Fri, May 31, 2019 at 06:02:53PM +0100, Andre Przywara wrote:
> On Thu, 30 May 2019 16:13:06 +0100
> Dave Martin <Dave.Martin@arm.com> wrote:
> 
> > update_headers.sh can break if the current working directory has a
> > funny name or if something odd is passed for LINUX_ROOT.
> 
> Do you actually have spaces in your Linux path? ;-)

No.  I'm assuming that people using a fancy desktop need to call it
"My Linux Kernel" in order to comprehend what it is though.

(Only joking!)

> > In the interest of cleanliness, quote where appropriate.
> > 
> > Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> 
> Looks alright to me:
> 
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>

[...]

Thanks
---Dave
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH kvmtool v3 2/9] update_headers.sh: Cleanly report failure on error

[Dave Martin]

If in intermediate step fails, update_headers.sh blindly continues
and may return success status.

To avoid errors going unnoticed when driving this script, exit and
report failure status as soon as something goes wrong.  For good
measure, also fail on expansion of undefined shell variables to aid
future maintainers.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 util/update_headers.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/util/update_headers.sh b/util/update_headers.sh
index 4ba1b9f..a7e21b8 100755
--- a/util/update_headers.sh
+++ b/util/update_headers.sh
@@ -7,6 +7,8 @@
 # using the lib/modules/`uname -r`/source link.
 ########################################################################
 
+set -ue
+
 if [ "$#" -ge 1 ]
 then
 	LINUX_ROOT="$1"
-- 
2.1.4

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH kvmtool v3 2/9] update_headers.sh: Cleanly report failure on error

[Andre Przywara]

On Thu, 30 May 2019 16:13:07 +0100
Dave Martin <Dave.Martin@arm.com> wrote:

> If in intermediate step fails, update_headers.sh blindly continues
> and may return success status.
> 
> To avoid errors going unnoticed when driving this script, exit and
> report failure status as soon as something goes wrong.  For good
> measure, also fail on expansion of undefined shell variables to aid
> future maintainers.
> 
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>

Both "u" and "e" seem to be standard and work in dash and ash, so:

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Thanks,
Andre.

> ---
>  util/update_headers.sh | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/util/update_headers.sh b/util/update_headers.sh
> index 4ba1b9f..a7e21b8 100755
> --- a/util/update_headers.sh
> +++ b/util/update_headers.sh
> @@ -7,6 +7,8 @@
>  # using the lib/modules/`uname -r`/source link.
>  ########################################################################
>  
> +set -ue
> +
>  if [ "$#" -ge 1 ]
>  then
>  	LINUX_ROOT="$1"

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH kvmtool v3 2/9] update_headers.sh: Cleanly report failure on error

[Dave Martin]

On Fri, May 31, 2019 at 06:03:10PM +0100, Andre Przywara wrote:
> On Thu, 30 May 2019 16:13:07 +0100
> Dave Martin <Dave.Martin@arm.com> wrote:
> 
> > If in intermediate step fails, update_headers.sh blindly continues
> > and may return success status.
> > 
> > To avoid errors going unnoticed when driving this script, exit and
> > report failure status as soon as something goes wrong.  For good
> > measure, also fail on expansion of undefined shell variables to aid
> > future maintainers.
> > 
> > Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> 
> Both "u" and "e" seem to be standard and work in dash and ash, so:
> 
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Thanks.

Those options have been there forever, so I presume they are specified
by POSIX... but I confess I didn't check.

Cheers
---Dave
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH kvmtool v3 3/9] update_headers.sh: arm64: Copy sve_context.h if available

[Dave Martin]

The SVE KVM support for arm64 includes the additional backend
header <asm/sve_context.h> from <asm/kvm.h>.

So update this header if it is available.

To avoid creating a sudden dependency on a specific minimum kernel
version, ignore the header if the source kernel tree doesn't have
it.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 util/update_headers.sh | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/util/update_headers.sh b/util/update_headers.sh
index a7e21b8..90d3ead 100755
--- a/util/update_headers.sh
+++ b/util/update_headers.sh
@@ -25,11 +25,22 @@ fi
 
 cp -- "$LINUX_ROOT/include/uapi/linux/kvm.h" include/linux
 
+unset KVMTOOL_PATH
+
+copy_arm64 () {
+	local src=$LINUX_ROOT/arch/$arch/include/uapi/asm/sve_context.h
+
+	if [ -e "$src" ]
+	then
+		cp -- "$src" "$KVMTOOL_PATH/include/asm"
+	fi
+}
+
 for arch in arm arm64 mips powerpc x86
 do
 	case "$arch" in
 		arm) KVMTOOL_PATH=arm/aarch32 ;;
-		arm64) KVMTOOL_PATH=arm/aarch64 ;;
+		arm64) KVMTOOL_PATH=arm/aarch64; copy_arm64 ;;
 		*) KVMTOOL_PATH=$arch ;;
 	esac
 	cp -- "$LINUX_ROOT/arch/$arch/include/uapi/asm/kvm.h" \
-- 
2.1.4

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH kvmtool v3 3/9] update_headers.sh: arm64: Copy sve_context.h if available

[Andre Przywara]

On Thu, 30 May 2019 16:13:08 +0100
Dave Martin <Dave.Martin@arm.com> wrote:

> The SVE KVM support for arm64 includes the additional backend
> header <asm/sve_context.h> from <asm/kvm.h>.
> 
> So update this header if it is available.
> 
> To avoid creating a sudden dependency on a specific minimum kernel
> version, ignore the header if the source kernel tree doesn't have
> it.
> 
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> ---
>  util/update_headers.sh | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/util/update_headers.sh b/util/update_headers.sh
> index a7e21b8..90d3ead 100755
> --- a/util/update_headers.sh
> +++ b/util/update_headers.sh
> @@ -25,11 +25,22 @@ fi
>  
>  cp -- "$LINUX_ROOT/include/uapi/linux/kvm.h" include/linux
>  
> +unset KVMTOOL_PATH
> +
> +copy_arm64 () {
> +	local src=$LINUX_ROOT/arch/$arch/include/uapi/asm/sve_context.h

To go with your previous patches, aren't you missing the quotes here?

> +
> +	if [ -e "$src" ]
> +	then
> +		cp -- "$src" "$KVMTOOL_PATH/include/asm"
> +	fi
> +}
> +

Maybe we can make this slightly more generic?
copy_optional_arch() {
	local src="$LINUX_ROOT/arch/$arch/include/uapi/$1"
	[ -r "$src" ] && cp -- "$src" "$KVMTOOL_PATH/include/asm"
}
...
	arm64) KVMTOOL_PATH=arm/aarch64
	       copy_optional_arch asm/sve_context.h
	       ;;

Cheers,
Andre.


>  for arch in arm arm64 mips powerpc x86
>  do
>  	case "$arch" in
>  		arm) KVMTOOL_PATH=arm/aarch32 ;;
> -		arm64) KVMTOOL_PATH=arm/aarch64 ;;
> +		arm64) KVMTOOL_PATH=arm/aarch64; copy_arm64 ;;
>  		*) KVMTOOL_PATH=$arch ;;
>  	esac
>  	cp -- "$LINUX_ROOT/arch/$arch/include/uapi/asm/kvm.h" \

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH kvmtool v3 3/9] update_headers.sh: arm64: Copy sve_context.h if available

[Dave Martin]

On Fri, May 31, 2019 at 06:03:40PM +0100, Andre Przywara wrote:
> On Thu, 30 May 2019 16:13:08 +0100
> Dave Martin <Dave.Martin@arm.com> wrote:
> 
> > The SVE KVM support for arm64 includes the additional backend
> > header <asm/sve_context.h> from <asm/kvm.h>.
> > 
> > So update this header if it is available.
> > 
> > To avoid creating a sudden dependency on a specific minimum kernel
> > version, ignore the header if the source kernel tree doesn't have
> > it.
> > 
> > Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> > ---
> >  util/update_headers.sh | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/util/update_headers.sh b/util/update_headers.sh
> > index a7e21b8..90d3ead 100755
> > --- a/util/update_headers.sh
> > +++ b/util/update_headers.sh
> > @@ -25,11 +25,22 @@ fi
> >  
> >  cp -- "$LINUX_ROOT/include/uapi/linux/kvm.h" include/linux
> >  
> > +unset KVMTOOL_PATH
> > +
> > +copy_arm64 () {
> > +	local src=$LINUX_ROOT/arch/$arch/include/uapi/asm/sve_context.h
> 
> To go with your previous patches, aren't you missing the quotes here?

Hmmm, good question.  This is "obviously" a fancy variable assignment,
and so there would be no word splitting after expansion.  So quotes
wouldn't be needed here, just as with a simple assignment.  bash and
ash seem to work this way.

dash doesn't though, and a padantic reading of the bash man page
suggests that the dash behaviour may be more correct: i.e., local
is just a command, whose arguments are expanded in the usual way,
even if it happens to assign variables as part of its behaviour.

So, while I'm not sure whether or not quotes are officially needed here,
I guess we should have them to be on the safe side.

> > +
> > +	if [ -e "$src" ]
> > +	then
> > +		cp -- "$src" "$KVMTOOL_PATH/include/asm"
> > +	fi
> > +}
> > +
> 
> Maybe we can make this slightly more generic?
> copy_optional_arch() {
> 	local src="$LINUX_ROOT/arch/$arch/include/uapi/$1"
> 	[ -r "$src" ] && cp -- "$src" "$KVMTOOL_PATH/include/asm"
> }
> ...
> 	arm64) KVMTOOL_PATH=arm/aarch64
> 	       copy_optional_arch asm/sve_context.h
> 	       ;;

Happy to change it along those lines.  It's certainly possible this will
be needed again later for some future arch header.

Also, foo && bar exits the shell if foo yields false and set -e is in
effect, so I've reverted back to using an if.

(I'm still a little confused though, since I struggled to reproduce this
behaviour outside the script.)

Cheers
---Dave
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH kvmtool v3 4/9] update_headers: Sync kvm UAPI headers with linux v5.1-rc1

[Dave Martin]

Pull in upstream UAPI headers, for subsequent arm64 SVE / ptrauth
support (among other things).

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 arm/aarch64/include/asm/kvm.h         | 43 ++++++++++++++++++++++++++++
 arm/aarch64/include/asm/sve_context.h | 53 +++++++++++++++++++++++++++++++++++
 include/linux/kvm.h                   | 15 ++++++++--
 powerpc/include/asm/kvm.h             | 48 +++++++++++++++++++++++++++++++
 x86/include/asm/kvm.h                 |  1 +
 5 files changed, 158 insertions(+), 2 deletions(-)
 create mode 100644 arm/aarch64/include/asm/sve_context.h

diff --git a/arm/aarch64/include/asm/kvm.h b/arm/aarch64/include/asm/kvm.h
index 97c3478..7b7ac0f 100644
--- a/arm/aarch64/include/asm/kvm.h
+++ b/arm/aarch64/include/asm/kvm.h
@@ -35,6 +35,7 @@
 #include <linux/psci.h>
 #include <linux/types.h>
 #include <asm/ptrace.h>
+#include <asm/sve_context.h>
 
 #define __KVM_HAVE_GUEST_DEBUG
 #define __KVM_HAVE_IRQ_LINE
@@ -102,6 +103,9 @@ struct kvm_regs {
 #define KVM_ARM_VCPU_EL1_32BIT		1 /* CPU running a 32bit VM */
 #define KVM_ARM_VCPU_PSCI_0_2		2 /* CPU uses PSCI v0.2 */
 #define KVM_ARM_VCPU_PMU_V3		3 /* Support guest PMUv3 */
+#define KVM_ARM_VCPU_SVE		4 /* enable SVE for this CPU */
+#define KVM_ARM_VCPU_PTRAUTH_ADDRESS	5 /* VCPU uses address authentication */
+#define KVM_ARM_VCPU_PTRAUTH_GENERIC	6 /* VCPU uses generic authentication */
 
 struct kvm_vcpu_init {
 	__u32 target;
@@ -226,6 +230,45 @@ struct kvm_vcpu_events {
 					 KVM_REG_ARM_FW | ((r) & 0xffff))
 #define KVM_REG_ARM_PSCI_VERSION	KVM_REG_ARM_FW_REG(0)
 
+/* SVE registers */
+#define KVM_REG_ARM64_SVE		(0x15 << KVM_REG_ARM_COPROC_SHIFT)
+
+/* Z- and P-regs occupy blocks at the following offsets within this range: */
+#define KVM_REG_ARM64_SVE_ZREG_BASE	0
+#define KVM_REG_ARM64_SVE_PREG_BASE	0x400
+#define KVM_REG_ARM64_SVE_FFR_BASE	0x600
+
+#define KVM_ARM64_SVE_NUM_ZREGS		__SVE_NUM_ZREGS
+#define KVM_ARM64_SVE_NUM_PREGS		__SVE_NUM_PREGS
+
+#define KVM_ARM64_SVE_MAX_SLICES	32
+
+#define KVM_REG_ARM64_SVE_ZREG(n, i)					\
+	(KVM_REG_ARM64 | KVM_REG_ARM64_SVE | KVM_REG_ARM64_SVE_ZREG_BASE | \
+	 KVM_REG_SIZE_U2048 |						\
+	 (((n) & (KVM_ARM64_SVE_NUM_ZREGS - 1)) << 5) |			\
+	 ((i) & (KVM_ARM64_SVE_MAX_SLICES - 1)))
+
+#define KVM_REG_ARM64_SVE_PREG(n, i)					\
+	(KVM_REG_ARM64 | KVM_REG_ARM64_SVE | KVM_REG_ARM64_SVE_PREG_BASE | \
+	 KVM_REG_SIZE_U256 |						\
+	 (((n) & (KVM_ARM64_SVE_NUM_PREGS - 1)) << 5) |			\
+	 ((i) & (KVM_ARM64_SVE_MAX_SLICES - 1)))
+
+#define KVM_REG_ARM64_SVE_FFR(i)					\
+	(KVM_REG_ARM64 | KVM_REG_ARM64_SVE | KVM_REG_ARM64_SVE_FFR_BASE | \
+	 KVM_REG_SIZE_U256 |						\
+	 ((i) & (KVM_ARM64_SVE_MAX_SLICES - 1)))
+
+#define KVM_ARM64_SVE_VQ_MIN __SVE_VQ_MIN
+#define KVM_ARM64_SVE_VQ_MAX __SVE_VQ_MAX
+
+/* Vector lengths pseudo-register: */
+#define KVM_REG_ARM64_SVE_VLS		(KVM_REG_ARM64 | KVM_REG_ARM64_SVE | \
+					 KVM_REG_SIZE_U512 | 0xffff)
+#define KVM_ARM64_SVE_VLS_WORDS	\
+	((KVM_ARM64_SVE_VQ_MAX - KVM_ARM64_SVE_VQ_MIN) / 64 + 1)
+
 /* Device Control API: ARM VGIC */
 #define KVM_DEV_ARM_VGIC_GRP_ADDR	0
 #define KVM_DEV_ARM_VGIC_GRP_DIST_REGS	1
diff --git a/arm/aarch64/include/asm/sve_context.h b/arm/aarch64/include/asm/sve_context.h
new file mode 100644
index 0000000..754ab75
--- /dev/null
+++ b/arm/aarch64/include/asm/sve_context.h
@@ -0,0 +1,53 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/* Copyright (C) 2017-2018 ARM Limited */
+
+/*
+ * For use by other UAPI headers only.
+ * Do not make direct use of header or its definitions.
+ */
+
+#ifndef _UAPI__ASM_SVE_CONTEXT_H
+#define _UAPI__ASM_SVE_CONTEXT_H
+
+#include <linux/types.h>
+
+#define __SVE_VQ_BYTES		16	/* number of bytes per quadword */
+
+#define __SVE_VQ_MIN		1
+#define __SVE_VQ_MAX		512
+
+#define __SVE_VL_MIN		(__SVE_VQ_MIN * __SVE_VQ_BYTES)
+#define __SVE_VL_MAX		(__SVE_VQ_MAX * __SVE_VQ_BYTES)
+
+#define __SVE_NUM_ZREGS		32
+#define __SVE_NUM_PREGS		16
+
+#define __sve_vl_valid(vl)			\
+	((vl) % __SVE_VQ_BYTES == 0 &&		\
+	 (vl) >= __SVE_VL_MIN &&		\
+	 (vl) <= __SVE_VL_MAX)
+
+#define __sve_vq_from_vl(vl)	((vl) / __SVE_VQ_BYTES)
+#define __sve_vl_from_vq(vq)	((vq) * __SVE_VQ_BYTES)
+
+#define __SVE_ZREG_SIZE(vq)	((__u32)(vq) * __SVE_VQ_BYTES)
+#define __SVE_PREG_SIZE(vq)	((__u32)(vq) * (__SVE_VQ_BYTES / 8))
+#define __SVE_FFR_SIZE(vq)	__SVE_PREG_SIZE(vq)
+
+#define __SVE_ZREGS_OFFSET	0
+#define __SVE_ZREG_OFFSET(vq, n) \
+	(__SVE_ZREGS_OFFSET + __SVE_ZREG_SIZE(vq) * (n))
+#define __SVE_ZREGS_SIZE(vq) \
+	(__SVE_ZREG_OFFSET(vq, __SVE_NUM_ZREGS) - __SVE_ZREGS_OFFSET)
+
+#define __SVE_PREGS_OFFSET(vq) \
+	(__SVE_ZREGS_OFFSET + __SVE_ZREGS_SIZE(vq))
+#define __SVE_PREG_OFFSET(vq, n) \
+	(__SVE_PREGS_OFFSET(vq) + __SVE_PREG_SIZE(vq) * (n))
+#define __SVE_PREGS_SIZE(vq) \
+	(__SVE_PREG_OFFSET(vq, __SVE_NUM_PREGS) - __SVE_PREGS_OFFSET(vq))
+
+#define __SVE_FFR_OFFSET(vq) \
+	(__SVE_PREGS_OFFSET(vq) + __SVE_PREGS_SIZE(vq))
+
+#endif /* ! _UAPI__ASM_SVE_CONTEXT_H */
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index 6d4ea4b..2fe12b4 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -986,8 +986,13 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_HYPERV_ENLIGHTENED_VMCS 163
 #define KVM_CAP_EXCEPTION_PAYLOAD 164
 #define KVM_CAP_ARM_VM_IPA_SIZE 165
-#define KVM_CAP_MANUAL_DIRTY_LOG_PROTECT 166
+#define KVM_CAP_MANUAL_DIRTY_LOG_PROTECT 166 /* Obsolete */
 #define KVM_CAP_HYPERV_CPUID 167
+#define KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2 168
+#define KVM_CAP_PPC_IRQ_XIVE 169
+#define KVM_CAP_ARM_SVE 170
+#define KVM_CAP_ARM_PTRAUTH_ADDRESS 171
+#define KVM_CAP_ARM_PTRAUTH_GENERIC 172
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
@@ -1145,6 +1150,7 @@ struct kvm_dirty_tlb {
 #define KVM_REG_SIZE_U256	0x0050000000000000ULL
 #define KVM_REG_SIZE_U512	0x0060000000000000ULL
 #define KVM_REG_SIZE_U1024	0x0070000000000000ULL
+#define KVM_REG_SIZE_U2048	0x0080000000000000ULL
 
 struct kvm_reg_list {
 	__u64 n; /* number of regs */
@@ -1211,6 +1217,8 @@ enum kvm_device_type {
 #define KVM_DEV_TYPE_ARM_VGIC_V3	KVM_DEV_TYPE_ARM_VGIC_V3
 	KVM_DEV_TYPE_ARM_VGIC_ITS,
 #define KVM_DEV_TYPE_ARM_VGIC_ITS	KVM_DEV_TYPE_ARM_VGIC_ITS
+	KVM_DEV_TYPE_XIVE,
+#define KVM_DEV_TYPE_XIVE		KVM_DEV_TYPE_XIVE
 	KVM_DEV_TYPE_MAX,
 };
 
@@ -1434,12 +1442,15 @@ struct kvm_enc_region {
 #define KVM_GET_NESTED_STATE         _IOWR(KVMIO, 0xbe, struct kvm_nested_state)
 #define KVM_SET_NESTED_STATE         _IOW(KVMIO,  0xbf, struct kvm_nested_state)
 
-/* Available with KVM_CAP_MANUAL_DIRTY_LOG_PROTECT */
+/* Available with KVM_CAP_MANUAL_DIRTY_LOG_PROTECT_2 */
 #define KVM_CLEAR_DIRTY_LOG          _IOWR(KVMIO, 0xc0, struct kvm_clear_dirty_log)
 
 /* Available with KVM_CAP_HYPERV_CPUID */
 #define KVM_GET_SUPPORTED_HV_CPUID _IOWR(KVMIO, 0xc1, struct kvm_cpuid2)
 
+/* Available with KVM_CAP_ARM_SVE */
+#define KVM_ARM_VCPU_FINALIZE	  _IOW(KVMIO,  0xc2, int)
+
 /* Secure Encrypted Virtualization command */
 enum sev_cmd_id {
 	/* Guest initialization commands */
diff --git a/powerpc/include/asm/kvm.h b/powerpc/include/asm/kvm.h
index 8c876c1..b0f72de 100644
--- a/powerpc/include/asm/kvm.h
+++ b/powerpc/include/asm/kvm.h
@@ -463,10 +463,12 @@ struct kvm_ppc_cpu_char {
 #define KVM_PPC_CPU_CHAR_BR_HINT_HONOURED	(1ULL << 58)
 #define KVM_PPC_CPU_CHAR_MTTRIG_THR_RECONF	(1ULL << 57)
 #define KVM_PPC_CPU_CHAR_COUNT_CACHE_DIS	(1ULL << 56)
+#define KVM_PPC_CPU_CHAR_BCCTR_FLUSH_ASSIST	(1ull << 54)
 
 #define KVM_PPC_CPU_BEHAV_FAVOUR_SECURITY	(1ULL << 63)
 #define KVM_PPC_CPU_BEHAV_L1D_FLUSH_PR		(1ULL << 62)
 #define KVM_PPC_CPU_BEHAV_BNDS_CHK_SPEC_BAR	(1ULL << 61)
+#define KVM_PPC_CPU_BEHAV_FLUSH_COUNT_CACHE	(1ull << 58)
 
 /* Per-vcpu XICS interrupt controller state */
 #define KVM_REG_PPC_ICP_STATE	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x8c)
@@ -480,6 +482,8 @@ struct kvm_ppc_cpu_char {
 #define  KVM_REG_PPC_ICP_PPRI_SHIFT	16	/* pending irq priority */
 #define  KVM_REG_PPC_ICP_PPRI_MASK	0xff
 
+#define KVM_REG_PPC_VP_STATE	(KVM_REG_PPC | KVM_REG_SIZE_U128 | 0x8d)
+
 /* Device control API: PPC-specific devices */
 #define KVM_DEV_MPIC_GRP_MISC		1
 #define   KVM_DEV_MPIC_BASE_ADDR	0	/* 64-bit */
@@ -675,4 +679,48 @@ struct kvm_ppc_cpu_char {
 #define  KVM_XICS_PRESENTED		(1ULL << 43)
 #define  KVM_XICS_QUEUED		(1ULL << 44)
 
+/* POWER9 XIVE Native Interrupt Controller */
+#define KVM_DEV_XIVE_GRP_CTRL		1
+#define   KVM_DEV_XIVE_RESET		1
+#define   KVM_DEV_XIVE_EQ_SYNC		2
+#define KVM_DEV_XIVE_GRP_SOURCE		2	/* 64-bit source identifier */
+#define KVM_DEV_XIVE_GRP_SOURCE_CONFIG	3	/* 64-bit source identifier */
+#define KVM_DEV_XIVE_GRP_EQ_CONFIG	4	/* 64-bit EQ identifier */
+#define KVM_DEV_XIVE_GRP_SOURCE_SYNC	5       /* 64-bit source identifier */
+
+/* Layout of 64-bit XIVE source attribute values */
+#define KVM_XIVE_LEVEL_SENSITIVE	(1ULL << 0)
+#define KVM_XIVE_LEVEL_ASSERTED		(1ULL << 1)
+
+/* Layout of 64-bit XIVE source configuration attribute values */
+#define KVM_XIVE_SOURCE_PRIORITY_SHIFT	0
+#define KVM_XIVE_SOURCE_PRIORITY_MASK	0x7
+#define KVM_XIVE_SOURCE_SERVER_SHIFT	3
+#define KVM_XIVE_SOURCE_SERVER_MASK	0xfffffff8ULL
+#define KVM_XIVE_SOURCE_MASKED_SHIFT	32
+#define KVM_XIVE_SOURCE_MASKED_MASK	0x100000000ULL
+#define KVM_XIVE_SOURCE_EISN_SHIFT	33
+#define KVM_XIVE_SOURCE_EISN_MASK	0xfffffffe00000000ULL
+
+/* Layout of 64-bit EQ identifier */
+#define KVM_XIVE_EQ_PRIORITY_SHIFT	0
+#define KVM_XIVE_EQ_PRIORITY_MASK	0x7
+#define KVM_XIVE_EQ_SERVER_SHIFT	3
+#define KVM_XIVE_EQ_SERVER_MASK		0xfffffff8ULL
+
+/* Layout of EQ configuration values (64 bytes) */
+struct kvm_ppc_xive_eq {
+	__u32 flags;
+	__u32 qshift;
+	__u64 qaddr;
+	__u32 qtoggle;
+	__u32 qindex;
+	__u8  pad[40];
+};
+
+#define KVM_XIVE_EQ_ALWAYS_NOTIFY	0x00000001
+
+#define KVM_XIVE_TIMA_PAGE_OFFSET	0
+#define KVM_XIVE_ESB_PAGE_OFFSET	4
+
 #endif /* __LINUX_KVM_POWERPC_H */
diff --git a/x86/include/asm/kvm.h b/x86/include/asm/kvm.h
index dabfcf7..7a0e64c 100644
--- a/x86/include/asm/kvm.h
+++ b/x86/include/asm/kvm.h
@@ -381,6 +381,7 @@ struct kvm_sync_regs {
 #define KVM_X86_QUIRK_LINT0_REENABLED	(1 << 0)
 #define KVM_X86_QUIRK_CD_NW_CLEARED	(1 << 1)
 #define KVM_X86_QUIRK_LAPIC_MMIO_HOLE	(1 << 2)
+#define KVM_X86_QUIRK_OUT_7E_INC_RIP	(1 << 3)
 
 #define KVM_STATE_NESTED_GUEST_MODE	0x00000001
 #define KVM_STATE_NESTED_RUN_PENDING	0x00000002
-- 
2.1.4

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH kvmtool v3 4/9] update_headers: Sync kvm UAPI headers with linux v5.1-rc1

[Andre Przywara]

On Thu, 30 May 2019 16:13:09 +0100
Dave Martin <Dave.Martin@arm.com> wrote:

> Subject: [PATCH kvmtool v3 4/9] update_headers: Sync kvm UAPI headers with linux v5.1-rc1

This is actually v5.2-rc1, isn't it?

Apart from that:

> Pull in upstream UAPI headers, for subsequent arm64 SVE / ptrauth
> support (among other things).
> 
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre.

> ---
>  arm/aarch64/include/asm/kvm.h         | 43 ++++++++++++++++++++++++++++
>  arm/aarch64/include/asm/sve_context.h | 53 +++++++++++++++++++++++++++++++++++
>  include/linux/kvm.h                   | 15 ++++++++--
>  powerpc/include/asm/kvm.h             | 48 +++++++++++++++++++++++++++++++
>  x86/include/asm/kvm.h                 |  1 +
>  5 files changed, 158 insertions(+), 2 deletions(-)
>  create mode 100644 arm/aarch64/include/asm/sve_context.h
> 
> diff --git a/arm/aarch64/include/asm/kvm.h b/arm/aarch64/include/asm/kvm.h
> index 97c3478..7b7ac0f 100644
> --- a/arm/aarch64/include/asm/kvm.h
> +++ b/arm/aarch64/include/asm/kvm.h
> @@ -35,6 +35,7 @@
>  #include <linux/psci.h>
>  #include <linux/types.h>
>  #include <asm/ptrace.h>
> +#include <asm/sve_context.h>
>  
>  #define __KVM_HAVE_GUEST_DEBUG
>  #define __KVM_HAVE_IRQ_LINE
> @@ -102,6 +103,9 @@ struct kvm_regs {
>  #define KVM_ARM_VCPU_EL1_32BIT		1 /* CPU running a 32bit VM */
>  #define KVM_ARM_VCPU_PSCI_0_2		2 /* CPU uses PSCI v0.2 */
>  #define KVM_ARM_VCPU_PMU_V3		3 /* Support guest PMUv3 */
> +#define KVM_ARM_VCPU_SVE		4 /* enable SVE for this CPU */
> +#define KVM_ARM_VCPU_PTRAUTH_ADDRESS	5 /* VCPU uses address authentication */
> +#define KVM_ARM_VCPU_PTRAUTH_GENERIC	6 /* VCPU uses generic authentication */
>  
>  struct kvm_vcpu_init {
>  	__u32 target;
> @@ -226,6 +230,45 @@ struct kvm_vcpu_events {
>  					 KVM_REG_ARM_FW | ((r) & 0xffff))
>  #define KVM_REG_ARM_PSCI_VERSION	KVM_REG_ARM_FW_REG(0)
>  
> +/* SVE registers */
> +#define KVM_REG_ARM64_SVE		(0x15 << KVM_REG_ARM_COPROC_SHIFT)
> +
> +/* Z- and P-regs occupy blocks at the following offsets within this range: */
> +#define KVM_REG_ARM64_SVE_ZREG_BASE	0
> +#define KVM_REG_ARM64_SVE_PREG_BASE	0x400
> +#define KVM_REG_ARM64_SVE_FFR_BASE	0x600
> +
> +#define KVM_ARM64_SVE_NUM_ZREGS		__SVE_NUM_ZREGS
> +#define KVM_ARM64_SVE_NUM_PREGS		__SVE_NUM_PREGS
> +
> +#define KVM_ARM64_SVE_MAX_SLICES	32
> +
> +#define KVM_REG_ARM64_SVE_ZREG(n, i)					\
> +	(KVM_REG_ARM64 | KVM_REG_ARM64_SVE | KVM_REG_ARM64_SVE_ZREG_BASE | \
> +	 KVM_REG_SIZE_U2048 |						\
> +	 (((n) & (KVM_ARM64_SVE_NUM_ZREGS - 1)) << 5) |			\
> +	 ((i) & (KVM_ARM64_SVE_MAX_SLICES - 1)))
> +
> +#define KVM_REG_ARM64_SVE_PREG(n, i)					\
> +	(KVM_REG_ARM64 | KVM_REG_ARM64_SVE | KVM_REG_ARM64_SVE_PREG_BASE | \
> +	 KVM_REG_SIZE_U256 |						\
> +	 (((n) & (KVM_ARM64_SVE_NUM_PREGS - 1)) << 5) |			\
> +	 ((i) & (KVM_ARM64_SVE_MAX_SLICES - 1)))
> +
> +#define KVM_REG_ARM64_SVE_FFR(i)					\
> +	(KVM_REG_ARM64 | KVM_REG_ARM64_SVE | KVM_REG_ARM64_SVE_FFR_BASE | \
> +	 KVM_REG_SIZE_U256 |						\
> +	 ((i) & (KVM_ARM64_SVE_MAX_SLICES - 1)))
> +
> +#define KVM_ARM64_SVE_VQ_MIN __SVE_VQ_MIN
> +#define KVM_ARM64_SVE_VQ_MAX __SVE_VQ_MAX
> +
> +/* Vector lengths pseudo-register: */
> +#define KVM_REG_ARM64_SVE_VLS		(KVM_REG_ARM64 | KVM_REG_ARM64_SVE | \
> +					 KVM_REG_SIZE_U512 | 0xffff)
> +#define KVM_ARM64_SVE_VLS_WORDS	\
> +	((KVM_ARM64_SVE_VQ_MAX - KVM_ARM64_SVE_VQ_MIN) / 64 + 1)
> +
>  /* Device Control API: ARM VGIC */
>  #define KVM_DEV_ARM_VGIC_GRP_ADDR	0
>  #define KVM_DEV_ARM_VGIC_GRP_DIST_REGS	1
> diff --git a/arm/aarch64/include/asm/sve_context.h b/arm/aarch64/include/asm/sve_context.h
> new file mode 100644
> index 0000000..754ab75
> --- /dev/null
> +++ b/arm/aarch64/include/asm/sve_context.h
> @@ -0,0 +1,53 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/* Copyright (C) 2017-2018 ARM Limited */
> +
> +/*
> + * For use by other UAPI headers only.
> + * Do not make direct use of header or its definitions.
> + */
> +
> +#ifndef _UAPI__ASM_SVE_CONTEXT_H
> +#define _UAPI__ASM_SVE_CONTEXT_H
> +
> +#include <linux/types.h>
> +
> +#define __SVE_VQ_BYTES		16	/* number of bytes per quadword */
> +
> +#define __SVE_VQ_MIN		1
> +#define __SVE_VQ_MAX		512
> +
> +#define __SVE_VL_MIN		(__SVE_VQ_MIN * __SVE_VQ_BYTES)
> +#define __SVE_VL_MAX		(__SVE_VQ_MAX * __SVE_VQ_BYTES)
> +
> +#define __SVE_NUM_ZREGS		32
> +#define __SVE_NUM_PREGS		16
> +
> +#define __sve_vl_valid(vl)			\
> +	((vl) % __SVE_VQ_BYTES == 0 &&		\
> +	 (vl) >= __SVE_VL_MIN &&		\
> +	 (vl) <= __SVE_VL_MAX)
> +
> +#define __sve_vq_from_vl(vl)	((vl) / __SVE_VQ_BYTES)
> +#define __sve_vl_from_vq(vq)	((vq) * __SVE_VQ_BYTES)
> +
> +#define __SVE_ZREG_SIZE(vq)	((__u32)(vq) * __SVE_VQ_BYTES)
> +#define __SVE_PREG_SIZE(vq)	((__u32)(vq) * (__SVE_VQ_BYTES / 8))
> +#define __SVE_FFR_SIZE(vq)	__SVE_PREG_SIZE(vq)
> +
> +#define __SVE_ZREGS_OFFSET	0
> +#define __SVE_ZREG_OFFSET(vq, n) \
> +	(__SVE_ZREGS_OFFSET + __SVE_ZREG_SIZE(vq) * (n))
> +#define __SVE_ZREGS_SIZE(vq) \
> +	(__SVE_ZREG_OFFSET(vq, __SVE_NUM_ZREGS) - __SVE_ZREGS_OFFSET)
> +
> +#define __SVE_PREGS_OFFSET(vq) \
> +	(__SVE_ZREGS_OFFSET + __SVE_ZREGS_SIZE(vq))
> +#define __SVE_PREG_OFFSET(vq, n) \
> +	(__SVE_PREGS_OFFSET(vq) + __SVE_PREG_SIZE(vq) * (n))
> +#define __SVE_PREGS_SIZE(vq) \
> +	(__SVE_PREG_OFFSET(vq, __SVE_NUM_PREGS) - __SVE_PREGS_OFFSET(vq))
> +
> +#define __SVE_FFR_OFFSET(vq) \
> +	(__SVE_PREGS_OFFSET(vq) + __SVE_PREGS_SIZE(vq))
> +
> +#endif /* ! _UAPI__ASM_SVE_CONTEXT_H */
> diff --git a/include/linux/kvm.h b/include/linux/kvm.h
> index 6d4ea4b..2fe12b4 100644
> --- a/include/linux/kvm.h
> +++ b/include/linux/kvm.h
> @@ -986,8 +986,13 @@ struct kvm_ppc_resize_hpt {
>  #define KVM_CAP_HYPERV_ENLIGHTENED_VMCS 163
>  #define KVM_CAP_EXCEPTION_PAYLOAD 164
>  #define KVM_CAP_ARM_VM_IPA_SIZE 165
> -#define KVM_CAP_MANUAL_DIRTY_LOG_PROTECT 166
> +#define KVM_CAP_MANUAL_DIRTY_LOG_PROTECT 166 /* Obsolete */
>  #define KVM_CAP_HYPERV_CPUID 167
> +#define KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2 168
> +#define KVM_CAP_PPC_IRQ_XIVE 169
> +#define KVM_CAP_ARM_SVE 170
> +#define KVM_CAP_ARM_PTRAUTH_ADDRESS 171
> +#define KVM_CAP_ARM_PTRAUTH_GENERIC 172
>  
>  #ifdef KVM_CAP_IRQ_ROUTING
>  
> @@ -1145,6 +1150,7 @@ struct kvm_dirty_tlb {
>  #define KVM_REG_SIZE_U256	0x0050000000000000ULL
>  #define KVM_REG_SIZE_U512	0x0060000000000000ULL
>  #define KVM_REG_SIZE_U1024	0x0070000000000000ULL
> +#define KVM_REG_SIZE_U2048	0x0080000000000000ULL
>  
>  struct kvm_reg_list {
>  	__u64 n; /* number of regs */
> @@ -1211,6 +1217,8 @@ enum kvm_device_type {
>  #define KVM_DEV_TYPE_ARM_VGIC_V3	KVM_DEV_TYPE_ARM_VGIC_V3
>  	KVM_DEV_TYPE_ARM_VGIC_ITS,
>  #define KVM_DEV_TYPE_ARM_VGIC_ITS	KVM_DEV_TYPE_ARM_VGIC_ITS
> +	KVM_DEV_TYPE_XIVE,
> +#define KVM_DEV_TYPE_XIVE		KVM_DEV_TYPE_XIVE
>  	KVM_DEV_TYPE_MAX,
>  };
>  
> @@ -1434,12 +1442,15 @@ struct kvm_enc_region {
>  #define KVM_GET_NESTED_STATE         _IOWR(KVMIO, 0xbe, struct kvm_nested_state)
>  #define KVM_SET_NESTED_STATE         _IOW(KVMIO,  0xbf, struct kvm_nested_state)
>  
> -/* Available with KVM_CAP_MANUAL_DIRTY_LOG_PROTECT */
> +/* Available with KVM_CAP_MANUAL_DIRTY_LOG_PROTECT_2 */
>  #define KVM_CLEAR_DIRTY_LOG          _IOWR(KVMIO, 0xc0, struct kvm_clear_dirty_log)
>  
>  /* Available with KVM_CAP_HYPERV_CPUID */
>  #define KVM_GET_SUPPORTED_HV_CPUID _IOWR(KVMIO, 0xc1, struct kvm_cpuid2)
>  
> +/* Available with KVM_CAP_ARM_SVE */
> +#define KVM_ARM_VCPU_FINALIZE	  _IOW(KVMIO,  0xc2, int)
> +
>  /* Secure Encrypted Virtualization command */
>  enum sev_cmd_id {
>  	/* Guest initialization commands */
> diff --git a/powerpc/include/asm/kvm.h b/powerpc/include/asm/kvm.h
> index 8c876c1..b0f72de 100644
> --- a/powerpc/include/asm/kvm.h
> +++ b/powerpc/include/asm/kvm.h
> @@ -463,10 +463,12 @@ struct kvm_ppc_cpu_char {
>  #define KVM_PPC_CPU_CHAR_BR_HINT_HONOURED	(1ULL << 58)
>  #define KVM_PPC_CPU_CHAR_MTTRIG_THR_RECONF	(1ULL << 57)
>  #define KVM_PPC_CPU_CHAR_COUNT_CACHE_DIS	(1ULL << 56)
> +#define KVM_PPC_CPU_CHAR_BCCTR_FLUSH_ASSIST	(1ull << 54)
>  
>  #define KVM_PPC_CPU_BEHAV_FAVOUR_SECURITY	(1ULL << 63)
>  #define KVM_PPC_CPU_BEHAV_L1D_FLUSH_PR		(1ULL << 62)
>  #define KVM_PPC_CPU_BEHAV_BNDS_CHK_SPEC_BAR	(1ULL << 61)
> +#define KVM_PPC_CPU_BEHAV_FLUSH_COUNT_CACHE	(1ull << 58)
>  
>  /* Per-vcpu XICS interrupt controller state */
>  #define KVM_REG_PPC_ICP_STATE	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x8c)
> @@ -480,6 +482,8 @@ struct kvm_ppc_cpu_char {
>  #define  KVM_REG_PPC_ICP_PPRI_SHIFT	16	/* pending irq priority */
>  #define  KVM_REG_PPC_ICP_PPRI_MASK	0xff
>  
> +#define KVM_REG_PPC_VP_STATE	(KVM_REG_PPC | KVM_REG_SIZE_U128 | 0x8d)
> +
>  /* Device control API: PPC-specific devices */
>  #define KVM_DEV_MPIC_GRP_MISC		1
>  #define   KVM_DEV_MPIC_BASE_ADDR	0	/* 64-bit */
> @@ -675,4 +679,48 @@ struct kvm_ppc_cpu_char {
>  #define  KVM_XICS_PRESENTED		(1ULL << 43)
>  #define  KVM_XICS_QUEUED		(1ULL << 44)
>  
> +/* POWER9 XIVE Native Interrupt Controller */
> +#define KVM_DEV_XIVE_GRP_CTRL		1
> +#define   KVM_DEV_XIVE_RESET		1
> +#define   KVM_DEV_XIVE_EQ_SYNC		2
> +#define KVM_DEV_XIVE_GRP_SOURCE		2	/* 64-bit source identifier */
> +#define KVM_DEV_XIVE_GRP_SOURCE_CONFIG	3	/* 64-bit source identifier */
> +#define KVM_DEV_XIVE_GRP_EQ_CONFIG	4	/* 64-bit EQ identifier */
> +#define KVM_DEV_XIVE_GRP_SOURCE_SYNC	5       /* 64-bit source identifier */
> +
> +/* Layout of 64-bit XIVE source attribute values */
> +#define KVM_XIVE_LEVEL_SENSITIVE	(1ULL << 0)
> +#define KVM_XIVE_LEVEL_ASSERTED		(1ULL << 1)
> +
> +/* Layout of 64-bit XIVE source configuration attribute values */
> +#define KVM_XIVE_SOURCE_PRIORITY_SHIFT	0
> +#define KVM_XIVE_SOURCE_PRIORITY_MASK	0x7
> +#define KVM_XIVE_SOURCE_SERVER_SHIFT	3
> +#define KVM_XIVE_SOURCE_SERVER_MASK	0xfffffff8ULL
> +#define KVM_XIVE_SOURCE_MASKED_SHIFT	32
> +#define KVM_XIVE_SOURCE_MASKED_MASK	0x100000000ULL
> +#define KVM_XIVE_SOURCE_EISN_SHIFT	33
> +#define KVM_XIVE_SOURCE_EISN_MASK	0xfffffffe00000000ULL
> +
> +/* Layout of 64-bit EQ identifier */
> +#define KVM_XIVE_EQ_PRIORITY_SHIFT	0
> +#define KVM_XIVE_EQ_PRIORITY_MASK	0x7
> +#define KVM_XIVE_EQ_SERVER_SHIFT	3
> +#define KVM_XIVE_EQ_SERVER_MASK		0xfffffff8ULL
> +
> +/* Layout of EQ configuration values (64 bytes) */
> +struct kvm_ppc_xive_eq {
> +	__u32 flags;
> +	__u32 qshift;
> +	__u64 qaddr;
> +	__u32 qtoggle;
> +	__u32 qindex;
> +	__u8  pad[40];
> +};
> +
> +#define KVM_XIVE_EQ_ALWAYS_NOTIFY	0x00000001
> +
> +#define KVM_XIVE_TIMA_PAGE_OFFSET	0
> +#define KVM_XIVE_ESB_PAGE_OFFSET	4
> +
>  #endif /* __LINUX_KVM_POWERPC_H */
> diff --git a/x86/include/asm/kvm.h b/x86/include/asm/kvm.h
> index dabfcf7..7a0e64c 100644
> --- a/x86/include/asm/kvm.h
> +++ b/x86/include/asm/kvm.h
> @@ -381,6 +381,7 @@ struct kvm_sync_regs {
>  #define KVM_X86_QUIRK_LINT0_REENABLED	(1 << 0)
>  #define KVM_X86_QUIRK_CD_NW_CLEARED	(1 << 1)
>  #define KVM_X86_QUIRK_LAPIC_MMIO_HOLE	(1 << 2)
> +#define KVM_X86_QUIRK_OUT_7E_INC_RIP	(1 << 3)
>  
>  #define KVM_STATE_NESTED_GUEST_MODE	0x00000001
>  #define KVM_STATE_NESTED_RUN_PENDING	0x00000002

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH kvmtool v3 4/9] update_headers: Sync kvm UAPI headers with linux v5.1-rc1

[Dave Martin]

On Fri, May 31, 2019 at 06:03:19PM +0100, Andre Przywara wrote:
> On Thu, 30 May 2019 16:13:09 +0100
> Dave Martin <Dave.Martin@arm.com> wrote:
> 
> > Subject: [PATCH kvmtool v3 4/9] update_headers: Sync kvm UAPI headers with linux v5.1-rc1
> 
> This is actually v5.2-rc1, isn't it?

Doh.  Yes.  Amended.

> Apart from that:
> 
> > Pull in upstream UAPI headers, for subsequent arm64 SVE / ptrauth
> > support (among other things).
> > 
> > Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> 
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Thanks
---Dave
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH kvmtool v3 5/9] KVM: arm/arm64: Add a vcpu feature for pointer authentication

[Dave Martin]

From: Amit Daniel Kachhap <amit.kachhap@arm.com>

This patch adds a runtime capabality for KVM tool to enable Arm64 8.3
Pointer Authentication in guest kernel. Two vcpu features
KVM_ARM_VCPU_PTRAUTH_[ADDRESS/GENERIC] are supplied together to enable
Pointer Authentication in KVM guest after checking the capability.

Command line options --enable-ptrauth and --disable-ptrauth are added
to use this feature. However, if those options are not provided then
also this feature is enabled if host supports this capability.

The macros defined in the headers are not in sync and should be replaced
from the upstream.

Signed-off-by: Amit Daniel Kachhap <amit.kachhap@arm.com>
Signed-off-by: Dave Martin <Dave.Martin@arm.com> [merge new kernel heaers]
---
 arm/aarch32/include/kvm/kvm-cpu-arch.h    |  2 ++
 arm/aarch64/include/kvm/kvm-config-arch.h |  6 +++++-
 arm/aarch64/include/kvm/kvm-cpu-arch.h    |  3 +++
 arm/include/arm-common/kvm-config-arch.h  |  2 ++
 arm/kvm-cpu.c                             | 20 ++++++++++++++++++--
 5 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/arm/aarch32/include/kvm/kvm-cpu-arch.h b/arm/aarch32/include/kvm/kvm-cpu-arch.h
index d28ea67..3ec6f03 100644
--- a/arm/aarch32/include/kvm/kvm-cpu-arch.h
+++ b/arm/aarch32/include/kvm/kvm-cpu-arch.h
@@ -13,4 +13,6 @@
 #define ARM_CPU_ID		0, 0, 0
 #define ARM_CPU_ID_MPIDR	5
 
+#define ARM_VCPU_PTRAUTH_FEATURE	0
+
 #endif /* KVM__KVM_CPU_ARCH_H */
diff --git a/arm/aarch64/include/kvm/kvm-config-arch.h b/arm/aarch64/include/kvm/kvm-config-arch.h
index 04be43d..0279b13 100644
--- a/arm/aarch64/include/kvm/kvm-config-arch.h
+++ b/arm/aarch64/include/kvm/kvm-config-arch.h
@@ -8,7 +8,11 @@
 			"Create PMUv3 device"),				\
 	OPT_U64('\0', "kaslr-seed", &(cfg)->kaslr_seed,			\
 			"Specify random seed for Kernel Address Space "	\
-			"Layout Randomization (KASLR)"),
+			"Layout Randomization (KASLR)"),		\
+	OPT_BOOLEAN('\0', "enable-ptrauth", &(cfg)->enable_ptrauth,	\
+			"Enables pointer authentication"),		\
+	OPT_BOOLEAN('\0', "disable-ptrauth", &(cfg)->disable_ptrauth,	\
+			"Disables pointer authentication"),
 
 #include "arm-common/kvm-config-arch.h"
 
diff --git a/arm/aarch64/include/kvm/kvm-cpu-arch.h b/arm/aarch64/include/kvm/kvm-cpu-arch.h
index a9d8563..9fa99fb 100644
--- a/arm/aarch64/include/kvm/kvm-cpu-arch.h
+++ b/arm/aarch64/include/kvm/kvm-cpu-arch.h
@@ -17,4 +17,7 @@
 #define ARM_CPU_CTRL		3, 0, 1, 0
 #define ARM_CPU_CTRL_SCTLR_EL1	0
 
+#define ARM_VCPU_PTRAUTH_FEATURE	((1UL << KVM_ARM_VCPU_PTRAUTH_ADDRESS) \
+					| (1UL << KVM_ARM_VCPU_PTRAUTH_GENERIC))
+
 #endif /* KVM__KVM_CPU_ARCH_H */
diff --git a/arm/include/arm-common/kvm-config-arch.h b/arm/include/arm-common/kvm-config-arch.h
index 5734c46..1b4287d 100644
--- a/arm/include/arm-common/kvm-config-arch.h
+++ b/arm/include/arm-common/kvm-config-arch.h
@@ -10,6 +10,8 @@ struct kvm_config_arch {
 	bool		aarch32_guest;
 	bool		has_pmuv3;
 	u64		kaslr_seed;
+	bool		enable_ptrauth;
+	bool		disable_ptrauth;
 	enum irqchip_type irqchip;
 	u64		fw_addr;
 };
diff --git a/arm/kvm-cpu.c b/arm/kvm-cpu.c
index 7780251..acd1d5f 100644
--- a/arm/kvm-cpu.c
+++ b/arm/kvm-cpu.c
@@ -68,6 +68,18 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id)
 		vcpu_init.features[0] |= (1UL << KVM_ARM_VCPU_PSCI_0_2);
 	}
 
+	/* Check Pointer Authentication command line arguments. */
+	if (kvm->cfg.arch.enable_ptrauth && kvm->cfg.arch.disable_ptrauth)
+		die("Both enable-ptrauth and disable-ptrauth option cannot be present");
+	/*
+	 * Always enable Pointer Authentication if system supports
+	 * this extension unless disable-ptrauth option is present.
+	 */
+	if (kvm__supports_extension(kvm, KVM_CAP_ARM_PTRAUTH_ADDRESS) &&
+	    kvm__supports_extension(kvm, KVM_CAP_ARM_PTRAUTH_GENERIC) &&
+	    !kvm->cfg.arch.disable_ptrauth)
+			vcpu_init.features[0] |= ARM_VCPU_PTRAUTH_FEATURE;
+
 	/*
 	 * If the preferred target ioctl is successful then
 	 * use preferred target else try each and every target type
@@ -106,8 +118,12 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id)
 			die("Unable to find matching target");
 	}
 
-	if (err || target->init(vcpu))
-		die("Unable to initialise vcpu");
+	if (err || target->init(vcpu)) {
+		if (kvm->cfg.arch.enable_ptrauth)
+			die("Unable to initialise vcpu with pointer authentication feature");
+		else
+			die("Unable to initialise vcpu");
+	}
 
 	coalesced_offset = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION,
 				 KVM_CAP_COALESCED_MMIO);
-- 
2.1.4

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH kvmtool v3 5/9] KVM: arm/arm64: Add a vcpu feature for pointer authentication

[Andre Przywara]

On Thu, 30 May 2019 16:13:10 +0100
Dave Martin <Dave.Martin@arm.com> wrote:

> From: Amit Daniel Kachhap <amit.kachhap@arm.com>
> 
> This patch adds a runtime capabality for KVM tool to enable Arm64 8.3
> Pointer Authentication in guest kernel. Two vcpu features
> KVM_ARM_VCPU_PTRAUTH_[ADDRESS/GENERIC] are supplied together to enable
> Pointer Authentication in KVM guest after checking the capability.
> 
> Command line options --enable-ptrauth and --disable-ptrauth are added
> to use this feature. However, if those options are not provided then
> also this feature is enabled if host supports this capability.

I don't really get the purpose of two options, I think that's quite confusing. Should the first one either be dropped at all or called something with "force"?
I guess the idea is to fail if pointer auth isn't available, but the option is supplied?

Or maybe have one option with parameters?
--ptrauth[,=enable,=disable]

> The macros defined in the headers are not in sync and should be replaced
> from the upstream.

This is no longer true, I guess?

Cheers,
Andre.

> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@arm.com>
> Signed-off-by: Dave Martin <Dave.Martin@arm.com> [merge new kernel heaers]
> ---
>  arm/aarch32/include/kvm/kvm-cpu-arch.h    |  2 ++
>  arm/aarch64/include/kvm/kvm-config-arch.h |  6 +++++-
>  arm/aarch64/include/kvm/kvm-cpu-arch.h    |  3 +++
>  arm/include/arm-common/kvm-config-arch.h  |  2 ++
>  arm/kvm-cpu.c                             | 20 ++++++++++++++++++--
>  5 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/arm/aarch32/include/kvm/kvm-cpu-arch.h b/arm/aarch32/include/kvm/kvm-cpu-arch.h
> index d28ea67..3ec6f03 100644
> --- a/arm/aarch32/include/kvm/kvm-cpu-arch.h
> +++ b/arm/aarch32/include/kvm/kvm-cpu-arch.h
> @@ -13,4 +13,6 @@
>  #define ARM_CPU_ID		0, 0, 0
>  #define ARM_CPU_ID_MPIDR	5
>  
> +#define ARM_VCPU_PTRAUTH_FEATURE	0
> +
>  #endif /* KVM__KVM_CPU_ARCH_H */
> diff --git a/arm/aarch64/include/kvm/kvm-config-arch.h b/arm/aarch64/include/kvm/kvm-config-arch.h
> index 04be43d..0279b13 100644
> --- a/arm/aarch64/include/kvm/kvm-config-arch.h
> +++ b/arm/aarch64/include/kvm/kvm-config-arch.h
> @@ -8,7 +8,11 @@
>  			"Create PMUv3 device"),				\
>  	OPT_U64('\0', "kaslr-seed", &(cfg)->kaslr_seed,			\
>  			"Specify random seed for Kernel Address Space "	\
> -			"Layout Randomization (KASLR)"),
> +			"Layout Randomization (KASLR)"),		\
> +	OPT_BOOLEAN('\0', "enable-ptrauth", &(cfg)->enable_ptrauth,	\
> +			"Enables pointer authentication"),		\
> +	OPT_BOOLEAN('\0', "disable-ptrauth", &(cfg)->disable_ptrauth,	\
> +			"Disables pointer authentication"),
>  
>  #include "arm-common/kvm-config-arch.h"
>  
> diff --git a/arm/aarch64/include/kvm/kvm-cpu-arch.h b/arm/aarch64/include/kvm/kvm-cpu-arch.h
> index a9d8563..9fa99fb 100644
> --- a/arm/aarch64/include/kvm/kvm-cpu-arch.h
> +++ b/arm/aarch64/include/kvm/kvm-cpu-arch.h
> @@ -17,4 +17,7 @@
>  #define ARM_CPU_CTRL		3, 0, 1, 0
>  #define ARM_CPU_CTRL_SCTLR_EL1	0
>  
> +#define ARM_VCPU_PTRAUTH_FEATURE	((1UL << KVM_ARM_VCPU_PTRAUTH_ADDRESS) \
> +					| (1UL << KVM_ARM_VCPU_PTRAUTH_GENERIC))
> +
>  #endif /* KVM__KVM_CPU_ARCH_H */
> diff --git a/arm/include/arm-common/kvm-config-arch.h b/arm/include/arm-common/kvm-config-arch.h
> index 5734c46..1b4287d 100644
> --- a/arm/include/arm-common/kvm-config-arch.h
> +++ b/arm/include/arm-common/kvm-config-arch.h
> @@ -10,6 +10,8 @@ struct kvm_config_arch {
>  	bool		aarch32_guest;
>  	bool		has_pmuv3;
>  	u64		kaslr_seed;
> +	bool		enable_ptrauth;
> +	bool		disable_ptrauth;
>  	enum irqchip_type irqchip;
>  	u64		fw_addr;
>  };
> diff --git a/arm/kvm-cpu.c b/arm/kvm-cpu.c
> index 7780251..acd1d5f 100644
> --- a/arm/kvm-cpu.c
> +++ b/arm/kvm-cpu.c
> @@ -68,6 +68,18 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id)
>  		vcpu_init.features[0] |= (1UL << KVM_ARM_VCPU_PSCI_0_2);
>  	}
>  
> +	/* Check Pointer Authentication command line arguments. */
> +	if (kvm->cfg.arch.enable_ptrauth && kvm->cfg.arch.disable_ptrauth)
> +		die("Both enable-ptrauth and disable-ptrauth option cannot be present");
> +	/*
> +	 * Always enable Pointer Authentication if system supports
> +	 * this extension unless disable-ptrauth option is present.
> +	 */
> +	if (kvm__supports_extension(kvm, KVM_CAP_ARM_PTRAUTH_ADDRESS) &&
> +	    kvm__supports_extension(kvm, KVM_CAP_ARM_PTRAUTH_GENERIC) &&
> +	    !kvm->cfg.arch.disable_ptrauth)
> +			vcpu_init.features[0] |= ARM_VCPU_PTRAUTH_FEATURE;
> +
>  	/*
>  	 * If the preferred target ioctl is successful then
>  	 * use preferred target else try each and every target type
> @@ -106,8 +118,12 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id)
>  			die("Unable to find matching target");
>  	}
>  
> -	if (err || target->init(vcpu))
> -		die("Unable to initialise vcpu");
> +	if (err || target->init(vcpu)) {
> +		if (kvm->cfg.arch.enable_ptrauth)
> +			die("Unable to initialise vcpu with pointer authentication feature");
> +		else
> +			die("Unable to initialise vcpu");
> +	}
>  
>  	coalesced_offset = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION,
>  				 KVM_CAP_COALESCED_MMIO);

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH kvmtool v3 5/9] KVM: arm/arm64: Add a vcpu feature for pointer authentication

[Dave Martin]

On Fri, May 31, 2019 at 06:04:16PM +0100, Andre Przywara wrote:
> On Thu, 30 May 2019 16:13:10 +0100
> Dave Martin <Dave.Martin@arm.com> wrote:
> 
> > From: Amit Daniel Kachhap <amit.kachhap@arm.com>
> > 
> > This patch adds a runtime capabality for KVM tool to enable Arm64 8.3
> > Pointer Authentication in guest kernel. Two vcpu features
> > KVM_ARM_VCPU_PTRAUTH_[ADDRESS/GENERIC] are supplied together to enable
> > Pointer Authentication in KVM guest after checking the capability.
> > 
> > Command line options --enable-ptrauth and --disable-ptrauth are added
> > to use this feature. However, if those options are not provided then
> > also this feature is enabled if host supports this capability.
> 
> I don't really get the purpose of two options, I think that's quite
> confusing. Should the first one either be dropped at all or called
> something with "force"?
> 
> I guess the idea is to fail if pointer auth isn't available, but the
> option is supplied?
> 
> Or maybe have one option with parameters?
> --ptrauth[,=enable,=disable]

So, I was following two principles here, either or both of which may be
bogus:

1) There should be a way to determine whether KVM turns a given feature
on or off (instead of magically defaulting to something).

2) To a first approaximation, kvmtool should allow each major KVM ABI
feature to be exercised.

3) By default, kvmtool should offer the maximum feature set possible to
the guest.


(3) is well established, but (1) and (2) may be open to question?

If we hold to both principles, it makes sense to have options
functionally equivalent to what I suggested (where KVM provides the
control in the first place), but there may be more convenient ways
to respell the options.

If we really can't decide, maybe it's better to drop the options
altogether until we have a real use case.

I've found the options very useful for testing and debugging on the SVE
side, but I can't comment on ptrauth.  Maybe someone else has a view?

> > The macros defined in the headers are not in sync and should be replaced
> > from the upstream.
> 
> This is no longer true, I guess?

Ah yes, that comment can go.

Cheers
---Dave
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH kvmtool v3 5/9] KVM: arm/arm64: Add a vcpu feature for pointer authentication

[Andre Przywara]

On Mon, 3 Jun 2019 12:23:03 +0100
Dave Martin <Dave.Martin@arm.com> wrote:

Hi Dave,

> On Fri, May 31, 2019 at 06:04:16PM +0100, Andre Przywara wrote:
> > On Thu, 30 May 2019 16:13:10 +0100
> > Dave Martin <Dave.Martin@arm.com> wrote:
> >   
> > > From: Amit Daniel Kachhap <amit.kachhap@arm.com>
> > > 
> > > This patch adds a runtime capabality for KVM tool to enable Arm64 8.3
> > > Pointer Authentication in guest kernel. Two vcpu features
> > > KVM_ARM_VCPU_PTRAUTH_[ADDRESS/GENERIC] are supplied together to enable
> > > Pointer Authentication in KVM guest after checking the capability.
> > > 
> > > Command line options --enable-ptrauth and --disable-ptrauth are added
> > > to use this feature. However, if those options are not provided then
> > > also this feature is enabled if host supports this capability.  
> > 
> > I don't really get the purpose of two options, I think that's quite
> > confusing. Should the first one either be dropped at all or called
> > something with "force"?
> > 
> > I guess the idea is to fail if pointer auth isn't available, but the
> > option is supplied?
> > 
> > Or maybe have one option with parameters?
> > --ptrauth[,=enable,=disable]  
> 
> So, I was following two principles here, either or both of which may be
> bogus:
> 
> 1) There should be a way to determine whether KVM turns a given feature
> on or off (instead of magically defaulting to something).
> 
> 2) To a first approaximation, kvmtool should allow each major KVM ABI
> feature to be exercised.
> 
> 3) By default, kvmtool should offer the maximum feature set possible to
> the guest.
> 
> 
> (3) is well established, but (1) and (2) may be open to question?
> 
> If we hold to both principles, it makes sense to have options
> functionally equivalent to what I suggested (where KVM provides the
> control in the first place), but there may be more convenient ways
> to respell the options.
> 
> If we really can't decide, maybe it's better to drop the options
> altogether until we have a real use case.

In general I prefer the lack of a *need* for options over tuneability, but my concern is not so much exposing this knob, but more how it's done ...

> I've found the options very useful for testing and debugging on the SVE
> side, but I can't comment on ptrauth.  Maybe someone else has a view?

Given that kvmtool was designed as a hacker tool, I find it quite useful to play around with those setting. I just have my gripes with those enable/disable pair, which are two related, but actually separate options, both polluting the command line options space and also being confusing to the user.
I would be much happier if we would have one option per feature and a parameter: "--ptrauth={enable,disable}". Omitting the option altogether defaults to "enabled-if-available". Specifying it will force it on or off, accompanied by an error message if either(?) if not possible. This would also remove the need for the somewhat awkward "don't enable both" check.
It would also more easily allow a common parser, to be used by both ptrauth and SVE, for instance.
We could even introduce an explicit "default" parameter value, just in case people want to spell this case out.

What do you think about this?

Cheers,
Andre.
> 
> > > The macros defined in the headers are not in sync and should be replaced
> > > from the upstream.  
> > 
> > This is no longer true, I guess?  
> 
> Ah yes, that comment can go.
> 
> Cheers
> ---Dave

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH kvmtool v3 5/9] KVM: arm/arm64: Add a vcpu feature for pointer authentication

[Dave Martin]

On Mon, Jun 03, 2019 at 03:03:48PM +0100, Andre Przywara wrote:
> On Mon, 3 Jun 2019 12:23:03 +0100
> Dave Martin <Dave.Martin@arm.com> wrote:
> 
> Hi Dave,
> 
> > On Fri, May 31, 2019 at 06:04:16PM +0100, Andre Przywara wrote:
> > > On Thu, 30 May 2019 16:13:10 +0100
> > > Dave Martin <Dave.Martin@arm.com> wrote:
> > >   
> > > > From: Amit Daniel Kachhap <amit.kachhap@arm.com>
> > > > 
> > > > This patch adds a runtime capabality for KVM tool to enable Arm64 8.3
> > > > Pointer Authentication in guest kernel. Two vcpu features
> > > > KVM_ARM_VCPU_PTRAUTH_[ADDRESS/GENERIC] are supplied together to enable
> > > > Pointer Authentication in KVM guest after checking the capability.
> > > > 
> > > > Command line options --enable-ptrauth and --disable-ptrauth are added
> > > > to use this feature. However, if those options are not provided then
> > > > also this feature is enabled if host supports this capability.  
> > > 
> > > I don't really get the purpose of two options, I think that's quite
> > > confusing. Should the first one either be dropped at all or called
> > > something with "force"?
> > > 
> > > I guess the idea is to fail if pointer auth isn't available, but the
> > > option is supplied?
> > > 
> > > Or maybe have one option with parameters?
> > > --ptrauth[,=enable,=disable]  
> > 
> > So, I was following two principles here, either or both of which may be
> > bogus:
> > 
> > 1) There should be a way to determine whether KVM turns a given feature
> > on or off (instead of magically defaulting to something).
> > 
> > 2) To a first approaximation, kvmtool should allow each major KVM ABI
> > feature to be exercised.
> > 
> > 3) By default, kvmtool should offer the maximum feature set possible to
> > the guest.
> > 
> > 
> > (3) is well established, but (1) and (2) may be open to question?
> > 
> > If we hold to both principles, it makes sense to have options
> > functionally equivalent to what I suggested (where KVM provides the
> > control in the first place), but there may be more convenient ways
> > to respell the options.
> > 
> > If we really can't decide, maybe it's better to drop the options
> > altogether until we have a real use case.
> 
> In general I prefer the lack of a *need* for options over tuneability, but my concern is not so much exposing this knob, but more how it's done ...
> 
> > I've found the options very useful for testing and debugging on the SVE
> > side, but I can't comment on ptrauth.  Maybe someone else has a view?
> 
> Given that kvmtool was designed as a hacker tool, I find it quite useful to play around with those setting. I just have my gripes with those enable/disable pair, which are two related, but actually separate options, both polluting the command line options space and also being confusing to the user.
> I would be much happier if we would have one option per feature and a parameter: "--ptrauth={enable,disable}". Omitting the option altogether defaults to "enabled-if-available". Specifying it will force it on or off, accompanied by an error message if either(?) if not possible. This would also remove the need for the somewhat awkward "don't enable both" check.
> It would also more easily allow a common parser, to be used by both ptrauth and SVE, for instance.
> We could even introduce an explicit "default" parameter value, just in case people want to spell this case out.
> 
> What do you think about this?

Happy to do something like that, though it looks like the decision to
drop the options altogether may preempt that...

Cheers
---Dave
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH kvmtool v3 5/9] KVM: arm/arm64: Add a vcpu feature for pointer authentication

[Will Deacon]

On Mon, Jun 03, 2019 at 12:23:03PM +0100, Dave Martin wrote:
> On Fri, May 31, 2019 at 06:04:16PM +0100, Andre Przywara wrote:
> > On Thu, 30 May 2019 16:13:10 +0100
> > Dave Martin <Dave.Martin@arm.com> wrote:
> > 
> > > From: Amit Daniel Kachhap <amit.kachhap@arm.com>
> > > 
> > > This patch adds a runtime capabality for KVM tool to enable Arm64 8.3
> > > Pointer Authentication in guest kernel. Two vcpu features
> > > KVM_ARM_VCPU_PTRAUTH_[ADDRESS/GENERIC] are supplied together to enable
> > > Pointer Authentication in KVM guest after checking the capability.
> > > 
> > > Command line options --enable-ptrauth and --disable-ptrauth are added
> > > to use this feature. However, if those options are not provided then
> > > also this feature is enabled if host supports this capability.
> > 
> > I don't really get the purpose of two options, I think that's quite
> > confusing. Should the first one either be dropped at all or called
> > something with "force"?
> > 
> > I guess the idea is to fail if pointer auth isn't available, but the
> > option is supplied?
> > 
> > Or maybe have one option with parameters?
> > --ptrauth[,=enable,=disable]
> 
> So, I was following two principles here, either or both of which may be
> bogus:
> 
> 1) There should be a way to determine whether KVM turns a given feature
> on or off (instead of magically defaulting to something).
> 
> 2) To a first approaximation, kvmtool should allow each major KVM ABI
> feature to be exercised.
> 
> 3) By default, kvmtool should offer the maximum feature set possible to
> the guest.
> 
> 
> (3) is well established, but (1) and (2) may be open to question?
> 
> If we hold to both principles, it makes sense to have options
> functionally equivalent to what I suggested (where KVM provides the
> control in the first place), but there may be more convenient ways
> to respell the options.
> 
> If we really can't decide, maybe it's better to drop the options
> altogether until we have a real use case.
> 
> I've found the options very useful for testing and debugging on the SVE
> side, but I can't comment on ptrauth.  Maybe someone else has a view?

I'd prefer to drop them, to be honest. Whilst they may have been useful
during SVE development, it's not clear to me that they will continue to
be as useful now that things should be settling down. It's probably useful
to print out any features that we've explicitly enabled (or failed to
enable), but I'd stop there for the time being.

Will
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH kvmtool v3 5/9] KVM: arm/arm64: Add a vcpu feature for pointer authentication

[Dave Martin]

On Mon, Jun 03, 2019 at 03:07:06PM +0100, Will Deacon wrote:
> On Mon, Jun 03, 2019 at 12:23:03PM +0100, Dave Martin wrote:
> > On Fri, May 31, 2019 at 06:04:16PM +0100, Andre Przywara wrote:
> > > On Thu, 30 May 2019 16:13:10 +0100
> > > Dave Martin <Dave.Martin@arm.com> wrote:
> > > 
> > > > From: Amit Daniel Kachhap <amit.kachhap@arm.com>
> > > > 
> > > > This patch adds a runtime capabality for KVM tool to enable Arm64 8.3
> > > > Pointer Authentication in guest kernel. Two vcpu features
> > > > KVM_ARM_VCPU_PTRAUTH_[ADDRESS/GENERIC] are supplied together to enable
> > > > Pointer Authentication in KVM guest after checking the capability.
> > > > 
> > > > Command line options --enable-ptrauth and --disable-ptrauth are added
> > > > to use this feature. However, if those options are not provided then
> > > > also this feature is enabled if host supports this capability.
> > > 
> > > I don't really get the purpose of two options, I think that's quite
> > > confusing. Should the first one either be dropped at all or called
> > > something with "force"?
> > > 
> > > I guess the idea is to fail if pointer auth isn't available, but the
> > > option is supplied?
> > > 
> > > Or maybe have one option with parameters?
> > > --ptrauth[,=enable,=disable]
> > 
> > So, I was following two principles here, either or both of which may be
> > bogus:
> > 
> > 1) There should be a way to determine whether KVM turns a given feature
> > on or off (instead of magically defaulting to something).
> > 
> > 2) To a first approaximation, kvmtool should allow each major KVM ABI
> > feature to be exercised.
> > 
> > 3) By default, kvmtool should offer the maximum feature set possible to
> > the guest.
> > 
> > 
> > (3) is well established, but (1) and (2) may be open to question?
> > 
> > If we hold to both principles, it makes sense to have options
> > functionally equivalent to what I suggested (where KVM provides the
> > control in the first place), but there may be more convenient ways
> > to respell the options.
> > 
> > If we really can't decide, maybe it's better to drop the options
> > altogether until we have a real use case.
> > 
> > I've found the options very useful for testing and debugging on the SVE
> > side, but I can't comment on ptrauth.  Maybe someone else has a view?
> 
> I'd prefer to drop them, to be honest. Whilst they may have been useful
> during SVE development, it's not clear to me that they will continue to
> be as useful now that things should be settling down. It's probably useful
> to print out any features that we've explicitly enabled (or failed to
> enable), but I'd stop there for the time being.

I don't have a strong view on this.

I'm happy to respin dropping the command line options and defaulting
everthing to on: for hacking purposes, it's easy to keep a local branch.

Cheers
---Dave
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH kvmtool v3 5/9] KVM: arm/arm64: Add a vcpu feature for pointer authentication

[Dave Martin]

@Peter, do you have an opinion on this (below) ?

On Thu, May 30, 2019 at 04:13:10PM +0100, Dave Martin wrote:
> From: Amit Daniel Kachhap <amit.kachhap@arm.com>
> 
> This patch adds a runtime capabality for KVM tool to enable Arm64 8.3
> Pointer Authentication in guest kernel. Two vcpu features
> KVM_ARM_VCPU_PTRAUTH_[ADDRESS/GENERIC] are supplied together to enable
> Pointer Authentication in KVM guest after checking the capability.
> 
> Command line options --enable-ptrauth and --disable-ptrauth are added
> to use this feature. However, if those options are not provided then
> also this feature is enabled if host supports this capability.
> 
> The macros defined in the headers are not in sync and should be replaced
> from the upstream.
> 
> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@arm.com>
> Signed-off-by: Dave Martin <Dave.Martin@arm.com> [merge new kernel heaers]
> ---
>  arm/aarch32/include/kvm/kvm-cpu-arch.h    |  2 ++
>  arm/aarch64/include/kvm/kvm-config-arch.h |  6 +++++-
>  arm/aarch64/include/kvm/kvm-cpu-arch.h    |  3 +++
>  arm/include/arm-common/kvm-config-arch.h  |  2 ++
>  arm/kvm-cpu.c                             | 20 ++++++++++++++++++--
>  5 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/arm/aarch32/include/kvm/kvm-cpu-arch.h b/arm/aarch32/include/kvm/kvm-cpu-arch.h
> index d28ea67..3ec6f03 100644
> --- a/arm/aarch32/include/kvm/kvm-cpu-arch.h
> +++ b/arm/aarch32/include/kvm/kvm-cpu-arch.h
> @@ -13,4 +13,6 @@
>  #define ARM_CPU_ID		0, 0, 0
>  #define ARM_CPU_ID_MPIDR	5
>  
> +#define ARM_VCPU_PTRAUTH_FEATURE	0
> +
>  #endif /* KVM__KVM_CPU_ARCH_H */
> diff --git a/arm/aarch64/include/kvm/kvm-config-arch.h b/arm/aarch64/include/kvm/kvm-config-arch.h
> index 04be43d..0279b13 100644
> --- a/arm/aarch64/include/kvm/kvm-config-arch.h
> +++ b/arm/aarch64/include/kvm/kvm-config-arch.h
> @@ -8,7 +8,11 @@
>  			"Create PMUv3 device"),				\
>  	OPT_U64('\0', "kaslr-seed", &(cfg)->kaslr_seed,			\
>  			"Specify random seed for Kernel Address Space "	\
> -			"Layout Randomization (KASLR)"),
> +			"Layout Randomization (KASLR)"),		\
> +	OPT_BOOLEAN('\0', "enable-ptrauth", &(cfg)->enable_ptrauth,	\
> +			"Enables pointer authentication"),		\
> +	OPT_BOOLEAN('\0', "disable-ptrauth", &(cfg)->disable_ptrauth,	\
> +			"Disables pointer authentication"),
>  
>  #include "arm-common/kvm-config-arch.h"
>  
> diff --git a/arm/aarch64/include/kvm/kvm-cpu-arch.h b/arm/aarch64/include/kvm/kvm-cpu-arch.h
> index a9d8563..9fa99fb 100644
> --- a/arm/aarch64/include/kvm/kvm-cpu-arch.h
> +++ b/arm/aarch64/include/kvm/kvm-cpu-arch.h
> @@ -17,4 +17,7 @@
>  #define ARM_CPU_CTRL		3, 0, 1, 0
>  #define ARM_CPU_CTRL_SCTLR_EL1	0
>  
> +#define ARM_VCPU_PTRAUTH_FEATURE	((1UL << KVM_ARM_VCPU_PTRAUTH_ADDRESS) \
> +					| (1UL << KVM_ARM_VCPU_PTRAUTH_GENERIC))
> +
>  #endif /* KVM__KVM_CPU_ARCH_H */
> diff --git a/arm/include/arm-common/kvm-config-arch.h b/arm/include/arm-common/kvm-config-arch.h
> index 5734c46..1b4287d 100644
> --- a/arm/include/arm-common/kvm-config-arch.h
> +++ b/arm/include/arm-common/kvm-config-arch.h
> @@ -10,6 +10,8 @@ struct kvm_config_arch {
>  	bool		aarch32_guest;
>  	bool		has_pmuv3;
>  	u64		kaslr_seed;
> +	bool		enable_ptrauth;
> +	bool		disable_ptrauth;
>  	enum irqchip_type irqchip;
>  	u64		fw_addr;
>  };
> diff --git a/arm/kvm-cpu.c b/arm/kvm-cpu.c
> index 7780251..acd1d5f 100644
> --- a/arm/kvm-cpu.c
> +++ b/arm/kvm-cpu.c
> @@ -68,6 +68,18 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id)
>  		vcpu_init.features[0] |= (1UL << KVM_ARM_VCPU_PSCI_0_2);
>  	}
>  
> +	/* Check Pointer Authentication command line arguments. */
> +	if (kvm->cfg.arch.enable_ptrauth && kvm->cfg.arch.disable_ptrauth)
> +		die("Both enable-ptrauth and disable-ptrauth option cannot be present");
> +	/*
> +	 * Always enable Pointer Authentication if system supports
> +	 * this extension unless disable-ptrauth option is present.
> +	 */
> +	if (kvm__supports_extension(kvm, KVM_CAP_ARM_PTRAUTH_ADDRESS) &&
> +	    kvm__supports_extension(kvm, KVM_CAP_ARM_PTRAUTH_GENERIC) &&
> +	    !kvm->cfg.arch.disable_ptrauth)
> +			vcpu_init.features[0] |= ARM_VCPU_PTRAUTH_FEATURE;
> +

Does anyone recall why we need these cap and feature flags for ptrauth
at all?

We have a window before v5.2 where we could remove them, but we need to
get a move on if so...


As I understand it, the main concern here was to support migrations
between nodes that have the same hardware but are running different
kernel versions, thus allowing a new kernel to be deployed across a
cluster without having to stop the world.

Thus, a guest created on an old kernel where KVM doesn's support
ptrauth wouldn't detect the caps and wouldn't set those VCPU_INIT
feature bits: the VCPU_INIT feature set would be recorded in the guest
metadata by userspace and used for creating vcpus on the new node when
migrating, forcing ptrauth still to be hidden from the guest even
if the new node's kernel supports ptrauth for KVM.  Fine.

However, this is equally a problem for other random CPU features, and
we don't handle those at all for now: any feature that the new kernel
supports and is present in the hardware will result in changed CPUID
registers exposed to the guest and thus migration failures.  This
applies even to features that require no KVM supervision whatever.

So migrating between arbitrary kernel versions doesn't work today.
For that, we'd need a way for the CPUID sysregs at the destination
node to values different that the default: we have no logic for that
today.

What problem(s) are we actually trying to solve here?

Do the ptrauth caps and feature flags actually solve anything?


(Note, the SVE case is different: there, the cap and VCPU_INIT flag are
there to work around an ABI break, so that old userspace doesn't see
impossibly-large registers through KVM_GET_ONE_REG etc.)

[...]

Cheers
---Dave
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH kvmtool v3 6/9] arm/arm64: Factor out ptrauth vcpu feature setup

[Dave Martin]

In the interest of readability, factor out the vcpu feature setup
for ptrauth into a separate function.

Also, because aarch32 doesn't have this feature or the related
command line options anyway, move the actual code into aarch64/.

Since ARM_VCPU_PTRAUTH_FEATURE is only there to make the ptrauth
feature setup code compile on arm, it is no longer needed: inline
and remove it.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 arm/aarch32/include/kvm/kvm-cpu-arch.h |  3 ++-
 arm/aarch64/include/kvm/kvm-cpu-arch.h |  3 +--
 arm/aarch64/kvm-cpu.c                  | 22 ++++++++++++++++++++++
 arm/kvm-cpu.c                          | 12 +-----------
 4 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/arm/aarch32/include/kvm/kvm-cpu-arch.h b/arm/aarch32/include/kvm/kvm-cpu-arch.h
index 3ec6f03..01983f0 100644
--- a/arm/aarch32/include/kvm/kvm-cpu-arch.h
+++ b/arm/aarch32/include/kvm/kvm-cpu-arch.h
@@ -13,6 +13,7 @@
 #define ARM_CPU_ID		0, 0, 0
 #define ARM_CPU_ID_MPIDR	5
 
-#define ARM_VCPU_PTRAUTH_FEATURE	0
+static inline void kvm_cpu__select_features(struct kvm *kvm,
+					    struct kvm_vcpu_init *init) { }
 
 #endif /* KVM__KVM_CPU_ARCH_H */
diff --git a/arm/aarch64/include/kvm/kvm-cpu-arch.h b/arm/aarch64/include/kvm/kvm-cpu-arch.h
index 9fa99fb..e6875fc 100644
--- a/arm/aarch64/include/kvm/kvm-cpu-arch.h
+++ b/arm/aarch64/include/kvm/kvm-cpu-arch.h
@@ -17,7 +17,6 @@
 #define ARM_CPU_CTRL		3, 0, 1, 0
 #define ARM_CPU_CTRL_SCTLR_EL1	0
 
-#define ARM_VCPU_PTRAUTH_FEATURE	((1UL << KVM_ARM_VCPU_PTRAUTH_ADDRESS) \
-					| (1UL << KVM_ARM_VCPU_PTRAUTH_GENERIC))
+void kvm_cpu__select_features(struct kvm *kvm, struct kvm_vcpu_init *init);
 
 #endif /* KVM__KVM_CPU_ARCH_H */
diff --git a/arm/aarch64/kvm-cpu.c b/arm/aarch64/kvm-cpu.c
index 0aaefaf..d3c32e0 100644
--- a/arm/aarch64/kvm-cpu.c
+++ b/arm/aarch64/kvm-cpu.c
@@ -128,6 +128,28 @@ static void reset_vcpu_aarch64(struct kvm_cpu *vcpu)
 	}
 }
 
+static void select_ptrauth_feature(struct kvm *kvm, struct kvm_vcpu_init *init)
+{
+	/* Check Pointer Authentication command line arguments. */
+	if (kvm->cfg.arch.enable_ptrauth && kvm->cfg.arch.disable_ptrauth)
+		die("Both enable-ptrauth and disable-ptrauth option cannot be present");
+	/*
+	 * Always enable Pointer Authentication if system supports
+	 * this extension unless disable-ptrauth option is present.
+	 */
+	if (kvm__supports_extension(kvm, KVM_CAP_ARM_PTRAUTH_ADDRESS) &&
+	    kvm__supports_extension(kvm, KVM_CAP_ARM_PTRAUTH_GENERIC) &&
+	    !kvm->cfg.arch.disable_ptrauth) {
+		init->features[0] |= 1UL << KVM_ARM_VCPU_PTRAUTH_ADDRESS;
+		init->features[0] |= 1UL << KVM_ARM_VCPU_PTRAUTH_GENERIC;
+	}
+}
+
+void kvm_cpu__select_features(struct kvm *kvm, struct kvm_vcpu_init *init)
+{
+	select_ptrauth_feature(kvm, init);
+}
+
 void kvm_cpu__reset_vcpu(struct kvm_cpu *vcpu)
 {
 	if (vcpu->kvm->cfg.arch.aarch32_guest)
diff --git a/arm/kvm-cpu.c b/arm/kvm-cpu.c
index acd1d5f..764fb05 100644
--- a/arm/kvm-cpu.c
+++ b/arm/kvm-cpu.c
@@ -68,17 +68,7 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id)
 		vcpu_init.features[0] |= (1UL << KVM_ARM_VCPU_PSCI_0_2);
 	}
 
-	/* Check Pointer Authentication command line arguments. */
-	if (kvm->cfg.arch.enable_ptrauth && kvm->cfg.arch.disable_ptrauth)
-		die("Both enable-ptrauth and disable-ptrauth option cannot be present");
-	/*
-	 * Always enable Pointer Authentication if system supports
-	 * this extension unless disable-ptrauth option is present.
-	 */
-	if (kvm__supports_extension(kvm, KVM_CAP_ARM_PTRAUTH_ADDRESS) &&
-	    kvm__supports_extension(kvm, KVM_CAP_ARM_PTRAUTH_GENERIC) &&
-	    !kvm->cfg.arch.disable_ptrauth)
-			vcpu_init.features[0] |= ARM_VCPU_PTRAUTH_FEATURE;
+	kvm_cpu__select_features(kvm, &vcpu_init);
 
 	/*
 	 * If the preferred target ioctl is successful then
-- 
2.1.4

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH kvmtool v3 6/9] arm/arm64: Factor out ptrauth vcpu feature setup

[Andre Przywara]

On Thu, 30 May 2019 16:13:11 +0100
Dave Martin <Dave.Martin@arm.com> wrote:

> In the interest of readability, factor out the vcpu feature setup
> for ptrauth into a separate function.
> 
> Also, because aarch32 doesn't have this feature or the related
> command line options anyway, move the actual code into aarch64/.
> 
> Since ARM_VCPU_PTRAUTH_FEATURE is only there to make the ptrauth
> feature setup code compile on arm, it is no longer needed: inline
> and remove it.

I am not sure this is useful as a separate patch, so can we just merge this into 5/9?

Cheers,
Andre.

> 
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> ---
>  arm/aarch32/include/kvm/kvm-cpu-arch.h |  3 ++-
>  arm/aarch64/include/kvm/kvm-cpu-arch.h |  3 +--
>  arm/aarch64/kvm-cpu.c                  | 22 ++++++++++++++++++++++
>  arm/kvm-cpu.c                          | 12 +-----------
>  4 files changed, 26 insertions(+), 14 deletions(-)
> 
> diff --git a/arm/aarch32/include/kvm/kvm-cpu-arch.h b/arm/aarch32/include/kvm/kvm-cpu-arch.h
> index 3ec6f03..01983f0 100644
> --- a/arm/aarch32/include/kvm/kvm-cpu-arch.h
> +++ b/arm/aarch32/include/kvm/kvm-cpu-arch.h
> @@ -13,6 +13,7 @@
>  #define ARM_CPU_ID		0, 0, 0
>  #define ARM_CPU_ID_MPIDR	5
>  
> -#define ARM_VCPU_PTRAUTH_FEATURE	0
> +static inline void kvm_cpu__select_features(struct kvm *kvm,
> +					    struct kvm_vcpu_init *init) { }
>  
>  #endif /* KVM__KVM_CPU_ARCH_H */
> diff --git a/arm/aarch64/include/kvm/kvm-cpu-arch.h b/arm/aarch64/include/kvm/kvm-cpu-arch.h
> index 9fa99fb..e6875fc 100644
> --- a/arm/aarch64/include/kvm/kvm-cpu-arch.h
> +++ b/arm/aarch64/include/kvm/kvm-cpu-arch.h
> @@ -17,7 +17,6 @@
>  #define ARM_CPU_CTRL		3, 0, 1, 0
>  #define ARM_CPU_CTRL_SCTLR_EL1	0
>  
> -#define ARM_VCPU_PTRAUTH_FEATURE	((1UL << KVM_ARM_VCPU_PTRAUTH_ADDRESS) \
> -					| (1UL << KVM_ARM_VCPU_PTRAUTH_GENERIC))
> +void kvm_cpu__select_features(struct kvm *kvm, struct kvm_vcpu_init *init);
>  
>  #endif /* KVM__KVM_CPU_ARCH_H */
> diff --git a/arm/aarch64/kvm-cpu.c b/arm/aarch64/kvm-cpu.c
> index 0aaefaf..d3c32e0 100644
> --- a/arm/aarch64/kvm-cpu.c
> +++ b/arm/aarch64/kvm-cpu.c
> @@ -128,6 +128,28 @@ static void reset_vcpu_aarch64(struct kvm_cpu *vcpu)
>  	}
>  }
>  
> +static void select_ptrauth_feature(struct kvm *kvm, struct kvm_vcpu_init *init)
> +{
> +	/* Check Pointer Authentication command line arguments. */
> +	if (kvm->cfg.arch.enable_ptrauth && kvm->cfg.arch.disable_ptrauth)
> +		die("Both enable-ptrauth and disable-ptrauth option cannot be present");
> +	/*
> +	 * Always enable Pointer Authentication if system supports
> +	 * this extension unless disable-ptrauth option is present.
> +	 */
> +	if (kvm__supports_extension(kvm, KVM_CAP_ARM_PTRAUTH_ADDRESS) &&
> +	    kvm__supports_extension(kvm, KVM_CAP_ARM_PTRAUTH_GENERIC) &&
> +	    !kvm->cfg.arch.disable_ptrauth) {
> +		init->features[0] |= 1UL << KVM_ARM_VCPU_PTRAUTH_ADDRESS;
> +		init->features[0] |= 1UL << KVM_ARM_VCPU_PTRAUTH_GENERIC;
> +	}
> +}
> +
> +void kvm_cpu__select_features(struct kvm *kvm, struct kvm_vcpu_init *init)
> +{
> +	select_ptrauth_feature(kvm, init);
> +}
> +
>  void kvm_cpu__reset_vcpu(struct kvm_cpu *vcpu)
>  {
>  	if (vcpu->kvm->cfg.arch.aarch32_guest)
> diff --git a/arm/kvm-cpu.c b/arm/kvm-cpu.c
> index acd1d5f..764fb05 100644
> --- a/arm/kvm-cpu.c
> +++ b/arm/kvm-cpu.c
> @@ -68,17 +68,7 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id)
>  		vcpu_init.features[0] |= (1UL << KVM_ARM_VCPU_PSCI_0_2);
>  	}
>  
> -	/* Check Pointer Authentication command line arguments. */
> -	if (kvm->cfg.arch.enable_ptrauth && kvm->cfg.arch.disable_ptrauth)
> -		die("Both enable-ptrauth and disable-ptrauth option cannot be present");
> -	/*
> -	 * Always enable Pointer Authentication if system supports
> -	 * this extension unless disable-ptrauth option is present.
> -	 */
> -	if (kvm__supports_extension(kvm, KVM_CAP_ARM_PTRAUTH_ADDRESS) &&
> -	    kvm__supports_extension(kvm, KVM_CAP_ARM_PTRAUTH_GENERIC) &&
> -	    !kvm->cfg.arch.disable_ptrauth)
> -			vcpu_init.features[0] |= ARM_VCPU_PTRAUTH_FEATURE;
> +	kvm_cpu__select_features(kvm, &vcpu_init);
>  
>  	/*
>  	 * If the preferred target ioctl is successful then

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH kvmtool v3 6/9] arm/arm64: Factor out ptrauth vcpu feature setup

[Dave Martin]

On Fri, May 31, 2019 at 06:04:36PM +0100, Andre Przywara wrote:
> On Thu, 30 May 2019 16:13:11 +0100
> Dave Martin <Dave.Martin@arm.com> wrote:
> 
> > In the interest of readability, factor out the vcpu feature setup
> > for ptrauth into a separate function.
> > 
> > Also, because aarch32 doesn't have this feature or the related
> > command line options anyway, move the actual code into aarch64/.
> > 
> > Since ARM_VCPU_PTRAUTH_FEATURE is only there to make the ptrauth
> > feature setup code compile on arm, it is no longer needed: inline
> > and remove it.
> 
> I am not sure this is useful as a separate patch, so can we just merge
> this into 5/9?

Could be.  I wanted to keep the changes against Amit's original patch
clear for now, so it's easier for him to review.

Cheers
---Dave
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH kvmtool v3 7/9] arm64: Make ptrauth enable/disable diagnostics more user-friendly

[Dave Martin]

To help the user understand what is going on, amend ptrauth
configuration diagnostic messages to refer to command line options
by the exact name used on the command line.

Also, provide a clean diagnostic when ptrauth is requested, but not
availble.  The generic "Unable to initialise vcpu" message is
rather cryptic for this case.

Since we now don't attempt to enable ptrauth at all unless KVM
reports the relevant capabilities, remove the error message for
that case too: in any case, we can't diagnose precisely why
KVM_ARM_VCPU_INIT failed, so the message may be misleading.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 arm/aarch64/include/kvm/kvm-config-arch.h |  4 ++--
 arm/aarch64/kvm-cpu.c                     | 15 +++++++++++----
 arm/kvm-cpu.c                             |  8 ++------
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/arm/aarch64/include/kvm/kvm-config-arch.h b/arm/aarch64/include/kvm/kvm-config-arch.h
index 0279b13..fe1699d 100644
--- a/arm/aarch64/include/kvm/kvm-config-arch.h
+++ b/arm/aarch64/include/kvm/kvm-config-arch.h
@@ -10,9 +10,9 @@
 			"Specify random seed for Kernel Address Space "	\
 			"Layout Randomization (KASLR)"),		\
 	OPT_BOOLEAN('\0', "enable-ptrauth", &(cfg)->enable_ptrauth,	\
-			"Enables pointer authentication"),		\
+			"Enable pointer authentication for the guest"),	\
 	OPT_BOOLEAN('\0', "disable-ptrauth", &(cfg)->disable_ptrauth,	\
-			"Disables pointer authentication"),
+			"Disable pointer authentication for the guest"),
 
 #include "arm-common/kvm-config-arch.h"
 
diff --git a/arm/aarch64/kvm-cpu.c b/arm/aarch64/kvm-cpu.c
index d3c32e0..08e4fd5 100644
--- a/arm/aarch64/kvm-cpu.c
+++ b/arm/aarch64/kvm-cpu.c
@@ -130,16 +130,23 @@ static void reset_vcpu_aarch64(struct kvm_cpu *vcpu)
 
 static void select_ptrauth_feature(struct kvm *kvm, struct kvm_vcpu_init *init)
 {
+	bool supported;
+
 	/* Check Pointer Authentication command line arguments. */
 	if (kvm->cfg.arch.enable_ptrauth && kvm->cfg.arch.disable_ptrauth)
-		die("Both enable-ptrauth and disable-ptrauth option cannot be present");
+		die("--enable-ptrauth conflicts with --disable-ptrauth");
+
+	supported = kvm__supports_extension(kvm, KVM_CAP_ARM_PTRAUTH_ADDRESS) &&
+		    kvm__supports_extension(kvm, KVM_CAP_ARM_PTRAUTH_GENERIC);
+
+	if (kvm->cfg.arch.enable_ptrauth && !supported)
+		die("--enable-ptrauth not supported on this host");
+
 	/*
 	 * Always enable Pointer Authentication if system supports
 	 * this extension unless disable-ptrauth option is present.
 	 */
-	if (kvm__supports_extension(kvm, KVM_CAP_ARM_PTRAUTH_ADDRESS) &&
-	    kvm__supports_extension(kvm, KVM_CAP_ARM_PTRAUTH_GENERIC) &&
-	    !kvm->cfg.arch.disable_ptrauth) {
+	if (supported && !kvm->cfg.arch.disable_ptrauth) {
 		init->features[0] |= 1UL << KVM_ARM_VCPU_PTRAUTH_ADDRESS;
 		init->features[0] |= 1UL << KVM_ARM_VCPU_PTRAUTH_GENERIC;
 	}
diff --git a/arm/kvm-cpu.c b/arm/kvm-cpu.c
index 764fb05..1652f6f 100644
--- a/arm/kvm-cpu.c
+++ b/arm/kvm-cpu.c
@@ -108,12 +108,8 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id)
 			die("Unable to find matching target");
 	}
 
-	if (err || target->init(vcpu)) {
-		if (kvm->cfg.arch.enable_ptrauth)
-			die("Unable to initialise vcpu with pointer authentication feature");
-		else
-			die("Unable to initialise vcpu");
-	}
+	if (err || target->init(vcpu))
+		die("Unable to initialise vcpu");
 
 	coalesced_offset = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION,
 				 KVM_CAP_COALESCED_MMIO);
-- 
2.1.4

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH kvmtool v3 7/9] arm64: Make ptrauth enable/disable diagnostics more user-friendly

[Andre Przywara]

On Thu, 30 May 2019 16:13:12 +0100
Dave Martin <Dave.Martin@arm.com> wrote:

> To help the user understand what is going on, amend ptrauth
> configuration diagnostic messages to refer to command line options
> by the exact name used on the command line.
> 
> Also, provide a clean diagnostic when ptrauth is requested, but not
> availble.  The generic "Unable to initialise vcpu" message is
> rather cryptic for this case.

Again I don't see much value in having this as a separate patch, as it
basically just touches code introduced two patches earlier. I think it
should be merged into 5/9.

> Since we now don't attempt to enable ptrauth at all unless KVM
> reports the relevant capabilities, remove the error message for
> that case too: in any case, we can't diagnose precisely why
> KVM_ARM_VCPU_INIT failed, so the message may be misleading.

So this leaves the only point where we use .enable_ptrauth to that error
message about the host not supporting it. Not sure if that's worth this
separate option?

Cheers,
Andre.

> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> ---
>  arm/aarch64/include/kvm/kvm-config-arch.h |  4 ++--
>  arm/aarch64/kvm-cpu.c                     | 15 +++++++++++----
>  arm/kvm-cpu.c                             |  8 ++------
>  3 files changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/arm/aarch64/include/kvm/kvm-config-arch.h b/arm/aarch64/include/kvm/kvm-config-arch.h
> index 0279b13..fe1699d 100644
> --- a/arm/aarch64/include/kvm/kvm-config-arch.h
> +++ b/arm/aarch64/include/kvm/kvm-config-arch.h
> @@ -10,9 +10,9 @@
>  			"Specify random seed for Kernel Address Space "	\
>  			"Layout Randomization (KASLR)"),		\
>  	OPT_BOOLEAN('\0', "enable-ptrauth", &(cfg)->enable_ptrauth,	\
> -			"Enables pointer authentication"),		\
> +			"Enable pointer authentication for the guest"),	\
>  	OPT_BOOLEAN('\0', "disable-ptrauth", &(cfg)->disable_ptrauth,	\
> -			"Disables pointer authentication"),
> +			"Disable pointer authentication for the guest"),
>  
>  #include "arm-common/kvm-config-arch.h"
>  
> diff --git a/arm/aarch64/kvm-cpu.c b/arm/aarch64/kvm-cpu.c
> index d3c32e0..08e4fd5 100644
> --- a/arm/aarch64/kvm-cpu.c
> +++ b/arm/aarch64/kvm-cpu.c
> @@ -130,16 +130,23 @@ static void reset_vcpu_aarch64(struct kvm_cpu *vcpu)
>  
>  static void select_ptrauth_feature(struct kvm *kvm, struct kvm_vcpu_init *init)
>  {
> +	bool supported;
> +
>  	/* Check Pointer Authentication command line arguments. */
>  	if (kvm->cfg.arch.enable_ptrauth && kvm->cfg.arch.disable_ptrauth)
> -		die("Both enable-ptrauth and disable-ptrauth option cannot be present");
> +		die("--enable-ptrauth conflicts with --disable-ptrauth");
> +
> +	supported = kvm__supports_extension(kvm, KVM_CAP_ARM_PTRAUTH_ADDRESS) &&
> +		    kvm__supports_extension(kvm, KVM_CAP_ARM_PTRAUTH_GENERIC);
> +
> +	if (kvm->cfg.arch.enable_ptrauth && !supported)
> +		die("--enable-ptrauth not supported on this host");
> +
>  	/*
>  	 * Always enable Pointer Authentication if system supports
>  	 * this extension unless disable-ptrauth option is present.
>  	 */
> -	if (kvm__supports_extension(kvm, KVM_CAP_ARM_PTRAUTH_ADDRESS) &&
> -	    kvm__supports_extension(kvm, KVM_CAP_ARM_PTRAUTH_GENERIC) &&
> -	    !kvm->cfg.arch.disable_ptrauth) {
> +	if (supported && !kvm->cfg.arch.disable_ptrauth) {
>  		init->features[0] |= 1UL << KVM_ARM_VCPU_PTRAUTH_ADDRESS;
>  		init->features[0] |= 1UL << KVM_ARM_VCPU_PTRAUTH_GENERIC;
>  	}
> diff --git a/arm/kvm-cpu.c b/arm/kvm-cpu.c
> index 764fb05..1652f6f 100644
> --- a/arm/kvm-cpu.c
> +++ b/arm/kvm-cpu.c
> @@ -108,12 +108,8 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id)
>  			die("Unable to find matching target");
>  	}
>  
> -	if (err || target->init(vcpu)) {
> -		if (kvm->cfg.arch.enable_ptrauth)
> -			die("Unable to initialise vcpu with pointer authentication feature");
> -		else
> -			die("Unable to initialise vcpu");
> -	}
> +	if (err || target->init(vcpu))
> +		die("Unable to initialise vcpu");
>  
>  	coalesced_offset = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION,
>  				 KVM_CAP_COALESCED_MMIO);

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH kvmtool v3 7/9] arm64: Make ptrauth enable/disable diagnostics more user-friendly

[Dave Martin]

On Fri, May 31, 2019 at 06:05:01PM +0100, Andre Przywara wrote:
> On Thu, 30 May 2019 16:13:12 +0100
> Dave Martin <Dave.Martin@arm.com> wrote:
> 
> > To help the user understand what is going on, amend ptrauth
> > configuration diagnostic messages to refer to command line options
> > by the exact name used on the command line.
> > 
> > Also, provide a clean diagnostic when ptrauth is requested, but not
> > availble.  The generic "Unable to initialise vcpu" message is
> > rather cryptic for this case.
> 
> Again I don't see much value in having this as a separate patch, as it
> basically just touches code introduced two patches earlier. I think it
> should be merged into 5/9.

Same as with the previous patch, I though it was better to keep it
separate for review purposes for now, since it makes changes on top of
Amit's existing patch.

> > Since we now don't attempt to enable ptrauth at all unless KVM
> > reports the relevant capabilities, remove the error message for
> > that case too: in any case, we can't diagnose precisely why
> > KVM_ARM_VCPU_INIT failed, so the message may be misleading.
> 
> So this leaves the only point where we use .enable_ptrauth to that error
> message about the host not supporting it. Not sure if that's worth this
> separate option?

There is indeed a question to be resolved here.  See my response to the
penultimate patch.

Cheers
---Dave
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH kvmtool v3 8/9] arm64: Add SVE support

[Dave Martin]

This patch adds --enable-sve/--disable-sve command line options to
allow the user to control whether the Scalable Vector Extension is
made available to the guest.

This requires use of the new KVM_ARM_VCPU_FINALIZE ioctl before the
vcpu is runnable, so a new hook kvm_cpu__configure_features() is
added to provide an appropriate place to do this work.

By default, SVE is enabled for the guest if the host supports it.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 arm/aarch32/include/kvm/kvm-cpu-arch.h    |  4 +++
 arm/aarch64/include/kvm/kvm-config-arch.h |  6 ++++-
 arm/aarch64/include/kvm/kvm-cpu-arch.h    |  1 +
 arm/aarch64/kvm-cpu.c                     | 41 +++++++++++++++++++++++++++++++
 arm/include/arm-common/kvm-config-arch.h  |  2 ++
 arm/kvm-cpu.c                             |  3 +++
 6 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/arm/aarch32/include/kvm/kvm-cpu-arch.h b/arm/aarch32/include/kvm/kvm-cpu-arch.h
index 01983f0..780e0e2 100644
--- a/arm/aarch32/include/kvm/kvm-cpu-arch.h
+++ b/arm/aarch32/include/kvm/kvm-cpu-arch.h
@@ -15,5 +15,9 @@
 
 static inline void kvm_cpu__select_features(struct kvm *kvm,
 					    struct kvm_vcpu_init *init) { }
+static inline int kvm_cpu__configure_features(struct kvm_cpu *vcpu)
+{
+	return 0;
+}
 
 #endif /* KVM__KVM_CPU_ARCH_H */
diff --git a/arm/aarch64/include/kvm/kvm-config-arch.h b/arm/aarch64/include/kvm/kvm-config-arch.h
index fe1699d..41e9d05 100644
--- a/arm/aarch64/include/kvm/kvm-config-arch.h
+++ b/arm/aarch64/include/kvm/kvm-config-arch.h
@@ -12,7 +12,11 @@
 	OPT_BOOLEAN('\0', "enable-ptrauth", &(cfg)->enable_ptrauth,	\
 			"Enable pointer authentication for the guest"),	\
 	OPT_BOOLEAN('\0', "disable-ptrauth", &(cfg)->disable_ptrauth,	\
-			"Disable pointer authentication for the guest"),
+			"Disable pointer authentication for the guest"), \
+	OPT_BOOLEAN('\0', "enable-sve", &(cfg)->enable_sve,		\
+			"Enable SVE for the guest"),			\
+	OPT_BOOLEAN('\0', "disable-sve", &(cfg)->disable_sve,		\
+			"Disable SVE for the guest"),
 
 #include "arm-common/kvm-config-arch.h"
 
diff --git a/arm/aarch64/include/kvm/kvm-cpu-arch.h b/arm/aarch64/include/kvm/kvm-cpu-arch.h
index e6875fc..8dfb82e 100644
--- a/arm/aarch64/include/kvm/kvm-cpu-arch.h
+++ b/arm/aarch64/include/kvm/kvm-cpu-arch.h
@@ -18,5 +18,6 @@
 #define ARM_CPU_CTRL_SCTLR_EL1	0
 
 void kvm_cpu__select_features(struct kvm *kvm, struct kvm_vcpu_init *init);
+int kvm_cpu__configure_features(struct kvm_cpu *vcpu);
 
 #endif /* KVM__KVM_CPU_ARCH_H */
diff --git a/arm/aarch64/kvm-cpu.c b/arm/aarch64/kvm-cpu.c
index 08e4fd5..cdfb22e 100644
--- a/arm/aarch64/kvm-cpu.c
+++ b/arm/aarch64/kvm-cpu.c
@@ -152,9 +152,50 @@ static void select_ptrauth_feature(struct kvm *kvm, struct kvm_vcpu_init *init)
 	}
 }
 
+static void select_sve_feature(struct kvm *kvm, struct kvm_vcpu_init *init)
+{
+	bool supported;
+
+	if (kvm->cfg.arch.enable_sve && kvm->cfg.arch.disable_sve)
+		die("--enable-sve conflicts with --disable-sve");
+
+	supported = kvm__supports_extension(kvm, KVM_CAP_ARM_SVE);
+
+	if (kvm->cfg.arch.enable_sve && !supported)
+		die("--enable-sve not supported on this host");
+
+	/* Default SVE to on if available and not explicitly disabled */
+	if (supported && !kvm->cfg.arch.disable_sve) {
+		kvm->cfg.arch.enable_sve = true;
+		init->features[0] |= 1UL << KVM_ARM_VCPU_SVE;
+	}
+}
+
 void kvm_cpu__select_features(struct kvm *kvm, struct kvm_vcpu_init *init)
 {
 	select_ptrauth_feature(kvm, init);
+	select_sve_feature(kvm, init);
+}
+
+static int configure_sve(struct kvm_cpu *vcpu)
+{
+	int feature = KVM_ARM_VCPU_SVE;
+
+	if (ioctl(vcpu->vcpu_fd, KVM_ARM_VCPU_FINALIZE, &feature)) {
+		pr_err("KVM_ARM_VCPU_FINALIZE: %s", strerror(errno));
+		return -1;
+	}
+
+	return 0;
+}
+
+int kvm_cpu__configure_features(struct kvm_cpu *vcpu)
+{
+	if (vcpu->kvm->cfg.arch.enable_sve)
+		if (configure_sve(vcpu))
+			return -1;
+
+	return 0;
 }
 
 void kvm_cpu__reset_vcpu(struct kvm_cpu *vcpu)
diff --git a/arm/include/arm-common/kvm-config-arch.h b/arm/include/arm-common/kvm-config-arch.h
index 1b4287d..40e3d1f 100644
--- a/arm/include/arm-common/kvm-config-arch.h
+++ b/arm/include/arm-common/kvm-config-arch.h
@@ -10,6 +10,8 @@ struct kvm_config_arch {
 	bool		aarch32_guest;
 	bool		has_pmuv3;
 	u64		kaslr_seed;
+	bool		enable_sve;
+	bool		disable_sve;
 	bool		enable_ptrauth;
 	bool		disable_ptrauth;
 	enum irqchip_type irqchip;
diff --git a/arm/kvm-cpu.c b/arm/kvm-cpu.c
index 1652f6f..554414f 100644
--- a/arm/kvm-cpu.c
+++ b/arm/kvm-cpu.c
@@ -124,6 +124,9 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id)
 	vcpu->cpu_compatible	= target->compatible;
 	vcpu->is_running	= true;
 
+	if (kvm_cpu__configure_features(vcpu))
+		die("Unable to configure requested vcpu features");
+
 	return vcpu;
 }
 
-- 
2.1.4

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH kvmtool v3 8/9] arm64: Add SVE support

[Andre Przywara]

On Thu, 30 May 2019 16:13:13 +0100
Dave Martin <Dave.Martin@arm.com> wrote:

> This patch adds --enable-sve/--disable-sve command line options to
> allow the user to control whether the Scalable Vector Extension is
> made available to the guest.

I guess I have a similar concern about this enable/disable pair being
confusing, though there is more sense here for SVE, given the impact of it
being enabled in the guest.

Maybe we can cover both pointer auth and SVE options with the same revised
approach?

Cheers,
Andre.


> This requires use of the new KVM_ARM_VCPU_FINALIZE ioctl before the
> vcpu is runnable, so a new hook kvm_cpu__configure_features() is
> added to provide an appropriate place to do this work.
> 
> By default, SVE is enabled for the guest if the host supports it.
> 
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> ---
>  arm/aarch32/include/kvm/kvm-cpu-arch.h    |  4 +++
>  arm/aarch64/include/kvm/kvm-config-arch.h |  6 ++++-
>  arm/aarch64/include/kvm/kvm-cpu-arch.h    |  1 +
>  arm/aarch64/kvm-cpu.c                     | 41 +++++++++++++++++++++++++++++++
>  arm/include/arm-common/kvm-config-arch.h  |  2 ++
>  arm/kvm-cpu.c                             |  3 +++
>  6 files changed, 56 insertions(+), 1 deletion(-)
> 
> diff --git a/arm/aarch32/include/kvm/kvm-cpu-arch.h b/arm/aarch32/include/kvm/kvm-cpu-arch.h
> index 01983f0..780e0e2 100644
> --- a/arm/aarch32/include/kvm/kvm-cpu-arch.h
> +++ b/arm/aarch32/include/kvm/kvm-cpu-arch.h
> @@ -15,5 +15,9 @@
>  
>  static inline void kvm_cpu__select_features(struct kvm *kvm,
>  					    struct kvm_vcpu_init *init) { }
> +static inline int kvm_cpu__configure_features(struct kvm_cpu *vcpu)
> +{
> +	return 0;
> +}
>  
>  #endif /* KVM__KVM_CPU_ARCH_H */
> diff --git a/arm/aarch64/include/kvm/kvm-config-arch.h b/arm/aarch64/include/kvm/kvm-config-arch.h
> index fe1699d..41e9d05 100644
> --- a/arm/aarch64/include/kvm/kvm-config-arch.h
> +++ b/arm/aarch64/include/kvm/kvm-config-arch.h
> @@ -12,7 +12,11 @@
>  	OPT_BOOLEAN('\0', "enable-ptrauth", &(cfg)->enable_ptrauth,	\
>  			"Enable pointer authentication for the guest"),	\
>  	OPT_BOOLEAN('\0', "disable-ptrauth", &(cfg)->disable_ptrauth,	\
> -			"Disable pointer authentication for the guest"),
> +			"Disable pointer authentication for the guest"), \
> +	OPT_BOOLEAN('\0', "enable-sve", &(cfg)->enable_sve,		\
> +			"Enable SVE for the guest"),			\
> +	OPT_BOOLEAN('\0', "disable-sve", &(cfg)->disable_sve,		\
> +			"Disable SVE for the guest"),
>  
>  #include "arm-common/kvm-config-arch.h"
>  
> diff --git a/arm/aarch64/include/kvm/kvm-cpu-arch.h b/arm/aarch64/include/kvm/kvm-cpu-arch.h
> index e6875fc..8dfb82e 100644
> --- a/arm/aarch64/include/kvm/kvm-cpu-arch.h
> +++ b/arm/aarch64/include/kvm/kvm-cpu-arch.h
> @@ -18,5 +18,6 @@
>  #define ARM_CPU_CTRL_SCTLR_EL1	0
>  
>  void kvm_cpu__select_features(struct kvm *kvm, struct kvm_vcpu_init *init);
> +int kvm_cpu__configure_features(struct kvm_cpu *vcpu);
>  
>  #endif /* KVM__KVM_CPU_ARCH_H */
> diff --git a/arm/aarch64/kvm-cpu.c b/arm/aarch64/kvm-cpu.c
> index 08e4fd5..cdfb22e 100644
> --- a/arm/aarch64/kvm-cpu.c
> +++ b/arm/aarch64/kvm-cpu.c
> @@ -152,9 +152,50 @@ static void select_ptrauth_feature(struct kvm *kvm, struct kvm_vcpu_init *init)
>  	}
>  }
>  
> +static void select_sve_feature(struct kvm *kvm, struct kvm_vcpu_init *init)
> +{
> +	bool supported;
> +
> +	if (kvm->cfg.arch.enable_sve && kvm->cfg.arch.disable_sve)
> +		die("--enable-sve conflicts with --disable-sve");
> +
> +	supported = kvm__supports_extension(kvm, KVM_CAP_ARM_SVE);
> +
> +	if (kvm->cfg.arch.enable_sve && !supported)
> +		die("--enable-sve not supported on this host");
> +
> +	/* Default SVE to on if available and not explicitly disabled */
> +	if (supported && !kvm->cfg.arch.disable_sve) {
> +		kvm->cfg.arch.enable_sve = true;
> +		init->features[0] |= 1UL << KVM_ARM_VCPU_SVE;
> +	}
> +}
> +
>  void kvm_cpu__select_features(struct kvm *kvm, struct kvm_vcpu_init *init)
>  {
>  	select_ptrauth_feature(kvm, init);
> +	select_sve_feature(kvm, init);
> +}
> +
> +static int configure_sve(struct kvm_cpu *vcpu)
> +{
> +	int feature = KVM_ARM_VCPU_SVE;
> +
> +	if (ioctl(vcpu->vcpu_fd, KVM_ARM_VCPU_FINALIZE, &feature)) {
> +		pr_err("KVM_ARM_VCPU_FINALIZE: %s", strerror(errno));
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +int kvm_cpu__configure_features(struct kvm_cpu *vcpu)
> +{
> +	if (vcpu->kvm->cfg.arch.enable_sve)
> +		if (configure_sve(vcpu))
> +			return -1;
> +
> +	return 0;
>  }
>  
>  void kvm_cpu__reset_vcpu(struct kvm_cpu *vcpu)
> diff --git a/arm/include/arm-common/kvm-config-arch.h b/arm/include/arm-common/kvm-config-arch.h
> index 1b4287d..40e3d1f 100644
> --- a/arm/include/arm-common/kvm-config-arch.h
> +++ b/arm/include/arm-common/kvm-config-arch.h
> @@ -10,6 +10,8 @@ struct kvm_config_arch {
>  	bool		aarch32_guest;
>  	bool		has_pmuv3;
>  	u64		kaslr_seed;
> +	bool		enable_sve;
> +	bool		disable_sve;
>  	bool		enable_ptrauth;
>  	bool		disable_ptrauth;
>  	enum irqchip_type irqchip;
> diff --git a/arm/kvm-cpu.c b/arm/kvm-cpu.c
> index 1652f6f..554414f 100644
> --- a/arm/kvm-cpu.c
> +++ b/arm/kvm-cpu.c
> @@ -124,6 +124,9 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id)
>  	vcpu->cpu_compatible	= target->compatible;
>  	vcpu->is_running	= true;
>  
> +	if (kvm_cpu__configure_features(vcpu))
> +		die("Unable to configure requested vcpu features");
> +
>  	return vcpu;
>  }
>  

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH kvmtool v3 8/9] arm64: Add SVE support

[Dave Martin]

On Fri, May 31, 2019 at 06:13:31PM +0100, Andre Przywara wrote:
> On Thu, 30 May 2019 16:13:13 +0100
> Dave Martin <Dave.Martin@arm.com> wrote:
> 
> > This patch adds --enable-sve/--disable-sve command line options to
> > allow the user to control whether the Scalable Vector Extension is
> > made available to the guest.
> 
> I guess I have a similar concern about this enable/disable pair being
> confusing, though there is more sense here for SVE, given the impact of it
> being enabled in the guest.
> 
> Maybe we can cover both pointer auth and SVE options with the same revised
> approach?

I agree that we should follow the same approach for both when we've
decided what approach to take.

(That was part of the reason for pulling both into the same series -- I
didn't want to end up randomly doing two different things without a
conscious intention to do so.)

Cheers
---Dave
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH kvmtool v3 9/9] arm64: Select SVE vector lengths via the command line

[Dave Martin]

In order to support use cases such as migration, it may be
important in some situations to restrict the set of SVE vector
lengths available to the guest.  It can also be useful to observe
the behaviour of guest OSes with different vector lengths.

To enable testing and experimentation for such configurations, this
patch adds a command-line option to allow setting of the set of
vector lengths to be made available to the guest.

For now, the setting is global: no means is offered to configure
individual guest vcpus independently of each other.

By default all vector lengths that the host can support are given
to the guest, as before.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 arm/aarch64/include/kvm/kvm-config-arch.h |  8 +++-
 arm/aarch64/kvm-cpu.c                     | 80 ++++++++++++++++++++++++++++++-
 arm/include/arm-common/kvm-config-arch.h  |  1 +
 3 files changed, 87 insertions(+), 2 deletions(-)

diff --git a/arm/aarch64/include/kvm/kvm-config-arch.h b/arm/aarch64/include/kvm/kvm-config-arch.h
index 41e9d05..a996612 100644
--- a/arm/aarch64/include/kvm/kvm-config-arch.h
+++ b/arm/aarch64/include/kvm/kvm-config-arch.h
@@ -1,6 +1,8 @@
 #ifndef KVM__KVM_CONFIG_ARCH_H
 #define KVM__KVM_CONFIG_ARCH_H
 
+int sve_vls_parser(const struct option *opt, const char *arg, int unset);
+
 #define ARM_OPT_ARCH_RUN(cfg)						\
 	OPT_BOOLEAN('\0', "aarch32", &(cfg)->aarch32_guest,		\
 			"Run AArch32 guest"),				\
@@ -16,7 +18,11 @@
 	OPT_BOOLEAN('\0', "enable-sve", &(cfg)->enable_sve,		\
 			"Enable SVE for the guest"),			\
 	OPT_BOOLEAN('\0', "disable-sve", &(cfg)->disable_sve,		\
-			"Disable SVE for the guest"),
+			"Disable SVE for the guest"),			\
+	OPT_CALLBACK('\0', "sve-vls", &(cfg)->sve_vqs,			\
+		     "comma-separated list of vector lengths, in 128-bit units", \
+		     "Set of vector lengths to enable for the guest",	\
+		     sve_vls_parser, NULL),
 
 #include "arm-common/kvm-config-arch.h"
 
diff --git a/arm/aarch64/kvm-cpu.c b/arm/aarch64/kvm-cpu.c
index cdfb22e..2c624c3 100644
--- a/arm/aarch64/kvm-cpu.c
+++ b/arm/aarch64/kvm-cpu.c
@@ -1,8 +1,13 @@
+#include <errno.h>
+#include <stdio.h>
+#include <string.h>
+
 #include "kvm/kvm-cpu.h"
 #include "kvm/kvm.h"
 #include "kvm/virtio.h"
 
 #include <asm/ptrace.h>
+#include <asm/sigcontext.h>
 
 #define COMPAT_PSR_F_BIT	0x00000040
 #define COMPAT_PSR_I_BIT	0x00000080
@@ -12,6 +17,65 @@
 #define SCTLR_EL1_E0E_MASK	(1 << 24)
 #define SCTLR_EL1_EE_MASK	(1 << 25)
 
+/*
+ * Work around old kernel headers that lack these definitions in
+ * <asm/sigcontext.h>:
+ */
+#ifndef SVE_VQ_MIN
+#define SVE_VQ_MIN 1
+#endif
+
+#ifndef SVE_VQ_MAX
+#define SVE_VQ_MAX 512
+#endif
+
+int sve_vls_parser(const struct option *opt, const char *arg, int unset)
+{
+	size_t offset = 0;
+	int vq, n, t;
+	u64 (*vqs)[(SVE_VQ_MAX + 1 - SVE_VQ_MIN + 63) / 64];
+	u64 **cfg_vqs = opt->value;
+
+	if (*cfg_vqs) {
+		pr_err("sve-vls: SVE vector lengths set may only be specified once");
+		return -1;
+	}
+
+	vqs = calloc(1, sizeof *vqs);
+	if (!vqs)
+		die("%s", strerror(ENOMEM));
+
+	offset = 0;
+	while (arg[offset]) {
+		n = -1;
+
+		t = sscanf(arg + offset,
+			   offset == 0 ? "%i%n" : ",%i%n",
+			   &vq, &n);
+		if (t == EOF || t < 1 || n <= 0) {
+			pr_err("sve-vls: Comma-separated list of vector lengths required");
+			goto error;
+		}
+
+		if (vq < SVE_VQ_MIN || vq > SVE_VQ_MAX) {
+			pr_err("sve-vls: Invalid vector length %d", vq);
+			goto error;
+		}
+
+		vq -= SVE_VQ_MIN;
+		(*vqs)[vq / 64] |= (u64)1 << (vq % 64);
+
+		offset += n;
+	}
+
+	*cfg_vqs = *vqs;
+	return 0;
+
+error:
+	free(vqs);
+	return -1;
+}
+
 static __u64 __core_reg_id(__u64 offset)
 {
 	__u64 id = KVM_REG_ARM64 | KVM_REG_ARM_CORE | offset;
@@ -180,6 +244,16 @@ void kvm_cpu__select_features(struct kvm *kvm, struct kvm_vcpu_init *init)
 static int configure_sve(struct kvm_cpu *vcpu)
 {
 	int feature = KVM_ARM_VCPU_SVE;
+	struct kvm_one_reg r = {
+		.id = KVM_REG_ARM64_SVE_VLS,
+		.addr = (u64)vcpu->kvm->cfg.arch.sve_vqs,
+	};
+
+	if (vcpu->kvm->cfg.arch.sve_vqs)
+		if (ioctl(vcpu->vcpu_fd, KVM_SET_ONE_REG, &r)) {
+			pr_err("Cannot set requested SVE vector lengths");
+			return -1;
+		}
 
 	if (ioctl(vcpu->vcpu_fd, KVM_ARM_VCPU_FINALIZE, &feature)) {
 		pr_err("KVM_ARM_VCPU_FINALIZE: %s", strerror(errno));
@@ -191,9 +265,13 @@ static int configure_sve(struct kvm_cpu *vcpu)
 
 int kvm_cpu__configure_features(struct kvm_cpu *vcpu)
 {
-	if (vcpu->kvm->cfg.arch.enable_sve)
+	if (vcpu->kvm->cfg.arch.enable_sve) {
 		if (configure_sve(vcpu))
 			return -1;
+	} else {
+		if (vcpu->kvm->cfg.arch.sve_vqs)
+			pr_warning("SVE vector lengths ignored");
+	}
 
 	return 0;
 }
diff --git a/arm/include/arm-common/kvm-config-arch.h b/arm/include/arm-common/kvm-config-arch.h
index 40e3d1f..b45201f 100644
--- a/arm/include/arm-common/kvm-config-arch.h
+++ b/arm/include/arm-common/kvm-config-arch.h
@@ -12,6 +12,7 @@ struct kvm_config_arch {
 	u64		kaslr_seed;
 	bool		enable_sve;
 	bool		disable_sve;
+	u64		*sve_vqs;
 	bool		enable_ptrauth;
 	bool		disable_ptrauth;
 	enum irqchip_type irqchip;
-- 
2.1.4

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm