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

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

[Andy Shevchenko]

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.

The patch changes sprintf() call to plain code which does the same in this
particular case.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 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;
+	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);
 
 	return get_builtin_firmware(cp, name);
 }
-- 
2.1.4

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

[Borislav Petkov]

On Tue, Jun 02, 2015 at 05:56:43PM +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.

Hrrm, I don't get that. How can sprintf() be out of scope?

Testing on 32-bit was successful here. How is x32 different?

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

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

[Andy Shevchenko]

On Tue, 2015-06-02 at 18:24 +0200, Borislav Petkov wrote:
> On Tue, Jun 02, 2015 at 05:56:43PM +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.
> 
> Hrrm, I don't get that. How can sprintf() be out of scope?

Initial paging setup is involved? I just sent a small update to previous
patch. Please, look at it.

> 
> Testing on 32-bit was successful here. How is x32 different?

Hmm… which Intel CPUs you run on?

Ah, one more thing we run our kernel from kexec (I hope it's not a case,
but who knows). 

> 
> Thanks.
> 


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

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

[Borislav Petkov]

On Tue, Jun 02, 2015 at 07:49:53PM +0300, Andy Shevchenko wrote:
> Initial paging setup is involved?

Yes, load_ucode_bsp happens before paging is enabled on 32-bit. And
that should remain that way - we want microcode application as early as
possible.

> Hmm… which Intel CPUs you run on?

processor       : 0
vendor_id       : GenuineIntel
cpu family      : 6
model           : 58
model name      : Intel(R) Core(TM) i7-3520M CPU @ 2.90GHz
stepping        : 9
microcode       : 0x1b

and an SNB one too.

> Ah, one more thing we run our kernel from kexec (I hope it's not a
> case, but who knows).

Does it work when you boot a normal 32-bit kernel with builtin microcode
on it?

In order to load built-in microcode, you'll have to enable the boot
options in the commit message to

  760d765b2bb6 ("x86/microcode: Parse built-in microcode early")

Let me know if something's not clear.

On Tue, Jun 02, 2015 at 07:46:29PM +0300, Andy Shevchenko wrote:
> > @@ -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.

Hmm, ok, so something's not able to stomach pre-paging. Can you dump
something from the failure, RIP, stack, whatever?

Can I reproduce it?

> So, I don't know which fix is better (or neither),

See above.

>  and wondering if we need to fix AMD part.

No need, AFAICT. It did work in my testing.

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

Yeah, that's why I'm asking.

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

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

[Andy Shevchenko]

On Tue, Jun 2, 2015 at 8:00 PM, Borislav Petkov <bp@suse.de> wrote:
> On Tue, Jun 02, 2015 at 07:49:53PM +0300, Andy Shevchenko wrote:
>> Initial paging setup is involved?
>
> Yes, load_ucode_bsp happens before paging is enabled on 32-bit. And
> that should remain that way - we want microcode application as early as
> possible.
>
>> Hmm… which Intel CPUs you run on?
>
> processor       : 0
> vendor_id       : GenuineIntel
> cpu family      : 6
> model           : 58
> model name      : Intel(R) Core(TM) i7-3520M CPU @ 2.90GHz
> stepping        : 9
> microcode       : 0x1b
>
> and an SNB one too.

Now at home already.
Will check tomorrow what we have.

>
>> Ah, one more thing we run our kernel from kexec (I hope it's not a
>> case, but who knows).
>
> Does it work when you boot a normal 32-bit kernel with builtin microcode
> on it?

Ditto.

>
> In order to load built-in microcode, you'll have to enable the boot
> options in the commit message to
>
>   760d765b2bb6 ("x86/microcode: Parse built-in microcode early")
>
> Let me know if something's not clear.

We are using slightly different (to upstream) i386_defconfig with some
devices enabled like serial console. As environment the Buildroot is
used with uclibc and busybox. No extra microcode is enabled.

