[kvmtool PATCH v2 0/2] arm64: Add MTE support

[kvmtool PATCH v2 0/2] arm64: Add MTE support

[Alexandru Elisei]

Add Memory Tagging Extension (MTE) support in kvmtool.

Changes since v1:

* Update headers to v5.17
* MTE capability is now enabled by default and the command line option
  --disable-mte has been added.

Alexandru Elisei (2):
  update_headers.sh: Sync ABI headers with Linux v5.17
  aarch64: Add support for MTE

 arm/aarch32/include/kvm/kvm-arch.h        |  3 +++
 arm/aarch64/include/asm/kvm.h             |  5 +++++
 arm/aarch64/include/kvm/kvm-arch.h        |  1 +
 arm/aarch64/include/kvm/kvm-config-arch.h |  2 ++
 arm/aarch64/kvm.c                         | 23 +++++++++++++++++++++++
 arm/include/arm-common/kvm-config-arch.h  |  1 +
 arm/kvm.c                                 |  2 ++
 include/linux/kvm.h                       | 18 ++++++++++++++++++
 x86/include/asm/kvm.h                     | 19 ++++++++++++++++++-
 9 files changed, 73 insertions(+), 1 deletion(-)

-- 
2.35.1

[kvmtool PATCH v2 1/2] update_headers.sh: Sync ABI headers with Linux v5.17

