All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] AMX support in Qemu
@ 2022-01-24  7:55 Yang Zhong
  2022-01-24  7:55 ` [PATCH 1/7] x86: Fix the 64-byte boundary enumeration for extended state Yang Zhong
                   ` (7 more replies)
  0 siblings, 8 replies; 16+ messages in thread
From: Yang Zhong @ 2022-01-24  7:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: yang.zhong, kevin.tian, seanjc, jing2.liu, wei.w.wang,
	guang.zeng, pbonzini

Intel introduces Advanced Matrix Extensions (AMX) [1] feature that
consists of configurable two-dimensional "TILE" registers and new
accelerator instructions that operate on them. TMUL (Tile matrix
MULtiply) is the first accelerator instruction set to use the new
registers.

Since AMX KVM patches have been merged into Linux release, this series
is based on latest Linux release.

According to the KVM design, the userspace VMM (e.g. Qemu) is expected
to request guest permission for the dynamically-enabled XSAVE features
only once when the first vCPU is created, and KVM checks guest permission
in KVM_SET_CPUID2.

Intel AMX is XSAVE supported and XSAVE enabled. Those extended features
has large state while current kvm_xsave only allows 4KB. The AMX KVM has
extended struct kvm_xsave to meet this requirenment and added one extra
KVM_GET_XSAVE2 ioctl to handle extended features. From our test, the AMX
live migration work well.

Notice: This version still includes some definitions in the linux-headers,
once Qemu sync those linux-headers, I will remove those definitions. So
please ignore those changes.

[1] Intel Architecture Instruction Set Extension Programming Reference
    https://software.intel.com/content/dam/develop/external/us/en/documents/\
    architecture-instruction-set-extensions-programming-reference.pdf

Thanks,
Yang
----

change history
--------------
rfc v1->v1:
   - Patch 1 changed commit message(Kevin and Paolo).
   - Patch 2 changed commit message(Kevin and Paolo).
   - Patch 3, below requirements from Paolo,
     - Called ARCH_REQ_XCOMP_GUEST_PERM from x86_cpu_enable_xsave_components.
       Used kvm_request_xsave_components() to replace x86_xsave_req_perm().
       Replaced syscall(ARCH_GET_XCOMP_GUEST_PERM) with kvm_arch_get_supported_cpuid()
       in kvm_request_xsave_components().
     - Changed kvm_cpu_xsave_init() to use host_cpuid() instead of
       kvm_arch_get_supported_cpuid().
     - Added the "function == 0xd" handle in kvm_arch_get_supported_cpuid().   
   - Patch 4, used "uint32_t ecx" to replace "uint32_t need_align, support_xfd".
   - Patch 6, below changes,
     - Changed the commit message(Kevin) and Used the new function
     - kvm_init_xsave() to replace some pieces of code(Wei).
     - Moved KVM_CAP_XSAVE2 extension check to kvm_arch_init_vcpu() to
       make the request permission before KVM_CAP_XSAVE2 extension check(Paolo).
   - Removed RFC prefix.

Jing Liu (5):
  x86: Fix the 64-byte boundary enumeration for extended state
  x86: Add AMX XTILECFG and XTILEDATA components
  x86: Add XFD faulting bit for state components
  x86: Add AMX CPUIDs enumeration
  x86: add support for KVM_CAP_XSAVE2 and AMX state migration

Yang Zhong (1):
  x86: Grant AMX permission for guest

Zeng Guang (1):
  x86: Support XFD and AMX xsave data migration

 linux-headers/asm-x86/kvm.h | 14 ++++++
 linux-headers/linux/kvm.h   |  2 +
 target/i386/cpu.h           | 46 +++++++++++++++++-
 target/i386/cpu.c           | 96 +++++++++++++++++++++++++++++++++++--
 target/i386/kvm/kvm-cpu.c   | 12 +++--
 target/i386/kvm/kvm.c       | 69 +++++++++++++++++++-------
 target/i386/machine.c       | 42 ++++++++++++++++
 target/i386/xsave_helper.c  | 35 ++++++++++++++
 8 files changed, 291 insertions(+), 25 deletions(-)



^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/7] x86: Fix the 64-byte boundary enumeration for extended state
  2022-01-24  7:55 [PATCH 0/7] AMX support in Qemu Yang Zhong
@ 2022-01-24  7:55 ` Yang Zhong
  2022-01-24  7:55 ` [PATCH 2/7] x86: Add AMX XTILECFG and XTILEDATA components Yang Zhong
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Yang Zhong @ 2022-01-24  7:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: yang.zhong, kevin.tian, seanjc, jing2.liu, wei.w.wang,
	guang.zeng, pbonzini

From: Jing Liu <jing2.liu@intel.com>

The extended state subleaves (EAX=0Dh, ECX=n, n>1).ECX[1]
indicate whether the extended state component locates
on the next 64-byte boundary following the preceding state
component when the compacted format of an XSAVE area is
used.

Right now, they are all zero because no supported component
needed the bit to be set, but the upcoming AMX feature will
use it.  Fix the subleaves value according to KVM's supported
cpuid.

Signed-off-by: Jing Liu <jing2.liu@intel.com>
Signed-off-by: Yang Zhong <yang.zhong@intel.com>
---
 target/i386/cpu.h         | 6 ++++++
 target/i386/cpu.c         | 1 +
 target/i386/kvm/kvm-cpu.c | 2 ++
 3 files changed, 9 insertions(+)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 9911d7c871..de1dc124ab 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -548,6 +548,11 @@ typedef enum X86Seg {
 #define XSTATE_Hi16_ZMM_MASK            (1ULL << XSTATE_Hi16_ZMM_BIT)
 #define XSTATE_PKRU_MASK                (1ULL << XSTATE_PKRU_BIT)
 
+#define ESA_FEATURE_ALIGN64_BIT         1
+
+#define ESA_FEATURE_ALIGN64_MASK        (1U << ESA_FEATURE_ALIGN64_BIT)
+
+
 /* CPUID feature words */
 typedef enum FeatureWord {
     FEAT_1_EDX,         /* CPUID[1].EDX */
@@ -1354,6 +1359,7 @@ QEMU_BUILD_BUG_ON(sizeof(XSavePKRU) != 0x8);
 typedef struct ExtSaveArea {
     uint32_t feature, bits;
     uint32_t offset, size;
+    uint32_t ecx;
 } ExtSaveArea;
 
 #define XSAVE_STATE_AREA_COUNT (XSTATE_PKRU_BIT + 1)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index aa9e636800..37f06b0b1a 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5487,6 +5487,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
                 const ExtSaveArea *esa = &x86_ext_save_areas[count];
                 *eax = esa->size;
                 *ebx = esa->offset;
+                *ecx = esa->ecx & ESA_FEATURE_ALIGN64_MASK;
             }
         }
         break;
diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
index d95028018e..033ca011ea 100644
--- a/target/i386/kvm/kvm-cpu.c
+++ b/target/i386/kvm/kvm-cpu.c
@@ -105,6 +105,8 @@ static void kvm_cpu_xsave_init(void)
                 assert(esa->size == sz);
                 esa->offset = kvm_arch_get_supported_cpuid(s, 0xd, i, R_EBX);
             }
