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

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

On 21/05/2014 22:28, Aravind Gopalakrishnan wrote:
> When HW tries to load a corrupted patch, it generates #GP
> and hangs the system. Use wrmsr_safe instead so that we
> fail to load microcode gracefully.
>
> Example on a Fam15h system-
> (XEN) microcode: CPU0 collect_cpu_info: patch_id=0x6000626
> (XEN) microcode: CPU0 size 7870, block size 2586 offset 76 equivID
> 0x6012 rev 0x6000637
> (XEN) microcode: CPU0 found a matching microcode update with version
> 0x6000637 (current=0x6000626)
> (XEN) traps.c:3073: GPF (0000): ffff82d08016f682 -> ffff82d08022d9f8
> (XEN) microcode: CPU0 update from revision 0x6000637 to 0x6000626 failed
>
> Signed-off-by: Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com>

I suspect that you will find with a *very* up-to-date master tree, an
early #GP should result in panic() instead of a hang, following
279840d5ea646  (If it doesn't I would be interested to know)

As for the patch itself, is it worth using the failure to purge this
patch from the cache? Nothing good can come of attempting to load the
same patch on a different cpu.

~Andrew

> ---
>  xen/arch/x86/microcode_amd.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c
> index e83f4b6..23637e2 100644
> --- a/xen/arch/x86/microcode_amd.c
> +++ b/xen/arch/x86/microcode_amd.c
> @@ -191,7 +191,7 @@ static int apply_microcode(int cpu)
>  
>      spin_lock_irqsave(&microcode_update_lock, flags);
>  
> -    wrmsrl(MSR_AMD_PATCHLOADER, (unsigned long)hdr);
> +    wrmsr_safe(MSR_AMD_PATCHLOADER, (unsigned long)hdr);
>  
>      /* get patch id after patching */
>      rdmsrl(MSR_AMD_PATCHLEVEL, rev);