From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v2 3/3] x86/AMD: clean up pre-canned family/revision handling for CPUID masking Date: Mon, 7 Apr 2014 11:48:58 +0100 Message-ID: <5342829A.8050900@citrix.com> References: <53428DCC020000780000607D@nat28.tlf.novell.com> <53428F7402000078000060BF@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============4082243587826396229==" Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WX76t-0002Oi-Ue for xen-devel@lists.xenproject.org; Mon, 07 Apr 2014 10:49:04 +0000 In-Reply-To: <53428F7402000078000060BF@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel , Keir Fraser , Aravind Gopalakrishnan , suravee.suthikulpanit@amd.com List-Id: xen-devel@lists.xenproject.org --===============4082243587826396229== Content-Type: multipart/alternative; boundary="------------040903060201090705010806" --------------040903060201090705010806 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit On 07/04/14 10:43, Jan Beulich wrote: > Make it so this is easier to extend, and move the parsing code/data > into .init.* sections. > > Signed-off-by: Jan Beulich > Reviewed-by: Aravind Gopalakrishnan > > --- a/xen/arch/x86/cpu/amd.c > +++ b/xen/arch/x86/cpu/amd.c > @@ -91,6 +91,51 @@ static inline int wrmsr_amd_safe(unsigne > return err; > } > > +static const struct cpuidmask { > + uint16_t fam; > + char rev[2]; > + unsigned int ecx, edx, ext_ecx, ext_edx; > +} pre_canned[] __initconst = { possibly name the structure amd_precanned_featuremasks or similar, to make it a little more descriptive than just "pre_canned" ? > +#define CAN(fam, id, rev) { \ > + fam, #rev, \ > + AMD_FEATURES_##id##_REV_##rev##_ECX, \ > + AMD_FEATURES_##id##_REV_##rev##_EDX, \ > + AMD_EXTFEATURES_##id##_REV_##rev##_ECX, \ > + AMD_EXTFEATURES_##id##_REV_##rev##_EDX \ > + } > +#define CAN_FAM(fam, rev) CAN(0x##fam, FAM##fam##h, rev) > +#define CAN_K8(rev) CAN(0x0f, K8, rev) > + CAN_FAM(11, B), > + CAN_FAM(10, C), > + CAN_FAM(10, B), > + CAN_K8(G), > + CAN_K8(F), > + CAN_K8(E), > + CAN_K8(D), > + CAN_K8(C) > +#undef CAN > +}; > + > +static const struct cpuidmask *__init noinline get_cpuidmask(const char *opt) > +{ > + unsigned long fam; > + char rev; > + unsigned int i; > + > + if (strncmp(opt, "fam_", 4)) > + return NULL; > + fam = simple_strtoul(opt + 4, &opt, 16); > + if (strncmp(opt, "_rev_", 5)) > + return NULL; > + rev = toupper(opt[5]); if ( opt[6] != '\0' ) return NULL; Otherwise this code will start accepting malformed values it would previously discard. ~Andrew > + > + for (i = 0; i < ARRAY_SIZE(pre_canned); ++i) > + if (fam == pre_canned[i].fam && rev == *pre_canned[i].rev) > + return &pre_canned[i]; > + > + return NULL; > +} > + > /* > * Mask the features and extended features returned by CPUID. Parameters are > * set from the boot line via two methods: > @@ -136,50 +181,18 @@ static void __devinit set_cpuidmask(cons > thermal_ecx = opt_cpuid_mask_thermal_ecx; > } else if (*opt_famrev == '\0') { > return; > - } else if (!strcmp(opt_famrev, "fam_0f_rev_c")) { > - feat_ecx = AMD_FEATURES_K8_REV_C_ECX; > - feat_edx = AMD_FEATURES_K8_REV_C_EDX; > - extfeat_ecx = AMD_EXTFEATURES_K8_REV_C_ECX; > - extfeat_edx = AMD_EXTFEATURES_K8_REV_C_EDX; > - } else if (!strcmp(opt_famrev, "fam_0f_rev_d")) { > - feat_ecx = AMD_FEATURES_K8_REV_D_ECX; > - feat_edx = AMD_FEATURES_K8_REV_D_EDX; > - extfeat_ecx = AMD_EXTFEATURES_K8_REV_D_ECX; > - extfeat_edx = AMD_EXTFEATURES_K8_REV_D_EDX; > - } else if (!strcmp(opt_famrev, "fam_0f_rev_e")) { > - feat_ecx = AMD_FEATURES_K8_REV_E_ECX; > - feat_edx = AMD_FEATURES_K8_REV_E_EDX; > - extfeat_ecx = AMD_EXTFEATURES_K8_REV_E_ECX; > - extfeat_edx = AMD_EXTFEATURES_K8_REV_E_EDX; > - } else if (!strcmp(opt_famrev, "fam_0f_rev_f")) { > - feat_ecx = AMD_FEATURES_K8_REV_F_ECX; > - feat_edx = AMD_FEATURES_K8_REV_F_EDX; > - extfeat_ecx = AMD_EXTFEATURES_K8_REV_F_ECX; > - extfeat_edx = AMD_EXTFEATURES_K8_REV_F_EDX; > - } else if (!strcmp(opt_famrev, "fam_0f_rev_g")) { > - feat_ecx = AMD_FEATURES_K8_REV_G_ECX; > - feat_edx = AMD_FEATURES_K8_REV_G_EDX; > - extfeat_ecx = AMD_EXTFEATURES_K8_REV_G_ECX; > - extfeat_edx = AMD_EXTFEATURES_K8_REV_G_EDX; > - } else if (!strcmp(opt_famrev, "fam_10_rev_b")) { > - feat_ecx = AMD_FEATURES_FAM10h_REV_B_ECX; > - feat_edx = AMD_FEATURES_FAM10h_REV_B_EDX; > - extfeat_ecx = AMD_EXTFEATURES_FAM10h_REV_B_ECX; > - extfeat_edx = AMD_EXTFEATURES_FAM10h_REV_B_EDX; > - } else if (!strcmp(opt_famrev, "fam_10_rev_c")) { > - feat_ecx = AMD_FEATURES_FAM10h_REV_C_ECX; > - feat_edx = AMD_FEATURES_FAM10h_REV_C_EDX; > - extfeat_ecx = AMD_EXTFEATURES_FAM10h_REV_C_ECX; > - extfeat_edx = AMD_EXTFEATURES_FAM10h_REV_C_EDX; > - } else if (!strcmp(opt_famrev, "fam_11_rev_b")) { > - feat_ecx = AMD_FEATURES_FAM11h_REV_B_ECX; > - feat_edx = AMD_FEATURES_FAM11h_REV_B_EDX; > - extfeat_ecx = AMD_EXTFEATURES_FAM11h_REV_B_ECX; > - extfeat_edx = AMD_EXTFEATURES_FAM11h_REV_B_EDX; > } else { > - printk("Invalid processor string: %s\n", opt_famrev); > - printk("CPUID will not be masked\n"); > - return; > + const struct cpuidmask *m = get_cpuidmask(opt_famrev); > + > + if (!m) { > + printk("Invalid processor string: %s\n", opt_famrev); > + printk("CPUID will not be masked\n"); > + return; > + } > + feat_ecx = m->ecx; > + feat_edx = m->edx; > + extfeat_ecx = m->ext_ecx; > + extfeat_edx = m->ext_edx; > } > > /* Setting bits in the CPUID mask MSR that are not set in the > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel --------------040903060201090705010806 Content-Type: text/html; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit
On 07/04/14 10:43, Jan Beulich wrote:
Make it so this is easier to extend, and move the parsing code/data
into .init.* sections.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Aravind Gopalakrishnan<aravind.gopalakrishnan@amd.com>