By default as far as I remember the FIRMWARE* is set to y for all options.

> On Tue, Jun 02, 2015 at 07:46:29PM +0300, Andy Shevchenko wrote:
>> > @@ -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.
>
> Hmm, ok, so something's not able to stomach pre-paging. Can you dump
> something from the failure, RIP, stack, whatever?

I have a totally empty screen (serial console). So, if you teach me
how to gather that I could do it later on.

>
> Can I reproduce it?

I can send our defconfig and kexec command line for sure.

[]

>>  and wondering if we need to fix AMD part.
>
> No need, AFAICT. It did work in my testing.

Ok. We have no AMD here :-)

-- 
With Best Regards,
Andy Shevchenko

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

[Borislav Petkov]

On Tue, Jun 02, 2015 at 08:31:58PM +0300, Andy Shevchenko wrote:
> Ditto.

"Ditto" means it doesn't work booting a normal 32-bit kernel too?

> I have a totally empty screen (serial console). So, if you teach me
> how to gather that I could do it later on.

That'll be hard - you'd probably need a hardware debugger or something
special to get a RIP or other data that early... Or hack up some very
early console printing with BIOS methods and so on. hpa had something
like that somewhere AFAIR...

> I can send our defconfig and kexec command line for sure.

Yes please.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

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

[Andy Shevchenko]

On Wed, 2015-06-03 at 12:56 +0200, Borislav Petkov wrote:
> On Tue, Jun 02, 2015 at 08:31:58PM +0300, Andy Shevchenko wrote:
> > Ditto.
> 
> "Ditto" means it doesn't work booting a normal 32-bit kernel too?

Ditto, meant to test when I will be at work again :-)

But, unfortunately it doesn't. I did two images of next-20150602 with
and without this patch and 100% success and failure respectively (do
each test twice).

I have tested with 32-bit kernel run as EFI 32 binary from EFI Shell on
ASuS T100TA (Intel BayTrail-T):

processor       : 0
vendor_id       : GenuineIntel
cpu family      : 6
model           : 55
model name      : Intel(R) Atom(TM) CPU  Z3740  @ 1.33GHz
stepping        : 3
microcode       : 0x31e
cpu MHz         : 1330.000

> > I have a totally empty screen (serial console). So, if you teach me
> > how to gather that I could do it later on.
> 
> That'll be hard - you'd probably need a hardware debugger or something
> special to get a RIP or other data that early... Or hack up some very
> early console printing with BIOS methods and so on. hpa had something
> like that somewhere AFAIR...

I could try kvm / qemu as well, though there might be not the same
conditions.

> > I can send our defconfig and kexec command line for sure.
> 
> Yes please.

Here is the difference to the i386_defconfig as in next-20150602:

diff --git a/arch/x86/configs/i386_defconfig
b/arch/x86/configs/i386_defconfig
index aaa1118..7579120 100644
--- a/arch/x86/configs/i386_defconfig
+++ b/arch/x86/configs/i386_defconfig
@@ -204,7 +204,7 @@ CONFIG_SERIAL_8250_DETECT_IRQ=y
 CONFIG_SERIAL_8250_RSA=y
 CONFIG_HW_RANDOM=y
 CONFIG_NVRAM=y
-CONFIG_HPET=y
+# CONFIG_HPET is not set
 # CONFIG_HPET_MMAP is not set
 CONFIG_I2C_I801=y
 CONFIG_WATCHDOG=y
@@ -212,7 +212,8 @@ CONFIG_AGP=y
 CONFIG_AGP_AMD64=y
 CONFIG_AGP_INTEL=y
 CONFIG_DRM=y
-CONFIG_DRM_I915=y
+# CONFIG_DRM_I915 is not set
+CONFIG_BACKLIGHT_LCD_SUPPORT=y
 CONFIG_FB_MODE_HELPERS=y
 CONFIG_FB_TILEBLITTING=y
 CONFIG_FB_EFI=y
