xen/include/asm-x86/msr-index.h | 123 ++++++++++++++++++++++------------------ 1 file changed, 68 insertions(+), 55 deletions(-)
Make a start on cleaning up the constants in msr-index.h.
No functional change - only formatting changes.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wl@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
Pulled out of a longer series which really ought to start making some
progress.
---
xen/include/asm-x86/msr-index.h | 123 ++++++++++++++++++++++------------------
1 file changed, 68 insertions(+), 55 deletions(-)
diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
index c320846c06..c831cd2c60 100644
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -1,7 +1,74 @@
#ifndef __ASM_MSR_INDEX_H
#define __ASM_MSR_INDEX_H
-/* CPU model specific register (MSR) numbers */
+/*
+ * CPU model specific register (MSR) numbers
+ *
+ * Definitions for an MSR should follow this style:
+ *
+ * #define MSR_$NAME 0x$INDEX
+ * #define $NAME_$BIT1 (_AC(1, ULL) << $POS1)
+ * #define $NAME_$BIT2 (_AC(1, ULL) << $POS2)
+ *
+ * Blocks of related constants should be sorted by MSR index. The constant
+ * names should be as concise as possible, and the bit names may have an
+ * abbreviated name.
+ */
+
+#define MSR_APIC_BASE 0x0000001b
+#define APIC_BASE_BSP (_AC(1, ULL) << 8)
+#define APIC_BASE_EXTD (_AC(1, ULL) << 10)
+#define APIC_BASE_ENABLE (_AC(1, ULL) << 11)
+#define APIC_BASE_ADDR_MASK 0x000ffffffffff000ULL
+
+#define MSR_TEST_CTRL 0x00000033
+#define TEST_CTRL_SPLITLOCK_DETECT (_AC(1, ULL) << 29)
+#define TEST_CTRL_SPLITLOCK_DISABLE (_AC(1, ULL) << 31)
+
+#define MSR_INTEL_CORE_THREAD_COUNT 0x00000035
+#define MSR_CTC_THREAD_MASK 0x0000ffff
+#define MSR_CTC_CORE_MASK 0xffff0000
+
+#define MSR_SPEC_CTRL 0x00000048
+#define SPEC_CTRL_IBRS (_AC(1, ULL) << 0)
+#define SPEC_CTRL_STIBP (_AC(1, ULL) << 1)
+#define SPEC_CTRL_SSBD (_AC(1, ULL) << 2)
+
+#define MSR_PRED_CMD 0x00000049
+#define PRED_CMD_IBPB (_AC(1, ULL) << 0)
+
+#define MSR_PPIN_CTL 0x0000004e
+#define PPIN_LOCKOUT (_AC(1, ULL) << 0)
+#define PPIN_ENABLE (_AC(1, ULL) << 1)
+#define MSR_PPIN 0x0000004f
+
+#define MSR_CORE_CAPABILITIES 0x000000cf
+#define CORE_CAPS_SPLITLOCK_DETECT (_AC(1, ULL) << 5)
+
+#define MSR_ARCH_CAPABILITIES 0x0000010a
+#define ARCH_CAPS_RDCL_NO (_AC(1, ULL) << 0)
+#define ARCH_CAPS_IBRS_ALL (_AC(1, ULL) << 1)
+#define ARCH_CAPS_RSBA (_AC(1, ULL) << 2)
+#define ARCH_CAPS_SKIP_L1DFL (_AC(1, ULL) << 3)
+#define ARCH_CAPS_SSB_NO (_AC(1, ULL) << 4)
+#define ARCH_CAPS_MDS_NO (_AC(1, ULL) << 5)
+#define ARCH_CAPS_IF_PSCHANGE_MC_NO (_AC(1, ULL) << 6)
+#define ARCH_CAPS_TSX_CTRL (_AC(1, ULL) << 7)
+#define ARCH_CAPS_TAA_NO (_AC(1, ULL) << 8)
+
+#define MSR_FLUSH_CMD 0x0000010b
+#define FLUSH_CMD_L1D (_AC(1, ULL) << 0)
+
+#define MSR_TSX_FORCE_ABORT 0x0000010f
+#define TSX_FORCE_ABORT_RTM (_AC(1, ULL) << 0)
+
+#define MSR_TSX_CTRL 0x00000122
+#define TSX_CTRL_RTM_DISABLE (_AC(1, ULL) << 0)
+#define TSX_CTRL_CPUID_CLEAR (_AC(1, ULL) << 1)
+
+/*
+ * Legacy MSR constants in need of cleanup. No new code below this comment.
+ */
/* x86-64 specific MSRs */
#define MSR_EFER 0xc0000080 /* extended feature register */
@@ -32,54 +99,6 @@
#define EFER_KNOWN_MASK (EFER_SCE | EFER_LME | EFER_LMA | EFER_NX | \
EFER_SVME | EFER_FFXSE)
-#define MSR_TEST_CTRL 0x00000033
-#define TEST_CTRL_SPLITLOCK_DETECT (_AC(1, ULL) << 29)
-#define TEST_CTRL_SPLITLOCK_DISABLE (_AC(1, ULL) << 31)
-
-#define MSR_INTEL_CORE_THREAD_COUNT 0x00000035
-#define MSR_CTC_THREAD_MASK 0x0000ffff
-#define MSR_CTC_CORE_MASK 0xffff0000
-
-/* Speculation Controls. */
-#define MSR_SPEC_CTRL 0x00000048
-#define SPEC_CTRL_IBRS (_AC(1, ULL) << 0)
-#define SPEC_CTRL_STIBP (_AC(1, ULL) << 1)
-#define SPEC_CTRL_SSBD (_AC(1, ULL) << 2)
-
-#define MSR_PRED_CMD 0x00000049
-#define PRED_CMD_IBPB (_AC(1, ULL) << 0)
-
-/* Intel Protected Processor Inventory Number */
-#define MSR_PPIN_CTL 0x0000004e
-#define MSR_PPIN 0x0000004f
-
-#define PPIN_LOCKOUT (_AC(1, ULL) << 0)
-#define PPIN_ENABLE (_AC(1, ULL) << 1)
-
-#define MSR_CORE_CAPABILITIES 0x000000cf
-#define CORE_CAPS_SPLITLOCK_DETECT (_AC(1, ULL) << 5)
-
-#define MSR_ARCH_CAPABILITIES 0x0000010a
-#define ARCH_CAPS_RDCL_NO (_AC(1, ULL) << 0)
-#define ARCH_CAPS_IBRS_ALL (_AC(1, ULL) << 1)
-#define ARCH_CAPS_RSBA (_AC(1, ULL) << 2)
-#define ARCH_CAPS_SKIP_L1DFL (_AC(1, ULL) << 3)
-#define ARCH_CAPS_SSB_NO (_AC(1, ULL) << 4)
-#define ARCH_CAPS_MDS_NO (_AC(1, ULL) << 5)
-#define ARCH_CAPS_IF_PSCHANGE_MC_NO (_AC(1, ULL) << 6)
-#define ARCH_CAPS_TSX_CTRL (_AC(1, ULL) << 7)
-#define ARCH_CAPS_TAA_NO (_AC(1, ULL) << 8)
-
-#define MSR_FLUSH_CMD 0x0000010b
-#define FLUSH_CMD_L1D (_AC(1, ULL) << 0)
-
-#define MSR_TSX_FORCE_ABORT 0x0000010f
-#define TSX_FORCE_ABORT_RTM (_AC(1, ULL) << 0)
-
-#define MSR_TSX_CTRL 0x00000122
-#define TSX_CTRL_RTM_DISABLE (_AC(1, ULL) << 0)
-#define TSX_CTRL_CPUID_CLEAR (_AC(1, ULL) << 1)
-
/* Intel MSRs. Some also available on other CPUs */
#define MSR_IA32_PERFCTR0 0x000000c1
#define MSR_IA32_A_PERFCTR0 0x000004c1
@@ -359,12 +378,6 @@
#define MSR_IA32_TSC_ADJUST 0x0000003b
-#define MSR_APIC_BASE 0x0000001b
-#define APIC_BASE_BSP (1<<8)
-#define APIC_BASE_EXTD (1<<10)
-#define APIC_BASE_ENABLE (1<<11)
-#define APIC_BASE_ADDR_MASK 0x000ffffffffff000ul
-
#define MSR_X2APIC_FIRST 0x00000800
#define MSR_X2APIC_LAST 0x00000bff
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-develOn 21.02.2020 16:19, Andrew Cooper wrote: > --- a/xen/include/asm-x86/msr-index.h > +++ b/xen/include/asm-x86/msr-index.h > @@ -1,7 +1,74 @@ > #ifndef __ASM_MSR_INDEX_H > #define __ASM_MSR_INDEX_H > > -/* CPU model specific register (MSR) numbers */ > +/* > + * CPU model specific register (MSR) numbers > + * > + * Definitions for an MSR should follow this style: > + * > + * #define MSR_$NAME 0x$INDEX > + * #define $NAME_$BIT1 (_AC(1, ULL) << $POS1) > + * #define $NAME_$BIT2 (_AC(1, ULL) << $POS2) > + * > + * Blocks of related constants should be sorted by MSR index. The constant > + * names should be as concise as possible, and the bit names may have an > + * abbreviated name. > + */ Hmm, while "Blocks of related constants" caters for e.g. AMD's MSR_AMD64_DR<n>_ADDRESS_MASK, I think for ease of lookup we may want to be slightly more strict, without requiring strong ordering. E.g. by also stating that multiple such blocks should be ordered relative to one another also numerically (much like we try to do in the insn emulator's huge switch() statement), based on their lowest numbered MSR. (As a nit, the example kind of implies that only single bit fields would ever occur. It might avoid questions if you gave a multi-bit example.) > +#define MSR_APIC_BASE 0x0000001b > +#define APIC_BASE_BSP (_AC(1, ULL) << 8) > +#define APIC_BASE_EXTD (_AC(1, ULL) << 10) > +#define APIC_BASE_ENABLE (_AC(1, ULL) << 11) > +#define APIC_BASE_ADDR_MASK 0x000ffffffffff000ULL > + > +#define MSR_TEST_CTRL 0x00000033 > +#define TEST_CTRL_SPLITLOCK_DETECT (_AC(1, ULL) << 29) > +#define TEST_CTRL_SPLITLOCK_DISABLE (_AC(1, ULL) << 31) > + > +#define MSR_INTEL_CORE_THREAD_COUNT 0x00000035 > +#define MSR_CTC_THREAD_MASK 0x0000ffff > +#define MSR_CTC_CORE_MASK 0xffff0000 > + > +#define MSR_SPEC_CTRL 0x00000048 > +#define SPEC_CTRL_IBRS (_AC(1, ULL) << 0) > +#define SPEC_CTRL_STIBP (_AC(1, ULL) << 1) > +#define SPEC_CTRL_SSBD (_AC(1, ULL) << 2) > + > +#define MSR_PRED_CMD 0x00000049 > +#define PRED_CMD_IBPB (_AC(1, ULL) << 0) > + > +#define MSR_PPIN_CTL 0x0000004e > +#define PPIN_LOCKOUT (_AC(1, ULL) << 0) > +#define PPIN_ENABLE (_AC(1, ULL) << 1) > +#define MSR_PPIN 0x0000004f > + > +#define MSR_CORE_CAPABILITIES 0x000000cf > +#define CORE_CAPS_SPLITLOCK_DETECT (_AC(1, ULL) << 5) > + > +#define MSR_ARCH_CAPABILITIES 0x0000010a > +#define ARCH_CAPS_RDCL_NO (_AC(1, ULL) << 0) > +#define ARCH_CAPS_IBRS_ALL (_AC(1, ULL) << 1) > +#define ARCH_CAPS_RSBA (_AC(1, ULL) << 2) > +#define ARCH_CAPS_SKIP_L1DFL (_AC(1, ULL) << 3) > +#define ARCH_CAPS_SSB_NO (_AC(1, ULL) << 4) > +#define ARCH_CAPS_MDS_NO (_AC(1, ULL) << 5) > +#define ARCH_CAPS_IF_PSCHANGE_MC_NO (_AC(1, ULL) << 6) > +#define ARCH_CAPS_TSX_CTRL (_AC(1, ULL) << 7) > +#define ARCH_CAPS_TAA_NO (_AC(1, ULL) << 8) > + > +#define MSR_FLUSH_CMD 0x0000010b > +#define FLUSH_CMD_L1D (_AC(1, ULL) << 0) > + > +#define MSR_TSX_FORCE_ABORT 0x0000010f > +#define TSX_FORCE_ABORT_RTM (_AC(1, ULL) << 0) > + > +#define MSR_TSX_CTRL 0x00000122 > +#define TSX_CTRL_RTM_DISABLE (_AC(1, ULL) << 0) > +#define TSX_CTRL_CPUID_CLEAR (_AC(1, ULL) << 1) > + > +/* > + * Legacy MSR constants in need of cleanup. No new code below this comment. > + */ "No new code" goes too far, I think. We shouldn't demand new bits for existing MSRs to be accompanied by other cleanup of that same MSR's definitions. "No new MSRs below ..." otoh would be fine with me. If you agree with both, feel free to add Reviewed-by: Jan Beulich <jbeulich@suse.com> Otherwise let's see what we can come to agree on. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
On 21/02/2020 15:41, Jan Beulich wrote: > On 21.02.2020 16:19, Andrew Cooper wrote: >> --- a/xen/include/asm-x86/msr-index.h >> +++ b/xen/include/asm-x86/msr-index.h >> @@ -1,7 +1,74 @@ >> #ifndef __ASM_MSR_INDEX_H >> #define __ASM_MSR_INDEX_H >> >> -/* CPU model specific register (MSR) numbers */ >> +/* >> + * CPU model specific register (MSR) numbers >> + * >> + * Definitions for an MSR should follow this style: >> + * >> + * #define MSR_$NAME 0x$INDEX >> + * #define $NAME_$BIT1 (_AC(1, ULL) << $POS1) >> + * #define $NAME_$BIT2 (_AC(1, ULL) << $POS2) >> + * >> + * Blocks of related constants should be sorted by MSR index. The constant >> + * names should be as concise as possible, and the bit names may have an >> + * abbreviated name. >> + */ > Hmm, while "Blocks of related constants" caters for e.g. AMD's > MSR_AMD64_DR<n>_ADDRESS_MASK, I think for ease of lookup we > may want to be slightly more strict, without requiring strong > ordering. E.g. by also stating that multiple such blocks should > be ordered relative to one another also numerically (much like > we try to do in the insn emulator's huge switch() statement), > based on their lowest numbered MSR. "Exceptions will be considered on a case-by-case basis" ? It is not as if we'll ever be able to write down rules which will apply uniformly to the whole file. > (As a nit, the example kind of implies that only single bit > fields would ever occur. It might avoid questions if you gave > a multi-bit example.) Single bit fields are the overwhelmingly common example. Would s/BIT/FIELD/ read any better, perhaps with $X and $Y instead of 1's in the _AC() ? > >> +#define MSR_APIC_BASE 0x0000001b >> +#define APIC_BASE_BSP (_AC(1, ULL) << 8) >> +#define APIC_BASE_EXTD (_AC(1, ULL) << 10) >> +#define APIC_BASE_ENABLE (_AC(1, ULL) << 11) >> +#define APIC_BASE_ADDR_MASK 0x000ffffffffff000ULL >> + >> +#define MSR_TEST_CTRL 0x00000033 >> +#define TEST_CTRL_SPLITLOCK_DETECT (_AC(1, ULL) << 29) >> +#define TEST_CTRL_SPLITLOCK_DISABLE (_AC(1, ULL) << 31) >> + >> +#define MSR_INTEL_CORE_THREAD_COUNT 0x00000035 >> +#define MSR_CTC_THREAD_MASK 0x0000ffff >> +#define MSR_CTC_CORE_MASK 0xffff0000 >> + >> +#define MSR_SPEC_CTRL 0x00000048 >> +#define SPEC_CTRL_IBRS (_AC(1, ULL) << 0) >> +#define SPEC_CTRL_STIBP (_AC(1, ULL) << 1) >> +#define SPEC_CTRL_SSBD (_AC(1, ULL) << 2) >> + >> +#define MSR_PRED_CMD 0x00000049 >> +#define PRED_CMD_IBPB (_AC(1, ULL) << 0) >> + >> +#define MSR_PPIN_CTL 0x0000004e >> +#define PPIN_LOCKOUT (_AC(1, ULL) << 0) >> +#define PPIN_ENABLE (_AC(1, ULL) << 1) >> +#define MSR_PPIN 0x0000004f >> + >> +#define MSR_CORE_CAPABILITIES 0x000000cf >> +#define CORE_CAPS_SPLITLOCK_DETECT (_AC(1, ULL) << 5) >> + >> +#define MSR_ARCH_CAPABILITIES 0x0000010a >> +#define ARCH_CAPS_RDCL_NO (_AC(1, ULL) << 0) >> +#define ARCH_CAPS_IBRS_ALL (_AC(1, ULL) << 1) >> +#define ARCH_CAPS_RSBA (_AC(1, ULL) << 2) >> +#define ARCH_CAPS_SKIP_L1DFL (_AC(1, ULL) << 3) >> +#define ARCH_CAPS_SSB_NO (_AC(1, ULL) << 4) >> +#define ARCH_CAPS_MDS_NO (_AC(1, ULL) << 5) >> +#define ARCH_CAPS_IF_PSCHANGE_MC_NO (_AC(1, ULL) << 6) >> +#define ARCH_CAPS_TSX_CTRL (_AC(1, ULL) << 7) >> +#define ARCH_CAPS_TAA_NO (_AC(1, ULL) << 8) >> + >> +#define MSR_FLUSH_CMD 0x0000010b >> +#define FLUSH_CMD_L1D (_AC(1, ULL) << 0) >> + >> +#define MSR_TSX_FORCE_ABORT 0x0000010f >> +#define TSX_FORCE_ABORT_RTM (_AC(1, ULL) << 0) >> + >> +#define MSR_TSX_CTRL 0x00000122 >> +#define TSX_CTRL_RTM_DISABLE (_AC(1, ULL) << 0) >> +#define TSX_CTRL_CPUID_CLEAR (_AC(1, ULL) << 1) >> + >> +/* >> + * Legacy MSR constants in need of cleanup. No new code below this comment. >> + */ > "No new code" goes too far, I think. We shouldn't demand new > bits for existing MSRs to be accompanied by other cleanup of > that same MSR's definitions. "No new MSRs below ..." otoh > would be fine with me. Ok. TBH, I don't expect anyone but core maintainers to even know this is here. It is mainly a concrete separation between the two halves. > If you agree with both, feel free to add > Reviewed-by: Jan Beulich <jbeulich@suse.com> Thanks, but I'd like your views on the suggestions above. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
On 21.02.2020 16:59, Andrew Cooper wrote: > On 21/02/2020 15:41, Jan Beulich wrote: >> On 21.02.2020 16:19, Andrew Cooper wrote: >>> --- a/xen/include/asm-x86/msr-index.h >>> +++ b/xen/include/asm-x86/msr-index.h >>> @@ -1,7 +1,74 @@ >>> #ifndef __ASM_MSR_INDEX_H >>> #define __ASM_MSR_INDEX_H >>> >>> -/* CPU model specific register (MSR) numbers */ >>> +/* >>> + * CPU model specific register (MSR) numbers >>> + * >>> + * Definitions for an MSR should follow this style: >>> + * >>> + * #define MSR_$NAME 0x$INDEX >>> + * #define $NAME_$BIT1 (_AC(1, ULL) << $POS1) >>> + * #define $NAME_$BIT2 (_AC(1, ULL) << $POS2) >>> + * >>> + * Blocks of related constants should be sorted by MSR index. The constant >>> + * names should be as concise as possible, and the bit names may have an >>> + * abbreviated name. >>> + */ >> Hmm, while "Blocks of related constants" caters for e.g. AMD's >> MSR_AMD64_DR<n>_ADDRESS_MASK, I think for ease of lookup we >> may want to be slightly more strict, without requiring strong >> ordering. E.g. by also stating that multiple such blocks should >> be ordered relative to one another also numerically (much like >> we try to do in the insn emulator's huge switch() statement), >> based on their lowest numbered MSR. > > "Exceptions will be considered on a case-by-case basis" ? Sounds good. > It is not as if we'll ever be able to write down rules which will apply > uniformly to the whole file. > >> (As a nit, the example kind of implies that only single bit >> fields would ever occur. It might avoid questions if you gave >> a multi-bit example.) > > Single bit fields are the overwhelmingly common example. Would > s/BIT/FIELD/ read any better, perhaps with $X and $Y instead of 1's in > the _AC() ? Yes. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
© 2016 - 2024 Red Hat, Inc.