+
+            esa->ecx = kvm_arch_get_supported_cpuid(s, 0xd, i, R_ECX);
         }
     }
 }


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/7] x86: Add AMX XTILECFG and XTILEDATA components
  2022-01-24  7:55 [PATCH 0/7] AMX support in Qemu Yang Zhong
  2022-01-24  7:55 ` [PATCH 1/7] x86: Fix the 64-byte boundary enumeration for extended state Yang Zhong
@ 2022-01-24  7:55 ` Yang Zhong
  2022-01-24  7:55 ` [PATCH 3/7] x86: Grant AMX permission for guest Yang Zhong
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Yang Zhong @ 2022-01-24  7:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: yang.zhong, kevin.tian, seanjc, jing2.liu, wei.w.wang,
	guang.zeng, pbonzini

From: Jing Liu <jing2.liu@intel.com>

The AMX TILECFG register and the TMMx tile data registers are
saved/restored via XSAVE, respectively in state component 17
(64 bytes) and state component 18 (8192 bytes).

Add AMX feature bits to x86_ext_save_areas array to set
up AMX components. Add structs that define the layout of
AMX XSAVE areas and use QEMU_BUILD_BUG_ON to validate the
structs sizes.

Signed-off-by: Jing Liu <jing2.liu@intel.com>
Signed-off-by: Yang Zhong <yang.zhong@intel.com>
---
 target/i386/cpu.h | 18 +++++++++++++++++-
 target/i386/cpu.c |  8 ++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index de1dc124ab..06d2d6bccf 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -537,6 +537,8 @@ typedef enum X86Seg {
 #define XSTATE_ZMM_Hi256_BIT            6
 #define XSTATE_Hi16_ZMM_BIT             7
 #define XSTATE_PKRU_BIT                 9
+#define XSTATE_XTILE_CFG_BIT            17
+#define XSTATE_XTILE_DATA_BIT           18
 
 #define XSTATE_FP_MASK                  (1ULL << XSTATE_FP_BIT)
 #define XSTATE_SSE_MASK                 (1ULL << XSTATE_SSE_BIT)
@@ -845,6 +847,8 @@ typedef uint64_t FeatureWordArray[FEATURE_WORDS];
 #define CPUID_7_0_EDX_TSX_LDTRK         (1U << 16)
 /* AVX512_FP16 instruction */
 #define CPUID_7_0_EDX_AVX512_FP16       (1U << 23)
+/* AMX tile (two-dimensional register) */
+#define CPUID_7_0_EDX_AMX_TILE          (1U << 24)
 /* Speculation Control */
 #define CPUID_7_0_EDX_SPEC_CTRL         (1U << 26)
 /* Single Thread Indirect Branch Predictors */
@@ -1348,6 +1352,16 @@ typedef struct XSavePKRU {
     uint32_t padding;
 } XSavePKRU;
 
+/* Ext. save area 17: AMX XTILECFG state */
+typedef struct XSaveXTILECFG {
+    uint8_t xtilecfg[64];
+} XSaveXTILECFG;
+
+/* Ext. save area 18: AMX XTILEDATA state */
+typedef struct XSaveXTILEDATA {
+    uint8_t xtiledata[8][1024];
+} XSaveXTILEDATA;
+
 QEMU_BUILD_BUG_ON(sizeof(XSaveAVX) != 0x100);
 QEMU_BUILD_BUG_ON(sizeof(XSaveBNDREG) != 0x40);
 QEMU_BUILD_BUG_ON(sizeof(XSaveBNDCSR) != 0x40);
@@ -1355,6 +1369,8 @@ QEMU_BUILD_BUG_ON(sizeof(XSaveOpmask) != 0x40);
 QEMU_BUILD_BUG_ON(sizeof(XSaveZMM_Hi256) != 0x200);
 QEMU_BUILD_BUG_ON(sizeof(XSaveHi16_ZMM) != 0x400);
 QEMU_BUILD_BUG_ON(sizeof(XSavePKRU) != 0x8);
+QEMU_BUILD_BUG_ON(sizeof(XSaveXTILECFG) != 0x40);
+QEMU_BUILD_BUG_ON(sizeof(XSaveXTILEDATA) != 0x2000);
 
 typedef struct ExtSaveArea {
     uint32_t feature, bits;
@@ -1362,7 +1378,7 @@ typedef struct ExtSaveArea {
     uint32_t ecx;
 } ExtSaveArea;
 
-#define XSAVE_STATE_AREA_COUNT (XSTATE_PKRU_BIT + 1)
+#define XSAVE_STATE_AREA_COUNT (XSTATE_XTILE_DATA_BIT + 1)
 
 extern ExtSaveArea x86_ext_save_areas[XSAVE_STATE_AREA_COUNT];
 
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 37f06b0b1a..3390820745 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1401,6 +1401,14 @@ ExtSaveArea x86_ext_save_areas[XSAVE_STATE_AREA_COUNT] = {
     [XSTATE_PKRU_BIT] =
           { .feature = FEAT_7_0_ECX, .bits = CPUID_7_0_ECX_PKU,
             .size = sizeof(XSavePKRU) },
+    [XSTATE_XTILE_CFG_BIT] = {
+        .feature = FEAT_7_0_EDX, .bits = CPUID_7_0_EDX_AMX_TILE,
+        .size = sizeof(XSaveXTILECFG),
+    },
+    [XSTATE_XTILE_DATA_BIT] = {
+        .feature = FEAT_7_0_EDX, .bits = CPUID_7_0_EDX_AMX_TILE,
+        .size = sizeof(XSaveXTILEDATA),
+    },
 };
 
 static uint32_t xsave_area_size(uint64_t mask)


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/7] x86: Grant AMX permission for guest
  2022-01-24  7:55 [PATCH 0/7] AMX support in Qemu Yang Zhong
  2022-01-24  7:55 ` [PATCH 1/7] x86: Fix the 64-byte boundary enumeration for extended state Yang Zhong
  2022-01-24  7:55 ` [PATCH 2/7] x86: Add AMX XTILECFG and XTILEDATA components Yang Zhong
@ 2022-01-24  7:55 ` Yang Zhong
  2022-01-24 10:16   ` Paolo Bonzini
  2022-01-24  7:55 ` [PATCH 4/7] x86: Add XFD faulting bit for state components Yang Zhong
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Yang Zhong @ 2022-01-24  7:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: yang.zhong, kevin.tian, seanjc, jing2.liu, wei.w.wang,
	guang.zeng, pbonzini

Kernel allocates 4K xstate buffer by default. For XSAVE features
which require large state component (e.g. AMX), Linux kernel
dynamically expands the xstate buffer only after the process has
acquired the necessary permissions. Those are called dynamically-
enabled XSAVE features (or dynamic xfeatures).

There are separate permissions for native tasks and guests.

Qemu should request the guest permissions for dynamic xfeatures
which will be exposed to the guest. This only needs to be done
once before the first vcpu is created.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Yang Zhong <yang.zhong@intel.com>
Signed-off-by: Jing Liu <jing2.liu@intel.com>
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
---
 target/i386/cpu.h         |  7 +++++++
 target/i386/cpu.c         | 31 +++++++++++++++++++++++++++++++
 target/i386/kvm/kvm-cpu.c | 12 ++++++------
 target/i386/kvm/kvm.c     |  6 ++++++
 4 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 06d2d6bccf..d4ad0f56bd 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -549,6 +549,13 @@ typedef enum X86Seg {
 #define XSTATE_ZMM_Hi256_MASK           (1ULL << XSTATE_ZMM_Hi256_BIT)
 #define XSTATE_Hi16_ZMM_MASK            (1ULL << XSTATE_Hi16_ZMM_BIT)
 #define XSTATE_PKRU_MASK                (1ULL << XSTATE_PKRU_BIT)
+#define XSTATE_XTILE_CFG_MASK           (1ULL << XSTATE_XTILE_CFG_BIT)
+#define XSTATE_XTILE_DATA_MASK          (1ULL << XSTATE_XTILE_DATA_BIT)
+#define XFEATURE_XTILE_MASK             (XSTATE_XTILE_CFG_MASK \
+                                         | XSTATE_XTILE_DATA_MASK)
+
+#define ARCH_GET_XCOMP_GUEST_PERM       0x1024
+#define ARCH_REQ_XCOMP_GUEST_PERM       0x1025
 
 #define ESA_FEATURE_ALIGN64_BIT         1
 
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 3390820745..29b0348c25 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -43,6 +43,10 @@
 #include "disas/capstone.h"
 #include "cpu-internal.h"
 
+#include <sys/syscall.h>
+
+bool request_perm;
+
 /* Helpers for building CPUID[2] descriptors: */
 
 struct CPUID2CacheDescriptorInfo {
@@ -6000,6 +6004,27 @@ static void x86_cpu_adjust_feat_level(X86CPU *cpu, FeatureWord w)
     }
 }
 
+static void kvm_request_xsave_components(X86CPU *cpu, uint32_t bit)
+{
+    KVMState *s = CPU(cpu)->kvm_state;
+
+    long rc = syscall(SYS_arch_prctl, ARCH_REQ_XCOMP_GUEST_PERM,
+                      bit);
+    if (rc) {
+        /*
+         * The older kernel version(<5.15) can't support
+         * ARCH_REQ_XCOMP_GUEST_PERM and directly return.
+         */
+        return;
+    }
+
+    rc = kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX);
+    if (!(rc & XFEATURE_XTILE_MASK)) {
+        error_report("get cpuid failure and rc=0x%lx", rc);
+        exit(EXIT_FAILURE);
+    }
+}
+
 /* Calculate XSAVE components based on the configured CPU feature flags */
 static void x86_cpu_enable_xsave_components(X86CPU *cpu)
 {
@@ -6021,6 +6046,12 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu)
         }
     }
 
+    /* Only request permission from fisrt vcpu. */
+    if (kvm_enabled() && !request_perm) {
+        kvm_request_xsave_components(cpu, XSTATE_XTILE_DATA_BIT);
+        request_perm = true;
+    }
+
     env->features[FEAT_XSAVE_COMP_LO] = mask;
     env->features[FEAT_XSAVE_COMP_HI] = mask >> 32;
 }
diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
index 033ca011ea..5ab6a0b9d2 100644
--- a/target/i386/kvm/kvm-cpu.c
+++ b/target/i386/kvm/kvm-cpu.c
@@ -84,7 +84,7 @@ static void kvm_cpu_max_instance_init(X86CPU *cpu)
 static void kvm_cpu_xsave_init(void)
 {
     static bool first = true;
-    KVMState *s = kvm_state;
+    uint32_t eax, ebx, ecx, edx;
     int i;
 
     if (!first) {
@@ -100,13 +100,13 @@ static void kvm_cpu_xsave_init(void)
         ExtSaveArea *esa = &x86_ext_save_areas[i];
 
         if (esa->size) {
-            int sz = kvm_arch_get_supported_cpuid(s, 0xd, i, R_EAX);
-            if (sz != 0) {
-                assert(esa->size == sz);
-                esa->offset = kvm_arch_get_supported_cpuid(s, 0xd, i, R_EBX);
+            host_cpuid(0xd, i, &eax, &ebx, &ecx, &edx);
+            if (eax != 0) {
+                assert(esa->size == eax);
+                esa->offset = ebx;
             }
 
-            esa->ecx = kvm_arch_get_supported_cpuid(s, 0xd, i, R_ECX);
+            esa->ecx = ecx;
         }
     }
 }
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 2c8feb4a6f..caf1388d8b 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -405,6 +405,12 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function,
         if (!has_msr_arch_capabs) {
             ret &= ~CPUID_7_0_EDX_ARCH_CAPABILITIES;
         }
+    } else if (function == 0xd && index == 0 && reg == R_EAX) {
+        /*
+         * We can set the AMX XTILE DATA flag, even if KVM does not
+         * return it on GET_SUPPORTED_CPUID.
+         */
+        ret |= XSTATE_XTILE_DATA_MASK;
     } else if (function == 0x80000001 && reg == R_ECX) {
         /*
          * It's safe to enable TOPOEXT even if it's not returned by


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/7] x86: Add XFD faulting bit for state components
  2022-01-24  7:55 [PATCH 0/7] AMX support in Qemu Yang Zhong
                   ` (2 preceding siblings ...)
  2022-01-24  7:55 ` [PATCH 3/7] x86: Grant AMX permission for guest Yang Zhong
@ 2022-01-24  7:55 ` Yang Zhong
  2022-01-24  7:55 ` [PATCH 5/7] x86: Add AMX CPUIDs enumeration Yang Zhong
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Yang Zhong @ 2022-01-24  7:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: yang.zhong, kevin.tian, seanjc, jing2.liu, wei.w.wang,
	guang.zeng, pbonzini

From: Jing Liu <jing2.liu@intel.com>

Intel introduces XFD faulting mechanism for extended
XSAVE features to dynamically enable the features in
runtime. If CPUID (EAX=0Dh, ECX=n, n>1).ECX[2] is set
as 1, it indicates support for XFD faulting of this
state component.

Signed-off-by: Jing Liu <jing2.liu@intel.com>
Signed-off-by: Yang Zhong <yang.zhong@intel.com>
---
 target/i386/cpu.h | 2 ++
 target/i386/cpu.c | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index d4ad0f56bd..f7fc2e97a6 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -558,8 +558,10 @@ typedef enum X86Seg {
 #define ARCH_REQ_XCOMP_GUEST_PERM       0x1025
 
 #define ESA_FEATURE_ALIGN64_BIT         1
+#define ESA_FEATURE_XFD_BIT             2
 
 #define ESA_FEATURE_ALIGN64_MASK        (1U << ESA_FEATURE_ALIGN64_BIT)
+#define ESA_FEATURE_XFD_MASK            (1U << ESA_FEATURE_XFD_BIT)
 
 
 /* CPUID feature words */
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 29b0348c25..ea13be0a19 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5499,7 +5499,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
                 const ExtSaveArea *esa = &x86_ext_save_areas[count];
                 *eax = esa->size;
                 *ebx = esa->offset;
-                *ecx = esa->ecx & ESA_FEATURE_ALIGN64_MASK;
+                *ecx = (esa->ecx & ESA_FEATURE_ALIGN64_MASK) |
+                       (esa->ecx & ESA_FEATURE_XFD_MASK);
             }
         }
         break;


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/7] x86: Add AMX CPUIDs enumeration
  2022-01-24  7:55 [PATCH 0/7] AMX support in Qemu Yang Zhong
                   ` (3 preceding siblings ...)
  2022-01-24  7:55 ` [PATCH 4/7] x86: Add XFD faulting bit for state components Yang Zhong