@@ -229,8 +230,8 @@ CONFIG_SND_MIXER_OSS=y
 CONFIG_SND_PCM_OSS=y
 CONFIG_SND_SEQUENCER_OSS=y
 CONFIG_SND_HRTIMER=y
-CONFIG_SND_HDA_INTEL=y
-CONFIG_SND_HDA_HWDEP=y
+# CONFIG_SND_HDA_INTEL is not set
+# CONFIG_SND_HDA_HWDEP is not set
 CONFIG_HIDRAW=y
 CONFIG_HID_GYRATION=y
 CONFIG_LOGITECH_FF=y
@@ -310,3 +311,55 @@ CONFIG_SECURITY_SELINUX_BOOTPARAM=y
 CONFIG_SECURITY_SELINUX_DISABLE=y
 CONFIG_CRYPTO_AES_586=y
 # CONFIG_CRYPTO_ANSI_CPRNG is not set
+CONFIG_FUNCTION_TRACER=y
+CONFIG_I2C_DESIGNWARE_PCI=y
+CONFIG_I2C_DESIGNWARE_PLATFORM=m
+CONFIG_GPIOLIB=y
+CONFIG_GPIO_INTEL_MID=y
+CONFIG_INTEL_MID_WATCHDOG=y
+CONFIG_NOP_USB_XCEIV=y
+CONFIG_USB_CHIPIDEA=y
+CONFIG_USB_CHIPIDEA_UDC=y
+CONFIG_USB_CHIPIDEA_HOST=y
+CONFIG_X86_EXTENDED_PLATFORM=y
+CONFIG_X86_INTEL_MID=y
+CONFIG_EFI_STUB=y
+CONFIG_EARLY_PRINTK_EFI=y
+CONFIG_FB=y
+CONFIG_FRAMEBUFFER_CONSOLE=y
+CONFIG_DYNAMIC_DEBUG=y
+CONFIG_USB_XHCI_HCD=y
+CONFIG_USB_DWC3=y
+CONFIG_USB_DWC3_GADGET=y
+CONFIG_USB_SERIAL=y
+CONFIG_USB_SERIAL_PL2303=y
+CONFIG_USB_USBNET=y
+CONFIG_USB_NET_AX88179_178A=y
+CONFIG_USB_NET_MCS7830=y
+CONFIG_USB_NET_AX8817X=y
+CONFIG_X86_INTEL_LPSS=y
+CONFIG_PM_RUNTIME=y
+CONFIG_DW_DMAC_CORE=m
+CONFIG_DW_DMAC=m
+CONFIG_DW_DMAC_PCI=m
+CONFIG_DMATEST=m
+CONFIG_SERIAL_8250_DMA=y
+CONFIG_SERIAL_8250_PCI=y
+CONFIG_SERIAL_8250_DW=m
+CONFIG_MMC=m
+CONFIG_MMC_SDHCI=m
+CONFIG_MMC_SDHCI_ACPI=m
+CONFIG_ACPI_DEBUG=y
+CONFIG_ACPI_PROCFS_POWER=y
+CONFIG_DMA_API_DEBUG=y
+CONFIG_DEBUG_LOCKDEP=y
+CONFIG_DEBUG_SHIRQ=y
+CONFIG_PINCTRL=y
+CONFIG_PINCTRL_BAYTRAIL=y
+CONFIG_PWM=y
+CONFIG_PWM_LPSS=m
+CONFIG_PWM_LPSS_PCI=m
+CONFIG_PWM_LPSS_PLATFORM=m
+CONFIG_SPI=y
+CONFIG_SPI_PXA2XX_PCI=m
+CONFIG_SPI_PXA2XX=m


kexec command:

kexec -l $bootdir/vmlinuz --initrd $bootdir/initrd
--command-line="$cmdline"

