Thanks John for doing this. It really is too bad we can't check for SMI counter support at runtime instead of having to hardcode the families like this. And I guess there is no way to access the constants defined in linux/arch/x86/include/asm/intel-family.h? I don't see any devel package for that. My main comment is that the turbostat.c checks also rely on intel_model_duplicates which collapses some of the families before probe_nhm_msrs is called. Looking at intel_model_duplicates, I think we're still missing some families: #define INTEL_FAM6_COMETLAKE 0xA5 /* Sky Lake */ #define INTEL_FAM6_COMETLAKE_L 0xA6 /* Sky Lake */ #define INTEL_FAM6_ICELAKE_L 0x7E /* Sunny Cove */ #define INTEL_FAM6_ICELAKE_NNPI 0x9D /* Sunny Cove */ #define INTEL_FAM6_TIGERLAKE_L 0x8C /* Willow Cove */ #define INTEL_FAM6_TIGERLAKE 0x8D /* Willow Cove */ #define INTEL_FAM6_ROCKETLAKE 0xA7 /* Cypress Cove */ #define INTEL_FAM6_LAKEFIELD 0x8A /* Sunny Cove / Tremont */ #define INTEL_FAM6_ALDERLAKE 0x97 /* Golden Cove / Gracemont */ #define INTEL_FAM6_ALDERLAKE_L 0x9A /* Golden Cove / Gracemont */ #define INTEL_FAM6_ATOM_TREMONT_L 0x9C /* Jasper Lake */ #define INTEL_FAM6_ICELAKE_D 0x6C /* Sunny Cove */ #define INTEL_FAM6_SAPPHIRERAPIDS_X 0x8F /* Golden Cove */ To make this easier to maintain in the future it might be nice to do the check in two parts - the first to eliminate the duplicates in the same way intel_model_duplicates does it and then the second part to check the reduced set (basically the existing switch statement. Maybe that's too much? Also, might be nice to update all the comments to include the constants from intel-family.h (I noticed you didn't update the first group in the switch statement). Bart On Thu, Apr 7, 2022 at 2:23 PM John Kacur <jkacur@xxxxxxxxxx> wrote: > > Update has_smi_counter() to include > > case 0x66: /* INTEL_FAM6_CANNONLAKE_L, Palm Cove */ > case 0x6A: /* INTEL_FAM6_ICELAKE_X */ > case 0x96: /* INTEL_FAM6_ATOM_TREMONT Elkhart Lake */ > case 0x86: /* INTEL_FAM6_ATOM_TREMONT_D, Jacobsville */ > > This function is based on probe_nhm_msrs() in file turbostat.c in the > linux kernel. > > This update also reorders the model numbers to more closely match the > order in probe_nhm_msrs, and adds comments to match symbolic names for > the model numbers. > > Reported-by: Bart Wensley <bwensley@xxxxxxxxxx> > Signed-off-by: John Kacur <jkacur@xxxxxxxxxx> > --- > src/cyclictest/cyclictest.c | 50 ++++++++++++++++++++----------------- > 1 file changed, 27 insertions(+), 23 deletions(-) > > diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c > index c9ed9e08f6e1..a4979ad81f21 100644 > --- a/src/cyclictest/cyclictest.c > +++ b/src/cyclictest/cyclictest.c > @@ -440,31 +440,35 @@ static int has_smi_counter(void) > case 0x2C: /* Westmere EP - Gulftown */ > case 0x2E: /* Nehalem-EX Xeon - Beckton */ > case 0x2F: /* Westmere-EX Xeon - Eagleton */ > - case 0x2A: /* SNB */ > - case 0x2D: /* SNB Xeon */ > - case 0x3A: /* IVB */ > - case 0x3E: /* IVB Xeon */ > - case 0x3C: /* HSW */ > - case 0x3F: /* HSX */ > - case 0x45: /* HSW */ > - case 0x46: /* HSW */ > - case 0x3D: /* BDW */ > - case 0x47: /* BDW */ > - case 0x4F: /* BDX */ > - case 0x56: /* BDX-DE */ > - case 0x4E: /* SKL */ > - case 0x5E: /* SKL */ > + case 0x2A: /* INTEL_FAM6_SANDYBRIDGE, SNB */ > + case 0x2D: /* INTEL_FAM6_SANDYBRIDGE_X, SNB Xeon */ > + case 0x3A: /* INTEL_FAM6_IVYBRIDGE, IVB */ > + case 0x3E: /* INTEL_FAM6_IVYBRIDGE_X, IVB Xeon */ > + case 0x3C: /* INTEL_FAM6_HASWELL, HSW */ > + case 0x46: /* INTEL_FAM6_HASWELL_G, HSW */ > + case 0x3F: /* INTEL_FAM6_HASWELL_X, HSX */ > + case 0x45: /* INTEL_FAM6_HASWELL_L, HSW */ > + case 0x3D: /* INTEL_FAM6_BROADWELL, BDW */ > + case 0x47: /* INTEL_FAM6_BROADWELL_G, BDW */ > + case 0x4F: /* INTEL_FAM6_BROADWELL_X, BDX */ > + case 0x56: /* INTEL_FAM6_BROADWELL_D, BDX-DE */ > + case 0x4E: /* INTEL_FAM6_SKYLAKE_L SKL */ > + case 0x66: /* INTEL_FAM6_CANNONLAKE_L, Palm Cove */ > + case 0x55: /* INTEL_FAM6_SKYLAKE_X, SKX */ > + case 0x6A: /* INTEL_FAM6_ICELAKE_X */ > + case 0x5E: /* INTEL_FAM6_SKYLAKE, SKL */ > case 0x8E: /* KBL */ > case 0x9E: /* KBL */ > - case 0x55: /* SKX */ > - case 0x37: /* BYT */ > - case 0x4D: /* AVN */ > - case 0x4C: /* AMT */ > - case 0x57: /* PHI */ > - case 0x5C: /* BXT */ > - case 0x5F: /* DNV */ > - case 0x7A: /* Gemini Lake */ > - case 0x85: /* Knights Mill */ > + case 0x37: /* INTEL_FAM6_ATOM_SILVERMONT, BYT */ > + case 0x4D: /* INTEL_FAM6_ATOM_SILVERMONT_D, AVN */ > + case 0x4C: /* INTEL_FAM6_ATOM_AIRMONT, AMT */ > + case 0x57: /* INTEL_FAM6_XEON_PHI_KNL, PHI */ > + case 0x5C: /* INTEL_FAM6_ATOM_GOLDMONT, BXT, Apollo Lake */ > + case 0x7A: /* INTEL_FAM6_ATOM_GOLDMONT_PLUS, Gemini Lake */ > + case 0x5F: /* INTEL_FAM6_ATOM_GOLDMONT_D, DNV, Denverton */ > + case 0x96: /* INTEL_FAM6_ATOM_TREMONT Elkhart Lake */ > + case 0x86: /* INTEL_FAM6_ATOM_TREMONT_D, Jacobsville */ > + case 0x85: /* INTEL_FAM6_XEON_PHI_KNM, Knights Mill */ > break; > default: > return 0; > -- > 2.35.1 >