@ 2022-01-24  7:55 ` Yang Zhong
  2022-01-24 10:13   ` Paolo Bonzini
  2022-01-24  7:55 ` [PATCH 6/7] x86: add support for KVM_CAP_XSAVE2 and AMX state migration Yang Zhong
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Yang Zhong @ 2022-01-24  7:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: yang.zhong, kevin.tian, seanjc, jing2.liu, wei.w.wang,
	guang.zeng, pbonzini

From: Jing Liu <jing2.liu@intel.com>

Add AMX primary feature bits XFD and AMX_TILE to
enumerate the CPU's AMX capability. Meanwhile, add
AMX TILE and TMUL CPUID leaf and subleaves which
exist when AMX TILE is present to provide the maximum
capability of TILE and TMUL.

Signed-off-by: Jing Liu <jing2.liu@intel.com>
Signed-off-by: Yang Zhong <yang.zhong@intel.com>
---
 target/i386/cpu.c     | 55 ++++++++++++++++++++++++++++++++++++++++---
 target/i386/kvm/kvm.c |  3 ++-
 2 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index ea13be0a19..9543762e7e 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -578,6 +578,18 @@ static CPUCacheInfo legacy_l3_cache = {
 #define INTEL_PT_CYCLE_BITMAP    0x1fff         /* Support 0,2^(0~11) */
 #define INTEL_PT_PSB_BITMAP      (0x003f << 16) /* Support 2K,4K,8K,16K,32K,64K */
 
+/* CPUID Leaf 0x1D constants: */
+#define INTEL_AMX_TILE_MAX_SUBLEAF     0x1
+#define INTEL_AMX_TOTAL_TILE_BYTES     0x2000
+#define INTEL_AMX_BYTES_PER_TILE       0x400
+#define INTEL_AMX_BYTES_PER_ROW        0x40
+#define INTEL_AMX_TILE_MAX_NAMES       0x8
+#define INTEL_AMX_TILE_MAX_ROWS        0x10
+
+/* CPUID Leaf 0x1E constants: */
+#define INTEL_AMX_TMUL_MAX_K           0x10
+#define INTEL_AMX_TMUL_MAX_N           0x40
+
 void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1,
                               uint32_t vendor2, uint32_t vendor3)
 {
@@ -847,8 +859,8 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
             "avx512-vp2intersect", NULL, "md-clear", NULL,
             NULL, NULL, "serialize", NULL,
             "tsx-ldtrk", NULL, NULL /* pconfig */, NULL,
-            NULL, NULL, NULL, "avx512-fp16",
-            NULL, NULL, "spec-ctrl", "stibp",
+            NULL, NULL, "amx-bf16", "avx512-fp16",
+            "amx-tile", "amx-int8", "spec-ctrl", "stibp",
             NULL, "arch-capabilities", "core-capability", "ssbd",
         },
         .cpuid = {
@@ -913,7 +925,7 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
         .type = CPUID_FEATURE_WORD,
         .feat_names = {
             "xsaveopt", "xsavec", "xgetbv1", "xsaves",
-            NULL, NULL, NULL, NULL,
+            "xfd", NULL, NULL, NULL,
             NULL, NULL, NULL, NULL,
             NULL, NULL, NULL, NULL,
             NULL, NULL, NULL, NULL,
@@ -5589,6 +5601,43 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         }
         break;
     }
+    case 0x1D: {
+        /* AMX TILE */
+        *eax = 0;
+        *ebx = 0;
+        *ecx = 0;
+        *edx = 0;
+        if (!(env->features[FEAT_7_0_EDX] & CPUID_7_0_EDX_AMX_TILE)) {
+            break;
+        }
+
+        if (count == 0) {
+            /* Highest numbered palette subleaf */
+            *eax = INTEL_AMX_TILE_MAX_SUBLEAF;
+        } else if (count == 1) {
+            *eax = INTEL_AMX_TOTAL_TILE_BYTES |
+                   (INTEL_AMX_BYTES_PER_TILE << 16);
+            *ebx = INTEL_AMX_BYTES_PER_ROW | (INTEL_AMX_TILE_MAX_NAMES << 16);
+            *ecx = INTEL_AMX_TILE_MAX_ROWS;
+        }
+        break;
+    }
+    case 0x1E: {
+        /* AMX TMUL */
+        *eax = 0;
+        *ebx = 0;
+        *ecx = 0;
+        *edx = 0;
+        if (!(env->features[FEAT_7_0_EDX] & CPUID_7_0_EDX_AMX_TILE)) {
+            break;
+        }
+
+        if (count == 0) {
+            /* Highest numbered palette subleaf */
+            *ebx = INTEL_AMX_TMUL_MAX_K | (INTEL_AMX_TMUL_MAX_N << 8);
+        }
+        break;
+    }
     case 0x40000000:
         /*
          * CPUID code in kvm_arch_init_vcpu() ignores stuff
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index caf1388d8b..25d26a15f8 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -1765,7 +1765,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
                 c = &cpuid_data.entries[cpuid_i++];
             }
             break;
-        case 0x14: {
+        case 0x14:
+        case 0x1d: {
             uint32_t times;
 
             c->function = i;


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 6/7] x86: add support for KVM_CAP_XSAVE2 and AMX state migration
  2022-01-24  7:55 [PATCH 0/7] AMX support in Qemu Yang Zhong
                   ` (4 preceding siblings ...)
  2022-01-24  7:55 ` [PATCH 5/7] x86: Add AMX CPUIDs enumeration Yang Zhong
@ 2022-01-24  7:55 ` Yang Zhong
  2022-01-24 10:15   ` Paolo Bonzini
  2022-01-24  7:55 ` [PATCH 7/7] x86: Support XFD and AMX xsave data migration Yang Zhong
  2022-01-24 10:17 ` [PATCH 0/7] AMX support in Qemu Paolo Bonzini
  7 siblings, 1 reply; 16+ messages in thread
From: Yang Zhong @ 2022-01-24  7:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: yang.zhong, kevin.tian, seanjc, jing2.liu, wei.w.wang,
	guang.zeng, pbonzini

From: Jing Liu <jing2.liu@intel.com>

When dynamic xfeatures (e.g. AMX) are used by the guest, the xsave
area would be larger than 4KB. KVM_GET_XSAVE2 and KVM_SET_XSAVE
under KVM_CAP_XSAVE2 works with a xsave buffer larger than 4KB.
Always use the new ioctls under KVM_CAP_XSAVE2 when KVM supports it.

Signed-off-by: Jing Liu <jing2.liu@intel.com>
Signed-off-by: Zeng Guang <guang.zeng@intel.com>
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
Signed-off-by: Yang Zhong <yang.zhong@intel.com>
---
 linux-headers/asm-x86/kvm.h | 14 +++++++++++++
 linux-headers/linux/kvm.h   |  2 ++
 target/i386/cpu.h           |  4 ++++
 target/i386/kvm/kvm.c       | 42 ++++++++++++++++++++++++-------------
 target/i386/xsave_helper.c  | 35 +++++++++++++++++++++++++++++++
 5 files changed, 82 insertions(+), 15 deletions(-)

diff --git a/linux-headers/asm-x86/kvm.h b/linux-headers/asm-x86/kvm.h
index 5a776a08f7..2e37b825cd 100644
--- a/linux-headers/asm-x86/kvm.h
+++ b/linux-headers/asm-x86/kvm.h
@@ -375,7 +375,21 @@ struct kvm_debugregs {
 
 /* for KVM_CAP_XSAVE */
 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
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index 02c5e7b7bb..af67be1b9e 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -1130,6 +1130,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_BINARY_STATS_FD 203
 #define KVM_CAP_EXIT_ON_EMULATION_FAILURE 204
 #define KVM_CAP_ARM_MTE 205
+#define KVM_CAP_XSAVE2  208
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
@@ -1550,6 +1551,7 @@ struct kvm_s390_ucas_mapping {
 /* Available with KVM_CAP_XSAVE */
 #define KVM_GET_XSAVE		  _IOR(KVMIO,  0xa4, struct kvm_xsave)
 #define KVM_SET_XSAVE		  _IOW(KVMIO,  0xa5, struct kvm_xsave)
+#define KVM_GET_XSAVE2		  _IOR(KVMIO,  0xcf, struct kvm_xsave)
 /* Available with KVM_CAP_XCRS */
 #define KVM_GET_XCRS		  _IOR(KVMIO,  0xa6, struct kvm_xcrs)
 #define KVM_SET_XCRS		  _IOW(KVMIO,  0xa7, struct kvm_xcrs)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index f7fc2e97a6..de9da38e42 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1528,6 +1528,10 @@ typedef struct CPUX86State {
     uint64_t opmask_regs[NB_OPMASK_REGS];
     YMMReg zmmh_regs[CPU_NB_REGS];
     ZMMReg hi16_zmm_regs[CPU_NB_REGS];
+#ifdef TARGET_X86_64
+    uint8_t xtilecfg[64];
+    uint8_t xtiledata[8192];
+#endif
 
     /* sysenter registers */
     uint32_t sysenter_cs;
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 25d26a15f8..5f931fbbc6 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -122,6 +122,7 @@ static uint32_t num_architectural_pmu_gp_counters;
 static uint32_t num_architectural_pmu_fixed_counters;
 
 static int has_xsave;
+static int has_xsave2;
 static int has_xcrs;
 static int has_pit_state2;
 static int has_sregs2;
@@ -1571,6 +1572,26 @@ static Error *invtsc_mig_blocker;
 
 #define KVM_MAX_CPUID_ENTRIES  100
 
+static void kvm_init_xsave(CPUX86State *env)
+{
+    if (has_xsave2) {
+        env->xsave_buf_len = QEMU_ALIGN_UP(has_xsave2, 4096);
+    } else if (has_xsave) {
+        env->xsave_buf_len = sizeof(struct kvm_xsave);
+    } else {
+        return;
+    }
+
+    env->xsave_buf = qemu_memalign(4096, env->xsave_buf_len);
+    memset(env->xsave_buf, 0, env->xsave_buf_len);
+     /*
+      * The allocated storage must be large enough for all of the
+      * possible XSAVE state components.
+      */
+    assert(kvm_arch_get_supported_cpuid(kvm_state, 0xd, 0, R_ECX) <=
+           env->xsave_buf_len);
+}
+
 int kvm_arch_init_vcpu(CPUState *cs)
 {
     struct {
@@ -1600,6 +1621,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
 
     cpuid_i = 0;
 
+    has_xsave2 = kvm_check_extension(cs->kvm_state, KVM_CAP_XSAVE2);
+
     r = kvm_arch_set_tsc_khz(cs);
     if (r < 0) {
         return r;
@@ -1988,19 +2011,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
     if (r) {
         goto fail;
     }
-
-    if (has_xsave) {
-        env->xsave_buf_len = sizeof(struct kvm_xsave);
-        env->xsave_buf = qemu_memalign(4096, env->xsave_buf_len);
-        memset(env->xsave_buf, 0, env->xsave_buf_len);
-
-        /*
-         * The allocated storage must be large enough for all of the
-         * possible XSAVE state components.
-         */
-        assert(kvm_arch_get_supported_cpuid(kvm_state, 0xd, 0, R_ECX)
-               <= env->xsave_buf_len);
-    }
+    kvm_init_xsave(env);
 
     max_nested_state_len = kvm_max_nested_state_length();
     if (max_nested_state_len > 0) {
@@ -3304,13 +3315,14 @@ static int kvm_get_xsave(X86CPU *cpu)
 {
     CPUX86State *env = &cpu->env;
     void *xsave = env->xsave_buf;
-    int ret;
+    int type, ret;
 
     if (!has_xsave) {
         return kvm_get_fpu(cpu);
     }
 
-    ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_XSAVE, xsave);
+    type = has_xsave2 ? KVM_GET_XSAVE2 : KVM_GET_XSAVE;
+    ret = kvm_vcpu_ioctl(CPU(cpu), type, xsave);
     if (ret < 0) {
         return ret;
     }
diff --git a/target/i386/xsave_helper.c b/target/i386/xsave_helper.c
index ac61a96344..c3d2c9e43c 100644
--- a/target/i386/xsave_helper.c
+++ b/target/i386/xsave_helper.c
@@ -5,6 +5,7 @@
 #include "qemu/osdep.h"
 
 #include "cpu.h"
+#include <asm/kvm.h>
 
 void x86_cpu_xsave_all_areas(X86CPU *cpu, void *buf, uint32_t buflen)
 {
@@ -126,6 +127,23 @@ void x86_cpu_xsave_all_areas(X86CPU *cpu, void *buf, uint32_t buflen)
 
         memcpy(pkru, &env->pkru, sizeof(env->pkru));
     }
+
+    e = &x86_ext_save_areas[XSTATE_XTILE_CFG_BIT];
+    if (e->size && e->offset) {
+        XSaveXTILECFG *tilecfg = buf + e->offset;
+
+        memcpy(tilecfg, &env->xtilecfg, sizeof(env->xtilecfg));
+    }
+
+    if (buflen > sizeof(struct kvm_xsave)) {
+        e = &x86_ext_save_areas[XSTATE_XTILE_DATA_BIT];
+
+        if (e->size && e->offset) {
+            XSaveXTILEDATA *tiledata = buf + e->offset;
+
+            memcpy(tiledata, &env->xtiledata, sizeof(env->xtiledata));
+        }
+    }
 #endif
 }
 
@@ -247,5 +265,22 @@ void x86_cpu_xrstor_all_areas(X86CPU *cpu, const void *buf, uint32_t buflen)
         pkru = buf + e->offset;
         memcpy(&env->pkru, pkru, sizeof(env->pkru));
     }
+
+    e = &x86_ext_save_areas[XSTATE_XTILE_CFG_BIT];
+    if (e->size && e->offset) {
+        const XSaveXTILECFG *tilecfg = buf + e->offset;
+
+        memcpy(&env->xtilecfg, tilecfg, sizeof(env->xtilecfg));
+    }
+
+    if (buflen > sizeof(struct kvm_xsave)) {
+        e = &x86_ext_save_areas[XSTATE_XTILE_DATA_BIT];
+
+        if (e->size && e->offset) {
+            const XSaveXTILEDATA *tiledata = buf + e->offset;
+
+            memcpy(&env->xtiledata, tiledata, sizeof(env->xtiledata));
+        }
+    }
 #endif
 }


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 7/7] x86: Support XFD and AMX xsave data migration
  2022-01-24  7:55 [PATCH 0/7] AMX support in Qemu Yang Zhong
                   ` (5 preceding siblings ...)
  2022-01-24  7:55 ` [PATCH 6/7] x86: add support for KVM_CAP_XSAVE2 and AMX state migration Yang Zhong
@ 2022-01-24  7:55 ` Yang Zhong
  2022-01-24 10:17 ` [PATCH 0/7] AMX support in Qemu Paolo Bonzini
  7 siblings, 0 replies; 16+ messages in thread
From: Yang Zhong @ 2022-01-24  7:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: yang.zhong, kevin.tian, seanjc, jing2.liu, wei.w.wang,
	guang.zeng, pbonzini

From: Zeng Guang <guang.zeng@intel.com>

XFD(eXtended Feature Disable) allows to enable a
feature on xsave state while preventing specific
user threads from using the feature.

Support save and restore XFD MSRs if CPUID.D.1.EAX[4]
enumerate to be valid. Likewise migrate the MSRs and
related xsave state necessarily.

Signed-off-by: Zeng Guang <guang.zeng@intel.com>
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
Signed-off-by: Yang Zhong <yang.zhong@intel.com>
---
 target/i386/cpu.h     |  9 +++++++++
 target/i386/kvm/kvm.c | 18 ++++++++++++++++++
 target/i386/machine.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 69 insertions(+)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index de9da38e42..509c16323a 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -505,6 +505,9 @@ typedef enum X86Seg {
 
 #define MSR_VM_HSAVE_PA                 0xc0010117
 
+#define MSR_IA32_XFD                    0x000001c4
+#define MSR_IA32_XFD_ERR                0x000001c5
+
 #define MSR_IA32_BNDCFGS                0x00000d90
 #define MSR_IA32_XSS                    0x00000da0
 #define MSR_IA32_UMWAIT_CONTROL         0xe1
@@ -873,6 +876,8 @@ typedef uint64_t FeatureWordArray[FEATURE_WORDS];
 #define CPUID_7_1_EAX_AVX_VNNI          (1U << 4)
 /* AVX512 BFloat16 Instruction */
 #define CPUID_7_1_EAX_AVX512_BF16       (1U << 5)
+/* XFD Extend Feature Disabled */
+#define CPUID_D_1_EAX_XFD               (1U << 4)
 
 /* Packets which contain IP payload have LIP values */
 #define CPUID_14_0_ECX_LIP              (1U << 31)
@@ -1617,6 +1622,10 @@ typedef struct CPUX86State {
     uint64_t msr_rtit_cr3_match;
     uint64_t msr_rtit_addrs[MAX_RTIT_ADDRS];
 
+    /* Per-VCPU XFD MSRs */
+    uint64_t msr_xfd;
+    uint64_t msr_xfd_err;
+
     /* exception/interrupt handling */
     int error_code;
     int exception_is_int;
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 5f931fbbc6..8dbda2420d 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -3260,6 +3260,13 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
                               env->msr_ia32_sgxlepubkeyhash[3]);
         }
 
+        if (env->features[FEAT_XSAVE] & CPUID_D_1_EAX_XFD) {
+            kvm_msr_entry_add(cpu, MSR_IA32_XFD,
+                              env->msr_xfd);
+            kvm_msr_entry_add(cpu, MSR_IA32_XFD_ERR,
+                              env->msr_xfd_err);
+        }
+
         /* Note: MSR_IA32_FEATURE_CONTROL is written separately, see
          *       kvm_put_msr_feature_control. */
     }
@@ -3652,6 +3659,11 @@ static int kvm_get_msrs(X86CPU *cpu)
         kvm_msr_entry_add(cpu, MSR_IA32_SGXLEPUBKEYHASH3, 0);
     }
 
+    if (env->features[FEAT_XSAVE] & CPUID_D_1_EAX_XFD) {
+        kvm_msr_entry_add(cpu, MSR_IA32_XFD, 0);
+        kvm_msr_entry_add(cpu, MSR_IA32_XFD_ERR, 0);
+    }
+
     ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MSRS, cpu->kvm_msr_buf);
     if (ret < 0) {
         return ret;
@@ -3948,6 +3960,12 @@ static int kvm_get_msrs(X86CPU *cpu)
             env->msr_ia32_sgxlepubkeyhash[index - MSR_IA32_SGXLEPUBKEYHASH0] =
                            msrs[i].data;
             break;
+        case MSR_IA32_XFD:
+            env->msr_xfd = msrs[i].data;
+            break;
+        case MSR_IA32_XFD_ERR:
+            env->msr_xfd_err = msrs[i].data;
+            break;
         }
     }
 
diff --git a/target/i386/machine.c b/target/i386/machine.c
index 6202f47793..1f9d0c46f1 100644
--- a/target/i386/machine.c
+++ b/target/i386/machine.c
@@ -1483,6 +1483,46 @@ static const VMStateDescription vmstate_pdptrs = {
     }
 };
 
+static bool xfd_msrs_needed(void *opaque)
+{
+    X86CPU *cpu = opaque;
+    CPUX86State *env = &cpu->env;
+
+    return !!(env->features[FEAT_XSAVE] & CPUID_D_1_EAX_XFD);
+}
+
+static const VMStateDescription vmstate_msr_xfd = {
+    .name = "cpu/msr_xfd",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = xfd_msrs_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT64(env.msr_xfd, X86CPU),
+        VMSTATE_UINT64(env.msr_xfd_err, X86CPU),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static bool amx_xtile_needed(void *opaque)
+{
+    X86CPU *cpu = opaque;
+    CPUX86State *env = &cpu->env;
+
+    return !!(env->features[FEAT_7_0_EDX] & CPUID_7_0_EDX_AMX_TILE);
+}
+
+static const VMStateDescription vmstate_amx_xtile = {
+    .name = "cpu/intel_amx_xtile",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = amx_xtile_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT8_ARRAY(env.xtilecfg, X86CPU, 64),
+        VMSTATE_UINT8_ARRAY(env.xtiledata, X86CPU, 8192),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 const VMStateDescription vmstate_x86_cpu = {
     .name = "cpu",
     .version_id = 12,
@@ -1622,6 +1662,8 @@ const VMStateDescription vmstate_x86_cpu = {
         &vmstate_msr_tsx_ctrl,
         &vmstate_msr_intel_sgx,
         &vmstate_pdptrs,
+        &vmstate_msr_xfd,
+        &vmstate_amx_xtile,
         NULL
     }
 };


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/7] x86: Add AMX CPUIDs enumeration
  2022-01-24  7:55 ` [PATCH 5/7] x86: Add AMX CPUIDs enumeration Yang Zhong