--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -91,6 +91,51 @@ static inline int wrmsr_amd_safe(unsigne
 	return err;
 }
 
+static const struct cpuidmask {
+	uint16_t fam;
+	char rev[2];
+	unsigned int ecx, edx, ext_ecx, ext_edx;
+} pre_canned[] __initconst = {

possibly name the structure amd_precanned_featuremasks or similar, to make it a little more descriptive than just "pre_canned" ?

+#define CAN(fam, id, rev) { \
+		fam, #rev, \
+		AMD_FEATURES_##id##_REV_##rev##_ECX, \
+		AMD_FEATURES_##id##_REV_##rev##_EDX, \
+		AMD_EXTFEATURES_##id##_REV_##rev##_ECX, \
+		AMD_EXTFEATURES_##id##_REV_##rev##_EDX \
+	}
+#define CAN_FAM(fam, rev) CAN(0x##fam, FAM##fam##h, rev)
+#define CAN_K8(rev)       CAN(0x0f,    K8,          rev)
+	CAN_FAM(11, B),
+	CAN_FAM(10, C),
+	CAN_FAM(10, B),
+	CAN_K8(G),
+	CAN_K8(F),
+	CAN_K8(E),
+	CAN_K8(D),
+	CAN_K8(C)
+#undef CAN
+};
+
+static const struct cpuidmask *__init noinline get_cpuidmask(const char *opt)
+{
+	unsigned long fam;
+	char rev;
+	unsigned int i;
+
+	if (strncmp(opt, "fam_", 4))
+		return NULL;
+	fam = simple_strtoul(opt + 4, &opt, 16);
+	if (strncmp(opt, "_rev_", 5))
+		return NULL;
+	rev = toupper(opt[5]);

if ( opt[6] != '\0' )
   return NULL;

Otherwise this code will start accepting malformed values it would previously discard.

~Andrew

+
+	for (i = 0; i < ARRAY_SIZE(pre_canned); ++i)
+		if (fam == pre_canned[i].fam && rev == *pre_canned[i].rev)
+			return &pre_canned[i];
+
+	return NULL;
+}
+
 /*
  * Mask the features and extended features returned by CPUID.  Parameters are
  * set from the boot line via two methods:
@@ -136,50 +181,18 @@ static void __devinit set_cpuidmask(cons
 		thermal_ecx = opt_cpuid_mask_thermal_ecx;
 	} else if (*opt_famrev == '\0') {
 		return;
-	} else if (!strcmp(opt_famrev, "fam_0f_rev_c")) {
-		feat_ecx = AMD_FEATURES_K8_REV_C_ECX;
-		feat_edx = AMD_FEATURES_K8_REV_C_EDX;
-		extfeat_ecx = AMD_EXTFEATURES_K8_REV_C_ECX;
-		extfeat_edx = AMD_EXTFEATURES_K8_REV_C_EDX;
-	} else if (!strcmp(opt_famrev, "fam_0f_rev_d")) {
-		feat_ecx = AMD_FEATURES_K8_REV_D_ECX;
-		feat_edx = AMD_FEATURES_K8_REV_D_EDX;
-		extfeat_ecx = AMD_EXTFEATURES_K8_REV_D_ECX;
-		extfeat_edx = AMD_EXTFEATURES_K8_REV_D_EDX;
-	} else if (!strcmp(opt_famrev, "fam_0f_rev_e")) {
-		feat_ecx = AMD_FEATURES_K8_REV_E_ECX;
-		feat_edx = AMD_FEATURES_K8_REV_E_EDX;
-		extfeat_ecx = AMD_EXTFEATURES_K8_REV_E_ECX;
-		extfeat_edx = AMD_EXTFEATURES_K8_REV_E_EDX;
-	} else if (!strcmp(opt_famrev, "fam_0f_rev_f")) {
-		feat_ecx = AMD_FEATURES_K8_REV_F_ECX;
-		feat_edx = AMD_FEATURES_K8_REV_F_EDX;
-		extfeat_ecx = AMD_EXTFEATURES_K8_REV_F_ECX;
-		extfeat_edx = AMD_EXTFEATURES_K8_REV_F_EDX;
-	} else if (!strcmp(opt_famrev, "fam_0f_rev_g")) {
-		feat_ecx = AMD_FEATURES_K8_REV_G_ECX;
-		feat_edx = AMD_FEATURES_K8_REV_G_EDX;
-		extfeat_ecx = AMD_EXTFEATURES_K8_REV_G_ECX;
-		extfeat_edx = AMD_EXTFEATURES_K8_REV_G_EDX;
-	} else if (!strcmp(opt_famrev, "fam_10_rev_b")) {
-		feat_ecx = AMD_FEATURES_FAM10h_REV_B_ECX;
-		feat_edx = AMD_FEATURES_FAM10h_REV_B_EDX;
-		extfeat_ecx = AMD_EXTFEATURES_FAM10h_REV_B_ECX;
-		extfeat_edx = AMD_EXTFEATURES_FAM10h_REV_B_EDX;
-	} else if (!strcmp(opt_famrev, "fam_10_rev_c")) {
-		feat_ecx = AMD_FEATURES_FAM10h_REV_C_ECX;
-		feat_edx = AMD_FEATURES_FAM10h_REV_C_EDX;
-		extfeat_ecx = AMD_EXTFEATURES_FAM10h_REV_C_ECX;
-		extfeat_edx = AMD_EXTFEATURES_FAM10h_REV_C_EDX;
-	} else if (!strcmp(opt_famrev, "fam_11_rev_b")) {
-		feat_ecx = AMD_FEATURES_FAM11h_REV_B_ECX;
-		feat_edx = AMD_FEATURES_FAM11h_REV_B_EDX;
-		extfeat_ecx = AMD_EXTFEATURES_FAM11h_REV_B_ECX;
-		extfeat_edx = AMD_EXTFEATURES_FAM11h_REV_B_EDX;
 	} else {
-		printk("Invalid processor string: %s\n", opt_famrev);
-		printk("CPUID will not be masked\n");
-		return;
+		const struct cpuidmask *m = get_cpuidmask(opt_famrev);
+
+		if (!m) {
+			printk("Invalid processor string: %s\n", opt_famrev);
+			printk("CPUID will not be masked\n");
+			return;
+		}
+		feat_ecx = m->ecx;
+		feat_edx = m->edx;
+		extfeat_ecx = m->ext_ecx;
+		extfeat_edx = m->ext_edx;
 	}
 
         /* Setting bits in the CPUID mask MSR that are not set in the





_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

--------------040903060201090705010806-- --===============4082243587826396229== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============4082243587826396229==--