Re: [Xen-devel] [PATCH] x86/amd: Protect set_cpuidmask() against #GP faults

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

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 <andrew.cooper3@xxxxxxxxxx>
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
> CC: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@xxxxxxx>
> CC: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> ---

Reviewed-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>


>   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");
>         }
>   }
>   

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