@ 2022-01-24 10:13   ` Paolo Bonzini
  2022-01-27 13:46     ` Yang Zhong
  0 siblings, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2022-01-24 10:13 UTC (permalink / raw)
  To: Yang Zhong, qemu-devel
  Cc: seanjc, kevin.tian, jing2.liu, wei.w.wang, guang.zeng

On 1/24/22 08:55, Yang Zhong wrote:
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index caf1388d8b..25d26a15f8 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -1765,7 +1765,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
>                   c = &cpuid_data.entries[cpuid_i++];
>               }
>               break;
> -        case 0x14: {
> +        case 0x14:
> +        case 0x1d: {

Should this include 0x1e as well?

Paolo

>               uint32_t times;
>   
>               c->function = i;



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 6/7] x86: add support for KVM_CAP_XSAVE2 and AMX state migration
  2022-01-24  7:55 ` [PATCH 6/7] x86: add support for KVM_CAP_XSAVE2 and AMX state migration Yang Zhong
@ 2022-01-24 10:15   ` Paolo Bonzini
  2022-01-27 13:47     ` Yang Zhong
  0 siblings, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2022-01-24 10:15 UTC (permalink / raw)
  To: Yang Zhong, qemu-devel
  Cc: seanjc, kevin.tian, jing2.liu, wei.w.wang, guang.zeng

On 1/24/22 08:55, Yang Zhong wrote:
> 
> +    if (buflen > sizeof(struct kvm_xsave)) {
> +        e = &x86_ext_save_areas[XSTATE_XTILE_DATA_BIT];
> +
> +        if (e->size && e->offset) {
> +            const XSaveXTILEDATA *tiledata = buf + e->offset;
> +
> +            memcpy(&env->xtiledata, tiledata, sizeof(env->xtiledata));
> +        }
> +    }

Slightly nicer:

     e = &x86_ext_save_areas[XSTATE_XTILE_DATA_BIT];
     if (e->size && e->offset && buflen >= e->size + e->offset) {
         ...
     }

Same for xsave.

Paolo


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/7] x86: Grant AMX permission for guest
  2022-01-24  7:55 ` [PATCH 3/7] x86: Grant AMX permission for guest Yang Zhong
@ 2022-01-24 10:16   ` Paolo Bonzini
  2022-01-27 13:45     ` Yang Zhong
  0 siblings, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2022-01-24 10:16 UTC (permalink / raw)
  To: Yang Zhong, qemu-devel
  Cc: seanjc, kevin.tian, jing2.liu, wei.w.wang, guang.zeng

On 1/24/22 08:55, Yang Zhong wrote:
> Kernel allocates 4K xstate buffer by default. For XSAVE features
> which require large state component (e.g. AMX), Linux kernel
> dynamically expands the xstate buffer only after the process has
> acquired the necessary permissions. Those are called dynamically-
> enabled XSAVE features (or dynamic xfeatures).
> 
> There are separate permissions for native tasks and guests.
> 
> Qemu should request the guest permissions for dynamic xfeatures
> which will be exposed to the guest. This only needs to be done
> once before the first vcpu is created.
> 
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Yang Zhong <yang.zhong@intel.com>
> Signed-off-by: Jing Liu <jing2.liu@intel.com>
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> ---
>   target/i386/cpu.h         |  7 +++++++
>   target/i386/cpu.c         | 31 +++++++++++++++++++++++++++++++
>   target/i386/kvm/kvm-cpu.c | 12 ++++++------
>   target/i386/kvm/kvm.c     |  6 ++++++
>   4 files changed, 50 insertions(+), 6 deletions(-)
> 
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 06d2d6bccf..d4ad0f56bd 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -549,6 +549,13 @@ typedef enum X86Seg {
>   #define XSTATE_ZMM_Hi256_MASK           (1ULL << XSTATE_ZMM_Hi256_BIT)
>   #define XSTATE_Hi16_ZMM_MASK            (1ULL << XSTATE_Hi16_ZMM_BIT)
>   #define XSTATE_PKRU_MASK                (1ULL << XSTATE_PKRU_BIT)
> +#define XSTATE_XTILE_CFG_MASK           (1ULL << XSTATE_XTILE_CFG_BIT)
> +#define XSTATE_XTILE_DATA_MASK          (1ULL << XSTATE_XTILE_DATA_BIT)
> +#define XFEATURE_XTILE_MASK             (XSTATE_XTILE_CFG_MASK \
> +                                         | XSTATE_XTILE_DATA_MASK)
> +
> +#define ARCH_GET_XCOMP_GUEST_PERM       0x1024
> +#define ARCH_REQ_XCOMP_GUEST_PERM       0x1025
>   
>   #define ESA_FEATURE_ALIGN64_BIT         1
>   
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 3390820745..29b0348c25 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -43,6 +43,10 @@
>   #include "disas/capstone.h"
>   #include "cpu-internal.h"
>   
> +#include <sys/syscall.h>
> +
> +bool request_perm;
> +
>   /* Helpers for building CPUID[2] descriptors: */
>   
>   struct CPUID2CacheDescriptorInfo {
> @@ -6000,6 +6004,27 @@ static void x86_cpu_adjust_feat_level(X86CPU *cpu, FeatureWord w)
>       }
>   }
>   
> +static void kvm_request_xsave_components(X86CPU *cpu, uint32_t bit)
> +{
> +    KVMState *s = CPU(cpu)->kvm_state;
> +
> +    long rc = syscall(SYS_arch_prctl, ARCH_REQ_XCOMP_GUEST_PERM,
> +                      bit);
> +    if (rc) {
> +        /*
> +         * The older kernel version(<5.15) can't support
> +         * ARCH_REQ_XCOMP_GUEST_PERM and directly return.
> +         */
> +        return;
> +    }
> +
> +    rc = kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX);
> +    if (!(rc & XFEATURE_XTILE_MASK)) {
> +        error_report("get cpuid failure and rc=0x%lx", rc);
> +        exit(EXIT_FAILURE);
> +    }
> +}
> +
>   /* Calculate XSAVE components based on the configured CPU feature flags */
>   static void x86_cpu_enable_xsave_components(X86CPU *cpu)
>   {
> @@ -6021,6 +6046,12 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu)
>           }
>       }
>   
> +    /* Only request permission from fisrt vcpu. */
> +    if (kvm_enabled() && !request_perm) {
> +        kvm_request_xsave_components(cpu, XSTATE_XTILE_DATA_BIT);
> +        request_perm = true;
> +    }

