* [PATCH RFC 0/9] x86: Merge cpuid and msr policy
@ 2023-03-29 20:51 Andrew Cooper
2023-03-29 20:51 ` [PATCH 1/9] x86: Rename struct cpu_policy to struct old_cpuid_policy Andrew Cooper
` (10 more replies)
0 siblings, 11 replies; 34+ messages in thread
From: Andrew Cooper @ 2023-03-29 20:51 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné, Wei Liu
tl;dr to add MSR_ARCH_CAPS features sensibly, cpu_{featureset<->policy}() need
to not operate on objects of differing lifetimes, so structs
{cpuid,msr}_policy need merging and cpu_policy is the obvious name.
But this does mean that we now have
cpu_policy->basic.$X
cpu_policy->feat.$Y
cpu_policy->arch_caps.$Z
and plenty of code now reads
d->arch.cpu_policy->feat.$Y
instead of
d->arch.cpuid->feat.$Y
The latter can be half-fixed with some union magic (see patch 9 commit
message). The former can be fixed by putting cpuid/msr infixes in cpu_policy,
which is doable but very invasive, and would make plenty of code read
d->arch.cpu_policy->cpuid.feat.$Y
and the two obviously shouldn't be done together.
So, RFC. Does this code layout look ok? If we want to make changes with
naming, now is very much the right time to get them sorted.
Patches 1-8 are pretty ready to go. Patch 9 is the remainder to take out the
temporary hacks, and I'm still in the process of merging the system policy
derivation.
Andrew Cooper (9):
x86: Rename struct cpu_policy to struct old_cpuid_policy
x86: Rename {domctl,sysctl}.cpu_policy.{cpuid,msr_policy} fields
x86: Rename struct cpuid_policy to struct cpu_policy
x86: Merge struct msr_policy into struct cpu_policy
x86: Merge the system {cpuid,msr} policy objects
x86: Merge a domain's {cpuid,msr} policy objects
x86: Merge xc_cpu_policy's cpuid and msr objects
x86: Drop struct old_cpu_policy
RFC: Everything else
tools/fuzz/cpu-policy/afl-policy-fuzzer.c | 15 +-
.../fuzz/x86_instruction_emulator/fuzz-emul.c | 2 +-
tools/libs/guest/xg_cpuid_x86.c | 48 +-
tools/libs/guest/xg_private.h | 5 +-
tools/tests/cpu-policy/test-cpu-policy.c | 50 +-
tools/tests/tsx/test-tsx.c | 58 +-
tools/tests/x86_emulator/Makefile | 2 +-
tools/tests/x86_emulator/test_x86_emulator.c | 2 +-
tools/tests/x86_emulator/x86-emulate.c | 2 +-
tools/tests/x86_emulator/x86-emulate.h | 2 +-
xen/arch/x86/Makefile | 1 +
xen/arch/x86/cpu-policy.c | 67 +++
xen/arch/x86/cpu/common.c | 4 +-
xen/arch/x86/cpu/mcheck/mce_intel.c | 2 +-
xen/arch/x86/cpu/vpmu_intel.c | 4 +-
xen/arch/x86/cpuid.c | 101 ++--
xen/arch/x86/domain.c | 18 +-
xen/arch/x86/domctl.c | 51 +-
xen/arch/x86/hvm/emulate.c | 2 +-
xen/arch/x86/hvm/hvm.c | 38 +-
xen/arch/x86/hvm/ioreq.c | 4 +-
xen/arch/x86/hvm/mtrr.c | 2 +-
xen/arch/x86/hvm/svm/svm.c | 18 +-
xen/arch/x86/hvm/svm/svmdebug.c | 2 +-
xen/arch/x86/hvm/vlapic.c | 2 +-
xen/arch/x86/hvm/vmx/vmx.c | 12 +-
xen/arch/x86/hvm/vmx/vvmx.c | 2 +-
xen/arch/x86/include/asm/cpu-policy.h | 18 +
xen/arch/x86/include/asm/cpuid.h | 10 -
xen/arch/x86/include/asm/domain.h | 4 +-
xen/arch/x86/include/asm/guest_pt.h | 4 +-
xen/arch/x86/include/asm/msr.h | 13 +-
xen/arch/x86/include/asm/paging.h | 2 +-
xen/arch/x86/mm/mem_sharing.c | 3 +-
xen/arch/x86/mm/shadow/hvm.c | 2 +-
xen/arch/x86/msr.c | 98 +---
xen/arch/x86/pv/domain.c | 2 +-
xen/arch/x86/pv/emul-priv-op.c | 6 +-
xen/arch/x86/pv/ro-page-fault.c | 2 +-
xen/arch/x86/sysctl.c | 77 +--
xen/arch/x86/traps.c | 2 +-
xen/arch/x86/x86_emulate.c | 2 +-
xen/arch/x86/x86_emulate/x86_emulate.c | 166 +++---
xen/arch/x86/x86_emulate/x86_emulate.h | 6 +-
xen/arch/x86/xstate.c | 4 +-
xen/include/public/domctl.h | 4 +-
xen/include/public/sysctl.h | 4 +-
xen/include/xen/lib/x86/cpu-policy.h | 540 +++++++++++++++++-
xen/include/xen/lib/x86/cpuid.h | 475 ---------------
xen/include/xen/lib/x86/msr.h | 104 ----
xen/lib/x86/cpuid.c | 12 +-
xen/lib/x86/msr.c | 6 +-
xen/lib/x86/policy.c | 8 +-
53 files changed, 986 insertions(+), 1104 deletions(-)
create mode 100644 xen/arch/x86/cpu-policy.c
create mode 100644 xen/arch/x86/include/asm/cpu-policy.h
delete mode 100644 xen/include/xen/lib/x86/cpuid.h
delete mode 100644 xen/include/xen/lib/x86/msr.h
--
2.30.2
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 1/9] x86: Rename struct cpu_policy to struct old_cpuid_policy
2023-03-29 20:51 [PATCH RFC 0/9] x86: Merge cpuid and msr policy Andrew Cooper
@ 2023-03-29 20:51 ` Andrew Cooper
2023-03-29 20:51 ` [PATCH 2/9] x86: Rename {domctl,sysctl}.cpu_policy.{cpuid,msr_policy} fields Andrew Cooper
` (9 subsequent siblings)
10 siblings, 0 replies; 34+ messages in thread
From: Andrew Cooper @ 2023-03-29 20:51 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné, Wei Liu
We want to merge struct cpuid_policy and struct msr_policy together, and the
result wants to be called struct cpu_policy.
The current struct cpu_policy, being a pair of pointers, isn't terribly
useful. Rename the type to struct old_cpu_policy, but it will disappear
entirely once the merge is complete.
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: Wei Liu <wl@xen.org>
---
tools/libs/guest/xg_cpuid_x86.c | 4 ++--
tools/tests/cpu-policy/test-cpu-policy.c | 4 ++--
xen/arch/x86/domctl.c | 4 ++--
xen/arch/x86/include/asm/cpuid.h | 2 +-
xen/arch/x86/sysctl.c | 4 ++--
xen/include/xen/lib/x86/cpu-policy.h | 6 +++---
xen/lib/x86/policy.c | 4 ++--
7 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/tools/libs/guest/xg_cpuid_x86.c b/tools/libs/guest/xg_cpuid_x86.c
index 4542878bbe88..1b02bc987af7 100644
--- a/tools/libs/guest/xg_cpuid_x86.c
+++ b/tools/libs/guest/xg_cpuid_x86.c
@@ -868,8 +868,8 @@ bool xc_cpu_policy_is_compatible(xc_interface *xch, xc_cpu_policy_t *host,
xc_cpu_policy_t *guest)
{
struct cpu_policy_errors err = INIT_CPU_POLICY_ERRORS;
- struct cpu_policy h = { &host->cpuid, &host->msr };
- struct cpu_policy g = { &guest->cpuid, &guest->msr };
+ struct old_cpu_policy h = { &host->cpuid, &host->msr };
+ struct old_cpu_policy g = { &guest->cpuid, &guest->msr };
int rc = x86_cpu_policies_are_compatible(&h, &g, &err);
if ( !rc )
diff --git a/tools/tests/cpu-policy/test-cpu-policy.c b/tools/tests/cpu-policy/test-cpu-policy.c
index d3f24fd6d274..909d6272f875 100644
--- a/tools/tests/cpu-policy/test-cpu-policy.c
+++ b/tools/tests/cpu-policy/test-cpu-policy.c
@@ -602,7 +602,7 @@ static void test_is_compatible_success(void)
for ( size_t i = 0; i < ARRAY_SIZE(tests); ++i )
{
struct test *t = &tests[i];
- struct cpu_policy sys = {
+ struct old_cpu_policy sys = {
&t->host_cpuid,
&t->host_msr,
}, new = {
@@ -654,7 +654,7 @@ static void test_is_compatible_failure(void)
for ( size_t i = 0; i < ARRAY_SIZE(tests); ++i )
{
struct test *t = &tests[i];
- struct cpu_policy sys = {
+ struct old_cpu_policy sys = {
&t->host_cpuid,
&t->host_msr,
}, new = {
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 2118fcad5dfe..0b41b279507e 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -40,8 +40,8 @@
static int update_domain_cpu_policy(struct domain *d,
xen_domctl_cpu_policy_t *xdpc)
{
- struct cpu_policy new = {};
- const struct cpu_policy *sys = is_pv_domain(d)
+ struct old_cpu_policy new = {};
+ const struct old_cpu_policy *sys = is_pv_domain(d)
? &system_policies[XEN_SYSCTL_cpu_policy_pv_max]
: &system_policies[XEN_SYSCTL_cpu_policy_hvm_max];
struct cpu_policy_errors err = INIT_CPU_POLICY_ERRORS;
diff --git a/xen/arch/x86/include/asm/cpuid.h b/xen/arch/x86/include/asm/cpuid.h
index 9c3637549a10..49b3128f06f9 100644
--- a/xen/arch/x86/include/asm/cpuid.h
+++ b/xen/arch/x86/include/asm/cpuid.h
@@ -51,7 +51,7 @@ extern struct cpuid_policy raw_cpuid_policy, host_cpuid_policy,
pv_max_cpuid_policy, pv_def_cpuid_policy,
hvm_max_cpuid_policy, hvm_def_cpuid_policy;
-extern const struct cpu_policy system_policies[];
+extern const struct old_cpu_policy system_policies[];
/* Check that all previously present features are still available. */
bool recheck_cpu_features(unsigned int cpu);
diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
index 16625b57f01f..3f5b092df16a 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -32,7 +32,7 @@
#include <asm/psr.h>
#include <asm/cpuid.h>
-const struct cpu_policy system_policies[6] = {
+const struct old_cpu_policy system_policies[6] = {
[ XEN_SYSCTL_cpu_policy_raw ] = {
&raw_cpuid_policy,
&raw_msr_policy,
@@ -391,7 +391,7 @@ long arch_do_sysctl(
case XEN_SYSCTL_get_cpu_policy:
{
- const struct cpu_policy *policy;
+ const struct old_cpu_policy *policy;
/* Reserved field set, or bad policy index? */
if ( sysctl->u.cpu_policy._rsvd ||
diff --git a/xen/include/xen/lib/x86/cpu-policy.h b/xen/include/xen/lib/x86/cpu-policy.h
index 5a2c4c7b2d90..3a5300d1078c 100644
--- a/xen/include/xen/lib/x86/cpu-policy.h
+++ b/xen/include/xen/lib/x86/cpu-policy.h
@@ -5,7 +5,7 @@
#include <xen/lib/x86/cpuid.h>
#include <xen/lib/x86/msr.h>
-struct cpu_policy
+struct old_cpu_policy
{
struct cpuid_policy *cpuid;
struct msr_policy *msr;
@@ -33,8 +33,8 @@ struct cpu_policy_errors
* incompatibility is detected, the optional err pointer may identify the
* problematic leaf/subleaf and/or MSR.
*/
-int x86_cpu_policies_are_compatible(const struct cpu_policy *host,
- const struct cpu_policy *guest,
+int x86_cpu_policies_are_compatible(const struct old_cpu_policy *host,
+ const struct old_cpu_policy *guest,
struct cpu_policy_errors *err);
#endif /* !XEN_LIB_X86_POLICIES_H */
diff --git a/xen/lib/x86/policy.c b/xen/lib/x86/policy.c
index f6cea4e2f9bd..2975711d7c6c 100644
--- a/xen/lib/x86/policy.c
+++ b/xen/lib/x86/policy.c
@@ -2,8 +2,8 @@
#include <xen/lib/x86/cpu-policy.h>
-int x86_cpu_policies_are_compatible(const struct cpu_policy *host,
- const struct cpu_policy *guest,
+int x86_cpu_policies_are_compatible(const struct old_cpu_policy *host,
+ const struct old_cpu_policy *guest,
struct cpu_policy_errors *err)
{
struct cpu_policy_errors e = INIT_CPU_POLICY_ERRORS;
--
2.30.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 2/9] x86: Rename {domctl,sysctl}.cpu_policy.{cpuid,msr_policy} fields
2023-03-29 20:51 [PATCH RFC 0/9] x86: Merge cpuid and msr policy Andrew Cooper
2023-03-29 20:51 ` [PATCH 1/9] x86: Rename struct cpu_policy to struct old_cpuid_policy Andrew Cooper
@ 2023-03-29 20:51 ` Andrew Cooper
2023-03-29 20:51 ` [PATCH 3/9] x86: Rename struct cpuid_policy to struct cpu_policy Andrew Cooper
` (8 subsequent siblings)
10 siblings, 0 replies; 34+ messages in thread
From: Andrew Cooper @ 2023-03-29 20:51 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné, Wei Liu
These weren't great names to begin with, and using {leaves,msrs} matches up
better with the existing nr_{leaves,msr} parameters anyway.
Furthermore,by renaming these fields we can get away with using some #define
trickary to avoid the struct {cpuid,msr}_policy merge needing to happen in a
single changeset.
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: Wei Liu <wl@xen.org>
---
tools/libs/guest/xg_cpuid_x86.c | 12 ++++++------
xen/arch/x86/domctl.c | 12 ++++++------
xen/arch/x86/sysctl.c | 8 ++++----
xen/include/public/domctl.h | 4 ++--
xen/include/public/sysctl.h | 4 ++--
5 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/tools/libs/guest/xg_cpuid_x86.c b/tools/libs/guest/xg_cpuid_x86.c
index 1b02bc987af7..5fae06e77804 100644
--- a/tools/libs/guest/xg_cpuid_x86.c
+++ b/tools/libs/guest/xg_cpuid_x86.c
@@ -145,9 +145,9 @@ static int get_system_cpu_policy(xc_interface *xch, uint32_t index,
sysctl.cmd = XEN_SYSCTL_get_cpu_policy;
sysctl.u.cpu_policy.index = index;
sysctl.u.cpu_policy.nr_leaves = *nr_leaves;
- set_xen_guest_handle(sysctl.u.cpu_policy.cpuid_policy, leaves);
+ set_xen_guest_handle(sysctl.u.cpu_policy.leaves, leaves);
sysctl.u.cpu_policy.nr_msrs = *nr_msrs;
- set_xen_guest_handle(sysctl.u.cpu_policy.msr_policy, msrs);
+ set_xen_guest_handle(sysctl.u.cpu_policy.msrs, msrs);
ret = do_sysctl(xch, &sysctl);
@@ -183,9 +183,9 @@ static int get_domain_cpu_policy(xc_interface *xch, uint32_t domid,
domctl.cmd = XEN_DOMCTL_get_cpu_policy;
domctl.domain = domid;
domctl.u.cpu_policy.nr_leaves = *nr_leaves;
- set_xen_guest_handle(domctl.u.cpu_policy.cpuid_policy, leaves);
+ set_xen_guest_handle(domctl.u.cpu_policy.leaves, leaves);
domctl.u.cpu_policy.nr_msrs = *nr_msrs;
- set_xen_guest_handle(domctl.u.cpu_policy.msr_policy, msrs);
+ set_xen_guest_handle(domctl.u.cpu_policy.msrs, msrs);
ret = do_domctl(xch, &domctl);
@@ -232,9 +232,9 @@ int xc_set_domain_cpu_policy(xc_interface *xch, uint32_t domid,
domctl.cmd = XEN_DOMCTL_set_cpu_policy;
domctl.domain = domid;
domctl.u.cpu_policy.nr_leaves = nr_leaves;
- set_xen_guest_handle(domctl.u.cpu_policy.cpuid_policy, leaves);
+ set_xen_guest_handle(domctl.u.cpu_policy.leaves, leaves);
domctl.u.cpu_policy.nr_msrs = nr_msrs;
- set_xen_guest_handle(domctl.u.cpu_policy.msr_policy, msrs);
+ set_xen_guest_handle(domctl.u.cpu_policy.msrs, msrs);
domctl.u.cpu_policy.err_leaf = -1;
domctl.u.cpu_policy.err_subleaf = -1;
domctl.u.cpu_policy.err_msr = -1;
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 0b41b279507e..944af63e68d0 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -54,10 +54,10 @@ static int update_domain_cpu_policy(struct domain *d,
/* Merge the toolstack provided data. */
if ( (ret = x86_cpuid_copy_from_buffer(
- new.cpuid, xdpc->cpuid_policy, xdpc->nr_leaves,
+ new.cpuid, xdpc->leaves, xdpc->nr_leaves,
&err.leaf, &err.subleaf)) ||
(ret = x86_msr_copy_from_buffer(
- new.msr, xdpc->msr_policy, xdpc->nr_msrs, &err.msr)) )
+ new.msr, xdpc->msrs, xdpc->nr_msrs, &err.msr)) )
goto out;
/* Trim any newly-stale out-of-range leaves. */
@@ -1317,20 +1317,20 @@ long arch_do_domctl(
case XEN_DOMCTL_get_cpu_policy:
/* Process the CPUID leaves. */
- if ( guest_handle_is_null(domctl->u.cpu_policy.cpuid_policy) )
+ if ( guest_handle_is_null(domctl->u.cpu_policy.leaves) )
domctl->u.cpu_policy.nr_leaves = CPUID_MAX_SERIALISED_LEAVES;
else if ( (ret = x86_cpuid_copy_to_buffer(
d->arch.cpuid,
- domctl->u.cpu_policy.cpuid_policy,
+ domctl->u.cpu_policy.leaves,
&domctl->u.cpu_policy.nr_leaves)) )
break;
/* Process the MSR entries. */
- if ( guest_handle_is_null(domctl->u.cpu_policy.msr_policy) )
+ if ( guest_handle_is_null(domctl->u.cpu_policy.msrs) )
domctl->u.cpu_policy.nr_msrs = MSR_MAX_SERIALISED_ENTRIES;
else if ( (ret = x86_msr_copy_to_buffer(
d->arch.msr,
- domctl->u.cpu_policy.msr_policy,
+ domctl->u.cpu_policy.msrs,
&domctl->u.cpu_policy.nr_msrs)) )
break;
diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
index 3f5b092df16a..3ed7c69f4315 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -411,11 +411,11 @@ long arch_do_sysctl(
}
/* Process the CPUID leaves. */
- if ( guest_handle_is_null(sysctl->u.cpu_policy.cpuid_policy) )
+ if ( guest_handle_is_null(sysctl->u.cpu_policy.leaves) )
sysctl->u.cpu_policy.nr_leaves = CPUID_MAX_SERIALISED_LEAVES;
else if ( (ret = x86_cpuid_copy_to_buffer(
policy->cpuid,
- sysctl->u.cpu_policy.cpuid_policy,
+ sysctl->u.cpu_policy.leaves,
&sysctl->u.cpu_policy.nr_leaves)) )
break;
@@ -427,11 +427,11 @@ long arch_do_sysctl(
}
/* Process the MSR entries. */
- if ( guest_handle_is_null(sysctl->u.cpu_policy.msr_policy) )
+ if ( guest_handle_is_null(sysctl->u.cpu_policy.msrs) )
sysctl->u.cpu_policy.nr_msrs = MSR_MAX_SERIALISED_ENTRIES;
else if ( (ret = x86_msr_copy_to_buffer(
policy->msr,
- sysctl->u.cpu_policy.msr_policy,
+ sysctl->u.cpu_policy.msrs,
&sysctl->u.cpu_policy.nr_msrs)) )
break;
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 7280e9f96816..529801c89ba3 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -683,8 +683,8 @@ struct xen_domctl_cpu_policy {
* 'cpuid_policy'. */
uint32_t nr_msrs; /* IN/OUT: Number of MSRs in/written to
* 'msr_policy' */
- XEN_GUEST_HANDLE_64(xen_cpuid_leaf_t) cpuid_policy; /* IN/OUT */
- XEN_GUEST_HANDLE_64(xen_msr_entry_t) msr_policy; /* IN/OUT */
+ XEN_GUEST_HANDLE_64(xen_cpuid_leaf_t) leaves; /* IN/OUT */
+ XEN_GUEST_HANDLE_64(xen_msr_entry_t) msrs; /* IN/OUT */
/*
* OUT, set_policy only. Written in some (but not all) error cases to
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index e8dded9fb94a..2b24d6bfd00e 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -1050,8 +1050,8 @@ struct xen_sysctl_cpu_policy {
* 'msr_policy', or the maximum number of MSRs if
* the guest handle is NULL. */
uint32_t _rsvd; /* Must be zero. */
- XEN_GUEST_HANDLE_64(xen_cpuid_leaf_t) cpuid_policy; /* OUT */
- XEN_GUEST_HANDLE_64(xen_msr_entry_t) msr_policy; /* OUT */
+ XEN_GUEST_HANDLE_64(xen_cpuid_leaf_t) leaves; /* OUT */
+ XEN_GUEST_HANDLE_64(xen_msr_entry_t) msrs; /* OUT */
};
typedef struct xen_sysctl_cpu_policy xen_sysctl_cpu_policy_t;
DEFINE_XEN_GUEST_HANDLE(xen_sysctl_cpu_policy_t);
--
2.30.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 3/9] x86: Rename struct cpuid_policy to struct cpu_policy
2023-03-29 20:51 [PATCH RFC 0/9] x86: Merge cpuid and msr policy Andrew Cooper
2023-03-29 20:51 ` [PATCH 1/9] x86: Rename struct cpu_policy to struct old_cpuid_policy Andrew Cooper
2023-03-29 20:51 ` [PATCH 2/9] x86: Rename {domctl,sysctl}.cpu_policy.{cpuid,msr_policy} fields Andrew Cooper
@ 2023-03-29 20:51 ` Andrew Cooper
2023-03-29 20:51 ` [PATCH 4/9] x86: Merge struct msr_policy into " Andrew Cooper
` (7 subsequent siblings)
10 siblings, 0 replies; 34+ messages in thread
From: Andrew Cooper @ 2023-03-29 20:51 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné, Wei Liu
Also merge lib/x86/cpuid.h entirely into lib/x86/cpu-policy.h
Use a temporary define to make struct cpuid_policy still work.
There's one forward declaration of struct cpuid_policy in
tools/tests/x86_emulator/x86-emulate.h that isn't covered by the define, and
it's easier to rename that now than to rearrange the includes.
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: Wei Liu <wl@xen.org>
---
tools/fuzz/cpu-policy/afl-policy-fuzzer.c | 2 +-
tools/tests/x86_emulator/Makefile | 2 +-
tools/tests/x86_emulator/x86-emulate.h | 2 +-
xen/arch/x86/include/asm/cpuid.h | 1 -
xen/arch/x86/x86_emulate/x86_emulate.h | 2 +-
xen/include/xen/lib/x86/cpu-policy.h | 463 ++++++++++++++++++++-
xen/include/xen/lib/x86/cpuid.h | 475 ----------------------
xen/lib/x86/cpuid.c | 2 +-
8 files changed, 467 insertions(+), 482 deletions(-)
delete mode 100644 xen/include/xen/lib/x86/cpuid.h
diff --git a/tools/fuzz/cpu-policy/afl-policy-fuzzer.c b/tools/fuzz/cpu-policy/afl-policy-fuzzer.c
index 7d0f274c6cdd..79e42e8bfd04 100644
--- a/tools/fuzz/cpu-policy/afl-policy-fuzzer.c
+++ b/tools/fuzz/cpu-policy/afl-policy-fuzzer.c
@@ -9,7 +9,7 @@
#include <getopt.h>
#include <xen-tools/common-macros.h>
-#include <xen/lib/x86/cpuid.h>
+#include <xen/lib/x86/cpu-policy.h>
#include <xen/lib/x86/msr.h>
#include <xen/domctl.h>
diff --git a/tools/tests/x86_emulator/Makefile b/tools/tests/x86_emulator/Makefile
index 7b07c31bbde4..2f7c58040004 100644
--- a/tools/tests/x86_emulator/Makefile
+++ b/tools/tests/x86_emulator/Makefile
@@ -286,7 +286,7 @@ HOSTCFLAGS += $(CFLAGS_xeninclude) -I. $(HOSTCFLAGS-$(XEN_COMPILE_ARCH))
x86.h := $(addprefix $(XEN_ROOT)/tools/include/xen/asm/,\
x86-vendors.h x86-defns.h msr-index.h) \
$(addprefix $(XEN_ROOT)/tools/include/xen/lib/x86/, \
- cpuid.h cpuid-autogen.h)
+ cpu-policy.h)
x86_emulate.h := x86-emulate.h x86_emulate/x86_emulate.h $(x86.h)
x86-emulate.o cpuid.o test_x86_emulator.o evex-disp8.o predicates.o wrappers.o: %.o: %.c $(x86_emulate.h)
diff --git a/tools/tests/x86_emulator/x86-emulate.h b/tools/tests/x86_emulator/x86-emulate.h
index 1af986f78d16..19544b0b3a8a 100644
--- a/tools/tests/x86_emulator/x86-emulate.h
+++ b/tools/tests/x86_emulator/x86-emulate.h
@@ -65,7 +65,7 @@
#define is_canonical_address(x) (((int64_t)(x) >> 47) == ((int64_t)(x) >> 63))
extern uint32_t mxcsr_mask;
-extern struct cpuid_policy cp;
+extern struct cpu_policy cp;
#define MMAP_SZ 16384
bool emul_test_init(void);
diff --git a/xen/arch/x86/include/asm/cpuid.h b/xen/arch/x86/include/asm/cpuid.h
index 49b3128f06f9..d418e8100dde 100644
--- a/xen/arch/x86/include/asm/cpuid.h
+++ b/xen/arch/x86/include/asm/cpuid.h
@@ -9,7 +9,6 @@
#include <xen/percpu.h>
#include <xen/lib/x86/cpu-policy.h>
-#include <xen/lib/x86/cpuid.h>
#include <public/sysctl.h>
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h b/xen/arch/x86/x86_emulate/x86_emulate.h
index bb7af967ffee..75015104fbdb 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.h
+++ b/xen/arch/x86/x86_emulate/x86_emulate.h
@@ -23,7 +23,7 @@
#ifndef __X86_EMULATE_H__
#define __X86_EMULATE_H__
-#include <xen/lib/x86/cpuid.h>
+#include <xen/lib/x86/cpu-policy.h>
#define MAX_INST_LEN 15
diff --git a/xen/include/xen/lib/x86/cpu-policy.h b/xen/include/xen/lib/x86/cpu-policy.h
index 3a5300d1078c..5e430d848021 100644
--- a/xen/include/xen/lib/x86/cpu-policy.h
+++ b/xen/include/xen/lib/x86/cpu-policy.h
@@ -2,9 +2,342 @@
#ifndef XEN_LIB_X86_POLICIES_H
#define XEN_LIB_X86_POLICIES_H
-#include <xen/lib/x86/cpuid.h>
+#include <xen/lib/x86/cpuid-autogen.h>
#include <xen/lib/x86/msr.h>
+#define FEATURESET_1d 0 /* 0x00000001.edx */
+#define FEATURESET_1c 1 /* 0x00000001.ecx */
+#define FEATURESET_e1d 2 /* 0x80000001.edx */
+#define FEATURESET_e1c 3 /* 0x80000001.ecx */
+#define FEATURESET_Da1 4 /* 0x0000000d:1.eax */
+#define FEATURESET_7b0 5 /* 0x00000007:0.ebx */
+#define FEATURESET_7c0 6 /* 0x00000007:0.ecx */
+#define FEATURESET_e7d 7 /* 0x80000007.edx */
+#define FEATURESET_e8b 8 /* 0x80000008.ebx */
+#define FEATURESET_7d0 9 /* 0x00000007:0.edx */
+#define FEATURESET_7a1 10 /* 0x00000007:1.eax */
+#define FEATURESET_e21a 11 /* 0x80000021.eax */
+#define FEATURESET_7b1 12 /* 0x00000007:1.ebx */
+#define FEATURESET_7d2 13 /* 0x00000007:2.edx */
+#define FEATURESET_7c1 14 /* 0x00000007:1.ecx */
+#define FEATURESET_7d1 15 /* 0x00000007:1.edx */
+
+struct cpuid_leaf
+{
+ uint32_t a, b, c, d;
+};
+
+/*
+ * Versions of GCC before 5 unconditionally reserve %rBX as the PIC hard
+ * register, and are unable to cope with spilling it. This results in a
+ * rather cryptic error:
+ * error: inconsistent operand constraints in an ‘asm’
+ *
+ * In affected situations, work around the issue by using a separate register
+ * to hold the the %rBX output, and xchg twice to leave %rBX preserved around
+ * the asm() statement.
+ */
+#if defined(__PIC__) && __GNUC__ < 5 && !defined(__clang__) && defined(__i386__)
+# define XCHG_BX "xchg %%ebx, %[bx];"
+# define BX_CON [bx] "=&r"
+#elif defined(__PIC__) && __GNUC__ < 5 && !defined(__clang__) && \
+ defined(__x86_64__) && (defined(__code_model_medium__) || \
+ defined(__code_model_large__))
+# define XCHG_BX "xchg %%rbx, %q[bx];"
+# define BX_CON [bx] "=&r"
+#else
+# define XCHG_BX ""
+# define BX_CON "=&b"
+#endif
+
+static inline void cpuid_leaf(uint32_t leaf, struct cpuid_leaf *l)
+{
+ asm ( XCHG_BX
+ "cpuid;"
+ XCHG_BX
+ : "=a" (l->a), BX_CON (l->b), "=&c" (l->c), "=&d" (l->d)
+ : "a" (leaf) );
+}
+
+static inline void cpuid_count_leaf(
+ uint32_t leaf, uint32_t subleaf, struct cpuid_leaf *l)
+{
+ asm ( XCHG_BX
+ "cpuid;"
+ XCHG_BX
+ : "=a" (l->a), BX_CON (l->b), "=c" (l->c), "=&d" (l->d)
+ : "a" (leaf), "c" (subleaf) );
+}
+
+#undef BX_CON
+#undef XCHG
+
+/**
+ * Given the vendor id from CPUID leaf 0, look up Xen's internal integer
+ * vendor ID. Returns X86_VENDOR_UNKNOWN for any unknown vendor.
+ */
+unsigned int x86_cpuid_lookup_vendor(uint32_t ebx, uint32_t ecx, uint32_t edx);
+
+/**
+ * Given Xen's internal vendor ID, return a string suitable for printing.
+ * Returns "Unknown" for any unrecognised ID.
+ */
+const char *x86_cpuid_vendor_to_str(unsigned int vendor);
+
+#define CPUID_GUEST_NR_BASIC (0xdu + 1)
+#define CPUID_GUEST_NR_CACHE (5u + 1)
+#define CPUID_GUEST_NR_FEAT (2u + 1)
+#define CPUID_GUEST_NR_TOPO (1u + 1)
+#define CPUID_GUEST_NR_XSTATE (62u + 1)
+#define CPUID_GUEST_NR_EXTD_INTEL (0x8u + 1)
+#define CPUID_GUEST_NR_EXTD_AMD (0x21u + 1)
+#define CPUID_GUEST_NR_EXTD MAX(CPUID_GUEST_NR_EXTD_INTEL, \
+ CPUID_GUEST_NR_EXTD_AMD)
+
+/*
+ * Maximum number of leaves a struct cpuid_policy turns into when serialised
+ * for interaction with the toolstack. (Sum of all leaves in each union, less
+ * the entries in basic which sub-unions hang off of.)
+ */
+#define CPUID_MAX_SERIALISED_LEAVES \
+ (CPUID_GUEST_NR_BASIC + \
+ CPUID_GUEST_NR_FEAT - !!CPUID_GUEST_NR_FEAT + \
+ CPUID_GUEST_NR_CACHE - !!CPUID_GUEST_NR_CACHE + \
+ CPUID_GUEST_NR_TOPO - !!CPUID_GUEST_NR_TOPO + \
+ CPUID_GUEST_NR_XSTATE - !!CPUID_GUEST_NR_XSTATE + \
+ CPUID_GUEST_NR_EXTD + 2 /* hv_limit and hv2_limit */ )
+
+struct cpu_policy
+{
+#define DECL_BITFIELD(word) _DECL_BITFIELD(FEATURESET_ ## word)
+#define _DECL_BITFIELD(x) __DECL_BITFIELD(x)
+#define __DECL_BITFIELD(x) CPUID_BITFIELD_ ## x
+
+ /* Basic leaves: 0x000000xx */
+ union {
+ struct cpuid_leaf raw[CPUID_GUEST_NR_BASIC];
+ struct {
+ /* Leaf 0x0 - Max and vendor. */
+ uint32_t max_leaf, vendor_ebx, vendor_ecx, vendor_edx;
+
+ /* Leaf 0x1 - Family/model/stepping and features. */
+ uint32_t raw_fms;
+ uint8_t :8, /* Brand ID. */
+ clflush_size, /* Number of 8-byte blocks per cache line. */
+ lppp, /* Logical processors per package. */
+ apic_id; /* Initial APIC ID. */
+ union {
+ uint32_t _1c;
+ struct { DECL_BITFIELD(1c); };
+ };
+ union {
+ uint32_t _1d;
+ struct { DECL_BITFIELD(1d); };
+ };
+
+ /* Leaf 0x2 - TLB/Cache/Prefetch. */
+ uint8_t l2_nr_queries; /* Documented as fixed to 1. */
+ uint8_t l2_desc[15];
+
+ uint64_t :64, :64; /* Leaf 0x3 - PSN. */
+ uint64_t :64, :64; /* Leaf 0x4 - Structured Cache. */
+ uint64_t :64, :64; /* Leaf 0x5 - MONITOR. */
+ uint64_t :64, :64; /* Leaf 0x6 - Therm/Perf. */
+ uint64_t :64, :64; /* Leaf 0x7 - Structured Features. */
+ uint64_t :64, :64; /* Leaf 0x8 - rsvd */
+ uint64_t :64, :64; /* Leaf 0x9 - DCA */
+
+ /* Leaf 0xa - Intel PMU. */
+ uint8_t pmu_version, _pmu[15];
+
+ uint64_t :64, :64; /* Leaf 0xb - Topology. */
+ uint64_t :64, :64; /* Leaf 0xc - rsvd */
+ uint64_t :64, :64; /* Leaf 0xd - XSTATE. */
+ };
+ } basic;
+
+ /* Structured cache leaf: 0x00000004[xx] */
+ union {
+ struct cpuid_leaf raw[CPUID_GUEST_NR_CACHE];
+ struct cpuid_cache_leaf {
+ uint32_t /* a */ type:5, level:3;
+ bool self_init:1, fully_assoc:1;
+ uint32_t :4, threads_per_cache:12, cores_per_package:6;
+ uint32_t /* b */ line_size:12, partitions:10, ways:10;
+ uint32_t /* c */ sets;
+ bool /* d */ wbinvd:1, inclusive:1, complex:1;
+ } subleaf[CPUID_GUEST_NR_CACHE];
+ } cache;
+
+ /* Structured feature leaf: 0x00000007[xx] */
+ union {
+ struct cpuid_leaf raw[CPUID_GUEST_NR_FEAT];
+ struct {
+ /* Subleaf 0. */
+ uint32_t max_subleaf;
+ union {
+ uint32_t _7b0;
+ struct { DECL_BITFIELD(7b0); };
+ };
+ union {
+ uint32_t _7c0;
+ struct { DECL_BITFIELD(7c0); };
+ };
+ union {
+ uint32_t _7d0;
+ struct { DECL_BITFIELD(7d0); };
+ };
+
+ /* Subleaf 1. */
+ union {
+ uint32_t _7a1;
+ struct { DECL_BITFIELD(7a1); };
+ };
+ union {
+ uint32_t _7b1;
+ struct { DECL_BITFIELD(7b1); };
+ };
+ union {
+ uint32_t _7c1;
+ struct { DECL_BITFIELD(7c1); };
+ };
+ union {
+ uint32_t _7d1;
+ struct { DECL_BITFIELD(7d1); };
+ };
+
+ /* Subleaf 2. */
+ uint32_t /* a */:32, /* b */:32, /* c */:32;
+ union {
+ uint32_t _7d2;
+ struct { DECL_BITFIELD(7d2); };
+ };
+ };
+ } feat;
+
+ /* Extended topology enumeration: 0x0000000B[xx] */
+ union {
+ struct cpuid_leaf raw[CPUID_GUEST_NR_TOPO];
+ struct cpuid_topo_leaf {
+ uint32_t id_shift:5, :27;
+ uint16_t nr_logical, :16;
+ uint8_t level, type, :8, :8;
+ uint32_t x2apic_id;
+ } subleaf[CPUID_GUEST_NR_TOPO];
+ } topo;
+
+ /* Xstate feature leaf: 0x0000000D[xx] */
+ union {
+ struct cpuid_leaf raw[CPUID_GUEST_NR_XSTATE];
+
+ struct {
+ /* Subleaf 0. */
+ uint32_t xcr0_low, /* b */:32, max_size, xcr0_high;
+
+ /* Subleaf 1. */
+ union {
+ uint32_t Da1;
+ struct { DECL_BITFIELD(Da1); };
+ };
+ uint32_t /* b */:32, xss_low, xss_high;
+ };
+
+ /* Per-component common state. Valid for i >= 2. */
+ struct {
+ uint32_t size, offset;
+ bool xss:1, align:1;
+ uint32_t _res_d;
+ } comp[CPUID_GUEST_NR_XSTATE];
+ } xstate;
+
+ /* Extended leaves: 0x800000xx */
+ union {
+ struct cpuid_leaf raw[CPUID_GUEST_NR_EXTD];
+ struct {
+ /* Leaf 0x80000000 - Max and vendor. */
+ uint32_t max_leaf, vendor_ebx, vendor_ecx, vendor_edx;
+
+ /* Leaf 0x80000001 - Family/model/stepping and features. */
+ uint32_t raw_fms, /* b */:32;
+ union {
+ uint32_t e1c;
+ struct { DECL_BITFIELD(e1c); };
+ };
+ union {
+ uint32_t e1d;
+ struct { DECL_BITFIELD(e1d); };
+ };
+
+ uint64_t :64, :64; /* Brand string. */
+ uint64_t :64, :64; /* Brand string. */
+ uint64_t :64, :64; /* Brand string. */
+ uint64_t :64, :64; /* L1 cache/TLB. */
+ uint64_t :64, :64; /* L2/3 cache/TLB. */
+
+ /* Leaf 0x80000007 - Advanced Power Management. */
+ uint32_t /* a */:32, /* b */:32, /* c */:32;
+ union {
+ uint32_t e7d;
+ struct { DECL_BITFIELD(e7d); };
+ };
+
+ /* Leaf 0x80000008 - Misc addr/feature info. */
+ uint8_t maxphysaddr, maxlinaddr, :8, :8;
+ union {
+ uint32_t e8b;
+ struct { DECL_BITFIELD(e8b); };
+ };
+ uint32_t nc:8, :4, apic_id_size:4, :16;
+ uint32_t /* d */:32;
+
+ uint64_t :64, :64; /* Leaf 0x80000009. */
+ uint64_t :64, :64; /* Leaf 0x8000000a - SVM rev and features. */
+ uint64_t :64, :64; /* Leaf 0x8000000b. */
+ uint64_t :64, :64; /* Leaf 0x8000000c. */
+ uint64_t :64, :64; /* Leaf 0x8000000d. */
+ uint64_t :64, :64; /* Leaf 0x8000000e. */
+ uint64_t :64, :64; /* Leaf 0x8000000f. */
+ uint64_t :64, :64; /* Leaf 0x80000010. */
+ uint64_t :64, :64; /* Leaf 0x80000011. */
+ uint64_t :64, :64; /* Leaf 0x80000012. */
+ uint64_t :64, :64; /* Leaf 0x80000013. */
+ uint64_t :64, :64; /* Leaf 0x80000014. */
+ uint64_t :64, :64; /* Leaf 0x80000015. */
+ uint64_t :64, :64; /* Leaf 0x80000016. */
+ uint64_t :64, :64; /* Leaf 0x80000017. */
+ uint64_t :64, :64; /* Leaf 0x80000018. */
+ uint64_t :64, :64; /* Leaf 0x80000019 - TLB 1GB Identifiers. */
+ uint64_t :64, :64; /* Leaf 0x8000001a - Performance related info. */
+ uint64_t :64, :64; /* Leaf 0x8000001b - IBS feature information. */
+ 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. */
+ uint64_t :64, :64; /* Leaf 0x80000020 - Platform QoS. */
+
+ /* Leaf 0x80000021 - Extended Feature 2 */
+ union {
+ uint32_t e21a;
+ struct { DECL_BITFIELD(e21a); };
+ };
+ uint32_t /* b */:32, /* c */:32, /* d */:32;
+ };
+ } extd;
+
+#undef __DECL_BITFIELD
+#undef _DECL_BITFIELD
+#undef DECL_BITFIELD
+
+ /* Toolstack selected Hypervisor max_leaf (if non-zero). */
+ uint8_t hv_limit, hv2_limit;
+
+ /* Value calculated from raw data above. */
+ uint8_t x86_vendor;
+};
+
+/* Temporary */
+#define cpuid_policy cpu_policy
+
struct old_cpu_policy
{
struct cpuid_policy *cpuid;
@@ -19,6 +352,134 @@ struct cpu_policy_errors
#define INIT_CPU_POLICY_ERRORS { -1, -1, -1 }
+/* Fill in a featureset bitmap from a CPUID policy. */
+static inline void cpuid_policy_to_featureset(
+ const struct cpuid_policy *p, uint32_t fs[FEATURESET_NR_ENTRIES])
+{
+ fs[FEATURESET_1d] = p->basic._1d;
+ fs[FEATURESET_1c] = p->basic._1c;
+ fs[FEATURESET_e1d] = p->extd.e1d;
+ fs[FEATURESET_e1c] = p->extd.e1c;
+ fs[FEATURESET_Da1] = p->xstate.Da1;
+ fs[FEATURESET_7b0] = p->feat._7b0;
+ fs[FEATURESET_7c0] = p->feat._7c0;
+ fs[FEATURESET_e7d] = p->extd.e7d;
+ fs[FEATURESET_e8b] = p->extd.e8b;
+ fs[FEATURESET_7d0] = p->feat._7d0;
+ fs[FEATURESET_7a1] = p->feat._7a1;
+ fs[FEATURESET_e21a] = p->extd.e21a;
+ fs[FEATURESET_7b1] = p->feat._7b1;
+ fs[FEATURESET_7d2] = p->feat._7d2;
+ fs[FEATURESET_7c1] = p->feat._7c1;
+ fs[FEATURESET_7d1] = p->feat._7d1;
+}
+
+/* Fill in a CPUID policy from a featureset bitmap. */
+static inline void cpuid_featureset_to_policy(
+ const uint32_t fs[FEATURESET_NR_ENTRIES], struct cpuid_policy *p)
+{
+ p->basic._1d = fs[FEATURESET_1d];
+ p->basic._1c = fs[FEATURESET_1c];
+ p->extd.e1d = fs[FEATURESET_e1d];
+ p->extd.e1c = fs[FEATURESET_e1c];
+ p->xstate.Da1 = fs[FEATURESET_Da1];
+ p->feat._7b0 = fs[FEATURESET_7b0];
+ p->feat._7c0 = fs[FEATURESET_7c0];
+ p->extd.e7d = fs[FEATURESET_e7d];
+ p->extd.e8b = fs[FEATURESET_e8b];
+ p->feat._7d0 = fs[FEATURESET_7d0];
+ p->feat._7a1 = fs[FEATURESET_7a1];
+ p->extd.e21a = fs[FEATURESET_e21a];
+ p->feat._7b1 = fs[FEATURESET_7b1];
+ p->feat._7d2 = fs[FEATURESET_7d2];
+ p->feat._7c1 = fs[FEATURESET_7c1];
+ p->feat._7d1 = fs[FEATURESET_7d1];
+}
+
+static inline uint64_t cpuid_policy_xcr0_max(const struct cpuid_policy *p)
+{
+ return ((uint64_t)p->xstate.xcr0_high << 32) | p->xstate.xcr0_low;
+}
+
+static inline uint64_t cpuid_policy_xstates(const struct cpuid_policy *p)
+{
+ uint64_t val = p->xstate.xcr0_high | p->xstate.xss_high;
+
+ return (val << 32) | p->xstate.xcr0_low | p->xstate.xss_low;
+}
+
+const uint32_t *x86_cpuid_lookup_deep_deps(uint32_t feature);
+
+/**
+ * Recalculate the content in a CPUID policy which is derived from raw data.
+ */
+void x86_cpuid_policy_recalc_synth(struct cpuid_policy *p);
+
+/**
+ * Fill a CPUID policy using the native CPUID instruction.
+ *
+ * No sanitisation is performed, but synthesised values are calculated.
+ * Values may be influenced by a hypervisor or from masking/faulting
+ * configuration.
+ */
+void x86_cpuid_policy_fill_native(struct cpuid_policy *p);
+
+/**
+ * Clear leaf data beyond the policies max leaf/subleaf settings.
+ *
+ * Policy serialisation purposefully omits out-of-range leaves, because there
+ * are a large number of them due to vendor differences. However, when
+ * constructing new policies (e.g. levelling down), it is possible to end up
+ * with out-of-range leaves with stale content in them. This helper clears
+ * them.
+ */
+void x86_cpuid_policy_clear_out_of_range_leaves(struct cpuid_policy *p);
+
+#ifdef __XEN__
+#include <public/arch-x86/xen.h>
+typedef XEN_GUEST_HANDLE_64(xen_cpuid_leaf_t) cpuid_leaf_buffer_t;
+#else
+#include <xen/arch-x86/xen.h>
+typedef xen_cpuid_leaf_t cpuid_leaf_buffer_t[];
+#endif
+
+/**
+ * Serialise a cpuid_policy object into an array of cpuid leaves.
+ *
+ * @param policy The cpuid_policy to serialise.
+ * @param leaves The array of leaves to serialise into.
+ * @param nr_entries The number of entries in 'leaves'.
+ * @returns -errno
+ *
+ * Writes at most CPUID_MAX_SERIALISED_LEAVES. May fail with -ENOBUFS if the
+ * leaves array is too short. On success, nr_entries is updated with the
+ * actual number of leaves written.
+ */
+int x86_cpuid_copy_to_buffer(const struct cpuid_policy *policy,
+ cpuid_leaf_buffer_t leaves, uint32_t *nr_entries);
+
+/**
+ * Unserialise a cpuid_policy object from an array of cpuid leaves.
+ *
+ * @param policy The cpuid_policy to unserialise into.
+ * @param leaves The array of leaves to unserialise from.
+ * @param nr_entries The number of entries in 'leaves'.
+ * @param err_leaf Optional hint for error diagnostics.
+ * @param err_subleaf Optional hint for error diagnostics.
+ * @returns -errno
+ *
+ * Reads at most CPUID_MAX_SERIALISED_LEAVES. May return -ERANGE if an
+ * incoming leaf is out of range of cpuid_policy, in which case the optional
+ * err_* pointers will identify the out-of-range indicies.
+ *
+ * No content validation of in-range leaves is performed. Synthesised data is
+ * recalculated.
+ */
+int x86_cpuid_copy_from_buffer(struct cpuid_policy *policy,
+ const cpuid_leaf_buffer_t leaves,
+ uint32_t nr_entries, uint32_t *err_leaf,
+ uint32_t *err_subleaf);
+
/*
* Calculate whether two policies are compatible.
*
diff --git a/xen/include/xen/lib/x86/cpuid.h b/xen/include/xen/lib/x86/cpuid.h
deleted file mode 100644
index fa98b371eef4..000000000000
--- a/xen/include/xen/lib/x86/cpuid.h
+++ /dev/null
@@ -1,475 +0,0 @@
-/* Common data structures and functions consumed by hypervisor and toolstack */
-#ifndef XEN_LIB_X86_CPUID_H
-#define XEN_LIB_X86_CPUID_H
-
-#include <xen/lib/x86/cpuid-autogen.h>
-
-#define FEATURESET_1d 0 /* 0x00000001.edx */
-#define FEATURESET_1c 1 /* 0x00000001.ecx */
-#define FEATURESET_e1d 2 /* 0x80000001.edx */
-#define FEATURESET_e1c 3 /* 0x80000001.ecx */
-#define FEATURESET_Da1 4 /* 0x0000000d:1.eax */
-#define FEATURESET_7b0 5 /* 0x00000007:0.ebx */
-#define FEATURESET_7c0 6 /* 0x00000007:0.ecx */
-#define FEATURESET_e7d 7 /* 0x80000007.edx */
-#define FEATURESET_e8b 8 /* 0x80000008.ebx */
-#define FEATURESET_7d0 9 /* 0x00000007:0.edx */
-#define FEATURESET_7a1 10 /* 0x00000007:1.eax */
-#define FEATURESET_e21a 11 /* 0x80000021.eax */
-#define FEATURESET_7b1 12 /* 0x00000007:1.ebx */
-#define FEATURESET_7d2 13 /* 0x00000007:2.edx */
-#define FEATURESET_7c1 14 /* 0x00000007:1.ecx */
-#define FEATURESET_7d1 15 /* 0x00000007:1.edx */
-
-struct cpuid_leaf
-{
- uint32_t a, b, c, d;
-};
-
-/*
- * Versions of GCC before 5 unconditionally reserve %rBX as the PIC hard
- * register, and are unable to cope with spilling it. This results in a
- * rather cryptic error:
- * error: inconsistent operand constraints in an ‘asm’
- *
- * In affected situations, work around the issue by using a separate register
- * to hold the the %rBX output, and xchg twice to leave %rBX preserved around
- * the asm() statement.
- */
-#if defined(__PIC__) && __GNUC__ < 5 && !defined(__clang__) && defined(__i386__)
-# define XCHG_BX "xchg %%ebx, %[bx];"
-# define BX_CON [bx] "=&r"
-#elif defined(__PIC__) && __GNUC__ < 5 && !defined(__clang__) && \
- defined(__x86_64__) && (defined(__code_model_medium__) || \
- defined(__code_model_large__))
-# define XCHG_BX "xchg %%rbx, %q[bx];"
-# define BX_CON [bx] "=&r"
-#else
-# define XCHG_BX ""
-# define BX_CON "=&b"
-#endif
-
-static inline void cpuid_leaf(uint32_t leaf, struct cpuid_leaf *l)
-{
- asm ( XCHG_BX
- "cpuid;"
- XCHG_BX
- : "=a" (l->a), BX_CON (l->b), "=&c" (l->c), "=&d" (l->d)
- : "a" (leaf) );
-}
-
-static inline void cpuid_count_leaf(
- uint32_t leaf, uint32_t subleaf, struct cpuid_leaf *l)
-{
- asm ( XCHG_BX
- "cpuid;"
- XCHG_BX
- : "=a" (l->a), BX_CON (l->b), "=c" (l->c), "=&d" (l->d)
- : "a" (leaf), "c" (subleaf) );
-}
-
-#undef BX_CON
-#undef XCHG
-
-/**
- * Given the vendor id from CPUID leaf 0, look up Xen's internal integer
- * vendor ID. Returns X86_VENDOR_UNKNOWN for any unknown vendor.
- */
-unsigned int x86_cpuid_lookup_vendor(uint32_t ebx, uint32_t ecx, uint32_t edx);
-
-/**
- * Given Xen's internal vendor ID, return a string suitable for printing.
- * Returns "Unknown" for any unrecognised ID.
- */
-const char *x86_cpuid_vendor_to_str(unsigned int vendor);
-
-#define CPUID_GUEST_NR_BASIC (0xdu + 1)
-#define CPUID_GUEST_NR_CACHE (5u + 1)
-#define CPUID_GUEST_NR_FEAT (2u + 1)
-#define CPUID_GUEST_NR_TOPO (1u + 1)
-#define CPUID_GUEST_NR_XSTATE (62u + 1)
-#define CPUID_GUEST_NR_EXTD_INTEL (0x8u + 1)
-#define CPUID_GUEST_NR_EXTD_AMD (0x21u + 1)
-#define CPUID_GUEST_NR_EXTD MAX(CPUID_GUEST_NR_EXTD_INTEL, \
- CPUID_GUEST_NR_EXTD_AMD)
-
-/*
- * Maximum number of leaves a struct cpuid_policy turns into when serialised
- * for interaction with the toolstack. (Sum of all leaves in each union, less
- * the entries in basic which sub-unions hang off of.)
- */
-#define CPUID_MAX_SERIALISED_LEAVES \
- (CPUID_GUEST_NR_BASIC + \
- CPUID_GUEST_NR_FEAT - !!CPUID_GUEST_NR_FEAT + \
- CPUID_GUEST_NR_CACHE - !!CPUID_GUEST_NR_CACHE + \
- CPUID_GUEST_NR_TOPO - !!CPUID_GUEST_NR_TOPO + \
- CPUID_GUEST_NR_XSTATE - !!CPUID_GUEST_NR_XSTATE + \
- CPUID_GUEST_NR_EXTD + 2 /* hv_limit and hv2_limit */ )
-
-struct cpuid_policy
-{
-#define DECL_BITFIELD(word) _DECL_BITFIELD(FEATURESET_ ## word)
-#define _DECL_BITFIELD(x) __DECL_BITFIELD(x)
-#define __DECL_BITFIELD(x) CPUID_BITFIELD_ ## x
-
- /* Basic leaves: 0x000000xx */
- union {
- struct cpuid_leaf raw[CPUID_GUEST_NR_BASIC];
- struct {
- /* Leaf 0x0 - Max and vendor. */
- uint32_t max_leaf, vendor_ebx, vendor_ecx, vendor_edx;
-
- /* Leaf 0x1 - Family/model/stepping and features. */
- uint32_t raw_fms;
- uint8_t :8, /* Brand ID. */
- clflush_size, /* Number of 8-byte blocks per cache line. */
- lppp, /* Logical processors per package. */
- apic_id; /* Initial APIC ID. */
- union {
- uint32_t _1c;
- struct { DECL_BITFIELD(1c); };
- };
- union {
- uint32_t _1d;
- struct { DECL_BITFIELD(1d); };
- };
-
- /* Leaf 0x2 - TLB/Cache/Prefetch. */
- uint8_t l2_nr_queries; /* Documented as fixed to 1. */
- uint8_t l2_desc[15];
-
- uint64_t :64, :64; /* Leaf 0x3 - PSN. */
- uint64_t :64, :64; /* Leaf 0x4 - Structured Cache. */
- uint64_t :64, :64; /* Leaf 0x5 - MONITOR. */
- uint64_t :64, :64; /* Leaf 0x6 - Therm/Perf. */
- uint64_t :64, :64; /* Leaf 0x7 - Structured Features. */
- uint64_t :64, :64; /* Leaf 0x8 - rsvd */
- uint64_t :64, :64; /* Leaf 0x9 - DCA */
-
- /* Leaf 0xa - Intel PMU. */
- uint8_t pmu_version, _pmu[15];
-
- uint64_t :64, :64; /* Leaf 0xb - Topology. */
- uint64_t :64, :64; /* Leaf 0xc - rsvd */
- uint64_t :64, :64; /* Leaf 0xd - XSTATE. */
- };
- } basic;
-
- /* Structured cache leaf: 0x00000004[xx] */
- union {
- struct cpuid_leaf raw[CPUID_GUEST_NR_CACHE];
- struct cpuid_cache_leaf {
- uint32_t /* a */ type:5, level:3;
- bool self_init:1, fully_assoc:1;
- uint32_t :4, threads_per_cache:12, cores_per_package:6;
- uint32_t /* b */ line_size:12, partitions:10, ways:10;
- uint32_t /* c */ sets;
- bool /* d */ wbinvd:1, inclusive:1, complex:1;
- } subleaf[CPUID_GUEST_NR_CACHE];
- } cache;
-
- /* Structured feature leaf: 0x00000007[xx] */
- union {
- struct cpuid_leaf raw[CPUID_GUEST_NR_FEAT];
- struct {
- /* Subleaf 0. */
- uint32_t max_subleaf;
- union {
- uint32_t _7b0;
- struct { DECL_BITFIELD(7b0); };
- };
- union {
- uint32_t _7c0;
- struct { DECL_BITFIELD(7c0); };
- };
- union {
- uint32_t _7d0;
- struct { DECL_BITFIELD(7d0); };
- };
-
- /* Subleaf 1. */
- union {
- uint32_t _7a1;
- struct { DECL_BITFIELD(7a1); };
- };
- union {
- uint32_t _7b1;
- struct { DECL_BITFIELD(7b1); };
- };
- union {
- uint32_t _7c1;
- struct { DECL_BITFIELD(7c1); };
- };
- union {
- uint32_t _7d1;
- struct { DECL_BITFIELD(7d1); };
- };
-
- /* Subleaf 2. */
- uint32_t /* a */:32, /* b */:32, /* c */:32;
- union {
- uint32_t _7d2;
- struct { DECL_BITFIELD(7d2); };
- };
- };
- } feat;
-
- /* Extended topology enumeration: 0x0000000B[xx] */
- union {
- struct cpuid_leaf raw[CPUID_GUEST_NR_TOPO];
- struct cpuid_topo_leaf {
- uint32_t id_shift:5, :27;
- uint16_t nr_logical, :16;
- uint8_t level, type, :8, :8;
- uint32_t x2apic_id;
- } subleaf[CPUID_GUEST_NR_TOPO];
- } topo;
-
- /* Xstate feature leaf: 0x0000000D[xx] */
- union {
- struct cpuid_leaf raw[CPUID_GUEST_NR_XSTATE];
-
- struct {
- /* Subleaf 0. */
- uint32_t xcr0_low, /* b */:32, max_size, xcr0_high;
-
- /* Subleaf 1. */
- union {
- uint32_t Da1;
- struct { DECL_BITFIELD(Da1); };
- };
- uint32_t /* b */:32, xss_low, xss_high;
- };
-
- /* Per-component common state. Valid for i >= 2. */
- struct {
- uint32_t size, offset;
- bool xss:1, align:1;
- uint32_t _res_d;
- } comp[CPUID_GUEST_NR_XSTATE];
- } xstate;
-
- /* Extended leaves: 0x800000xx */
- union {
- struct cpuid_leaf raw[CPUID_GUEST_NR_EXTD];
- struct {
- /* Leaf 0x80000000 - Max and vendor. */
- uint32_t max_leaf, vendor_ebx, vendor_ecx, vendor_edx;
-
- /* Leaf 0x80000001 - Family/model/stepping and features. */
- uint32_t raw_fms, /* b */:32;
- union {
- uint32_t e1c;
- struct { DECL_BITFIELD(e1c); };
- };
- union {
- uint32_t e1d;
- struct { DECL_BITFIELD(e1d); };
- };
-
- uint64_t :64, :64; /* Brand string. */
- uint64_t :64, :64; /* Brand string. */
- uint64_t :64, :64; /* Brand string. */
- uint64_t :64, :64; /* L1 cache/TLB. */
- uint64_t :64, :64; /* L2/3 cache/TLB. */
-
- /* Leaf 0x80000007 - Advanced Power Management. */
- uint32_t /* a */:32, /* b */:32, /* c */:32;
- union {
- uint32_t e7d;
- struct { DECL_BITFIELD(e7d); };
- };
-
- /* Leaf 0x80000008 - Misc addr/feature info. */
- uint8_t maxphysaddr, maxlinaddr, :8, :8;
- union {
- uint32_t e8b;
- struct { DECL_BITFIELD(e8b); };
- };
- uint32_t nc:8, :4, apic_id_size:4, :16;
- uint32_t /* d */:32;
-
- uint64_t :64, :64; /* Leaf 0x80000009. */
- uint64_t :64, :64; /* Leaf 0x8000000a - SVM rev and features. */
- uint64_t :64, :64; /* Leaf 0x8000000b. */
- uint64_t :64, :64; /* Leaf 0x8000000c. */
- uint64_t :64, :64; /* Leaf 0x8000000d. */
- uint64_t :64, :64; /* Leaf 0x8000000e. */
- uint64_t :64, :64; /* Leaf 0x8000000f. */
- uint64_t :64, :64; /* Leaf 0x80000010. */
- uint64_t :64, :64; /* Leaf 0x80000011. */
- uint64_t :64, :64; /* Leaf 0x80000012. */
- uint64_t :64, :64; /* Leaf 0x80000013. */
- uint64_t :64, :64; /* Leaf 0x80000014. */
- uint64_t :64, :64; /* Leaf 0x80000015. */
- uint64_t :64, :64; /* Leaf 0x80000016. */
- uint64_t :64, :64; /* Leaf 0x80000017. */
- uint64_t :64, :64; /* Leaf 0x80000018. */
- uint64_t :64, :64; /* Leaf 0x80000019 - TLB 1GB Identifiers. */
- uint64_t :64, :64; /* Leaf 0x8000001a - Performance related info. */
- uint64_t :64, :64; /* Leaf 0x8000001b - IBS feature information. */
- 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. */
- uint64_t :64, :64; /* Leaf 0x80000020 - Platform QoS. */
-
- /* Leaf 0x80000021 - Extended Feature 2 */
- union {
- uint32_t e21a;
- struct { DECL_BITFIELD(e21a); };
- };
- uint32_t /* b */:32, /* c */:32, /* d */:32;
- };
- } extd;
-
-#undef __DECL_BITFIELD
-#undef _DECL_BITFIELD
-#undef DECL_BITFIELD
-
- /* Toolstack selected Hypervisor max_leaf (if non-zero). */
- uint8_t hv_limit, hv2_limit;
-
- /* Value calculated from raw data above. */
- uint8_t x86_vendor;
-};
-
-/* Fill in a featureset bitmap from a CPUID policy. */
-static inline void cpuid_policy_to_featureset(
- const struct cpuid_policy *p, uint32_t fs[FEATURESET_NR_ENTRIES])
-{
- fs[FEATURESET_1d] = p->basic._1d;
- fs[FEATURESET_1c] = p->basic._1c;
- fs[FEATURESET_e1d] = p->extd.e1d;
- fs[FEATURESET_e1c] = p->extd.e1c;
- fs[FEATURESET_Da1] = p->xstate.Da1;
- fs[FEATURESET_7b0] = p->feat._7b0;
- fs[FEATURESET_7c0] = p->feat._7c0;
- fs[FEATURESET_e7d] = p->extd.e7d;
- fs[FEATURESET_e8b] = p->extd.e8b;
- fs[FEATURESET_7d0] = p->feat._7d0;
- fs[FEATURESET_7a1] = p->feat._7a1;
- fs[FEATURESET_e21a] = p->extd.e21a;
- fs[FEATURESET_7b1] = p->feat._7b1;
- fs[FEATURESET_7d2] = p->feat._7d2;
- fs[FEATURESET_7c1] = p->feat._7c1;
- fs[FEATURESET_7d1] = p->feat._7d1;
-}
-
-/* Fill in a CPUID policy from a featureset bitmap. */
-static inline void cpuid_featureset_to_policy(
- const uint32_t fs[FEATURESET_NR_ENTRIES], struct cpuid_policy *p)
-{
- p->basic._1d = fs[FEATURESET_1d];
- p->basic._1c = fs[FEATURESET_1c];
- p->extd.e1d = fs[FEATURESET_e1d];
- p->extd.e1c = fs[FEATURESET_e1c];
- p->xstate.Da1 = fs[FEATURESET_Da1];
- p->feat._7b0 = fs[FEATURESET_7b0];
- p->feat._7c0 = fs[FEATURESET_7c0];
- p->extd.e7d = fs[FEATURESET_e7d];
- p->extd.e8b = fs[FEATURESET_e8b];
- p->feat._7d0 = fs[FEATURESET_7d0];
- p->feat._7a1 = fs[FEATURESET_7a1];
- p->extd.e21a = fs[FEATURESET_e21a];
- p->feat._7b1 = fs[FEATURESET_7b1];
- p->feat._7d2 = fs[FEATURESET_7d2];
- p->feat._7c1 = fs[FEATURESET_7c1];
- p->feat._7d1 = fs[FEATURESET_7d1];
-}
-
-static inline uint64_t cpuid_policy_xcr0_max(const struct cpuid_policy *p)
-{
- return ((uint64_t)p->xstate.xcr0_high << 32) | p->xstate.xcr0_low;
-}
-
-static inline uint64_t cpuid_policy_xstates(const struct cpuid_policy *p)
-{
- uint64_t val = p->xstate.xcr0_high | p->xstate.xss_high;
-
- return (val << 32) | p->xstate.xcr0_low | p->xstate.xss_low;
-}
-
-const uint32_t *x86_cpuid_lookup_deep_deps(uint32_t feature);
-
-/**
- * Recalculate the content in a CPUID policy which is derived from raw data.
- */
-void x86_cpuid_policy_recalc_synth(struct cpuid_policy *p);
-
-/**
- * Fill a CPUID policy using the native CPUID instruction.
- *
- * No sanitisation is performed, but synthesised values are calculated.
- * Values may be influenced by a hypervisor or from masking/faulting
- * configuration.
- */
-void x86_cpuid_policy_fill_native(struct cpuid_policy *p);
-
-/**
- * Clear leaf data beyond the policies max leaf/subleaf settings.
- *
- * Policy serialisation purposefully omits out-of-range leaves, because there
- * are a large number of them due to vendor differences. However, when
- * constructing new policies (e.g. levelling down), it is possible to end up
- * with out-of-range leaves with stale content in them. This helper clears
- * them.
- */
-void x86_cpuid_policy_clear_out_of_range_leaves(struct cpuid_policy *p);
-
-#ifdef __XEN__
-#include <public/arch-x86/xen.h>
-typedef XEN_GUEST_HANDLE_64(xen_cpuid_leaf_t) cpuid_leaf_buffer_t;
-#else
-#include <xen/arch-x86/xen.h>
-typedef xen_cpuid_leaf_t cpuid_leaf_buffer_t[];
-#endif
-
-/**
- * Serialise a cpuid_policy object into an array of cpuid leaves.
- *
- * @param policy The cpuid_policy to serialise.
- * @param leaves The array of leaves to serialise into.
- * @param nr_entries The number of entries in 'leaves'.
- * @returns -errno
- *
- * Writes at most CPUID_MAX_SERIALISED_LEAVES. May fail with -ENOBUFS if the
- * leaves array is too short. On success, nr_entries is updated with the
- * actual number of leaves written.
- */
-int x86_cpuid_copy_to_buffer(const struct cpuid_policy *policy,
- cpuid_leaf_buffer_t leaves, uint32_t *nr_entries);
-
-/**
- * Unserialise a cpuid_policy object from an array of cpuid leaves.
- *
- * @param policy The cpuid_policy to unserialise into.
- * @param leaves The array of leaves to unserialise from.
- * @param nr_entries The number of entries in 'leaves'.
- * @param err_leaf Optional hint for error diagnostics.
- * @param err_subleaf Optional hint for error diagnostics.
- * @returns -errno
- *
- * Reads at most CPUID_MAX_SERIALISED_LEAVES. May return -ERANGE if an
- * incoming leaf is out of range of cpuid_policy, in which case the optional
- * err_* pointers will identify the out-of-range indicies.
- *
- * No content validation of in-range leaves is performed. Synthesised data is
- * recalculated.
- */
-int x86_cpuid_copy_from_buffer(struct cpuid_policy *policy,
- const cpuid_leaf_buffer_t leaves,
- uint32_t nr_entries, uint32_t *err_leaf,
- uint32_t *err_subleaf);
-
-#endif /* !XEN_LIB_X86_CPUID_H */
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * tab-width: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/lib/x86/cpuid.c b/xen/lib/x86/cpuid.c
index 8eb88314f53c..e81f76c779c0 100644
--- a/xen/lib/x86/cpuid.c
+++ b/xen/lib/x86/cpuid.c
@@ -1,6 +1,6 @@
#include "private.h"
-#include <xen/lib/x86/cpuid.h>
+#include <xen/lib/x86/cpu-policy.h>
static void zero_leaves(struct cpuid_leaf *l,
unsigned int first, unsigned int last)
--
2.30.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 4/9] x86: Merge struct msr_policy into struct cpu_policy
2023-03-29 20:51 [PATCH RFC 0/9] x86: Merge cpuid and msr policy Andrew Cooper
` (2 preceding siblings ...)
2023-03-29 20:51 ` [PATCH 3/9] x86: Rename struct cpuid_policy to struct cpu_policy Andrew Cooper
@ 2023-03-29 20:51 ` Andrew Cooper
2023-03-30 9:30 ` Jan Beulich
2023-03-29 20:51 ` [PATCH 5/9] x86: Merge the system {cpuid,msr} policy objects Andrew Cooper
` (6 subsequent siblings)
10 siblings, 1 reply; 34+ messages in thread
From: Andrew Cooper @ 2023-03-29 20:51 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné, Wei Liu
As with the cpuid side, use a temporary define to make struct msr_policy still
work.
Note, this means that domains now have two separate struct cpu_policy
allocations with disjoint information, and system policies are in a similar
position. Both will be deduplicated in the following patches.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
---
tools/fuzz/cpu-policy/afl-policy-fuzzer.c | 1 -
xen/arch/x86/include/asm/msr.h | 3 +-
xen/include/xen/lib/x86/cpu-policy.h | 81 ++++++++++++++++-
xen/include/xen/lib/x86/msr.h | 104 ----------------------
xen/lib/x86/msr.c | 2 +-
5 files changed, 83 insertions(+), 108 deletions(-)
delete mode 100644 xen/include/xen/lib/x86/msr.h
diff --git a/tools/fuzz/cpu-policy/afl-policy-fuzzer.c b/tools/fuzz/cpu-policy/afl-policy-fuzzer.c
index 79e42e8bfd04..0ce3d8e16626 100644
--- a/tools/fuzz/cpu-policy/afl-policy-fuzzer.c
+++ b/tools/fuzz/cpu-policy/afl-policy-fuzzer.c
@@ -10,7 +10,6 @@
#include <xen-tools/common-macros.h>
#include <xen/lib/x86/cpu-policy.h>
-#include <xen/lib/x86/msr.h>
#include <xen/domctl.h>
static bool debug;
diff --git a/xen/arch/x86/include/asm/msr.h b/xen/arch/x86/include/asm/msr.h
index 7946b6b24c11..02eddd919c27 100644
--- a/xen/arch/x86/include/asm/msr.h
+++ b/xen/arch/x86/include/asm/msr.h
@@ -6,8 +6,9 @@
#include <xen/types.h>
#include <xen/percpu.h>
#include <xen/errno.h>
+#include <xen/kernel.h>
-#include <xen/lib/x86/msr.h>
+#include <xen/lib/x86/cpu-policy.h>
#include <asm/asm_defns.h>
#include <asm/cpufeature.h>
diff --git a/xen/include/xen/lib/x86/cpu-policy.h b/xen/include/xen/lib/x86/cpu-policy.h
index 5e430d848021..5af756a02da0 100644
--- a/xen/include/xen/lib/x86/cpu-policy.h
+++ b/xen/include/xen/lib/x86/cpu-policy.h
@@ -3,7 +3,6 @@
#define XEN_LIB_X86_POLICIES_H
#include <xen/lib/x86/cpuid-autogen.h>
-#include <xen/lib/x86/msr.h>
#define FEATURESET_1d 0 /* 0x00000001.edx */
#define FEATURESET_1c 1 /* 0x00000001.ecx */
@@ -107,6 +106,9 @@ const char *x86_cpuid_vendor_to_str(unsigned int vendor);
CPUID_GUEST_NR_XSTATE - !!CPUID_GUEST_NR_XSTATE + \
CPUID_GUEST_NR_EXTD + 2 /* hv_limit and hv2_limit */ )
+/* Maximum number of MSRs written when serialising msr_policy. */
+#define MSR_MAX_SERIALISED_ENTRIES 2
+
struct cpu_policy
{
#define DECL_BITFIELD(word) _DECL_BITFIELD(FEATURESET_ ## word)
@@ -324,6 +326,44 @@ struct cpu_policy
};
} extd;
+ /*
+ * 0x000000ce - MSR_INTEL_PLATFORM_INFO
+ *
+ * This MSR is non-architectural, but for simplicy we allow it to be read
+ * unconditionally. CPUID Faulting support can be fully emulated for HVM
+ * guests so can be offered unconditionally, while support for PV guests
+ * is dependent on real hardware support.
+ */
+ union {
+ uint32_t raw;
+ struct {
+ uint32_t :31;
+ bool cpuid_faulting:1;
+ };
+ } platform_info;
+
+ /*
+ * 0x0000010a - MSR_ARCH_CAPABILITIES
+ *
+ * This is an Intel-only MSR, which provides miscellaneous enumeration,
+ * including those which indicate that microarchitectrual sidechannels are
+ * fixed in hardware.
+ */
+ union {
+ uint32_t raw;
+ struct {
+ bool rdcl_no:1;
+ bool ibrs_all:1;
+ bool rsba:1;
+ bool skip_l1dfl:1;
+ bool ssb_no:1;
+ bool mds_no:1;
+ bool if_pschange_mc_no:1;
+ bool tsx_ctrl:1;
+ bool taa_no:1;
+ };
+ } arch_caps;
+
#undef __DECL_BITFIELD
#undef _DECL_BITFIELD
#undef DECL_BITFIELD
@@ -337,6 +377,7 @@ struct cpu_policy
/* Temporary */
#define cpuid_policy cpu_policy
+#define msr_policy cpu_policy
struct old_cpu_policy
{
@@ -438,9 +479,11 @@ void x86_cpuid_policy_clear_out_of_range_leaves(struct cpuid_policy *p);
#ifdef __XEN__
#include <public/arch-x86/xen.h>
typedef XEN_GUEST_HANDLE_64(xen_cpuid_leaf_t) cpuid_leaf_buffer_t;
+typedef XEN_GUEST_HANDLE_64(xen_msr_entry_t) msr_entry_buffer_t;
#else
#include <xen/arch-x86/xen.h>
typedef xen_cpuid_leaf_t cpuid_leaf_buffer_t[];
+typedef xen_msr_entry_t msr_entry_buffer_t[];
#endif
/**
@@ -480,6 +523,42 @@ int x86_cpuid_copy_from_buffer(struct cpuid_policy *policy,
uint32_t nr_entries, uint32_t *err_leaf,
uint32_t *err_subleaf);
+/**
+ * Serialise an msr_policy object into an array.
+ *
+ * @param policy The msr_policy to serialise.
+ * @param msrs The array of msrs to serialise into.
+ * @param nr_entries The number of entries in 'msrs'.
+ * @returns -errno
+ *
+ * Writes at most MSR_MAX_SERIALISED_ENTRIES. May fail with -ENOBUFS if the
+ * buffer array is too short. On success, nr_entries is updated with the
+ * actual number of msrs written.
+ */
+int x86_msr_copy_to_buffer(const struct msr_policy *policy,
+ msr_entry_buffer_t msrs, uint32_t *nr_entries);
+
+/**
+ * Unserialise an msr_policy object from an array of msrs.
+ *
+ * @param policy The msr_policy object to unserialise into.
+ * @param msrs The array of msrs to unserialise from.
+ * @param nr_entries The number of entries in 'msrs'.
+ * @param err_msr Optional hint for error diagnostics.
+ * @returns -errno
+ *
+ * Reads at most MSR_MAX_SERIALISED_ENTRIES. May fail for a number of reasons
+ * based on the content in an individual 'msrs' entry, including the MSR index
+ * not being valid in the policy, the flags field being nonzero, or if the
+ * value provided would truncate when stored in the policy. In such cases,
+ * the optional err_* pointer will identify the problematic MSR.
+ *
+ * No content validation is performed on the data stored in the policy object.
+ */
+int x86_msr_copy_from_buffer(struct msr_policy *policy,
+ const msr_entry_buffer_t msrs, uint32_t nr_entries,
+ uint32_t *err_msr);
+
/*
* Calculate whether two policies are compatible.
*
diff --git a/xen/include/xen/lib/x86/msr.h b/xen/include/xen/lib/x86/msr.h
deleted file mode 100644
index 48ba4a59c036..000000000000
--- a/xen/include/xen/lib/x86/msr.h
+++ /dev/null
@@ -1,104 +0,0 @@
-/* Common data structures and functions consumed by hypervisor and toolstack */
-#ifndef XEN_LIB_X86_MSR_H
-#define XEN_LIB_X86_MSR_H
-
-/* Maximum number of MSRs written when serialising msr_policy. */
-#define MSR_MAX_SERIALISED_ENTRIES 2
-
-/* MSR policy object for shared per-domain MSRs */
-struct msr_policy
-{
- /*
- * 0x000000ce - MSR_INTEL_PLATFORM_INFO
- *
- * This MSR is non-architectural, but for simplicy we allow it to be read
- * unconditionally. CPUID Faulting support can be fully emulated for HVM
- * guests so can be offered unconditionally, while support for PV guests
- * is dependent on real hardware support.
- */
- union {
- uint32_t raw;
- struct {
- uint32_t :31;
- bool cpuid_faulting:1;
- };
- } platform_info;
-
- /*
- * 0x0000010a - MSR_ARCH_CAPABILITIES
- *
- * This is an Intel-only MSR, which provides miscellaneous enumeration,
- * including those which indicate that microarchitectrual sidechannels are
- * fixed in hardware.
- */
- union {
- uint32_t raw;
- struct {
- bool rdcl_no:1;
- bool ibrs_all:1;
- bool rsba:1;
- bool skip_l1dfl:1;
- bool ssb_no:1;
- bool mds_no:1;
- bool if_pschange_mc_no:1;
- bool tsx_ctrl:1;
- bool taa_no:1;
- };
- } arch_caps;
-};
-
-#ifdef __XEN__
-#include <public/arch-x86/xen.h>
-typedef XEN_GUEST_HANDLE_64(xen_msr_entry_t) msr_entry_buffer_t;
-#else
-#include <xen/arch-x86/xen.h>
-typedef xen_msr_entry_t msr_entry_buffer_t[];
-#endif
-
-/**
- * Serialise an msr_policy object into an array.
- *
- * @param policy The msr_policy to serialise.
- * @param msrs The array of msrs to serialise into.
- * @param nr_entries The number of entries in 'msrs'.
- * @returns -errno
- *
- * Writes at most MSR_MAX_SERIALISED_ENTRIES. May fail with -ENOBUFS if the
- * buffer array is too short. On success, nr_entries is updated with the
- * actual number of msrs written.
- */
-int x86_msr_copy_to_buffer(const struct msr_policy *policy,
- msr_entry_buffer_t msrs, uint32_t *nr_entries);
-
-/**
- * Unserialise an msr_policy object from an array of msrs.
- *
- * @param policy The msr_policy object to unserialise into.
- * @param msrs The array of msrs to unserialise from.
- * @param nr_entries The number of entries in 'msrs'.
- * @param err_msr Optional hint for error diagnostics.
- * @returns -errno
- *
- * Reads at most MSR_MAX_SERIALISED_ENTRIES. May fail for a number of reasons
- * based on the content in an individual 'msrs' entry, including the MSR index
- * not being valid in the policy, the flags field being nonzero, or if the
- * value provided would truncate when stored in the policy. In such cases,
- * the optional err_* pointer will identify the problematic MSR.
- *
- * No content validation is performed on the data stored in the policy object.
- */
-int x86_msr_copy_from_buffer(struct msr_policy *policy,
- const msr_entry_buffer_t msrs, uint32_t nr_entries,
- uint32_t *err_msr);
-
-#endif /* !XEN_LIB_X86_MSR_H */
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * tab-width: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/lib/x86/msr.c b/xen/lib/x86/msr.c
index 7d71e92a380a..c4d885e7b568 100644
--- a/xen/lib/x86/msr.c
+++ b/xen/lib/x86/msr.c
@@ -1,6 +1,6 @@
#include "private.h"
-#include <xen/lib/x86/msr.h>
+#include <xen/lib/x86/cpu-policy.h>
/*
* Copy a single MSR into the provided msr_entry_buffer_t buffer, performing a
--
2.30.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 5/9] x86: Merge the system {cpuid,msr} policy objects
2023-03-29 20:51 [PATCH RFC 0/9] x86: Merge cpuid and msr policy Andrew Cooper
` (3 preceding siblings ...)
2023-03-29 20:51 ` [PATCH 4/9] x86: Merge struct msr_policy into " Andrew Cooper
@ 2023-03-29 20:51 ` Andrew Cooper
2023-03-30 9:40 ` Jan Beulich
2023-03-29 20:51 ` [PATCH 6/9] x86: Merge a domain's " Andrew Cooper
` (5 subsequent siblings)
10 siblings, 1 reply; 34+ messages in thread
From: Andrew Cooper @ 2023-03-29 20:51 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné, Wei Liu
Right now, they're the same underlying type, containing disjoint information.
Introduce a new cpu-policy.{h,c} to be the new location for all policy
handling logic. Place the combined objects in __ro_after_init, which has
appeared since the original logic was written.
As we're trying to phase out the use of struct old_cpu_policy entirely, rework
update_domain_cpu_policy() to not pointer-chase through system_policies[].
This in turn allows system_policies[] in sysctl.c to become static and reduced
in scope to XEN_SYSCTL_get_cpu_policy.
No practical change, but we do half the amount of compile time space required
for the system policies, which saves 6x almost-2k at the moment.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
---
xen/arch/x86/Makefile | 1 +
xen/arch/x86/cpu-policy.c | 18 +++++++
xen/arch/x86/cpu/common.c | 4 +-
xen/arch/x86/cpuid.c | 66 +++++++++++---------------
xen/arch/x86/domctl.c | 17 +++++--
xen/arch/x86/include/asm/cpu-policy.h | 14 ++++++
xen/arch/x86/include/asm/cpuid.h | 6 ---
xen/arch/x86/include/asm/msr.h | 7 ---
xen/arch/x86/msr.c | 38 ++++++---------
xen/arch/x86/sysctl.c | 67 ++++++++++-----------------
10 files changed, 114 insertions(+), 124 deletions(-)
create mode 100644 xen/arch/x86/cpu-policy.c
create mode 100644 xen/arch/x86/include/asm/cpu-policy.h
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 5accbe4c6746..f213a6b56a4d 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -18,6 +18,7 @@ obj-y += bitops.o
obj-bin-y += bzimage.init.o
obj-bin-y += clear_page.o
obj-bin-y += copy_page.o
+obj-y += cpu-policy.o
obj-y += cpuid.o
obj-$(CONFIG_PV) += compat.o
obj-$(CONFIG_PV32) += x86_64/compat.o
diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
new file mode 100644
index 000000000000..663e9a084c53
--- /dev/null
+++ b/xen/arch/x86/cpu-policy.c
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#include <xen/cache.h>
+#include <xen/kernel.h>
+
+#include <xen/lib/x86/cpu-policy.h>
+
+#include <asm/cpu-policy.h>
+
+struct cpu_policy __ro_after_init raw_cpu_policy;
+struct cpu_policy __ro_after_init host_cpu_policy;
+#ifdef CONFIG_PV
+struct cpu_policy __ro_after_init pv_max_cpu_policy;
+struct cpu_policy __ro_after_init pv_def_cpu_policy;
+#endif
+#ifdef CONFIG_HVM
+struct cpu_policy __ro_after_init hvm_max_cpu_policy;
+struct cpu_policy __ro_after_init hvm_def_cpu_policy;
+#endif
diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index 5ad347534a22..f11dcda57a69 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -3,6 +3,8 @@
#include <xen/delay.h>
#include <xen/param.h>
#include <xen/smp.h>
+
+#include <asm/cpu-policy.h>
#include <asm/current.h>
#include <asm/debugreg.h>
#include <asm/processor.h>
@@ -141,7 +143,7 @@ bool __init probe_cpuid_faulting(void)
return false;
if ((rc = rdmsr_safe(MSR_INTEL_PLATFORM_INFO, val)) == 0)
- raw_msr_policy.platform_info.cpuid_faulting =
+ raw_cpu_policy.platform_info.cpuid_faulting =
val & MSR_PLATFORM_INFO_CPUID_FAULTING;
if (rc ||
diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index b22725c492e7..0916bfe175c8 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -4,6 +4,7 @@
#include <xen/sched.h>
#include <xen/nospec.h>
#include <asm/amd.h>
+#include <asm/cpu-policy.h>
#include <asm/cpuid.h>
#include <asm/hvm/hvm.h>
#include <asm/hvm/nestedhvm.h>
@@ -142,17 +143,6 @@ static void zero_leaves(struct cpuid_leaf *l,
memset(&l[first], 0, sizeof(*l) * (last - first + 1));
}
-struct cpuid_policy __read_mostly raw_cpuid_policy,
- __read_mostly host_cpuid_policy;
-#ifdef CONFIG_PV
-struct cpuid_policy __read_mostly pv_max_cpuid_policy;
-struct cpuid_policy __read_mostly pv_def_cpuid_policy;
-#endif
-#ifdef CONFIG_HVM
-struct cpuid_policy __read_mostly hvm_max_cpuid_policy;
-struct cpuid_policy __read_mostly hvm_def_cpuid_policy;
-#endif
-
static void sanitise_featureset(uint32_t *fs)
{
/* for_each_set_bit() uses unsigned longs. Extend with zeroes. */
@@ -344,7 +334,7 @@ static void recalculate_misc(struct cpuid_policy *p)
static void __init calculate_raw_policy(void)
{
- struct cpuid_policy *p = &raw_cpuid_policy;
+ struct cpuid_policy *p = &raw_cpu_policy;
x86_cpuid_policy_fill_native(p);
@@ -354,10 +344,10 @@ static void __init calculate_raw_policy(void)
static void __init calculate_host_policy(void)
{
- struct cpuid_policy *p = &host_cpuid_policy;
+ struct cpuid_policy *p = &host_cpu_policy;
unsigned int max_extd_leaf;
- *p = raw_cpuid_policy;
+ *p = raw_cpu_policy;
p->basic.max_leaf =
min_t(uint32_t, p->basic.max_leaf, ARRAY_SIZE(p->basic.raw) - 1);
@@ -449,17 +439,17 @@ static void __init guest_common_feature_adjustments(uint32_t *fs)
* of IBRS by using the AMD feature bit. An administrator may wish for
* performance reasons to offer IBPB without IBRS.
*/
- if ( host_cpuid_policy.feat.ibrsb )
+ if ( host_cpu_policy.feat.ibrsb )
__set_bit(X86_FEATURE_IBPB, fs);
}
static void __init calculate_pv_max_policy(void)
{
- struct cpuid_policy *p = &pv_max_cpuid_policy;
+ struct cpuid_policy *p = &pv_max_cpu_policy;
uint32_t pv_featureset[FSCAPINTS];
unsigned int i;
- *p = host_cpuid_policy;
+ *p = host_cpu_policy;
cpuid_policy_to_featureset(p, pv_featureset);
for ( i = 0; i < ARRAY_SIZE(pv_featureset); ++i )
@@ -486,11 +476,11 @@ static void __init calculate_pv_max_policy(void)
static void __init calculate_pv_def_policy(void)
{
- struct cpuid_policy *p = &pv_def_cpuid_policy;
+ struct cpuid_policy *p = &pv_def_cpu_policy;
uint32_t pv_featureset[FSCAPINTS];
unsigned int i;
- *p = pv_max_cpuid_policy;
+ *p = pv_max_cpu_policy;
cpuid_policy_to_featureset(p, pv_featureset);
for ( i = 0; i < ARRAY_SIZE(pv_featureset); ++i )
@@ -506,12 +496,12 @@ static void __init calculate_pv_def_policy(void)
static void __init calculate_hvm_max_policy(void)
{
- struct cpuid_policy *p = &hvm_max_cpuid_policy;
+ struct cpuid_policy *p = &hvm_max_cpu_policy;
uint32_t hvm_featureset[FSCAPINTS];
unsigned int i;
const uint32_t *hvm_featuremask;
- *p = host_cpuid_policy;
+ *p = host_cpu_policy;
cpuid_policy_to_featureset(p, hvm_featureset);
hvm_featuremask = hvm_hap_supported() ?
@@ -539,7 +529,7 @@ static void __init calculate_hvm_max_policy(void)
* HVM guests are able if running in protected mode.
*/
if ( (boot_cpu_data.x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) &&
- raw_cpuid_policy.basic.sep )
+ raw_cpu_policy.basic.sep )
__set_bit(X86_FEATURE_SEP, hvm_featureset);
/*
@@ -597,12 +587,12 @@ static void __init calculate_hvm_max_policy(void)
static void __init calculate_hvm_def_policy(void)
{
- struct cpuid_policy *p = &hvm_def_cpuid_policy;
+ struct cpuid_policy *p = &hvm_def_cpu_policy;
uint32_t hvm_featureset[FSCAPINTS];
unsigned int i;
const uint32_t *hvm_featuremask;
- *p = hvm_max_cpuid_policy;
+ *p = hvm_max_cpu_policy;
cpuid_policy_to_featureset(p, hvm_featureset);
hvm_featuremask = hvm_hap_supported() ?
@@ -670,8 +660,8 @@ void recalculate_cpuid_policy(struct domain *d)
{
struct cpuid_policy *p = d->arch.cpuid;
const struct cpuid_policy *max = is_pv_domain(d)
- ? (IS_ENABLED(CONFIG_PV) ? &pv_max_cpuid_policy : NULL)
- : (IS_ENABLED(CONFIG_HVM) ? &hvm_max_cpuid_policy : NULL);
+ ? (IS_ENABLED(CONFIG_PV) ? &pv_max_cpu_policy : NULL)
+ : (IS_ENABLED(CONFIG_HVM) ? &hvm_max_cpu_policy : NULL);
uint32_t fs[FSCAPINTS], max_fs[FSCAPINTS];
unsigned int i;
@@ -746,7 +736,7 @@ void recalculate_cpuid_policy(struct domain *d)
/* Fold host's FDP_EXCP_ONLY and NO_FPU_SEL into guest's view. */
fs[FEATURESET_7b0] &= ~(cpufeat_mask(X86_FEATURE_FDP_EXCP_ONLY) |
cpufeat_mask(X86_FEATURE_NO_FPU_SEL));
- fs[FEATURESET_7b0] |= (host_cpuid_policy.feat._7b0 &
+ fs[FEATURESET_7b0] |= (host_cpu_policy.feat._7b0 &
(cpufeat_mask(X86_FEATURE_FDP_EXCP_ONLY) |
cpufeat_mask(X86_FEATURE_NO_FPU_SEL)));
@@ -797,8 +787,8 @@ void recalculate_cpuid_policy(struct domain *d)
int init_domain_cpuid_policy(struct domain *d)
{
struct cpuid_policy *p = is_pv_domain(d)
- ? (IS_ENABLED(CONFIG_PV) ? &pv_def_cpuid_policy : NULL)
- : (IS_ENABLED(CONFIG_HVM) ? &hvm_def_cpuid_policy : NULL);
+ ? (IS_ENABLED(CONFIG_PV) ? &pv_def_cpu_policy : NULL)
+ : (IS_ENABLED(CONFIG_HVM) ? &hvm_def_cpu_policy : NULL);
if ( !p )
{
@@ -1102,7 +1092,7 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
if ( is_pv_domain(d) && is_hardware_domain(d) &&
guest_kernel_mode(v, regs) && cpu_has_monitor &&
regs->entry_vector == TRAP_gp_fault )
- *res = raw_cpuid_policy.basic.raw[5];
+ *res = raw_cpu_policy.basic.raw[5];
break;
case 0x7:
@@ -1234,14 +1224,14 @@ static void __init __maybe_unused build_assertions(void)
/* Find some more clever allocation scheme if this trips. */
BUILD_BUG_ON(sizeof(struct cpuid_policy) > PAGE_SIZE);
- BUILD_BUG_ON(sizeof(raw_cpuid_policy.basic) !=
- sizeof(raw_cpuid_policy.basic.raw));
- BUILD_BUG_ON(sizeof(raw_cpuid_policy.feat) !=
- sizeof(raw_cpuid_policy.feat.raw));
- BUILD_BUG_ON(sizeof(raw_cpuid_policy.xstate) !=
- sizeof(raw_cpuid_policy.xstate.raw));
- BUILD_BUG_ON(sizeof(raw_cpuid_policy.extd) !=
- sizeof(raw_cpuid_policy.extd.raw));
+ BUILD_BUG_ON(sizeof(raw_cpu_policy.basic) !=
+ sizeof(raw_cpu_policy.basic.raw));
+ BUILD_BUG_ON(sizeof(raw_cpu_policy.feat) !=
+ sizeof(raw_cpu_policy.feat.raw));
+ BUILD_BUG_ON(sizeof(raw_cpu_policy.xstate) !=
+ sizeof(raw_cpu_policy.xstate.raw));
+ BUILD_BUG_ON(sizeof(raw_cpu_policy.extd) !=
+ sizeof(raw_cpu_policy.extd.raw));
}
/*
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 944af63e68d0..5800bb10bc4a 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -35,18 +35,25 @@
#include <asm/mem_sharing.h>
#include <asm/xstate.h>
#include <asm/psr.h>
-#include <asm/cpuid.h>
+#include <asm/cpu-policy.h>
static int update_domain_cpu_policy(struct domain *d,
xen_domctl_cpu_policy_t *xdpc)
{
struct old_cpu_policy new = {};
- const struct old_cpu_policy *sys = is_pv_domain(d)
- ? &system_policies[XEN_SYSCTL_cpu_policy_pv_max]
- : &system_policies[XEN_SYSCTL_cpu_policy_hvm_max];
+ struct cpu_policy *sys = is_pv_domain(d)
+ ? (IS_ENABLED(CONFIG_PV) ? &pv_max_cpu_policy : NULL)
+ : (IS_ENABLED(CONFIG_HVM) ? &hvm_max_cpu_policy : NULL);
+ struct old_cpu_policy old_sys = { sys, sys };
struct cpu_policy_errors err = INIT_CPU_POLICY_ERRORS;
int ret = -ENOMEM;
+ if ( !sys )
+ {
+ ASSERT_UNREACHABLE();
+ return -EOPNOTSUPP;
+ }
+
/* Start by copying the domain's existing policies. */
if ( !(new.cpuid = xmemdup(d->arch.cpuid)) ||
!(new.msr = xmemdup(d->arch.msr)) )
@@ -64,7 +71,7 @@ static int update_domain_cpu_policy(struct domain *d,
x86_cpuid_policy_clear_out_of_range_leaves(new.cpuid);
/* Audit the combined dataset. */
- ret = x86_cpu_policies_are_compatible(sys, &new, &err);
+ ret = x86_cpu_policies_are_compatible(&old_sys, &new, &err);
if ( ret )
goto out;
diff --git a/xen/arch/x86/include/asm/cpu-policy.h b/xen/arch/x86/include/asm/cpu-policy.h
new file mode 100644
index 000000000000..eef14bb4267e
--- /dev/null
+++ b/xen/arch/x86/include/asm/cpu-policy.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef X86_CPU_POLICY_H
+#define X86_CPU_POLICY_H
+
+struct cpu_policy;
+
+extern struct cpu_policy raw_cpu_policy;
+extern struct cpu_policy host_cpu_policy;
+extern struct cpu_policy pv_max_cpu_policy;
+extern struct cpu_policy pv_def_cpu_policy;
+extern struct cpu_policy hvm_max_cpu_policy;
+extern struct cpu_policy hvm_def_cpu_policy;
+
+#endif /* X86_CPU_POLICY_H */
diff --git a/xen/arch/x86/include/asm/cpuid.h b/xen/arch/x86/include/asm/cpuid.h
index d418e8100dde..ea0586277331 100644
--- a/xen/arch/x86/include/asm/cpuid.h
+++ b/xen/arch/x86/include/asm/cpuid.h
@@ -46,12 +46,6 @@ DECLARE_PER_CPU(struct cpuidmasks, cpuidmasks);
/* Default masking MSR values, calculated at boot. */
extern struct cpuidmasks cpuidmask_defaults;
-extern struct cpuid_policy raw_cpuid_policy, host_cpuid_policy,
- pv_max_cpuid_policy, pv_def_cpuid_policy,
- hvm_max_cpuid_policy, hvm_def_cpuid_policy;
-
-extern const struct old_cpu_policy system_policies[];
-
/* Check that all previously present features are still available. */
bool recheck_cpu_features(unsigned int cpu);
diff --git a/xen/arch/x86/include/asm/msr.h b/xen/arch/x86/include/asm/msr.h
index 02eddd919c27..022230acc0af 100644
--- a/xen/arch/x86/include/asm/msr.h
+++ b/xen/arch/x86/include/asm/msr.h
@@ -292,13 +292,6 @@ static inline void wrmsr_tsc_aux(uint32_t val)
uint64_t msr_spec_ctrl_valid_bits(const struct cpuid_policy *cp);
-extern struct msr_policy raw_msr_policy,
- host_msr_policy,
- pv_max_msr_policy,
- pv_def_msr_policy,
- hvm_max_msr_policy,
- hvm_def_msr_policy;
-
/* Container object for per-vCPU MSRs */
struct vcpu_msrs
{
diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index 7ddf0078c3a2..bff26bc4e2b5 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -25,6 +25,7 @@
#include <xen/sched.h>
#include <asm/amd.h>
+#include <asm/cpu-policy.h>
#include <asm/debugreg.h>
#include <asm/hvm/nestedhvm.h>
#include <asm/hvm/viridian.h>
@@ -37,20 +38,9 @@
DEFINE_PER_CPU(uint32_t, tsc_aux);
-struct msr_policy __read_mostly raw_msr_policy,
- __read_mostly host_msr_policy;
-#ifdef CONFIG_PV
-struct msr_policy __read_mostly pv_max_msr_policy;
-struct msr_policy __read_mostly pv_def_msr_policy;
-#endif
-#ifdef CONFIG_HVM
-struct msr_policy __read_mostly hvm_max_msr_policy;
-struct msr_policy __read_mostly hvm_def_msr_policy;
-#endif
-
static void __init calculate_raw_policy(void)
{
- struct msr_policy *mp = &raw_msr_policy;
+ struct msr_policy *mp = &raw_cpu_policy;
/* 0x000000ce MSR_INTEL_PLATFORM_INFO */
/* Was already added by probe_cpuid_faulting() */
@@ -61,9 +51,9 @@ static void __init calculate_raw_policy(void)
static void __init calculate_host_policy(void)
{
- struct msr_policy *mp = &host_msr_policy;
+ struct msr_policy *mp = &host_cpu_policy;
- *mp = raw_msr_policy;
+ *mp = raw_cpu_policy;
/* 0x000000ce MSR_INTEL_PLATFORM_INFO */
/* probe_cpuid_faulting() sanity checks presence of MISC_FEATURES_ENABLES */
@@ -81,25 +71,25 @@ static void __init calculate_host_policy(void)
static void __init calculate_pv_max_policy(void)
{
- struct msr_policy *mp = &pv_max_msr_policy;
+ struct msr_policy *mp = &pv_max_cpu_policy;
- *mp = host_msr_policy;
+ *mp = host_cpu_policy;
mp->arch_caps.raw = 0; /* Not supported yet. */
}
static void __init calculate_pv_def_policy(void)
{
- struct msr_policy *mp = &pv_def_msr_policy;
+ struct msr_policy *mp = &pv_def_cpu_policy;
- *mp = pv_max_msr_policy;
+ *mp = pv_max_cpu_policy;
}
static void __init calculate_hvm_max_policy(void)
{
- struct msr_policy *mp = &hvm_max_msr_policy;
+ struct msr_policy *mp = &hvm_max_cpu_policy;
- *mp = host_msr_policy;
+ *mp = host_cpu_policy;
/* It's always possible to emulate CPUID faulting for HVM guests */
mp->platform_info.cpuid_faulting = true;
@@ -109,9 +99,9 @@ static void __init calculate_hvm_max_policy(void)
static void __init calculate_hvm_def_policy(void)
{
- struct msr_policy *mp = &hvm_def_msr_policy;
+ struct msr_policy *mp = &hvm_def_cpu_policy;
- *mp = hvm_max_msr_policy;
+ *mp = hvm_max_cpu_policy;
}
void __init init_guest_msr_policy(void)
@@ -135,8 +125,8 @@ void __init init_guest_msr_policy(void)
int init_domain_msr_policy(struct domain *d)
{
struct msr_policy *mp = is_pv_domain(d)
- ? (IS_ENABLED(CONFIG_PV) ? &pv_def_msr_policy : NULL)
- : (IS_ENABLED(CONFIG_HVM) ? &hvm_def_msr_policy : NULL);
+ ? (IS_ENABLED(CONFIG_PV) ? &pv_def_cpu_policy : NULL)
+ : (IS_ENABLED(CONFIG_HVM) ? &hvm_def_cpu_policy : NULL);
if ( !mp )
{
diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
index 3ed7c69f4315..3b2efecede2f 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -30,38 +30,7 @@
#include <xen/cpu.h>
#include <xsm/xsm.h>
#include <asm/psr.h>
-#include <asm/cpuid.h>
-
-const struct old_cpu_policy system_policies[6] = {
- [ XEN_SYSCTL_cpu_policy_raw ] = {
- &raw_cpuid_policy,
- &raw_msr_policy,
- },
- [ XEN_SYSCTL_cpu_policy_host ] = {
- &host_cpuid_policy,
- &host_msr_policy,
- },
-#ifdef CONFIG_PV
- [ XEN_SYSCTL_cpu_policy_pv_max ] = {
- &pv_max_cpuid_policy,
- &pv_max_msr_policy,
- },
- [ XEN_SYSCTL_cpu_policy_pv_default ] = {
- &pv_def_cpuid_policy,
- &pv_def_msr_policy,
- },
-#endif
-#ifdef CONFIG_HVM
- [ XEN_SYSCTL_cpu_policy_hvm_max ] = {
- &hvm_max_cpuid_policy,
- &hvm_max_msr_policy,
- },
- [ XEN_SYSCTL_cpu_policy_hvm_default ] = {
- &hvm_def_cpuid_policy,
- &hvm_def_msr_policy,
- },
-#endif
-};
+#include <asm/cpu-policy.h>
struct l3_cache_info {
int ret;
@@ -327,15 +296,15 @@ long arch_do_sysctl(
case XEN_SYSCTL_get_cpu_featureset:
{
static const struct cpuid_policy *const policy_table[6] = {
- [XEN_SYSCTL_cpu_featureset_raw] = &raw_cpuid_policy,
- [XEN_SYSCTL_cpu_featureset_host] = &host_cpuid_policy,
+ [XEN_SYSCTL_cpu_featureset_raw] = &raw_cpu_policy,
+ [XEN_SYSCTL_cpu_featureset_host] = &host_cpu_policy,
#ifdef CONFIG_PV
- [XEN_SYSCTL_cpu_featureset_pv] = &pv_def_cpuid_policy,
- [XEN_SYSCTL_cpu_featureset_pv_max] = &pv_max_cpuid_policy,
+ [XEN_SYSCTL_cpu_featureset_pv] = &pv_def_cpu_policy,
+ [XEN_SYSCTL_cpu_featureset_pv_max] = &pv_max_cpu_policy,
#endif
#ifdef CONFIG_HVM
- [XEN_SYSCTL_cpu_featureset_hvm] = &hvm_def_cpuid_policy,
- [XEN_SYSCTL_cpu_featureset_hvm_max] = &hvm_max_cpuid_policy,
+ [XEN_SYSCTL_cpu_featureset_hvm] = &hvm_def_cpu_policy,
+ [XEN_SYSCTL_cpu_featureset_hvm_max] = &hvm_max_cpu_policy,
#endif
};
const struct cpuid_policy *p = NULL;
@@ -391,7 +360,19 @@ long arch_do_sysctl(
case XEN_SYSCTL_get_cpu_policy:
{
- const struct old_cpu_policy *policy;
+ static const struct cpu_policy *system_policies[6] = {
+ [XEN_SYSCTL_cpu_policy_raw] = &raw_cpu_policy,
+ [XEN_SYSCTL_cpu_policy_host] = &host_cpu_policy,
+#ifdef CONFIG_PV
+ [XEN_SYSCTL_cpu_policy_pv_max] = &pv_max_cpu_policy,
+ [XEN_SYSCTL_cpu_policy_pv_default] = &pv_def_cpu_policy,
+#endif
+#ifdef CONFIG_HVM
+ [XEN_SYSCTL_cpu_policy_hvm_max] = &hvm_max_cpu_policy,
+ [XEN_SYSCTL_cpu_policy_hvm_default] = &hvm_def_cpu_policy,
+#endif
+ };
+ const struct cpu_policy *policy;
/* Reserved field set, or bad policy index? */
if ( sysctl->u.cpu_policy._rsvd ||
@@ -400,11 +381,11 @@ long arch_do_sysctl(
ret = -EINVAL;
break;
}
- policy = &system_policies[
+ policy = system_policies[
array_index_nospec(sysctl->u.cpu_policy.index,
ARRAY_SIZE(system_policies))];
- if ( !policy->cpuid || !policy->msr )
+ if ( !policy )
{
ret = -EOPNOTSUPP;
break;
@@ -414,7 +395,7 @@ long arch_do_sysctl(
if ( guest_handle_is_null(sysctl->u.cpu_policy.leaves) )
sysctl->u.cpu_policy.nr_leaves = CPUID_MAX_SERIALISED_LEAVES;
else if ( (ret = x86_cpuid_copy_to_buffer(
- policy->cpuid,
+ policy,
sysctl->u.cpu_policy.leaves,
&sysctl->u.cpu_policy.nr_leaves)) )
break;
@@ -430,7 +411,7 @@ long arch_do_sysctl(
if ( guest_handle_is_null(sysctl->u.cpu_policy.msrs) )
sysctl->u.cpu_policy.nr_msrs = MSR_MAX_SERIALISED_ENTRIES;
else if ( (ret = x86_msr_copy_to_buffer(
- policy->msr,
+ policy,
sysctl->u.cpu_policy.msrs,
&sysctl->u.cpu_policy.nr_msrs)) )
break;
--
2.30.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 6/9] x86: Merge a domain's {cpuid,msr} policy objects
2023-03-29 20:51 [PATCH RFC 0/9] x86: Merge cpuid and msr policy Andrew Cooper
` (4 preceding siblings ...)
2023-03-29 20:51 ` [PATCH 5/9] x86: Merge the system {cpuid,msr} policy objects Andrew Cooper
@ 2023-03-29 20:51 ` Andrew Cooper
2023-03-30 10:00 ` Jan Beulich
2023-03-29 20:51 ` [PATCH 7/9] x86: Merge xc_cpu_policy's cpuid and msr objects Andrew Cooper
` (4 subsequent siblings)
10 siblings, 1 reply; 34+ messages in thread
From: Andrew Cooper @ 2023-03-29 20:51 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné, Wei Liu
Right now, they're the same underlying type, containing disjoint information.
Drop the d->arch.msr pointer, and union d->arch.cpuid to give it a second name
of cpu_policy in the interim.
Merge init_domain_{cpuid,msr}_policy() into a single init_domain_cpu_policy(),
moving the implementation into cpu-policy.c
No practical change, but this does half the memory allocated for a policy
information per domain.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
---
xen/arch/x86/cpu-policy.c | 49 +++++++++++++++++++++++++
xen/arch/x86/cpuid.c | 23 ------------
xen/arch/x86/domain.c | 16 +++------
xen/arch/x86/domctl.c | 35 +++++++++---------
xen/arch/x86/include/asm/cpu-policy.h | 4 +++
xen/arch/x86/include/asm/cpuid.h | 3 --
xen/arch/x86/include/asm/domain.h | 7 ++--
xen/arch/x86/include/asm/msr.h | 1 -
xen/arch/x86/mm/mem_sharing.c | 3 +-
xen/arch/x86/msr.c | 52 ++-------------------------
10 files changed, 83 insertions(+), 110 deletions(-)
diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
index 663e9a084c53..e9ac1269c35a 100644
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -1,10 +1,13 @@
/* SPDX-License-Identifier: GPL-2.0-or-later */
#include <xen/cache.h>
#include <xen/kernel.h>
+#include <xen/sched.h>
#include <xen/lib/x86/cpu-policy.h>
#include <asm/cpu-policy.h>
+#include <asm/msr-index.h>
+#include <asm/setup.h>
struct cpu_policy __ro_after_init raw_cpu_policy;
struct cpu_policy __ro_after_init host_cpu_policy;
@@ -16,3 +19,49 @@ struct cpu_policy __ro_after_init pv_def_cpu_policy;
struct cpu_policy __ro_after_init hvm_max_cpu_policy;
struct cpu_policy __ro_after_init hvm_def_cpu_policy;
#endif
+
+int init_domain_cpu_policy(struct domain *d)
+{
+ struct cpu_policy *p = is_pv_domain(d)
+ ? (IS_ENABLED(CONFIG_PV) ? &pv_def_cpu_policy : NULL)
+ : (IS_ENABLED(CONFIG_HVM) ? &hvm_def_cpu_policy : NULL);
+
+ if ( !p )
+ {
+ ASSERT_UNREACHABLE();
+ return -EOPNOTSUPP;
+ }
+
+ p = xmemdup(p);
+ if ( !p )
+ return -ENOMEM;
+
+ /* See comment in ctxt_switch_levelling() */
+ if ( !opt_dom0_cpuid_faulting && is_control_domain(d) && is_pv_domain(d) )
+ p->platform_info.cpuid_faulting = false;
+
+ /*
+ * Expose the "hardware speculation behaviour" bits of ARCH_CAPS to dom0,
+ * so dom0 can turn off workarounds as appropriate. Temporary, until the
+ * domain policy logic gains a better understanding of MSRs.
+ */
+ if ( is_hardware_domain(d) && cpu_has_arch_caps )
+ {
+ uint64_t val;
+
+ rdmsrl(MSR_ARCH_CAPABILITIES, val);
+
+ p->arch_caps.raw = val &
+ (ARCH_CAPS_RDCL_NO | ARCH_CAPS_IBRS_ALL | ARCH_CAPS_RSBA |
+ ARCH_CAPS_SSB_NO | ARCH_CAPS_MDS_NO | ARCH_CAPS_IF_PSCHANGE_MC_NO |
+ ARCH_CAPS_TAA_NO | ARCH_CAPS_SBDR_SSDP_NO | ARCH_CAPS_FBSDP_NO |
+ ARCH_CAPS_PSDP_NO | ARCH_CAPS_FB_CLEAR | ARCH_CAPS_RRSBA |
+ ARCH_CAPS_BHI_NO | ARCH_CAPS_PBRSB_NO);
+ }
+
+ d->arch.cpu_policy = p;
+
+ recalculate_cpuid_policy(d);
+
+ return 0;
+}
diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index 0916bfe175c8..df3e503ced9d 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -784,29 +784,6 @@ void recalculate_cpuid_policy(struct domain *d)
p->extd.raw[0x19] = EMPTY_LEAF;
}
-int init_domain_cpuid_policy(struct domain *d)
-{
- struct cpuid_policy *p = is_pv_domain(d)
- ? (IS_ENABLED(CONFIG_PV) ? &pv_def_cpu_policy : NULL)
- : (IS_ENABLED(CONFIG_HVM) ? &hvm_def_cpu_policy : NULL);
-
- if ( !p )
- {
- ASSERT_UNREACHABLE();
- return -EOPNOTSUPP;
- }
-
- p = xmemdup(p);
- if ( !p )
- return -ENOMEM;
-
- d->arch.cpuid = p;
-
- recalculate_cpuid_policy(d);
-
- return 0;
-}
-
void __init init_dom0_cpuid_policy(struct domain *d)
{
struct cpuid_policy *p = d->arch.cpuid;
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index d5847f70f890..7cf66aee042c 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -66,6 +66,7 @@
#ifdef CONFIG_COMPAT
#include <compat/vcpu.h>
#endif
+#include <asm/cpu-policy.h>
#include <asm/psr.h>
#include <asm/pv/domain.h>
#include <asm/pv/mm.h>
@@ -573,7 +574,6 @@ int arch_vcpu_create(struct vcpu *v)
/* Idle domain */
v->arch.cr3 = __pa(idle_pg_table);
rc = 0;
- v->arch.msrs = ZERO_BLOCK_PTR; /* Catch stray misuses */
}
if ( rc )
@@ -743,8 +743,7 @@ int arch_domain_create(struct domain *d,
d->arch.ctxt_switch = &idle_csw;
- d->arch.cpuid = ZERO_BLOCK_PTR; /* Catch stray misuses. */
- d->arch.msr = ZERO_BLOCK_PTR;
+ d->arch.cpu_policy = ZERO_BLOCK_PTR; /* Catch stray misuses. */
return 0;
}
@@ -799,10 +798,7 @@ int arch_domain_create(struct domain *d,
goto fail;
paging_initialised = true;
- if ( (rc = init_domain_cpuid_policy(d)) )
- goto fail;
-
- if ( (rc = init_domain_msr_policy(d)) )
+ if ( (rc = init_domain_cpu_policy(d)) )
goto fail;
d->arch.ioport_caps =
@@ -873,8 +869,7 @@ int arch_domain_create(struct domain *d,
iommu_domain_destroy(d);
cleanup_domain_irq_mapping(d);
free_xenheap_page(d->shared_info);
- xfree(d->arch.cpuid);
- xfree(d->arch.msr);
+ XFREE(d->arch.cpu_policy);
if ( paging_initialised )
paging_final_teardown(d);
free_perdomain_mappings(d);
@@ -888,8 +883,7 @@ void arch_domain_destroy(struct domain *d)
hvm_domain_destroy(d);
xfree(d->arch.e820);
- xfree(d->arch.cpuid);
- xfree(d->arch.msr);
+ XFREE(d->arch.cpu_policy);
free_domain_pirqs(d);
if ( !is_idle_domain(d) )
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 5800bb10bc4a..81be25c67731 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -40,11 +40,11 @@
static int update_domain_cpu_policy(struct domain *d,
xen_domctl_cpu_policy_t *xdpc)
{
- struct old_cpu_policy new = {};
+ struct cpu_policy *new;
struct cpu_policy *sys = is_pv_domain(d)
? (IS_ENABLED(CONFIG_PV) ? &pv_max_cpu_policy : NULL)
: (IS_ENABLED(CONFIG_HVM) ? &hvm_max_cpu_policy : NULL);
- struct old_cpu_policy old_sys = { sys, sys };
+ struct old_cpu_policy old_sys = { sys, sys }, old_new;
struct cpu_policy_errors err = INIT_CPU_POLICY_ERRORS;
int ret = -ENOMEM;
@@ -54,33 +54,33 @@ static int update_domain_cpu_policy(struct domain *d,
return -EOPNOTSUPP;
}
- /* Start by copying the domain's existing policies. */
- if ( !(new.cpuid = xmemdup(d->arch.cpuid)) ||
- !(new.msr = xmemdup(d->arch.msr)) )
+ /* Start by copying the domain's existing policy. */
+ if ( !(new = xmemdup(d->arch.cpu_policy)) )
goto out;
+ old_new = (struct old_cpu_policy){ new, new };
+
/* Merge the toolstack provided data. */
if ( (ret = x86_cpuid_copy_from_buffer(
- new.cpuid, xdpc->leaves, xdpc->nr_leaves,
+ new, xdpc->leaves, xdpc->nr_leaves,
&err.leaf, &err.subleaf)) ||
(ret = x86_msr_copy_from_buffer(
- new.msr, xdpc->msrs, xdpc->nr_msrs, &err.msr)) )
+ new, xdpc->msrs, xdpc->nr_msrs, &err.msr)) )
goto out;
/* Trim any newly-stale out-of-range leaves. */
- x86_cpuid_policy_clear_out_of_range_leaves(new.cpuid);
+ x86_cpuid_policy_clear_out_of_range_leaves(new);
/* Audit the combined dataset. */
- ret = x86_cpu_policies_are_compatible(&old_sys, &new, &err);
+ ret = x86_cpu_policies_are_compatible(&old_sys, &old_new, &err);
if ( ret )
goto out;
/*
- * Audit was successful. Replace existing policies, leaving the old
- * policies to be freed.
+ * Audit was successful. Replace the existing policy, leaving the old one
+ * to be freed.
*/
- SWAP(new.cpuid, d->arch.cpuid);
- SWAP(new.msr, d->arch.msr);
+ SWAP(new, d->arch.cpu_policy);
/* TODO: Drop when x86_cpu_policies_are_compatible() is completed. */
recalculate_cpuid_policy(d);
@@ -89,9 +89,8 @@ static int update_domain_cpu_policy(struct domain *d,
domain_cpu_policy_changed(d);
out:
- /* Free whichever cpuid/msr structs are not installed in struct domain. */
- xfree(new.cpuid);
- xfree(new.msr);
+ /* Free whichever struct is not installed in struct domain. */
+ xfree(new);
if ( ret )
{
@@ -1327,7 +1326,7 @@ long arch_do_domctl(
if ( guest_handle_is_null(domctl->u.cpu_policy.leaves) )
domctl->u.cpu_policy.nr_leaves = CPUID_MAX_SERIALISED_LEAVES;
else if ( (ret = x86_cpuid_copy_to_buffer(
- d->arch.cpuid,
+ d->arch.cpu_policy,
domctl->u.cpu_policy.leaves,
&domctl->u.cpu_policy.nr_leaves)) )
break;
@@ -1336,7 +1335,7 @@ long arch_do_domctl(
if ( guest_handle_is_null(domctl->u.cpu_policy.msrs) )
domctl->u.cpu_policy.nr_msrs = MSR_MAX_SERIALISED_ENTRIES;
else if ( (ret = x86_msr_copy_to_buffer(
- d->arch.msr,
+ d->arch.cpu_policy,
domctl->u.cpu_policy.msrs,
&domctl->u.cpu_policy.nr_msrs)) )
break;
diff --git a/xen/arch/x86/include/asm/cpu-policy.h b/xen/arch/x86/include/asm/cpu-policy.h
index eef14bb4267e..9ba34bbf5ea1 100644
--- a/xen/arch/x86/include/asm/cpu-policy.h
+++ b/xen/arch/x86/include/asm/cpu-policy.h
@@ -3,6 +3,7 @@
#define X86_CPU_POLICY_H
struct cpu_policy;
+struct domain;
extern struct cpu_policy raw_cpu_policy;
extern struct cpu_policy host_cpu_policy;
@@ -11,4 +12,7 @@ extern struct cpu_policy pv_def_cpu_policy;
extern struct cpu_policy hvm_max_cpu_policy;
extern struct cpu_policy hvm_def_cpu_policy;
+/* Allocate and initialise a CPU policy suitable for the domain. */
+int init_domain_cpu_policy(struct domain *d);
+
#endif /* X86_CPU_POLICY_H */
diff --git a/xen/arch/x86/include/asm/cpuid.h b/xen/arch/x86/include/asm/cpuid.h
index ea0586277331..7f81b998ce01 100644
--- a/xen/arch/x86/include/asm/cpuid.h
+++ b/xen/arch/x86/include/asm/cpuid.h
@@ -49,9 +49,6 @@ extern struct cpuidmasks cpuidmask_defaults;
/* Check that all previously present features are still available. */
bool recheck_cpu_features(unsigned int cpu);
-/* Allocate and initialise a CPUID policy suitable for the domain. */
-int init_domain_cpuid_policy(struct domain *d);
-
/* Apply dom0-specific tweaks to the CPUID policy. */
void init_dom0_cpuid_policy(struct domain *d);
diff --git a/xen/arch/x86/include/asm/domain.h b/xen/arch/x86/include/asm/domain.h
index 17780ad9db2f..a1deab1d0dcc 100644
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -386,9 +386,10 @@ struct arch_domain
*/
uint8_t x87_fip_width;
- /* CPUID and MSR policy objects. */
- struct cpuid_policy *cpuid;
- struct msr_policy *msr;
+ union {
+ struct cpu_policy *cpuid; /* Temporary */
+ struct cpu_policy *cpu_policy;
+ };
struct PITState vpit;
diff --git a/xen/arch/x86/include/asm/msr.h b/xen/arch/x86/include/asm/msr.h
index 022230acc0af..b59a51d238a7 100644
--- a/xen/arch/x86/include/asm/msr.h
+++ b/xen/arch/x86/include/asm/msr.h
@@ -419,7 +419,6 @@ struct vcpu_msrs
};
void init_guest_msr_policy(void);
-int init_domain_msr_policy(struct domain *d);
int init_vcpu_msr_policy(struct vcpu *v);
/*
diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index 649d93dc5444..5b3449db7a11 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -1902,8 +1902,7 @@ static int fork(struct domain *cd, struct domain *d)
domain_pause(d);
cd->max_pages = d->max_pages;
- *cd->arch.cpuid = *d->arch.cpuid;
- *cd->arch.msr = *d->arch.msr;
+ *cd->arch.cpu_policy = *d->arch.cpu_policy;
cd->vmtrace_size = d->vmtrace_size;
cd->parent = d;
}
diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index bff26bc4e2b5..672961dd3ac1 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -122,50 +122,6 @@ void __init init_guest_msr_policy(void)
}
}
-int init_domain_msr_policy(struct domain *d)
-{
- struct msr_policy *mp = is_pv_domain(d)
- ? (IS_ENABLED(CONFIG_PV) ? &pv_def_cpu_policy : NULL)
- : (IS_ENABLED(CONFIG_HVM) ? &hvm_def_cpu_policy : NULL);
-
- if ( !mp )
- {
- ASSERT_UNREACHABLE();
- return -EOPNOTSUPP;
- }
-
- mp = xmemdup(mp);
- if ( !mp )
- return -ENOMEM;
-
- /* See comment in ctxt_switch_levelling() */
- if ( !opt_dom0_cpuid_faulting && is_control_domain(d) && is_pv_domain(d) )
- mp->platform_info.cpuid_faulting = false;
-
- /*
- * Expose the "hardware speculation behaviour" bits of ARCH_CAPS to dom0,
- * so dom0 can turn off workarounds as appropriate. Temporary, until the
- * domain policy logic gains a better understanding of MSRs.
- */
- if ( is_hardware_domain(d) && cpu_has_arch_caps )
- {
- uint64_t val;
-
- rdmsrl(MSR_ARCH_CAPABILITIES, val);
-
- mp->arch_caps.raw = val &
- (ARCH_CAPS_RDCL_NO | ARCH_CAPS_IBRS_ALL | ARCH_CAPS_RSBA |
- ARCH_CAPS_SSB_NO | ARCH_CAPS_MDS_NO | ARCH_CAPS_IF_PSCHANGE_MC_NO |
- ARCH_CAPS_TAA_NO | ARCH_CAPS_SBDR_SSDP_NO | ARCH_CAPS_FBSDP_NO |
- ARCH_CAPS_PSDP_NO | ARCH_CAPS_FB_CLEAR | ARCH_CAPS_RRSBA |
- ARCH_CAPS_BHI_NO | ARCH_CAPS_PBRSB_NO);
- }
-
- d->arch.msr = mp;
-
- return 0;
-}
-
int init_vcpu_msr_policy(struct vcpu *v)
{
struct vcpu_msrs *msrs = xzalloc(struct vcpu_msrs);
@@ -183,7 +139,6 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
const struct vcpu *curr = current;
const struct domain *d = v->domain;
const struct cpuid_policy *cp = d->arch.cpuid;
- const struct msr_policy *mp = d->arch.msr;
const struct vcpu_msrs *msrs = v->arch.msrs;
int ret = X86EMUL_OKAY;
@@ -267,13 +222,13 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
goto get_reg;
case MSR_INTEL_PLATFORM_INFO:
- *val = mp->platform_info.raw;
+ *val = cp->platform_info.raw;
break;
case MSR_ARCH_CAPABILITIES:
if ( !cp->feat.arch_caps )
goto gp_fault;
- *val = mp->arch_caps.raw;
+ *val = cp->arch_caps.raw;
break;
case MSR_INTEL_MISC_FEATURES_ENABLES:
@@ -474,7 +429,6 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
const struct vcpu *curr = current;
struct domain *d = v->domain;
const struct cpuid_policy *cp = d->arch.cpuid;
- const struct msr_policy *mp = d->arch.msr;
struct vcpu_msrs *msrs = v->arch.msrs;
int ret = X86EMUL_OKAY;
@@ -574,7 +528,7 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
bool old_cpuid_faulting = msrs->misc_features_enables.cpuid_faulting;
rsvd = ~0ull;
- if ( mp->platform_info.cpuid_faulting )
+ if ( cp->platform_info.cpuid_faulting )
rsvd &= ~MSR_MISC_FEATURES_CPUID_FAULTING;
if ( val & rsvd )
--
2.30.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 7/9] x86: Merge xc_cpu_policy's cpuid and msr objects
2023-03-29 20:51 [PATCH RFC 0/9] x86: Merge cpuid and msr policy Andrew Cooper
` (5 preceding siblings ...)
2023-03-29 20:51 ` [PATCH 6/9] x86: Merge a domain's " Andrew Cooper
@ 2023-03-29 20:51 ` Andrew Cooper
2023-03-30 10:04 ` Jan Beulich
2023-03-29 20:51 ` [PATCH 8/9] x86: Drop struct old_cpu_policy Andrew Cooper
` (3 subsequent siblings)
10 siblings, 1 reply; 34+ messages in thread
From: Andrew Cooper @ 2023-03-29 20:51 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné, Wei Liu
Right now, they're the same underlying type, containing disjoint information.
Use a single object instead. Also take the opportunity to rename 'entries' to
'msrs' which is more descriptive, and more in line with nr_msrs being the
count of MSR entries in the API.
test-tsx uses xg_private.h to access the internals of xc_cpu_policy, so needs
updating at the same time.
No practical change, but it does reduce the size of struct xc_cpu_policy by
~2k.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
---
tools/libs/guest/xg_cpuid_x86.c | 36 ++++++++++----------
tools/libs/guest/xg_private.h | 5 ++-
tools/tests/tsx/test-tsx.c | 58 ++++++++++++++++-----------------
3 files changed, 48 insertions(+), 51 deletions(-)
diff --git a/tools/libs/guest/xg_cpuid_x86.c b/tools/libs/guest/xg_cpuid_x86.c
index 5fae06e77804..5061fe357767 100644
--- a/tools/libs/guest/xg_cpuid_x86.c
+++ b/tools/libs/guest/xg_cpuid_x86.c
@@ -431,7 +431,7 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid, bool restore,
xc_dominfo_t di;
unsigned int i, nr_leaves, nr_msrs;
xen_cpuid_leaf_t *leaves = NULL;
- struct cpuid_policy *p = NULL;
+ struct cpu_policy *p = NULL;
uint32_t err_leaf = -1, err_subleaf = -1, err_msr = -1;
uint32_t host_featureset[FEATURESET_NR_ENTRIES] = {};
uint32_t len = ARRAY_SIZE(host_featureset);
@@ -692,7 +692,7 @@ static int deserialize_policy(xc_interface *xch, xc_cpu_policy_t *policy,
uint32_t err_leaf = -1, err_subleaf = -1, err_msr = -1;
int rc;
- rc = x86_cpuid_copy_from_buffer(&policy->cpuid, policy->leaves,
+ rc = x86_cpuid_copy_from_buffer(&policy->policy, policy->leaves,
nr_leaves, &err_leaf, &err_subleaf);
if ( rc )
{
@@ -702,7 +702,7 @@ static int deserialize_policy(xc_interface *xch, xc_cpu_policy_t *policy,
return rc;
}
- rc = x86_msr_copy_from_buffer(&policy->msr, policy->entries,
+ rc = x86_msr_copy_from_buffer(&policy->policy, policy->msrs,
nr_entries, &err_msr);
if ( rc )
{
@@ -719,18 +719,18 @@ int xc_cpu_policy_get_system(xc_interface *xch, unsigned int policy_idx,
xc_cpu_policy_t *policy)
{
unsigned int nr_leaves = ARRAY_SIZE(policy->leaves);
- unsigned int nr_entries = ARRAY_SIZE(policy->entries);
+ unsigned int nr_msrs = ARRAY_SIZE(policy->msrs);
int rc;
rc = get_system_cpu_policy(xch, policy_idx, &nr_leaves, policy->leaves,
- &nr_entries, policy->entries);
+ &nr_msrs, policy->msrs);
if ( rc )
{
PERROR("Failed to obtain %u policy", policy_idx);
return rc;
}
- rc = deserialize_policy(xch, policy, nr_leaves, nr_entries);
+ rc = deserialize_policy(xch, policy, nr_leaves, nr_msrs);
if ( rc )
{
errno = -rc;
@@ -744,18 +744,18 @@ int xc_cpu_policy_get_domain(xc_interface *xch, uint32_t domid,
xc_cpu_policy_t *policy)
{
unsigned int nr_leaves = ARRAY_SIZE(policy->leaves);
- unsigned int nr_entries = ARRAY_SIZE(policy->entries);
+ unsigned int nr_msrs = ARRAY_SIZE(policy->msrs);
int rc;
rc = get_domain_cpu_policy(xch, domid, &nr_leaves, policy->leaves,
- &nr_entries, policy->entries);
+ &nr_msrs, policy->msrs);
if ( rc )
{
PERROR("Failed to obtain domain %u policy", domid);
return rc;
}
- rc = deserialize_policy(xch, policy, nr_leaves, nr_entries);
+ rc = deserialize_policy(xch, policy, nr_leaves, nr_msrs);
if ( rc )
{
errno = -rc;
@@ -770,16 +770,16 @@ int xc_cpu_policy_set_domain(xc_interface *xch, uint32_t domid,
{
uint32_t err_leaf = -1, err_subleaf = -1, err_msr = -1;
unsigned int nr_leaves = ARRAY_SIZE(policy->leaves);
- unsigned int nr_entries = ARRAY_SIZE(policy->entries);
+ unsigned int nr_msrs = ARRAY_SIZE(policy->msrs);
int rc;
rc = xc_cpu_policy_serialise(xch, policy, policy->leaves, &nr_leaves,
- policy->entries, &nr_entries);
+ policy->msrs, &nr_msrs);
if ( rc )
return rc;
rc = xc_set_domain_cpu_policy(xch, domid, nr_leaves, policy->leaves,
- nr_entries, policy->entries,
+ nr_msrs, policy->msrs,
&err_leaf, &err_subleaf, &err_msr);
if ( rc )
{
@@ -802,7 +802,7 @@ int xc_cpu_policy_serialise(xc_interface *xch, const xc_cpu_policy_t *p,
if ( leaves )
{
- rc = x86_cpuid_copy_to_buffer(&p->cpuid, leaves, nr_leaves);
+ rc = x86_cpuid_copy_to_buffer(&p->policy, leaves, nr_leaves);
if ( rc )
{
ERROR("Failed to serialize CPUID policy");
@@ -813,7 +813,7 @@ int xc_cpu_policy_serialise(xc_interface *xch, const xc_cpu_policy_t *p,
if ( msrs )
{
- rc = x86_msr_copy_to_buffer(&p->msr, msrs, nr_msrs);
+ rc = x86_msr_copy_to_buffer(&p->policy, msrs, nr_msrs);
if ( rc )
{
ERROR("Failed to serialize MSR policy");
@@ -831,7 +831,7 @@ int xc_cpu_policy_update_cpuid(xc_interface *xch, xc_cpu_policy_t *policy,
uint32_t nr)
{
unsigned int err_leaf = -1, err_subleaf = -1;
- int rc = x86_cpuid_copy_from_buffer(&policy->cpuid, leaves, nr,
+ int rc = x86_cpuid_copy_from_buffer(&policy->policy, leaves, nr,
&err_leaf, &err_subleaf);
if ( rc )
@@ -850,7 +850,7 @@ int xc_cpu_policy_update_msrs(xc_interface *xch, xc_cpu_policy_t *policy,
const xen_msr_entry_t *msrs, uint32_t nr)
{
unsigned int err_msr = -1;
- int rc = x86_msr_copy_from_buffer(&policy->msr, msrs, nr, &err_msr);
+ int rc = x86_msr_copy_from_buffer(&policy->policy, msrs, nr, &err_msr);
if ( rc )
{
@@ -868,8 +868,8 @@ bool xc_cpu_policy_is_compatible(xc_interface *xch, xc_cpu_policy_t *host,
xc_cpu_policy_t *guest)
{
struct cpu_policy_errors err = INIT_CPU_POLICY_ERRORS;
- struct old_cpu_policy h = { &host->cpuid, &host->msr };
- struct old_cpu_policy g = { &guest->cpuid, &guest->msr };
+ struct old_cpu_policy h = { &host->policy, &host->policy };
+ struct old_cpu_policy g = { &guest->policy, &guest->policy };
int rc = x86_cpu_policies_are_compatible(&h, &g, &err);
if ( !rc )
diff --git a/tools/libs/guest/xg_private.h b/tools/libs/guest/xg_private.h
index 09e24f122760..e729a8106c3e 100644
--- a/tools/libs/guest/xg_private.h
+++ b/tools/libs/guest/xg_private.h
@@ -173,10 +173,9 @@ int pin_table(xc_interface *xch, unsigned int type, unsigned long mfn,
#include <xen/lib/x86/cpu-policy.h>
struct xc_cpu_policy {
- struct cpuid_policy cpuid;
- struct msr_policy msr;
+ struct cpu_policy policy;
xen_cpuid_leaf_t leaves[CPUID_MAX_SERIALISED_LEAVES];
- xen_msr_entry_t entries[MSR_MAX_SERIALISED_ENTRIES];
+ xen_msr_entry_t msrs[MSR_MAX_SERIALISED_ENTRIES];
};
#endif /* x86 */
diff --git a/tools/tests/tsx/test-tsx.c b/tools/tests/tsx/test-tsx.c
index d6d98c299bf9..6164cd86c466 100644
--- a/tools/tests/tsx/test-tsx.c
+++ b/tools/tests/tsx/test-tsx.c
@@ -151,15 +151,15 @@ static void test_tsx_msrs(void)
{
printf("Testing MSR_TSX_FORCE_ABORT consistency\n");
test_tsx_msr_consistency(
- MSR_TSX_FORCE_ABORT, host.cpuid.feat.tsx_force_abort);
+ MSR_TSX_FORCE_ABORT, host.policy.feat.tsx_force_abort);
printf("Testing MSR_TSX_CTRL consistency\n");
test_tsx_msr_consistency(
- MSR_TSX_CTRL, host.msr.arch_caps.tsx_ctrl);
+ MSR_TSX_CTRL, host.policy.arch_caps.tsx_ctrl);
printf("Testing MSR_MCU_OPT_CTRL consistency\n");
test_tsx_msr_consistency(
- MSR_MCU_OPT_CTRL, host.cpuid.feat.srbds_ctrl);
+ MSR_MCU_OPT_CTRL, host.policy.feat.srbds_ctrl);
}
/*
@@ -281,7 +281,7 @@ static void test_rtm_behaviour(void)
else
return fail(" Got unexpected behaviour %d\n", rtm_behaviour);
- if ( host.cpuid.feat.rtm )
+ if ( host.policy.feat.rtm )
{
if ( rtm_behaviour == RTM_UD )
fail(" Host reports RTM, but appears unavailable\n");
@@ -297,53 +297,51 @@ static void dump_tsx_details(const struct xc_cpu_policy *p, const char *pref)
{
printf(" %s RTM %u, HLE %u, TSX_FORCE_ABORT %u, RTM_ALWAYS_ABORT %u, TSX_CTRL %u\n",
pref,
- p->cpuid.feat.rtm,
- p->cpuid.feat.hle,
- p->cpuid.feat.tsx_force_abort,
- p->cpuid.feat.rtm_always_abort,
- p->msr.arch_caps.tsx_ctrl);
+ p->policy.feat.rtm,
+ p->policy.feat.hle,
+ p->policy.feat.tsx_force_abort,
+ p->policy.feat.rtm_always_abort,
+ p->policy.arch_caps.tsx_ctrl);
}
/* Sanity test various invariants we expect in the default/max policies. */
static void test_guest_policies(const struct xc_cpu_policy *max,
const struct xc_cpu_policy *def)
{
- const struct cpuid_policy *cm = &max->cpuid;
- const struct cpuid_policy *cd = &def->cpuid;
- const struct msr_policy *mm = &max->msr;
- const struct msr_policy *md = &def->msr;
+ const struct cpu_policy *m = &max->policy;
+ const struct cpu_policy *d = &def->policy;
dump_tsx_details(max, "Max:");
dump_tsx_details(def, "Def:");
- if ( ((cm->feat.raw[0].d | cd->feat.raw[0].d) &
+ if ( ((m->feat.raw[0].d | d->feat.raw[0].d) &
(bitmaskof(X86_FEATURE_TSX_FORCE_ABORT) |
bitmaskof(X86_FEATURE_RTM_ALWAYS_ABORT) |
bitmaskof(X86_FEATURE_SRBDS_CTRL))) ||
- ((mm->arch_caps.raw | md->arch_caps.raw) & ARCH_CAPS_TSX_CTRL) )
+ ((m->arch_caps.raw | d->arch_caps.raw) & ARCH_CAPS_TSX_CTRL) )
fail(" Xen-only TSX controls offered to guest\n");
switch ( rtm_behaviour )
{
case RTM_UD:
- if ( (cm->feat.raw[0].b | cd->feat.raw[0].b) &
+ if ( (m->feat.raw[0].b | d->feat.raw[0].b) &
(bitmaskof(X86_FEATURE_HLE) | bitmaskof(X86_FEATURE_RTM)) )
fail(" HLE/RTM offered to guests despite not being available\n");
break;
case RTM_ABORT:
- if ( cd->feat.raw[0].b &
+ if ( d->feat.raw[0].b &
(bitmaskof(X86_FEATURE_HLE) | bitmaskof(X86_FEATURE_RTM)) )
fail(" HLE/RTM offered to guests by default despite not being usable\n");
break;
case RTM_OK:
- if ( !cm->feat.rtm || !cd->feat.rtm )
+ if ( !m->feat.rtm || !d->feat.rtm )
fail(" RTM not offered to guests despite being available\n");
break;
}
- if ( cd->feat.hle )
+ if ( d->feat.hle )
fail(" Fail: HLE offered in default policy\n");
}
@@ -387,18 +385,18 @@ static void test_guest(struct xen_domctl_createdomain *c)
/*
* Check defaults given to the guest.
*/
- if ( guest_policy.cpuid.feat.rtm != (rtm_behaviour == RTM_OK) )
+ if ( guest_policy.policy.feat.rtm != (rtm_behaviour == RTM_OK) )
fail(" RTM %u in guest, despite rtm behaviour\n",
- guest_policy.cpuid.feat.rtm);
+ guest_policy.policy.feat.rtm);
- if ( guest_policy.cpuid.feat.hle ||
- guest_policy.cpuid.feat.tsx_force_abort ||
- guest_policy.cpuid.feat.rtm_always_abort ||
- guest_policy.cpuid.feat.srbds_ctrl ||
- guest_policy.msr.arch_caps.tsx_ctrl )
+ if ( guest_policy.policy.feat.hle ||
+ guest_policy.policy.feat.tsx_force_abort ||
+ guest_policy.policy.feat.rtm_always_abort ||
+ guest_policy.policy.feat.srbds_ctrl ||
+ guest_policy.policy.arch_caps.tsx_ctrl )
fail(" Unexpected features advertised\n");
- if ( host.cpuid.feat.rtm )
+ if ( host.policy.feat.rtm )
{
unsigned int _7b0;
@@ -406,7 +404,7 @@ static void test_guest(struct xen_domctl_createdomain *c)
* If host RTM is available, all combinations of guest flags should be
* possible. Flip both HLE/RTM to check non-default settings.
*/
- _7b0 = (guest_policy.cpuid.feat.raw[0].b ^=
+ _7b0 = (guest_policy.policy.feat.raw[0].b ^=
(bitmaskof(X86_FEATURE_HLE) | bitmaskof(X86_FEATURE_RTM)));
/* Set the new policy. */
@@ -429,10 +427,10 @@ static void test_guest(struct xen_domctl_createdomain *c)
dump_tsx_details(&guest_policy, "Cur:");
- if ( guest_policy.cpuid.feat.raw[0].b != _7b0 )
+ if ( guest_policy.policy.feat.raw[0].b != _7b0 )
{
fail(" Expected CPUID.7[1].b 0x%08x differs from actual 0x%08x\n",
- _7b0, guest_policy.cpuid.feat.raw[0].b);
+ _7b0, guest_policy.policy.feat.raw[0].b);
goto out;
}
}
--
2.30.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 8/9] x86: Drop struct old_cpu_policy
2023-03-29 20:51 [PATCH RFC 0/9] x86: Merge cpuid and msr policy Andrew Cooper
` (6 preceding siblings ...)
2023-03-29 20:51 ` [PATCH 7/9] x86: Merge xc_cpu_policy's cpuid and msr objects Andrew Cooper
@ 2023-03-29 20:51 ` Andrew Cooper
2023-03-30 10:08 ` Jan Beulich
2023-03-29 20:51 ` [PATCH 9/9] RFC: Everything else Andrew Cooper
` (2 subsequent siblings)
10 siblings, 1 reply; 34+ messages in thread
From: Andrew Cooper @ 2023-03-29 20:51 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné, Wei Liu
With all the complicated callers of x86_cpu_policies_are_compatible() updated
to use a single cpu_policy object, we can drop the final user of struct
old_cpu_policy.
Update x86_cpu_policies_are_compatible() to take (new) cpu_policy pointers,
reducing the amount of internal pointer chasing, and update all callers to
pass their cpu_policy objects directly.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
---
tools/libs/guest/xg_cpuid_x86.c | 4 +--
tools/tests/cpu-policy/test-cpu-policy.c | 40 ++++++------------------
xen/arch/x86/domctl.c | 7 ++---
xen/include/xen/lib/x86/cpu-policy.h | 12 ++-----
xen/lib/x86/policy.c | 12 +++----
5 files changed, 22 insertions(+), 53 deletions(-)
diff --git a/tools/libs/guest/xg_cpuid_x86.c b/tools/libs/guest/xg_cpuid_x86.c
index 5061fe357767..14c00304c03c 100644
--- a/tools/libs/guest/xg_cpuid_x86.c
+++ b/tools/libs/guest/xg_cpuid_x86.c
@@ -868,9 +868,7 @@ bool xc_cpu_policy_is_compatible(xc_interface *xch, xc_cpu_policy_t *host,
xc_cpu_policy_t *guest)
{
struct cpu_policy_errors err = INIT_CPU_POLICY_ERRORS;
- struct old_cpu_policy h = { &host->policy, &host->policy };
- struct old_cpu_policy g = { &guest->policy, &guest->policy };
- int rc = x86_cpu_policies_are_compatible(&h, &g, &err);
+ int rc = x86_cpu_policies_are_compatible(&host->policy, &host->policy, &err);
if ( !rc )
return true;
diff --git a/tools/tests/cpu-policy/test-cpu-policy.c b/tools/tests/cpu-policy/test-cpu-policy.c
index 909d6272f875..5b38702b1c62 100644
--- a/tools/tests/cpu-policy/test-cpu-policy.c
+++ b/tools/tests/cpu-policy/test-cpu-policy.c
@@ -574,23 +574,20 @@ static void test_is_compatible_success(void)
{
static struct test {
const char *name;
- struct cpuid_policy host_cpuid;
- struct cpuid_policy guest_cpuid;
- struct msr_policy host_msr;
- struct msr_policy guest_msr;
+ struct cpu_policy host, guest;
} tests[] = {
{
.name = "Host CPUID faulting, Guest not",
- .host_msr = {
+ .host = {
.platform_info.cpuid_faulting = true,
},
},
{
.name = "Host CPUID faulting, Guest wanted",
- .host_msr = {
+ .host = {
.platform_info.cpuid_faulting = true,
},
- .guest_msr = {
+ .guest = {
.platform_info.cpuid_faulting = true,
},
},
@@ -602,15 +599,8 @@ static void test_is_compatible_success(void)
for ( size_t i = 0; i < ARRAY_SIZE(tests); ++i )
{
struct test *t = &tests[i];
- struct old_cpu_policy sys = {
- &t->host_cpuid,
- &t->host_msr,
- }, new = {
- &t->guest_cpuid,
- &t->guest_msr,
- };
struct cpu_policy_errors e;
- int res = x86_cpu_policies_are_compatible(&sys, &new, &e);
+ int res = x86_cpu_policies_are_compatible(&t->host, &t->guest, &e);
/* Check the expected error output. */
if ( res != 0 || memcmp(&no_errors, &e, sizeof(no_errors)) )
@@ -624,25 +614,22 @@ static void test_is_compatible_failure(void)
{
static struct test {
const char *name;
- struct cpuid_policy host_cpuid;
- struct cpuid_policy guest_cpuid;
- struct msr_policy host_msr;
- struct msr_policy guest_msr;
+ struct cpu_policy host, guest;
struct cpu_policy_errors e;
} tests[] = {
{
.name = "Host basic.max_leaf out of range",
- .guest_cpuid.basic.max_leaf = 1,
+ .guest.basic.max_leaf = 1,
.e = { 0, -1, -1 },
},
{
.name = "Host extd.max_leaf out of range",
- .guest_cpuid.extd.max_leaf = 1,
+ .guest.extd.max_leaf = 1,
.e = { 0x80000000, -1, -1 },
},
{
.name = "Host no CPUID faulting, Guest wanted",
- .guest_msr = {
+ .guest = {
.platform_info.cpuid_faulting = true,
},
.e = { -1, -1, 0xce },
@@ -654,15 +641,8 @@ static void test_is_compatible_failure(void)
for ( size_t i = 0; i < ARRAY_SIZE(tests); ++i )
{
struct test *t = &tests[i];
- struct old_cpu_policy sys = {
- &t->host_cpuid,
- &t->host_msr,
- }, new = {
- &t->guest_cpuid,
- &t->guest_msr,
- };
struct cpu_policy_errors e;
- int res = x86_cpu_policies_are_compatible(&sys, &new, &e);
+ int res = x86_cpu_policies_are_compatible(&t->host, &t->guest, &e);
/* Check the expected error output. */
if ( res == 0 || memcmp(&t->e, &e, sizeof(t->e)) )
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 81be25c67731..c02528594102 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -41,10 +41,9 @@ static int update_domain_cpu_policy(struct domain *d,
xen_domctl_cpu_policy_t *xdpc)
{
struct cpu_policy *new;
- struct cpu_policy *sys = is_pv_domain(d)
+ const struct cpu_policy *sys = is_pv_domain(d)
? (IS_ENABLED(CONFIG_PV) ? &pv_max_cpu_policy : NULL)
: (IS_ENABLED(CONFIG_HVM) ? &hvm_max_cpu_policy : NULL);
- struct old_cpu_policy old_sys = { sys, sys }, old_new;
struct cpu_policy_errors err = INIT_CPU_POLICY_ERRORS;
int ret = -ENOMEM;
@@ -58,8 +57,6 @@ static int update_domain_cpu_policy(struct domain *d,
if ( !(new = xmemdup(d->arch.cpu_policy)) )
goto out;
- old_new = (struct old_cpu_policy){ new, new };
-
/* Merge the toolstack provided data. */
if ( (ret = x86_cpuid_copy_from_buffer(
new, xdpc->leaves, xdpc->nr_leaves,
@@ -72,7 +69,7 @@ static int update_domain_cpu_policy(struct domain *d,
x86_cpuid_policy_clear_out_of_range_leaves(new);
/* Audit the combined dataset. */
- ret = x86_cpu_policies_are_compatible(&old_sys, &old_new, &err);
+ ret = x86_cpu_policies_are_compatible(sys, new, &err);
if ( ret )
goto out;
diff --git a/xen/include/xen/lib/x86/cpu-policy.h b/xen/include/xen/lib/x86/cpu-policy.h
index 5af756a02da0..51f88f1e217e 100644
--- a/xen/include/xen/lib/x86/cpu-policy.h
+++ b/xen/include/xen/lib/x86/cpu-policy.h
@@ -379,12 +379,6 @@ struct cpu_policy
#define cpuid_policy cpu_policy
#define msr_policy cpu_policy
-struct old_cpu_policy
-{
- struct cpuid_policy *cpuid;
- struct msr_policy *msr;
-};
-
struct cpu_policy_errors
{
uint32_t leaf, subleaf;
@@ -559,7 +553,7 @@ int x86_msr_copy_from_buffer(struct msr_policy *policy,
const msr_entry_buffer_t msrs, uint32_t nr_entries,
uint32_t *err_msr);
-/*
+/**
* Calculate whether two policies are compatible.
*
* i.e. Can a VM configured with @guest run on a CPU supporting @host.
@@ -573,8 +567,8 @@ int x86_msr_copy_from_buffer(struct msr_policy *policy,
* incompatibility is detected, the optional err pointer may identify the
* problematic leaf/subleaf and/or MSR.
*/
-int x86_cpu_policies_are_compatible(const struct old_cpu_policy *host,
- const struct old_cpu_policy *guest,
+int x86_cpu_policies_are_compatible(const struct cpu_policy *host,
+ const struct cpu_policy *guest,
struct cpu_policy_errors *err);
#endif /* !XEN_LIB_X86_POLICIES_H */
diff --git a/xen/lib/x86/policy.c b/xen/lib/x86/policy.c
index 2975711d7c6c..a9c60000af9d 100644
--- a/xen/lib/x86/policy.c
+++ b/xen/lib/x86/policy.c
@@ -2,8 +2,8 @@
#include <xen/lib/x86/cpu-policy.h>
-int x86_cpu_policies_are_compatible(const struct old_cpu_policy *host,
- const struct old_cpu_policy *guest,
+int x86_cpu_policies_are_compatible(const struct cpu_policy *host,
+ const struct cpu_policy *guest,
struct cpu_policy_errors *err)
{
struct cpu_policy_errors e = INIT_CPU_POLICY_ERRORS;
@@ -15,18 +15,18 @@ int x86_cpu_policies_are_compatible(const struct old_cpu_policy *host,
#define FAIL_MSR(m) \
do { e.msr = (m); goto out; } while ( 0 )
- if ( guest->cpuid->basic.max_leaf > host->cpuid->basic.max_leaf )
+ if ( guest->basic.max_leaf > host->basic.max_leaf )
FAIL_CPUID(0, NA);
- if ( guest->cpuid->feat.max_subleaf > host->cpuid->feat.max_subleaf )
+ if ( guest->feat.max_subleaf > host->feat.max_subleaf )
FAIL_CPUID(7, 0);
- if ( guest->cpuid->extd.max_leaf > host->cpuid->extd.max_leaf )
+ if ( guest->extd.max_leaf > host->extd.max_leaf )
FAIL_CPUID(0x80000000, NA);
/* TODO: Audit more CPUID data. */
- if ( ~host->msr->platform_info.raw & guest->msr->platform_info.raw )
+ if ( ~host->platform_info.raw & guest->platform_info.raw )
FAIL_MSR(MSR_INTEL_PLATFORM_INFO);
#undef FAIL_MSR
--
2.30.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 9/9] RFC: Everything else
2023-03-29 20:51 [PATCH RFC 0/9] x86: Merge cpuid and msr policy Andrew Cooper
` (7 preceding siblings ...)
2023-03-29 20:51 ` [PATCH 8/9] x86: Drop struct old_cpu_policy Andrew Cooper
@ 2023-03-29 20:51 ` Andrew Cooper
2023-03-30 10:16 ` Jan Beulich
2023-03-30 10:18 ` [PATCH RFC 0/9] x86: Merge cpuid and msr policy Jan Beulich
2023-03-30 11:07 ` Roger Pau Monné
10 siblings, 1 reply; 34+ messages in thread
From: Andrew Cooper @ 2023-03-29 20:51 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné, Wei Liu
Looking at this diff, I'm wondering whether keeping
union {
struct cpu_policy *cpuid;
struct cpu_policy *cpu_policy;
};
permentantly might be a good idea, because d->arch.cpuid->$X reads rather
better than d->arch.cpu_policy->$X
Thoughts?
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
---
tools/fuzz/cpu-policy/afl-policy-fuzzer.c | 12 +-
.../fuzz/x86_instruction_emulator/fuzz-emul.c | 2 +-
tools/tests/cpu-policy/test-cpu-policy.c | 10 +-
tools/tests/x86_emulator/test_x86_emulator.c | 2 +-
tools/tests/x86_emulator/x86-emulate.c | 2 +-
xen/arch/x86/cpu/mcheck/mce_intel.c | 2 +-
xen/arch/x86/cpu/vpmu_intel.c | 4 +-
xen/arch/x86/cpuid.c | 26 +--
xen/arch/x86/domain.c | 2 +-
xen/arch/x86/hvm/emulate.c | 2 +-
xen/arch/x86/hvm/hvm.c | 38 ++--
xen/arch/x86/hvm/ioreq.c | 4 +-
xen/arch/x86/hvm/mtrr.c | 2 +-
xen/arch/x86/hvm/svm/svm.c | 18 +-
xen/arch/x86/hvm/svm/svmdebug.c | 2 +-
xen/arch/x86/hvm/vlapic.c | 2 +-
xen/arch/x86/hvm/vmx/vmx.c | 12 +-
xen/arch/x86/hvm/vmx/vvmx.c | 2 +-
xen/arch/x86/include/asm/domain.h | 5 +-
xen/arch/x86/include/asm/guest_pt.h | 4 +-
xen/arch/x86/include/asm/msr.h | 2 +-
xen/arch/x86/include/asm/paging.h | 2 +-
xen/arch/x86/mm/shadow/hvm.c | 2 +-
xen/arch/x86/msr.c | 24 +--
xen/arch/x86/pv/domain.c | 2 +-
xen/arch/x86/pv/emul-priv-op.c | 6 +-
xen/arch/x86/pv/ro-page-fault.c | 2 +-
xen/arch/x86/sysctl.c | 4 +-
xen/arch/x86/traps.c | 2 +-
xen/arch/x86/x86_emulate.c | 2 +-
xen/arch/x86/x86_emulate/x86_emulate.c | 166 +++++++++---------
xen/arch/x86/x86_emulate/x86_emulate.h | 4 +-
xen/arch/x86/xstate.c | 4 +-
xen/include/xen/lib/x86/cpu-policy.h | 28 ++-
xen/lib/x86/cpuid.c | 10 +-
xen/lib/x86/msr.c | 4 +-
36 files changed, 205 insertions(+), 212 deletions(-)
diff --git a/tools/fuzz/cpu-policy/afl-policy-fuzzer.c b/tools/fuzz/cpu-policy/afl-policy-fuzzer.c
index 0ce3d8e16626..58b4dbacf96b 100644
--- a/tools/fuzz/cpu-policy/afl-policy-fuzzer.c
+++ b/tools/fuzz/cpu-policy/afl-policy-fuzzer.c
@@ -16,9 +16,9 @@ static bool debug;
#define EMPTY_LEAF ((struct cpuid_leaf){})
-static void check_cpuid(struct cpuid_policy *cp)
+static void check_cpuid(struct cpu_policy *cp)
{
- struct cpuid_policy new = {};
+ struct cpu_policy new = {};
size_t data_end;
xen_cpuid_leaf_t *leaves = malloc(CPUID_MAX_SERIALISED_LEAVES *
sizeof(xen_cpuid_leaf_t));
@@ -76,9 +76,9 @@ static void check_cpuid(struct cpuid_policy *cp)
free(leaves);
}
-static void check_msr(struct msr_policy *mp)
+static void check_msr(struct cpu_policy *mp)
{
- struct msr_policy new = {};
+ struct cpu_policy new = {};
xen_msr_entry_t *msrs = malloc(MSR_MAX_SERIALISED_ENTRIES *
sizeof(xen_msr_entry_t));
unsigned int nr = MSR_MAX_SERIALISED_ENTRIES;
@@ -144,8 +144,8 @@ int main(int argc, char **argv)
while ( __AFL_LOOP(1000) )
#endif
{
- struct cpuid_policy *cp = NULL;
- struct msr_policy *mp = NULL;
+ struct cpu_policy *cp = NULL;
+ struct cpu_policy *mp = NULL;
if ( fp != stdin )
{
diff --git a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
index 966e46bee199..4885a68210d0 100644
--- a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
+++ b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
@@ -893,7 +893,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data_p, size_t size)
struct x86_emulate_ctxt ctxt = {
.data = &state,
.regs = &input.regs,
- .cpuid = &cp,
+ .cpu_policy = &cp,
.addr_size = 8 * sizeof(void *),
.sp_size = 8 * sizeof(void *),
};
diff --git a/tools/tests/cpu-policy/test-cpu-policy.c b/tools/tests/cpu-policy/test-cpu-policy.c
index 5b38702b1c62..a4ca07f33973 100644
--- a/tools/tests/cpu-policy/test-cpu-policy.c
+++ b/tools/tests/cpu-policy/test-cpu-policy.c
@@ -98,7 +98,7 @@ static bool msrs_are_sorted(const xen_msr_entry_t *entries, unsigned int nr)
static void test_cpuid_current(void)
{
- struct cpuid_policy p;
+ struct cpu_policy p;
xen_cpuid_leaf_t leaves[CPUID_MAX_SERIALISED_LEAVES];
unsigned int nr = ARRAY_SIZE(leaves);
int rc;
@@ -118,7 +118,7 @@ static void test_cpuid_current(void)
static void test_cpuid_serialise_success(void)
{
static const struct test {
- struct cpuid_policy p;
+ struct cpu_policy p;
const char *name;
unsigned int nr_leaves;
} tests[] = {
@@ -242,7 +242,7 @@ static void test_cpuid_serialise_success(void)
static void test_msr_serialise_success(void)
{
static const struct test {
- struct msr_policy p;
+ struct cpu_policy p;
const char *name;
unsigned int nr_msrs;
} tests[] = {
@@ -430,7 +430,7 @@ static void test_cpuid_out_of_range_clearing(void)
static const struct test {
const char *name;
unsigned int nr_markers;
- struct cpuid_policy p;
+ struct cpu_policy p;
} tests[] = {
{
.name = "basic",
@@ -550,7 +550,7 @@ static void test_cpuid_out_of_range_clearing(void)
for ( size_t i = 0; i < ARRAY_SIZE(tests); ++i )
{
const struct test *t = &tests[i];
- struct cpuid_policy *p = memdup(&t->p);
+ struct cpu_policy *p = memdup(&t->p);
void *ptr;
unsigned int nr_markers;
diff --git a/tools/tests/x86_emulator/test_x86_emulator.c b/tools/tests/x86_emulator/test_x86_emulator.c
index 31586f805726..7b7fbaaf45ec 100644
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -909,7 +909,7 @@ int main(int argc, char **argv)
ctxt.regs = ®s;
ctxt.force_writeback = 0;
- ctxt.cpuid = &cp;
+ ctxt.cpu_policy = &cp;
ctxt.lma = sizeof(void *) == 8;
ctxt.addr_size = 8 * sizeof(void *);
ctxt.sp_size = 8 * sizeof(void *);
diff --git a/tools/tests/x86_emulator/x86-emulate.c b/tools/tests/x86_emulator/x86-emulate.c
index ea286d6ad87b..5ad282b57545 100644
--- a/tools/tests/x86_emulator/x86-emulate.c
+++ b/tools/tests/x86_emulator/x86-emulate.c
@@ -38,7 +38,7 @@
#define put_stub(stb) ((stb).addr = 0)
uint32_t mxcsr_mask = 0x0000ffbf;
-struct cpuid_policy cp;
+struct cpu_policy cp;
static char fpu_save_area[0x4000] __attribute__((__aligned__((64))));
static bool use_xsave;
diff --git a/xen/arch/x86/cpu/mcheck/mce_intel.c b/xen/arch/x86/cpu/mcheck/mce_intel.c
index 301533722d1a..2f23f02923d2 100644
--- a/xen/arch/x86/cpu/mcheck/mce_intel.c
+++ b/xen/arch/x86/cpu/mcheck/mce_intel.c
@@ -1008,7 +1008,7 @@ int vmce_intel_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
int vmce_intel_rdmsr(const struct vcpu *v, uint32_t msr, uint64_t *val)
{
- const struct cpuid_policy *cp = v->domain->arch.cpuid;
+ const struct cpu_policy *cp = v->domain->arch.cpu_policy;
unsigned int bank = msr - MSR_IA32_MC0_CTL2;
switch ( msr )
diff --git a/xen/arch/x86/cpu/vpmu_intel.c b/xen/arch/x86/cpu/vpmu_intel.c
index bcfa187a14b6..045f40dd582e 100644
--- a/xen/arch/x86/cpu/vpmu_intel.c
+++ b/xen/arch/x86/cpu/vpmu_intel.c
@@ -839,8 +839,8 @@ static int cf_check core2_vpmu_initialise(struct vcpu *v)
u64 msr_content;
static bool_t ds_warned;
- if ( v->domain->arch.cpuid->basic.pmu_version <= 1 ||
- v->domain->arch.cpuid->basic.pmu_version >= 6 )
+ if ( v->domain->arch.cpu_policy->basic.pmu_version <= 1 ||
+ v->domain->arch.cpu_policy->basic.pmu_version >= 6 )
return -EINVAL;
if ( (arch_pmc_cnt + fixed_pmc_cnt) == 0 )
diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index df3e503ced9d..c7e268e14918 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -178,7 +178,7 @@ static void sanitise_featureset(uint32_t *fs)
}
}
-static void recalculate_xstate(struct cpuid_policy *p)
+static void recalculate_xstate(struct cpu_policy *p)
{
uint64_t xstates = XSTATE_FP_SSE;
uint32_t xstate_size = XSTATE_AREA_MIN_SIZE;
@@ -256,7 +256,7 @@ static void recalculate_xstate(struct cpuid_policy *p)
* Misc adjustments to the policy. Mostly clobbering reserved fields and
* duplicating shared fields. Intentionally hidden fields are annotated.
*/
-static void recalculate_misc(struct cpuid_policy *p)
+static void recalculate_misc(struct cpu_policy *p)
{
p->basic.raw_fms &= 0x0fff0fff; /* Clobber Processor Type on Intel. */
p->basic.apic_id = 0; /* Dynamic. */
@@ -334,7 +334,7 @@ static void recalculate_misc(struct cpuid_policy *p)
static void __init calculate_raw_policy(void)
{
- struct cpuid_policy *p = &raw_cpu_policy;
+ struct cpu_policy *p = &raw_cpu_policy;
x86_cpuid_policy_fill_native(p);
@@ -344,7 +344,7 @@ static void __init calculate_raw_policy(void)
static void __init calculate_host_policy(void)
{
- struct cpuid_policy *p = &host_cpu_policy;
+ struct cpu_policy *p = &host_cpu_policy;
unsigned int max_extd_leaf;
*p = raw_cpu_policy;
@@ -445,7 +445,7 @@ static void __init guest_common_feature_adjustments(uint32_t *fs)
static void __init calculate_pv_max_policy(void)
{
- struct cpuid_policy *p = &pv_max_cpu_policy;
+ struct cpu_policy *p = &pv_max_cpu_policy;
uint32_t pv_featureset[FSCAPINTS];
unsigned int i;
@@ -476,7 +476,7 @@ static void __init calculate_pv_max_policy(void)
static void __init calculate_pv_def_policy(void)
{
- struct cpuid_policy *p = &pv_def_cpu_policy;
+ struct cpu_policy *p = &pv_def_cpu_policy;
uint32_t pv_featureset[FSCAPINTS];
unsigned int i;
@@ -496,7 +496,7 @@ static void __init calculate_pv_def_policy(void)
static void __init calculate_hvm_max_policy(void)
{
- struct cpuid_policy *p = &hvm_max_cpu_policy;
+ struct cpu_policy *p = &hvm_max_cpu_policy;
uint32_t hvm_featureset[FSCAPINTS];
unsigned int i;
const uint32_t *hvm_featuremask;
@@ -587,7 +587,7 @@ static void __init calculate_hvm_max_policy(void)
static void __init calculate_hvm_def_policy(void)
{
- struct cpuid_policy *p = &hvm_def_cpu_policy;
+ struct cpu_policy *p = &hvm_def_cpu_policy;
uint32_t hvm_featureset[FSCAPINTS];
unsigned int i;
const uint32_t *hvm_featuremask;
@@ -658,8 +658,8 @@ bool recheck_cpu_features(unsigned int cpu)
void recalculate_cpuid_policy(struct domain *d)
{
- struct cpuid_policy *p = d->arch.cpuid;
- const struct cpuid_policy *max = is_pv_domain(d)
+ struct cpu_policy *p = d->arch.cpu_policy;
+ const struct cpu_policy *max = is_pv_domain(d)
? (IS_ENABLED(CONFIG_PV) ? &pv_max_cpu_policy : NULL)
: (IS_ENABLED(CONFIG_HVM) ? &hvm_max_cpu_policy : NULL);
uint32_t fs[FSCAPINTS], max_fs[FSCAPINTS];
@@ -786,7 +786,7 @@ void recalculate_cpuid_policy(struct domain *d)
void __init init_dom0_cpuid_policy(struct domain *d)
{
- struct cpuid_policy *p = d->arch.cpuid;
+ struct cpu_policy *p = d->arch.cpu_policy;
/* dom0 can't migrate. Give it ITSC if available. */
if ( cpu_has_itsc )
@@ -824,7 +824,7 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
uint32_t subleaf, struct cpuid_leaf *res)
{
const struct domain *d = v->domain;
- const struct cpuid_policy *p = d->arch.cpuid;
+ const struct cpu_policy *p = d->arch.cpu_policy;
*res = EMPTY_LEAF;
@@ -1199,7 +1199,7 @@ static void __init __maybe_unused build_assertions(void)
BUILD_BUG_ON(ARRAY_SIZE(deep_features) != FSCAPINTS);
/* Find some more clever allocation scheme if this trips. */
- BUILD_BUG_ON(sizeof(struct cpuid_policy) > PAGE_SIZE);
+ BUILD_BUG_ON(sizeof(struct cpu_policy) > PAGE_SIZE);
BUILD_BUG_ON(sizeof(raw_cpu_policy.basic) !=
sizeof(raw_cpu_policy.basic.raw));
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 7cf66aee042c..c628619ead2c 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -283,7 +283,7 @@ void update_guest_memory_policy(struct vcpu *v,
void domain_cpu_policy_changed(struct domain *d)
{
- const struct cpuid_policy *p = d->arch.cpuid;
+ const struct cpu_policy *p = d->arch.cpu_policy;
struct vcpu *v;
if ( is_pv_domain(d) )
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index 95364deb1996..53f58c805185 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -2846,7 +2846,7 @@ void hvm_emulate_init_once(
hvmemul_ctxt->validate = validate;
hvmemul_ctxt->ctxt.regs = regs;
- hvmemul_ctxt->ctxt.cpuid = curr->domain->arch.cpuid;
+ hvmemul_ctxt->ctxt.cpu_policy = curr->domain->arch.cpu_policy;
hvmemul_ctxt->ctxt.force_writeback = true;
}
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index d326fa1c0136..0cf1b483dbbe 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -925,7 +925,7 @@ const char *hvm_efer_valid(const struct vcpu *v, uint64_t value,
signed int cr0_pg)
{
const struct domain *d = v->domain;
- const struct cpuid_policy *p = d->arch.cpuid;
+ const struct cpu_policy *p = d->arch.cpu_policy;
if ( value & ~EFER_KNOWN_MASK )
return "Unknown bits set";
@@ -962,7 +962,7 @@ const char *hvm_efer_valid(const struct vcpu *v, uint64_t value,
/* These bits in CR4 can be set by the guest. */
unsigned long hvm_cr4_guest_valid_bits(const struct domain *d)
{
- const struct cpuid_policy *p = d->arch.cpuid;
+ const struct cpu_policy *p = d->arch.cpu_policy;
bool mce, vmxe, cet;
/* Logic broken out simply to aid readability below. */
@@ -1040,7 +1040,7 @@ static int cf_check hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
return -EINVAL;
}
- if ( ctxt.cr3 >> d->arch.cpuid->extd.maxphysaddr )
+ if ( ctxt.cr3 >> d->arch.cpu_policy->extd.maxphysaddr )
{
printk(XENLOG_G_ERR "HVM%d restore: bad CR3 %#" PRIx64 "\n",
d->domain_id, ctxt.cr3);
@@ -1418,7 +1418,7 @@ static int cf_check hvm_save_cpu_msrs(struct vcpu *v, hvm_domain_context_t *h)
switch ( msr )
{
case MSR_SPEC_CTRL:
- val &= msr_spec_ctrl_valid_bits(d->arch.cpuid);
+ val &= msr_spec_ctrl_valid_bits(d->arch.cpu_policy);
break;
}
@@ -2397,7 +2397,7 @@ int hvm_set_cr3(unsigned long value, bool noflush, bool may_defer)
struct vcpu *curr = current;
struct domain *currd = curr->domain;
- if ( value >> currd->arch.cpuid->extd.maxphysaddr )
+ if ( value >> currd->arch.cpu_policy->extd.maxphysaddr )
{
HVM_DBG_LOG(DBG_LEVEL_1,
"Attempt to set reserved CR3 bit(s): %lx", value);
@@ -2666,7 +2666,7 @@ bool hvm_vcpu_virtual_to_linear(
else if ( last_byte > reg->limit )
goto out; /* last byte is beyond limit */
else if ( last_byte < offset &&
- v->domain->arch.cpuid->x86_vendor == X86_VENDOR_AMD )
+ v->domain->arch.cpu_policy->x86_vendor == X86_VENDOR_AMD )
goto out; /* access wraps */
}
@@ -3554,12 +3554,12 @@ int hvm_msr_read_intercept(unsigned int msr, uint64_t *msr_content)
break;
case MSR_MTRRcap:
- if ( !d->arch.cpuid->basic.mtrr )
+ if ( !d->arch.cpu_policy->basic.mtrr )
goto gp_fault;
*msr_content = v->arch.hvm.mtrr.mtrr_cap;
break;
case MSR_MTRRdefType:
- if ( !d->arch.cpuid->basic.mtrr )
+ if ( !d->arch.cpu_policy->basic.mtrr )
goto gp_fault;
*msr_content = v->arch.hvm.mtrr.def_type |
MASK_INSR(v->arch.hvm.mtrr.enabled, MTRRdefType_E) |
@@ -3567,27 +3567,27 @@ int hvm_msr_read_intercept(unsigned int msr, uint64_t *msr_content)
MTRRdefType_FE);
break;
case MSR_MTRRfix64K_00000:
- if ( !d->arch.cpuid->basic.mtrr )
+ if ( !d->arch.cpu_policy->basic.mtrr )
goto gp_fault;
*msr_content = fixed_range_base[0];
break;
case MSR_MTRRfix16K_80000:
case MSR_MTRRfix16K_A0000:
- if ( !d->arch.cpuid->basic.mtrr )
+ if ( !d->arch.cpu_policy->basic.mtrr )
goto gp_fault;
index = msr - MSR_MTRRfix16K_80000;
*msr_content = fixed_range_base[array_index_nospec(index + 1,
ARRAY_SIZE(v->arch.hvm.mtrr.fixed_ranges))];
break;
case MSR_MTRRfix4K_C0000...MSR_MTRRfix4K_F8000:
- if ( !d->arch.cpuid->basic.mtrr )
+ if ( !d->arch.cpu_policy->basic.mtrr )
goto gp_fault;
index = msr - MSR_MTRRfix4K_C0000;
*msr_content = fixed_range_base[array_index_nospec(index + 3,
ARRAY_SIZE(v->arch.hvm.mtrr.fixed_ranges))];
break;
case MSR_IA32_MTRR_PHYSBASE(0)...MSR_IA32_MTRR_PHYSMASK(MTRR_VCNT_MAX - 1):
- if ( !d->arch.cpuid->basic.mtrr )
+ if ( !d->arch.cpu_policy->basic.mtrr )
goto gp_fault;
index = msr - MSR_IA32_MTRR_PHYSBASE(0);
if ( (index / 2) >=
@@ -3693,14 +3693,14 @@ int hvm_msr_write_intercept(unsigned int msr, uint64_t msr_content,
goto gp_fault;
case MSR_MTRRdefType:
- if ( !d->arch.cpuid->basic.mtrr )
+ if ( !d->arch.cpu_policy->basic.mtrr )
goto gp_fault;
if ( !mtrr_def_type_msr_set(v->domain, &v->arch.hvm.mtrr,
msr_content) )
goto gp_fault;
break;
case MSR_MTRRfix64K_00000:
- if ( !d->arch.cpuid->basic.mtrr )
+ if ( !d->arch.cpu_policy->basic.mtrr )
goto gp_fault;
if ( !mtrr_fix_range_msr_set(v->domain, &v->arch.hvm.mtrr, 0,
msr_content) )
@@ -3708,7 +3708,7 @@ int hvm_msr_write_intercept(unsigned int msr, uint64_t msr_content,
break;
case MSR_MTRRfix16K_80000:
case MSR_MTRRfix16K_A0000:
- if ( !d->arch.cpuid->basic.mtrr )
+ if ( !d->arch.cpu_policy->basic.mtrr )
goto gp_fault;
index = msr - MSR_MTRRfix16K_80000 + 1;
if ( !mtrr_fix_range_msr_set(v->domain, &v->arch.hvm.mtrr,
@@ -3716,7 +3716,7 @@ int hvm_msr_write_intercept(unsigned int msr, uint64_t msr_content,
goto gp_fault;
break;
case MSR_MTRRfix4K_C0000...MSR_MTRRfix4K_F8000:
- if ( !d->arch.cpuid->basic.mtrr )
+ if ( !d->arch.cpu_policy->basic.mtrr )
goto gp_fault;
index = msr - MSR_MTRRfix4K_C0000 + 3;
if ( !mtrr_fix_range_msr_set(v->domain, &v->arch.hvm.mtrr,
@@ -3724,7 +3724,7 @@ int hvm_msr_write_intercept(unsigned int msr, uint64_t msr_content,
goto gp_fault;
break;
case MSR_IA32_MTRR_PHYSBASE(0)...MSR_IA32_MTRR_PHYSMASK(MTRR_VCNT_MAX - 1):
- if ( !d->arch.cpuid->basic.mtrr )
+ if ( !d->arch.cpu_policy->basic.mtrr )
goto gp_fault;
index = msr - MSR_IA32_MTRR_PHYSBASE(0);
if ( ((index / 2) >=
@@ -3830,7 +3830,7 @@ void hvm_ud_intercept(struct cpu_user_regs *regs)
{
struct vcpu *cur = current;
bool should_emulate =
- cur->domain->arch.cpuid->x86_vendor != boot_cpu_data.x86_vendor;
+ cur->domain->arch.cpu_policy->x86_vendor != boot_cpu_data.x86_vendor;
struct hvm_emulate_ctxt ctxt;
hvm_emulate_init_once(&ctxt, opt_hvm_fep ? NULL : is_cross_vendor, regs);
@@ -4737,7 +4737,7 @@ static int do_altp2m_op(
case HVMOP_altp2m_set_suppress_ve_multi:
{
- uint64_t max_phys_addr = (1UL << d->arch.cpuid->extd.maxphysaddr) - 1;
+ uint64_t max_phys_addr = (1UL << d->arch.cpu_policy->extd.maxphysaddr) - 1;
a.u.suppress_ve_multi.last_gfn = min(a.u.suppress_ve_multi.last_gfn,
max_phys_addr);
diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
index 0bdcca1e1a5f..f397ce68cdb5 100644
--- a/xen/arch/x86/hvm/ioreq.c
+++ b/xen/arch/x86/hvm/ioreq.c
@@ -295,9 +295,9 @@ bool arch_ioreq_server_get_type_addr(const struct domain *d,
*addr = ((uint64_t)sbdf.sbdf << 32) | reg;
/* AMD extended configuration space access? */
if ( CF8_ADDR_HI(cf8) &&
- d->arch.cpuid->x86_vendor == X86_VENDOR_AMD &&
+ d->arch.cpu_policy->x86_vendor == X86_VENDOR_AMD &&
(x86_fam = get_cpu_family(
- d->arch.cpuid->basic.raw_fms, NULL, NULL)) >= 0x10 &&
+ d->arch.cpu_policy->basic.raw_fms, NULL, NULL)) >= 0x10 &&
x86_fam < 0x17 )
{
uint64_t msr_val;
diff --git a/xen/arch/x86/hvm/mtrr.c b/xen/arch/x86/hvm/mtrr.c
index f1a88d761635..805e6d4ff243 100644
--- a/xen/arch/x86/hvm/mtrr.c
+++ b/xen/arch/x86/hvm/mtrr.c
@@ -460,7 +460,7 @@ bool_t mtrr_var_range_msr_set(
return 0;
if ( d == current->domain )
- phys_addr = d->arch.cpuid->extd.maxphysaddr;
+ phys_addr = d->arch.cpu_policy->extd.maxphysaddr;
else
phys_addr = paddr_bits;
msr_mask = ~((((uint64_t)1) << phys_addr) - 1);
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 02563e4b7027..5c2399e3eaba 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -328,7 +328,7 @@ static void svm_save_dr(struct vcpu *v)
v->arch.hvm.flag_dr_dirty = 0;
vmcb_set_dr_intercepts(vmcb, ~0u);
- if ( v->domain->arch.cpuid->extd.dbext )
+ if ( v->domain->arch.cpu_policy->extd.dbext )
{
svm_intercept_msr(v, MSR_AMD64_DR0_ADDRESS_MASK, MSR_INTERCEPT_RW);
svm_intercept_msr(v, MSR_AMD64_DR1_ADDRESS_MASK, MSR_INTERCEPT_RW);
@@ -359,7 +359,7 @@ static void __restore_debug_registers(struct vmcb_struct *vmcb, struct vcpu *v)
ASSERT(v == current);
- if ( v->domain->arch.cpuid->extd.dbext )
+ if ( v->domain->arch.cpu_policy->extd.dbext )
{
svm_intercept_msr(v, MSR_AMD64_DR0_ADDRESS_MASK, MSR_INTERCEPT_NONE);
svm_intercept_msr(v, MSR_AMD64_DR1_ADDRESS_MASK, MSR_INTERCEPT_NONE);
@@ -583,11 +583,11 @@ static void cf_check svm_cpuid_policy_changed(struct vcpu *v)
{
struct svm_vcpu *svm = &v->arch.hvm.svm;
struct vmcb_struct *vmcb = svm->vmcb;
- const struct cpuid_policy *cp = v->domain->arch.cpuid;
+ const struct cpu_policy *cp = v->domain->arch.cpu_policy;
u32 bitmap = vmcb_get_exception_intercepts(vmcb);
if ( opt_hvm_fep ||
- (v->domain->arch.cpuid->x86_vendor != boot_cpu_data.x86_vendor) )
+ (v->domain->arch.cpu_policy->x86_vendor != boot_cpu_data.x86_vendor) )
bitmap |= (1U << TRAP_invalid_op);
else
bitmap &= ~(1U << TRAP_invalid_op);
@@ -967,7 +967,7 @@ static void cf_check svm_ctxt_switch_from(struct vcpu *v)
*/
if ( v->arch.msrs->virt_spec_ctrl.raw & SPEC_CTRL_SSBD )
{
- ASSERT(v->domain->arch.cpuid->extd.virt_ssbd);
+ ASSERT(v->domain->arch.cpu_policy->extd.virt_ssbd);
amd_set_legacy_ssbd(false);
}
}
@@ -1001,7 +1001,7 @@ static void cf_check svm_ctxt_switch_to(struct vcpu *v)
/* Load SSBD if set by the guest. */
if ( v->arch.msrs->virt_spec_ctrl.raw & SPEC_CTRL_SSBD )
{
- ASSERT(v->domain->arch.cpuid->extd.virt_ssbd);
+ ASSERT(v->domain->arch.cpu_policy->extd.virt_ssbd);
amd_set_legacy_ssbd(true);
}
}
@@ -1928,7 +1928,7 @@ static int cf_check svm_msr_read_intercept(
case MSR_AMD_OSVW_ID_LENGTH:
case MSR_AMD_OSVW_STATUS:
- if ( !d->arch.cpuid->extd.osvw )
+ if ( !d->arch.cpu_policy->extd.osvw )
goto gpf;
*msr_content = d->arch.hvm.svm.osvw.raw[msr - MSR_AMD_OSVW_ID_LENGTH];
break;
@@ -2121,7 +2121,7 @@ static int cf_check svm_msr_write_intercept(
case MSR_AMD_OSVW_ID_LENGTH:
case MSR_AMD_OSVW_STATUS:
- if ( !d->arch.cpuid->extd.osvw )
+ if ( !d->arch.cpu_policy->extd.osvw )
goto gpf;
/* Write-discard */
break;
@@ -2187,7 +2187,7 @@ static void svm_vmexit_do_rdtsc(struct cpu_user_regs *regs, bool rdtscp)
const struct domain *currd = curr->domain;
unsigned int inst_len;
- if ( rdtscp && !currd->arch.cpuid->extd.rdtscp )
+ if ( rdtscp && !currd->arch.cpu_policy->extd.rdtscp )
{
hvm_inject_hw_exception(TRAP_invalid_op, X86_EVENT_NO_EC);
return;
diff --git a/xen/arch/x86/hvm/svm/svmdebug.c b/xen/arch/x86/hvm/svm/svmdebug.c
index bce86f0ef706..05a2d119c744 100644
--- a/xen/arch/x86/hvm/svm/svmdebug.c
+++ b/xen/arch/x86/hvm/svm/svmdebug.c
@@ -130,7 +130,7 @@ bool svm_vmcb_isvalid(const char *from, const struct vmcb_struct *vmcb,
((cr3 & 7) ||
((!(cr4 & X86_CR4_PAE) || (efer & EFER_LMA)) && (cr3 & 0xfe0)) ||
((efer & EFER_LMA) &&
- (cr3 >> v->domain->arch.cpuid->extd.maxphysaddr))) )
+ (cr3 >> v->domain->arch.cpu_policy->extd.maxphysaddr))) )
PRINTF("CR3: MBZ bits are set (%#"PRIx64")\n", cr3);
valid = hvm_cr4_guest_valid_bits(v->domain);
diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index dc93b5e930b1..f4f5ffc673e5 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -1083,7 +1083,7 @@ static void set_x2apic_id(struct vlapic *vlapic)
int guest_wrmsr_apic_base(struct vcpu *v, uint64_t value)
{
- const struct cpuid_policy *cp = v->domain->arch.cpuid;
+ const struct cpu_policy *cp = v->domain->arch.cpu_policy;
struct vlapic *vlapic = vcpu_vlapic(v);
if ( !has_vlapic(v->domain) )
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index e05588505871..593400a8608f 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -710,7 +710,7 @@ static void vmx_restore_host_msrs(void)
static void vmx_save_guest_msrs(struct vcpu *v)
{
- const struct cpuid_policy *cp = v->domain->arch.cpuid;
+ const struct cpu_policy *cp = v->domain->arch.cpu_policy;
struct vcpu_msrs *msrs = v->arch.msrs;
/*
@@ -731,7 +731,7 @@ static void vmx_save_guest_msrs(struct vcpu *v)
static void vmx_restore_guest_msrs(struct vcpu *v)
{
- const struct cpuid_policy *cp = v->domain->arch.cpuid;
+ const struct cpu_policy *cp = v->domain->arch.cpu_policy;
const struct vcpu_msrs *msrs = v->arch.msrs;
write_gs_shadow(v->arch.hvm.vmx.shadow_gs);
@@ -784,11 +784,11 @@ void vmx_update_exception_bitmap(struct vcpu *v)
static void cf_check vmx_cpuid_policy_changed(struct vcpu *v)
{
- const struct cpuid_policy *cp = v->domain->arch.cpuid;
+ const struct cpu_policy *cp = v->domain->arch.cpu_policy;
int rc = 0;
if ( opt_hvm_fep ||
- (v->domain->arch.cpuid->x86_vendor != boot_cpu_data.x86_vendor) )
+ (v->domain->arch.cpu_policy->x86_vendor != boot_cpu_data.x86_vendor) )
v->arch.hvm.vmx.exception_bitmap |= (1U << TRAP_invalid_op);
else
v->arch.hvm.vmx.exception_bitmap &= ~(1U << TRAP_invalid_op);
@@ -3521,7 +3521,7 @@ static int cf_check vmx_msr_write_intercept(
unsigned int msr, uint64_t msr_content)
{
struct vcpu *v = current;
- const struct cpuid_policy *cp = v->domain->arch.cpuid;
+ const struct cpu_policy *cp = v->domain->arch.cpu_policy;
HVM_DBG_LOG(DBG_LEVEL_MSR, "ecx=%#x, msr_value=%#"PRIx64, msr, msr_content);
@@ -4451,7 +4451,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
vmx_invlpg_intercept(exit_qualification);
break;
case EXIT_REASON_RDTSCP:
- if ( !currd->arch.cpuid->extd.rdtscp )
+ if ( !currd->arch.cpu_policy->extd.rdtscp )
{
hvm_inject_hw_exception(TRAP_invalid_op, X86_EVENT_NO_EC);
break;
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 674cdabb0736..f78247d9122c 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -2175,7 +2175,7 @@ int nvmx_msr_read_intercept(unsigned int msr, u64 *msr_content)
int r = 1;
/* VMX capablity MSRs are available only when guest supports VMX. */
- if ( !nestedhvm_enabled(d) || !d->arch.cpuid->basic.vmx )
+ if ( !nestedhvm_enabled(d) || !d->arch.cpu_policy->basic.vmx )
return 0;
/*
diff --git a/xen/arch/x86/include/asm/domain.h b/xen/arch/x86/include/asm/domain.h
index a1deab1d0dcc..6ab396c28589 100644
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -386,10 +386,7 @@ struct arch_domain
*/
uint8_t x87_fip_width;
- union {
- struct cpu_policy *cpuid; /* Temporary */
- struct cpu_policy *cpu_policy;
- };
+ struct cpu_policy *cpu_policy;
struct PITState vpit;
diff --git a/xen/arch/x86/include/asm/guest_pt.h b/xen/arch/x86/include/asm/guest_pt.h
index 6802db2a415a..bb5e5c4b9fba 100644
--- a/xen/arch/x86/include/asm/guest_pt.h
+++ b/xen/arch/x86/include/asm/guest_pt.h
@@ -293,7 +293,7 @@ static always_inline bool guest_pks_enabled(const struct vcpu *v)
static always_inline uint64_t guest_rsvd_bits(const struct vcpu *v)
{
return ((PADDR_MASK &
- ~((1ul << v->domain->arch.cpuid->extd.maxphysaddr) - 1)) |
+ ~((1ul << v->domain->arch.cpu_policy->extd.maxphysaddr) - 1)) |
(guest_nx_enabled(v) ? 0 : put_pte_flags(_PAGE_NX_BIT)));
}
@@ -332,7 +332,7 @@ static always_inline bool guest_l4e_rsvd_bits(const struct vcpu *v,
guest_l4e_t l4e)
{
return l4e.l4 & (guest_rsvd_bits(v) | GUEST_L4_PAGETABLE_RSVD |
- ((v->domain->arch.cpuid->x86_vendor == X86_VENDOR_AMD)
+ ((v->domain->arch.cpu_policy->x86_vendor == X86_VENDOR_AMD)
? _PAGE_GLOBAL : 0));
}
#endif /* GUEST_PAGING_LEVELS >= 4 */
diff --git a/xen/arch/x86/include/asm/msr.h b/xen/arch/x86/include/asm/msr.h
index b59a51d238a7..3efa4b02449e 100644
--- a/xen/arch/x86/include/asm/msr.h
+++ b/xen/arch/x86/include/asm/msr.h
@@ -290,7 +290,7 @@ static inline void wrmsr_tsc_aux(uint32_t val)
}
}
-uint64_t msr_spec_ctrl_valid_bits(const struct cpuid_policy *cp);
+uint64_t msr_spec_ctrl_valid_bits(const struct cpu_policy *cp);
/* Container object for per-vCPU MSRs */
struct vcpu_msrs
diff --git a/xen/arch/x86/include/asm/paging.h b/xen/arch/x86/include/asm/paging.h
index 2647b95e67a7..5ae8745bb190 100644
--- a/xen/arch/x86/include/asm/paging.h
+++ b/xen/arch/x86/include/asm/paging.h
@@ -346,7 +346,7 @@ int paging_set_allocation(struct domain *d, unsigned int pages,
/* Is gfn within maxphysaddr for the domain? */
static inline bool gfn_valid(const struct domain *d, gfn_t gfn)
{
- return !(gfn_x(gfn) >> (d->arch.cpuid->extd.maxphysaddr - PAGE_SHIFT));
+ return !(gfn_x(gfn) >> (d->arch.cpu_policy->extd.maxphysaddr - PAGE_SHIFT));
}
/* Maxphysaddr supportable by the paging infrastructure. */
diff --git a/xen/arch/x86/mm/shadow/hvm.c b/xen/arch/x86/mm/shadow/hvm.c
index e2ee1c77056f..cc84af01925a 100644
--- a/xen/arch/x86/mm/shadow/hvm.c
+++ b/xen/arch/x86/mm/shadow/hvm.c
@@ -319,7 +319,7 @@ const struct x86_emulate_ops *shadow_init_emulation(
memset(sh_ctxt, 0, sizeof(*sh_ctxt));
sh_ctxt->ctxt.regs = regs;
- sh_ctxt->ctxt.cpuid = curr->domain->arch.cpuid;
+ sh_ctxt->ctxt.cpu_policy = curr->domain->arch.cpu_policy;
sh_ctxt->ctxt.lma = hvm_long_mode_active(curr);
/* Segment cache initialisation. Primed with CS. */
diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index 672961dd3ac1..bf6c8f5f36ec 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -40,7 +40,7 @@ DEFINE_PER_CPU(uint32_t, tsc_aux);
static void __init calculate_raw_policy(void)
{
- struct msr_policy *mp = &raw_cpu_policy;
+ struct cpu_policy *mp = &raw_cpu_policy;
/* 0x000000ce MSR_INTEL_PLATFORM_INFO */
/* Was already added by probe_cpuid_faulting() */
@@ -51,7 +51,7 @@ static void __init calculate_raw_policy(void)
static void __init calculate_host_policy(void)
{
- struct msr_policy *mp = &host_cpu_policy;
+ struct cpu_policy *mp = &host_cpu_policy;
*mp = raw_cpu_policy;
@@ -71,7 +71,7 @@ static void __init calculate_host_policy(void)
static void __init calculate_pv_max_policy(void)
{
- struct msr_policy *mp = &pv_max_cpu_policy;
+ struct cpu_policy *mp = &pv_max_cpu_policy;
*mp = host_cpu_policy;
@@ -80,14 +80,14 @@ static void __init calculate_pv_max_policy(void)
static void __init calculate_pv_def_policy(void)
{
- struct msr_policy *mp = &pv_def_cpu_policy;
+ struct cpu_policy *mp = &pv_def_cpu_policy;
*mp = pv_max_cpu_policy;
}
static void __init calculate_hvm_max_policy(void)
{
- struct msr_policy *mp = &hvm_max_cpu_policy;
+ struct cpu_policy *mp = &hvm_max_cpu_policy;
*mp = host_cpu_policy;
@@ -99,7 +99,7 @@ static void __init calculate_hvm_max_policy(void)
static void __init calculate_hvm_def_policy(void)
{
- struct msr_policy *mp = &hvm_def_cpu_policy;
+ struct cpu_policy *mp = &hvm_def_cpu_policy;
*mp = hvm_max_cpu_policy;
}
@@ -138,7 +138,7 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
{
const struct vcpu *curr = current;
const struct domain *d = v->domain;
- const struct cpuid_policy *cp = d->arch.cpuid;
+ const struct cpu_policy *cp = d->arch.cpu_policy;
const struct vcpu_msrs *msrs = v->arch.msrs;
int ret = X86EMUL_OKAY;
@@ -409,7 +409,7 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
* separate CPUID features for this functionality, but only set will be
* active.
*/
-uint64_t msr_spec_ctrl_valid_bits(const struct cpuid_policy *cp)
+uint64_t msr_spec_ctrl_valid_bits(const struct cpu_policy *cp)
{
bool ssbd = cp->feat.ssbd || cp->extd.amd_ssbd;
bool psfd = cp->feat.intel_psfd || cp->extd.psfd;
@@ -428,7 +428,7 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
{
const struct vcpu *curr = current;
struct domain *d = v->domain;
- const struct cpuid_policy *cp = d->arch.cpuid;
+ const struct cpu_policy *cp = d->arch.cpu_policy;
struct vcpu_msrs *msrs = v->arch.msrs;
int ret = X86EMUL_OKAY;
@@ -469,7 +469,7 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
* for backwards compatiblity, the OS should write 0 to it before
* trying to access the current microcode version.
*/
- if ( d->arch.cpuid->x86_vendor != X86_VENDOR_INTEL || val != 0 )
+ if ( d->arch.cpu_policy->x86_vendor != X86_VENDOR_INTEL || val != 0 )
goto gp_fault;
break;
@@ -479,7 +479,7 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
* to AMD CPUs as well (at least the architectural/CPUID part does).
*/
if ( is_pv_domain(d) ||
- d->arch.cpuid->x86_vendor != X86_VENDOR_AMD )
+ d->arch.cpu_policy->x86_vendor != X86_VENDOR_AMD )
goto gp_fault;
break;
@@ -491,7 +491,7 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
* by any CPUID bit.
*/
if ( is_pv_domain(d) ||
- d->arch.cpuid->x86_vendor != X86_VENDOR_INTEL )
+ d->arch.cpu_policy->x86_vendor != X86_VENDOR_INTEL )
goto gp_fault;
break;
diff --git a/xen/arch/x86/pv/domain.c b/xen/arch/x86/pv/domain.c
index f94f28c8e271..dbda57894543 100644
--- a/xen/arch/x86/pv/domain.c
+++ b/xen/arch/x86/pv/domain.c
@@ -145,7 +145,7 @@ static void release_compat_l4(struct vcpu *v)
unsigned long pv_fixup_guest_cr4(const struct vcpu *v, unsigned long cr4)
{
- const struct cpuid_policy *p = v->domain->arch.cpuid;
+ const struct cpu_policy *p = v->domain->arch.cpu_policy;
/* Discard attempts to set guest controllable bits outside of the policy. */
cr4 &= ~((p->basic.tsc ? 0 : X86_CR4_TSD) |
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index 5da00e24e4ff..04416f197951 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -885,7 +885,7 @@ static int cf_check read_msr(
{
struct vcpu *curr = current;
const struct domain *currd = curr->domain;
- const struct cpuid_policy *cp = currd->arch.cpuid;
+ const struct cpu_policy *cp = currd->arch.cpu_policy;
bool vpmu_msr = false, warn = false;
uint64_t tmp;
int ret;
@@ -1034,7 +1034,7 @@ static int cf_check write_msr(
{
struct vcpu *curr = current;
const struct domain *currd = curr->domain;
- const struct cpuid_policy *cp = currd->arch.cpuid;
+ const struct cpu_policy *cp = currd->arch.cpu_policy;
bool vpmu_msr = false;
int ret;
@@ -1327,7 +1327,7 @@ int pv_emulate_privileged_op(struct cpu_user_regs *regs)
struct domain *currd = curr->domain;
struct priv_op_ctxt ctxt = {
.ctxt.regs = regs,
- .ctxt.cpuid = currd->arch.cpuid,
+ .ctxt.cpu_policy = currd->arch.cpu_policy,
.ctxt.lma = !is_pv_32bit_domain(currd),
};
int rc;
diff --git a/xen/arch/x86/pv/ro-page-fault.c b/xen/arch/x86/pv/ro-page-fault.c
index 5963f5ee2d51..0d02c7d2ab10 100644
--- a/xen/arch/x86/pv/ro-page-fault.c
+++ b/xen/arch/x86/pv/ro-page-fault.c
@@ -356,7 +356,7 @@ int pv_ro_page_fault(unsigned long addr, struct cpu_user_regs *regs)
unsigned int addr_size = is_pv_32bit_domain(currd) ? 32 : BITS_PER_LONG;
struct x86_emulate_ctxt ctxt = {
.regs = regs,
- .cpuid = currd->arch.cpuid,
+ .cpu_policy = currd->arch.cpu_policy,
.addr_size = addr_size,
.sp_size = addr_size,
.lma = addr_size > 32,
diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
index 3b2efecede2f..14d3dbca147b 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -295,7 +295,7 @@ long arch_do_sysctl(
case XEN_SYSCTL_get_cpu_featureset:
{
- static const struct cpuid_policy *const policy_table[6] = {
+ static const struct cpu_policy *policy_table[6] = {
[XEN_SYSCTL_cpu_featureset_raw] = &raw_cpu_policy,
[XEN_SYSCTL_cpu_featureset_host] = &host_cpu_policy,
#ifdef CONFIG_PV
@@ -307,7 +307,7 @@ long arch_do_sysctl(
[XEN_SYSCTL_cpu_featureset_hvm_max] = &hvm_max_cpu_policy,
#endif
};
- const struct cpuid_policy *p = NULL;
+ const struct cpu_policy *p = NULL;
uint32_t featureset[FSCAPINTS];
unsigned int nr;
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index cade9e12f8fa..d12004b1c6fc 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1035,7 +1035,7 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, uint32_t leaf,
uint32_t subleaf, struct cpuid_leaf *res)
{
const struct domain *d = v->domain;
- const struct cpuid_policy *p = d->arch.cpuid;
+ const struct cpu_policy *p = d->arch.cpu_policy;
uint32_t base = is_viridian_domain(d) ? 0x40000100 : 0x40000000;
uint32_t idx = leaf - base;
unsigned int limit = is_viridian_domain(d) ? p->hv2_limit : p->hv_limit;
diff --git a/xen/arch/x86/x86_emulate.c b/xen/arch/x86/x86_emulate.c
index 8c7d18521807..ff94758e0ebe 100644
--- a/xen/arch/x86/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate.c
@@ -74,7 +74,7 @@ int cf_check x86emul_read_xcr(
return X86EMUL_OKAY;
case 1:
- if ( current->domain->arch.cpuid->xstate.xgetbv1 )
+ if ( current->domain->arch.cpu_policy->xstate.xgetbv1 )
break;
/* fall through */
default:
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
index e38f98b54726..a26a2934b6a7 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1923,7 +1923,7 @@ in_protmode(
}
static bool
-_amd_like(const struct cpuid_policy *cp)
+_amd_like(const struct cpu_policy *cp)
{
return cp->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON);
}
@@ -1931,87 +1931,87 @@ _amd_like(const struct cpuid_policy *cp)
static bool
amd_like(const struct x86_emulate_ctxt *ctxt)
{
- return _amd_like(ctxt->cpuid);
+ return _amd_like(ctxt->cpu_policy);
}
-#define vcpu_has_fpu() (ctxt->cpuid->basic.fpu)
-#define vcpu_has_sep() (ctxt->cpuid->basic.sep)
-#define vcpu_has_cx8() (ctxt->cpuid->basic.cx8)
-#define vcpu_has_cmov() (ctxt->cpuid->basic.cmov)
-#define vcpu_has_clflush() (ctxt->cpuid->basic.clflush)
-#define vcpu_has_mmx() (ctxt->cpuid->basic.mmx)
-#define vcpu_has_fxsr() (ctxt->cpuid->basic.fxsr)
-#define vcpu_has_sse() (ctxt->cpuid->basic.sse)
-#define vcpu_has_sse2() (ctxt->cpuid->basic.sse2)
-#define vcpu_has_sse3() (ctxt->cpuid->basic.sse3)
-#define vcpu_has_pclmulqdq() (ctxt->cpuid->basic.pclmulqdq)
-#define vcpu_has_ssse3() (ctxt->cpuid->basic.ssse3)
-#define vcpu_has_fma() (ctxt->cpuid->basic.fma)
-#define vcpu_has_cx16() (ctxt->cpuid->basic.cx16)
-#define vcpu_has_sse4_1() (ctxt->cpuid->basic.sse4_1)
-#define vcpu_has_sse4_2() (ctxt->cpuid->basic.sse4_2)
-#define vcpu_has_movbe() (ctxt->cpuid->basic.movbe)
-#define vcpu_has_popcnt() (ctxt->cpuid->basic.popcnt)
-#define vcpu_has_aesni() (ctxt->cpuid->basic.aesni)
-#define vcpu_has_avx() (ctxt->cpuid->basic.avx)
-#define vcpu_has_f16c() (ctxt->cpuid->basic.f16c)
-#define vcpu_has_rdrand() (ctxt->cpuid->basic.rdrand)
-
-#define vcpu_has_mmxext() (ctxt->cpuid->extd.mmxext || vcpu_has_sse())
-#define vcpu_has_3dnow_ext() (ctxt->cpuid->extd._3dnowext)
-#define vcpu_has_3dnow() (ctxt->cpuid->extd._3dnow)
-#define vcpu_has_lahf_lm() (ctxt->cpuid->extd.lahf_lm)
-#define vcpu_has_cr8_legacy() (ctxt->cpuid->extd.cr8_legacy)
-#define vcpu_has_lzcnt() (ctxt->cpuid->extd.abm)
-#define vcpu_has_sse4a() (ctxt->cpuid->extd.sse4a)
-#define vcpu_has_misalignsse() (ctxt->cpuid->extd.misalignsse)
-#define vcpu_has_xop() (ctxt->cpuid->extd.xop)
-#define vcpu_has_fma4() (ctxt->cpuid->extd.fma4)
-#define vcpu_has_tbm() (ctxt->cpuid->extd.tbm)
-#define vcpu_has_clzero() (ctxt->cpuid->extd.clzero)
-#define vcpu_has_wbnoinvd() (ctxt->cpuid->extd.wbnoinvd)
-#define vcpu_has_nscb() (ctxt->cpuid->extd.nscb)
-
-#define vcpu_has_bmi1() (ctxt->cpuid->feat.bmi1)
-#define vcpu_has_hle() (ctxt->cpuid->feat.hle)
-#define vcpu_has_avx2() (ctxt->cpuid->feat.avx2)
-#define vcpu_has_bmi2() (ctxt->cpuid->feat.bmi2)
-#define vcpu_has_invpcid() (ctxt->cpuid->feat.invpcid)
-#define vcpu_has_rtm() (ctxt->cpuid->feat.rtm)
-#define vcpu_has_mpx() (ctxt->cpuid->feat.mpx)
-#define vcpu_has_avx512f() (ctxt->cpuid->feat.avx512f)
-#define vcpu_has_avx512dq() (ctxt->cpuid->feat.avx512dq)
-#define vcpu_has_rdseed() (ctxt->cpuid->feat.rdseed)
-#define vcpu_has_adx() (ctxt->cpuid->feat.adx)
-#define vcpu_has_smap() (ctxt->cpuid->feat.smap)
-#define vcpu_has_avx512_ifma() (ctxt->cpuid->feat.avx512_ifma)
-#define vcpu_has_clflushopt() (ctxt->cpuid->feat.clflushopt)
-#define vcpu_has_clwb() (ctxt->cpuid->feat.clwb)
-#define vcpu_has_avx512pf() (ctxt->cpuid->feat.avx512pf)
-#define vcpu_has_avx512er() (ctxt->cpuid->feat.avx512er)
-#define vcpu_has_avx512cd() (ctxt->cpuid->feat.avx512cd)
-#define vcpu_has_sha() (ctxt->cpuid->feat.sha)
-#define vcpu_has_avx512bw() (ctxt->cpuid->feat.avx512bw)
-#define vcpu_has_avx512vl() (ctxt->cpuid->feat.avx512vl)
-#define vcpu_has_avx512_vbmi() (ctxt->cpuid->feat.avx512_vbmi)
-#define vcpu_has_avx512_vbmi2() (ctxt->cpuid->feat.avx512_vbmi2)
-#define vcpu_has_gfni() (ctxt->cpuid->feat.gfni)
-#define vcpu_has_vaes() (ctxt->cpuid->feat.vaes)
-#define vcpu_has_vpclmulqdq() (ctxt->cpuid->feat.vpclmulqdq)
-#define vcpu_has_avx512_vnni() (ctxt->cpuid->feat.avx512_vnni)
-#define vcpu_has_avx512_bitalg() (ctxt->cpuid->feat.avx512_bitalg)
-#define vcpu_has_avx512_vpopcntdq() (ctxt->cpuid->feat.avx512_vpopcntdq)
-#define vcpu_has_rdpid() (ctxt->cpuid->feat.rdpid)
-#define vcpu_has_movdiri() (ctxt->cpuid->feat.movdiri)
-#define vcpu_has_movdir64b() (ctxt->cpuid->feat.movdir64b)
-#define vcpu_has_enqcmd() (ctxt->cpuid->feat.enqcmd)
-#define vcpu_has_avx512_4vnniw() (ctxt->cpuid->feat.avx512_4vnniw)
-#define vcpu_has_avx512_4fmaps() (ctxt->cpuid->feat.avx512_4fmaps)
-#define vcpu_has_avx512_vp2intersect() (ctxt->cpuid->feat.avx512_vp2intersect)
-#define vcpu_has_serialize() (ctxt->cpuid->feat.serialize)
-#define vcpu_has_tsxldtrk() (ctxt->cpuid->feat.tsxldtrk)
-#define vcpu_has_avx_vnni() (ctxt->cpuid->feat.avx_vnni)
-#define vcpu_has_avx512_bf16() (ctxt->cpuid->feat.avx512_bf16)
+#define vcpu_has_fpu() (ctxt->cpu_policy->basic.fpu)
+#define vcpu_has_sep() (ctxt->cpu_policy->basic.sep)
+#define vcpu_has_cx8() (ctxt->cpu_policy->basic.cx8)
+#define vcpu_has_cmov() (ctxt->cpu_policy->basic.cmov)
+#define vcpu_has_clflush() (ctxt->cpu_policy->basic.clflush)
+#define vcpu_has_mmx() (ctxt->cpu_policy->basic.mmx)
+#define vcpu_has_fxsr() (ctxt->cpu_policy->basic.fxsr)
+#define vcpu_has_sse() (ctxt->cpu_policy->basic.sse)
+#define vcpu_has_sse2() (ctxt->cpu_policy->basic.sse2)
+#define vcpu_has_sse3() (ctxt->cpu_policy->basic.sse3)
+#define vcpu_has_pclmulqdq() (ctxt->cpu_policy->basic.pclmulqdq)
+#define vcpu_has_ssse3() (ctxt->cpu_policy->basic.ssse3)
+#define vcpu_has_fma() (ctxt->cpu_policy->basic.fma)
+#define vcpu_has_cx16() (ctxt->cpu_policy->basic.cx16)
+#define vcpu_has_sse4_1() (ctxt->cpu_policy->basic.sse4_1)
+#define vcpu_has_sse4_2() (ctxt->cpu_policy->basic.sse4_2)
+#define vcpu_has_movbe() (ctxt->cpu_policy->basic.movbe)
+#define vcpu_has_popcnt() (ctxt->cpu_policy->basic.popcnt)
+#define vcpu_has_aesni() (ctxt->cpu_policy->basic.aesni)
+#define vcpu_has_avx() (ctxt->cpu_policy->basic.avx)
+#define vcpu_has_f16c() (ctxt->cpu_policy->basic.f16c)
+#define vcpu_has_rdrand() (ctxt->cpu_policy->basic.rdrand)
+
+#define vcpu_has_mmxext() (ctxt->cpu_policy->extd.mmxext || vcpu_has_sse())
+#define vcpu_has_3dnow_ext() (ctxt->cpu_policy->extd._3dnowext)
+#define vcpu_has_3dnow() (ctxt->cpu_policy->extd._3dnow)
+#define vcpu_has_lahf_lm() (ctxt->cpu_policy->extd.lahf_lm)
+#define vcpu_has_cr8_legacy() (ctxt->cpu_policy->extd.cr8_legacy)
+#define vcpu_has_lzcnt() (ctxt->cpu_policy->extd.abm)
+#define vcpu_has_sse4a() (ctxt->cpu_policy->extd.sse4a)
+#define vcpu_has_misalignsse() (ctxt->cpu_policy->extd.misalignsse)
+#define vcpu_has_xop() (ctxt->cpu_policy->extd.xop)
+#define vcpu_has_fma4() (ctxt->cpu_policy->extd.fma4)
+#define vcpu_has_tbm() (ctxt->cpu_policy->extd.tbm)
+#define vcpu_has_clzero() (ctxt->cpu_policy->extd.clzero)
+#define vcpu_has_wbnoinvd() (ctxt->cpu_policy->extd.wbnoinvd)
+#define vcpu_has_nscb() (ctxt->cpu_policy->extd.nscb)
+
+#define vcpu_has_bmi1() (ctxt->cpu_policy->feat.bmi1)
+#define vcpu_has_hle() (ctxt->cpu_policy->feat.hle)
+#define vcpu_has_avx2() (ctxt->cpu_policy->feat.avx2)
+#define vcpu_has_bmi2() (ctxt->cpu_policy->feat.bmi2)
+#define vcpu_has_invpcid() (ctxt->cpu_policy->feat.invpcid)
+#define vcpu_has_rtm() (ctxt->cpu_policy->feat.rtm)
+#define vcpu_has_mpx() (ctxt->cpu_policy->feat.mpx)
+#define vcpu_has_avx512f() (ctxt->cpu_policy->feat.avx512f)
+#define vcpu_has_avx512dq() (ctxt->cpu_policy->feat.avx512dq)
+#define vcpu_has_rdseed() (ctxt->cpu_policy->feat.rdseed)
+#define vcpu_has_adx() (ctxt->cpu_policy->feat.adx)
+#define vcpu_has_smap() (ctxt->cpu_policy->feat.smap)
+#define vcpu_has_avx512_ifma() (ctxt->cpu_policy->feat.avx512_ifma)
+#define vcpu_has_clflushopt() (ctxt->cpu_policy->feat.clflushopt)
+#define vcpu_has_clwb() (ctxt->cpu_policy->feat.clwb)
+#define vcpu_has_avx512pf() (ctxt->cpu_policy->feat.avx512pf)
+#define vcpu_has_avx512er() (ctxt->cpu_policy->feat.avx512er)
+#define vcpu_has_avx512cd() (ctxt->cpu_policy->feat.avx512cd)
+#define vcpu_has_sha() (ctxt->cpu_policy->feat.sha)
+#define vcpu_has_avx512bw() (ctxt->cpu_policy->feat.avx512bw)
+#define vcpu_has_avx512vl() (ctxt->cpu_policy->feat.avx512vl)
+#define vcpu_has_avx512_vbmi() (ctxt->cpu_policy->feat.avx512_vbmi)
+#define vcpu_has_avx512_vbmi2() (ctxt->cpu_policy->feat.avx512_vbmi2)
+#define vcpu_has_gfni() (ctxt->cpu_policy->feat.gfni)
+#define vcpu_has_vaes() (ctxt->cpu_policy->feat.vaes)
+#define vcpu_has_vpclmulqdq() (ctxt->cpu_policy->feat.vpclmulqdq)
+#define vcpu_has_avx512_vnni() (ctxt->cpu_policy->feat.avx512_vnni)
+#define vcpu_has_avx512_bitalg() (ctxt->cpu_policy->feat.avx512_bitalg)
+#define vcpu_has_avx512_vpopcntdq() (ctxt->cpu_policy->feat.avx512_vpopcntdq)
+#define vcpu_has_rdpid() (ctxt->cpu_policy->feat.rdpid)
+#define vcpu_has_movdiri() (ctxt->cpu_policy->feat.movdiri)
+#define vcpu_has_movdir64b() (ctxt->cpu_policy->feat.movdir64b)
+#define vcpu_has_enqcmd() (ctxt->cpu_policy->feat.enqcmd)
+#define vcpu_has_avx512_4vnniw() (ctxt->cpu_policy->feat.avx512_4vnniw)
+#define vcpu_has_avx512_4fmaps() (ctxt->cpu_policy->feat.avx512_4fmaps)
+#define vcpu_has_avx512_vp2intersect() (ctxt->cpu_policy->feat.avx512_vp2intersect)
+#define vcpu_has_serialize() (ctxt->cpu_policy->feat.serialize)
+#define vcpu_has_tsxldtrk() (ctxt->cpu_policy->feat.tsxldtrk)
+#define vcpu_has_avx_vnni() (ctxt->cpu_policy->feat.avx_vnni)
+#define vcpu_has_avx512_bf16() (ctxt->cpu_policy->feat.avx512_bf16)
#define vcpu_must_have(feat) \
generate_exception_if(!vcpu_has_##feat(), EXC_UD)
@@ -2078,7 +2078,7 @@ protmode_load_seg(
struct x86_emulate_ctxt *ctxt,
const struct x86_emulate_ops *ops)
{
- const struct cpuid_policy *cp = ctxt->cpuid;
+ const struct cpu_policy *cp = ctxt->cpu_policy;
enum x86_segment sel_seg = (sel & 4) ? x86_seg_ldtr : x86_seg_gdtr;
struct { uint32_t a, b; } desc, desc_hi = {};
uint8_t dpl, rpl;
@@ -5888,7 +5888,7 @@ x86_emulate(
base = ad_bytes == 8 ? _regs.r(ax) :
ad_bytes == 4 ? _regs.eax : _regs.ax;
- limit = ctxt->cpuid->basic.clflush_size * 8;
+ limit = ctxt->cpu_policy->basic.clflush_size * 8;
generate_exception_if(limit < sizeof(long) ||
(limit & (limit - 1)), EXC_UD);
base &= ~(limit - 1);
@@ -6211,7 +6211,7 @@ x86_emulate(
* in fact risking to make guest OSes vulnerable to the equivalent of
* XSA-7 (CVE-2012-0217).
*/
- generate_exception_if(ctxt->cpuid->x86_vendor == X86_VENDOR_INTEL &&
+ generate_exception_if(ctxt->cpu_policy->x86_vendor == X86_VENDOR_INTEL &&
op_bytes == 8 && !is_canonical_address(_regs.rcx),
EXC_GP, 0);
#endif
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h b/xen/arch/x86/x86_emulate/x86_emulate.h
index 75015104fbdb..8c98a398643a 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.h
+++ b/xen/arch/x86/x86_emulate/x86_emulate.h
@@ -565,8 +565,8 @@ struct x86_emulate_ctxt
* Input-only state:
*/
- /* CPUID Policy for the domain. */
- const struct cpuid_policy *cpuid;
+ /* CPU Policy for the domain. */
+ const struct cpu_policy *cpu_policy;
/* Set this if writes may have side effects. */
bool force_writeback;
diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index d481e1db3e7e..22038a251f6d 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -684,7 +684,7 @@ void xstate_init(struct cpuinfo_x86 *c)
int validate_xstate(const struct domain *d, uint64_t xcr0, uint64_t xcr0_accum,
const struct xsave_hdr *hdr)
{
- uint64_t xcr0_max = cpuid_policy_xcr0_max(d->arch.cpuid);
+ uint64_t xcr0_max = cpuid_policy_xcr0_max(d->arch.cpu_policy);
unsigned int i;
if ( (hdr->xstate_bv & ~xcr0_accum) ||
@@ -708,7 +708,7 @@ int validate_xstate(const struct domain *d, uint64_t xcr0, uint64_t xcr0_accum,
int handle_xsetbv(u32 index, u64 new_bv)
{
struct vcpu *curr = current;
- uint64_t xcr0_max = cpuid_policy_xcr0_max(curr->domain->arch.cpuid);
+ uint64_t xcr0_max = cpuid_policy_xcr0_max(curr->domain->arch.cpu_policy);
u64 mask;
if ( index != XCR_XFEATURE_ENABLED_MASK )
diff --git a/xen/include/xen/lib/x86/cpu-policy.h b/xen/include/xen/lib/x86/cpu-policy.h
index 51f88f1e217e..82b4c1c0546f 100644
--- a/xen/include/xen/lib/x86/cpu-policy.h
+++ b/xen/include/xen/lib/x86/cpu-policy.h
@@ -94,7 +94,7 @@ const char *x86_cpuid_vendor_to_str(unsigned int vendor);
CPUID_GUEST_NR_EXTD_AMD)
/*
- * Maximum number of leaves a struct cpuid_policy turns into when serialised
+ * Maximum number of leaves a struct cpu_policy turns into when serialised
* for interaction with the toolstack. (Sum of all leaves in each union, less
* the entries in basic which sub-unions hang off of.)
*/
@@ -375,10 +375,6 @@ struct cpu_policy
uint8_t x86_vendor;
};
-/* Temporary */
-#define cpuid_policy cpu_policy
-#define msr_policy cpu_policy
-
struct cpu_policy_errors
{
uint32_t leaf, subleaf;
@@ -389,7 +385,7 @@ struct cpu_policy_errors
/* Fill in a featureset bitmap from a CPUID policy. */
static inline void cpuid_policy_to_featureset(
- const struct cpuid_policy *p, uint32_t fs[FEATURESET_NR_ENTRIES])
+ const struct cpu_policy *p, uint32_t fs[FEATURESET_NR_ENTRIES])
{
fs[FEATURESET_1d] = p->basic._1d;
fs[FEATURESET_1c] = p->basic._1c;
@@ -411,7 +407,7 @@ static inline void cpuid_policy_to_featureset(
/* Fill in a CPUID policy from a featureset bitmap. */
static inline void cpuid_featureset_to_policy(
- const uint32_t fs[FEATURESET_NR_ENTRIES], struct cpuid_policy *p)
+ const uint32_t fs[FEATURESET_NR_ENTRIES], struct cpu_policy *p)
{
p->basic._1d = fs[FEATURESET_1d];
p->basic._1c = fs[FEATURESET_1c];
@@ -431,12 +427,12 @@ static inline void cpuid_featureset_to_policy(
p->feat._7d1 = fs[FEATURESET_7d1];
}
-static inline uint64_t cpuid_policy_xcr0_max(const struct cpuid_policy *p)
+static inline uint64_t cpuid_policy_xcr0_max(const struct cpu_policy *p)
{
return ((uint64_t)p->xstate.xcr0_high << 32) | p->xstate.xcr0_low;
}
-static inline uint64_t cpuid_policy_xstates(const struct cpuid_policy *p)
+static inline uint64_t cpuid_policy_xstates(const struct cpu_policy *p)
{
uint64_t val = p->xstate.xcr0_high | p->xstate.xss_high;
@@ -448,7 +444,7 @@ const uint32_t *x86_cpuid_lookup_deep_deps(uint32_t feature);
/**
* Recalculate the content in a CPUID policy which is derived from raw data.
*/
-void x86_cpuid_policy_recalc_synth(struct cpuid_policy *p);
+void x86_cpuid_policy_recalc_synth(struct cpu_policy *p);
/**
* Fill a CPUID policy using the native CPUID instruction.
@@ -457,7 +453,7 @@ void x86_cpuid_policy_recalc_synth(struct cpuid_policy *p);
* Values may be influenced by a hypervisor or from masking/faulting
* configuration.
*/
-void x86_cpuid_policy_fill_native(struct cpuid_policy *p);
+void x86_cpuid_policy_fill_native(struct cpu_policy *p);
/**
* Clear leaf data beyond the policies max leaf/subleaf settings.
@@ -468,7 +464,7 @@ void x86_cpuid_policy_fill_native(struct cpuid_policy *p);
* with out-of-range leaves with stale content in them. This helper clears
* them.
*/
-void x86_cpuid_policy_clear_out_of_range_leaves(struct cpuid_policy *p);
+void x86_cpuid_policy_clear_out_of_range_leaves(struct cpu_policy *p);
#ifdef __XEN__
#include <public/arch-x86/xen.h>
@@ -492,7 +488,7 @@ typedef xen_msr_entry_t msr_entry_buffer_t[];
* leaves array is too short. On success, nr_entries is updated with the
* actual number of leaves written.
*/
-int x86_cpuid_copy_to_buffer(const struct cpuid_policy *policy,
+int x86_cpuid_copy_to_buffer(const struct cpu_policy *policy,
cpuid_leaf_buffer_t leaves, uint32_t *nr_entries);
/**
@@ -512,7 +508,7 @@ int x86_cpuid_copy_to_buffer(const struct cpuid_policy *policy,
* No content validation of in-range leaves is performed. Synthesised data is
* recalculated.
*/
-int x86_cpuid_copy_from_buffer(struct cpuid_policy *policy,
+int x86_cpuid_copy_from_buffer(struct cpu_policy *policy,
const cpuid_leaf_buffer_t leaves,
uint32_t nr_entries, uint32_t *err_leaf,
uint32_t *err_subleaf);
@@ -529,7 +525,7 @@ int x86_cpuid_copy_from_buffer(struct cpuid_policy *policy,
* buffer array is too short. On success, nr_entries is updated with the
* actual number of msrs written.
*/
-int x86_msr_copy_to_buffer(const struct msr_policy *policy,
+int x86_msr_copy_to_buffer(const struct cpu_policy *policy,
msr_entry_buffer_t msrs, uint32_t *nr_entries);
/**
@@ -549,7 +545,7 @@ int x86_msr_copy_to_buffer(const struct msr_policy *policy,
*
* No content validation is performed on the data stored in the policy object.
*/
-int x86_msr_copy_from_buffer(struct msr_policy *policy,
+int x86_msr_copy_from_buffer(struct cpu_policy *policy,
const msr_entry_buffer_t msrs, uint32_t nr_entries,
uint32_t *err_msr);
diff --git a/xen/lib/x86/cpuid.c b/xen/lib/x86/cpuid.c
index e81f76c779c0..58a14163b1cb 100644
--- a/xen/lib/x86/cpuid.c
+++ b/xen/lib/x86/cpuid.c
@@ -60,13 +60,13 @@ const char *x86_cpuid_vendor_to_str(unsigned int vendor)
}
}
-void x86_cpuid_policy_recalc_synth(struct cpuid_policy *p)
+void x86_cpuid_policy_recalc_synth(struct cpu_policy *p)
{
p->x86_vendor = x86_cpuid_lookup_vendor(
p->basic.vendor_ebx, p->basic.vendor_ecx, p->basic.vendor_edx);
}
-void x86_cpuid_policy_fill_native(struct cpuid_policy *p)
+void x86_cpuid_policy_fill_native(struct cpu_policy *p)
{
unsigned int i;
@@ -183,7 +183,7 @@ void x86_cpuid_policy_fill_native(struct cpuid_policy *p)
x86_cpuid_policy_recalc_synth(p);
}
-void x86_cpuid_policy_clear_out_of_range_leaves(struct cpuid_policy *p)
+void x86_cpuid_policy_clear_out_of_range_leaves(struct cpu_policy *p)
{
unsigned int i;
@@ -291,7 +291,7 @@ static int copy_leaf_to_buffer(uint32_t leaf, uint32_t subleaf,
return 0;
}
-int x86_cpuid_copy_to_buffer(const struct cpuid_policy *p,
+int x86_cpuid_copy_to_buffer(const struct cpu_policy *p,
cpuid_leaf_buffer_t leaves, uint32_t *nr_entries_p)
{
const uint32_t nr_entries = *nr_entries_p;
@@ -377,7 +377,7 @@ int x86_cpuid_copy_to_buffer(const struct cpuid_policy *p,
return 0;
}
-int x86_cpuid_copy_from_buffer(struct cpuid_policy *p,
+int x86_cpuid_copy_from_buffer(struct cpu_policy *p,
const cpuid_leaf_buffer_t leaves,
uint32_t nr_entries, uint32_t *err_leaf,
uint32_t *err_subleaf)
diff --git a/xen/lib/x86/msr.c b/xen/lib/x86/msr.c
index c4d885e7b568..e04b9ca01302 100644
--- a/xen/lib/x86/msr.c
+++ b/xen/lib/x86/msr.c
@@ -23,7 +23,7 @@ static int copy_msr_to_buffer(uint32_t idx, uint64_t val,
return 0;
}
-int x86_msr_copy_to_buffer(const struct msr_policy *p,
+int x86_msr_copy_to_buffer(const struct cpu_policy *p,
msr_entry_buffer_t msrs, uint32_t *nr_entries_p)
{
const uint32_t nr_entries = *nr_entries_p;
@@ -48,7 +48,7 @@ int x86_msr_copy_to_buffer(const struct msr_policy *p,
return 0;
}
-int x86_msr_copy_from_buffer(struct msr_policy *p,
+int x86_msr_copy_from_buffer(struct cpu_policy *p,
const msr_entry_buffer_t msrs, uint32_t nr_entries,
uint32_t *err_msr)
{
--
2.30.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 4/9] x86: Merge struct msr_policy into struct cpu_policy
2023-03-29 20:51 ` [PATCH 4/9] x86: Merge struct msr_policy into " Andrew Cooper
@ 2023-03-30 9:30 ` Jan Beulich
2023-03-30 11:11 ` Andrew Cooper
0 siblings, 1 reply; 34+ messages in thread
From: Jan Beulich @ 2023-03-30 9:30 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Roger Pau Monné, Wei Liu, Xen-devel
On 29.03.2023 22:51, Andrew Cooper wrote:
> --- a/xen/include/xen/lib/x86/cpu-policy.h
> +++ b/xen/include/xen/lib/x86/cpu-policy.h
> @@ -3,7 +3,6 @@
> #define XEN_LIB_X86_POLICIES_H
>
> #include <xen/lib/x86/cpuid-autogen.h>
> -#include <xen/lib/x86/msr.h>
>
> #define FEATURESET_1d 0 /* 0x00000001.edx */
> #define FEATURESET_1c 1 /* 0x00000001.ecx */
> @@ -107,6 +106,9 @@ const char *x86_cpuid_vendor_to_str(unsigned int vendor);
> CPUID_GUEST_NR_XSTATE - !!CPUID_GUEST_NR_XSTATE + \
> CPUID_GUEST_NR_EXTD + 2 /* hv_limit and hv2_limit */ )
>
> +/* Maximum number of MSRs written when serialising msr_policy. */
> +#define MSR_MAX_SERIALISED_ENTRIES 2
The comment better wouldn't refer to msr_policy anymore, I think. I also
wonder whether the comment wouldn't better move ...
> @@ -324,6 +326,44 @@ struct cpu_policy
> };
> } extd;
>
> + /*
> + * 0x000000ce - MSR_INTEL_PLATFORM_INFO
... e.g. above here, to increase the chance of it being spotted that
it needs updating if another MSR is added here.
Jan
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 5/9] x86: Merge the system {cpuid,msr} policy objects
2023-03-29 20:51 ` [PATCH 5/9] x86: Merge the system {cpuid,msr} policy objects Andrew Cooper
@ 2023-03-30 9:40 ` Jan Beulich
2023-03-30 11:19 ` Andrew Cooper
0 siblings, 1 reply; 34+ messages in thread
From: Jan Beulich @ 2023-03-30 9:40 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Roger Pau Monné, Wei Liu, Xen-devel
On 29.03.2023 22:51, Andrew Cooper wrote:
> Right now, they're the same underlying type, containing disjoint information.
>
> Introduce a new cpu-policy.{h,c} to be the new location for all policy
> handling logic. Place the combined objects in __ro_after_init, which has
> appeared since the original logic was written.
>
> As we're trying to phase out the use of struct old_cpu_policy entirely, rework
> update_domain_cpu_policy() to not pointer-chase through system_policies[].
>
> This in turn allows system_policies[] in sysctl.c to become static and reduced
> in scope to XEN_SYSCTL_get_cpu_policy.
>
> No practical change, but we do half the amount of compile time space required
> for the system policies, which saves 6x almost-2k at the moment.
But what you save here is only what was introduced earlier in the series,
if I'm not mistaken (just like the "containing disjoint" further up)? Also
do you mean "halve" in place of "half"?
> @@ -391,7 +360,19 @@ long arch_do_sysctl(
>
> case XEN_SYSCTL_get_cpu_policy:
> {
> - const struct old_cpu_policy *policy;
> + static const struct cpu_policy *system_policies[6] = {
Another const wanted here after *?
Jan
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 6/9] x86: Merge a domain's {cpuid,msr} policy objects
2023-03-29 20:51 ` [PATCH 6/9] x86: Merge a domain's " Andrew Cooper
@ 2023-03-30 10:00 ` Jan Beulich
2023-03-30 11:14 ` Andrew Cooper
0 siblings, 1 reply; 34+ messages in thread
From: Jan Beulich @ 2023-03-30 10:00 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Roger Pau Monné, Wei Liu, Xen-devel
On 29.03.2023 22:51, Andrew Cooper wrote:
> @@ -573,7 +574,6 @@ int arch_vcpu_create(struct vcpu *v)
> /* Idle domain */
> v->arch.cr3 = __pa(idle_pg_table);
> rc = 0;
> - v->arch.msrs = ZERO_BLOCK_PTR; /* Catch stray misuses */
> }
Is this intentional? It's a vCPU pointer here, not a domain one.
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -40,11 +40,11 @@
> static int update_domain_cpu_policy(struct domain *d,
> xen_domctl_cpu_policy_t *xdpc)
> {
> - struct old_cpu_policy new = {};
> + struct cpu_policy *new;
> struct cpu_policy *sys = is_pv_domain(d)
> ? (IS_ENABLED(CONFIG_PV) ? &pv_max_cpu_policy : NULL)
> : (IS_ENABLED(CONFIG_HVM) ? &hvm_max_cpu_policy : NULL);
> - struct old_cpu_policy old_sys = { sys, sys };
> + struct old_cpu_policy old_sys = { sys, sys }, old_new;
Interesting name, but as long as it's transitional only, that's of course
fine.
Jan
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 7/9] x86: Merge xc_cpu_policy's cpuid and msr objects
2023-03-29 20:51 ` [PATCH 7/9] x86: Merge xc_cpu_policy's cpuid and msr objects Andrew Cooper
@ 2023-03-30 10:04 ` Jan Beulich
2023-03-30 11:56 ` Andrew Cooper
0 siblings, 1 reply; 34+ messages in thread
From: Jan Beulich @ 2023-03-30 10:04 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Roger Pau Monné, Wei Liu, Xen-devel
On 29.03.2023 22:51, Andrew Cooper wrote:
> /* Sanity test various invariants we expect in the default/max policies. */
> static void test_guest_policies(const struct xc_cpu_policy *max,
> const struct xc_cpu_policy *def)
> {
> - const struct cpuid_policy *cm = &max->cpuid;
> - const struct cpuid_policy *cd = &def->cpuid;
> - const struct msr_policy *mm = &max->msr;
> - const struct msr_policy *md = &def->msr;
> + const struct cpu_policy *m = &max->policy;
> + const struct cpu_policy *d = &def->policy;
Looks like you could reduce code churn by keeping "cm" and "cd" as the
names, which would also be consistent with you continuing to use "cp"
in hypervisor code.
Jan
> dump_tsx_details(max, "Max:");
> dump_tsx_details(def, "Def:");
>
> - if ( ((cm->feat.raw[0].d | cd->feat.raw[0].d) &
> + if ( ((m->feat.raw[0].d | d->feat.raw[0].d) &
> (bitmaskof(X86_FEATURE_TSX_FORCE_ABORT) |
> bitmaskof(X86_FEATURE_RTM_ALWAYS_ABORT) |
> bitmaskof(X86_FEATURE_SRBDS_CTRL))) ||
> - ((mm->arch_caps.raw | md->arch_caps.raw) & ARCH_CAPS_TSX_CTRL) )
> + ((m->arch_caps.raw | d->arch_caps.raw) & ARCH_CAPS_TSX_CTRL) )
> fail(" Xen-only TSX controls offered to guest\n");
>
> switch ( rtm_behaviour )
> {
> case RTM_UD:
> - if ( (cm->feat.raw[0].b | cd->feat.raw[0].b) &
> + if ( (m->feat.raw[0].b | d->feat.raw[0].b) &
> (bitmaskof(X86_FEATURE_HLE) | bitmaskof(X86_FEATURE_RTM)) )
> fail(" HLE/RTM offered to guests despite not being available\n");
> break;
>
> case RTM_ABORT:
> - if ( cd->feat.raw[0].b &
> + if ( d->feat.raw[0].b &
> (bitmaskof(X86_FEATURE_HLE) | bitmaskof(X86_FEATURE_RTM)) )
> fail(" HLE/RTM offered to guests by default despite not being usable\n");
> break;
>
> case RTM_OK:
> - if ( !cm->feat.rtm || !cd->feat.rtm )
> + if ( !m->feat.rtm || !d->feat.rtm )
> fail(" RTM not offered to guests despite being available\n");
> break;
> }
>
> - if ( cd->feat.hle )
> + if ( d->feat.hle )
> fail(" Fail: HLE offered in default policy\n");
> }
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 8/9] x86: Drop struct old_cpu_policy
2023-03-29 20:51 ` [PATCH 8/9] x86: Drop struct old_cpu_policy Andrew Cooper
@ 2023-03-30 10:08 ` Jan Beulich
2023-03-30 11:57 ` Andrew Cooper
0 siblings, 1 reply; 34+ messages in thread
From: Jan Beulich @ 2023-03-30 10:08 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Roger Pau Monné, Wei Liu, Xen-devel
On 29.03.2023 22:51, Andrew Cooper wrote:
> --- a/tools/libs/guest/xg_cpuid_x86.c
> +++ b/tools/libs/guest/xg_cpuid_x86.c
> @@ -868,9 +868,7 @@ bool xc_cpu_policy_is_compatible(xc_interface *xch, xc_cpu_policy_t *host,
> xc_cpu_policy_t *guest)
> {
> struct cpu_policy_errors err = INIT_CPU_POLICY_ERRORS;
> - struct old_cpu_policy h = { &host->policy, &host->policy };
> - struct old_cpu_policy g = { &guest->policy, &guest->policy };
> - int rc = x86_cpu_policies_are_compatible(&h, &g, &err);
> + int rc = x86_cpu_policies_are_compatible(&host->policy, &host->policy, &err);
One of the two surely wants to be &guest->policy?
Jan
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 9/9] RFC: Everything else
2023-03-29 20:51 ` [PATCH 9/9] RFC: Everything else Andrew Cooper
@ 2023-03-30 10:16 ` Jan Beulich
2023-03-30 12:01 ` Andrew Cooper
0 siblings, 1 reply; 34+ messages in thread
From: Jan Beulich @ 2023-03-30 10:16 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Roger Pau Monné, Wei Liu, Xen-devel
On 29.03.2023 22:51, Andrew Cooper wrote:
> Looking at this diff, I'm wondering whether keeping
>
> union {
> struct cpu_policy *cpuid;
> struct cpu_policy *cpu_policy;
> };
>
> permentantly might be a good idea, because d->arch.cpuid->$X reads rather
> better than d->arch.cpu_policy->$X
>
> Thoughts?
I'm not overly fussed, but perhaps yes. Nevertheless e.g. ...
> --- a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
> +++ b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
> @@ -893,7 +893,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data_p, size_t size)
> struct x86_emulate_ctxt ctxt = {
> .data = &state,
> .regs = &input.regs,
> - .cpuid = &cp,
> + .cpu_policy = &cp,
... this and ...
> --- a/tools/tests/x86_emulator/test_x86_emulator.c
> +++ b/tools/tests/x86_emulator/test_x86_emulator.c
> @@ -909,7 +909,7 @@ int main(int argc, char **argv)
>
> ctxt.regs = ®s;
> ctxt.force_writeback = 0;
> - ctxt.cpuid = &cp;
> + ctxt.cpu_policy = &cp;
... this imo want keeping as you have it here.
Jan
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH RFC 0/9] x86: Merge cpuid and msr policy
2023-03-29 20:51 [PATCH RFC 0/9] x86: Merge cpuid and msr policy Andrew Cooper
` (8 preceding siblings ...)
2023-03-29 20:51 ` [PATCH 9/9] RFC: Everything else Andrew Cooper
@ 2023-03-30 10:18 ` Jan Beulich
2023-03-30 12:08 ` Andrew Cooper
2023-03-30 11:07 ` Roger Pau Monné
10 siblings, 1 reply; 34+ messages in thread
From: Jan Beulich @ 2023-03-30 10:18 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Roger Pau Monné, Wei Liu, Xen-devel
On 29.03.2023 22:51, Andrew Cooper wrote:
> tl;dr to add MSR_ARCH_CAPS features sensibly, cpu_{featureset<->policy}() need
> to not operate on objects of differing lifetimes, so structs
> {cpuid,msr}_policy need merging and cpu_policy is the obvious name.
>
> But this does mean that we now have
>
> cpu_policy->basic.$X
> cpu_policy->feat.$Y
> cpu_policy->arch_caps.$Z
>
> and plenty of code now reads
>
> d->arch.cpu_policy->feat.$Y
>
> instead of
>
> d->arch.cpuid->feat.$Y
>
> The latter can be half-fixed with some union magic (see patch 9 commit
> message). The former can be fixed by putting cpuid/msr infixes in cpu_policy,
> which is doable but very invasive, and would make plenty of code read
>
> d->arch.cpu_policy->cpuid.feat.$Y
>
> and the two obviously shouldn't be done together.
>
> So, RFC. Does this code layout look ok? If we want to make changes with
> naming, now is very much the right time to get them sorted.
>
> Patches 1-8 are pretty ready to go. Patch 9 is the remainder to take out the
> temporary hacks, and I'm still in the process of merging the system policy
> derivation.
>
> Andrew Cooper (9):
> x86: Rename struct cpu_policy to struct old_cpuid_policy
> x86: Rename {domctl,sysctl}.cpu_policy.{cpuid,msr_policy} fields
Nit: I guess the last closing brace wants moving forward a little.
> x86: Rename struct cpuid_policy to struct cpu_policy
> x86: Merge struct msr_policy into struct cpu_policy
> x86: Merge the system {cpuid,msr} policy objects
> x86: Merge a domain's {cpuid,msr} policy objects
> x86: Merge xc_cpu_policy's cpuid and msr objects
> x86: Drop struct old_cpu_policy
With the small comments on individual patches taken care of one way or
another, up to here:
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan
> RFC: Everything else
>
> tools/fuzz/cpu-policy/afl-policy-fuzzer.c | 15 +-
> .../fuzz/x86_instruction_emulator/fuzz-emul.c | 2 +-
> tools/libs/guest/xg_cpuid_x86.c | 48 +-
> tools/libs/guest/xg_private.h | 5 +-
> tools/tests/cpu-policy/test-cpu-policy.c | 50 +-
> tools/tests/tsx/test-tsx.c | 58 +-
> tools/tests/x86_emulator/Makefile | 2 +-
> tools/tests/x86_emulator/test_x86_emulator.c | 2 +-
> tools/tests/x86_emulator/x86-emulate.c | 2 +-
> tools/tests/x86_emulator/x86-emulate.h | 2 +-
> xen/arch/x86/Makefile | 1 +
> xen/arch/x86/cpu-policy.c | 67 +++
> xen/arch/x86/cpu/common.c | 4 +-
> xen/arch/x86/cpu/mcheck/mce_intel.c | 2 +-
> xen/arch/x86/cpu/vpmu_intel.c | 4 +-
> xen/arch/x86/cpuid.c | 101 ++--
> xen/arch/x86/domain.c | 18 +-
> xen/arch/x86/domctl.c | 51 +-
> xen/arch/x86/hvm/emulate.c | 2 +-
> xen/arch/x86/hvm/hvm.c | 38 +-
> xen/arch/x86/hvm/ioreq.c | 4 +-
> xen/arch/x86/hvm/mtrr.c | 2 +-
> xen/arch/x86/hvm/svm/svm.c | 18 +-
> xen/arch/x86/hvm/svm/svmdebug.c | 2 +-
> xen/arch/x86/hvm/vlapic.c | 2 +-
> xen/arch/x86/hvm/vmx/vmx.c | 12 +-
> xen/arch/x86/hvm/vmx/vvmx.c | 2 +-
> xen/arch/x86/include/asm/cpu-policy.h | 18 +
> xen/arch/x86/include/asm/cpuid.h | 10 -
> xen/arch/x86/include/asm/domain.h | 4 +-
> xen/arch/x86/include/asm/guest_pt.h | 4 +-
> xen/arch/x86/include/asm/msr.h | 13 +-
> xen/arch/x86/include/asm/paging.h | 2 +-
> xen/arch/x86/mm/mem_sharing.c | 3 +-
> xen/arch/x86/mm/shadow/hvm.c | 2 +-
> xen/arch/x86/msr.c | 98 +---
> xen/arch/x86/pv/domain.c | 2 +-
> xen/arch/x86/pv/emul-priv-op.c | 6 +-
> xen/arch/x86/pv/ro-page-fault.c | 2 +-
> xen/arch/x86/sysctl.c | 77 +--
> xen/arch/x86/traps.c | 2 +-
> xen/arch/x86/x86_emulate.c | 2 +-
> xen/arch/x86/x86_emulate/x86_emulate.c | 166 +++---
> xen/arch/x86/x86_emulate/x86_emulate.h | 6 +-
> xen/arch/x86/xstate.c | 4 +-
> xen/include/public/domctl.h | 4 +-
> xen/include/public/sysctl.h | 4 +-
> xen/include/xen/lib/x86/cpu-policy.h | 540 +++++++++++++++++-
> xen/include/xen/lib/x86/cpuid.h | 475 ---------------
> xen/include/xen/lib/x86/msr.h | 104 ----
> xen/lib/x86/cpuid.c | 12 +-
> xen/lib/x86/msr.c | 6 +-
> xen/lib/x86/policy.c | 8 +-
> 53 files changed, 986 insertions(+), 1104 deletions(-)
> create mode 100644 xen/arch/x86/cpu-policy.c
> create mode 100644 xen/arch/x86/include/asm/cpu-policy.h
> delete mode 100644 xen/include/xen/lib/x86/cpuid.h
> delete mode 100644 xen/include/xen/lib/x86/msr.h
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH RFC 0/9] x86: Merge cpuid and msr policy
2023-03-29 20:51 [PATCH RFC 0/9] x86: Merge cpuid and msr policy Andrew Cooper
` (9 preceding siblings ...)
2023-03-30 10:18 ` [PATCH RFC 0/9] x86: Merge cpuid and msr policy Jan Beulich
@ 2023-03-30 11:07 ` Roger Pau Monné
2023-03-30 12:59 ` Andrew Cooper
10 siblings, 1 reply; 34+ messages in thread
From: Roger Pau Monné @ 2023-03-30 11:07 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Jan Beulich, Wei Liu
On Wed, Mar 29, 2023 at 09:51:28PM +0100, Andrew Cooper wrote:
> tl;dr to add MSR_ARCH_CAPS features sensibly, cpu_{featureset<->policy}() need
> to not operate on objects of differing lifetimes, so structs
> {cpuid,msr}_policy need merging and cpu_policy is the obvious name.
So the problem is that there's a chance we might get a cpu_policy
object that contains a valid (allocated) cpuid object, but not an msr
one?
Or the problem is rather with the domain struct containing separate
cpuid/msr fields not encapsulated in the cpu_policy struct?
I don't think the current set of cpu_policy operations permit you to
operate on incomplete objects anyway.
I assume you are worried about the usage of x86_msr_copy_from_buffer()
for example that could load data into the MSR_ARCH_CAPS field for the
msr object without checking that the corresponding CPUID bit is set?
> But this does mean that we now have
>
> cpu_policy->basic.$X
> cpu_policy->feat.$Y
> cpu_policy->arch_caps.$Z
I'm not sure I like the fact that we now can't differentiate between
policy fields related to MSRs or CPUID leafs.
Isn't there a chance we might in the future get some name space
collision by us having decided to unify both?
Thanks, Roger.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 4/9] x86: Merge struct msr_policy into struct cpu_policy
2023-03-30 9:30 ` Jan Beulich
@ 2023-03-30 11:11 ` Andrew Cooper
2023-03-30 11:15 ` Jan Beulich
0 siblings, 1 reply; 34+ messages in thread
From: Andrew Cooper @ 2023-03-30 11:11 UTC (permalink / raw)
To: Jan Beulich; +Cc: Roger Pau Monné, Wei Liu, Xen-devel
On 30/03/2023 10:30 am, Jan Beulich wrote:
> On 29.03.2023 22:51, Andrew Cooper wrote:
>> --- a/xen/include/xen/lib/x86/cpu-policy.h
>> +++ b/xen/include/xen/lib/x86/cpu-policy.h
>> @@ -3,7 +3,6 @@
>> #define XEN_LIB_X86_POLICIES_H
>>
>> #include <xen/lib/x86/cpuid-autogen.h>
>> -#include <xen/lib/x86/msr.h>
>>
>> #define FEATURESET_1d 0 /* 0x00000001.edx */
>> #define FEATURESET_1c 1 /* 0x00000001.ecx */
>> @@ -107,6 +106,9 @@ const char *x86_cpuid_vendor_to_str(unsigned int vendor);
>> CPUID_GUEST_NR_XSTATE - !!CPUID_GUEST_NR_XSTATE + \
>> CPUID_GUEST_NR_EXTD + 2 /* hv_limit and hv2_limit */ )
>>
>> +/* Maximum number of MSRs written when serialising msr_policy. */
>> +#define MSR_MAX_SERIALISED_ENTRIES 2
> The comment better wouldn't refer to msr_policy anymore, I think.
Ah yes. (There's so much comment and library cleanup still to do)
> I also
> wonder whether the comment wouldn't better move ...
>
>> @@ -324,6 +326,44 @@ struct cpu_policy
>> };
>> } extd;
>>
>> + /*
>> + * 0x000000ce - MSR_INTEL_PLATFORM_INFO
> ... e.g. above here, to increase the chance of it being spotted that
> it needs updating if another MSR is added here.
I'm not sure about that. In its current position, it's next to it's
CPUID partner.
The unit tests in test-cpu-policy cross-check that we never get -ENOBUFS
for a MSR_MAX_* sized destination, so I'm not worried about it actually
getting stale.
But, with the new merged policies, I need to change the serialising
behaviour anyway.
Right now we serialise everything unconditionally because that's the
only way that merging incremental deltas could be made to work, but now
the MSR enumeration bits are in the same structure we can use those
instead, along with the various "clear" passes we already have.
~Andrew
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 6/9] x86: Merge a domain's {cpuid,msr} policy objects
2023-03-30 10:00 ` Jan Beulich
@ 2023-03-30 11:14 ` Andrew Cooper
0 siblings, 0 replies; 34+ messages in thread
From: Andrew Cooper @ 2023-03-30 11:14 UTC (permalink / raw)
To: Jan Beulich; +Cc: Roger Pau Monné, Wei Liu, Xen-devel
On 30/03/2023 11:00 am, Jan Beulich wrote:
> On 29.03.2023 22:51, Andrew Cooper wrote:
>> @@ -573,7 +574,6 @@ int arch_vcpu_create(struct vcpu *v)
>> /* Idle domain */
>> v->arch.cr3 = __pa(idle_pg_table);
>> rc = 0;
>> - v->arch.msrs = ZERO_BLOCK_PTR; /* Catch stray misuses */
>> }
> Is this intentional? It's a vCPU pointer here, not a domain one.
Ah, no. And it answers one of my TODO notes that I hadn't got to yet
(of why MSRs was different to CPUID in this case).
It looks like it got caught in my `arch.msrs` rework. I'll drop this hunk.
>
>> --- a/xen/arch/x86/domctl.c
>> +++ b/xen/arch/x86/domctl.c
>> @@ -40,11 +40,11 @@
>> static int update_domain_cpu_policy(struct domain *d,
>> xen_domctl_cpu_policy_t *xdpc)
>> {
>> - struct old_cpu_policy new = {};
>> + struct cpu_policy *new;
>> struct cpu_policy *sys = is_pv_domain(d)
>> ? (IS_ENABLED(CONFIG_PV) ? &pv_max_cpu_policy : NULL)
>> : (IS_ENABLED(CONFIG_HVM) ? &hvm_max_cpu_policy : NULL);
>> - struct old_cpu_policy old_sys = { sys, sys };
>> + struct old_cpu_policy old_sys = { sys, sys }, old_new;
> Interesting name, but as long as it's transitional only, that's of course
> fine.
Yeah... it was the best I could come up with.
It does get removed in patch 8.
~Andrew
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 4/9] x86: Merge struct msr_policy into struct cpu_policy
2023-03-30 11:11 ` Andrew Cooper
@ 2023-03-30 11:15 ` Jan Beulich
0 siblings, 0 replies; 34+ messages in thread
From: Jan Beulich @ 2023-03-30 11:15 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Roger Pau Monné, Wei Liu, Xen-devel
On 30.03.2023 13:11, Andrew Cooper wrote:
> On 30/03/2023 10:30 am, Jan Beulich wrote:
>> On 29.03.2023 22:51, Andrew Cooper wrote:
>>> --- a/xen/include/xen/lib/x86/cpu-policy.h
>>> +++ b/xen/include/xen/lib/x86/cpu-policy.h
>>> @@ -3,7 +3,6 @@
>>> #define XEN_LIB_X86_POLICIES_H
>>>
>>> #include <xen/lib/x86/cpuid-autogen.h>
>>> -#include <xen/lib/x86/msr.h>
>>>
>>> #define FEATURESET_1d 0 /* 0x00000001.edx */
>>> #define FEATURESET_1c 1 /* 0x00000001.ecx */
>>> @@ -107,6 +106,9 @@ const char *x86_cpuid_vendor_to_str(unsigned int vendor);
>>> CPUID_GUEST_NR_XSTATE - !!CPUID_GUEST_NR_XSTATE + \
>>> CPUID_GUEST_NR_EXTD + 2 /* hv_limit and hv2_limit */ )
>>>
>>> +/* Maximum number of MSRs written when serialising msr_policy. */
>>> +#define MSR_MAX_SERIALISED_ENTRIES 2
>> The comment better wouldn't refer to msr_policy anymore, I think.
>
> Ah yes. (There's so much comment and library cleanup still to do)
>
>> I also
>> wonder whether the comment wouldn't better move ...
>>
>>> @@ -324,6 +326,44 @@ struct cpu_policy
>>> };
>>> } extd;
>>>
>>> + /*
>>> + * 0x000000ce - MSR_INTEL_PLATFORM_INFO
>> ... e.g. above here, to increase the chance of it being spotted that
>> it needs updating if another MSR is added here.
>
> I'm not sure about that. In its current position, it's next to it's
> CPUID partner.
>
> The unit tests in test-cpu-policy cross-check that we never get -ENOBUFS
> for a MSR_MAX_* sized destination, so I'm not worried about it actually
> getting stale.
Well, I wasn't worried so much about ending up with a bad commit, but
rather that something you could spot on the first pass while writing
new code might cause you a build or test failure later on, resulting
in extra hunting. But this was merely a thought, nothing I would insist
one (the more that your CPUID sibling argument is of course also
relevant).
Jan
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 5/9] x86: Merge the system {cpuid,msr} policy objects
2023-03-30 9:40 ` Jan Beulich
@ 2023-03-30 11:19 ` Andrew Cooper
0 siblings, 0 replies; 34+ messages in thread
From: Andrew Cooper @ 2023-03-30 11:19 UTC (permalink / raw)
To: Jan Beulich; +Cc: Roger Pau Monné, Wei Liu, Xen-devel
On 30/03/2023 10:40 am, Jan Beulich wrote:
> On 29.03.2023 22:51, Andrew Cooper wrote:
>> Right now, they're the same underlying type, containing disjoint information.
>>
>> Introduce a new cpu-policy.{h,c} to be the new location for all policy
>> handling logic. Place the combined objects in __ro_after_init, which has
>> appeared since the original logic was written.
>>
>> As we're trying to phase out the use of struct old_cpu_policy entirely, rework
>> update_domain_cpu_policy() to not pointer-chase through system_policies[].
>>
>> This in turn allows system_policies[] in sysctl.c to become static and reduced
>> in scope to XEN_SYSCTL_get_cpu_policy.
>>
>> No practical change, but we do half the amount of compile time space required
>> for the system policies, which saves 6x almost-2k at the moment.
> But what you save here is only what was introduced earlier in the series,
> if I'm not mistaken (just like the "containing disjoint" further up)?
Hmm yeah - I should reword to make it clearer that this is returning to
how it was before the series.
> Also do you mean "halve" in place of "half"?
Yes.
>
>> @@ -391,7 +360,19 @@ long arch_do_sysctl(
>>
>> case XEN_SYSCTL_get_cpu_policy:
>> {
>> - const struct old_cpu_policy *policy;
>> + static const struct cpu_policy *system_policies[6] = {
> Another const wanted here after *?
Will do.
~Andrew
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 7/9] x86: Merge xc_cpu_policy's cpuid and msr objects
2023-03-30 10:04 ` Jan Beulich
@ 2023-03-30 11:56 ` Andrew Cooper
0 siblings, 0 replies; 34+ messages in thread
From: Andrew Cooper @ 2023-03-30 11:56 UTC (permalink / raw)
To: Jan Beulich; +Cc: Roger Pau Monné, Wei Liu, Xen-devel
On 30/03/2023 11:04 am, Jan Beulich wrote:
> On 29.03.2023 22:51, Andrew Cooper wrote:
>> /* Sanity test various invariants we expect in the default/max policies. */
>> static void test_guest_policies(const struct xc_cpu_policy *max,
>> const struct xc_cpu_policy *def)
>> {
>> - const struct cpuid_policy *cm = &max->cpuid;
>> - const struct cpuid_policy *cd = &def->cpuid;
>> - const struct msr_policy *mm = &max->msr;
>> - const struct msr_policy *md = &def->msr;
>> + const struct cpu_policy *m = &max->policy;
>> + const struct cpu_policy *d = &def->policy;
> Looks like you could reduce code churn by keeping "cm" and "cd" as the
> names, which would also be consistent with you continuing to use "cp"
> in hypervisor code.
It's a unit test - I'm not worried about churn; I'm more concerned about
the end readability.
I did consider swapping {d,m} with {max,def} so the logic below reads:
if ( ((max->feat.raw[0].d | def->feat.raw[0].d) &
but it occurs to me that because the policies are now merged, I can get
rid of all of the xc_cpu_policy passing and use cpu_policy instead.
(For inspection purposes, we don't care about the serialisation buffers
on the tail of xc_cpu_policy).
It will be a slightly larger change, but the end result will be better IMO.
~Andrew
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 8/9] x86: Drop struct old_cpu_policy
2023-03-30 10:08 ` Jan Beulich
@ 2023-03-30 11:57 ` Andrew Cooper
0 siblings, 0 replies; 34+ messages in thread
From: Andrew Cooper @ 2023-03-30 11:57 UTC (permalink / raw)
To: Jan Beulich; +Cc: Roger Pau Monné, Wei Liu, Xen-devel
On 30/03/2023 11:08 am, Jan Beulich wrote:
> On 29.03.2023 22:51, Andrew Cooper wrote:
>> --- a/tools/libs/guest/xg_cpuid_x86.c
>> +++ b/tools/libs/guest/xg_cpuid_x86.c
>> @@ -868,9 +868,7 @@ bool xc_cpu_policy_is_compatible(xc_interface *xch, xc_cpu_policy_t *host,
>> xc_cpu_policy_t *guest)
>> {
>> struct cpu_policy_errors err = INIT_CPU_POLICY_ERRORS;
>> - struct old_cpu_policy h = { &host->policy, &host->policy };
>> - struct old_cpu_policy g = { &guest->policy, &guest->policy };
>> - int rc = x86_cpu_policies_are_compatible(&h, &g, &err);
>> + int rc = x86_cpu_policies_are_compatible(&host->policy, &host->policy, &err);
> One of the two surely wants to be &guest->policy?
Ah, yes it does. The second, specifically.
(One of the problems of having an API that's not used by anything
interesting yet.)
~Andrew
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 9/9] RFC: Everything else
2023-03-30 10:16 ` Jan Beulich
@ 2023-03-30 12:01 ` Andrew Cooper
2023-03-30 12:06 ` Jan Beulich
0 siblings, 1 reply; 34+ messages in thread
From: Andrew Cooper @ 2023-03-30 12:01 UTC (permalink / raw)
To: Jan Beulich; +Cc: Roger Pau Monné, Wei Liu, Xen-devel
On 30/03/2023 11:16 am, Jan Beulich wrote:
> On 29.03.2023 22:51, Andrew Cooper wrote:
>> Looking at this diff, I'm wondering whether keeping
>>
>> union {
>> struct cpu_policy *cpuid;
>> struct cpu_policy *cpu_policy;
>> };
>>
>> permentantly might be a good idea, because d->arch.cpuid->$X reads rather
>> better than d->arch.cpu_policy->$X
>>
>> Thoughts?
> I'm not overly fussed, but perhaps yes.
If we were to do this, we ought to keep d->arch.msr too for the same reason.
(Honestly - I'm still undecided on whether this is a good idea or not...)
> Nevertheless e.g. ...
>
>> --- a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
>> +++ b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
>> @@ -893,7 +893,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data_p, size_t size)
>> struct x86_emulate_ctxt ctxt = {
>> .data = &state,
>> .regs = &input.regs,
>> - .cpuid = &cp,
>> + .cpu_policy = &cp,
> ... this and ...
>
>> --- a/tools/tests/x86_emulator/test_x86_emulator.c
>> +++ b/tools/tests/x86_emulator/test_x86_emulator.c
>> @@ -909,7 +909,7 @@ int main(int argc, char **argv)
>>
>> ctxt.regs = ®s;
>> ctxt.force_writeback = 0;
>> - ctxt.cpuid = &cp;
>> + ctxt.cpu_policy = &cp;
> ... this imo want keeping as you have it here.
Ok, so that's a firm "switch to using cpu_policy for emul_ctxt" then.
Which is fine - in fact it's one I'd already started splitting out of
this patch.
~Andrew
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 9/9] RFC: Everything else
2023-03-30 12:01 ` Andrew Cooper
@ 2023-03-30 12:06 ` Jan Beulich
2023-03-30 12:31 ` Andrew Cooper
0 siblings, 1 reply; 34+ messages in thread
From: Jan Beulich @ 2023-03-30 12:06 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Roger Pau Monné, Wei Liu, Xen-devel
On 30.03.2023 14:01, Andrew Cooper wrote:
> On 30/03/2023 11:16 am, Jan Beulich wrote:
>> On 29.03.2023 22:51, Andrew Cooper wrote:
>>> Looking at this diff, I'm wondering whether keeping
>>>
>>> union {
>>> struct cpu_policy *cpuid;
>>> struct cpu_policy *cpu_policy;
>>> };
>>>
>>> permentantly might be a good idea, because d->arch.cpuid->$X reads rather
>>> better than d->arch.cpu_policy->$X
>>>
>>> Thoughts?
>> I'm not overly fussed, but perhaps yes.
>
> If we were to do this, we ought to keep d->arch.msr too for the same reason.
I'm not sure this is necessarily a consequence.
> (Honestly - I'm still undecided on whether this is a good idea or not...)
>
>> Nevertheless e.g. ...
>>
>>> --- a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
>>> +++ b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
>>> @@ -893,7 +893,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data_p, size_t size)
>>> struct x86_emulate_ctxt ctxt = {
>>> .data = &state,
>>> .regs = &input.regs,
>>> - .cpuid = &cp,
>>> + .cpu_policy = &cp,
>> ... this and ...
>>
>>> --- a/tools/tests/x86_emulator/test_x86_emulator.c
>>> +++ b/tools/tests/x86_emulator/test_x86_emulator.c
>>> @@ -909,7 +909,7 @@ int main(int argc, char **argv)
>>>
>>> ctxt.regs = ®s;
>>> ctxt.force_writeback = 0;
>>> - ctxt.cpuid = &cp;
>>> + ctxt.cpu_policy = &cp;
>> ... this imo want keeping as you have it here.
>
> Ok, so that's a firm "switch to using cpu_policy for emul_ctxt" then.
>
> Which is fine - in fact it's one I'd already started splitting out of
> this patch.
Hmm, wait - CPUID "basic" and "feat" and alike uses I still would prefer
to see using "cpuid". It's really only such initializers which (imo
even logically) want to use the name with the wider coverage.
Jan
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH RFC 0/9] x86: Merge cpuid and msr policy
2023-03-30 10:18 ` [PATCH RFC 0/9] x86: Merge cpuid and msr policy Jan Beulich
@ 2023-03-30 12:08 ` Andrew Cooper
0 siblings, 0 replies; 34+ messages in thread
From: Andrew Cooper @ 2023-03-30 12:08 UTC (permalink / raw)
To: Jan Beulich; +Cc: Roger Pau Monné, Wei Liu, Xen-devel
On 30/03/2023 11:18 am, Jan Beulich wrote:
> On 29.03.2023 22:51, Andrew Cooper wrote:
>> Andrew Cooper (9):
>> x86: Rename struct cpu_policy to struct old_cpuid_policy
>> x86: Rename {domctl,sysctl}.cpu_policy.{cpuid,msr_policy} fields
> Nit: I guess the last closing brace wants moving forward a little.
Oops yes.
>> x86: Rename struct cpuid_policy to struct cpu_policy
>> x86: Merge struct msr_policy into struct cpu_policy
>> x86: Merge the system {cpuid,msr} policy objects
>> x86: Merge a domain's {cpuid,msr} policy objects
>> x86: Merge xc_cpu_policy's cpuid and msr objects
>> x86: Drop struct old_cpu_policy
> With the small comments on individual patches taken care of one way or
> another, up to here:
>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Thanks. I'll fold this in on these patches, but I don't intend to
commit anything until I've got the full series together.
Right now there's still a lot of shuffling of minor things (comments)
between patches, and not-so-minor things (deciding to split things
differently).
~Andrew
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 9/9] RFC: Everything else
2023-03-30 12:06 ` Jan Beulich
@ 2023-03-30 12:31 ` Andrew Cooper
2023-03-30 12:55 ` Jan Beulich
0 siblings, 1 reply; 34+ messages in thread
From: Andrew Cooper @ 2023-03-30 12:31 UTC (permalink / raw)
To: Jan Beulich; +Cc: Roger Pau Monné, Wei Liu, Xen-devel
On 30/03/2023 1:06 pm, Jan Beulich wrote:
> On 30.03.2023 14:01, Andrew Cooper wrote:
>> On 30/03/2023 11:16 am, Jan Beulich wrote:
>>>> --- a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
>>>> +++ b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
>>>> @@ -893,7 +893,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data_p, size_t size)
>>>> struct x86_emulate_ctxt ctxt = {
>>>> .data = &state,
>>>> .regs = &input.regs,
>>>> - .cpuid = &cp,
>>>> + .cpu_policy = &cp,
>>> ... this and ...
>>>
>>>> --- a/tools/tests/x86_emulator/test_x86_emulator.c
>>>> +++ b/tools/tests/x86_emulator/test_x86_emulator.c
>>>> @@ -909,7 +909,7 @@ int main(int argc, char **argv)
>>>>
>>>> ctxt.regs = ®s;
>>>> ctxt.force_writeback = 0;
>>>> - ctxt.cpuid = &cp;
>>>> + ctxt.cpu_policy = &cp;
>>> ... this imo want keeping as you have it here.
>> Ok, so that's a firm "switch to using cpu_policy for emul_ctxt" then.
>>
>> Which is fine - in fact it's one I'd already started splitting out of
>> this patch.
> Hmm, wait - CPUID "basic" and "feat" and alike uses I still would prefer
> to see using "cpuid". It's really only such initializers which (imo
> even logically) want to use the name with the wider coverage.
So its the other way around and you're saying you don't want the field
name to change, and you don't want to see
-#define vcpu_has_fpu() (ctxt->cpuid->basic.fpu)
+#define vcpu_has_fpu() (ctxt->cpu_policy->basic.fpu)
in the resulting diff ?
~Andrew
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 9/9] RFC: Everything else
2023-03-30 12:31 ` Andrew Cooper
@ 2023-03-30 12:55 ` Jan Beulich
0 siblings, 0 replies; 34+ messages in thread
From: Jan Beulich @ 2023-03-30 12:55 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Roger Pau Monné, Wei Liu, Xen-devel
On 30.03.2023 14:31, Andrew Cooper wrote:
> On 30/03/2023 1:06 pm, Jan Beulich wrote:
>> On 30.03.2023 14:01, Andrew Cooper wrote:
>>> On 30/03/2023 11:16 am, Jan Beulich wrote:
>>>>> --- a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
>>>>> +++ b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
>>>>> @@ -893,7 +893,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data_p, size_t size)
>>>>> struct x86_emulate_ctxt ctxt = {
>>>>> .data = &state,
>>>>> .regs = &input.regs,
>>>>> - .cpuid = &cp,
>>>>> + .cpu_policy = &cp,
>>>> ... this and ...
>>>>
>>>>> --- a/tools/tests/x86_emulator/test_x86_emulator.c
>>>>> +++ b/tools/tests/x86_emulator/test_x86_emulator.c
>>>>> @@ -909,7 +909,7 @@ int main(int argc, char **argv)
>>>>>
>>>>> ctxt.regs = ®s;
>>>>> ctxt.force_writeback = 0;
>>>>> - ctxt.cpuid = &cp;
>>>>> + ctxt.cpu_policy = &cp;
>>>> ... this imo want keeping as you have it here.
>>> Ok, so that's a firm "switch to using cpu_policy for emul_ctxt" then.
>>>
>>> Which is fine - in fact it's one I'd already started splitting out of
>>> this patch.
>> Hmm, wait - CPUID "basic" and "feat" and alike uses I still would prefer
>> to see using "cpuid". It's really only such initializers which (imo
>> even logically) want to use the name with the wider coverage.
>
> So its the other way around and you're saying you don't want the field
> name to change, and you don't want to see
>
> -#define vcpu_has_fpu() (ctxt->cpuid->basic.fpu)
> +#define vcpu_has_fpu() (ctxt->cpu_policy->basic.fpu)
>
> in the resulting diff ?
Neither. I want the change as seen in the diff further up, but indeed
I'd prefer if the two quoted diff lines in your most recent reply
weren't there. This would again be by way of using a union, this time
in struct x86_emulate_ctxt:
/* CPU Policy for the domain. */
union {
const struct cpu_policy *cpuid;
const struct cpu_policy *cpu_policy;
};
Having said that: I realize that this is against what C mandates for
the use of unions, but since we (ab)use unions in similar ways in many
other places, I don't think we need to be concerned. Furthermore this
specific use could, aiui, be "legalized" by making the declaration
/* CPU Policy for the domain. */
union {
struct {
const struct cpu_policy *cpuid;
};
struct {
const struct cpu_policy *cpu_policy;
};
};
Jan
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH RFC 0/9] x86: Merge cpuid and msr policy
2023-03-30 11:07 ` Roger Pau Monné
@ 2023-03-30 12:59 ` Andrew Cooper
2023-03-30 14:43 ` Roger Pau Monné
0 siblings, 1 reply; 34+ messages in thread
From: Andrew Cooper @ 2023-03-30 12:59 UTC (permalink / raw)
To: Roger Pau Monné; +Cc: Xen-devel, Jan Beulich, Wei Liu
On 30/03/2023 12:07 pm, Roger Pau Monné wrote:
> On Wed, Mar 29, 2023 at 09:51:28PM +0100, Andrew Cooper wrote:
>> tl;dr to add MSR_ARCH_CAPS features sensibly, cpu_{featureset<->policy}() need
>> to not operate on objects of differing lifetimes, so structs
>> {cpuid,msr}_policy need merging and cpu_policy is the obvious name.
> So the problem is that there's a chance we might get a cpu_policy
> object that contains a valid (allocated) cpuid object, but not an msr
> one?
No - not cpu_policy. It is that we can get a cpuid_policy and an
msr_policy that aren't at the same point in their lifecycle.
... which is exactly what happens right now for the raw/host msr right
now if you featureset_to_policy() to include MSR data.
Merging the two together into cpu_policy causes there to be a single
object lifecycle.
It's probably worth repeating the advise from the footnote in
https://lwn.net/Articles/193245/ again. Get your datastructures right,
and the code takes care of itself. Don't get them right, and the code
tends to be unmaintainable.
>> But this does mean that we now have
>>
>> cpu_policy->basic.$X
>> cpu_policy->feat.$Y
>> cpu_policy->arch_caps.$Z
> I'm not sure I like the fact that we now can't differentiate between
> policy fields related to MSRs or CPUID leafs.
>
> Isn't there a chance we might in the future get some name space
> collision by us having decided to unify both?
The names are chosen by me so far, and the compiler will tell us if
things actually collide.
And renaming the existing field is a perfectly acceptable way of
resolving a conflict which arises in the future.
But yes - this was the whole point of asking the question.
~Andrew
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH RFC 0/9] x86: Merge cpuid and msr policy
2023-03-30 12:59 ` Andrew Cooper
@ 2023-03-30 14:43 ` Roger Pau Monné
2023-04-03 10:55 ` Andrew Cooper
0 siblings, 1 reply; 34+ messages in thread
From: Roger Pau Monné @ 2023-03-30 14:43 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Jan Beulich, Wei Liu
On Thu, Mar 30, 2023 at 01:59:37PM +0100, Andrew Cooper wrote:
> On 30/03/2023 12:07 pm, Roger Pau Monné wrote:
> > On Wed, Mar 29, 2023 at 09:51:28PM +0100, Andrew Cooper wrote:
> >> tl;dr to add MSR_ARCH_CAPS features sensibly, cpu_{featureset<->policy}() need
> >> to not operate on objects of differing lifetimes, so structs
> >> {cpuid,msr}_policy need merging and cpu_policy is the obvious name.
> > So the problem is that there's a chance we might get a cpu_policy
> > object that contains a valid (allocated) cpuid object, but not an msr
> > one?
>
> No - not cpu_policy. It is that we can get a cpuid_policy and an
> msr_policy that aren't at the same point in their lifecycle.
>
> ... which is exactly what happens right now for the raw/host msr right
> now if you featureset_to_policy() to include MSR data.
I see, but that's mostly because we handle the featureset_to_policy()
in two different places for CPUID vs MSR, those need to be unified
into a single helper that does both at the same point.
I assume not having such pointers in side of cpu_policy makes it
clearer that both msr and cpuid should be handled at the same time,
but ultimately this would imply passing a cpu_policy object to
featureset_to_policy() so that both CPUID and MSR sub-structs are
filled from the same featureset.
Sorry, maybe I'm being a bit dull here, just would like to understand
the motivation of the change.
> Merging the two together into cpu_policy causes there to be a single
> object lifecycle.
>
>
> It's probably worth repeating the advise from the footnote in
> https://lwn.net/Articles/193245/ again. Get your datastructures right,
> and the code takes care of itself. Don't get them right, and the code
> tends to be unmaintainable.
>
>
> >> But this does mean that we now have
> >>
> >> cpu_policy->basic.$X
> >> cpu_policy->feat.$Y
> >> cpu_policy->arch_caps.$Z
> > I'm not sure I like the fact that we now can't differentiate between
> > policy fields related to MSRs or CPUID leafs.
> >
> > Isn't there a chance we might in the future get some name space
> > collision by us having decided to unify both?
>
> The names are chosen by me so far, and the compiler will tell us if
> things actually collide.
>
> And renaming the existing field is a perfectly acceptable way of
> resolving a conflict which arises in the future.
>
> But yes - this was the whole point of asking the question.
I think I would prefer to keep the cpu_policy->{cpuid,msr}.
distinction if it doesn't interfere with further work.
Thanks, Roger.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH RFC 0/9] x86: Merge cpuid and msr policy
2023-03-30 14:43 ` Roger Pau Monné
@ 2023-04-03 10:55 ` Andrew Cooper
2023-04-03 11:47 ` Roger Pau Monné
0 siblings, 1 reply; 34+ messages in thread
From: Andrew Cooper @ 2023-04-03 10:55 UTC (permalink / raw)
To: Roger Pau Monné; +Cc: Xen-devel, Jan Beulich, Wei Liu
On 30/03/2023 3:43 pm, Roger Pau Monné wrote:
> On Thu, Mar 30, 2023 at 01:59:37PM +0100, Andrew Cooper wrote:
>> On 30/03/2023 12:07 pm, Roger Pau Monné wrote:
>>> On Wed, Mar 29, 2023 at 09:51:28PM +0100, Andrew Cooper wrote:
>>>> tl;dr to add MSR_ARCH_CAPS features sensibly, cpu_{featureset<->policy}() need
>>>> to not operate on objects of differing lifetimes, so structs
>>>> {cpuid,msr}_policy need merging and cpu_policy is the obvious name.
>>> So the problem is that there's a chance we might get a cpu_policy
>>> object that contains a valid (allocated) cpuid object, but not an msr
>>> one?
>> No - not cpu_policy. It is that we can get a cpuid_policy and an
>> msr_policy that aren't at the same point in their lifecycle.
>>
>> ... which is exactly what happens right now for the raw/host msr right
>> now if you featureset_to_policy() to include MSR data.
> I see, but that's mostly because we handle the featureset_to_policy()
> in two different places for CPUID vs MSR, those need to be unified
> into a single helper that does both at the same point.
>
> I assume not having such pointers in side of cpu_policy makes it
> clearer that both msr and cpuid should be handled at the same time,
> but ultimately this would imply passing a cpu_policy object to
> featureset_to_policy() so that both CPUID and MSR sub-structs are
> filled from the same featureset.
>
> Sorry, maybe I'm being a bit dull here, just would like to understand
> the motivation of the change.
That's pretty much it. Forcing them to be one object removes a class of
errors, and makes the resulting code easier to follow.
(Based on having tried to do the non-merged approach first, and deciding
that it's not code I'm willing to try putting upstream...)
>> Merging the two together into cpu_policy causes there to be a single
>> object lifecycle.
>>
>>
>> It's probably worth repeating the advise from the footnote in
>> https://lwn.net/Articles/193245/ again. Get your datastructures right,
>> and the code takes care of itself. Don't get them right, and the code
>> tends to be unmaintainable.
>>
>>
>>>> But this does mean that we now have
>>>>
>>>> cpu_policy->basic.$X
>>>> cpu_policy->feat.$Y
>>>> cpu_policy->arch_caps.$Z
>>> I'm not sure I like the fact that we now can't differentiate between
>>> policy fields related to MSRs or CPUID leafs.
>>>
>>> Isn't there a chance we might in the future get some name space
>>> collision by us having decided to unify both?
>> The names are chosen by me so far, and the compiler will tell us if
>> things actually collide.
>>
>> And renaming the existing field is a perfectly acceptable way of
>> resolving a conflict which arises in the future.
>>
>> But yes - this was the whole point of asking the question.
> I think I would prefer to keep the cpu_policy->{cpuid,msr}.
> distinction if it doesn't interfere with further work.
Unfortunately that's the opposite of what Jan asked for. What I have
done, based on the prior conversation is:
struct arch_domain {
...
/*
* The domain's CPU Policy. "cpu_policy" is considered the canonical
* pointer, but the "cpuid" and "msr" aliases exist so the most
* appropriate one can be used for local code clarity.
*/
union {
struct cpu_policy *cpu_policy;
struct cpu_policy *cpuid;
struct cpu_policy *msr;
};
So all the cases where you do have d->arch.cpuid.feat.$X, this continues
to work.
The cases where you pull a cpu_policy out into a local variable, there
will be no cpuid or msr infix, but these cases already have no cpuid/msr
part to their naming.
~Andrew
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH RFC 0/9] x86: Merge cpuid and msr policy
2023-04-03 10:55 ` Andrew Cooper
@ 2023-04-03 11:47 ` Roger Pau Monné
2023-04-03 11:48 ` Andrew Cooper
0 siblings, 1 reply; 34+ messages in thread
From: Roger Pau Monné @ 2023-04-03 11:47 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Jan Beulich, Wei Liu
On Mon, Apr 03, 2023 at 11:55:57AM +0100, Andrew Cooper wrote:
> On 30/03/2023 3:43 pm, Roger Pau Monné wrote:
> > On Thu, Mar 30, 2023 at 01:59:37PM +0100, Andrew Cooper wrote:
> >> On 30/03/2023 12:07 pm, Roger Pau Monné wrote:
> >>> On Wed, Mar 29, 2023 at 09:51:28PM +0100, Andrew Cooper wrote:
> >>>> But this does mean that we now have
> >>>>
> >>>> cpu_policy->basic.$X
> >>>> cpu_policy->feat.$Y
> >>>> cpu_policy->arch_caps.$Z
> >>> I'm not sure I like the fact that we now can't differentiate between
> >>> policy fields related to MSRs or CPUID leafs.
> >>>
> >>> Isn't there a chance we might in the future get some name space
> >>> collision by us having decided to unify both?
> >> The names are chosen by me so far, and the compiler will tell us if
> >> things actually collide.
> >>
> >> And renaming the existing field is a perfectly acceptable way of
> >> resolving a conflict which arises in the future.
> >>
> >> But yes - this was the whole point of asking the question.
> > I think I would prefer to keep the cpu_policy->{cpuid,msr}.
> > distinction if it doesn't interfere with further work.
>
> Unfortunately that's the opposite of what Jan asked for. What I have
> done, based on the prior conversation is:
>
> struct arch_domain {
> ...
>
> /*
> * The domain's CPU Policy. "cpu_policy" is considered the canonical
> * pointer, but the "cpuid" and "msr" aliases exist so the most
> * appropriate one can be used for local code clarity.
> */
> union {
> struct cpu_policy *cpu_policy;
> struct cpu_policy *cpuid;
> struct cpu_policy *msr;
> };
>
> So all the cases where you do have d->arch.cpuid.feat.$X, this continues
> to work.
>
> The cases where you pull a cpu_policy out into a local variable, there
> will be no cpuid or msr infix, but these cases already have no cpuid/msr
> part to their naming.
I see. I'm fine with this. There's still the remote possibility of
field name clash between cpuid and msr names, but we can likely sort
this out if we ever get into this position.
Thanks, Roger.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH RFC 0/9] x86: Merge cpuid and msr policy
2023-04-03 11:47 ` Roger Pau Monné
@ 2023-04-03 11:48 ` Andrew Cooper
0 siblings, 0 replies; 34+ messages in thread
From: Andrew Cooper @ 2023-04-03 11:48 UTC (permalink / raw)
To: Roger Pau Monné; +Cc: Xen-devel, Jan Beulich, Wei Liu
On 03/04/2023 12:47 pm, Roger Pau Monné wrote:
> On Mon, Apr 03, 2023 at 11:55:57AM +0100, Andrew Cooper wrote:
>> On 30/03/2023 3:43 pm, Roger Pau Monné wrote:
>>> On Thu, Mar 30, 2023 at 01:59:37PM +0100, Andrew Cooper wrote:
>>>> On 30/03/2023 12:07 pm, Roger Pau Monné wrote:
>>>>> On Wed, Mar 29, 2023 at 09:51:28PM +0100, Andrew Cooper wrote:
>>>>>> But this does mean that we now have
>>>>>>
>>>>>> cpu_policy->basic.$X
>>>>>> cpu_policy->feat.$Y
>>>>>> cpu_policy->arch_caps.$Z
>>>>> I'm not sure I like the fact that we now can't differentiate between
>>>>> policy fields related to MSRs or CPUID leafs.
>>>>>
>>>>> Isn't there a chance we might in the future get some name space
>>>>> collision by us having decided to unify both?
>>>> The names are chosen by me so far, and the compiler will tell us if
>>>> things actually collide.
>>>>
>>>> And renaming the existing field is a perfectly acceptable way of
>>>> resolving a conflict which arises in the future.
>>>>
>>>> But yes - this was the whole point of asking the question.
>>> I think I would prefer to keep the cpu_policy->{cpuid,msr}.
>>> distinction if it doesn't interfere with further work.
>> Unfortunately that's the opposite of what Jan asked for. What I have
>> done, based on the prior conversation is:
>>
>> struct arch_domain {
>> ...
>>
>> /*
>> * The domain's CPU Policy. "cpu_policy" is considered the canonical
>> * pointer, but the "cpuid" and "msr" aliases exist so the most
>> * appropriate one can be used for local code clarity.
>> */
>> union {
>> struct cpu_policy *cpu_policy;
>> struct cpu_policy *cpuid;
>> struct cpu_policy *msr;
>> };
>>
>> So all the cases where you do have d->arch.cpuid.feat.$X, this continues
>> to work.
>>
>> The cases where you pull a cpu_policy out into a local variable, there
>> will be no cpuid or msr infix, but these cases already have no cpuid/msr
>> part to their naming.
> I see. I'm fine with this. There's still the remote possibility of
> field name clash between cpuid and msr names, but we can likely sort
> this out if we ever get into this position.
Thanks. Yeah, we can rename if things become problematic.
~Andrew
^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2023-04-03 11:49 UTC | newest]
Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-29 20:51 [PATCH RFC 0/9] x86: Merge cpuid and msr policy Andrew Cooper
2023-03-29 20:51 ` [PATCH 1/9] x86: Rename struct cpu_policy to struct old_cpuid_policy Andrew Cooper
2023-03-29 20:51 ` [PATCH 2/9] x86: Rename {domctl,sysctl}.cpu_policy.{cpuid,msr_policy} fields Andrew Cooper
2023-03-29 20:51 ` [PATCH 3/9] x86: Rename struct cpuid_policy to struct cpu_policy Andrew Cooper
2023-03-29 20:51 ` [PATCH 4/9] x86: Merge struct msr_policy into " Andrew Cooper
2023-03-30 9:30 ` Jan Beulich
2023-03-30 11:11 ` Andrew Cooper
2023-03-30 11:15 ` Jan Beulich
2023-03-29 20:51 ` [PATCH 5/9] x86: Merge the system {cpuid,msr} policy objects Andrew Cooper
2023-03-30 9:40 ` Jan Beulich
2023-03-30 11:19 ` Andrew Cooper
2023-03-29 20:51 ` [PATCH 6/9] x86: Merge a domain's " Andrew Cooper
2023-03-30 10:00 ` Jan Beulich
2023-03-30 11:14 ` Andrew Cooper
2023-03-29 20:51 ` [PATCH 7/9] x86: Merge xc_cpu_policy's cpuid and msr objects Andrew Cooper
2023-03-30 10:04 ` Jan Beulich
2023-03-30 11:56 ` Andrew Cooper
2023-03-29 20:51 ` [PATCH 8/9] x86: Drop struct old_cpu_policy Andrew Cooper
2023-03-30 10:08 ` Jan Beulich
2023-03-30 11:57 ` Andrew Cooper
2023-03-29 20:51 ` [PATCH 9/9] RFC: Everything else Andrew Cooper
2023-03-30 10:16 ` Jan Beulich
2023-03-30 12:01 ` Andrew Cooper
2023-03-30 12:06 ` Jan Beulich
2023-03-30 12:31 ` Andrew Cooper
2023-03-30 12:55 ` Jan Beulich
2023-03-30 10:18 ` [PATCH RFC 0/9] x86: Merge cpuid and msr policy Jan Beulich
2023-03-30 12:08 ` Andrew Cooper
2023-03-30 11:07 ` Roger Pau Monné
2023-03-30 12:59 ` Andrew Cooper
2023-03-30 14:43 ` Roger Pau Monné
2023-04-03 10:55 ` Andrew Cooper
2023-04-03 11:47 ` Roger Pau Monné
2023-04-03 11:48 ` Andrew Cooper
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.