From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH] x86/amd: Protect set_cpuidmask() against #GP faults Date: Thu, 05 Jun 2014 09:27:03 -0400 Message-ID: <53907027.3050605@oracle.com> References: <1401967206-27715-1-git-send-email-andrew.cooper3@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1401967206-27715-1-git-send-email-andrew.cooper3@citrix.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: Andrew Cooper Cc: Aravind Gopalakrishnan , Suravee Suthikulpanit , Jan Beulich , Xen-devel List-Id: xen-devel@lists.xenproject.org On 06/05/2014 07:20 AM, Andrew Cooper wrote: > Virtual environments such as Xen HVM containers and VirtualBox do not > necessarily provide support for feature masking MSRs. > > As their presence is detected by model numbers alone, and their use predicated > on command line parameters, use the safe() variants of {wr,rd}msr() to avoid > dying with an early #GP fault. > > In fact, use the password variants in all cases because: > a) they are safe to use even if not strictly required > b) have a more useful function prototype for this purposes > > Signed-off-by: Andrew Cooper > CC: Jan Beulich > CC: Suravee Suthikulpanit > CC: Aravind Gopalakrishnan > CC: Boris Ostrovsky > --- Reviewed-by: Boris Ostrovsky > xen/arch/x86/cpu/amd.c | 41 ++++++++++++++++++++++++++--------------- > 1 file changed, 26 insertions(+), 15 deletions(-) > > diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c > index ea158cb..dc38d69 100644 > --- a/xen/arch/x86/cpu/amd.c > +++ b/xen/arch/x86/cpu/amd.c > @@ -151,7 +151,7 @@ static void __devinit set_cpuidmask(const struct cpuinfo_x86 *c) > static unsigned int extfeat_ecx, extfeat_edx; > static unsigned int l7s0_eax, l7s0_ebx; > static unsigned int thermal_ecx; > - static bool_t skip_l7s0_eax_ebx, skip_thermal_ecx; > + static bool_t skip_feat, skip_extfeat, skip_l7s0, skip_thermal; > static enum { not_parsed, no_mask, set_mask } status; > unsigned int eax, ebx, ecx, edx; > > @@ -220,7 +220,7 @@ static void __devinit set_cpuidmask(const struct cpuinfo_x86 *c) > printk("Writing CPUID leaf 7 subleaf 0 feature mask EAX:EBX -> %08Xh:%08Xh\n", > l7s0_eax, l7s0_ebx); > } else > - skip_l7s0_eax_ebx = 1; > + skip_l7s0 = 1; > > /* Only Fam15 has the respective MSR. */ > ecx = c->x86 == 0x15 && c->cpuid_level >= 6 ? cpuid_ecx(6) : 0; > @@ -229,22 +229,33 @@ static void __devinit set_cpuidmask(const struct cpuinfo_x86 *c) > printk("Writing CPUID thermal/power feature mask ECX -> %08Xh\n", > thermal_ecx); > } else > - skip_thermal_ecx = 1; > + skip_thermal = 1; > > setmask: > /* AMD processors prior to family 10h required a 32-bit password */ > - if (c->x86 >= 0x10) { > - wrmsr(MSR_K8_FEATURE_MASK, feat_edx, feat_ecx); > - wrmsr(MSR_K8_EXT_FEATURE_MASK, extfeat_edx, extfeat_ecx); > - if (!skip_l7s0_eax_ebx) > - wrmsr(MSR_AMD_L7S0_FEATURE_MASK, l7s0_ebx, l7s0_eax); > - if (!skip_thermal_ecx) { > - rdmsr(MSR_AMD_THRM_FEATURE_MASK, eax, edx); > - wrmsr(MSR_AMD_THRM_FEATURE_MASK, thermal_ecx, edx); > - } > - } else { > - wrmsr_amd(MSR_K8_FEATURE_MASK, feat_edx, feat_ecx); > - wrmsr_amd(MSR_K8_EXT_FEATURE_MASK, extfeat_edx, extfeat_ecx); > + if (!skip_feat && > + wrmsr_amd_safe(MSR_K8_FEATURE_MASK, feat_edx, feat_ecx)) { > + skip_feat = 1; > + printk("Failed to set CPUID feature mask\n"); > + } > + > + if (!skip_extfeat && > + wrmsr_amd_safe(MSR_K8_EXT_FEATURE_MASK, extfeat_edx, extfeat_ecx)) { > + skip_extfeat = 1; > + printk("Failed to set CPUID extended feature mask\n"); > + } > + > + if (!skip_l7s0 && > + wrmsr_amd_safe(MSR_AMD_L7S0_FEATURE_MASK, l7s0_ebx, l7s0_eax)) { > + skip_l7s0 = 1; > + printk("Failed to set CPUID leaf 7 subleaf 0 feature mask\n"); > + } > + > + if (!skip_thermal && > + (rdmsr_amd_safe(MSR_AMD_THRM_FEATURE_MASK, &eax, &edx) || > + wrmsr_amd_safe(MSR_AMD_THRM_FEATURE_MASK, thermal_ecx, edx))){ > + skip_thermal = 1; > + printk("Failed to set CPUID thermal/power feature mask\n"); > } > } >