You should pass "mask" here, or "mask & XSTATE_DYNAMIC_MASK" so that the 
components are only requested if necessary.

>       env->features[FEAT_XSAVE_COMP_LO] = mask;
>       env->features[FEAT_XSAVE_COMP_HI] = mask >> 32;
>   }
> diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
> index 033ca011ea..5ab6a0b9d2 100644
> --- a/target/i386/kvm/kvm-cpu.c
> +++ b/target/i386/kvm/kvm-cpu.c
> @@ -84,7 +84,7 @@ static void kvm_cpu_max_instance_init(X86CPU *cpu)
>   static void kvm_cpu_xsave_init(void)
>   {
>       static bool first = true;
> -    KVMState *s = kvm_state;
> +    uint32_t eax, ebx, ecx, edx;
>       int i;
>   
>       if (!first) {
> @@ -100,13 +100,13 @@ static void kvm_cpu_xsave_init(void)
>           ExtSaveArea *esa = &x86_ext_save_areas[i];
>   
>           if (esa->size) {
> -            int sz = kvm_arch_get_supported_cpuid(s, 0xd, i, R_EAX);
> -            if (sz != 0) {
> -                assert(esa->size == sz);
> -                esa->offset = kvm_arch_get_supported_cpuid(s, 0xd, i, R_EBX);
> +            host_cpuid(0xd, i, &eax, &ebx, &ecx, &edx);
> +            if (eax != 0) {
> +                assert(esa->size == eax);
> +                esa->offset = ebx;
>               }
>   
> -            esa->ecx = kvm_arch_get_supported_cpuid(s, 0xd, i, R_ECX);
> +            esa->ecx = ecx;

I think esa->ecx should be assigned inside the "if".  This is also true 
in patch 1.

>           }
>       }
>   }
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index 2c8feb4a6f..caf1388d8b 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -405,6 +405,12 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function,
>           if (!has_msr_arch_capabs) {
>               ret &= ~CPUID_7_0_EDX_ARCH_CAPABILITIES;
>           }
> +    } else if (function == 0xd && index == 0 && reg == R_EAX) {
> +        /*
> +         * We can set the AMX XTILE DATA flag, even if KVM does not
> +         * return it on GET_SUPPORTED_CPUID.
> +         */
> +        ret |= XSTATE_XTILE_DATA_MASK;
>       } else if (function == 0x80000001 && reg == R_ECX) {
>           /*
>            * It's safe to enable TOPOEXT even if it's not returned by
> 

Instead of setting XSTATE_XTILE_DATA_MASK blindly, you need something 
like arch_prctl(ARCH_GET_XCOMP_GUEST_SUPP).  However, this arch_prctl 
only exists in the bare-metal version ARCH_GET_XCOMP_SUPP, and it would 
need access to the variable supported_xcr0 that KVM exports.  So I think 
it's better to implement it as a new KVM_CHECK_EXTENSION value instead 
of a prctl.

Paolo


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/7] AMX support in Qemu
  2022-01-24  7:55 [PATCH 0/7] AMX support in Qemu Yang Zhong
                   ` (6 preceding siblings ...)
  2022-01-24  7:55 ` [PATCH 7/7] x86: Support XFD and AMX xsave data migration Yang Zhong
@ 2022-01-24 10:17 ` Paolo Bonzini
  7 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2022-01-24 10:17 UTC (permalink / raw)
  To: Yang Zhong, qemu-devel
  Cc: seanjc, kevin.tian, jing2.liu, wei.w.wang, guang.zeng

On 1/24/22 08:55, Yang Zhong wrote:
> Intel introduces Advanced Matrix Extensions (AMX) [1] feature that
> consists of configurable two-dimensional "TILE" registers and new
> accelerator instructions that operate on them. TMUL (Tile matrix
> MULtiply) is the first accelerator instruction set to use the new
> registers.
> 
> Since AMX KVM patches have been merged into Linux release, this series
> is based on latest Linux release.
> 
> According to the KVM design, the userspace VMM (e.g. Qemu) is expected
> to request guest permission for the dynamically-enabled XSAVE features
> only once when the first vCPU is created, and KVM checks guest permission
> in KVM_SET_CPUID2.
> 
> Intel AMX is XSAVE supported and XSAVE enabled. Those extended features
> has large state while current kvm_xsave only allows 4KB. The AMX KVM has
> extended struct kvm_xsave to meet this requirenment and added one extra
> KVM_GET_XSAVE2 ioctl to handle extended features. From our test, the AMX
> live migration work well.
> 
> Notice: This version still includes some definitions in the linux-headers,
> once Qemu sync those linux-headers, I will remove those definitions. So
> please ignore those changes.

Yes, no problem with that.

I think the KVM API is insufficient and needs a small but important 
extra feature, equivalent to ARCH_GET_XCOMP_SUPP.  We can implement that 
easily in 5.17 though.

Paolo

> [1] Intel Architecture Instruction Set Extension Programming Reference
>      https://software.intel.com/content/dam/develop/external/us/en/documents/\
>      architecture-instruction-set-extensions-programming-reference.pdf



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/7] x86: Grant AMX permission for guest
  2022-01-24 10:16   ` Paolo Bonzini
@ 2022-01-27 13:45     ` Yang Zhong
  2022-01-28 11:09       ` Paolo Bonzini
  0 siblings, 1 reply; 16+ messages in thread
From: Yang Zhong @ 2022-01-27 13:45 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: yang.zhong, kevin.tian, seanjc, jing2.liu, qemu-devel,
	wei.w.wang, guang.zeng

On Mon, Jan 24, 2022 at 11:16:36AM +0100, Paolo Bonzini wrote:
> On 1/24/22 08:55, Yang Zhong wrote:
> >Kernel allocates 4K xstate buffer by default. For XSAVE features
> >which require large state component (e.g. AMX), Linux kernel
> >dynamically expands the xstate buffer only after the process has
> >acquired the necessary permissions. Those are called dynamically-
> >enabled XSAVE features (or dynamic xfeatures).
> >
> >There are separate permissions for native tasks and guests.
> >
> >Qemu should request the guest permissions for dynamic xfeatures
> >which will be exposed to the guest. This only needs to be done
> >once before the first vcpu is created.
> >
> >Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> >Signed-off-by: Yang Zhong <yang.zhong@intel.com>
> >Signed-off-by: Jing Liu <jing2.liu@intel.com>
> >Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> >---
> >  target/i386/cpu.h         |  7 +++++++
> >  target/i386/cpu.c         | 31 +++++++++++++++++++++++++++++++
> >  target/i386/kvm/kvm-cpu.c | 12 ++++++------
> >  target/i386/kvm/kvm.c     |  6 ++++++
> >  4 files changed, 50 insertions(+), 6 deletions(-)
> >
> >diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> >index 06d2d6bccf..d4ad0f56bd 100644
> >--- a/target/i386/cpu.h
> >+++ b/target/i386/cpu.h
> >@@ -549,6 +549,13 @@ typedef enum X86Seg {
> >  #define XSTATE_ZMM_Hi256_MASK           (1ULL << XSTATE_ZMM_Hi256_BIT)
> >  #define XSTATE_Hi16_ZMM_MASK            (1ULL << XSTATE_Hi16_ZMM_BIT)
> >  #define XSTATE_PKRU_MASK                (1ULL << XSTATE_PKRU_BIT)
> >+#define XSTATE_XTILE_CFG_MASK           (1ULL << XSTATE_XTILE_CFG_BIT)
> >+#define XSTATE_XTILE_DATA_MASK          (1ULL << XSTATE_XTILE_DATA_BIT)
> >+#define XFEATURE_XTILE_MASK             (XSTATE_XTILE_CFG_MASK \
> >+                                         | XSTATE_XTILE_DATA_MASK)
> >+
> >+#define ARCH_GET_XCOMP_GUEST_PERM       0x1024
> >+#define ARCH_REQ_XCOMP_GUEST_PERM       0x1025
> >  #define ESA_FEATURE_ALIGN64_BIT         1
> >diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> >index 3390820745..29b0348c25 100644
> >--- a/target/i386/cpu.c
> >+++ b/target/i386/cpu.c
> >@@ -43,6 +43,10 @@
> >  #include "disas/capstone.h"
> >  #include "cpu-internal.h"
> >+#include <sys/syscall.h>
> >+
> >+bool request_perm;
> >+
> >  /* Helpers for building CPUID[2] descriptors: */
> >  struct CPUID2CacheDescriptorInfo {
> >@@ -6000,6 +6004,27 @@ static void x86_cpu_adjust_feat_level(X86CPU *cpu, FeatureWord w)
> >      }
> >  }
> >+static void kvm_request_xsave_components(X86CPU *cpu, uint32_t bit)
> >+{
> >+    KVMState *s = CPU(cpu)->kvm_state;
> >+
> >+    long rc = syscall(SYS_arch_prctl, ARCH_REQ_XCOMP_GUEST_PERM,
> >+                      bit);
> >+    if (rc) {
> >+        /*
> >+         * The older kernel version(<5.15) can't support
> >+         * ARCH_REQ_XCOMP_GUEST_PERM and directly return.
> >+         */
> >+        return;
> >+    }
> >+
> >+    rc = kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX);
> >+    if (!(rc & XFEATURE_XTILE_MASK)) {
> >+        error_report("get cpuid failure and rc=0x%lx", rc);
> >+        exit(EXIT_FAILURE);
> >+    }
> >+}
> >+
> >  /* Calculate XSAVE components based on the configured CPU feature flags */
> >  static void x86_cpu_enable_xsave_components(X86CPU *cpu)
> >  {
> >@@ -6021,6 +6046,12 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu)
> >          }
> >      }
> >+    /* Only request permission from fisrt vcpu. */
> >+    if (kvm_enabled() && !request_perm) {
> >+        kvm_request_xsave_components(cpu, XSTATE_XTILE_DATA_BIT);
> >+        request_perm = true;
> >+    }
> 
> You should pass "mask" here, or "mask & XSTATE_DYNAMIC_MASK" so that
> the components are only requested if necessary.

  Thanks, I will pass "mask" here, which can make kvm_request_xsave_components()
  reused in the future.

  Yang 