[Alexandru Elisei]

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 arm/aarch64/include/asm/kvm.h |  5 +++++
 include/linux/kvm.h           | 18 ++++++++++++++++++
 x86/include/asm/kvm.h         | 19 ++++++++++++++++++-
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/arm/aarch64/include/asm/kvm.h b/arm/aarch64/include/asm/kvm.h
index b3edde68bc3e..323e251ed37b 100644
--- a/arm/aarch64/include/asm/kvm.h
+++ b/arm/aarch64/include/asm/kvm.h
@@ -281,6 +281,11 @@ struct kvm_arm_copy_mte_tags {
 #define KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_2_NOT_REQUIRED	3
 #define KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_2_ENABLED     	(1U << 4)
 
+#define KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_3	KVM_REG_ARM_FW_REG(3)
+#define KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_3_NOT_AVAIL		0
+#define KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_3_AVAIL		1
+#define KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_3_NOT_REQUIRED	2
+
 /* SVE registers */
 #define KVM_REG_ARM64_SVE		(0x15 << KVM_REG_ARM_COPROC_SHIFT)
 
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index 1daa45268de2..507ee1f2aa96 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -1131,6 +1131,10 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_EXIT_ON_EMULATION_FAILURE 204
 #define KVM_CAP_ARM_MTE 205
 #define KVM_CAP_VM_MOVE_ENC_CONTEXT_FROM 206
+#define KVM_CAP_VM_GPA_BITS 207
+#define KVM_CAP_XSAVE2 208
+#define KVM_CAP_SYS_ATTRIBUTES 209
+#define KVM_CAP_PPC_AIL_MODE_3 210
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
@@ -1162,11 +1166,20 @@ struct kvm_irq_routing_hv_sint {
 	__u32 sint;
 };
 
+struct kvm_irq_routing_xen_evtchn {
+	__u32 port;
+	__u32 vcpu;
+	__u32 priority;
+};
+
+#define KVM_IRQ_ROUTING_XEN_EVTCHN_PRIO_2LEVEL ((__u32)(-1))
+
 /* gsi routing entry types */
 #define KVM_IRQ_ROUTING_IRQCHIP 1
 #define KVM_IRQ_ROUTING_MSI 2
 #define KVM_IRQ_ROUTING_S390_ADAPTER 3
 #define KVM_IRQ_ROUTING_HV_SINT 4
+#define KVM_IRQ_ROUTING_XEN_EVTCHN 5
 
 struct kvm_irq_routing_entry {
 	__u32 gsi;
@@ -1178,6 +1191,7 @@ struct kvm_irq_routing_entry {
 		struct kvm_irq_routing_msi msi;
 		struct kvm_irq_routing_s390_adapter adapter;
 		struct kvm_irq_routing_hv_sint hv_sint;
+		struct kvm_irq_routing_xen_evtchn xen_evtchn;
 		__u32 pad[8];
 	} u;
 };
@@ -1208,6 +1222,7 @@ struct kvm_x86_mce {
 #define KVM_XEN_HVM_CONFIG_INTERCEPT_HCALL	(1 << 1)
 #define KVM_XEN_HVM_CONFIG_SHARED_INFO		(1 << 2)
 #define KVM_XEN_HVM_CONFIG_RUNSTATE		(1 << 3)
+#define KVM_XEN_HVM_CONFIG_EVTCHN_2LEVEL	(1 << 4)
 
 struct kvm_xen_hvm_config {
 	__u32 flags;
@@ -2031,4 +2046,7 @@ struct kvm_stats_desc {
 
 #define KVM_GET_STATS_FD  _IO(KVMIO,  0xce)
 
+/* Available with KVM_CAP_XSAVE2 */
+#define KVM_GET_XSAVE2		  _IOR(KVMIO,  0xcf, struct kvm_xsave)
+
 #endif /* __LINUX_KVM_H */
diff --git a/x86/include/asm/kvm.h b/x86/include/asm/kvm.h
index 5a776a08f78c..bf6e96011dfe 100644
--- a/x86/include/asm/kvm.h
+++ b/x86/include/asm/kvm.h
@@ -373,9 +373,23 @@ struct kvm_debugregs {
 	__u64 reserved[9];
 };
 
-/* for KVM_CAP_XSAVE */
+/* for KVM_CAP_XSAVE and KVM_CAP_XSAVE2 */
 struct kvm_xsave {
+	/*
+	 * KVM_GET_XSAVE2 and KVM_SET_XSAVE write and read as many bytes
+	 * as are returned by KVM_CHECK_EXTENSION(KVM_CAP_XSAVE2)
+	 * respectively, when invoked on the vm file descriptor.
+	 *
+	 * The size value returned by KVM_CHECK_EXTENSION(KVM_CAP_XSAVE2)
+	 * will always be at least 4096. Currently, it is only greater
+	 * than 4096 if a dynamic feature has been enabled with
+	 * ``arch_prctl()``, but this may change in the future.
+	 *
+	 * The offsets of the state save areas in struct kvm_xsave follow
+	 * the contents of CPUID leaf 0xD on the host.
+	 */
 	__u32 region[1024];
+	__u32 extra[0];
 };
 
 #define KVM_MAX_XCRS	16
@@ -438,6 +452,9 @@ struct kvm_sync_regs {
 
 #define KVM_STATE_VMX_PREEMPTION_TIMER_DEADLINE	0x00000001
 
+/* attributes for system fd (group 0) */
+#define KVM_X86_XCOMP_GUEST_SUPP	0
+
 struct kvm_vmx_nested_state_data {
 	__u8 vmcs12[KVM_STATE_NESTED_VMX_VMCS_SIZE];
 	__u8 shadow_vmcs12[KVM_STATE_NESTED_VMX_VMCS_SIZE];
-- 
2.35.1

[kvmtool PATCH v2 2/2] aarch64: Add support for MTE

[Alexandru Elisei]

MTE has been supported in Linux since commit 673638f434ee ("KVM: arm64:
Expose KVM_ARM_CAP_MTE"), add support for it in kvmtool. MTE is enabled by
default.

Enabling the MTE capability incurs a cost, both in time (for each
translation fault the tags need to be cleared), and in space (the tags need
to be saved when a physical page is swapped out). This overhead is expected
to be negligible for most users, but for those cases where they matter
(like performance benchmarks), a --disable-mte option has been added.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 arm/aarch32/include/kvm/kvm-arch.h        |  3 +++
 arm/aarch64/include/kvm/kvm-arch.h        |  1 +
 arm/aarch64/include/kvm/kvm-config-arch.h |  2 ++
 arm/aarch64/kvm.c                         | 23 +++++++++++++++++++++++
 arm/include/arm-common/kvm-config-arch.h  |  1 +
 arm/kvm.c                                 |  2 ++
 6 files changed, 32 insertions(+)

diff --git a/arm/aarch32/include/kvm/kvm-arch.h b/arm/aarch32/include/kvm/kvm-arch.h
index bee2fc255a82..5616b27e257e 100644
--- a/arm/aarch32/include/kvm/kvm-arch.h
+++ b/arm/aarch32/include/kvm/kvm-arch.h
@@ -5,6 +5,9 @@
 
 #define kvm__arch_get_kern_offset(...)	0x8000
 
+struct kvm;
+static inline void kvm__arch_enable_mte(struct kvm *kvm) {}
+
 #define ARM_MAX_MEMORY(...)	ARM_LOMAP_MAX_MEMORY
 
 #define MAX_PAGE_SIZE	SZ_4K
diff --git a/arm/aarch64/include/kvm/kvm-arch.h b/arm/aarch64/include/kvm/kvm-arch.h
index 5e5ee41211ed..9124f6919d0f 100644
--- a/arm/aarch64/include/kvm/kvm-arch.h
+++ b/arm/aarch64/include/kvm/kvm-arch.h
@@ -6,6 +6,7 @@
 struct kvm;
 unsigned long long kvm__arch_get_kern_offset(struct kvm *kvm, int fd);
 int kvm__arch_get_ipa_limit(struct kvm *kvm);
+void kvm__arch_enable_mte(struct kvm *kvm);
 
 #define ARM_MAX_MEMORY(kvm)	({					\
 	u64 max_ram;							\
diff --git a/arm/aarch64/include/kvm/kvm-config-arch.h b/arm/aarch64/include/kvm/kvm-config-arch.h
index 04be43dfa9b2..df4a15ff00a7 100644
--- a/arm/aarch64/include/kvm/kvm-config-arch.h
+++ b/arm/aarch64/include/kvm/kvm-config-arch.h
@@ -6,6 +6,8 @@
 			"Run AArch32 guest"),				\
 	OPT_BOOLEAN('\0', "pmu", &(cfg)->has_pmuv3,			\
 			"Create PMUv3 device"),				\
+	OPT_BOOLEAN('\0', "disable-mte", &(cfg)->mte_disabled,		\
+			"Disable Memory Tagging Extension capability"),	\
 	OPT_U64('\0', "kaslr-seed", &(cfg)->kaslr_seed,			\
 			"Specify random seed for Kernel Address Space "	\
 			"Layout Randomization (KASLR)"),
diff --git a/arm/aarch64/kvm.c b/arm/aarch64/kvm.c
index 56a0aedc263d..1035171a00f0 100644
--- a/arm/aarch64/kvm.c
+++ b/arm/aarch64/kvm.c
@@ -81,3 +81,26 @@ int kvm__get_vm_type(struct kvm *kvm)
 
 	return KVM_VM_TYPE_ARM_IPA_SIZE(ipa_bits);
 }
+
+void kvm__arch_enable_mte(struct kvm *kvm)
+{
+	struct kvm_enable_cap cap = {
+		.cap = KVM_CAP_ARM_MTE,
+	};
+
+	if (kvm->cfg.arch.mte_disabled) {
+		pr_debug("MTE capability disabled by user");
+		return;
+	}
+
+	if (!kvm__supports_extension(kvm, KVM_CAP_ARM_MTE)) {
+		kvm->cfg.arch.mte_disabled = true;
+		pr_debug("MTE capability not present");
+		return;
+	}
+
+	if (ioctl(kvm->vm_fd, KVM_ENABLE_CAP, &cap))
+		die_perror("KVM_ENABLE_CAP(KVM_CAP_ARM_MTE)");
+
+	pr_debug("MTE capability enabled");
+}
diff --git a/arm/include/arm-common/kvm-config-arch.h b/arm/include/arm-common/kvm-config-arch.h
index 5734c46ab9e6..f2049994d859 100644
--- a/arm/include/arm-common/kvm-config-arch.h
+++ b/arm/include/arm-common/kvm-config-arch.h
@@ -9,6 +9,7 @@ struct kvm_config_arch {
 	bool		virtio_trans_pci;
 	bool		aarch32_guest;
 	bool		has_pmuv3;
+	bool		mte_disabled;
 	u64		kaslr_seed;
 	enum irqchip_type irqchip;
 	u64		fw_addr;
diff --git a/arm/kvm.c b/arm/kvm.c
index 80d233f13d0b..c5913000e1ed 100644
--- a/arm/kvm.c
+++ b/arm/kvm.c
@@ -86,6 +86,8 @@ void kvm__arch_init(struct kvm *kvm, const char *hugetlbfs_path, u64 ram_size)
 	/* Create the virtual GIC. */
 	if (gic__create(kvm, kvm->cfg.arch.irqchip))
 		die("Failed to create virtual GIC");
+
+	kvm__arch_enable_mte(kvm);
 }
 
 #define FDT_ALIGN	SZ_2M
-- 
2.35.1

Re: [kvmtool PATCH v2 2/2] aarch64: Add support for MTE

[Vladimir Murzin]

Hi Alexandru,

On 3/24/22 11:39 AM, Alexandru Elisei wrote:
> MTE has been supported in Linux since commit 673638f434ee ("KVM: arm64:
> Expose KVM_ARM_CAP_MTE"), add support for it in kvmtool. MTE is enabled by
> default.
> 
> Enabling the MTE capability incurs a cost, both in time (for each
> translation fault the tags need to be cleared), and in space (the tags need
> to be saved when a physical page is swapped out). This overhead is expected
> to be negligible for most users, but for those cases where they matter
> (like performance benchmarks), a --disable-mte option has been added.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>   arm/aarch32/include/kvm/kvm-arch.h        |  3 +++
>   arm/aarch64/include/kvm/kvm-arch.h        |  1 +
>   arm/aarch64/include/kvm/kvm-config-arch.h |  2 ++
>   arm/aarch64/kvm.c                         | 23 +++++++++++++++++++++++
>   arm/include/arm-common/kvm-config-arch.h  |  1 +
>   arm/kvm.c                                 |  2 ++
>   6 files changed, 32 insertions(+)
> 
> diff --git a/arm/aarch32/include/kvm/kvm-arch.h b/arm/aarch32/include/kvm/kvm-arch.h
> index bee2fc255a82..5616b27e257e 100644
> --- a/arm/aarch32/include/kvm/kvm-arch.h
> +++ b/arm/aarch32/include/kvm/kvm-arch.h
> @@ -5,6 +5,9 @@
>   
>   #define kvm__arch_get_kern_offset(...)	0x8000
>   
> +struct kvm;
> +static inline void kvm__arch_enable_mte(struct kvm *kvm) {}
> +
>   #define ARM_MAX_MEMORY(...)	ARM_LOMAP_MAX_MEMORY
>   
>   #define MAX_PAGE_SIZE	SZ_4K
> diff --git a/arm/aarch64/include/kvm/kvm-arch.h b/arm/aarch64/include/kvm/kvm-arch.h
> index 5e5ee41211ed..9124f6919d0f 100644
> --- a/arm/aarch64/include/kvm/kvm-arch.h
> +++ b/arm/aarch64/include/kvm/kvm-arch.h
> @@ -6,6 +6,7 @@
>   struct kvm;
>   unsigned long long kvm__arch_get_kern_offset(struct kvm *kvm, int fd);
>   int kvm__arch_get_ipa_limit(struct kvm *kvm);
> +void kvm__arch_enable_mte(struct kvm *kvm);
>   
>   #define ARM_MAX_MEMORY(kvm)	({					\
>   	u64 max_ram;							\
> diff --git a/arm/aarch64/include/kvm/kvm-config-arch.h b/arm/aarch64/include/kvm/kvm-config-arch.h
> index 04be43dfa9b2..df4a15ff00a7 100644
> --- a/arm/aarch64/include/kvm/kvm-config-arch.h
> +++ b/arm/aarch64/include/kvm/kvm-config-arch.h
> @@ -6,6 +6,8 @@
>   			"Run AArch32 guest"),				\
>   	OPT_BOOLEAN('\0', "pmu", &(cfg)->has_pmuv3,			\
>   			"Create PMUv3 device"),				\
> +	OPT_BOOLEAN('\0', "disable-mte", &(cfg)->mte_disabled,		\
> +			"Disable Memory Tagging Extension capability"),	\
>   	OPT_U64('\0', "kaslr-seed", &(cfg)->kaslr_seed,			\
>   			"Specify random seed for Kernel Address Space "	\
>   			"Layout Randomization (KASLR)"),
> diff --git a/arm/aarch64/kvm.c b/arm/aarch64/kvm.c
> index 56a0aedc263d..1035171a00f0 100644
> --- a/arm/aarch64/kvm.c
> +++ b/arm/aarch64/kvm.c
> @@ -81,3 +81,26 @@ int kvm__get_vm_type(struct kvm *kvm)
>   
>   	return KVM_VM_TYPE_ARM_IPA_SIZE(ipa_bits);
>   }
> +
> +void kvm__arch_enable_mte(struct kvm *kvm)
> +{
> +	struct kvm_enable_cap cap = {
> +		.cap = KVM_CAP_ARM_MTE,
> +	};
> +
> +	if (kvm->cfg.arch.mte_disabled) {
> +		pr_debug("MTE capability disabled by user");
> +		return;
> +	}

Nitpick:  I'd move that bellow capability check, so it'd appear only in
setups which support MTE

> +
> +	if (!kvm__supports_extension(kvm, KVM_CAP_ARM_MTE)) {
> +		kvm->cfg.arch.mte_disabled = true;
> +		pr_debug("MTE capability not present");
> +		return;
> +	}
> +
> +	if (ioctl(kvm->vm_fd, KVM_ENABLE_CAP, &cap))
> +		die_perror("KVM_ENABLE_CAP(KVM_CAP_ARM_MTE)");
> +
> +	pr_debug("MTE capability enabled");
> +}
> diff --git a/arm/include/arm-common/kvm-config-arch.h b/arm/include/arm-common/kvm-config-arch.h
> index 5734c46ab9e6..f2049994d859 100644
> --- a/arm/include/arm-common/kvm-config-arch.h
> +++ b/arm/include/arm-common/kvm-config-arch.h
> @@ -9,6 +9,7 @@ struct kvm_config_arch {
>   	bool		virtio_trans_pci;
>   	bool		aarch32_guest;
>   	bool		has_pmuv3;
> +	bool		mte_disabled;
>   	u64		kaslr_seed;
>   	enum irqchip_type irqchip;
>   	u64		fw_addr;
> diff --git a/arm/kvm.c b/arm/kvm.c
> index 80d233f13d0b..c5913000e1ed 100644
> --- a/arm/kvm.c
> +++ b/arm/kvm.c
> @@ -86,6 +86,8 @@ void kvm__arch_init(struct kvm *kvm, const char *hugetlbfs_path, u64 ram_size)
>   	/* Create the virtual GIC. */
>   	if (gic__create(kvm, kvm->cfg.arch.irqchip))
>   		die("Failed to create virtual GIC");
> +
> +	kvm__arch_enable_mte(kvm);
>   }
>   
>   #define FDT_ALIGN	SZ_2M

FWIW:

Reviewed-by: Vladimir Murzin <vladimir.murzin@arm.com>
Tested-by: Vladimir Murzin <vladimir.murzin@arm.com>


Thanks!

Vladimir

Re: [kvmtool PATCH v2 2/2] aarch64: Add support for MTE

[Alexandru Elisei]

Hi,

On Thu, Mar 24, 2022 at 02:19:58PM +0000, Vladimir Murzin wrote:
> Hi Alexandru,
> 
> On 3/24/22 11:39 AM, Alexandru Elisei wrote:
> > MTE has been supported in Linux since commit 673638f434ee ("KVM: arm64:
> > Expose KVM_ARM_CAP_MTE"), add support for it in kvmtool. MTE is enabled by
> > default.
> > 
> > Enabling the MTE capability incurs a cost, both in time (for each
> > translation fault the tags need to be cleared), and in space (the tags need
> > to be saved when a physical page is swapped out). This overhead is expected
> > to be negligible for most users, but for those cases where they matter
> > (like performance benchmarks), a --disable-mte option has been added.
> > 
> > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > ---
> >   arm/aarch32/include/kvm/kvm-arch.h        |  3 +++
> >   arm/aarch64/include/kvm/kvm-arch.h        |  1 +
> >   arm/aarch64/include/kvm/kvm-config-arch.h |  2 ++
> >   arm/aarch64/kvm.c                         | 23 +++++++++++++++++++++++
> >   arm/include/arm-common/kvm-config-arch.h  |  1 +
> >   arm/kvm.c                                 |  2 ++
> >   6 files changed, 32 insertions(+)
> > 
> > diff --git a/arm/aarch32/include/kvm/kvm-arch.h b/arm/aarch32/include/kvm/kvm-arch.h
> > index bee2fc255a82..5616b27e257e 100644
> > --- a/arm/aarch32/include/kvm/kvm-arch.h
> > +++ b/arm/aarch32/include/kvm/kvm-arch.h
> > @@ -5,6 +5,9 @@
> >   #define kvm__arch_get_kern_offset(...)	0x8000
> > +struct kvm;
> > +static inline void kvm__arch_enable_mte(struct kvm *kvm) {}
> > +
> >   #define ARM_MAX_MEMORY(...)	ARM_LOMAP_MAX_MEMORY
> >   #define MAX_PAGE_SIZE	SZ_4K
> > diff --git a/arm/aarch64/include/kvm/kvm-arch.h b/arm/aarch64/include/kvm/kvm-arch.h
> > index 5e5ee41211ed..9124f6919d0f 100644
> > --- a/arm/aarch64/include/kvm/kvm-arch.h
> > +++ b/arm/aarch64/include/kvm/kvm-arch.h
> > @@ -6,6 +6,7 @@
> >   struct kvm;
> >   unsigned long long kvm__arch_get_kern_offset(struct kvm *kvm, int fd);
> >   int kvm__arch_get_ipa_limit(struct kvm *kvm);
> > +void kvm__arch_enable_mte(struct kvm *kvm);
> >   #define ARM_MAX_MEMORY(kvm)	({					\
> >   	u64 max_ram;							\
> > diff --git a/arm/aarch64/include/kvm/kvm-config-arch.h b/arm/aarch64/include/kvm/kvm-config-arch.h
> > index 04be43dfa9b2..df4a15ff00a7 100644
> > --- a/arm/aarch64/include/kvm/kvm-config-arch.h
> > +++ b/arm/aarch64/include/kvm/kvm-config-arch.h
> > @@ -6,6 +6,8 @@
> >   			"Run AArch32 guest"),				\
> >   	OPT_BOOLEAN('\0', "pmu", &(cfg)->has_pmuv3,			\
> >   			"Create PMUv3 device"),				\
> > +	OPT_BOOLEAN('\0', "disable-mte", &(cfg)->mte_disabled,		\
> > +			"Disable Memory Tagging Extension capability"),	\
> >   	OPT_U64('\0', "kaslr-seed", &(cfg)->kaslr_seed,			\
> >   			"Specify random seed for Kernel Address Space "	\
> >   			"Layout Randomization (KASLR)"),
> > diff --git a/arm/aarch64/kvm.c b/arm/aarch64/kvm.c
> > index 56a0aedc263d..1035171a00f0 100644
> > --- a/arm/aarch64/kvm.c
> > +++ b/arm/aarch64/kvm.c
> > @@ -81,3 +81,26 @@ int kvm__get_vm_type(struct kvm *kvm)
> >   	return KVM_VM_TYPE_ARM_IPA_SIZE(ipa_bits);
> >   }
> > +
> > +void kvm__arch_enable_mte(struct kvm *kvm)
> > +{
> > +	struct kvm_enable_cap cap = {
> > +		.cap = KVM_CAP_ARM_MTE,
> > +	};
> > +
> > +	if (kvm->cfg.arch.mte_disabled) {
> > +		pr_debug("MTE capability disabled by user");
> > +		return;
> > +	}
> 
> Nitpick:  I'd move that bellow capability check, so it'd appear only in
> setups which support MTE

I have no problem moving it, but I'm curious why you think it would be
useful. If the user disables MTE from kvmtool's command line doesn't that
mean that they aren't interested if the host supports it? It looks a bit
unexpected to me for kvmtool to check if the host supports MTE when the
user doesn't want to enable the cap.

> 
> > +
> > +	if (!kvm__supports_extension(kvm, KVM_CAP_ARM_MTE)) {
> > +		kvm->cfg.arch.mte_disabled = true;
> > +		pr_debug("MTE capability not present");
> > +		return;
> > +	}
> > +
> > +	if (ioctl(kvm->vm_fd, KVM_ENABLE_CAP, &cap))
> > +		die_perror("KVM_ENABLE_CAP(KVM_CAP_ARM_MTE)");
> > +
> > +	pr_debug("MTE capability enabled");
> > +}
> > diff --git a/arm/include/arm-common/kvm-config-arch.h b/arm/include/arm-common/kvm-config-arch.h
> > index 5734c46ab9e6..f2049994d859 100644
> > --- a/arm/include/arm-common/kvm-config-arch.h
> > +++ b/arm/include/arm-common/kvm-config-arch.h
> > @@ -9,6 +9,7 @@ struct kvm_config_arch {
> >   	bool		virtio_trans_pci;
> >   	bool		aarch32_guest;
> >   	bool		has_pmuv3;
> > +	bool		mte_disabled;
> >   	u64		kaslr_seed;
> >   	enum irqchip_type irqchip;
> >   	u64		fw_addr;
> > diff --git a/arm/kvm.c b/arm/kvm.c
> > index 80d233f13d0b..c5913000e1ed 100644
> > --- a/arm/kvm.c
> > +++ b/arm/kvm.c
> > @@ -86,6 +86,8 @@ void kvm__arch_init(struct kvm *kvm, const char *hugetlbfs_path, u64 ram_size)
> >   	/* Create the virtual GIC. */
> >   	if (gic__create(kvm, kvm->cfg.arch.irqchip))
> >   		die("Failed to create virtual GIC");
> > +
> > +	kvm__arch_enable_mte(kvm);
> >   }
> >   #define FDT_ALIGN	SZ_2M
> 
> FWIW:
> 
> Reviewed-by: Vladimir Murzin <vladimir.murzin@arm.com>
> Tested-by: Vladimir Murzin <vladimir.murzin@arm.com>

Thanks!

Alex

Re: [kvmtool PATCH v2 2/2] aarch64: Add support for MTE

[Vladimir Murzin]

On 3/24/22 5:09 PM, Alexandru Elisei wrote:
> Hi,
> 
> On Thu, Mar 24, 2022 at 02:19:58PM +0000, Vladimir Murzin wrote:
>> Hi Alexandru,
>>
>> On 3/24/22 11:39 AM, Alexandru Elisei wrote:
>>> MTE has been supported in Linux since commit 673638f434ee ("KVM: arm64:
>>> Expose KVM_ARM_CAP_MTE"), add support for it in kvmtool. MTE is enabled by
>>> default.
>>>
>>> Enabling the MTE capability incurs a cost, both in time (for each
>>> translation fault the tags need to be cleared), and in space (the tags need
>>> to be saved when a physical page is swapped out). This overhead is expected
>>> to be negligible for most users, but for those cases where they matter
>>> (like performance benchmarks), a --disable-mte option has been added.
>>>
>>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
>>> ---
>>>    arm/aarch32/include/kvm/kvm-arch.h        |  3 +++
>>>    arm/aarch64/include/kvm/kvm-arch.h        |  1 +
>>>    arm/aarch64/include/kvm/kvm-config-arch.h |  2 ++
>>>    arm/aarch64/kvm.c                         | 23 +++++++++++++++++++++++
>>>    arm/include/arm-common/kvm-config-arch.h  |  1 +
>>>    arm/kvm.c                                 |  2 ++
>>>    6 files changed, 32 insertions(+)
>>>
>>> diff --git a/arm/aarch32/include/kvm/kvm-arch.h b/arm/aarch32/include/kvm/kvm-arch.h
>>> index bee2fc255a82..5616b27e257e 100644
>>> --- a/arm/aarch32/include/kvm/kvm-arch.h
>>> +++ b/arm/aarch32/include/kvm/kvm-arch.h
>>> @@ -5,6 +5,9 @@
>>>    #define kvm__arch_get_kern_offset(...)	0x8000
>>> +struct kvm;
>>> +static inline void kvm__arch_enable_mte(struct kvm *kvm) {}
>>> +
>>>    #define ARM_MAX_MEMORY(...)	ARM_LOMAP_MAX_MEMORY
>>>    #define MAX_PAGE_SIZE	SZ_4K
>>> diff --git a/arm/aarch64/include/kvm/kvm-arch.h b/arm/aarch64/include/kvm/kvm-arch.h
>>> index 5e5ee41211ed..9124f6919d0f 100644
>>> --- a/arm/aarch64/include/kvm/kvm-arch.h
>>> +++ b/arm/aarch64/include/kvm/kvm-arch.h
>>> @@ -6,6 +6,7 @@
>>>    struct kvm;
>>>    unsigned long long kvm__arch_get_kern_offset(struct kvm *kvm, int fd);
>>>    int kvm__arch_get_ipa_limit(struct kvm *kvm);
>>> +void kvm__arch_enable_mte(struct kvm *kvm);
>>>    #define ARM_MAX_MEMORY(kvm)	({					\
>>>    	u64 max_ram;							\
>>> diff --git a/arm/aarch64/include/kvm/kvm-config-arch.h b/arm/aarch64/include/kvm/kvm-config-arch.h
>>> index 04be43dfa9b2..df4a15ff00a7 100644
>>> --- a/arm/aarch64/include/kvm/kvm-config-arch.h
>>> +++ b/arm/aarch64/include/kvm/kvm-config-arch.h
>>> @@ -6,6 +6,8 @@
>>>    			"Run AArch32 guest"),				\
>>>    	OPT_BOOLEAN('\0', "pmu", &(cfg)->has_pmuv3,			\
>>>    			"Create PMUv3 device"),				\
>>> +	OPT_BOOLEAN('\0', "disable-mte", &(cfg)->mte_disabled,		\
>>> +			"Disable Memory Tagging Extension capability"),	\
>>>    	OPT_U64('\0', "kaslr-seed", &(cfg)->kaslr_seed,			\
>>>    			"Specify random seed for Kernel Address Space "	\
>>>    			"Layout Randomization (KASLR)"),
>>> diff --git a/arm/aarch64/kvm.c b/arm/aarch64/kvm.c
>>> index 56a0aedc263d..1035171a00f0 100644
>>> --- a/arm/aarch64/kvm.c
>>> +++ b/arm/aarch64/kvm.c
>>> @@ -81,3 +81,26 @@ int kvm__get_vm_type(struct kvm *kvm)
>>>    	return KVM_VM_TYPE_ARM_IPA_SIZE(ipa_bits);
>>>    }
>>> +
>>> +void kvm__arch_enable_mte(struct kvm *kvm)
>>> +{
>>> +	struct kvm_enable_cap cap = {
>>> +		.cap = KVM_CAP_ARM_MTE,
>>> +	};
>>> +
>>> +	if (kvm->cfg.arch.mte_disabled) {
>>> +		pr_debug("MTE capability disabled by user");
>>> +		return;
>>> +	}
>>
>> Nitpick:  I'd move that bellow capability check, so it'd appear only in
>> setups which support MTE
> 
> I have no problem moving it, but I'm curious why you think it would be
> useful. If the user disables MTE from kvmtool's command line doesn't that
> mean that they aren't interested if the host supports it? It looks a bit
> unexpected to me for kvmtool to check if the host supports MTE when the
> user doesn't want to enable the cap.

It is probably matters of taste (so nitpick). I'm thinking if somebody has
scripts with --disable-mte which runs on zoo of hardware - we would output
"MTE capability disabled by user" even on those which do no support MTE at
all... "MTE capability not present", IMO, would look nicer on those machines

Anyway, it is not a show stopper ;)

Cheers
Vladimir