Re: Builtin microcode does nothing..

[Gabriel C]

On 26.05.2016 12:03, Borislav Petkov wrote:
> On Thu, May 26, 2016 at 01:36:51AM +0200, Gabriel C wrote:
> >   Hangs on the second box too also..
> Ok, please try the diff below ontop to see if it fixes your issue.
>
> Looks like I'd need to rewrite the figuring out where the microcode data
> is. Currently, it is ugly and error prone.

 With this patch ontop your your tip brach all is fine.
 Tested on both server and both are up and running.


> ---
> diff --git a/arch/x86/include/asm/microcode.h b/arch/x86/include/asm/microcode.h
> index 01c2d14ec05f..4bee5bdbaf2c 100644
> --- a/arch/x86/include/asm/microcode.h
> +++ b/arch/x86/include/asm/microcode.h
> @@ -143,8 +143,13 @@ static inline bool
>   get_builtin_firmware(struct cpio_data *cd, const char *name)	{ return false; }
>   #endif
>   
> +static bool initrd_valid;
> +
>   static inline unsigned long get_initrd_start(void)
>   {
> +	if (!initrd_valid)
> +		return 0;
> +
>   #ifdef CONFIG_BLK_DEV_INITRD
>   	return initrd_start;
>   #else
> @@ -154,6 +159,9 @@ static inline unsigned long get_initrd_start(void)
>   
>   static inline unsigned long get_initrd_start_addr(void)
>   {
> +	if (!initrd_valid)
> +		return 0;
> +
>   #ifdef CONFIG_BLK_DEV_INITRD
>   #ifdef CONFIG_X86_32
>   	unsigned long *initrd_start_p = (unsigned long *)__pa_nodebug(&initrd_start);
> diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
> index 43f7caff4749..7ed06c397e2b 100644
> --- a/arch/x86/kernel/cpu/microcode/intel.c
> +++ b/arch/x86/kernel/cpu/microcode/intel.c
> @@ -55,6 +55,8 @@ static struct mc_saved_data {
>   	struct microcode_intel **mc_saved;
>   } mc_saved_data;
>   
> +static bool initrd_valid;
> +
>   /* Go through saved patches and find the one suitable for the current CPU. */
>   static enum ucode_state
>   find_microcode_patch(struct microcode_intel **saved,
> @@ -533,12 +535,10 @@ static bool __init load_builtin_intel_microcode(struct cpio_data *cp)
>   
>   static __init enum ucode_state
>   scan_microcode(struct mc_saved_data *mcs, unsigned long *mc_ptrs,
> -	       unsigned long *start, unsigned long size,
> +	       unsigned long start, unsigned long size,
>   	       struct ucode_cpu_info *uci)
>   {
> -	struct cpio_data cd;
> -	cd.data = NULL;
> -	cd.size = 0;
> +	struct cpio_data cd = { NULL, 0, "" };
>   
>   	/* try built-in microcode first */
>   	if (load_builtin_intel_microcode(&cd))
> @@ -547,21 +547,23 @@ scan_microcode(struct mc_saved_data *mcs, unsigned long *mc_ptrs,
>   		 * the boot loader, by mistake or simply forgotten there. That's
>   		 * fine, we ignore it since we've found builtin microcode.
>   		 */
> -		*start = 0;
> +		initrd_valid = false;
>   	else {
>   #ifdef CONFIG_BLK_DEV_INITRD
>   		static __initdata char ucode_name[] = "kernel/x86/microcode/GenuineIntel.bin";
>   		char *p = IS_ENABLED(CONFIG_X86_32) ? (char *)__pa_nodebug(ucode_name)
>   						    : ucode_name;
>   
> -		cd = find_cpio_data(p, (void *)*start, size, NULL);
> -		if (!cd.data)
> +		cd = find_cpio_data(p, (void *)start, size, NULL);
> +		if (cd.data)
> +			initrd_valid = true;
> +		else
>   #endif
>   			return UCODE_ERROR;
>   	}
>   
> -	return get_matching_model_microcode(*start, cd.data, cd.size,
> -					    mcs, mc_ptrs, uci);
> +	return get_matching_model_microcode(initrd_valid ? start : 0,
> +					    cd.data, cd.size, mcs, mc_ptrs, uci);
>   }
>   
>   /*
> @@ -703,20 +705,16 @@ static void __init
>   _load_ucode_intel_bsp(struct mc_saved_data *mcs, unsigned long *mc_ptrs,
>   		      unsigned long start, unsigned long size)
>   {
> -	unsigned long _start = start;
>   	struct ucode_cpu_info uci;
>   	enum ucode_state ret;
>   
>   	collect_cpu_info_early(&uci);
>   
> -	ret = scan_microcode(mcs, mc_ptrs, &_start, size, &uci);
> +	ret = scan_microcode(mcs, mc_ptrs, start, size, &uci);
>   	if (ret != UCODE_OK)
>   		return;
>   
> -	/* Pass updated starting address of blobs to the next routine.  */
> -	start = _start;
> -
> -	ret = load_microcode(mcs, mc_ptrs, start, &uci);
> +	ret = load_microcode(mcs, mc_ptrs, initrd_valid ? start : 0, &uci);
>   	if (ret != UCODE_OK)
>   		return;
>