| Message ID | 20170424175430.395-2-mohit.gambhir@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 24.04.17 at 19:54, <mohit.gambhir@oracle.com> wrote: > --- a/arch/x86/include/arch/msr-index.h > +++ b/arch/x86/include/arch/msr-index.h > @@ -38,6 +38,17 @@ > #define MSR_GS_BASE 0xc0000101 > #define MSR_SHADOW_GS_BASE 0xc0000102 > > +#define MSR_IA32_PMC(n) (0x000000c1 + (n)) > +#define MSR_IA32_PERFEVTSEL(n) (0x00000186 + (n)) > +#define MSR_IA32_DEBUGCTL 0x000001d9 This still sits in the middle of PMU ones, despite not being purely PMU related. Jan > +#define MSR_IA32_FIXED_CTR(n) (0x00000309 + (n)) > +#define MSR_IA32_PERF_CAPABILITIES 0x00000345 > +#define MSR_IA32_FIXED_CTR_CTRL 0x0000038d > +#define MSR_IA32_PERF_GLOBAL_CTRL 0x0000038f > +#define MSR_IA32_PERF_GLOBAL_STATUS 0x0000038e > +#define MSR_IA32_PERF_GLOBAL_OVF_CTRL 0x00000390 > +#define MSR_IA32_A_PMC(n) (0x000004c1 + (n))
On 04/25/2017 04:36 AM, Jan Beulich wrote: >>>> On 24.04.17 at 19:54, <mohit.gambhir@oracle.com> wrote: >> --- a/arch/x86/include/arch/msr-index.h >> +++ b/arch/x86/include/arch/msr-index.h >> @@ -38,6 +38,17 @@ >> #define MSR_GS_BASE 0xc0000101 >> #define MSR_SHADOW_GS_BASE 0xc0000102 >> >> +#define MSR_IA32_PMC(n) (0x000000c1 + (n)) >> +#define MSR_IA32_PERFEVTSEL(n) (0x00000186 + (n)) >> +#define MSR_IA32_DEBUGCTL 0x000001d9 > This still sits in the middle of PMU ones, despite not being purely > PMU related. > > Jan Its actually already defined in that file as MSR_DEBUGCTL so I am just going to remove this redefinition. Mohit >> +#define MSR_IA32_FIXED_CTR(n) (0x00000309 + (n)) >> +#define MSR_IA32_PERF_CAPABILITIES 0x00000345 >> +#define MSR_IA32_FIXED_CTR_CTRL 0x0000038d >> +#define MSR_IA32_PERF_GLOBAL_CTRL 0x0000038f >> +#define MSR_IA32_PERF_GLOBAL_STATUS 0x0000038e >> +#define MSR_IA32_PERF_GLOBAL_OVF_CTRL 0x00000390 >> +#define MSR_IA32_A_PMC(n) (0x000004c1 + (n)) >
On 24/04/17 18:54, Mohit Gambhir wrote: > This patch adds Intel PMU MSR addresses as macros for VPMU testing > > Signed-off-by: Mohit Gambhir <mohit.gambhir@oracle.com> > --- > arch/x86/include/arch/msr-index.h | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/arch/x86/include/arch/msr-index.h b/arch/x86/include/arch/msr-index.h > index 2e90079..3a79025 100644 > --- a/arch/x86/include/arch/msr-index.h > +++ b/arch/x86/include/arch/msr-index.h > @@ -38,6 +38,17 @@ > #define MSR_GS_BASE 0xc0000101 > #define MSR_SHADOW_GS_BASE 0xc0000102 > > +#define MSR_IA32_PMC(n) (0x000000c1 + (n)) > +#define MSR_IA32_PERFEVTSEL(n) (0x00000186 + (n)) > +#define MSR_IA32_DEBUGCTL 0x000001d9 I have just recently pushed the LBR/TSX test case, which adds DEBUGCTL. > +#define MSR_IA32_FIXED_CTR(n) (0x00000309 + (n)) > +#define MSR_IA32_PERF_CAPABILITIES 0x00000345 > +#define MSR_IA32_FIXED_CTR_CTRL 0x0000038d > +#define MSR_IA32_PERF_GLOBAL_CTRL 0x0000038f > +#define MSR_IA32_PERF_GLOBAL_STATUS 0x0000038e > +#define MSR_IA32_PERF_GLOBAL_OVF_CTRL 0x00000390 > +#define MSR_IA32_A_PMC(n) (0x000004c1 + (n)) Please drop the IA32 infixes. They only add extra clutter. Please also keep the entire file sorted by MSR index, which will require splitting this block into two. What is the difference between (what will be) MSR_PMC() and MSR_A_PMC() ? ~Andrew
On 04/25/2017 11:24 AM, Andrew Cooper wrote: > On 24/04/17 18:54, Mohit Gambhir wrote: >> This patch adds Intel PMU MSR addresses as macros for VPMU testing >> >> Signed-off-by: Mohit Gambhir <mohit.gambhir@oracle.com> >> --- >> arch/x86/include/arch/msr-index.h | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/arch/x86/include/arch/msr-index.h b/arch/x86/include/arch/msr-index.h >> index 2e90079..3a79025 100644 >> --- a/arch/x86/include/arch/msr-index.h >> +++ b/arch/x86/include/arch/msr-index.h >> @@ -38,6 +38,17 @@ >> #define MSR_GS_BASE 0xc0000101 >> #define MSR_SHADOW_GS_BASE 0xc0000102 >> >> +#define MSR_IA32_PMC(n) (0x000000c1 + (n)) >> +#define MSR_IA32_PERFEVTSEL(n) (0x00000186 + (n)) >> +#define MSR_IA32_DEBUGCTL 0x000001d9 > I have just recently pushed the LBR/TSX test case, which adds DEBUGCTL. OK >> +#define MSR_IA32_FIXED_CTR(n) (0x00000309 + (n)) >> +#define MSR_IA32_PERF_CAPABILITIES 0x00000345 >> +#define MSR_IA32_FIXED_CTR_CTRL 0x0000038d >> +#define MSR_IA32_PERF_GLOBAL_CTRL 0x0000038f >> +#define MSR_IA32_PERF_GLOBAL_STATUS 0x0000038e >> +#define MSR_IA32_PERF_GLOBAL_OVF_CTRL 0x00000390 >> +#define MSR_IA32_A_PMC(n) (0x000004c1 + (n)) > Please drop the IA32 infixes. They only add extra clutter. Please also > keep the entire file sorted by MSR index, which will require splitting > this block into two. OK > What is the difference between (what will be) MSR_PMC() and MSR_A_PMC() ? MSR_A_PMCx are alias addresses to MSR_PMCx that are used for full-width read/write access while MSR_PMCx can only be used for 32 bit read/writes. You can take a look at Intel SDM Vol 3B section 18.2.5 "Full-Width Write to Performance Counter Registers" for details. > > ~Andrew > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > https://lists.xen.org/xen-devel
>>> On 25.04.17 at 17:24, <andrew.cooper3@citrix.com> wrote: > On 24/04/17 18:54, Mohit Gambhir wrote: >> This patch adds Intel PMU MSR addresses as macros for VPMU testing >> >> Signed-off-by: Mohit Gambhir <mohit.gambhir@oracle.com> >> --- >> arch/x86/include/arch/msr-index.h | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/arch/x86/include/arch/msr-index.h > b/arch/x86/include/arch/msr-index.h >> index 2e90079..3a79025 100644 >> --- a/arch/x86/include/arch/msr-index.h >> +++ b/arch/x86/include/arch/msr-index.h >> @@ -38,6 +38,17 @@ >> #define MSR_GS_BASE 0xc0000101 >> #define MSR_SHADOW_GS_BASE 0xc0000102 >> >> +#define MSR_IA32_PMC(n) (0x000000c1 + (n)) >> +#define MSR_IA32_PERFEVTSEL(n) (0x00000186 + (n)) >> +#define MSR_IA32_DEBUGCTL 0x000001d9 > > I have just recently pushed the LBR/TSX test case, which adds DEBUGCTL. > >> +#define MSR_IA32_FIXED_CTR(n) (0x00000309 + (n)) >> +#define MSR_IA32_PERF_CAPABILITIES 0x00000345 >> +#define MSR_IA32_FIXED_CTR_CTRL 0x0000038d >> +#define MSR_IA32_PERF_GLOBAL_CTRL 0x0000038f >> +#define MSR_IA32_PERF_GLOBAL_STATUS 0x0000038e >> +#define MSR_IA32_PERF_GLOBAL_OVF_CTRL 0x00000390 >> +#define MSR_IA32_A_PMC(n) (0x000004c1 + (n)) > > Please drop the IA32 infixes. They only add extra clutter. Please also > keep the entire file sorted by MSR index, which will require splitting > this block into two. Are you sure about this? The infix helps distinguish architectural from non-architectural MSRs (which arguably is a questionable distinction by itself, considering what MSR stands for). Jan
On 26/04/2017 07:07, Jan Beulich wrote: >>>> On 25.04.17 at 17:24, <andrew.cooper3@citrix.com> wrote: >> On 24/04/17 18:54, Mohit Gambhir wrote: >>> This patch adds Intel PMU MSR addresses as macros for VPMU testing >>> >>> Signed-off-by: Mohit Gambhir <mohit.gambhir@oracle.com> >>> --- >>> arch/x86/include/arch/msr-index.h | 11 +++++++++++ >>> 1 file changed, 11 insertions(+) >>> >>> diff --git a/arch/x86/include/arch/msr-index.h >> b/arch/x86/include/arch/msr-index.h >>> index 2e90079..3a79025 100644 >>> --- a/arch/x86/include/arch/msr-index.h >>> +++ b/arch/x86/include/arch/msr-index.h >>> @@ -38,6 +38,17 @@ >>> #define MSR_GS_BASE 0xc0000101 >>> #define MSR_SHADOW_GS_BASE 0xc0000102 >>> >>> +#define MSR_IA32_PMC(n) (0x000000c1 + (n)) >>> +#define MSR_IA32_PERFEVTSEL(n) (0x00000186 + (n)) >>> +#define MSR_IA32_DEBUGCTL 0x000001d9 >> I have just recently pushed the LBR/TSX test case, which adds DEBUGCTL. >> >>> +#define MSR_IA32_FIXED_CTR(n) (0x00000309 + (n)) >>> +#define MSR_IA32_PERF_CAPABILITIES 0x00000345 >>> +#define MSR_IA32_FIXED_CTR_CTRL 0x0000038d >>> +#define MSR_IA32_PERF_GLOBAL_CTRL 0x0000038f >>> +#define MSR_IA32_PERF_GLOBAL_STATUS 0x0000038e >>> +#define MSR_IA32_PERF_GLOBAL_OVF_CTRL 0x00000390 >>> +#define MSR_IA32_A_PMC(n) (0x000004c1 + (n)) >> Please drop the IA32 infixes. They only add extra clutter. Please also >> keep the entire file sorted by MSR index, which will require splitting >> this block into two. > Are you sure about this? The infix helps distinguish architectural > from non-architectural MSRs (which arguably is a questionable > distinction by itself, considering what MSR stands for). The IA32 infix isn't useful because it isn't used consistently; this is a side effect of being Intel-specific notation, and changes over time. (For example, have you ever seen MSR_IA32_STAR referred to by its Intel name?) Currently, the only two MSRs used by the common XTF code are MSR_EFER and the hypercall writing MSR as found in the Xen CPUID leaves. I don't expect this to really change, meaning that all MSR use is local to the test in question, and that is in a better position to know how/when to use them. ~Andrew
diff --git a/arch/x86/include/arch/msr-index.h b/arch/x86/include/arch/msr-index.h index 2e90079..3a79025 100644 --- a/arch/x86/include/arch/msr-index.h +++ b/arch/x86/include/arch/msr-index.h @@ -38,6 +38,17 @@ #define MSR_GS_BASE 0xc0000101 #define MSR_SHADOW_GS_BASE 0xc0000102 +#define MSR_IA32_PMC(n) (0x000000c1 + (n)) +#define MSR_IA32_PERFEVTSEL(n) (0x00000186 + (n)) +#define MSR_IA32_DEBUGCTL 0x000001d9 +#define MSR_IA32_FIXED_CTR(n) (0x00000309 + (n)) +#define MSR_IA32_PERF_CAPABILITIES 0x00000345 +#define MSR_IA32_FIXED_CTR_CTRL 0x0000038d +#define MSR_IA32_PERF_GLOBAL_CTRL 0x0000038f +#define MSR_IA32_PERF_GLOBAL_STATUS 0x0000038e +#define MSR_IA32_PERF_GLOBAL_OVF_CTRL 0x00000390 +#define MSR_IA32_A_PMC(n) (0x000004c1 + (n)) + #endif /* XFT_X86_MSR_INDEX_H */ /*
This patch adds Intel PMU MSR addresses as macros for VPMU testing Signed-off-by: Mohit Gambhir <mohit.gambhir@oracle.com> --- arch/x86/include/arch/msr-index.h | 11 +++++++++++ 1 file changed, 11 insertions(+)