Re: [PATCH 1/1] x86/microcode: vsnprintf() might be unavailable

[Andy Shevchenko]

[+ Mika, who also somehow involved in the investigation]

On Tue, 2015-06-02 at 17:56 +0300, Andy Shevchenko wrote:
> On Intel CPUs with x32 kernels we call load_builtin_intel_microcode() from
> head_32.S on quite earlier stage. At that point sprintf() might be out of scope
> to be called. As a result the 32-bit kernel does not boot on Intel CPUs. It has
> been tested on Braswell and Merrifield.
> 

Small update below.

> The patch changes sprintf() call to plain code which does the same in this
> particular case.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
>  arch/x86/kernel/cpu/microcode/intel_early.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/microcode/intel_early.c b/arch/x86/kernel/cpu/microcode/intel_early.c
> index 10dff3f..a250f41 100644
> --- a/arch/x86/kernel/cpu/microcode/intel_early.c
> +++ b/arch/x86/kernel/cpu/microcode/intel_early.c
> @@ -523,9 +523,9 @@ EXPORT_SYMBOL_GPL(save_mc_for_early);
>  
>  static bool __init load_builtin_intel_microcode(struct cpio_data *cp)
>  {
> -	u32 eax = 0x00000001, ebx, ecx = 0, edx;
> -	int family, model, stepping;
> -	char name[30];
> +	unsigned int eax = 0x00000001, ebx, ecx = 0, edx;
> +	unsigned int family, model, stepping;

Change types to be in align with function prototypes.

> +	char name[30] = "intel-ucode", *p = name + 11;
>  
>  	native_cpuid(&eax, &ebx, &ecx, &edx);
>  
> @@ -533,7 +533,12 @@ static bool __init load_builtin_intel_microcode(struct cpio_data *cp)
>  	model    = x86_model(eax);
>  	stepping = eax & 0xf;
>  
> -	sprintf(name, "intel-ucode/%02x-%02x-%02x", family, model, stepping);
> +	*p++ = '/';
> +	p = hex_byte_pack(p, family);
> +	*p++ = '-';
> +	p = hex_byte_pack(p, model);
> +	*p++ = '-';
> +	p = hex_byte_pack(p, stepping);
>  

Forgot to add *p = '\0'; here, but it doesn't really mater for the idea.

What I would like to tell that if I move 'call load_ucode_bsp' after
paging is done, it seems sprintf() starts working nicely. So, I don't
know which fix is better (or neither), and wondering if we need to fix
AMD part.

Please, share your opinions, suggestions.

P.S. It would be really nice to get an expert / detailed explanation
what is going on there.

>  	return get_builtin_firmware(cp, name);
>  }

-- 
Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
Intel Finland Oy

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/