> 
> >      env->features[FEAT_XSAVE_COMP_LO] = mask;
> >      env->features[FEAT_XSAVE_COMP_HI] = mask >> 32;
> >  }
> >diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
> >index 033ca011ea..5ab6a0b9d2 100644
> >--- a/target/i386/kvm/kvm-cpu.c
> >+++ b/target/i386/kvm/kvm-cpu.c
> >@@ -84,7 +84,7 @@ static void kvm_cpu_max_instance_init(X86CPU *cpu)
> >  static void kvm_cpu_xsave_init(void)
> >  {
> >      static bool first = true;
> >-    KVMState *s = kvm_state;
> >+    uint32_t eax, ebx, ecx, edx;
> >      int i;
> >      if (!first) {
> >@@ -100,13 +100,13 @@ static void kvm_cpu_xsave_init(void)
> >          ExtSaveArea *esa = &x86_ext_save_areas[i];
> >          if (esa->size) {
> >-            int sz = kvm_arch_get_supported_cpuid(s, 0xd, i, R_EAX);
> >-            if (sz != 0) {
> >-                assert(esa->size == sz);
> >-                esa->offset = kvm_arch_get_supported_cpuid(s, 0xd, i, R_EBX);
> >+            host_cpuid(0xd, i, &eax, &ebx, &ecx, &edx);
> >+            if (eax != 0) {
> >+                assert(esa->size == eax);
> >+                esa->offset = ebx;
> >              }
> >-            esa->ecx = kvm_arch_get_supported_cpuid(s, 0xd, i, R_ECX);
> >+            esa->ecx = ecx;
> 
> I think esa->ecx should be assigned inside the "if".  This is also
> true in patch 1.

  Yes, you are right, thanks!
  
  Yang

> 
> >          }
> >      }
> >  }
> >diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> >index 2c8feb4a6f..caf1388d8b 100644
> >--- a/target/i386/kvm/kvm.c
> >+++ b/target/i386/kvm/kvm.c
> >@@ -405,6 +405,12 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function,
> >          if (!has_msr_arch_capabs) {
> >              ret &= ~CPUID_7_0_EDX_ARCH_CAPABILITIES;
> >          }
> >+    } else if (function == 0xd && index == 0 && reg == R_EAX) {
> >+        /*
> >+         * We can set the AMX XTILE DATA flag, even if KVM does not
> >+         * return it on GET_SUPPORTED_CPUID.
> >+         */
> >+        ret |= XSTATE_XTILE_DATA_MASK;
> >      } else if (function == 0x80000001 && reg == R_ECX) {
> >          /*
> >           * It's safe to enable TOPOEXT even if it's not returned by
> >
> 
> Instead of setting XSTATE_XTILE_DATA_MASK blindly, you need
> something like arch_prctl(ARCH_GET_XCOMP_GUEST_SUPP).  However, this
> arch_prctl only exists in the bare-metal version
> ARCH_GET_XCOMP_SUPP, and it would need access to the variable
> supported_xcr0 that KVM exports.  So I think it's better to
> implement it as a new KVM_CHECK_EXTENSION value instead of a prctl.
> 
  
  Thanks Paolo, from your below KVM changes:
  https://lore.kernel.org/kvm/20220126152210.3044876-3-pbonzini@redhat.com/T/#m7bf9a03c47c29d21deb78604bc290a45aa5e98f5

  So the changes in kvm_arch_get_supported_cpuid() like below? 
  +    } else if (function == 0xd && index == 0 && reg == R_EAX) {
  +	struct kvm_device_attr attr = {
  +		.group = 0,
  +		.attr = KVM_X86_XCOMP_GUEST_SUPP,
  +		.addr = (unsigned long) &bitmask
  +	};
  +
  +	kvm_fd = open_kvm_dev_path_or_exit();
  +	rc = ioctl(kvm_fd, KVM_GET_DEVICE_ATTR, &attr);
  +	close(kvm_fd);
  +     ret = bitmask;
  +   } 

  This can get supported_xcr0 from KVM side.

  So in the kvm_request_xsave_components(), we can do below steps?
 
  +    /* Check supported_xr0 firstly */ 
  +    rc = kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX);
  +    if (!(rc & XFEATURE_XTILE_MASK)) {
  +        error_report("host xcr0 can't support AMX xdata and rc=0x%lx", rc);
  +        exit(EXIT_FAILURE);
  +    }

  +   /* request amx permission */
  +   syscall(ARCH_REQ_XCOMP_GUEST_PERM, xdata_bit);


  +   /* check amx permission */
  +   syscall(ARCH_GET_XCOMP_GUEST_PERM);
 
  Please help check those steps, thanks!

  Yang 

