Re: [PATCH 2/2] x86, amd_ucode: Safeguard against #GP

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 30/07/14 17:13, Aravind Gopalakrishnan wrote:
> When HW tries to load a corrupted patch, it generates #GP
> and depending on 'noreboot' parameter on grub, the system
> is either stuck in a reboot loop or is hung. Use wrmsr_safe
> instead of wrmsrl so that we fail to load microcode gracefully.
>
> Signed-off-by: Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com>
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

> ---
>  xen/arch/x86/microcode_amd.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c
> index a2bdd20..57646c4 100644
> --- a/xen/arch/x86/microcode_amd.c
> +++ b/xen/arch/x86/microcode_amd.c
> @@ -193,6 +193,7 @@ static int apply_microcode(int cpu)
>      uint32_t rev;
>      struct microcode_amd *mc_amd = uci->mc.mc_amd;
>      struct microcode_header_amd *hdr;
> +    int hw_err;
>  
>      /* We should bind the task to the CPU */
>      BUG_ON(raw_smp_processor_id() != cpu);
> @@ -206,7 +207,7 @@ static int apply_microcode(int cpu)
>  
>      spin_lock_irqsave(&microcode_update_lock, flags);
>  
> -    wrmsrl(MSR_AMD_PATCHLOADER, (unsigned long)hdr);
> +    hw_err = wrmsr_safe(MSR_AMD_PATCHLOADER, (unsigned long)hdr);
>  
>      /* get patch id after patching */
>      rdmsrl(MSR_AMD_PATCHLEVEL, rev);
> @@ -214,7 +215,7 @@ static int apply_microcode(int cpu)
>      spin_unlock_irqrestore(&microcode_update_lock, flags);
>  
>      /* check current patch id and patch's id for match */
> -    if ( rev != hdr->patch_id )
> +    if ( hw_err || (rev != hdr->patch_id) )
>      {
>          printk(KERN_ERR "microcode: CPU%d update from revision "
>                 "%#x to %#x failed\n", cpu, rev, hdr->patch_id);