cmdline (in case of kexec the acpi_rdsp=0xdeadbeaf is added, 0xdeadbeaf
not an actual address):

console=tty1 console=ttyS0,115200n8 ignore_loglevel spidev.bufsiz=8192


P.S. I could test any configuration you suppose if it's needed. 

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

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

[Shevchenko, Andriy]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1233 bytes --]

On Wed, 2015-06-03 at 18:27 +0300, Andy Shevchenko wrote:
> > > I have a totally empty screen (serial console). So, if you teach me
> > > how to gather that I could do it later on.
> > 
> > That'll be hard - you'd probably need a hardware debugger or something
> > special to get a RIP or other data that early... Or hack up some very
> > early console printing with BIOS methods and so on. hpa had something
> > like that somewhere AFAIR...
> 
> I could try kvm / qemu as well, though there might be not the same
> conditions.

Yeah, qemu runs fine.

-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

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

[Borislav Petkov]

On Wed, Jun 03, 2015 at 05:49:05PM +0000, Shevchenko, Andriy wrote:
> On Wed, 2015-06-03 at 18:27 +0300, Andy Shevchenko wrote:
> > > > I have a totally empty screen (serial console). So, if you teach me
> > > > how to gather that I could do it later on.
> > > 
> > > That'll be hard - you'd probably need a hardware debugger or something
> > > special to get a RIP or other data that early... Or hack up some very
> > > early console printing with BIOS methods and so on. hpa had something
> > > like that somewhere AFAIR...
> > 
> > I could try kvm / qemu as well, though there might be not the same
> > conditions.
> 
> Yeah, qemu runs fine.

So, I did some more experiments and even if we build the string
properly, we choke later in get_builtin_firmware(). Actually, we don't
choke but we don't find firwmare because we're running with physical
addresses and those __start/__stop_builtin_fw things are virtual
addresses. Yuck!

So I'm gravitating towards disabling builtin microcode loading on
32-bit. For now, as the merge window is going to be open soon, until
we/I have fixed it properly. And I don't have a clear solution now - all
I can think of is ugly ifdeffery which I'd like to avoid.

Grrr.

---
diff --git a/arch/x86/kernel/cpu/microcode/amd_early.c b/arch/x86/kernel/cpu/microcode/amd_early.c
index 9208a36d0f03..9243cd839829 100644
--- a/arch/x86/kernel/cpu/microcode/amd_early.c
+++ b/arch/x86/kernel/cpu/microcode/amd_early.c
@@ -230,6 +230,7 @@ static void apply_ucode_in_initrd(void *ucode, size_t size, bool save_patch)
 
 static bool __init load_builtin_amd_microcode(struct cpio_data *cp, int family)
 {
+#ifdef CONFIG_X86_64
 	char fw_name[36] = "amd-ucode/microcode_amd.bin";
 
 	if (family >= 0x15)
@@ -237,6 +238,9 @@ static bool __init load_builtin_amd_microcode(struct cpio_data *cp, int family)
 			 "amd-ucode/microcode_amd_fam%.2xh.bin", family);
 
 	return get_builtin_firmware(cp, fw_name);
+#else
+	return false;
+#endif
 }
 
 void __init load_ucode_amd_bsp(int family)
diff --git a/arch/x86/kernel/cpu/microcode/intel_early.c b/arch/x86/kernel/cpu/microcode/intel_early.c
index 10dff3f3f686..b4858d892592 100644
--- a/arch/x86/kernel/cpu/microcode/intel_early.c
+++ b/arch/x86/kernel/cpu/microcode/intel_early.c
@@ -523,6 +523,7 @@ EXPORT_SYMBOL_GPL(save_mc_for_early);
 
 static bool __init load_builtin_intel_microcode(struct cpio_data *cp)
 {
+#ifdef CONFIG_X86_64
 	u32 eax = 0x00000001, ebx, ecx = 0, edx;
 	int family, model, stepping;
 	char name[30];
@@ -536,6 +537,9 @@ static bool __init load_builtin_intel_microcode(struct cpio_data *cp)
 	sprintf(name, "intel-ucode/%02x-%02x-%02x", family, model, stepping);
 
 	return get_builtin_firmware(cp, name);
+#else
+	return false;
+#endif
 }
 
 static __initdata char ucode_name[] = "kernel/x86/microcode/GenuineIntel.bin";

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