> Paolo


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/7] x86: Add AMX CPUIDs enumeration
  2022-01-24 10:13   ` Paolo Bonzini
@ 2022-01-27 13:46     ` Yang Zhong
  0 siblings, 0 replies; 16+ messages in thread
From: Yang Zhong @ 2022-01-27 13:46 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: yang.zhong, kevin.tian, seanjc, jing2.liu, qemu-devel,
	wei.w.wang, guang.zeng

On Mon, Jan 24, 2022 at 11:13:07AM +0100, Paolo Bonzini wrote:
> On 1/24/22 08:55, Yang Zhong wrote:
> >diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> >index caf1388d8b..25d26a15f8 100644
> >--- a/target/i386/kvm/kvm.c
> >+++ b/target/i386/kvm/kvm.c
> >@@ -1765,7 +1765,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
> >                  c = &cpuid_data.entries[cpuid_i++];
> >              }
> >              break;
> >-        case 0x14: {
> >+        case 0x14:
> >+        case 0x1d: {
> 
> Should this include 0x1e as well?
>
  
  Thanks, we missed this.

  Yang 
 
> Paolo
> 
> >              uint32_t times;
> >              c->function = i;


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 6/7] x86: add support for KVM_CAP_XSAVE2 and AMX state migration
  2022-01-24 10:15   ` Paolo Bonzini
@ 2022-01-27 13:47     ` Yang Zhong
  0 siblings, 0 replies; 16+ messages in thread
From: Yang Zhong @ 2022-01-27 13:47 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: yang.zhong, kevin.tian, seanjc, jing2.liu, qemu-devel,
	wei.w.wang, guang.zeng

On Mon, Jan 24, 2022 at 11:15:25AM +0100, Paolo Bonzini wrote:
> On 1/24/22 08:55, Yang Zhong wrote:
> >
> >+    if (buflen > sizeof(struct kvm_xsave)) {
> >+        e = &x86_ext_save_areas[XSTATE_XTILE_DATA_BIT];
> >+
> >+        if (e->size && e->offset) {
> >+            const XSaveXTILEDATA *tiledata = buf + e->offset;
> >+
> >+            memcpy(&env->xtiledata, tiledata, sizeof(env->xtiledata));
> >+        }
> >+    }
> 
> Slightly nicer:
> 
>     e = &x86_ext_save_areas[XSTATE_XTILE_DATA_BIT];
>     if (e->size && e->offset && buflen >= e->size + e->offset) {
>         ...
>     }
> 
> Same for xsave.

     Thanks Paolo, new version will change this.

     Yang
> 
> Paolo


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/7] x86: Grant AMX permission for guest
  2022-01-27 13:45     ` Yang Zhong
@ 2022-01-28 11:09       ` Paolo Bonzini
  0 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2022-01-28 11:09 UTC (permalink / raw)
  To: Yang Zhong
  Cc: kevin.tian, seanjc, jing2.liu, qemu-devel, wei.w.wang, guang.zeng

On 1/27/22 14:45, Yang Zhong wrote:
>    
>    Thanks Paolo, from your below KVM changes:
>    https://lore.kernel.org/kvm/20220126152210.3044876-3-pbonzini@redhat.com/T/#m7bf9a03c47c29d21deb78604bc290a45aa5e98f5
> 
>    So the changes in kvm_arch_get_supported_cpuid() like below?
>    +    } else if (function == 0xd && index == 0 && reg == R_EAX) {

&& (reg == R_EAX || reg == R_EDX)

>    +	struct kvm_device_attr attr = {
>    +		.group = 0,
>    +		.attr = KVM_X86_XCOMP_GUEST_SUPP,
>    +		.addr = (unsigned long) &bitmask
>    +	};
>    +
>    +	kvm_fd = open_kvm_dev_path_or_exit();
>    +	rc = ioctl(kvm_fd, KVM_GET_DEVICE_ATTR, &attr);

Yes, you need to check if it fails though (and in that case do not 
assign).  The file descriptor is already there in QEMU, so you can use 
kvm_ioctl.

>   +     ret = bitmask;

To support R_EDX as well:

ret = (reg == R_EAX) ? bitmask : bitmask >> 32;

>    So in the kvm_request_xsave_components(), we can do below steps?
>   
>    +    /* Check supported_xr0 firstly */
>    +    rc = kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX);
>    +    if (!(rc & XFEATURE_XTILE_MASK)) {

rc = kvm_arch_get_supported_cpuid(s, 0xd, 0,
                                   (xdata_bit < 32 ? R_EAX : R_EDX));
if (!(rc & BIT(xdata_bit & 31)) {
     ...
}

Also please use warn_report and don't exit.  With e.g. "-cpu 
foo,amx,xfd,enforce" you should not reach this code at all if the host 
lacks AMX.

Thanks,

Paolo

>    +        error_report("host xcr0 can't support AMX xdata and rc=0x%lx", rc);
>    +        exit(EXIT_FAILURE);
>    +    }
> 
>    +   /* request amx permission */
>    +   syscall(ARCH_REQ_XCOMP_GUEST_PERM, xdata_bit);
> 
> 
>    +   /* check amx permission */
>    +   syscall(ARCH_GET_XCOMP_GUEST_PERM);



^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2022-01-28 11:14 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-24  7:55 [PATCH 0/7] AMX support in Qemu Yang Zhong
2022-01-24  7:55 ` [PATCH 1/7] x86: Fix the 64-byte boundary enumeration for extended state Yang Zhong
2022-01-24  7:55 ` [PATCH 2/7] x86: Add AMX XTILECFG and XTILEDATA components Yang Zhong
2022-01-24  7:55 ` [PATCH 3/7] x86: Grant AMX permission for guest Yang Zhong
2022-01-24 10:16   ` Paolo Bonzini
2022-01-27 13:45     ` Yang Zhong
2022-01-28 11:09       ` Paolo Bonzini
2022-01-24  7:55 ` [PATCH 4/7] x86: Add XFD faulting bit for state components Yang Zhong
2022-01-24  7:55 ` [PATCH 5/7] x86: Add AMX CPUIDs enumeration Yang Zhong
2022-01-24 10:13   ` Paolo Bonzini
2022-01-27 13:46     ` Yang Zhong
2022-01-24  7:55 ` [PATCH 6/7] x86: add support for KVM_CAP_XSAVE2 and AMX state migration Yang Zhong
2022-01-24 10:15   ` Paolo Bonzini
2022-01-27 13:47     ` Yang Zhong
2022-01-24  7:55 ` [PATCH 7/7] x86: Support XFD and AMX xsave data migration Yang Zhong
2022-01-24 10:17 ` [PATCH 0/7] AMX support in Qemu Paolo Bonzini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.