* [RFC PATCH 1/7] target/i386: Declare constants for XSAVE offsets
2021-05-20 14:56 [RFC PATCH 0/7] Support protection keys in an AMD EPYC-Milan VM David Edmondson
@ 2021-05-20 14:56 ` David Edmondson
2021-05-20 14:56 ` [RFC PATCH 2/7] target/i386: Use " David Edmondson
` (8 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: David Edmondson @ 2021-05-20 14:56 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, kvm, Marcelo Tosatti, Richard Henderson,
David Edmondson, Babu Moger, Paolo Bonzini
Declare and use manifest constants for the XSAVE state component
offsets.
Signed-off-by: David Edmondson <david.edmondson@oracle.com>
---
target/i386/cpu.h | 30 +++++++++++++++++++++++-------
1 file changed, 23 insertions(+), 7 deletions(-)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index e6836393f7..1fb732f366 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1305,6 +1305,22 @@ typedef struct XSavePKRU {
uint32_t padding;
} XSavePKRU;
+#define XSAVE_FCW_FSW_OFFSET 0x000
+#define XSAVE_FTW_FOP_OFFSET 0x004
+#define XSAVE_CWD_RIP_OFFSET 0x008
+#define XSAVE_CWD_RDP_OFFSET 0x010
+#define XSAVE_MXCSR_OFFSET 0x018
+#define XSAVE_ST_SPACE_OFFSET 0x020
+#define XSAVE_XMM_SPACE_OFFSET 0x0a0
+#define XSAVE_XSTATE_BV_OFFSET 0x200
+#define XSAVE_AVX_OFFSET 0x240
+#define XSAVE_BNDREG_OFFSET 0x3c0
+#define XSAVE_BNDCSR_OFFSET 0x400
+#define XSAVE_OPMASK_OFFSET 0x440
+#define XSAVE_ZMM_HI256_OFFSET 0x480
+#define XSAVE_HI16_ZMM_OFFSET 0x680
+#define XSAVE_PKRU_OFFSET 0xa80
+
typedef struct X86XSaveArea {
X86LegacyXSaveArea legacy;
X86XSaveHeader header;
@@ -1325,19 +1341,19 @@ typedef struct X86XSaveArea {
XSavePKRU pkru_state;
} X86XSaveArea;
-QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, avx_state) != 0x240);
+QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, avx_state) != XSAVE_AVX_OFFSET);
QEMU_BUILD_BUG_ON(sizeof(XSaveAVX) != 0x100);
-QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, bndreg_state) != 0x3c0);
+QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, bndreg_state) != XSAVE_BNDREG_OFFSET);
QEMU_BUILD_BUG_ON(sizeof(XSaveBNDREG) != 0x40);
-QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, bndcsr_state) != 0x400);
+QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, bndcsr_state) != XSAVE_BNDCSR_OFFSET);
QEMU_BUILD_BUG_ON(sizeof(XSaveBNDCSR) != 0x40);
-QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, opmask_state) != 0x440);
+QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, opmask_state) != XSAVE_OPMASK_OFFSET);
QEMU_BUILD_BUG_ON(sizeof(XSaveOpmask) != 0x40);
-QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, zmm_hi256_state) != 0x480);
+QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, zmm_hi256_state) != XSAVE_ZMM_HI256_OFFSET);
QEMU_BUILD_BUG_ON(sizeof(XSaveZMM_Hi256) != 0x200);
-QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, hi16_zmm_state) != 0x680);
+QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, hi16_zmm_state) != XSAVE_HI16_ZMM_OFFSET);
QEMU_BUILD_BUG_ON(sizeof(XSaveHi16_ZMM) != 0x400);
-QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, pkru_state) != 0xA80);
+QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, pkru_state) != XSAVE_PKRU_OFFSET);
QEMU_BUILD_BUG_ON(sizeof(XSavePKRU) != 0x8);
typedef enum TPRAccess {
--
2.30.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* [RFC PATCH 2/7] target/i386: Use constants for XSAVE offsets
2021-05-20 14:56 [RFC PATCH 0/7] Support protection keys in an AMD EPYC-Milan VM David Edmondson
2021-05-20 14:56 ` [RFC PATCH 1/7] target/i386: Declare constants for XSAVE offsets David Edmondson
@ 2021-05-20 14:56 ` David Edmondson
2021-05-20 14:56 ` [RFC PATCH 3/7] target/i386: Clarify the padding requirements of X86XSaveArea David Edmondson
` (7 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: David Edmondson @ 2021-05-20 14:56 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, kvm, Marcelo Tosatti, Richard Henderson,
David Edmondson, Babu Moger, Paolo Bonzini
Where existing constants for XSAVE offsets exists, use them.
Signed-off-by: David Edmondson <david.edmondson@oracle.com>
---
target/i386/kvm/kvm.c | 56 ++++++++++++++-----------------------------
1 file changed, 18 insertions(+), 38 deletions(-)
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index d972eb4705..aff0774fef 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -2397,44 +2397,24 @@ static int kvm_put_fpu(X86CPU *cpu)
return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_FPU, &fpu);
}
-#define XSAVE_FCW_FSW 0
-#define XSAVE_FTW_FOP 1
-#define XSAVE_CWD_RIP 2
-#define XSAVE_CWD_RDP 4
-#define XSAVE_MXCSR 6
-#define XSAVE_ST_SPACE 8
-#define XSAVE_XMM_SPACE 40
-#define XSAVE_XSTATE_BV 128
-#define XSAVE_YMMH_SPACE 144
-#define XSAVE_BNDREGS 240
-#define XSAVE_BNDCSR 256
-#define XSAVE_OPMASK 272
-#define XSAVE_ZMM_Hi256 288
-#define XSAVE_Hi16_ZMM 416
-#define XSAVE_PKRU 672
-
-#define XSAVE_BYTE_OFFSET(word_offset) \
- ((word_offset) * sizeof_field(struct kvm_xsave, region[0]))
-
-#define ASSERT_OFFSET(word_offset, field) \
- QEMU_BUILD_BUG_ON(XSAVE_BYTE_OFFSET(word_offset) != \
- offsetof(X86XSaveArea, field))
-
-ASSERT_OFFSET(XSAVE_FCW_FSW, legacy.fcw);
-ASSERT_OFFSET(XSAVE_FTW_FOP, legacy.ftw);
-ASSERT_OFFSET(XSAVE_CWD_RIP, legacy.fpip);
-ASSERT_OFFSET(XSAVE_CWD_RDP, legacy.fpdp);
-ASSERT_OFFSET(XSAVE_MXCSR, legacy.mxcsr);
-ASSERT_OFFSET(XSAVE_ST_SPACE, legacy.fpregs);
-ASSERT_OFFSET(XSAVE_XMM_SPACE, legacy.xmm_regs);
-ASSERT_OFFSET(XSAVE_XSTATE_BV, header.xstate_bv);
-ASSERT_OFFSET(XSAVE_YMMH_SPACE, avx_state);
-ASSERT_OFFSET(XSAVE_BNDREGS, bndreg_state);
-ASSERT_OFFSET(XSAVE_BNDCSR, bndcsr_state);
-ASSERT_OFFSET(XSAVE_OPMASK, opmask_state);
-ASSERT_OFFSET(XSAVE_ZMM_Hi256, zmm_hi256_state);
-ASSERT_OFFSET(XSAVE_Hi16_ZMM, hi16_zmm_state);
-ASSERT_OFFSET(XSAVE_PKRU, pkru_state);
+#define ASSERT_OFFSET(offset, field) \
+ QEMU_BUILD_BUG_ON(offset != offsetof(X86XSaveArea, field))
+
+ASSERT_OFFSET(XSAVE_FCW_FSW_OFFSET, legacy.fcw);
+ASSERT_OFFSET(XSAVE_FTW_FOP_OFFSET, legacy.ftw);
+ASSERT_OFFSET(XSAVE_CWD_RIP_OFFSET, legacy.fpip);
+ASSERT_OFFSET(XSAVE_CWD_RDP_OFFSET, legacy.fpdp);
+ASSERT_OFFSET(XSAVE_MXCSR_OFFSET, legacy.mxcsr);
+ASSERT_OFFSET(XSAVE_ST_SPACE_OFFSET, legacy.fpregs);
+ASSERT_OFFSET(XSAVE_XMM_SPACE_OFFSET, legacy.xmm_regs);
+ASSERT_OFFSET(XSAVE_XSTATE_BV_OFFSET, header.xstate_bv);
+ASSERT_OFFSET(XSAVE_AVX_OFFSET, avx_state);
+ASSERT_OFFSET(XSAVE_BNDREG_OFFSET, bndreg_state);
+ASSERT_OFFSET(XSAVE_BNDCSR_OFFSET, bndcsr_state);
+ASSERT_OFFSET(XSAVE_OPMASK_OFFSET, opmask_state);
+ASSERT_OFFSET(XSAVE_ZMM_HI256_OFFSET, zmm_hi256_state);
+ASSERT_OFFSET(XSAVE_HI16_ZMM_OFFSET, hi16_zmm_state);
+ASSERT_OFFSET(XSAVE_PKRU_OFFSET, pkru_state);
static int kvm_put_xsave(X86CPU *cpu)
{
--
2.30.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* [RFC PATCH 3/7] target/i386: Clarify the padding requirements of X86XSaveArea
2021-05-20 14:56 [RFC PATCH 0/7] Support protection keys in an AMD EPYC-Milan VM David Edmondson
2021-05-20 14:56 ` [RFC PATCH 1/7] target/i386: Declare constants for XSAVE offsets David Edmondson
2021-05-20 14:56 ` [RFC PATCH 2/7] target/i386: Use " David Edmondson
@ 2021-05-20 14:56 ` David Edmondson
2021-05-20 14:56 ` [RFC PATCH 4/7] target/i386: Prepare for per-vendor X86XSaveArea layout David Edmondson
` (6 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: David Edmondson @ 2021-05-20 14:56 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, kvm, Marcelo Tosatti, Richard Henderson,
David Edmondson, Babu Moger, Paolo Bonzini
Replace the hard-coded size of offsets or structure elements with
defined constants or sizeof().
Signed-off-by: David Edmondson <david.edmondson@oracle.com>
---
target/i386/cpu.h | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 1fb732f366..0bb365bddf 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1329,7 +1329,13 @@ typedef struct X86XSaveArea {
/* AVX State: */
XSaveAVX avx_state;
- uint8_t padding[960 - 576 - sizeof(XSaveAVX)];
+
+ /* Ensure that XSaveBNDREG is properly aligned. */
+ uint8_t padding[XSAVE_BNDREG_OFFSET
+ - sizeof(X86LegacyXSaveArea)
+ - sizeof(X86XSaveHeader)
+ - sizeof(XSaveAVX)];
+
/* MPX State: */
XSaveBNDREG bndreg_state;
XSaveBNDCSR bndcsr_state;
--
2.30.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* [RFC PATCH 4/7] target/i386: Prepare for per-vendor X86XSaveArea layout
2021-05-20 14:56 [RFC PATCH 0/7] Support protection keys in an AMD EPYC-Milan VM David Edmondson
` (2 preceding siblings ...)
2021-05-20 14:56 ` [RFC PATCH 3/7] target/i386: Clarify the padding requirements of X86XSaveArea David Edmondson
@ 2021-05-20 14:56 ` David Edmondson
2021-05-20 14:56 ` [RFC PATCH 5/7] target/i386: Introduce AMD X86XSaveArea sub-union David Edmondson
` (5 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: David Edmondson @ 2021-05-20 14:56 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, kvm, Marcelo Tosatti, Richard Henderson,
David Edmondson, Babu Moger, Paolo Bonzini
Move Intel specific components of X86XSaveArea into a sub-union.
Signed-off-by: David Edmondson <david.edmondson@oracle.com>
---
target/i386/cpu.c | 12 ++++----
target/i386/cpu.h | 55 +++++++++++++++++++++---------------
target/i386/kvm/kvm.c | 12 ++++----
target/i386/tcg/fpu_helper.c | 12 ++++----
target/i386/xsave_helper.c | 24 ++++++++--------
5 files changed, 63 insertions(+), 52 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index c496bfa1c2..4f481691b4 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1418,27 +1418,27 @@ static const ExtSaveArea x86_ext_save_areas[] = {
.size = sizeof(XSaveAVX) },
[XSTATE_BNDREGS_BIT] =
{ .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_MPX,
- .offset = offsetof(X86XSaveArea, bndreg_state),
+ .offset = offsetof(X86XSaveArea, intel.bndreg_state),
.size = sizeof(XSaveBNDREG) },
[XSTATE_BNDCSR_BIT] =
{ .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_MPX,
- .offset = offsetof(X86XSaveArea, bndcsr_state),
+ .offset = offsetof(X86XSaveArea, intel.bndcsr_state),
.size = sizeof(XSaveBNDCSR) },
[XSTATE_OPMASK_BIT] =
{ .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_AVX512F,
- .offset = offsetof(X86XSaveArea, opmask_state),
+ .offset = offsetof(X86XSaveArea, intel.opmask_state),
.size = sizeof(XSaveOpmask) },
[XSTATE_ZMM_Hi256_BIT] =
{ .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_AVX512F,
- .offset = offsetof(X86XSaveArea, zmm_hi256_state),
+ .offset = offsetof(X86XSaveArea, intel.zmm_hi256_state),
.size = sizeof(XSaveZMM_Hi256) },
[XSTATE_Hi16_ZMM_BIT] =
{ .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_AVX512F,
- .offset = offsetof(X86XSaveArea, hi16_zmm_state),
+ .offset = offsetof(X86XSaveArea, intel.hi16_zmm_state),
.size = sizeof(XSaveHi16_ZMM) },
[XSTATE_PKRU_BIT] =
{ .feature = FEAT_7_0_ECX, .bits = CPUID_7_0_ECX_PKU,
- .offset = offsetof(X86XSaveArea, pkru_state),
+ .offset = offsetof(X86XSaveArea, intel.pkru_state),
.size = sizeof(XSavePKRU) },
};
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 0bb365bddf..f1ce4e3008 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1330,36 +1330,47 @@ typedef struct X86XSaveArea {
/* AVX State: */
XSaveAVX avx_state;
- /* Ensure that XSaveBNDREG is properly aligned. */
- uint8_t padding[XSAVE_BNDREG_OFFSET
- - sizeof(X86LegacyXSaveArea)
- - sizeof(X86XSaveHeader)
- - sizeof(XSaveAVX)];
-
- /* MPX State: */
- XSaveBNDREG bndreg_state;
- XSaveBNDCSR bndcsr_state;
- /* AVX-512 State: */
- XSaveOpmask opmask_state;
- XSaveZMM_Hi256 zmm_hi256_state;
- XSaveHi16_ZMM hi16_zmm_state;
- /* PKRU State: */
- XSavePKRU pkru_state;
+ union {
+ struct {
+ /* Ensure that XSaveBNDREG is properly aligned. */
+ uint8_t padding[XSAVE_BNDREG_OFFSET
+ - sizeof(X86LegacyXSaveArea)
+ - sizeof(X86XSaveHeader)
+ - sizeof(XSaveAVX)];
+
+ /* MPX State: */
+ XSaveBNDREG bndreg_state;
+ XSaveBNDCSR bndcsr_state;
+ /* AVX-512 State: */
+ XSaveOpmask opmask_state;
+ XSaveZMM_Hi256 zmm_hi256_state;
+ XSaveHi16_ZMM hi16_zmm_state;
+ /* PKRU State: */
+ XSavePKRU pkru_state;
+ } intel;
+ };
} X86XSaveArea;
-QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, avx_state) != XSAVE_AVX_OFFSET);
+QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, avx_state)
+ != XSAVE_AVX_OFFSET);
QEMU_BUILD_BUG_ON(sizeof(XSaveAVX) != 0x100);
-QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, bndreg_state) != XSAVE_BNDREG_OFFSET);
+QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, intel.bndreg_state)
+ != XSAVE_BNDREG_OFFSET);
QEMU_BUILD_BUG_ON(sizeof(XSaveBNDREG) != 0x40);
-QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, bndcsr_state) != XSAVE_BNDCSR_OFFSET);
+QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, intel.bndcsr_state)
+ != XSAVE_BNDCSR_OFFSET);
QEMU_BUILD_BUG_ON(sizeof(XSaveBNDCSR) != 0x40);
-QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, opmask_state) != XSAVE_OPMASK_OFFSET);
+QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, intel.opmask_state)
+ != XSAVE_OPMASK_OFFSET);
QEMU_BUILD_BUG_ON(sizeof(XSaveOpmask) != 0x40);
-QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, zmm_hi256_state) != XSAVE_ZMM_HI256_OFFSET);
+QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, intel.zmm_hi256_state)
+ != XSAVE_ZMM_HI256_OFFSET);
QEMU_BUILD_BUG_ON(sizeof(XSaveZMM_Hi256) != 0x200);
-QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, hi16_zmm_state) != XSAVE_HI16_ZMM_OFFSET);
+QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, intel.hi16_zmm_state)
+ != XSAVE_HI16_ZMM_OFFSET);
QEMU_BUILD_BUG_ON(sizeof(XSaveHi16_ZMM) != 0x400);
-QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, pkru_state) != XSAVE_PKRU_OFFSET);
+QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, intel.pkru_state)
+ != XSAVE_PKRU_OFFSET);
QEMU_BUILD_BUG_ON(sizeof(XSavePKRU) != 0x8);
typedef enum TPRAccess {
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index aff0774fef..417776a635 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -2409,12 +2409,12 @@ ASSERT_OFFSET(XSAVE_ST_SPACE_OFFSET, legacy.fpregs);
ASSERT_OFFSET(XSAVE_XMM_SPACE_OFFSET, legacy.xmm_regs);
ASSERT_OFFSET(XSAVE_XSTATE_BV_OFFSET, header.xstate_bv);
ASSERT_OFFSET(XSAVE_AVX_OFFSET, avx_state);
-ASSERT_OFFSET(XSAVE_BNDREG_OFFSET, bndreg_state);
-ASSERT_OFFSET(XSAVE_BNDCSR_OFFSET, bndcsr_state);
-ASSERT_OFFSET(XSAVE_OPMASK_OFFSET, opmask_state);
-ASSERT_OFFSET(XSAVE_ZMM_HI256_OFFSET, zmm_hi256_state);
-ASSERT_OFFSET(XSAVE_HI16_ZMM_OFFSET, hi16_zmm_state);
-ASSERT_OFFSET(XSAVE_PKRU_OFFSET, pkru_state);
+ASSERT_OFFSET(XSAVE_BNDREG_OFFSET, intel.bndreg_state);
+ASSERT_OFFSET(XSAVE_BNDCSR_OFFSET, intel.bndcsr_state);
+ASSERT_OFFSET(XSAVE_OPMASK_OFFSET, intel.opmask_state);
+ASSERT_OFFSET(XSAVE_ZMM_HI256_OFFSET, intel.zmm_hi256_state);
+ASSERT_OFFSET(XSAVE_HI16_ZMM_OFFSET, intel.hi16_zmm_state);
+ASSERT_OFFSET(XSAVE_PKRU_OFFSET, intel.pkru_state);
static int kvm_put_xsave(X86CPU *cpu)
{
diff --git a/target/i386/tcg/fpu_helper.c b/target/i386/tcg/fpu_helper.c
index 1b30f1bb73..fba2de5b04 100644
--- a/target/i386/tcg/fpu_helper.c
+++ b/target/i386/tcg/fpu_helper.c
@@ -2637,13 +2637,13 @@ static void do_xsave(CPUX86State *env, target_ulong ptr, uint64_t rfbm,
do_xsave_sse(env, ptr, ra);
}
if (opt & XSTATE_BNDREGS_MASK) {
- do_xsave_bndregs(env, ptr + XO(bndreg_state), ra);
+ do_xsave_bndregs(env, ptr + XO(intel.bndreg_state), ra);
}
if (opt & XSTATE_BNDCSR_MASK) {
- do_xsave_bndcsr(env, ptr + XO(bndcsr_state), ra);
+ do_xsave_bndcsr(env, ptr + XO(intel.bndcsr_state), ra);
}
if (opt & XSTATE_PKRU_MASK) {
- do_xsave_pkru(env, ptr + XO(pkru_state), ra);
+ do_xsave_pkru(env, ptr + XO(intel.pkru_state), ra);
}
/* Update the XSTATE_BV field. */
@@ -2836,7 +2836,7 @@ void helper_xrstor(CPUX86State *env, target_ulong ptr, uint64_t rfbm)
}
if (rfbm & XSTATE_BNDREGS_MASK) {
if (xstate_bv & XSTATE_BNDREGS_MASK) {
- do_xrstor_bndregs(env, ptr + XO(bndreg_state), ra);
+ do_xrstor_bndregs(env, ptr + XO(intel.bndreg_state), ra);
env->hflags |= HF_MPX_IU_MASK;
} else {
memset(env->bnd_regs, 0, sizeof(env->bnd_regs));
@@ -2845,7 +2845,7 @@ void helper_xrstor(CPUX86State *env, target_ulong ptr, uint64_t rfbm)
}
if (rfbm & XSTATE_BNDCSR_MASK) {
if (xstate_bv & XSTATE_BNDCSR_MASK) {
- do_xrstor_bndcsr(env, ptr + XO(bndcsr_state), ra);
+ do_xrstor_bndcsr(env, ptr + XO(intel.bndcsr_state), ra);
} else {
memset(&env->bndcs_regs, 0, sizeof(env->bndcs_regs));
}
@@ -2854,7 +2854,7 @@ void helper_xrstor(CPUX86State *env, target_ulong ptr, uint64_t rfbm)
if (rfbm & XSTATE_PKRU_MASK) {
uint64_t old_pkru = env->pkru;
if (xstate_bv & XSTATE_PKRU_MASK) {
- do_xrstor_pkru(env, ptr + XO(pkru_state), ra);
+ do_xrstor_pkru(env, ptr + XO(intel.pkru_state), ra);
} else {
env->pkru = 0;
}
diff --git a/target/i386/xsave_helper.c b/target/i386/xsave_helper.c
index 818115e7d2..97dbab85d1 100644
--- a/target/i386/xsave_helper.c
+++ b/target/i386/xsave_helper.c
@@ -31,16 +31,16 @@ void x86_cpu_xsave_all_areas(X86CPU *cpu, X86XSaveArea *buf)
sizeof env->fpregs);
xsave->legacy.mxcsr = env->mxcsr;
xsave->header.xstate_bv = env->xstate_bv;
- memcpy(&xsave->bndreg_state.bnd_regs, env->bnd_regs,
+ memcpy(&xsave->intel.bndreg_state.bnd_regs, env->bnd_regs,
sizeof env->bnd_regs);
- xsave->bndcsr_state.bndcsr = env->bndcs_regs;
- memcpy(&xsave->opmask_state.opmask_regs, env->opmask_regs,
+ xsave->intel.bndcsr_state.bndcsr = env->bndcs_regs;
+ memcpy(&xsave->intel.opmask_state.opmask_regs, env->opmask_regs,
sizeof env->opmask_regs);
for (i = 0; i < CPU_NB_REGS; i++) {
uint8_t *xmm = xsave->legacy.xmm_regs[i];
uint8_t *ymmh = xsave->avx_state.ymmh[i];
- uint8_t *zmmh = xsave->zmm_hi256_state.zmm_hi256[i];
+ uint8_t *zmmh = xsave->intel.zmm_hi256_state.zmm_hi256[i];
stq_p(xmm, env->xmm_regs[i].ZMM_Q(0));
stq_p(xmm+8, env->xmm_regs[i].ZMM_Q(1));
stq_p(ymmh, env->xmm_regs[i].ZMM_Q(2));
@@ -52,9 +52,9 @@ void x86_cpu_xsave_all_areas(X86CPU *cpu, X86XSaveArea *buf)
}
#ifdef TARGET_X86_64
- memcpy(&xsave->hi16_zmm_state.hi16_zmm, &env->xmm_regs[16],
+ memcpy(&xsave->intel.hi16_zmm_state.hi16_zmm, &env->xmm_regs[16],
16 * sizeof env->xmm_regs[16]);
- memcpy(&xsave->pkru_state, &env->pkru, sizeof env->pkru);
+ memcpy(&xsave->intel.pkru_state, &env->pkru, sizeof env->pkru);
#endif
}
@@ -83,16 +83,16 @@ void x86_cpu_xrstor_all_areas(X86CPU *cpu, const X86XSaveArea *buf)
memcpy(env->fpregs, &xsave->legacy.fpregs,
sizeof env->fpregs);
env->xstate_bv = xsave->header.xstate_bv;
- memcpy(env->bnd_regs, &xsave->bndreg_state.bnd_regs,
+ memcpy(env->bnd_regs, &xsave->intel.bndreg_state.bnd_regs,
sizeof env->bnd_regs);
- env->bndcs_regs = xsave->bndcsr_state.bndcsr;
- memcpy(env->opmask_regs, &xsave->opmask_state.opmask_regs,
+ env->bndcs_regs = xsave->intel.bndcsr_state.bndcsr;
+ memcpy(env->opmask_regs, &xsave->intel.opmask_state.opmask_regs,
sizeof env->opmask_regs);
for (i = 0; i < CPU_NB_REGS; i++) {
const uint8_t *xmm = xsave->legacy.xmm_regs[i];
const uint8_t *ymmh = xsave->avx_state.ymmh[i];
- const uint8_t *zmmh = xsave->zmm_hi256_state.zmm_hi256[i];
+ const uint8_t *zmmh = xsave->intel.zmm_hi256_state.zmm_hi256[i];
env->xmm_regs[i].ZMM_Q(0) = ldq_p(xmm);
env->xmm_regs[i].ZMM_Q(1) = ldq_p(xmm+8);
env->xmm_regs[i].ZMM_Q(2) = ldq_p(ymmh);
@@ -104,9 +104,9 @@ void x86_cpu_xrstor_all_areas(X86CPU *cpu, const X86XSaveArea *buf)
}
#ifdef TARGET_X86_64
- memcpy(&env->xmm_regs[16], &xsave->hi16_zmm_state.hi16_zmm,
+ memcpy(&env->xmm_regs[16], &xsave->intel.hi16_zmm_state.hi16_zmm,
16 * sizeof env->xmm_regs[16]);
- memcpy(&env->pkru, &xsave->pkru_state, sizeof env->pkru);
+ memcpy(&env->pkru, &xsave->intel.pkru_state, sizeof env->pkru);
#endif
}
--
2.30.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* [RFC PATCH 5/7] target/i386: Introduce AMD X86XSaveArea sub-union
2021-05-20 14:56 [RFC PATCH 0/7] Support protection keys in an AMD EPYC-Milan VM David Edmondson
` (3 preceding siblings ...)
2021-05-20 14:56 ` [RFC PATCH 4/7] target/i386: Prepare for per-vendor X86XSaveArea layout David Edmondson
@ 2021-05-20 14:56 ` David Edmondson
2021-05-20 14:56 ` [RFC PATCH 6/7] target/i386: Adjust AMD XSAVE PKRU area offset in CPUID leaf 0xd David Edmondson
` (4 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: David Edmondson @ 2021-05-20 14:56 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, kvm, Marcelo Tosatti, Richard Henderson,
David Edmondson, Babu Moger, Paolo Bonzini
AMD stores the pkru_state at a different offset to Intel.
Signed-off-by: David Edmondson <david.edmondson@oracle.com>
---
target/i386/cpu.h | 17 +++++++++++++++--
target/i386/kvm/kvm.c | 3 ++-
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index f1ce4e3008..99f0d5d851 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1319,7 +1319,8 @@ typedef struct XSavePKRU {
#define XSAVE_OPMASK_OFFSET 0x440
#define XSAVE_ZMM_HI256_OFFSET 0x480
#define XSAVE_HI16_ZMM_OFFSET 0x680
-#define XSAVE_PKRU_OFFSET 0xa80
+#define XSAVE_INTEL_PKRU_OFFSET 0xa80
+#define XSAVE_AMD_PKRU_OFFSET 0x980
typedef struct X86XSaveArea {
X86LegacyXSaveArea legacy;
@@ -1348,6 +1349,16 @@ typedef struct X86XSaveArea {
/* PKRU State: */
XSavePKRU pkru_state;
} intel;
+ struct {
+ /* Ensure that XSavePKRU is properly aligned. */
+ uint8_t padding[XSAVE_AMD_PKRU_OFFSET
+ - sizeof(X86LegacyXSaveArea)
+ - sizeof(X86XSaveHeader)
+ - sizeof(XSaveAVX)];
+
+ /* PKRU State: */
+ XSavePKRU pkru_state;
+ } amd;
};
} X86XSaveArea;
@@ -1370,7 +1381,9 @@ QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, intel.hi16_zmm_state)
!= XSAVE_HI16_ZMM_OFFSET);
QEMU_BUILD_BUG_ON(sizeof(XSaveHi16_ZMM) != 0x400);
QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, intel.pkru_state)
- != XSAVE_PKRU_OFFSET);
+ != XSAVE_INTEL_PKRU_OFFSET);
+QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, amd.pkru_state)
+ != XSAVE_AMD_PKRU_OFFSET);
QEMU_BUILD_BUG_ON(sizeof(XSavePKRU) != 0x8);
typedef enum TPRAccess {
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 417776a635..9dd7db060d 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -2414,7 +2414,8 @@ ASSERT_OFFSET(XSAVE_BNDCSR_OFFSET, intel.bndcsr_state);
ASSERT_OFFSET(XSAVE_OPMASK_OFFSET, intel.opmask_state);
ASSERT_OFFSET(XSAVE_ZMM_HI256_OFFSET, intel.zmm_hi256_state);
ASSERT_OFFSET(XSAVE_HI16_ZMM_OFFSET, intel.hi16_zmm_state);
-ASSERT_OFFSET(XSAVE_PKRU_OFFSET, intel.pkru_state);
+ASSERT_OFFSET(XSAVE_INTEL_PKRU_OFFSET, intel.pkru_state);
+ASSERT_OFFSET(XSAVE_AMD_PKRU_OFFSET, amd.pkru_state);
static int kvm_put_xsave(X86CPU *cpu)
{
--
2.30.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* [RFC PATCH 6/7] target/i386: Adjust AMD XSAVE PKRU area offset in CPUID leaf 0xd
2021-05-20 14:56 [RFC PATCH 0/7] Support protection keys in an AMD EPYC-Milan VM David Edmondson
` (4 preceding siblings ...)
2021-05-20 14:56 ` [RFC PATCH 5/7] target/i386: Introduce AMD X86XSaveArea sub-union David Edmondson
@ 2021-05-20 14:56 ` David Edmondson
2021-05-20 14:56 ` [RFC PATCH 7/7] target/i386: Manipulate only AMD XSAVE state on AMD David Edmondson
` (3 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: David Edmondson @ 2021-05-20 14:56 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, kvm, Marcelo Tosatti, Richard Henderson,
David Edmondson, Babu Moger, Paolo Bonzini
AMD stores the pkru_state at a different offset to Intel, so update
the CPUID leaf which indicates such.
Signed-off-by: David Edmondson <david.edmondson@oracle.com>
---
target/i386/cpu.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 4f481691b4..9340a477a3 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1397,7 +1397,7 @@ typedef struct ExtSaveArea {
uint32_t offset, size;
} ExtSaveArea;
-static const ExtSaveArea x86_ext_save_areas[] = {
+static ExtSaveArea x86_ext_save_areas[] = {
[XSTATE_FP_BIT] = {
/* x87 FP state component is always enabled if XSAVE is supported */
.feature = FEAT_1_ECX, .bits = CPUID_EXT_XSAVE,
@@ -6088,6 +6088,11 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool verbose)
mark_unavailable_features(cpu, FEAT_7_0_EBX, CPUID_7_0_EBX_INTEL_PT, prefix);
}
}
+
+ if (IS_AMD_CPU(env)) {
+ x86_ext_save_areas[XSTATE_PKRU_BIT].offset =
+ offsetof(X86XSaveArea, amd.pkru_state);
+ }
}
static void x86_cpu_hyperv_realize(X86CPU *cpu)
--
2.30.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* [RFC PATCH 7/7] target/i386: Manipulate only AMD XSAVE state on AMD
2021-05-20 14:56 [RFC PATCH 0/7] Support protection keys in an AMD EPYC-Milan VM David Edmondson
` (5 preceding siblings ...)
2021-05-20 14:56 ` [RFC PATCH 6/7] target/i386: Adjust AMD XSAVE PKRU area offset in CPUID leaf 0xd David Edmondson
@ 2021-05-20 14:56 ` David Edmondson
2021-05-20 15:15 ` [RFC PATCH 0/7] Support protection keys in an AMD EPYC-Milan VM no-reply
` (2 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: David Edmondson @ 2021-05-20 14:56 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, kvm, Marcelo Tosatti, Richard Henderson,
David Edmondson, Babu Moger, Paolo Bonzini
On AMD CPUs, ensure to save/load only the relevant XSAVE state.
Signed-off-by: David Edmondson <david.edmondson@oracle.com>
---
target/i386/tcg/fpu_helper.c | 12 +++++--
target/i386/xsave_helper.c | 70 ++++++++++++++++++++++--------------
2 files changed, 54 insertions(+), 28 deletions(-)
diff --git a/target/i386/tcg/fpu_helper.c b/target/i386/tcg/fpu_helper.c
index fba2de5b04..f1d4704b34 100644
--- a/target/i386/tcg/fpu_helper.c
+++ b/target/i386/tcg/fpu_helper.c
@@ -2643,7 +2643,11 @@ static void do_xsave(CPUX86State *env, target_ulong ptr, uint64_t rfbm,
do_xsave_bndcsr(env, ptr + XO(intel.bndcsr_state), ra);
}
if (opt & XSTATE_PKRU_MASK) {
- do_xsave_pkru(env, ptr + XO(intel.pkru_state), ra);
+ if (IS_AMD_CPU(env)) {
+ do_xsave_pkru(env, ptr + XO(amd.pkru_state), ra);
+ } else {
+ do_xsave_pkru(env, ptr + XO(intel.pkru_state), ra);
+ }
}
/* Update the XSTATE_BV field. */
@@ -2854,7 +2858,11 @@ void helper_xrstor(CPUX86State *env, target_ulong ptr, uint64_t rfbm)
if (rfbm & XSTATE_PKRU_MASK) {
uint64_t old_pkru = env->pkru;
if (xstate_bv & XSTATE_PKRU_MASK) {
- do_xrstor_pkru(env, ptr + XO(intel.pkru_state), ra);
+ if (IS_AMD_CPU(env)) {
+ do_xrstor_pkru(env, ptr + XO(amd.pkru_state), ra);
+ } else {
+ do_xrstor_pkru(env, ptr + XO(intel.pkru_state), ra);
+ }
} else {
env->pkru = 0;
}
diff --git a/target/i386/xsave_helper.c b/target/i386/xsave_helper.c
index 97dbab85d1..6b4501cf29 100644
--- a/target/i386/xsave_helper.c
+++ b/target/i386/xsave_helper.c
@@ -10,6 +10,7 @@ void x86_cpu_xsave_all_areas(X86CPU *cpu, X86XSaveArea *buf)
{
CPUX86State *env = &cpu->env;
X86XSaveArea *xsave = buf;
+ const bool is_amd = IS_AMD_CPU(env);
uint16_t cwd, swd, twd;
int i;
@@ -31,30 +32,38 @@ void x86_cpu_xsave_all_areas(X86CPU *cpu, X86XSaveArea *buf)
sizeof env->fpregs);
xsave->legacy.mxcsr = env->mxcsr;
xsave->header.xstate_bv = env->xstate_bv;
- memcpy(&xsave->intel.bndreg_state.bnd_regs, env->bnd_regs,
- sizeof env->bnd_regs);
- xsave->intel.bndcsr_state.bndcsr = env->bndcs_regs;
- memcpy(&xsave->intel.opmask_state.opmask_regs, env->opmask_regs,
- sizeof env->opmask_regs);
+ if (!is_amd) {
+ memcpy(&xsave->intel.bndreg_state.bnd_regs, env->bnd_regs,
+ sizeof env->bnd_regs);
+ xsave->intel.bndcsr_state.bndcsr = env->bndcs_regs;
+ memcpy(&xsave->intel.opmask_state.opmask_regs, env->opmask_regs,
+ sizeof env->opmask_regs);
+ }
for (i = 0; i < CPU_NB_REGS; i++) {
uint8_t *xmm = xsave->legacy.xmm_regs[i];
uint8_t *ymmh = xsave->avx_state.ymmh[i];
- uint8_t *zmmh = xsave->intel.zmm_hi256_state.zmm_hi256[i];
stq_p(xmm, env->xmm_regs[i].ZMM_Q(0));
stq_p(xmm+8, env->xmm_regs[i].ZMM_Q(1));
stq_p(ymmh, env->xmm_regs[i].ZMM_Q(2));
stq_p(ymmh+8, env->xmm_regs[i].ZMM_Q(3));
- stq_p(zmmh, env->xmm_regs[i].ZMM_Q(4));
- stq_p(zmmh+8, env->xmm_regs[i].ZMM_Q(5));
- stq_p(zmmh+16, env->xmm_regs[i].ZMM_Q(6));
- stq_p(zmmh+24, env->xmm_regs[i].ZMM_Q(7));
+ if (!is_amd) {
+ uint8_t *zmmh = xsave->intel.zmm_hi256_state.zmm_hi256[i];
+ stq_p(zmmh, env->xmm_regs[i].ZMM_Q(4));
+ stq_p(zmmh+8, env->xmm_regs[i].ZMM_Q(5));
+ stq_p(zmmh+16, env->xmm_regs[i].ZMM_Q(6));
+ stq_p(zmmh+24, env->xmm_regs[i].ZMM_Q(7));
+ }
}
#ifdef TARGET_X86_64
- memcpy(&xsave->intel.hi16_zmm_state.hi16_zmm, &env->xmm_regs[16],
- 16 * sizeof env->xmm_regs[16]);
- memcpy(&xsave->intel.pkru_state, &env->pkru, sizeof env->pkru);
+ if (is_amd) {
+ memcpy(&xsave->amd.pkru_state, &env->pkru, sizeof env->pkru);
+ } else {
+ memcpy(&xsave->intel.hi16_zmm_state.hi16_zmm, &env->xmm_regs[16],
+ 16 * sizeof env->xmm_regs[16]);
+ memcpy(&xsave->intel.pkru_state, &env->pkru, sizeof env->pkru);
+ }
#endif
}
@@ -64,6 +73,7 @@ void x86_cpu_xrstor_all_areas(X86CPU *cpu, const X86XSaveArea *buf)
CPUX86State *env = &cpu->env;
const X86XSaveArea *xsave = buf;
+ const bool is_amd = IS_AMD_CPU(env);
int i;
uint16_t cwd, swd, twd;
@@ -83,30 +93,38 @@ void x86_cpu_xrstor_all_areas(X86CPU *cpu, const X86XSaveArea *buf)
memcpy(env->fpregs, &xsave->legacy.fpregs,
sizeof env->fpregs);
env->xstate_bv = xsave->header.xstate_bv;
- memcpy(env->bnd_regs, &xsave->intel.bndreg_state.bnd_regs,
- sizeof env->bnd_regs);
- env->bndcs_regs = xsave->intel.bndcsr_state.bndcsr;
- memcpy(env->opmask_regs, &xsave->intel.opmask_state.opmask_regs,
- sizeof env->opmask_regs);
+ if (!is_amd) {
+ memcpy(env->bnd_regs, &xsave->intel.bndreg_state.bnd_regs,
+ sizeof env->bnd_regs);
+ env->bndcs_regs = xsave->intel.bndcsr_state.bndcsr;
+ memcpy(env->opmask_regs, &xsave->intel.opmask_state.opmask_regs,
+ sizeof env->opmask_regs);
+ }
for (i = 0; i < CPU_NB_REGS; i++) {
const uint8_t *xmm = xsave->legacy.xmm_regs[i];
const uint8_t *ymmh = xsave->avx_state.ymmh[i];
- const uint8_t *zmmh = xsave->intel.zmm_hi256_state.zmm_hi256[i];
env->xmm_regs[i].ZMM_Q(0) = ldq_p(xmm);
env->xmm_regs[i].ZMM_Q(1) = ldq_p(xmm+8);
env->xmm_regs[i].ZMM_Q(2) = ldq_p(ymmh);
env->xmm_regs[i].ZMM_Q(3) = ldq_p(ymmh+8);
- env->xmm_regs[i].ZMM_Q(4) = ldq_p(zmmh);
- env->xmm_regs[i].ZMM_Q(5) = ldq_p(zmmh+8);
- env->xmm_regs[i].ZMM_Q(6) = ldq_p(zmmh+16);
- env->xmm_regs[i].ZMM_Q(7) = ldq_p(zmmh+24);
+ if (!is_amd) {
+ const uint8_t *zmmh = xsave->intel.zmm_hi256_state.zmm_hi256[i];
+ env->xmm_regs[i].ZMM_Q(4) = ldq_p(zmmh);
+ env->xmm_regs[i].ZMM_Q(5) = ldq_p(zmmh+8);
+ env->xmm_regs[i].ZMM_Q(6) = ldq_p(zmmh+16);
+ env->xmm_regs[i].ZMM_Q(7) = ldq_p(zmmh+24);
+ }
}
#ifdef TARGET_X86_64
- memcpy(&env->xmm_regs[16], &xsave->intel.hi16_zmm_state.hi16_zmm,
- 16 * sizeof env->xmm_regs[16]);
- memcpy(&env->pkru, &xsave->intel.pkru_state, sizeof env->pkru);
+ if (is_amd) {
+ memcpy(&env->pkru, &xsave->amd.pkru_state, sizeof env->pkru);
+ } else {
+ memcpy(&env->xmm_regs[16], &xsave->intel.hi16_zmm_state.hi16_zmm,
+ 16 * sizeof env->xmm_regs[16]);
+ memcpy(&env->pkru, &xsave->intel.pkru_state, sizeof env->pkru);
+ }
#endif
}
--
2.30.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [RFC PATCH 0/7] Support protection keys in an AMD EPYC-Milan VM
2021-05-20 14:56 [RFC PATCH 0/7] Support protection keys in an AMD EPYC-Milan VM David Edmondson
` (6 preceding siblings ...)
2021-05-20 14:56 ` [RFC PATCH 7/7] target/i386: Manipulate only AMD XSAVE state on AMD David Edmondson
@ 2021-05-20 15:15 ` no-reply
2021-06-08 8:24 ` David Edmondson
2021-06-11 16:01 ` Paolo Bonzini
9 siblings, 0 replies; 14+ messages in thread
From: no-reply @ 2021-05-20 15:15 UTC (permalink / raw)
To: david.edmondson
Cc: ehabkost, kvm, mtosatti, richard.henderson, qemu-devel,
david.edmondson, babu.moger, pbonzini
Patchew URL: https://patchew.org/QEMU/20210520145647.3483809-1-david.edmondson@oracle.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20210520145647.3483809-1-david.edmondson@oracle.com
Subject: [RFC PATCH 0/7] Support protection keys in an AMD EPYC-Milan VM
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/20210520145647.3483809-1-david.edmondson@oracle.com -> patchew/20210520145647.3483809-1-david.edmondson@oracle.com
Switched to a new branch 'test'
9761ad4 target/i386: Manipulate only AMD XSAVE state on AMD
fcba7d5 target/i386: Adjust AMD XSAVE PKRU area offset in CPUID leaf 0xd
0493da2 target/i386: Introduce AMD X86XSaveArea sub-union
e8d400c target/i386: Prepare for per-vendor X86XSaveArea layout
5a78b06 target/i386: Clarify the padding requirements of X86XSaveArea
ba3c3af target/i386: Use constants for XSAVE offsets
844afa9 target/i386: Declare constants for XSAVE offsets
=== OUTPUT BEGIN ===
1/7 Checking commit 844afa9929e3 (target/i386: Declare constants for XSAVE offsets)
WARNING: line over 80 characters
#60: FILE: target/i386/cpu.h:1352:
+QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, zmm_hi256_state) != XSAVE_ZMM_HI256_OFFSET);
WARNING: line over 80 characters
#63: FILE: target/i386/cpu.h:1354:
+QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, hi16_zmm_state) != XSAVE_HI16_ZMM_OFFSET);
total: 0 errors, 2 warnings, 48 lines checked
Patch 1/7 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/7 Checking commit ba3c3afe9795 (target/i386: Use constants for XSAVE offsets)
3/7 Checking commit 5a78b069f6d4 (target/i386: Clarify the padding requirements of X86XSaveArea)
4/7 Checking commit e8d400c3e40e (target/i386: Prepare for per-vendor X86XSaveArea layout)
5/7 Checking commit 0493da29bcd9 (target/i386: Introduce AMD X86XSaveArea sub-union)
6/7 Checking commit fcba7d58090d (target/i386: Adjust AMD XSAVE PKRU area offset in CPUID leaf 0xd)
7/7 Checking commit 9761ad41cd68 (target/i386: Manipulate only AMD XSAVE state on AMD)
ERROR: spaces required around that '+' (ctx:VxV)
#90: FILE: target/i386/xsave_helper.c:53:
+ stq_p(zmmh+8, env->xmm_regs[i].ZMM_Q(5));
^
ERROR: spaces required around that '+' (ctx:VxV)
#91: FILE: target/i386/xsave_helper.c:54:
+ stq_p(zmmh+16, env->xmm_regs[i].ZMM_Q(6));
^
ERROR: spaces required around that '+' (ctx:VxV)
#92: FILE: target/i386/xsave_helper.c:55:
+ stq_p(zmmh+24, env->xmm_regs[i].ZMM_Q(7));
^
ERROR: spaces required around that '+' (ctx:VxV)
#150: FILE: target/i386/xsave_helper.c:114:
+ env->xmm_regs[i].ZMM_Q(5) = ldq_p(zmmh+8);
^
ERROR: spaces required around that '+' (ctx:VxV)
#151: FILE: target/i386/xsave_helper.c:115:
+ env->xmm_regs[i].ZMM_Q(6) = ldq_p(zmmh+16);
^
ERROR: spaces required around that '+' (ctx:VxV)
#152: FILE: target/i386/xsave_helper.c:116:
+ env->xmm_regs[i].ZMM_Q(7) = ldq_p(zmmh+24);
^
total: 6 errors, 0 warnings, 140 lines checked
Patch 7/7 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20210520145647.3483809-1-david.edmondson@oracle.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFC PATCH 0/7] Support protection keys in an AMD EPYC-Milan VM
2021-05-20 14:56 [RFC PATCH 0/7] Support protection keys in an AMD EPYC-Milan VM David Edmondson
` (7 preceding siblings ...)
2021-05-20 15:15 ` [RFC PATCH 0/7] Support protection keys in an AMD EPYC-Milan VM no-reply
@ 2021-06-08 8:24 ` David Edmondson
2021-07-01 21:24 ` Babu Moger
2021-06-11 16:01 ` Paolo Bonzini
9 siblings, 1 reply; 14+ messages in thread
From: David Edmondson @ 2021-06-08 8:24 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, kvm, Marcelo Tosatti, Richard Henderson,
Babu Moger, Paolo Bonzini
On Thursday, 2021-05-20 at 15:56:40 +01, David Edmondson wrote:
> AMD EPYC-Milan CPUs introduced support for protection keys, previously
> available only with Intel CPUs.
>
> AMD chose to place the XSAVE state component for the protection keys
> at a different offset in the XSAVE state area than that chosen by
> Intel.
>
> To accommodate this, modify QEMU to behave appropriately on AMD
> systems, allowing a VM to properly take advantage of the new feature.
>
> Further, avoid manipulating XSAVE state components that are not
> present on AMD systems.
>
> The code in patch 6 that changes the CPUID 0x0d leaf is mostly dumped
> somewhere that seemed to work - I'm not sure where it really belongs.
Ping - any thoughts about this approach?
> David Edmondson (7):
> target/i386: Declare constants for XSAVE offsets
> target/i386: Use constants for XSAVE offsets
> target/i386: Clarify the padding requirements of X86XSaveArea
> target/i386: Prepare for per-vendor X86XSaveArea layout
> target/i386: Introduce AMD X86XSaveArea sub-union
> target/i386: Adjust AMD XSAVE PKRU area offset in CPUID leaf 0xd
> target/i386: Manipulate only AMD XSAVE state on AMD
>
> target/i386/cpu.c | 19 +++++----
> target/i386/cpu.h | 80 ++++++++++++++++++++++++++++--------
> target/i386/kvm/kvm.c | 57 +++++++++----------------
> target/i386/tcg/fpu_helper.c | 20 ++++++---
> target/i386/xsave_helper.c | 70 +++++++++++++++++++------------
> 5 files changed, 152 insertions(+), 94 deletions(-)
>
> --
> 2.30.2
dme.
--
You know your green from your red.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFC PATCH 0/7] Support protection keys in an AMD EPYC-Milan VM
2021-06-08 8:24 ` David Edmondson
@ 2021-07-01 21:24 ` Babu Moger
2021-07-01 21:32 ` David Edmondson
0 siblings, 1 reply; 14+ messages in thread
From: Babu Moger @ 2021-07-01 21:24 UTC (permalink / raw)
To: David Edmondson, qemu-devel
Cc: kvm, Eduardo Habkost, Paolo Bonzini, Marcelo Tosatti, Richard Henderson
David, Are you still working on v2 of these series? I was going to test
and review. Thanks
> -----Original Message-----
> From: David Edmondson <dme@dme.org>
> Sent: Tuesday, June 8, 2021 3:25 AM
> To: qemu-devel@nongnu.org
> Cc: kvm@vger.kernel.org; Eduardo Habkost <ehabkost@redhat.com>; Paolo
> Bonzini <pbonzini@redhat.com>; Marcelo Tosatti <mtosatti@redhat.com>;
> Richard Henderson <richard.henderson@linaro.org>; Moger, Babu
> <Babu.Moger@amd.com>
> Subject: Re: [RFC PATCH 0/7] Support protection keys in an AMD EPYC-Milan
> VM
>
> On Thursday, 2021-05-20 at 15:56:40 +01, David Edmondson wrote:
>
> > AMD EPYC-Milan CPUs introduced support for protection keys, previously
> > available only with Intel CPUs.
> >
> > AMD chose to place the XSAVE state component for the protection keys
> > at a different offset in the XSAVE state area than that chosen by
> > Intel.
> >
> > To accommodate this, modify QEMU to behave appropriately on AMD
> > systems, allowing a VM to properly take advantage of the new feature.
> >
> > Further, avoid manipulating XSAVE state components that are not
> > present on AMD systems.
> >
> > The code in patch 6 that changes the CPUID 0x0d leaf is mostly dumped
> > somewhere that seemed to work - I'm not sure where it really belongs.
>
> Ping - any thoughts about this approach?
>
> > David Edmondson (7):
> > target/i386: Declare constants for XSAVE offsets
> > target/i386: Use constants for XSAVE offsets
> > target/i386: Clarify the padding requirements of X86XSaveArea
> > target/i386: Prepare for per-vendor X86XSaveArea layout
> > target/i386: Introduce AMD X86XSaveArea sub-union
> > target/i386: Adjust AMD XSAVE PKRU area offset in CPUID leaf 0xd
> > target/i386: Manipulate only AMD XSAVE state on AMD
> >
> > target/i386/cpu.c | 19 +++++----
> > target/i386/cpu.h | 80 ++++++++++++++++++++++++++++--------
> > target/i386/kvm/kvm.c | 57 +++++++++----------------
> > target/i386/tcg/fpu_helper.c | 20 ++++++---
> > target/i386/xsave_helper.c | 70 +++++++++++++++++++------------
> > 5 files changed, 152 insertions(+), 94 deletions(-)
> >
> > --
> > 2.30.2
>
> dme.
> --
> You know your green from your red.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 0/7] Support protection keys in an AMD EPYC-Milan VM
2021-07-01 21:24 ` Babu Moger
@ 2021-07-01 21:32 ` David Edmondson
0 siblings, 0 replies; 14+ messages in thread
From: David Edmondson @ 2021-07-01 21:32 UTC (permalink / raw)
To: Babu Moger, qemu-devel
Cc: Paolo Bonzini, Marcelo Tosatti, Richard Henderson, Eduardo Habkost, kvm
On Thursday, 2021-07-01 at 16:24:51 -05, Babu Moger wrote:
> David, Are you still working on v2 of these series? I was going to test
> and review. Thanks
Yes. I have something that works, but it's messy in places. I hope to
get it out in a couple of days.
>> -----Original Message-----
>> From: David Edmondson <dme@dme.org>
>> Sent: Tuesday, June 8, 2021 3:25 AM
>> To: qemu-devel@nongnu.org
>> Cc: kvm@vger.kernel.org; Eduardo Habkost <ehabkost@redhat.com>; Paolo
>> Bonzini <pbonzini@redhat.com>; Marcelo Tosatti <mtosatti@redhat.com>;
>> Richard Henderson <richard.henderson@linaro.org>; Moger, Babu
>> <Babu.Moger@amd.com>
>> Subject: Re: [RFC PATCH 0/7] Support protection keys in an AMD EPYC-Milan
>> VM
>>
>> On Thursday, 2021-05-20 at 15:56:40 +01, David Edmondson wrote:
>>
>> > AMD EPYC-Milan CPUs introduced support for protection keys, previously
>> > available only with Intel CPUs.
>> >
>> > AMD chose to place the XSAVE state component for the protection keys
>> > at a different offset in the XSAVE state area than that chosen by
>> > Intel.
>> >
>> > To accommodate this, modify QEMU to behave appropriately on AMD
>> > systems, allowing a VM to properly take advantage of the new feature.
>> >
>> > Further, avoid manipulating XSAVE state components that are not
>> > present on AMD systems.
>> >
>> > The code in patch 6 that changes the CPUID 0x0d leaf is mostly dumped
>> > somewhere that seemed to work - I'm not sure where it really belongs.
>>
>> Ping - any thoughts about this approach?
>>
>> > David Edmondson (7):
>> > target/i386: Declare constants for XSAVE offsets
>> > target/i386: Use constants for XSAVE offsets
>> > target/i386: Clarify the padding requirements of X86XSaveArea
>> > target/i386: Prepare for per-vendor X86XSaveArea layout
>> > target/i386: Introduce AMD X86XSaveArea sub-union
>> > target/i386: Adjust AMD XSAVE PKRU area offset in CPUID leaf 0xd
>> > target/i386: Manipulate only AMD XSAVE state on AMD
>> >
>> > target/i386/cpu.c | 19 +++++----
>> > target/i386/cpu.h | 80 ++++++++++++++++++++++++++++--------
>> > target/i386/kvm/kvm.c | 57 +++++++++----------------
>> > target/i386/tcg/fpu_helper.c | 20 ++++++---
>> > target/i386/xsave_helper.c | 70 +++++++++++++++++++------------
>> > 5 files changed, 152 insertions(+), 94 deletions(-)
>> >
>> > --
>> > 2.30.2
>>
>> dme.
>> --
>> You know your green from your red.
dme.
--
When you were the brightest star, who were the shadows?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 0/7] Support protection keys in an AMD EPYC-Milan VM
2021-05-20 14:56 [RFC PATCH 0/7] Support protection keys in an AMD EPYC-Milan VM David Edmondson
` (8 preceding siblings ...)
2021-06-08 8:24 ` David Edmondson
@ 2021-06-11 16:01 ` Paolo Bonzini
2021-06-14 16:21 ` David Edmondson
9 siblings, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2021-06-11 16:01 UTC (permalink / raw)
To: David Edmondson, qemu-devel
Cc: Babu Moger, Marcelo Tosatti, Richard Henderson, Eduardo Habkost, kvm
First of all, sorry for the delayed review.
On 20/05/21 16:56, David Edmondson wrote:
> AMD EPYC-Milan CPUs introduced support for protection keys, previously
> available only with Intel CPUs.
>
> AMD chose to place the XSAVE state component for the protection keys
> at a different offset in the XSAVE state area than that chosen by
> Intel.
>
> To accommodate this, modify QEMU to behave appropriately on AMD
> systems, allowing a VM to properly take advantage of the new feature.
Uff, that sucks. :(
If I understand correctly, the problem is that the layout of
KVM_GET_XSAVE/KVM_SET_XSAVE depends on the host CPUID, which in
retrospect would be obvious. Is that correct? If so, it would make
sense and might even be easier to drop all usage of X86XSaveArea:
* update ext_save_areas based on CPUID information in kvm_cpu_instance_init
* make x86_cpu_xsave_all_areas and x86_cpu_xrstor_all_areas use the
ext_save_areas offsets to build pointers to XSaveAVX, XSaveBNDREG, etc.
What do you think?
Paolo
> Further, avoid manipulating XSAVE state components that are not
> present on AMD systems.
>
> The code in patch 6 that changes the CPUID 0x0d leaf is mostly dumped
> somewhere that seemed to work - I'm not sure where it really belongs.
>
> David Edmondson (7):
> target/i386: Declare constants for XSAVE offsets
> target/i386: Use constants for XSAVE offsets
> target/i386: Clarify the padding requirements of X86XSaveArea
> target/i386: Prepare for per-vendor X86XSaveArea layout
> target/i386: Introduce AMD X86XSaveArea sub-union
> target/i386: Adjust AMD XSAVE PKRU area offset in CPUID leaf 0xd
> target/i386: Manipulate only AMD XSAVE state on AMD
>
> target/i386/cpu.c | 19 +++++----
> target/i386/cpu.h | 80 ++++++++++++++++++++++++++++--------
> target/i386/kvm/kvm.c | 57 +++++++++----------------
> target/i386/tcg/fpu_helper.c | 20 ++++++---
> target/i386/xsave_helper.c | 70 +++++++++++++++++++------------
> 5 files changed, 152 insertions(+), 94 deletions(-)
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFC PATCH 0/7] Support protection keys in an AMD EPYC-Milan VM
2021-06-11 16:01 ` Paolo Bonzini
@ 2021-06-14 16:21 ` David Edmondson
0 siblings, 0 replies; 14+ messages in thread
From: David Edmondson @ 2021-06-14 16:21 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel
Cc: Babu Moger, Marcelo Tosatti, Richard Henderson, Eduardo Habkost, kvm
On Friday, 2021-06-11 at 18:01:55 +02, Paolo Bonzini wrote:
> First of all, sorry for the delayed review.
>
> On 20/05/21 16:56, David Edmondson wrote:
>> AMD EPYC-Milan CPUs introduced support for protection keys, previously
>> available only with Intel CPUs.
>>
>> AMD chose to place the XSAVE state component for the protection keys
>> at a different offset in the XSAVE state area than that chosen by
>> Intel.
>>
>> To accommodate this, modify QEMU to behave appropriately on AMD
>> systems, allowing a VM to properly take advantage of the new feature.
>
> Uff, that sucks. :(
>
> If I understand correctly, the problem is that the layout of
> KVM_GET_XSAVE/KVM_SET_XSAVE depends on the host CPUID, which in
> retrospect would be obvious. Is that correct?
Yes.
> If so, it would make sense and might even be easier to drop all usage
> of X86XSaveArea:
>
> * update ext_save_areas based on CPUID information in kvm_cpu_instance_init
>
> * make x86_cpu_xsave_all_areas and x86_cpu_xrstor_all_areas use the
> ext_save_areas offsets to build pointers to XSaveAVX, XSaveBNDREG, etc.
>
> What do you think?
I will produce a patch and send it out.
> Paolo
>
>> Further, avoid manipulating XSAVE state components that are not
>> present on AMD systems.
>>
>> The code in patch 6 that changes the CPUID 0x0d leaf is mostly dumped
>> somewhere that seemed to work - I'm not sure where it really belongs.
>>
>> David Edmondson (7):
>> target/i386: Declare constants for XSAVE offsets
>> target/i386: Use constants for XSAVE offsets
>> target/i386: Clarify the padding requirements of X86XSaveArea
>> target/i386: Prepare for per-vendor X86XSaveArea layout
>> target/i386: Introduce AMD X86XSaveArea sub-union
>> target/i386: Adjust AMD XSAVE PKRU area offset in CPUID leaf 0xd
>> target/i386: Manipulate only AMD XSAVE state on AMD
>>
>> target/i386/cpu.c | 19 +++++----
>> target/i386/cpu.h | 80 ++++++++++++++++++++++++++++--------
>> target/i386/kvm/kvm.c | 57 +++++++++----------------
>> target/i386/tcg/fpu_helper.c | 20 ++++++---
>> target/i386/xsave_helper.c | 70 +++++++++++++++++++------------
>> 5 files changed, 152 insertions(+), 94 deletions(-)
>>
dme.
--
Oliver darling, call Mister Haney, I think our speakers are blown.
^ permalink raw reply [flat|nested] 14+ messages in thread