[PATCH] x86/microcode: Disable builtin microcode loading on 32-bit for now

[Borislav Petkov]

Ok, here's an actual patch. I'm very sorry for the confusion and big
thanks guys for catching it on time, before it hits the merge window!

Much appreciated. :-D

---
From: Borislav Petkov <bp@suse.de>
Date: Thu, 4 Jun 2015 09:52:37 +0200
Subject: [PATCH] x86/microcode: Disable builtin microcode loading on 32-bit for now

Andy Shevchenko reported machine freezes when booting latest tip on
x32 setups. Problem is, the builtin microcode handling cannot really
work that early, when we haven't even enabled paging. A proper fix
would involve handling that case specially as every other early 32-bit
boot case in the microcode loader and would require much more involved
changes for which it is too late now, more than a week before the
upcoming merge window.

So, disable the builtin microcode loading only on 32-bit for now.

Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: <x86@kernel.org>
Link: http://lkml.kernel.org/r/1433257003-159485-1-git-send-email-andriy.shevchenko@linux.intel.com
Signed-off-by: 
---
 arch/x86/kernel/cpu/microcode/amd_early.c   | 4 ++++
 arch/x86/kernel/cpu/microcode/intel_early.c | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/arch/x86/kernel/cpu/microcode/amd_early.c b/arch/x86/kernel/cpu/microcode/amd_early.c
index 9208a36d0f03..9243cd839829 100644
--- a/arch/x86/kernel/cpu/microcode/amd_early.c
+++ b/arch/x86/kernel/cpu/microcode/amd_early.c
@@ -230,6 +230,7 @@ static void apply_ucode_in_initrd(void *ucode, size_t size, bool save_patch)
 
 static bool __init load_builtin_amd_microcode(struct cpio_data *cp, int family)
 {
+#ifdef CONFIG_X86_64
 	char fw_name[36] = "amd-ucode/microcode_amd.bin";
 
 	if (family >= 0x15)
@@ -237,6 +238,9 @@ static bool __init load_builtin_amd_microcode(struct cpio_data *cp, int family)
 			 "amd-ucode/microcode_amd_fam%.2xh.bin", family);
 
 	return get_builtin_firmware(cp, fw_name);
+#else
+	return false;
+#endif
 }
 
 void __init load_ucode_amd_bsp(int family)
diff --git a/arch/x86/kernel/cpu/microcode/intel_early.c b/arch/x86/kernel/cpu/microcode/intel_early.c
index 10dff3f3f686..b4858d892592 100644
--- a/arch/x86/kernel/cpu/microcode/intel_early.c
+++ b/arch/x86/kernel/cpu/microcode/intel_early.c
@@ -523,6 +523,7 @@ EXPORT_SYMBOL_GPL(save_mc_for_early);
 
 static bool __init load_builtin_intel_microcode(struct cpio_data *cp)
 {
+#ifdef CONFIG_X86_64
 	u32 eax = 0x00000001, ebx, ecx = 0, edx;
 	int family, model, stepping;
 	char name[30];
@@ -536,6 +537,9 @@ static bool __init load_builtin_intel_microcode(struct cpio_data *cp)
 	sprintf(name, "intel-ucode/%02x-%02x-%02x", family, model, stepping);
 
 	return get_builtin_firmware(cp, name);
+#else
+	return false;
+#endif
 }
 
 static __initdata char ucode_name[] = "kernel/x86/microcode/GenuineIntel.bin";
-- 
2.3.5


