* [PATCH 0/5] x86: AMD CPUID handling improvements
@ 2024-04-29 15:16 Andrew Cooper
2024-04-29 15:16 ` [PATCH 1/5] x86/cpu-policy: Infrastructure for the AMD SVM and SEV leaves Andrew Cooper
` (4 more replies)
0 siblings, 5 replies; 20+ messages in thread
From: Andrew Cooper @ 2024-04-29 15:16 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou, Sergiy Kibrik,
George Dunlap, Andrei Semenov, Vaishali Thakkar
This is (half) the series I've promised various people, to untangle some AMD
CPUID handling. It moves the SVM feature leaf into the standard
x86_capabilities[] infrastructure.
On a random Milan system, with this in place, xen-cpuid reports:
Dynamic sets:
Raw 178bfbff:7eda320b:2fd3fbff:75c237ff:0000000f:219c95a9:0040069c:00006799:91bef75f:00000000:00000000:0000204d:00000000:00000000:00000000:00000000:00000000:00000000:119b9cff:0101fd3f
[18] CPUID 0x8000000a.edx npt v-lbr svm-lock nrips v-tsc-rate vmcb-cleanbits flush-by-asid decode-assist pause-filter <11> pause-thresh v-loadsave v-gif <17> npt-sss v-spec-ctrl <23> <24> <28>
[19] CPUID 0x8000001f.eax sme sev <2> sev-es sev-snp <5> <8> <10> <11> <12> <13> <14> <15> <16> <24>
Host 178bf3ff:76da320b:2fd3fbff:644037ff:0000000f:219c95a9:0040068c:00000780:319ed205:00000000:00000000:18000045:00000000:00000000:00000000:00000000:00000000:00000000:001994ff:00000000
[18] CPUID 0x8000000a.edx npt v-lbr svm-lock nrips v-tsc-rate vmcb-cleanbits flush-by-asid decode-assist pause-filter pause-thresh v-loadsave v-gif npt-sss v-spec-ctrl
[19] CPUID 0x8000001f.eax
HVM Max 178bfbff:f6fa3203:2e500800:440001f7:0000000f:219c05a9:0040060c:00000100:331ed005:00000000:00000000:18000045:00000000:00000000:00000000:00000000:00000000:00000000:000004ab:00000000
[18] CPUID 0x8000000a.edx npt v-lbr nrips vmcb-cleanbits decode-assist pause-filter
[19] CPUID 0x8000001f.eax
Unforunately, I haven't managed to do the second half to make the host_policy
usable earlier on boot. Untanling __setup_xen() is proving stuborn due to
(ab)uses of the MB1 module list.
Andrew Cooper (5):
x86/cpu-policy: Infrastructure for the AMD SVM and SEV leaves
x86/cpu-policy: Add SVM features already used by Xen
x86/spec-ctrl: Remove open-coded check of SVM_FEATURE_SPEC_CTRL
x86/svm: Switch SVM features over normal cpu_has_*
x86/cpu-policy: Introduce some SEV features
tools/libs/light/libxl_cpuid.c | 2 +
tools/misc/xen-cpuid.c | 24 ++++++++++
xen/arch/x86/cpu-policy.c | 17 +++----
xen/arch/x86/cpu/common.c | 4 ++
xen/arch/x86/hvm/svm/asid.c | 5 +--
xen/arch/x86/hvm/svm/emulate.c | 3 +-
xen/arch/x86/hvm/svm/intr.c | 1 -
xen/arch/x86/hvm/svm/nestedsvm.c | 14 +++---
xen/arch/x86/hvm/svm/svm.c | 50 +++++----------------
xen/arch/x86/hvm/svm/vmcb.c | 1 -
xen/arch/x86/include/asm/cpufeature.h | 16 +++++++
xen/arch/x86/include/asm/hvm/svm/svm.h | 36 ---------------
xen/arch/x86/spec_ctrl.c | 7 +--
xen/include/public/arch-x86/cpufeatureset.h | 22 +++++++++
xen/include/xen/lib/x86/cpu-policy.h | 24 +++++++++-
xen/lib/x86/cpuid.c | 4 ++
xen/tools/gen-cpuid.py | 7 +++
17 files changed, 128 insertions(+), 109 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/5] x86/cpu-policy: Infrastructure for the AMD SVM and SEV leaves
2024-04-29 15:16 [PATCH 0/5] x86: AMD CPUID handling improvements Andrew Cooper
@ 2024-04-29 15:16 ` Andrew Cooper
2024-04-30 12:45 ` Jan Beulich
2024-04-29 15:16 ` [PATCH 2/5] x86/cpu-policy: Add SVM features already used by Xen Andrew Cooper
` (3 subsequent siblings)
4 siblings, 1 reply; 20+ messages in thread
From: Andrew Cooper @ 2024-04-29 15:16 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou, Sergiy Kibrik,
George Dunlap, Andrei Semenov, Vaishali Thakkar
Allocate two new feature leaves, and extend cpu_policy with the non-feature
fields too.
The CPUID dependency between the SVM bit on the whole SVM leaf is
intentionally deferred, to avoid transiently breaking nested virt.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Xenia Ragiadakou <xenia.ragiadakou@amd.com>
CC: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
CC: George Dunlap <george.dunlap@citrix.com>
CC: Andrei Semenov <andrei.semenov@vates.fr>
CC: Vaishali Thakkar <vaishali.thakkar@vates.tech>
---
tools/libs/light/libxl_cpuid.c | 2 ++
tools/misc/xen-cpuid.c | 10 +++++++++
xen/arch/x86/cpu/common.c | 4 ++++
xen/include/public/arch-x86/cpufeatureset.h | 4 ++++
xen/include/xen/lib/x86/cpu-policy.h | 24 +++++++++++++++++++--
xen/lib/x86/cpuid.c | 4 ++++
6 files changed, 46 insertions(+), 2 deletions(-)
diff --git a/tools/libs/light/libxl_cpuid.c b/tools/libs/light/libxl_cpuid.c
index ce4f3c7095ba..c7a8b76f541d 100644
--- a/tools/libs/light/libxl_cpuid.c
+++ b/tools/libs/light/libxl_cpuid.c
@@ -342,6 +342,8 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list *policy, const char* str)
CPUID_ENTRY(0x00000007, 1, CPUID_REG_EDX),
MSR_ENTRY(0x10a, CPUID_REG_EAX),
MSR_ENTRY(0x10a, CPUID_REG_EDX),
+ CPUID_ENTRY(0x8000000a, NA, CPUID_REG_EDX),
+ CPUID_ENTRY(0x8000001f, NA, CPUID_REG_EAX),
#undef MSR_ENTRY
#undef CPUID_ENTRY
};
diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
index 8893547bebce..ab09410a05d6 100644
--- a/tools/misc/xen-cpuid.c
+++ b/tools/misc/xen-cpuid.c
@@ -264,6 +264,14 @@ static const char *const str_m10Ah[32] =
{
};
+static const char *const str_eAd[32] =
+{
+};
+
+static const char *const str_e1Fa[32] =
+{
+};
+
static const struct {
const char *name;
const char *abbr;
@@ -288,6 +296,8 @@ static const struct {
{ "CPUID 0x00000007:1.edx", "7d1", str_7d1 },
{ "MSR_ARCH_CAPS.lo", "m10Al", str_m10Al },
{ "MSR_ARCH_CAPS.hi", "m10Ah", str_m10Ah },
+ { "CPUID 0x8000000a.edx", "eAd", str_eAd },
+ { "CPUID 0x8000001f.eax", "e1Fa", str_e1Fa },
};
#define COL_ALIGN "24"
diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index 28d7f34c4dbe..25b11e6472b8 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -477,6 +477,10 @@ static void generic_identify(struct cpuinfo_x86 *c)
c->x86_capability[FEATURESET_e7d] = cpuid_edx(0x80000007);
if (c->extended_cpuid_level >= 0x80000008)
c->x86_capability[FEATURESET_e8b] = cpuid_ebx(0x80000008);
+ if (c->extended_cpuid_level >= 0x8000000a)
+ c->x86_capability[FEATURESET_eAd] = cpuid_edx(0x8000000a);
+ if (c->extended_cpuid_level >= 0x8000001f)
+ c->x86_capability[FEATURESET_e1Fa] = cpuid_eax(0x8000001f);
if (c->extended_cpuid_level >= 0x80000021)
c->x86_capability[FEATURESET_e21a] = cpuid_eax(0x80000021);
diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
index 53f13dec31f7..0f869214811e 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -357,6 +357,10 @@ XEN_CPUFEATURE(RFDS_CLEAR, 16*32+28) /*!A Register File(s) cleared by VE
/* Intel-defined CPU features, MSR_ARCH_CAPS 0x10a.edx, word 17 */
+/* AMD-defined CPU features, CPUID level 0x8000000a.edx, word 18 */
+
+/* AMD-defined CPU features, CPUID level 0x8000001f.eax, word 19 */
+
#endif /* XEN_CPUFEATURE */
/* Clean up from a default include. Close the enum (for C). */
diff --git a/xen/include/xen/lib/x86/cpu-policy.h b/xen/include/xen/lib/x86/cpu-policy.h
index d5e447e9dc06..936e00e4da73 100644
--- a/xen/include/xen/lib/x86/cpu-policy.h
+++ b/xen/include/xen/lib/x86/cpu-policy.h
@@ -22,6 +22,8 @@
#define FEATURESET_7d1 15 /* 0x00000007:1.edx */
#define FEATURESET_m10Al 16 /* 0x0000010a.eax */
#define FEATURESET_m10Ah 17 /* 0x0000010a.edx */
+#define FEATURESET_eAd 18 /* 0x8000000a.edx */
+#define FEATURESET_e1Fa 19 /* 0x8000001f.eax */
struct cpuid_leaf
{
@@ -296,7 +298,16 @@ struct cpu_policy
uint32_t /* d */:32;
uint64_t :64, :64; /* Leaf 0x80000009. */
- uint64_t :64, :64; /* Leaf 0x8000000a - SVM rev and features. */
+
+ /* Leaf 0x8000000a - SVM rev and features. */
+ uint8_t svm_rev, :8, :8, :8;
+ uint32_t /* b */ :32;
+ uint32_t nr_asids;
+ union {
+ uint32_t eAd;
+ struct { DECL_BITFIELD(eAd); };
+ };
+
uint64_t :64, :64; /* Leaf 0x8000000b. */
uint64_t :64, :64; /* Leaf 0x8000000c. */
uint64_t :64, :64; /* Leaf 0x8000000d. */
@@ -317,7 +328,16 @@ struct cpu_policy
uint64_t :64, :64; /* Leaf 0x8000001c. */
uint64_t :64, :64; /* Leaf 0x8000001d - Cache properties. */
uint64_t :64, :64; /* Leaf 0x8000001e - Extd APIC/Core/Node IDs. */
- uint64_t :64, :64; /* Leaf 0x8000001f - AMD Secure Encryption. */
+
+ /* Leaf 0x8000001f - AMD Secure Encryption. */
+ union {
+ uint32_t e1Fa;
+ struct { DECL_BITFIELD(e1Fa); };
+ };
+ uint32_t cbit:6, paddr_reduce:6, nr_vmpls:4, :16;
+ uint32_t nr_enc_guests;
+ uint32_t sev_min_asid;
+
uint64_t :64, :64; /* Leaf 0x80000020 - Platform QoS. */
/* Leaf 0x80000021 - Extended Feature 2 */
diff --git a/xen/lib/x86/cpuid.c b/xen/lib/x86/cpuid.c
index eb7698dc7325..14deb01a6d0b 100644
--- a/xen/lib/x86/cpuid.c
+++ b/xen/lib/x86/cpuid.c
@@ -81,6 +81,8 @@ void x86_cpu_policy_to_featureset(
fs[FEATURESET_7d1] = p->feat._7d1;
fs[FEATURESET_m10Al] = p->arch_caps.lo;
fs[FEATURESET_m10Ah] = p->arch_caps.hi;
+ fs[FEATURESET_eAd] = p->extd.eAd;
+ fs[FEATURESET_e1Fa] = p->extd.e1Fa;
}
void x86_cpu_featureset_to_policy(
@@ -104,6 +106,8 @@ void x86_cpu_featureset_to_policy(
p->feat._7d1 = fs[FEATURESET_7d1];
p->arch_caps.lo = fs[FEATURESET_m10Al];
p->arch_caps.hi = fs[FEATURESET_m10Ah];
+ p->extd.eAd = fs[FEATURESET_eAd];
+ p->extd.e1Fa = fs[FEATURESET_e1Fa];
}
void x86_cpu_policy_recalc_synth(struct cpu_policy *p)
--
2.30.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/5] x86/cpu-policy: Add SVM features already used by Xen
2024-04-29 15:16 [PATCH 0/5] x86: AMD CPUID handling improvements Andrew Cooper
2024-04-29 15:16 ` [PATCH 1/5] x86/cpu-policy: Infrastructure for the AMD SVM and SEV leaves Andrew Cooper
@ 2024-04-29 15:16 ` Andrew Cooper
2024-04-29 15:24 ` Andrew Cooper
` (2 more replies)
2024-04-29 15:16 ` [PATCH 3/5] x86/spec-ctrl: Remove open-coded check of SVM_FEATURE_SPEC_CTRL Andrew Cooper
` (2 subsequent siblings)
4 siblings, 3 replies; 20+ messages in thread
From: Andrew Cooper @ 2024-04-29 15:16 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou, Sergiy Kibrik,
George Dunlap, Andrei Semenov, Vaishali Thakkar
These will replace svm_feature_flags and the SVM_FEATURE_* constants over the
next few changes. Take the opportunity to rationalise some names.
Drop the opencoded "inherit from host" logic in calculate_hvm_max_policy() and
use 'h'/'!' annotations. The logic needs to operate on fs, not the policy
object, given its position within the function.
Drop some trailing whitespace introduced when this block of code was last
moved.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Xenia Ragiadakou <xenia.ragiadakou@amd.com>
CC: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
CC: George Dunlap <george.dunlap@citrix.com>
CC: Andrei Semenov <andrei.semenov@vates.fr>
CC: Vaishali Thakkar <vaishali.thakkar@vates.tech>
---
tools/misc/xen-cpuid.c | 11 +++++++++++
xen/arch/x86/cpu-policy.c | 17 +++++------------
xen/include/public/arch-x86/cpufeatureset.h | 14 ++++++++++++++
xen/tools/gen-cpuid.py | 3 +++
4 files changed, 33 insertions(+), 12 deletions(-)
diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
index ab09410a05d6..0d01b0e797f1 100644
--- a/tools/misc/xen-cpuid.c
+++ b/tools/misc/xen-cpuid.c
@@ -266,6 +266,17 @@ static const char *const str_m10Ah[32] =
static const char *const str_eAd[32] =
{
+ [ 0] = "npt", [ 1] = "v-lbr",
+ [ 2] = "svm-lock", [ 3] = "nrips",
+ [ 4] = "v-tsc-rate", [ 5] = "vmcb-cleanbits",
+ [ 6] = "flush-by-asid", [ 7] = "decode-assist",
+
+ [10] = "pause-filter",
+ [12] = "pause-thresh",
+ /* 14 */ [15] = "v-loadsave",
+ [16] = "v-gif",
+ /* 18 */ [19] = "npt-sss",
+ [20] = "v-spec-ctrl",
};
static const char *const str_e1Fa[32] =
diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
index 4b6d96276399..da4401047e89 100644
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -9,7 +9,6 @@
#include <asm/amd.h>
#include <asm/cpu-policy.h>
#include <asm/hvm/nestedhvm.h>
-#include <asm/hvm/svm/svm.h>
#include <asm/intel-family.h>
#include <asm/msr-index.h>
#include <asm/paging.h>
@@ -748,22 +747,16 @@ static void __init calculate_hvm_max_policy(void)
if ( !cpu_has_vmx )
__clear_bit(X86_FEATURE_PKS, fs);
- /*
+ /*
* Make adjustments to possible (nested) virtualization features exposed
* to the guest
*/
- if ( p->extd.svm )
+ if ( test_bit(X86_FEATURE_SVM, fs) )
{
- /* Clamp to implemented features which require hardware support. */
- p->extd.raw[0xa].d &= ((1u << SVM_FEATURE_NPT) |
- (1u << SVM_FEATURE_LBRV) |
- (1u << SVM_FEATURE_NRIPS) |
- (1u << SVM_FEATURE_PAUSEFILTER) |
- (1u << SVM_FEATURE_DECODEASSISTS));
- /* Enable features which are always emulated. */
- p->extd.raw[0xa].d |= (1u << SVM_FEATURE_VMCBCLEAN);
+ /* Xen always emulates cleanbits. */
+ __set_bit(X86_FEATURE_VMCB_CLEANBITS, fs);
}
-
+
guest_common_max_feature_adjustments(fs);
guest_common_feature_adjustments(fs);
diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
index 0f869214811e..80d252a38c2d 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -358,6 +358,20 @@ XEN_CPUFEATURE(RFDS_CLEAR, 16*32+28) /*!A Register File(s) cleared by VE
/* Intel-defined CPU features, MSR_ARCH_CAPS 0x10a.edx, word 17 */
/* AMD-defined CPU features, CPUID level 0x8000000a.edx, word 18 */
+XEN_CPUFEATURE(NPT, 18*32+ 0) /*h Nested PageTables */
+XEN_CPUFEATURE(V_LBR, 18*32+ 1) /*h Virtualised LBR */
+XEN_CPUFEATURE(SVM_LOCK, 18*32+ 2) /* SVM locking MSR */
+XEN_CPUFEATURE(NRIPS, 18*32+ 3) /*h Next-RIP saved on VMExit */
+XEN_CPUFEATURE(V_TSC_RATE, 18*32+ 4) /* Virtualised TSC Ratio */
+XEN_CPUFEATURE(VMCB_CLEANBITS, 18*32+ 5) /*! VMCB Clean-bits */
+XEN_CPUFEATURE(FLUSH_BY_ASID, 18*32+ 6) /* TLB Flush by ASID */
+XEN_CPUFEATURE(DECODE_ASSIST, 18*32+ 7) /*h Decode assists */
+XEN_CPUFEATURE(PAUSE_FILTER, 18*32+10) /*h Pause filter */
+XEN_CPUFEATURE(PAUSE_THRESH, 18*32+12) /* Pause filter threshold */
+XEN_CPUFEATURE(V_LOADSAVE, 18*32+15) /* Virtualised VMLOAD/SAVE */
+XEN_CPUFEATURE(V_GIF, 18*32+16) /* Virtualised GIF */
+XEN_CPUFEATURE(NPT_SSS, 18*32+19) /* NPT Supervisor Shadow Stacks */
+XEN_CPUFEATURE(V_SPEC_CTRL, 18*32+20) /* Virtualised MSR_SPEC_CTRL */
/* AMD-defined CPU features, CPUID level 0x8000001f.eax, word 19 */
diff --git a/xen/tools/gen-cpuid.py b/xen/tools/gen-cpuid.py
index bf3f9ec01e6e..f07b1f4cf905 100755
--- a/xen/tools/gen-cpuid.py
+++ b/xen/tools/gen-cpuid.py
@@ -280,6 +280,9 @@ def crunch_numbers(state):
# standard 3DNow in the earlier K6 processors.
_3DNOW: [_3DNOWEXT],
+ # The SVM bit enumerates the whole SVM leave.
+ SVM: list(range(NPT, NPT + 32)),
+
# This is just the dependency between AVX512 and AVX2 of XSTATE
# feature flags. If want to use AVX512, AVX2 must be supported and
# enabled. Certain later extensions, acting on 256-bit vectors of
--
2.30.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/5] x86/spec-ctrl: Remove open-coded check of SVM_FEATURE_SPEC_CTRL
2024-04-29 15:16 [PATCH 0/5] x86: AMD CPUID handling improvements Andrew Cooper
2024-04-29 15:16 ` [PATCH 1/5] x86/cpu-policy: Infrastructure for the AMD SVM and SEV leaves Andrew Cooper
2024-04-29 15:16 ` [PATCH 2/5] x86/cpu-policy: Add SVM features already used by Xen Andrew Cooper
@ 2024-04-29 15:16 ` Andrew Cooper
2024-04-30 13:13 ` Jan Beulich
2024-04-29 15:16 ` [PATCH 4/5] x86/svm: Switch SVM features over normal cpu_has_* Andrew Cooper
2024-04-29 15:16 ` [PATCH 5/5] x86/cpu-policy: Introduce some SEV features Andrew Cooper
4 siblings, 1 reply; 20+ messages in thread
From: Andrew Cooper @ 2024-04-29 15:16 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou, Sergiy Kibrik,
George Dunlap, Andrei Semenov, Vaishali Thakkar
Now that the SVM feature leaf has been included in normal feature handling, it
is available early enough for init_speculation_mitigations() to use.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Xenia Ragiadakou <xenia.ragiadakou@amd.com>
CC: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
CC: George Dunlap <george.dunlap@citrix.com>
CC: Andrei Semenov <andrei.semenov@vates.fr>
CC: Vaishali Thakkar <vaishali.thakkar@vates.tech>
---
xen/arch/x86/include/asm/cpufeature.h | 3 +++
xen/arch/x86/spec_ctrl.c | 7 +------
2 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/xen/arch/x86/include/asm/cpufeature.h b/xen/arch/x86/include/asm/cpufeature.h
index 9bc553681f4a..77cfd900cb56 100644
--- a/xen/arch/x86/include/asm/cpufeature.h
+++ b/xen/arch/x86/include/asm/cpufeature.h
@@ -217,6 +217,9 @@ static inline bool boot_cpu_has(unsigned int feat)
#define cpu_has_rfds_no boot_cpu_has(X86_FEATURE_RFDS_NO)
#define cpu_has_rfds_clear boot_cpu_has(X86_FEATURE_RFDS_CLEAR)
+/* CPUID level 0x8000000a.edx */
+#define cpu_has_v_spec_ctrl boot_cpu_has(X86_FEATURE_V_SPEC_CTRL)
+
/* Synthesized. */
#define cpu_has_arch_perfmon boot_cpu_has(X86_FEATURE_ARCH_PERFMON)
#define cpu_has_cpuid_faulting boot_cpu_has(X86_FEATURE_CPUID_FAULTING)
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 40f6ae017010..0bda9d01def5 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -11,7 +11,6 @@
#include <xen/warning.h>
#include <asm/amd.h>
-#include <asm/hvm/svm/svm.h>
#include <asm/intel-family.h>
#include <asm/microcode.h>
#include <asm/msr.h>
@@ -1896,12 +1895,8 @@ void __init init_speculation_mitigations(void)
*
* No need for SCF_ist_sc_msr because Xen's value is restored
* atomically WRT NMIs in the VMExit path.
- *
- * TODO: Adjust cpu_has_svm_spec_ctrl to be usable earlier on boot.
*/
- if ( opt_msr_sc_hvm &&
- (boot_cpu_data.extended_cpuid_level >= 0x8000000aU) &&
- (cpuid_edx(0x8000000aU) & (1u << SVM_FEATURE_SPEC_CTRL)) )
+ if ( opt_msr_sc_hvm && cpu_has_v_spec_ctrl )
setup_force_cpu_cap(X86_FEATURE_SC_MSR_HVM);
}
--
2.30.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 4/5] x86/svm: Switch SVM features over normal cpu_has_*
2024-04-29 15:16 [PATCH 0/5] x86: AMD CPUID handling improvements Andrew Cooper
` (2 preceding siblings ...)
2024-04-29 15:16 ` [PATCH 3/5] x86/spec-ctrl: Remove open-coded check of SVM_FEATURE_SPEC_CTRL Andrew Cooper
@ 2024-04-29 15:16 ` Andrew Cooper
2024-04-30 5:51 ` Vaishali Thakkar
2024-04-30 13:25 ` Jan Beulich
2024-04-29 15:16 ` [PATCH 5/5] x86/cpu-policy: Introduce some SEV features Andrew Cooper
4 siblings, 2 replies; 20+ messages in thread
From: Andrew Cooper @ 2024-04-29 15:16 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou, Sergiy Kibrik,
George Dunlap, Andrei Semenov, Vaishali Thakkar
Delete the boot time rendering of advanced features. It's entirely ad-hoc and
not even everything printed here is used by Xen. It is available in
`xen-cpuid` now.
With (only) svm_load_segs_{,prefetch}() declared now in svm.h, only svm.c and
domain.c which need the header. Clean up all others.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Xenia Ragiadakou <xenia.ragiadakou@amd.com>
CC: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
CC: George Dunlap <george.dunlap@citrix.com>
CC: Andrei Semenov <andrei.semenov@vates.fr>
CC: Vaishali Thakkar <vaishali.thakkar@vates.tech>
---
xen/arch/x86/hvm/svm/asid.c | 5 ++-
xen/arch/x86/hvm/svm/emulate.c | 3 +-
xen/arch/x86/hvm/svm/intr.c | 1 -
xen/arch/x86/hvm/svm/nestedsvm.c | 14 ++++----
xen/arch/x86/hvm/svm/svm.c | 50 +++++++-------------------
xen/arch/x86/hvm/svm/vmcb.c | 1 -
xen/arch/x86/include/asm/cpufeature.h | 10 ++++++
xen/arch/x86/include/asm/hvm/svm/svm.h | 36 -------------------
8 files changed, 31 insertions(+), 89 deletions(-)
diff --git a/xen/arch/x86/hvm/svm/asid.c b/xen/arch/x86/hvm/svm/asid.c
index 7977a8e86b53..6117a362d310 100644
--- a/xen/arch/x86/hvm/svm/asid.c
+++ b/xen/arch/x86/hvm/svm/asid.c
@@ -6,7 +6,6 @@
#include <asm/amd.h>
#include <asm/hvm/nestedhvm.h>
-#include <asm/hvm/svm/svm.h>
#include "svm.h"
@@ -39,7 +38,7 @@ void svm_asid_handle_vmrun(void)
{
vmcb_set_asid(vmcb, true);
vmcb->tlb_control =
- cpu_has_svm_flushbyasid ? TLB_CTRL_FLUSH_ASID : TLB_CTRL_FLUSH_ALL;
+ cpu_has_flush_by_asid ? TLB_CTRL_FLUSH_ASID : TLB_CTRL_FLUSH_ALL;
return;
}
@@ -48,7 +47,7 @@ void svm_asid_handle_vmrun(void)
vmcb->tlb_control =
!need_flush ? TLB_CTRL_NO_FLUSH :
- cpu_has_svm_flushbyasid ? TLB_CTRL_FLUSH_ASID : TLB_CTRL_FLUSH_ALL;
+ cpu_has_flush_by_asid ? TLB_CTRL_FLUSH_ASID : TLB_CTRL_FLUSH_ALL;
}
/*
diff --git a/xen/arch/x86/hvm/svm/emulate.c b/xen/arch/x86/hvm/svm/emulate.c
index 93ac1d3435f9..da6e21b2e270 100644
--- a/xen/arch/x86/hvm/svm/emulate.c
+++ b/xen/arch/x86/hvm/svm/emulate.c
@@ -11,7 +11,6 @@
#include <asm/msr.h>
#include <asm/hvm/emulate.h>
#include <asm/hvm/hvm.h>
-#include <asm/hvm/svm/svm.h>
#include <asm/hvm/svm/vmcb.h>
#include "svm.h"
@@ -20,7 +19,7 @@ static unsigned long svm_nextrip_insn_length(struct vcpu *v)
{
struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
- if ( !cpu_has_svm_nrips )
+ if ( !cpu_has_nrips )
return 0;
#ifndef NDEBUG
diff --git a/xen/arch/x86/hvm/svm/intr.c b/xen/arch/x86/hvm/svm/intr.c
index 4805c5567213..facd2894a2c6 100644
--- a/xen/arch/x86/hvm/svm/intr.c
+++ b/xen/arch/x86/hvm/svm/intr.c
@@ -17,7 +17,6 @@
#include <asm/hvm/hvm.h>
#include <asm/hvm/io.h>
#include <asm/hvm/vlapic.h>
-#include <asm/hvm/svm/svm.h>
#include <asm/hvm/nestedhvm.h> /* for nestedhvm_vcpu_in_guestmode */
#include <asm/vm_event.h>
#include <xen/event.h>
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index 35a2cbfd7d13..255af112661f 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -6,7 +6,6 @@
*/
#include <asm/hvm/support.h>
-#include <asm/hvm/svm/svm.h>
#include <asm/hvm/svm/vmcb.h>
#include <asm/hvm/nestedhvm.h>
#include <asm/hvm/svm/svmdebug.h>
@@ -1620,7 +1619,7 @@ void svm_nested_features_on_efer_update(struct vcpu *v)
{
if ( !vmcb->virt_ext.fields.vloadsave_enable &&
paging_mode_hap(v->domain) &&
- cpu_has_svm_vloadsave )
+ cpu_has_v_loadsave )
{
vmcb->virt_ext.fields.vloadsave_enable = 1;
general2_intercepts = vmcb_get_general2_intercepts(vmcb);
@@ -1629,8 +1628,7 @@ void svm_nested_features_on_efer_update(struct vcpu *v)
vmcb_set_general2_intercepts(vmcb, general2_intercepts);
}
- if ( !vmcb->_vintr.fields.vgif_enable &&
- cpu_has_svm_vgif )
+ if ( !vmcb->_vintr.fields.vgif_enable && cpu_has_v_gif )
{
vintr = vmcb_get_vintr(vmcb);
vintr.fields.vgif = svm->ns_gif;
@@ -1675,8 +1673,8 @@ void __init start_nested_svm(struct hvm_function_table *hvm_function_table)
*/
hvm_function_table->caps.nested_virt =
hvm_function_table->caps.hap &&
- cpu_has_svm_lbrv &&
- cpu_has_svm_nrips &&
- cpu_has_svm_flushbyasid &&
- cpu_has_svm_decode;
+ cpu_has_v_lbr &&
+ cpu_has_nrips &&
+ cpu_has_flush_by_asid &&
+ cpu_has_decode_assist;
}
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 4719fffae589..16eb875aab94 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1287,7 +1287,7 @@ static void cf_check svm_inject_event(const struct x86_event *event)
* that hardware doesn't perform DPL checking on injection.
*/
if ( event->type == X86_EVENTTYPE_PRI_SW_EXCEPTION ||
- (!cpu_has_svm_nrips && (event->type >= X86_EVENTTYPE_SW_INTERRUPT)) )
+ (!cpu_has_nrips && (event->type >= X86_EVENTTYPE_SW_INTERRUPT)) )
svm_emul_swint_injection(&_event);
switch ( _event.vector | -(_event.type == X86_EVENTTYPE_SW_INTERRUPT) )
@@ -1341,7 +1341,7 @@ static void cf_check svm_inject_event(const struct x86_event *event)
switch ( _event.type )
{
case X86_EVENTTYPE_SW_INTERRUPT: /* int $n */
- if ( cpu_has_svm_nrips )
+ if ( cpu_has_nrips )
vmcb->nextrip = regs->rip + _event.insn_len;
else
regs->rip += _event.insn_len;
@@ -1355,7 +1355,7 @@ static void cf_check svm_inject_event(const struct x86_event *event)
* semantics.
*/
regs->rip += _event.insn_len;
- if ( cpu_has_svm_nrips )
+ if ( cpu_has_nrips )
vmcb->nextrip = regs->rip;
eventinj.type = X86_EVENTTYPE_HW_EXCEPTION;
break;
@@ -1365,7 +1365,7 @@ static void cf_check svm_inject_event(const struct x86_event *event)
* Hardware special cases HW_EXCEPTION with vectors 3 and 4 as having
* trap semantics, and will perform DPL checks.
*/
- if ( cpu_has_svm_nrips )
+ if ( cpu_has_nrips )
vmcb->nextrip = regs->rip + _event.insn_len;
else
regs->rip += _event.insn_len;
@@ -1982,7 +1982,7 @@ static int cf_check svm_msr_write_intercept(
case MSR_IA32_DEBUGCTLMSR:
vmcb_set_debugctlmsr(vmcb, msr_content);
- if ( !msr_content || !cpu_has_svm_lbrv )
+ if ( !msr_content || !cpu_has_v_lbr )
break;
vmcb->virt_ext.fields.lbr_enable = 1;
svm_disable_intercept_for_msr(v, MSR_IA32_DEBUGCTLMSR);
@@ -2480,8 +2480,6 @@ static struct hvm_function_table __initdata_cf_clobber svm_function_table = {
const struct hvm_function_table * __init start_svm(void)
{
- bool printed = false;
-
svm_host_osvw_reset();
if ( _svm_cpu_up(true) )
@@ -2493,38 +2491,14 @@ const struct hvm_function_table * __init start_svm(void)
setup_vmcb_dump();
- if ( boot_cpu_data.extended_cpuid_level >= 0x8000000aU )
- svm_feature_flags = cpuid_edx(0x8000000aU);
-
- printk("SVM: Supported advanced features:\n");
-
/* DecodeAssists fast paths assume nextrip is valid for fast rIP update. */
- if ( !cpu_has_svm_nrips )
- __clear_bit(SVM_FEATURE_DECODEASSISTS, &svm_feature_flags);
+ if ( !cpu_has_nrips )
+ setup_clear_cpu_cap(X86_FEATURE_DECODE_ASSIST);
if ( cpu_has_tsc_ratio )
svm_function_table.tsc_scaling.ratio_frac_bits = 32;
-#define P(p,s) if ( p ) { printk(" - %s\n", s); printed = 1; }
- P(cpu_has_svm_npt, "Nested Page Tables (NPT)");
- P(cpu_has_svm_lbrv, "Last Branch Record (LBR) Virtualisation");
- P(cpu_has_svm_nrips, "Next-RIP Saved on #VMEXIT");
- P(cpu_has_svm_cleanbits, "VMCB Clean Bits");
- P(cpu_has_svm_flushbyasid, "TLB flush by ASID");
- P(cpu_has_svm_decode, "DecodeAssists");
- P(cpu_has_svm_vloadsave, "Virtual VMLOAD/VMSAVE");
- P(cpu_has_svm_vgif, "Virtual GIF");
- P(cpu_has_pause_filter, "Pause-Intercept Filter");
- P(cpu_has_pause_thresh, "Pause-Intercept Filter Threshold");
- P(cpu_has_tsc_ratio, "TSC Rate MSR");
- P(cpu_has_svm_sss, "NPT Supervisor Shadow Stack");
- P(cpu_has_svm_spec_ctrl, "MSR_SPEC_CTRL virtualisation");
-#undef P
-
- if ( !printed )
- printk(" - none\n");
-
- svm_function_table.caps.hap = cpu_has_svm_npt;
+ svm_function_table.caps.hap = cpu_has_npt;
svm_function_table.caps.hap_superpage_2mb = true;
svm_function_table.caps.hap_superpage_1gb = cpu_has_page1gb;
@@ -2761,7 +2735,7 @@ void asmlinkage svm_vmexit_handler(void)
regs->rax, regs->rbx, regs->rcx,
regs->rdx, regs->rsi, regs->rdi);
- if ( cpu_has_svm_decode )
+ if ( cpu_has_decode_assist )
v->arch.hvm.svm.cached_insn_len = vmcb->guest_ins_len & 0xf;
rc = paging_fault(va, regs);
v->arch.hvm.svm.cached_insn_len = 0;
@@ -2906,14 +2880,14 @@ void asmlinkage svm_vmexit_handler(void)
case VMEXIT_CR0_READ ... VMEXIT_CR15_READ:
case VMEXIT_CR0_WRITE ... VMEXIT_CR15_WRITE:
- if ( cpu_has_svm_decode && vmcb->ei.mov_cr.mov_insn )
+ if ( cpu_has_decode_assist && vmcb->ei.mov_cr.mov_insn )
svm_vmexit_do_cr_access(vmcb, regs);
else if ( !hvm_emulate_one_insn(x86_insn_is_cr_access, "CR access") )
hvm_inject_hw_exception(X86_EXC_GP, 0);
break;
case VMEXIT_INVLPG:
- if ( cpu_has_svm_decode )
+ if ( cpu_has_decode_assist )
{
svm_invlpg_intercept(vmcb->exitinfo1);
__update_guest_eip(regs, vmcb->nextrip - vmcb->rip);
@@ -2994,7 +2968,7 @@ void asmlinkage svm_vmexit_handler(void)
break;
case VMEXIT_NPF:
- if ( cpu_has_svm_decode )
+ if ( cpu_has_decode_assist )
v->arch.hvm.svm.cached_insn_len = vmcb->guest_ins_len & 0xf;
rc = vmcb->ei.npf.ec & PFEC_page_present
? p2m_pt_handle_deferred_changes(vmcb->ei.npf.gpa) : 0;
diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
index 4e1f61dbe038..4452ab1263d4 100644
--- a/xen/arch/x86/hvm/svm/vmcb.c
+++ b/xen/arch/x86/hvm/svm/vmcb.c
@@ -15,7 +15,6 @@
#include <asm/hvm/svm/vmcb.h>
#include <asm/msr-index.h>
#include <asm/p2m.h>
-#include <asm/hvm/svm/svm.h>
#include <asm/hvm/svm/svmdebug.h>
#include <asm/spec_ctrl.h>
diff --git a/xen/arch/x86/include/asm/cpufeature.h b/xen/arch/x86/include/asm/cpufeature.h
index 77cfd900cb56..b6fb8c24423c 100644
--- a/xen/arch/x86/include/asm/cpufeature.h
+++ b/xen/arch/x86/include/asm/cpufeature.h
@@ -218,6 +218,16 @@ static inline bool boot_cpu_has(unsigned int feat)
#define cpu_has_rfds_clear boot_cpu_has(X86_FEATURE_RFDS_CLEAR)
/* CPUID level 0x8000000a.edx */
+#define cpu_has_npt boot_cpu_has(X86_FEATURE_NPT)
+#define cpu_has_v_lbr boot_cpu_has(X86_FEATURE_V_LBR)
+#define cpu_has_nrips boot_cpu_has(X86_FEATURE_NRIPS)
+#define cpu_has_tsc_ratio boot_cpu_has(X86_FEATURE_V_TSC_RATE)
+#define cpu_has_flush_by_asid boot_cpu_has(X86_FEATURE_FLUSH_BY_ASID)
+#define cpu_has_decode_assist boot_cpu_has(X86_FEATURE_DECODE_ASSIST)
+#define cpu_has_pause_filter boot_cpu_has(X86_FEATURE_PAUSE_FILTER)
+#define cpu_has_pause_thresh boot_cpu_has(X86_FEATURE_PAUSE_THRESH)
+#define cpu_has_v_loadsave boot_cpu_has(X86_FEATURE_V_LOADSAVE)
+#define cpu_has_v_gif boot_cpu_has(X86_FEATURE_V_GIF)
#define cpu_has_v_spec_ctrl boot_cpu_has(X86_FEATURE_V_SPEC_CTRL)
/* Synthesized. */
diff --git a/xen/arch/x86/include/asm/hvm/svm/svm.h b/xen/arch/x86/include/asm/hvm/svm/svm.h
index 4eeeb25da90c..06a951225e64 100644
--- a/xen/arch/x86/include/asm/hvm/svm/svm.h
+++ b/xen/arch/x86/include/asm/hvm/svm/svm.h
@@ -21,40 +21,4 @@ bool svm_load_segs(unsigned int ldt_ents, unsigned long ldt_base,
unsigned long fs_base, unsigned long gs_base,
unsigned long gs_shadow);
-extern u32 svm_feature_flags;
-
-#define SVM_FEATURE_NPT 0 /* Nested page table support */
-#define SVM_FEATURE_LBRV 1 /* LBR virtualization support */
-#define SVM_FEATURE_SVML 2 /* SVM locking MSR support */
-#define SVM_FEATURE_NRIPS 3 /* Next RIP save on VMEXIT support */
-#define SVM_FEATURE_TSCRATEMSR 4 /* TSC ratio MSR support */
-#define SVM_FEATURE_VMCBCLEAN 5 /* VMCB clean bits support */
-#define SVM_FEATURE_FLUSHBYASID 6 /* TLB flush by ASID support */
-#define SVM_FEATURE_DECODEASSISTS 7 /* Decode assists support */
-#define SVM_FEATURE_PAUSEFILTER 10 /* Pause intercept filter support */
-#define SVM_FEATURE_PAUSETHRESH 12 /* Pause intercept filter support */
-#define SVM_FEATURE_VLOADSAVE 15 /* virtual vmload/vmsave */
-#define SVM_FEATURE_VGIF 16 /* Virtual GIF */
-#define SVM_FEATURE_SSS 19 /* NPT Supervisor Shadow Stacks */
-#define SVM_FEATURE_SPEC_CTRL 20 /* MSR_SPEC_CTRL virtualisation */
-
-static inline bool cpu_has_svm_feature(unsigned int feat)
-{
- return svm_feature_flags & (1u << feat);
-}
-#define cpu_has_svm_npt cpu_has_svm_feature(SVM_FEATURE_NPT)
-#define cpu_has_svm_lbrv cpu_has_svm_feature(SVM_FEATURE_LBRV)
-#define cpu_has_svm_svml cpu_has_svm_feature(SVM_FEATURE_SVML)
-#define cpu_has_svm_nrips cpu_has_svm_feature(SVM_FEATURE_NRIPS)
-#define cpu_has_svm_cleanbits cpu_has_svm_feature(SVM_FEATURE_VMCBCLEAN)
-#define cpu_has_svm_flushbyasid cpu_has_svm_feature(SVM_FEATURE_FLUSHBYASID)
-#define cpu_has_svm_decode cpu_has_svm_feature(SVM_FEATURE_DECODEASSISTS)
-#define cpu_has_svm_vgif cpu_has_svm_feature(SVM_FEATURE_VGIF)
-#define cpu_has_pause_filter cpu_has_svm_feature(SVM_FEATURE_PAUSEFILTER)
-#define cpu_has_pause_thresh cpu_has_svm_feature(SVM_FEATURE_PAUSETHRESH)
-#define cpu_has_tsc_ratio cpu_has_svm_feature(SVM_FEATURE_TSCRATEMSR)
-#define cpu_has_svm_vloadsave cpu_has_svm_feature(SVM_FEATURE_VLOADSAVE)
-#define cpu_has_svm_sss cpu_has_svm_feature(SVM_FEATURE_SSS)
-#define cpu_has_svm_spec_ctrl cpu_has_svm_feature(SVM_FEATURE_SPEC_CTRL)
-
#endif /* __ASM_X86_HVM_SVM_H__ */
--
2.30.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 5/5] x86/cpu-policy: Introduce some SEV features
2024-04-29 15:16 [PATCH 0/5] x86: AMD CPUID handling improvements Andrew Cooper
` (3 preceding siblings ...)
2024-04-29 15:16 ` [PATCH 4/5] x86/svm: Switch SVM features over normal cpu_has_* Andrew Cooper
@ 2024-04-29 15:16 ` Andrew Cooper
2024-04-30 6:15 ` Vaishali Thakkar
2024-04-30 13:54 ` Jan Beulich
4 siblings, 2 replies; 20+ messages in thread
From: Andrew Cooper @ 2024-04-29 15:16 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou, Sergiy Kibrik,
George Dunlap, Andrei Semenov, Vaishali Thakkar
For display purposes only right now.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Xenia Ragiadakou <xenia.ragiadakou@amd.com>
CC: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
CC: George Dunlap <george.dunlap@citrix.com>
CC: Andrei Semenov <andrei.semenov@vates.fr>
CC: Vaishali Thakkar <vaishali.thakkar@vates.tech>
This is only half the work to get SEV working nicely. The other
half (rearranging __start_xen() so we can move the host policy collection
earlier) is still a work-in-progress.
---
tools/misc/xen-cpuid.c | 3 +++
xen/arch/x86/include/asm/cpufeature.h | 3 +++
xen/include/public/arch-x86/cpufeatureset.h | 4 ++++
xen/tools/gen-cpuid.py | 6 +++++-
4 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
index 0d01b0e797f1..1463e0429ba1 100644
--- a/tools/misc/xen-cpuid.c
+++ b/tools/misc/xen-cpuid.c
@@ -281,6 +281,9 @@ static const char *const str_eAd[32] =
static const char *const str_e1Fa[32] =
{
+ [ 0] = "sme", [ 1] = "sev",
+ /* 2 */ [ 3] = "sev-es",
+ [ 4] = "sev-snp",
};
static const struct {
diff --git a/xen/arch/x86/include/asm/cpufeature.h b/xen/arch/x86/include/asm/cpufeature.h
index b6fb8c24423c..732f0d2bf758 100644
--- a/xen/arch/x86/include/asm/cpufeature.h
+++ b/xen/arch/x86/include/asm/cpufeature.h
@@ -230,6 +230,9 @@ static inline bool boot_cpu_has(unsigned int feat)
#define cpu_has_v_gif boot_cpu_has(X86_FEATURE_V_GIF)
#define cpu_has_v_spec_ctrl boot_cpu_has(X86_FEATURE_V_SPEC_CTRL)
+/* CPUID level 0x8000001f.eax */
+#define cpu_has_sev boot_cpu_has(X86_FEATURE_SEV)
+
/* Synthesized. */
#define cpu_has_arch_perfmon boot_cpu_has(X86_FEATURE_ARCH_PERFMON)
#define cpu_has_cpuid_faulting boot_cpu_has(X86_FEATURE_CPUID_FAULTING)
diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
index 80d252a38c2d..7ee0f2329151 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -374,6 +374,10 @@ XEN_CPUFEATURE(NPT_SSS, 18*32+19) /* NPT Supervisor Shadow Stacks *
XEN_CPUFEATURE(V_SPEC_CTRL, 18*32+20) /* Virtualised MSR_SPEC_CTRL */
/* AMD-defined CPU features, CPUID level 0x8000001f.eax, word 19 */
+XEN_CPUFEATURE(SME, 19*32+ 0) /* Secure Memory Encryption */
+XEN_CPUFEATURE(SEV, 19*32+ 1) /* Secure Encryped VM */
+XEN_CPUFEATURE(SEV_ES, 19*32+ 3) /* SEV Encrypted State */
+XEN_CPUFEATURE(SEV_SNP, 19*32+ 4) /* SEV Secure Nested Paging */
#endif /* XEN_CPUFEATURE */
diff --git a/xen/tools/gen-cpuid.py b/xen/tools/gen-cpuid.py
index f07b1f4cf905..bff4d9389ff6 100755
--- a/xen/tools/gen-cpuid.py
+++ b/xen/tools/gen-cpuid.py
@@ -281,7 +281,7 @@ def crunch_numbers(state):
_3DNOW: [_3DNOWEXT],
# The SVM bit enumerates the whole SVM leave.
- SVM: list(range(NPT, NPT + 32)),
+ SVM: list(range(NPT, NPT + 32)) + [SEV],
# This is just the dependency between AVX512 and AVX2 of XSTATE
# feature flags. If want to use AVX512, AVX2 must be supported and
@@ -341,6 +341,10 @@ def crunch_numbers(state):
# The behaviour described by RRSBA depend on eIBRS being active.
EIBRS: [RRSBA],
+
+ SEV: [SEV_ES],
+
+ SEV_ES: [SEV_SNP],
}
deep_features = tuple(sorted(deps.keys()))
--
2.30.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/5] x86/cpu-policy: Add SVM features already used by Xen
2024-04-29 15:16 ` [PATCH 2/5] x86/cpu-policy: Add SVM features already used by Xen Andrew Cooper
@ 2024-04-29 15:24 ` Andrew Cooper
2024-04-30 13:02 ` Jan Beulich
2024-05-01 10:00 ` George Dunlap
2 siblings, 0 replies; 20+ messages in thread
From: Andrew Cooper @ 2024-04-29 15:24 UTC (permalink / raw)
To: Xen-devel
Cc: Jan Beulich, Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou, Sergiy Kibrik,
George Dunlap, Andrei Semenov, Vaishali Thakkar
On 29/04/2024 4:16 pm, Andrew Cooper wrote:
> diff --git a/xen/tools/gen-cpuid.py b/xen/tools/gen-cpuid.py
> index bf3f9ec01e6e..f07b1f4cf905 100755
> --- a/xen/tools/gen-cpuid.py
> +++ b/xen/tools/gen-cpuid.py
> @@ -280,6 +280,9 @@ def crunch_numbers(state):
> # standard 3DNow in the earlier K6 processors.
> _3DNOW: [_3DNOWEXT],
>
> + # The SVM bit enumerates the whole SVM leave.
> + SVM: list(range(NPT, NPT + 32)),
This should say leaf. Fixed locally.
~Andrew
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/5] x86/svm: Switch SVM features over normal cpu_has_*
2024-04-29 15:16 ` [PATCH 4/5] x86/svm: Switch SVM features over normal cpu_has_* Andrew Cooper
@ 2024-04-30 5:51 ` Vaishali Thakkar
2024-04-30 13:25 ` Jan Beulich
1 sibling, 0 replies; 20+ messages in thread
From: Vaishali Thakkar @ 2024-04-30 5:51 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel
Cc: Jan Beulich, Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou, Sergiy Kibrik,
George Dunlap, Andrei Semenov
On 4/29/24 5:16 PM, Andrew Cooper wrote:
> Delete the boot time rendering of advanced features. It's entirely ad-hoc and
> not even everything printed here is used by Xen. It is available in
> `xen-cpuid` now.
>
> With (only) svm_load_segs_{,prefetch}() declared now in svm.h, only svm.c and
> domain.c which need the header. Clean up all others.
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Vaishali Thakkar <vaishali.thakkar@vates.tech>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Xenia Ragiadakou <xenia.ragiadakou@amd.com>
> CC: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
> CC: George Dunlap <george.dunlap@citrix.com>
> CC: Andrei Semenov <andrei.semenov@vates.fr>
> CC: Vaishali Thakkar <vaishali.thakkar@vates.tech>
> ---
> xen/arch/x86/hvm/svm/asid.c | 5 ++-
> xen/arch/x86/hvm/svm/emulate.c | 3 +-
> xen/arch/x86/hvm/svm/intr.c | 1 -
> xen/arch/x86/hvm/svm/nestedsvm.c | 14 ++++----
> xen/arch/x86/hvm/svm/svm.c | 50 +++++++-------------------
> xen/arch/x86/hvm/svm/vmcb.c | 1 -
> xen/arch/x86/include/asm/cpufeature.h | 10 ++++++
> xen/arch/x86/include/asm/hvm/svm/svm.h | 36 -------------------
> 8 files changed, 31 insertions(+), 89 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/svm/asid.c b/xen/arch/x86/hvm/svm/asid.c
> index 7977a8e86b53..6117a362d310 100644
> --- a/xen/arch/x86/hvm/svm/asid.c
> +++ b/xen/arch/x86/hvm/svm/asid.c
> @@ -6,7 +6,6 @@
>
> #include <asm/amd.h>
> #include <asm/hvm/nestedhvm.h>
> -#include <asm/hvm/svm/svm.h>
>
> #include "svm.h"
>
> @@ -39,7 +38,7 @@ void svm_asid_handle_vmrun(void)
> {
> vmcb_set_asid(vmcb, true);
> vmcb->tlb_control =
> - cpu_has_svm_flushbyasid ? TLB_CTRL_FLUSH_ASID : TLB_CTRL_FLUSH_ALL;
> + cpu_has_flush_by_asid ? TLB_CTRL_FLUSH_ASID : TLB_CTRL_FLUSH_ALL;
> return;
> }
>
> @@ -48,7 +47,7 @@ void svm_asid_handle_vmrun(void)
>
> vmcb->tlb_control =
> !need_flush ? TLB_CTRL_NO_FLUSH :
> - cpu_has_svm_flushbyasid ? TLB_CTRL_FLUSH_ASID : TLB_CTRL_FLUSH_ALL;
> + cpu_has_flush_by_asid ? TLB_CTRL_FLUSH_ASID : TLB_CTRL_FLUSH_ALL;
> }
>
> /*
> diff --git a/xen/arch/x86/hvm/svm/emulate.c b/xen/arch/x86/hvm/svm/emulate.c
> index 93ac1d3435f9..da6e21b2e270 100644
> --- a/xen/arch/x86/hvm/svm/emulate.c
> +++ b/xen/arch/x86/hvm/svm/emulate.c
> @@ -11,7 +11,6 @@
> #include <asm/msr.h>
> #include <asm/hvm/emulate.h>
> #include <asm/hvm/hvm.h>
> -#include <asm/hvm/svm/svm.h>
> #include <asm/hvm/svm/vmcb.h>
>
> #include "svm.h"
> @@ -20,7 +19,7 @@ static unsigned long svm_nextrip_insn_length(struct vcpu *v)
> {
> struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
>
> - if ( !cpu_has_svm_nrips )
> + if ( !cpu_has_nrips )
> return 0;
>
> #ifndef NDEBUG
> diff --git a/xen/arch/x86/hvm/svm/intr.c b/xen/arch/x86/hvm/svm/intr.c
> index 4805c5567213..facd2894a2c6 100644
> --- a/xen/arch/x86/hvm/svm/intr.c
> +++ b/xen/arch/x86/hvm/svm/intr.c
> @@ -17,7 +17,6 @@
> #include <asm/hvm/hvm.h>
> #include <asm/hvm/io.h>
> #include <asm/hvm/vlapic.h>
> -#include <asm/hvm/svm/svm.h>
> #include <asm/hvm/nestedhvm.h> /* for nestedhvm_vcpu_in_guestmode */
> #include <asm/vm_event.h>
> #include <xen/event.h>
> diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
> index 35a2cbfd7d13..255af112661f 100644
> --- a/xen/arch/x86/hvm/svm/nestedsvm.c
> +++ b/xen/arch/x86/hvm/svm/nestedsvm.c
> @@ -6,7 +6,6 @@
> */
>
> #include <asm/hvm/support.h>
> -#include <asm/hvm/svm/svm.h>
> #include <asm/hvm/svm/vmcb.h>
> #include <asm/hvm/nestedhvm.h>
> #include <asm/hvm/svm/svmdebug.h>
> @@ -1620,7 +1619,7 @@ void svm_nested_features_on_efer_update(struct vcpu *v)
> {
> if ( !vmcb->virt_ext.fields.vloadsave_enable &&
> paging_mode_hap(v->domain) &&
> - cpu_has_svm_vloadsave )
> + cpu_has_v_loadsave )
> {
> vmcb->virt_ext.fields.vloadsave_enable = 1;
> general2_intercepts = vmcb_get_general2_intercepts(vmcb);
> @@ -1629,8 +1628,7 @@ void svm_nested_features_on_efer_update(struct vcpu *v)
> vmcb_set_general2_intercepts(vmcb, general2_intercepts);
> }
>
> - if ( !vmcb->_vintr.fields.vgif_enable &&
> - cpu_has_svm_vgif )
> + if ( !vmcb->_vintr.fields.vgif_enable && cpu_has_v_gif )
> {
> vintr = vmcb_get_vintr(vmcb);
> vintr.fields.vgif = svm->ns_gif;
> @@ -1675,8 +1673,8 @@ void __init start_nested_svm(struct hvm_function_table *hvm_function_table)
> */
> hvm_function_table->caps.nested_virt =
> hvm_function_table->caps.hap &&
> - cpu_has_svm_lbrv &&
> - cpu_has_svm_nrips &&
> - cpu_has_svm_flushbyasid &&
> - cpu_has_svm_decode;
> + cpu_has_v_lbr &&
> + cpu_has_nrips &&
> + cpu_has_flush_by_asid &&
> + cpu_has_decode_assist;
> }
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index 4719fffae589..16eb875aab94 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -1287,7 +1287,7 @@ static void cf_check svm_inject_event(const struct x86_event *event)
> * that hardware doesn't perform DPL checking on injection.
> */
> if ( event->type == X86_EVENTTYPE_PRI_SW_EXCEPTION ||
> - (!cpu_has_svm_nrips && (event->type >= X86_EVENTTYPE_SW_INTERRUPT)) )
> + (!cpu_has_nrips && (event->type >= X86_EVENTTYPE_SW_INTERRUPT)) )
> svm_emul_swint_injection(&_event);
>
> switch ( _event.vector | -(_event.type == X86_EVENTTYPE_SW_INTERRUPT) )
> @@ -1341,7 +1341,7 @@ static void cf_check svm_inject_event(const struct x86_event *event)
> switch ( _event.type )
> {
> case X86_EVENTTYPE_SW_INTERRUPT: /* int $n */
> - if ( cpu_has_svm_nrips )
> + if ( cpu_has_nrips )
> vmcb->nextrip = regs->rip + _event.insn_len;
> else
> regs->rip += _event.insn_len;
> @@ -1355,7 +1355,7 @@ static void cf_check svm_inject_event(const struct x86_event *event)
> * semantics.
> */
> regs->rip += _event.insn_len;
> - if ( cpu_has_svm_nrips )
> + if ( cpu_has_nrips )
> vmcb->nextrip = regs->rip;
> eventinj.type = X86_EVENTTYPE_HW_EXCEPTION;
> break;
> @@ -1365,7 +1365,7 @@ static void cf_check svm_inject_event(const struct x86_event *event)
> * Hardware special cases HW_EXCEPTION with vectors 3 and 4 as having
> * trap semantics, and will perform DPL checks.
> */
> - if ( cpu_has_svm_nrips )
> + if ( cpu_has_nrips )
> vmcb->nextrip = regs->rip + _event.insn_len;
> else
> regs->rip += _event.insn_len;
> @@ -1982,7 +1982,7 @@ static int cf_check svm_msr_write_intercept(
>
> case MSR_IA32_DEBUGCTLMSR:
> vmcb_set_debugctlmsr(vmcb, msr_content);
> - if ( !msr_content || !cpu_has_svm_lbrv )
> + if ( !msr_content || !cpu_has_v_lbr )
> break;
> vmcb->virt_ext.fields.lbr_enable = 1;
> svm_disable_intercept_for_msr(v, MSR_IA32_DEBUGCTLMSR);
> @@ -2480,8 +2480,6 @@ static struct hvm_function_table __initdata_cf_clobber svm_function_table = {
>
> const struct hvm_function_table * __init start_svm(void)
> {
> - bool printed = false;
> -
> svm_host_osvw_reset();
>
> if ( _svm_cpu_up(true) )
> @@ -2493,38 +2491,14 @@ const struct hvm_function_table * __init start_svm(void)
>
> setup_vmcb_dump();
>
> - if ( boot_cpu_data.extended_cpuid_level >= 0x8000000aU )
> - svm_feature_flags = cpuid_edx(0x8000000aU);
> -
> - printk("SVM: Supported advanced features:\n");
> -
> /* DecodeAssists fast paths assume nextrip is valid for fast rIP update. */
> - if ( !cpu_has_svm_nrips )
> - __clear_bit(SVM_FEATURE_DECODEASSISTS, &svm_feature_flags);
> + if ( !cpu_has_nrips )
> + setup_clear_cpu_cap(X86_FEATURE_DECODE_ASSIST);
>
> if ( cpu_has_tsc_ratio )
> svm_function_table.tsc_scaling.ratio_frac_bits = 32;
>
> -#define P(p,s) if ( p ) { printk(" - %s\n", s); printed = 1; }
> - P(cpu_has_svm_npt, "Nested Page Tables (NPT)");
> - P(cpu_has_svm_lbrv, "Last Branch Record (LBR) Virtualisation");
> - P(cpu_has_svm_nrips, "Next-RIP Saved on #VMEXIT");
> - P(cpu_has_svm_cleanbits, "VMCB Clean Bits");
> - P(cpu_has_svm_flushbyasid, "TLB flush by ASID");
> - P(cpu_has_svm_decode, "DecodeAssists");
> - P(cpu_has_svm_vloadsave, "Virtual VMLOAD/VMSAVE");
> - P(cpu_has_svm_vgif, "Virtual GIF");
> - P(cpu_has_pause_filter, "Pause-Intercept Filter");
> - P(cpu_has_pause_thresh, "Pause-Intercept Filter Threshold");
> - P(cpu_has_tsc_ratio, "TSC Rate MSR");
> - P(cpu_has_svm_sss, "NPT Supervisor Shadow Stack");
> - P(cpu_has_svm_spec_ctrl, "MSR_SPEC_CTRL virtualisation");
> -#undef P
> -
> - if ( !printed )
> - printk(" - none\n");
> -
> - svm_function_table.caps.hap = cpu_has_svm_npt;
> + svm_function_table.caps.hap = cpu_has_npt;
> svm_function_table.caps.hap_superpage_2mb = true;
> svm_function_table.caps.hap_superpage_1gb = cpu_has_page1gb;
>
> @@ -2761,7 +2735,7 @@ void asmlinkage svm_vmexit_handler(void)
> regs->rax, regs->rbx, regs->rcx,
> regs->rdx, regs->rsi, regs->rdi);
>
> - if ( cpu_has_svm_decode )
> + if ( cpu_has_decode_assist )
> v->arch.hvm.svm.cached_insn_len = vmcb->guest_ins_len & 0xf;
> rc = paging_fault(va, regs);
> v->arch.hvm.svm.cached_insn_len = 0;
> @@ -2906,14 +2880,14 @@ void asmlinkage svm_vmexit_handler(void)
>
> case VMEXIT_CR0_READ ... VMEXIT_CR15_READ:
> case VMEXIT_CR0_WRITE ... VMEXIT_CR15_WRITE:
> - if ( cpu_has_svm_decode && vmcb->ei.mov_cr.mov_insn )
> + if ( cpu_has_decode_assist && vmcb->ei.mov_cr.mov_insn )
> svm_vmexit_do_cr_access(vmcb, regs);
> else if ( !hvm_emulate_one_insn(x86_insn_is_cr_access, "CR access") )
> hvm_inject_hw_exception(X86_EXC_GP, 0);
> break;
>
> case VMEXIT_INVLPG:
> - if ( cpu_has_svm_decode )
> + if ( cpu_has_decode_assist )
> {
> svm_invlpg_intercept(vmcb->exitinfo1);
> __update_guest_eip(regs, vmcb->nextrip - vmcb->rip);
> @@ -2994,7 +2968,7 @@ void asmlinkage svm_vmexit_handler(void)
> break;
>
> case VMEXIT_NPF:
> - if ( cpu_has_svm_decode )
> + if ( cpu_has_decode_assist )
> v->arch.hvm.svm.cached_insn_len = vmcb->guest_ins_len & 0xf;
> rc = vmcb->ei.npf.ec & PFEC_page_present
> ? p2m_pt_handle_deferred_changes(vmcb->ei.npf.gpa) : 0;
> diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
> index 4e1f61dbe038..4452ab1263d4 100644
> --- a/xen/arch/x86/hvm/svm/vmcb.c
> +++ b/xen/arch/x86/hvm/svm/vmcb.c
> @@ -15,7 +15,6 @@
> #include <asm/hvm/svm/vmcb.h>
> #include <asm/msr-index.h>
> #include <asm/p2m.h>
> -#include <asm/hvm/svm/svm.h>
> #include <asm/hvm/svm/svmdebug.h>
> #include <asm/spec_ctrl.h>
>
> diff --git a/xen/arch/x86/include/asm/cpufeature.h b/xen/arch/x86/include/asm/cpufeature.h
> index 77cfd900cb56..b6fb8c24423c 100644
> --- a/xen/arch/x86/include/asm/cpufeature.h
> +++ b/xen/arch/x86/include/asm/cpufeature.h
> @@ -218,6 +218,16 @@ static inline bool boot_cpu_has(unsigned int feat)
> #define cpu_has_rfds_clear boot_cpu_has(X86_FEATURE_RFDS_CLEAR)
>
> /* CPUID level 0x8000000a.edx */
> +#define cpu_has_npt boot_cpu_has(X86_FEATURE_NPT)
> +#define cpu_has_v_lbr boot_cpu_has(X86_FEATURE_V_LBR)
> +#define cpu_has_nrips boot_cpu_has(X86_FEATURE_NRIPS)
> +#define cpu_has_tsc_ratio boot_cpu_has(X86_FEATURE_V_TSC_RATE)
> +#define cpu_has_flush_by_asid boot_cpu_has(X86_FEATURE_FLUSH_BY_ASID)
> +#define cpu_has_decode_assist boot_cpu_has(X86_FEATURE_DECODE_ASSIST)
> +#define cpu_has_pause_filter boot_cpu_has(X86_FEATURE_PAUSE_FILTER)
> +#define cpu_has_pause_thresh boot_cpu_has(X86_FEATURE_PAUSE_THRESH)
> +#define cpu_has_v_loadsave boot_cpu_has(X86_FEATURE_V_LOADSAVE)
> +#define cpu_has_v_gif boot_cpu_has(X86_FEATURE_V_GIF)
> #define cpu_has_v_spec_ctrl boot_cpu_has(X86_FEATURE_V_SPEC_CTRL)
>
> /* Synthesized. */
> diff --git a/xen/arch/x86/include/asm/hvm/svm/svm.h b/xen/arch/x86/include/asm/hvm/svm/svm.h
> index 4eeeb25da90c..06a951225e64 100644
> --- a/xen/arch/x86/include/asm/hvm/svm/svm.h
> +++ b/xen/arch/x86/include/asm/hvm/svm/svm.h
> @@ -21,40 +21,4 @@ bool svm_load_segs(unsigned int ldt_ents, unsigned long ldt_base,
> unsigned long fs_base, unsigned long gs_base,
> unsigned long gs_shadow);
>
> -extern u32 svm_feature_flags;
> -
> -#define SVM_FEATURE_NPT 0 /* Nested page table support */
> -#define SVM_FEATURE_LBRV 1 /* LBR virtualization support */
> -#define SVM_FEATURE_SVML 2 /* SVM locking MSR support */
> -#define SVM_FEATURE_NRIPS 3 /* Next RIP save on VMEXIT support */
> -#define SVM_FEATURE_TSCRATEMSR 4 /* TSC ratio MSR support */
> -#define SVM_FEATURE_VMCBCLEAN 5 /* VMCB clean bits support */
> -#define SVM_FEATURE_FLUSHBYASID 6 /* TLB flush by ASID support */
> -#define SVM_FEATURE_DECODEASSISTS 7 /* Decode assists support */
> -#define SVM_FEATURE_PAUSEFILTER 10 /* Pause intercept filter support */
> -#define SVM_FEATURE_PAUSETHRESH 12 /* Pause intercept filter support */
> -#define SVM_FEATURE_VLOADSAVE 15 /* virtual vmload/vmsave */
> -#define SVM_FEATURE_VGIF 16 /* Virtual GIF */
> -#define SVM_FEATURE_SSS 19 /* NPT Supervisor Shadow Stacks */
> -#define SVM_FEATURE_SPEC_CTRL 20 /* MSR_SPEC_CTRL virtualisation */
> -
> -static inline bool cpu_has_svm_feature(unsigned int feat)
> -{
> - return svm_feature_flags & (1u << feat);
> -}
> -#define cpu_has_svm_npt cpu_has_svm_feature(SVM_FEATURE_NPT)
> -#define cpu_has_svm_lbrv cpu_has_svm_feature(SVM_FEATURE_LBRV)
> -#define cpu_has_svm_svml cpu_has_svm_feature(SVM_FEATURE_SVML)
> -#define cpu_has_svm_nrips cpu_has_svm_feature(SVM_FEATURE_NRIPS)
> -#define cpu_has_svm_cleanbits cpu_has_svm_feature(SVM_FEATURE_VMCBCLEAN)
> -#define cpu_has_svm_flushbyasid cpu_has_svm_feature(SVM_FEATURE_FLUSHBYASID)
> -#define cpu_has_svm_decode cpu_has_svm_feature(SVM_FEATURE_DECODEASSISTS)
> -#define cpu_has_svm_vgif cpu_has_svm_feature(SVM_FEATURE_VGIF)
> -#define cpu_has_pause_filter cpu_has_svm_feature(SVM_FEATURE_PAUSEFILTER)
> -#define cpu_has_pause_thresh cpu_has_svm_feature(SVM_FEATURE_PAUSETHRESH)
> -#define cpu_has_tsc_ratio cpu_has_svm_feature(SVM_FEATURE_TSCRATEMSR)
> -#define cpu_has_svm_vloadsave cpu_has_svm_feature(SVM_FEATURE_VLOADSAVE)
> -#define cpu_has_svm_sss cpu_has_svm_feature(SVM_FEATURE_SSS)
> -#define cpu_has_svm_spec_ctrl cpu_has_svm_feature(SVM_FEATURE_SPEC_CTRL)
> -
> #endif /* __ASM_X86_HVM_SVM_H__ */
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 5/5] x86/cpu-policy: Introduce some SEV features
2024-04-29 15:16 ` [PATCH 5/5] x86/cpu-policy: Introduce some SEV features Andrew Cooper
@ 2024-04-30 6:15 ` Vaishali Thakkar
2024-04-30 13:54 ` Jan Beulich
1 sibling, 0 replies; 20+ messages in thread
From: Vaishali Thakkar @ 2024-04-30 6:15 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel
Cc: Jan Beulich, Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou, Sergiy Kibrik,
George Dunlap, Andrei Semenov
On 4/29/24 5:16 PM, Andrew Cooper wrote:
> For display purposes only right now.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Vaishali Thakkar <vaishali.thakkar@vates.tech>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Xenia Ragiadakou <xenia.ragiadakou@amd.com>
> CC: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
> CC: George Dunlap <george.dunlap@citrix.com>
> CC: Andrei Semenov <andrei.semenov@vates.fr>
> CC: Vaishali Thakkar <vaishali.thakkar@vates.tech>
>
> This is only half the work to get SEV working nicely. The other
> half (rearranging __start_xen() so we can move the host policy collection
> earlier) is still a work-in-progress.
> ---
> tools/misc/xen-cpuid.c | 3 +++
> xen/arch/x86/include/asm/cpufeature.h | 3 +++
> xen/include/public/arch-x86/cpufeatureset.h | 4 ++++
> xen/tools/gen-cpuid.py | 6 +++++-
> 4 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
> index 0d01b0e797f1..1463e0429ba1 100644
> --- a/tools/misc/xen-cpuid.c
> +++ b/tools/misc/xen-cpuid.c
> @@ -281,6 +281,9 @@ static const char *const str_eAd[32] =
>
> static const char *const str_e1Fa[32] =
> {
> + [ 0] = "sme", [ 1] = "sev",
> + /* 2 */ [ 3] = "sev-es",
> + [ 4] = "sev-snp",
> };
>
> static const struct {
> diff --git a/xen/arch/x86/include/asm/cpufeature.h b/xen/arch/x86/include/asm/cpufeature.h
> index b6fb8c24423c..732f0d2bf758 100644
> --- a/xen/arch/x86/include/asm/cpufeature.h
> +++ b/xen/arch/x86/include/asm/cpufeature.h
> @@ -230,6 +230,9 @@ static inline bool boot_cpu_has(unsigned int feat)
> #define cpu_has_v_gif boot_cpu_has(X86_FEATURE_V_GIF)
> #define cpu_has_v_spec_ctrl boot_cpu_has(X86_FEATURE_V_SPEC_CTRL)
>
> +/* CPUID level 0x8000001f.eax */
> +#define cpu_has_sev boot_cpu_has(X86_FEATURE_SEV)
> +
> /* Synthesized. */
> #define cpu_has_arch_perfmon boot_cpu_has(X86_FEATURE_ARCH_PERFMON)
> #define cpu_has_cpuid_faulting boot_cpu_has(X86_FEATURE_CPUID_FAULTING)
> diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
> index 80d252a38c2d..7ee0f2329151 100644
> --- a/xen/include/public/arch-x86/cpufeatureset.h
> +++ b/xen/include/public/arch-x86/cpufeatureset.h
> @@ -374,6 +374,10 @@ XEN_CPUFEATURE(NPT_SSS, 18*32+19) /* NPT Supervisor Shadow Stacks *
> XEN_CPUFEATURE(V_SPEC_CTRL, 18*32+20) /* Virtualised MSR_SPEC_CTRL */
>
> /* AMD-defined CPU features, CPUID level 0x8000001f.eax, word 19 */
> +XEN_CPUFEATURE(SME, 19*32+ 0) /* Secure Memory Encryption */
> +XEN_CPUFEATURE(SEV, 19*32+ 1) /* Secure Encryped VM */
> +XEN_CPUFEATURE(SEV_ES, 19*32+ 3) /* SEV Encrypted State */
> +XEN_CPUFEATURE(SEV_SNP, 19*32+ 4) /* SEV Secure Nested Paging */
>
> #endif /* XEN_CPUFEATURE */
>
> diff --git a/xen/tools/gen-cpuid.py b/xen/tools/gen-cpuid.py
> index f07b1f4cf905..bff4d9389ff6 100755
> --- a/xen/tools/gen-cpuid.py
> +++ b/xen/tools/gen-cpuid.py
> @@ -281,7 +281,7 @@ def crunch_numbers(state):
> _3DNOW: [_3DNOWEXT],
>
> # The SVM bit enumerates the whole SVM leave.
> - SVM: list(range(NPT, NPT + 32)),
> + SVM: list(range(NPT, NPT + 32)) + [SEV],
>
> # This is just the dependency between AVX512 and AVX2 of XSTATE
> # feature flags. If want to use AVX512, AVX2 must be supported and
> @@ -341,6 +341,10 @@ def crunch_numbers(state):
>
> # The behaviour described by RRSBA depend on eIBRS being active.
> EIBRS: [RRSBA],
> +
> + SEV: [SEV_ES],
> +
> + SEV_ES: [SEV_SNP],
> }
>
> deep_features = tuple(sorted(deps.keys()))
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/5] x86/cpu-policy: Infrastructure for the AMD SVM and SEV leaves
2024-04-29 15:16 ` [PATCH 1/5] x86/cpu-policy: Infrastructure for the AMD SVM and SEV leaves Andrew Cooper
@ 2024-04-30 12:45 ` Jan Beulich
2024-04-30 13:25 ` Andrew Cooper
0 siblings, 1 reply; 20+ messages in thread
From: Jan Beulich @ 2024-04-30 12:45 UTC (permalink / raw)
To: Andrew Cooper
Cc: Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou, Sergiy Kibrik,
George Dunlap, Andrei Semenov, Vaishali Thakkar, Xen-devel
On 29.04.2024 17:16, Andrew Cooper wrote:
> Allocate two new feature leaves, and extend cpu_policy with the non-feature
> fields too.
>
> The CPUID dependency between the SVM bit on the whole SVM leaf is
> intentionally deferred, to avoid transiently breaking nested virt.
In reply to this I meant to ask that you at least add those dependencies in
commented-out form, such that from looking at gen-cpuid.py it becomes clear
they're intentionally omitted. But you don't add feature identifiers either,
making dependencies impossible to express. Maybe this sentence was really
meant for another of the patches? (Then my request would actually apply
there.)
> @@ -296,7 +298,16 @@ struct cpu_policy
> uint32_t /* d */:32;
>
> uint64_t :64, :64; /* Leaf 0x80000009. */
> - uint64_t :64, :64; /* Leaf 0x8000000a - SVM rev and features. */
> +
> + /* Leaf 0x8000000a - SVM rev and features. */
> + uint8_t svm_rev, :8, :8, :8;
> + uint32_t /* b */ :32;
> + uint32_t nr_asids;
According to the doc I'm looking at it is %ebx which holds the number of
ASIDs and %ecx is reserved. With that adjusted
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/5] x86/cpu-policy: Add SVM features already used by Xen
2024-04-29 15:16 ` [PATCH 2/5] x86/cpu-policy: Add SVM features already used by Xen Andrew Cooper
2024-04-29 15:24 ` Andrew Cooper
@ 2024-04-30 13:02 ` Jan Beulich
2024-05-01 10:00 ` George Dunlap
2 siblings, 0 replies; 20+ messages in thread
From: Jan Beulich @ 2024-04-30 13:02 UTC (permalink / raw)
To: Andrew Cooper
Cc: Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou, Sergiy Kibrik,
George Dunlap, Andrei Semenov, Vaishali Thakkar, Xen-devel
On 29.04.2024 17:16, Andrew Cooper wrote:
> --- a/xen/include/public/arch-x86/cpufeatureset.h
> +++ b/xen/include/public/arch-x86/cpufeatureset.h
> @@ -358,6 +358,20 @@ XEN_CPUFEATURE(RFDS_CLEAR, 16*32+28) /*!A Register File(s) cleared by VE
> /* Intel-defined CPU features, MSR_ARCH_CAPS 0x10a.edx, word 17 */
>
> /* AMD-defined CPU features, CPUID level 0x8000000a.edx, word 18 */
> +XEN_CPUFEATURE(NPT, 18*32+ 0) /*h Nested PageTables */
> +XEN_CPUFEATURE(V_LBR, 18*32+ 1) /*h Virtualised LBR */
> +XEN_CPUFEATURE(SVM_LOCK, 18*32+ 2) /* SVM locking MSR */
> +XEN_CPUFEATURE(NRIPS, 18*32+ 3) /*h Next-RIP saved on VMExit */
> +XEN_CPUFEATURE(V_TSC_RATE, 18*32+ 4) /* Virtualised TSC Ratio */
> +XEN_CPUFEATURE(VMCB_CLEANBITS, 18*32+ 5) /*! VMCB Clean-bits */
Wouldn't this better be marked !h nevertheless?
As to the name - does it need to be this long? VMCB_CLEAN would be in line
with the PM. But yeah, while CLEANBITS would be clear in the context of this
leaf, there might be whatever other "cleanbits" elsewhere, so some qualifier
is wanted, I guess. As to the V_ prefixes you use for several of the
features: Isn't there a risk of this being ambiguous towards VT-x? Maybe
they should all be SVM_*, even if ...
> +XEN_CPUFEATURE(FLUSH_BY_ASID, 18*32+ 6) /* TLB Flush by ASID */
> +XEN_CPUFEATURE(DECODE_ASSIST, 18*32+ 7) /*h Decode assists */
> +XEN_CPUFEATURE(PAUSE_FILTER, 18*32+10) /*h Pause filter */
> +XEN_CPUFEATURE(PAUSE_THRESH, 18*32+12) /* Pause filter threshold */
> +XEN_CPUFEATURE(V_LOADSAVE, 18*32+15) /* Virtualised VMLOAD/SAVE */
> +XEN_CPUFEATURE(V_GIF, 18*32+16) /* Virtualised GIF */
... these two at least are clearly SVM terminology and hence already
unambiguous.
> +XEN_CPUFEATURE(NPT_SSS, 18*32+19) /* NPT Supervisor Shadow Stacks */
> +XEN_CPUFEATURE(V_SPEC_CTRL, 18*32+20) /* Virtualised MSR_SPEC_CTRL */
Whereas this, when used somewhere in isolation, would not make clear
whether AMD's or Intel's is meant.
> --- a/xen/tools/gen-cpuid.py
> +++ b/xen/tools/gen-cpuid.py
> @@ -280,6 +280,9 @@ def crunch_numbers(state):
> # standard 3DNow in the earlier K6 processors.
> _3DNOW: [_3DNOWEXT],
>
> + # The SVM bit enumerates the whole SVM leave.
> + SVM: list(range(NPT, NPT + 32)),
Seeing this and taking it together with the somewhat confusing (to me) part
of the description of patch 1: What is it then that you try to avoid there,
when adding the dependencies here is okay, while doing so there would be
entirely impossible (short of there being identifiers)?
Jan
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/5] x86/spec-ctrl: Remove open-coded check of SVM_FEATURE_SPEC_CTRL
2024-04-29 15:16 ` [PATCH 3/5] x86/spec-ctrl: Remove open-coded check of SVM_FEATURE_SPEC_CTRL Andrew Cooper
@ 2024-04-30 13:13 ` Jan Beulich
0 siblings, 0 replies; 20+ messages in thread
From: Jan Beulich @ 2024-04-30 13:13 UTC (permalink / raw)
To: Andrew Cooper
Cc: Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou, Sergiy Kibrik,
George Dunlap, Andrei Semenov, Vaishali Thakkar, Xen-devel
On 29.04.2024 17:16, Andrew Cooper wrote:
> Now that the SVM feature leaf has been included in normal feature handling, it
> is available early enough for init_speculation_mitigations() to use.
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
even if ...
> --- a/xen/arch/x86/include/asm/cpufeature.h
> +++ b/xen/arch/x86/include/asm/cpufeature.h
> @@ -217,6 +217,9 @@ static inline bool boot_cpu_has(unsigned int feat)
> #define cpu_has_rfds_no boot_cpu_has(X86_FEATURE_RFDS_NO)
> #define cpu_has_rfds_clear boot_cpu_has(X86_FEATURE_RFDS_CLEAR)
>
> +/* CPUID level 0x8000000a.edx */
> +#define cpu_has_v_spec_ctrl boot_cpu_has(X86_FEATURE_V_SPEC_CTRL)
... the names here were to change (see comment on the earlier patch). In
fact ...
> @@ -1896,12 +1895,8 @@ void __init init_speculation_mitigations(void)
> *
> * No need for SCF_ist_sc_msr because Xen's value is restored
> * atomically WRT NMIs in the VMExit path.
> - *
> - * TODO: Adjust cpu_has_svm_spec_ctrl to be usable earlier on boot.
> */
> - if ( opt_msr_sc_hvm &&
> - (boot_cpu_data.extended_cpuid_level >= 0x8000000aU) &&
> - (cpuid_edx(0x8000000aU) & (1u << SVM_FEATURE_SPEC_CTRL)) )
> + if ( opt_msr_sc_hvm && cpu_has_v_spec_ctrl )
> setup_force_cpu_cap(X86_FEATURE_SC_MSR_HVM);
> }
... the use here demonstrates my earlier point quite well: It being AMD's
feature is completely invisible here when not considering the code being
replaced. But yes, when looking at the entire comment / block, it still
is visible.
Jan
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/5] x86/cpu-policy: Infrastructure for the AMD SVM and SEV leaves
2024-04-30 12:45 ` Jan Beulich
@ 2024-04-30 13:25 ` Andrew Cooper
2024-04-30 13:33 ` Jan Beulich
2024-05-01 9:16 ` George Dunlap
0 siblings, 2 replies; 20+ messages in thread
From: Andrew Cooper @ 2024-04-30 13:25 UTC (permalink / raw)
To: Jan Beulich
Cc: Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou, Sergiy Kibrik,
George Dunlap, Andrei Semenov, Vaishali Thakkar, Xen-devel
On 30/04/2024 1:45 pm, Jan Beulich wrote:
> On 29.04.2024 17:16, Andrew Cooper wrote:
>> Allocate two new feature leaves, and extend cpu_policy with the non-feature
>> fields too.
>>
>> The CPUID dependency between the SVM bit on the whole SVM leaf is
>> intentionally deferred, to avoid transiently breaking nested virt.
> In reply to this I meant to ask that you at least add those dependencies in
> commented-out form, such that from looking at gen-cpuid.py it becomes clear
> they're intentionally omitted. But you don't add feature identifiers either,
> making dependencies impossible to express. Maybe this sentence was really
> meant for another of the patches? (Then my request would actually apply
> there.)
This is necessary because c/s 4f8b0e94d7ca is buggy. Notice how it puts
an edit to the policy object in the middle of a block of logic editing
the featureset, which ends with writing the featureset back over the
policy object.
And it's not the first outstanding problem from what is a very small
number of nested-virt patches so far...
>> @@ -296,7 +298,16 @@ struct cpu_policy
>> uint32_t /* d */:32;
>>
>> uint64_t :64, :64; /* Leaf 0x80000009. */
>> - uint64_t :64, :64; /* Leaf 0x8000000a - SVM rev and features. */
>> +
>> + /* Leaf 0x8000000a - SVM rev and features. */
>> + uint8_t svm_rev, :8, :8, :8;
>> + uint32_t /* b */ :32;
>> + uint32_t nr_asids;
> According to the doc I'm looking at it is %ebx which holds the number of
> ASIDs and %ecx is reserved. With that adjusted
That's fun... The PPR I used for this has it wrong. A sample of others
match the APM, so I'll raise a bug with AMD against this PPR.
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Thanks.
~Andrew
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/5] x86/svm: Switch SVM features over normal cpu_has_*
2024-04-29 15:16 ` [PATCH 4/5] x86/svm: Switch SVM features over normal cpu_has_* Andrew Cooper
2024-04-30 5:51 ` Vaishali Thakkar
@ 2024-04-30 13:25 ` Jan Beulich
1 sibling, 0 replies; 20+ messages in thread
From: Jan Beulich @ 2024-04-30 13:25 UTC (permalink / raw)
To: Andrew Cooper
Cc: Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou, Sergiy Kibrik,
George Dunlap, Andrei Semenov, Vaishali Thakkar, Xen-devel
On 29.04.2024 17:16, Andrew Cooper wrote:
> @@ -2493,38 +2491,14 @@ const struct hvm_function_table * __init start_svm(void)
>
> setup_vmcb_dump();
>
> - if ( boot_cpu_data.extended_cpuid_level >= 0x8000000aU )
> - svm_feature_flags = cpuid_edx(0x8000000aU);
> -
> - printk("SVM: Supported advanced features:\n");
> -
> /* DecodeAssists fast paths assume nextrip is valid for fast rIP update. */
> - if ( !cpu_has_svm_nrips )
> - __clear_bit(SVM_FEATURE_DECODEASSISTS, &svm_feature_flags);
> + if ( !cpu_has_nrips )
> + setup_clear_cpu_cap(X86_FEATURE_DECODE_ASSIST);
Should we grow any alternatives patching based on this feature (or any
other which someone might later add clearing of next to here as well),
this is too late: alternative_instructions() runs a bit earlier than
do_presmp_initcalls(). If this is to be kept, a comment towards the
possible pitfall is imo required.
Jan
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/5] x86/cpu-policy: Infrastructure for the AMD SVM and SEV leaves
2024-04-30 13:25 ` Andrew Cooper
@ 2024-04-30 13:33 ` Jan Beulich
2024-05-01 9:16 ` George Dunlap
1 sibling, 0 replies; 20+ messages in thread
From: Jan Beulich @ 2024-04-30 13:33 UTC (permalink / raw)
To: Andrew Cooper
Cc: Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou, Sergiy Kibrik,
George Dunlap, Andrei Semenov, Vaishali Thakkar, Xen-devel
On 30.04.2024 15:25, Andrew Cooper wrote:
> On 30/04/2024 1:45 pm, Jan Beulich wrote:
>> On 29.04.2024 17:16, Andrew Cooper wrote:
>>> Allocate two new feature leaves, and extend cpu_policy with the non-feature
>>> fields too.
>>>
>>> The CPUID dependency between the SVM bit on the whole SVM leaf is
>>> intentionally deferred, to avoid transiently breaking nested virt.
>> In reply to this I meant to ask that you at least add those dependencies in
>> commented-out form, such that from looking at gen-cpuid.py it becomes clear
>> they're intentionally omitted. But you don't add feature identifiers either,
>> making dependencies impossible to express. Maybe this sentence was really
>> meant for another of the patches? (Then my request would actually apply
>> there.)
>
> This is necessary because c/s 4f8b0e94d7ca is buggy. Notice how it puts
> an edit to the policy object in the middle of a block of logic editing
> the featureset, which ends with writing the featureset back over the
> policy object.
When seeing the description of that next patch replacing that code, I first
thought you're right about that being buggy (i.e. not achieving the intended
effect). But imo it isn't really buggy, as x86_cpu_featureset_to_policy()
doesn't overwrite that leaf in the policy prior to the adjustment made there
by this very patch. Nevertheless it also wasn't intended to be that way, I
agree (and I should have noticed while reviewing the earlier change).
This means, however, that there _is_ breakage now between this and the next
patch, as now said leaf is indeed overwritten after its custom setting in
calculate_hvm_max_policy(). So maybe you want to defer the
x86_cpu_featureset_to_policy() adjustment until patch 2.
Jan
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 5/5] x86/cpu-policy: Introduce some SEV features
2024-04-29 15:16 ` [PATCH 5/5] x86/cpu-policy: Introduce some SEV features Andrew Cooper
2024-04-30 6:15 ` Vaishali Thakkar
@ 2024-04-30 13:54 ` Jan Beulich
1 sibling, 0 replies; 20+ messages in thread
From: Jan Beulich @ 2024-04-30 13:54 UTC (permalink / raw)
To: Andrew Cooper
Cc: Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou, Sergiy Kibrik,
George Dunlap, Andrei Semenov, Vaishali Thakkar, Xen-devel
On 29.04.2024 17:16, Andrew Cooper wrote:
> For display purposes only right now.
And limited to a narrow subset, presumably intentionally.
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/5] x86/cpu-policy: Infrastructure for the AMD SVM and SEV leaves
2024-04-30 13:25 ` Andrew Cooper
2024-04-30 13:33 ` Jan Beulich
@ 2024-05-01 9:16 ` George Dunlap
1 sibling, 0 replies; 20+ messages in thread
From: George Dunlap @ 2024-05-01 9:16 UTC (permalink / raw)
To: Andrew Cooper
Cc: Jan Beulich, Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou, Sergiy Kibrik,
Andrei Semenov, Vaishali Thakkar, Xen-devel
On Tue, Apr 30, 2024 at 2:25 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> On 30/04/2024 1:45 pm, Jan Beulich wrote:
> > On 29.04.2024 17:16, Andrew Cooper wrote:
> >> Allocate two new feature leaves, and extend cpu_policy with the non-feature
> >> fields too.
> >>
> >> The CPUID dependency between the SVM bit on the whole SVM leaf is
> >> intentionally deferred, to avoid transiently breaking nested virt.
> > In reply to this I meant to ask that you at least add those dependencies in
> > commented-out form, such that from looking at gen-cpuid.py it becomes clear
> > they're intentionally omitted. But you don't add feature identifiers either,
> > making dependencies impossible to express. Maybe this sentence was really
> > meant for another of the patches? (Then my request would actually apply
> > there.)
>
> This is necessary because c/s 4f8b0e94d7ca is buggy. Notice how it puts
> an edit to the policy object in the middle of a block of logic editing
> the featureset, which ends with writing the featureset back over the
> policy object.
>
> And it's not the first outstanding problem from what is a very small
> number of nested-virt patches so far...
I specifically raised this on the x86 maintainers call, and you said
to go ahead with it.
-George
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/5] x86/cpu-policy: Add SVM features already used by Xen
2024-04-29 15:16 ` [PATCH 2/5] x86/cpu-policy: Add SVM features already used by Xen Andrew Cooper
2024-04-29 15:24 ` Andrew Cooper
2024-04-30 13:02 ` Jan Beulich
@ 2024-05-01 10:00 ` George Dunlap
2024-05-01 10:39 ` Andrew Cooper
2 siblings, 1 reply; 20+ messages in thread
From: George Dunlap @ 2024-05-01 10:00 UTC (permalink / raw)
To: Andrew Cooper
Cc: Xen-devel, Jan Beulich, Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou, Sergiy Kibrik,
Andrei Semenov, Vaishali Thakkar
On Mon, Apr 29, 2024 at 4:16 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> These will replace svm_feature_flags and the SVM_FEATURE_* constants over the
> next few changes. Take the opportunity to rationalise some names.
>
> Drop the opencoded "inherit from host" logic in calculate_hvm_max_policy() and
> use 'h'/'!' annotations. The logic needs to operate on fs, not the policy
> object, given its position within the function.
>
> Drop some trailing whitespace introduced when this block of code was last
> moved.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Xenia Ragiadakou <xenia.ragiadakou@amd.com>
> CC: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
> CC: George Dunlap <george.dunlap@citrix.com>
> CC: Andrei Semenov <andrei.semenov@vates.fr>
> CC: Vaishali Thakkar <vaishali.thakkar@vates.tech>
> ---
> tools/misc/xen-cpuid.c | 11 +++++++++++
> xen/arch/x86/cpu-policy.c | 17 +++++------------
> xen/include/public/arch-x86/cpufeatureset.h | 14 ++++++++++++++
> xen/tools/gen-cpuid.py | 3 +++
> 4 files changed, 33 insertions(+), 12 deletions(-)
>
> diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
> index ab09410a05d6..0d01b0e797f1 100644
> --- a/tools/misc/xen-cpuid.c
> +++ b/tools/misc/xen-cpuid.c
> @@ -266,6 +266,17 @@ static const char *const str_m10Ah[32] =
>
> static const char *const str_eAd[32] =
> {
> + [ 0] = "npt", [ 1] = "v-lbr",
> + [ 2] = "svm-lock", [ 3] = "nrips",
> + [ 4] = "v-tsc-rate", [ 5] = "vmcb-cleanbits",
> + [ 6] = "flush-by-asid", [ 7] = "decode-assist",
> +
> + [10] = "pause-filter",
> + [12] = "pause-thresh",
> + /* 14 */ [15] = "v-loadsave",
> + [16] = "v-gif",
> + /* 18 */ [19] = "npt-sss",
> + [20] = "v-spec-ctrl",
> };
>
> static const char *const str_e1Fa[32] =
> diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
> index 4b6d96276399..da4401047e89 100644
> --- a/xen/arch/x86/cpu-policy.c
> +++ b/xen/arch/x86/cpu-policy.c
> @@ -9,7 +9,6 @@
> #include <asm/amd.h>
> #include <asm/cpu-policy.h>
> #include <asm/hvm/nestedhvm.h>
> -#include <asm/hvm/svm/svm.h>
> #include <asm/intel-family.h>
> #include <asm/msr-index.h>
> #include <asm/paging.h>
> @@ -748,22 +747,16 @@ static void __init calculate_hvm_max_policy(void)
> if ( !cpu_has_vmx )
> __clear_bit(X86_FEATURE_PKS, fs);
>
> - /*
> + /*
> * Make adjustments to possible (nested) virtualization features exposed
> * to the guest
> */
> - if ( p->extd.svm )
> + if ( test_bit(X86_FEATURE_SVM, fs) )
> {
> - /* Clamp to implemented features which require hardware support. */
> - p->extd.raw[0xa].d &= ((1u << SVM_FEATURE_NPT) |
> - (1u << SVM_FEATURE_LBRV) |
> - (1u << SVM_FEATURE_NRIPS) |
> - (1u << SVM_FEATURE_PAUSEFILTER) |
> - (1u << SVM_FEATURE_DECODEASSISTS));
> - /* Enable features which are always emulated. */
> - p->extd.raw[0xa].d |= (1u << SVM_FEATURE_VMCBCLEAN);
> + /* Xen always emulates cleanbits. */
> + __set_bit(X86_FEATURE_VMCB_CLEANBITS, fs);
> }
What about this line, at the end of recalculate_cpuid_policy()?
if ( !p->extd.svm )
p->extd.raw[0xa] = EMPTY_LEAF;
Is there a reason to continue to operate directly on the policy here,
or would it be better to do this up in the featureset section?
-George
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/5] x86/cpu-policy: Add SVM features already used by Xen
2024-05-01 10:00 ` George Dunlap
@ 2024-05-01 10:39 ` Andrew Cooper
2024-05-01 10:51 ` George Dunlap
0 siblings, 1 reply; 20+ messages in thread
From: Andrew Cooper @ 2024-05-01 10:39 UTC (permalink / raw)
To: George Dunlap
Cc: Xen-devel, Jan Beulich, Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou, Sergiy Kibrik,
Andrei Semenov, Vaishali Thakkar
On 01/05/2024 11:00 am, George Dunlap wrote:
> On Mon, Apr 29, 2024 at 4:16 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> These will replace svm_feature_flags and the SVM_FEATURE_* constants over the
>> next few changes. Take the opportunity to rationalise some names.
>>
>> Drop the opencoded "inherit from host" logic in calculate_hvm_max_policy() and
>> use 'h'/'!' annotations. The logic needs to operate on fs, not the policy
>> object, given its position within the function.
>>
>> Drop some trailing whitespace introduced when this block of code was last
>> moved.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>> CC: Stefano Stabellini <sstabellini@kernel.org>
>> CC: Xenia Ragiadakou <xenia.ragiadakou@amd.com>
>> CC: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
>> CC: George Dunlap <george.dunlap@citrix.com>
>> CC: Andrei Semenov <andrei.semenov@vates.fr>
>> CC: Vaishali Thakkar <vaishali.thakkar@vates.tech>
>> ---
>> tools/misc/xen-cpuid.c | 11 +++++++++++
>> xen/arch/x86/cpu-policy.c | 17 +++++------------
>> xen/include/public/arch-x86/cpufeatureset.h | 14 ++++++++++++++
>> xen/tools/gen-cpuid.py | 3 +++
>> 4 files changed, 33 insertions(+), 12 deletions(-)
>>
>> diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
>> index ab09410a05d6..0d01b0e797f1 100644
>> --- a/tools/misc/xen-cpuid.c
>> +++ b/tools/misc/xen-cpuid.c
>> @@ -266,6 +266,17 @@ static const char *const str_m10Ah[32] =
>>
>> static const char *const str_eAd[32] =
>> {
>> + [ 0] = "npt", [ 1] = "v-lbr",
>> + [ 2] = "svm-lock", [ 3] = "nrips",
>> + [ 4] = "v-tsc-rate", [ 5] = "vmcb-cleanbits",
>> + [ 6] = "flush-by-asid", [ 7] = "decode-assist",
>> +
>> + [10] = "pause-filter",
>> + [12] = "pause-thresh",
>> + /* 14 */ [15] = "v-loadsave",
>> + [16] = "v-gif",
>> + /* 18 */ [19] = "npt-sss",
>> + [20] = "v-spec-ctrl",
>> };
>>
>> static const char *const str_e1Fa[32] =
>> diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
>> index 4b6d96276399..da4401047e89 100644
>> --- a/xen/arch/x86/cpu-policy.c
>> +++ b/xen/arch/x86/cpu-policy.c
>> @@ -9,7 +9,6 @@
>> #include <asm/amd.h>
>> #include <asm/cpu-policy.h>
>> #include <asm/hvm/nestedhvm.h>
>> -#include <asm/hvm/svm/svm.h>
>> #include <asm/intel-family.h>
>> #include <asm/msr-index.h>
>> #include <asm/paging.h>
>> @@ -748,22 +747,16 @@ static void __init calculate_hvm_max_policy(void)
>> if ( !cpu_has_vmx )
>> __clear_bit(X86_FEATURE_PKS, fs);
>>
>> - /*
>> + /*
>> * Make adjustments to possible (nested) virtualization features exposed
>> * to the guest
>> */
>> - if ( p->extd.svm )
>> + if ( test_bit(X86_FEATURE_SVM, fs) )
>> {
>> - /* Clamp to implemented features which require hardware support. */
>> - p->extd.raw[0xa].d &= ((1u << SVM_FEATURE_NPT) |
>> - (1u << SVM_FEATURE_LBRV) |
>> - (1u << SVM_FEATURE_NRIPS) |
>> - (1u << SVM_FEATURE_PAUSEFILTER) |
>> - (1u << SVM_FEATURE_DECODEASSISTS));
>> - /* Enable features which are always emulated. */
>> - p->extd.raw[0xa].d |= (1u << SVM_FEATURE_VMCBCLEAN);
>> + /* Xen always emulates cleanbits. */
>> + __set_bit(X86_FEATURE_VMCB_CLEANBITS, fs);
>> }
> What about this line, at the end of recalculate_cpuid_policy()?
>
> if ( !p->extd.svm )
> p->extd.raw[0xa] = EMPTY_LEAF;
>
> Is there a reason to continue to operate directly on the policy here,
> or would it be better to do this up in the featureset section?
That's still needed.
Otherwise in a !SVM VM you still see svm_rev and nr_asids in a leaf that
should be all zeroes.
~Andrew
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/5] x86/cpu-policy: Add SVM features already used by Xen
2024-05-01 10:39 ` Andrew Cooper
@ 2024-05-01 10:51 ` George Dunlap
0 siblings, 0 replies; 20+ messages in thread
From: George Dunlap @ 2024-05-01 10:51 UTC (permalink / raw)
To: Andrew Cooper
Cc: Xen-devel, Jan Beulich, Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou, Sergiy Kibrik,
Andrei Semenov, Vaishali Thakkar
On Wed, May 1, 2024 at 11:39 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> On 01/05/2024 11:00 am, George Dunlap wrote:
> > On Mon, Apr 29, 2024 at 4:16 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> >> These will replace svm_feature_flags and the SVM_FEATURE_* constants over the
> >> next few changes. Take the opportunity to rationalise some names.
> >>
> >> Drop the opencoded "inherit from host" logic in calculate_hvm_max_policy() and
> >> use 'h'/'!' annotations. The logic needs to operate on fs, not the policy
> >> object, given its position within the function.
> >>
> >> Drop some trailing whitespace introduced when this block of code was last
> >> moved.
> >>
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >> ---
> >> CC: Jan Beulich <JBeulich@suse.com>
> >> CC: Roger Pau Monné <roger.pau@citrix.com>
> >> CC: Stefano Stabellini <sstabellini@kernel.org>
> >> CC: Xenia Ragiadakou <xenia.ragiadakou@amd.com>
> >> CC: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
> >> CC: George Dunlap <george.dunlap@citrix.com>
> >> CC: Andrei Semenov <andrei.semenov@vates.fr>
> >> CC: Vaishali Thakkar <vaishali.thakkar@vates.tech>
> >> ---
> >> tools/misc/xen-cpuid.c | 11 +++++++++++
> >> xen/arch/x86/cpu-policy.c | 17 +++++------------
> >> xen/include/public/arch-x86/cpufeatureset.h | 14 ++++++++++++++
> >> xen/tools/gen-cpuid.py | 3 +++
> >> 4 files changed, 33 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
> >> index ab09410a05d6..0d01b0e797f1 100644
> >> --- a/tools/misc/xen-cpuid.c
> >> +++ b/tools/misc/xen-cpuid.c
> >> @@ -266,6 +266,17 @@ static const char *const str_m10Ah[32] =
> >>
> >> static const char *const str_eAd[32] =
> >> {
> >> + [ 0] = "npt", [ 1] = "v-lbr",
> >> + [ 2] = "svm-lock", [ 3] = "nrips",
> >> + [ 4] = "v-tsc-rate", [ 5] = "vmcb-cleanbits",
> >> + [ 6] = "flush-by-asid", [ 7] = "decode-assist",
> >> +
> >> + [10] = "pause-filter",
> >> + [12] = "pause-thresh",
> >> + /* 14 */ [15] = "v-loadsave",
> >> + [16] = "v-gif",
> >> + /* 18 */ [19] = "npt-sss",
> >> + [20] = "v-spec-ctrl",
> >> };
> >>
> >> static const char *const str_e1Fa[32] =
> >> diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
> >> index 4b6d96276399..da4401047e89 100644
> >> --- a/xen/arch/x86/cpu-policy.c
> >> +++ b/xen/arch/x86/cpu-policy.c
> >> @@ -9,7 +9,6 @@
> >> #include <asm/amd.h>
> >> #include <asm/cpu-policy.h>
> >> #include <asm/hvm/nestedhvm.h>
> >> -#include <asm/hvm/svm/svm.h>
> >> #include <asm/intel-family.h>
> >> #include <asm/msr-index.h>
> >> #include <asm/paging.h>
> >> @@ -748,22 +747,16 @@ static void __init calculate_hvm_max_policy(void)
> >> if ( !cpu_has_vmx )
> >> __clear_bit(X86_FEATURE_PKS, fs);
> >>
> >> - /*
> >> + /*
> >> * Make adjustments to possible (nested) virtualization features exposed
> >> * to the guest
> >> */
> >> - if ( p->extd.svm )
> >> + if ( test_bit(X86_FEATURE_SVM, fs) )
> >> {
> >> - /* Clamp to implemented features which require hardware support. */
> >> - p->extd.raw[0xa].d &= ((1u << SVM_FEATURE_NPT) |
> >> - (1u << SVM_FEATURE_LBRV) |
> >> - (1u << SVM_FEATURE_NRIPS) |
> >> - (1u << SVM_FEATURE_PAUSEFILTER) |
> >> - (1u << SVM_FEATURE_DECODEASSISTS));
> >> - /* Enable features which are always emulated. */
> >> - p->extd.raw[0xa].d |= (1u << SVM_FEATURE_VMCBCLEAN);
> >> + /* Xen always emulates cleanbits. */
> >> + __set_bit(X86_FEATURE_VMCB_CLEANBITS, fs);
> >> }
> > What about this line, at the end of recalculate_cpuid_policy()?
> >
> > if ( !p->extd.svm )
> > p->extd.raw[0xa] = EMPTY_LEAF;
> >
> > Is there a reason to continue to operate directly on the policy here,
> > or would it be better to do this up in the featureset section?
>
> That's still needed.
>
> Otherwise in a !SVM VM you still see svm_rev and nr_asids in a leaf that
> should be all zeroes.
...Uh, yes of course we still need to clear the non-existent CPUID
bits. I was asking if we should change *how* we should clear them.
In the hunk I responded to, when enabling VMCBCLEAN, we switch from
setting bits in the policy, to setting bits in the featureset. I was
asking whether it would make sense to do something like
if !test_bit(X86_FEATURE_SVM, fs)
fs[FEATURESET_eAd] = 0;
before the x86_cpu_featureset_to_policy() instead.
-George
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2024-05-01 10:51 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-29 15:16 [PATCH 0/5] x86: AMD CPUID handling improvements Andrew Cooper
2024-04-29 15:16 ` [PATCH 1/5] x86/cpu-policy: Infrastructure for the AMD SVM and SEV leaves Andrew Cooper
2024-04-30 12:45 ` Jan Beulich
2024-04-30 13:25 ` Andrew Cooper
2024-04-30 13:33 ` Jan Beulich
2024-05-01 9:16 ` George Dunlap
2024-04-29 15:16 ` [PATCH 2/5] x86/cpu-policy: Add SVM features already used by Xen Andrew Cooper
2024-04-29 15:24 ` Andrew Cooper
2024-04-30 13:02 ` Jan Beulich
2024-05-01 10:00 ` George Dunlap
2024-05-01 10:39 ` Andrew Cooper
2024-05-01 10:51 ` George Dunlap
2024-04-29 15:16 ` [PATCH 3/5] x86/spec-ctrl: Remove open-coded check of SVM_FEATURE_SPEC_CTRL Andrew Cooper
2024-04-30 13:13 ` Jan Beulich
2024-04-29 15:16 ` [PATCH 4/5] x86/svm: Switch SVM features over normal cpu_has_* Andrew Cooper
2024-04-30 5:51 ` Vaishali Thakkar
2024-04-30 13:25 ` Jan Beulich
2024-04-29 15:16 ` [PATCH 5/5] x86/cpu-policy: Introduce some SEV features Andrew Cooper
2024-04-30 6:15 ` Vaishali Thakkar
2024-04-30 13:54 ` Jan Beulich
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.