-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH] x86/microcode: Disable builtin microcode loading on 32-bit for now

[Shevchenko, Andriy]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3817 bytes --]

On Thu, 2015-06-04 at 11:21 +0200, Borislav Petkov wrote:
> Ok, here's an actual patch. I'm very sorry for the confusion and big
> thanks guys for catching it on time, before it hits the merge window!
> 
> Much appreciated. :-D

Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Thanks!

> ---
> From: Borislav Petkov <bp@suse.de>
> Date: Thu, 4 Jun 2015 09:52:37 +0200
> Subject: [PATCH] x86/microcode: Disable builtin microcode loading on 32-bit for now
> 
> Andy Shevchenko reported machine freezes when booting latest tip on
> x32 setups. Problem is, the builtin microcode handling cannot really
> work that early, when we haven't even enabled paging. A proper fix
> would involve handling that case specially as every other early 32-bit
> boot case in the microcode loader and would require much more involved
> changes for which it is too late now, more than a week before the
> upcoming merge window.
> 
> So, disable the builtin microcode loading only on 32-bit for now.
> 
> Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> Cc: <x86@kernel.org>
> Link: http://lkml.kernel.org/r/1433257003-159485-1-git-send-email-andriy.shevchenko@linux.intel.com
> Signed-off-by: 
> ---
>  arch/x86/kernel/cpu/microcode/amd_early.c   | 4 ++++
>  arch/x86/kernel/cpu/microcode/intel_early.c | 4 ++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/microcode/amd_early.c b/arch/x86/kernel/cpu/microcode/amd_early.c
> index 9208a36d0f03..9243cd839829 100644
> --- a/arch/x86/kernel/cpu/microcode/amd_early.c
> +++ b/arch/x86/kernel/cpu/microcode/amd_early.c
> @@ -230,6 +230,7 @@ static void apply_ucode_in_initrd(void *ucode, size_t size, bool save_patch)
>  
>  static bool __init load_builtin_amd_microcode(struct cpio_data *cp, int family)
>  {
> +#ifdef CONFIG_X86_64
>  	char fw_name[36] = "amd-ucode/microcode_amd.bin";
>  
>  	if (family >= 0x15)
> @@ -237,6 +238,9 @@ static bool __init load_builtin_amd_microcode(struct cpio_data *cp, int family)
>  			 "amd-ucode/microcode_amd_fam%.2xh.bin", family);
>  
>  	return get_builtin_firmware(cp, fw_name);
> +#else
> +	return false;
> +#endif
>  }
>  
>  void __init load_ucode_amd_bsp(int family)
> diff --git a/arch/x86/kernel/cpu/microcode/intel_early.c b/arch/x86/kernel/cpu/microcode/intel_early.c
> index 10dff3f3f686..b4858d892592 100644
> --- a/arch/x86/kernel/cpu/microcode/intel_early.c
> +++ b/arch/x86/kernel/cpu/microcode/intel_early.c
> @@ -523,6 +523,7 @@ EXPORT_SYMBOL_GPL(save_mc_for_early);
>  
>  static bool __init load_builtin_intel_microcode(struct cpio_data *cp)
>  {
> +#ifdef CONFIG_X86_64
>  	u32 eax = 0x00000001, ebx, ecx = 0, edx;
>  	int family, model, stepping;
>  	char name[30];
> @@ -536,6 +537,9 @@ static bool __init load_builtin_intel_microcode(struct cpio_data *cp)
>  	sprintf(name, "intel-ucode/%02x-%02x-%02x", family, model, stepping);
>  
>  	return get_builtin_firmware(cp, name);
> +#else
> +	return false;
> +#endif
>  }
>  
>  static __initdata char ucode_name[] = "kernel/x86/microcode/GenuineIntel.bin";
> -- 
> 2.3.5
> 
> 


-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

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@linux.intel.com>
> ---
>  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@intel.com>
Intel Finland Oy