Fix built-in early-load Intel microcode alignment

Fix built-in early-load Intel microcode alignment

[Jari Ruusu]

Intel Software Developer's Manual, volume 3, chapter 9.11.6 says:
"Note that the microcode update must be aligned on a 16-byte
boundary and the size of the microcode update must be 1-KByte
granular"

When early-load Intel microcode is loaded from initramfs,
userspace tool 'iucode_tool' has already 16-byte aligned those
microcode bits in that initramfs image. Image that was created
something like this:

 iucode_tool --write-earlyfw=FOO.cpio microcode-files...

However, when early-load Intel microcode is loaded from built-in
firmware BLOB using CONFIG_EXTRA_FIRMWARE= kernel config option,
that 16-byte alignment is not guaranteed.

Fix this by forcing all built-in firmware BLOBs to 16-byte
alignment.


Signed-off-by: Jari Ruusu <jari.ruusu@gmail.com>

--- a/drivers/base/firmware_loader/builtin/Makefile
+++ b/drivers/base/firmware_loader/builtin/Makefile
@@ -17,7 +17,7 @@
 filechk_fwbin = \
 	echo "/* Generated by $(src)/Makefile */"		;\
 	echo "    .section .rodata"				;\
-	echo "    .p2align $(ASM_ALIGN)"			;\
+	echo "    .p2align 4"					;\
 	echo "_fw_$(FWSTR)_bin:"				;\
 	echo "    .incbin \"$(fwdir)/$(FWNAME)\""		;\
 	echo "_fw_end:"						;\

-- 
Jari Ruusu  4096R/8132F189 12D6 4C3A DCDA 0AA4 27BD  ACDF F073 3C80 8132 F189

Re: Fix built-in early-load Intel microcode alignment

[Jari Ruusu]

On 1/12/20, Jari Ruusu <jari.ruusu@gmail.com> wrote:
> Intel Software Developer's Manual, volume 3, chapter 9.11.6 says:
> "Note that the microcode update must be aligned on a 16-byte
> boundary and the size of the microcode update must be 1-KByte
> granular"
>
> When early-load Intel microcode is loaded from initramfs,
> userspace tool 'iucode_tool' has already 16-byte aligned those
> microcode bits in that initramfs image. Image that was created
> something like this:
>
>  iucode_tool --write-earlyfw=FOO.cpio microcode-files...
>
> However, when early-load Intel microcode is loaded from built-in
> firmware BLOB using CONFIG_EXTRA_FIRMWARE= kernel config option,
> that 16-byte alignment is not guaranteed.
>
> Fix this by forcing all built-in firmware BLOBs to 16-byte
> alignment.

Backport of "Fix built-in early-load Intel microcode alignment"
for linux-4.19 and older stable kernels.

Signed-off-by: Jari Ruusu <jari.ruusu@gmail.com>

--- a/firmware/Makefile
+++ b/firmware/Makefile
@@ -19,7 +19,7 @@
 		  PROGBITS=$(if $(CONFIG_ARM),%,@)progbits;		     \
 		  echo "/* Generated by firmware/Makefile */"		> $@;\
 		  echo "    .section .rodata"				>>$@;\
-		  echo "    .p2align $${ASM_ALIGN}"			>>$@;\
+		  echo "    .p2align 4"					>>$@;\
 		  echo "_fw_$${FWSTR}_bin:"				>>$@;\
 		  echo "    .incbin \"$(2)\""				>>$@;\
 		  echo "_fw_end:"					>>$@;\

-- 
Jari Ruusu  4096R/8132F189 12D6 4C3A DCDA 0AA4 27BD  ACDF F073 3C80 8132 F189

Re: Fix built-in early-load Intel microcode alignment

[Greg KH]

On Sun, Jan 12, 2020 at 03:03:44PM +0200, Jari Ruusu wrote:
> On 1/12/20, Jari Ruusu <jari.ruusu@gmail.com> wrote:
> > Intel Software Developer's Manual, volume 3, chapter 9.11.6 says:
> > "Note that the microcode update must be aligned on a 16-byte
> > boundary and the size of the microcode update must be 1-KByte
> > granular"
> >
> > When early-load Intel microcode is loaded from initramfs,
> > userspace tool 'iucode_tool' has already 16-byte aligned those
> > microcode bits in that initramfs image. Image that was created
> > something like this:
> >
> >  iucode_tool --write-earlyfw=FOO.cpio microcode-files...
> >
> > However, when early-load Intel microcode is loaded from built-in
> > firmware BLOB using CONFIG_EXTRA_FIRMWARE= kernel config option,
> > that 16-byte alignment is not guaranteed.
> >
> > Fix this by forcing all built-in firmware BLOBs to 16-byte
> > alignment.
> 
> Backport of "Fix built-in early-load Intel microcode alignment"
> for linux-4.19 and older stable kernels.

Any hint as to what that git commit id is?

thanks,

greg k-h

Re: Fix built-in early-load Intel microcode alignment

[Jari Ruusu]

On 1/12/20, Greg KH <greg@kroah.com> wrote:
> On Sun, Jan 12, 2020 at 03:03:44PM +0200, Jari Ruusu wrote:
>> Backport of "Fix built-in early-load Intel microcode alignment"
>> for linux-4.19 and older stable kernels.
>
> Any hint as to what that git commit id is?

It is not in mainline yet.
You have far better chances of pushing it to mainline than I do.

-- 
Jari Ruusu  4096R/8132F189 12D6 4C3A DCDA 0AA4 27BD  ACDF F073 3C80 8132 F189

Re: Fix built-in early-load Intel microcode alignment

[Greg KH]

On Mon, Jan 13, 2020 at 08:30:27AM +0200, Jari Ruusu wrote:
> On 1/12/20, Greg KH <greg@kroah.com> wrote:
> > On Sun, Jan 12, 2020 at 03:03:44PM +0200, Jari Ruusu wrote:
> >> Backport of "Fix built-in early-load Intel microcode alignment"
> >> for linux-4.19 and older stable kernels.
> >
> > Any hint as to what that git commit id is?
> 
> It is not in mainline yet.
> You have far better chances of pushing it to mainline than I do.

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

Re: Fix built-in early-load Intel microcode alignment

[Luis Chamberlain]

On Sun, Jan 12, 2020 at 03:00:53PM +0200, Jari Ruusu wrote:
> Intel Software Developer's Manual, volume 3, chapter 9.11.6 says:
> "Note that the microcode update must be aligned on a 16-byte
> boundary and the size of the microcode update must be 1-KByte
> granular"
> 
> When early-load Intel microcode is loaded from initramfs,
> userspace tool 'iucode_tool' has already 16-byte aligned those
> microcode bits in that initramfs image. Image that was created
> something like this:
> 
>  iucode_tool --write-earlyfw=FOO.cpio microcode-files...
> 
> However, when early-load Intel microcode is loaded from built-in
> firmware BLOB using CONFIG_EXTRA_FIRMWARE= kernel config option,
> that 16-byte alignment is not guaranteed.

Thanks for the patch!

So what happens with you use the built-in firmware loader for
the Intel microcode at this time? I am surprised this issue
wasn't reported earlier, so thanks for picking it up, but to
be complete such a change requires a bit more information.

What exactly happens now?

> Fix this by forcing all built-in firmware BLOBs to 16-byte
> alignment.

That's a huge stretch, see below.

> Signed-off-by: Jari Ruusu <jari.ruusu@gmail.com>
> 
> --- a/drivers/base/firmware_loader/builtin/Makefile
> +++ b/drivers/base/firmware_loader/builtin/Makefile
> @@ -17,7 +17,7 @@
>  filechk_fwbin = \
>  	echo "/* Generated by $(src)/Makefile */"		;\
>  	echo "    .section .rodata"				;\
> -	echo "    .p2align $(ASM_ALIGN)"			;\
> +	echo "    .p2align 4"					;\

You are forcing 16 byte alignment to *all* built-in firmware, and some
architectures may have a different requirement. If things used to work
with ASM_ALIGN which is a construct only used for this code, but your
change fixes it with Intel microcode loading -- it however *may* break
things for other built-in firmware used. In particular if you note above
it used to align things to 2^3 so 8 bytes if on CONFIG_64BIT, otherwise
things get aligned to 2^2 so 4 bytes.

So I'd like to determine first if we really need this. Then if so,
either add a new global config option, and worst comes to worst
figure out a way to do it per driver. I don't think we'd need it
per driver.

If set as a global new config option, we can use the same logic and
allow an architecture override if the user / architecture kconfig
configures it such:

config ARCH_DEFAULT_FIRMWARE_ALIGNMENT
	string "Default architecture firmware aligmnent"
	"4" if 64BIT
	"3" if !64BIT

config FIRMWARE_BUILTIN_ALIGN
	string "Built in firmware aligment requirement"
	default ARCH_DEFAULT_FIRMWARE_ALIGNMENT if !ARCH_CUSTOM_FIRMWARE_ALIGNMENT
	default ARCH_CUSTOM_FIRMWARE_ALIGNMENT_VAL if ARCH_CUSTOM_FIRMWARE_ALIGNMENT
	  Some good description goes here

Or something like that.

 Luis

Re: Fix built-in early-load Intel microcode alignment

[Linus Torvalds]

On Mon, Jan 13, 2020 at 7:47 AM Luis Chamberlain <mcgrof@kernel.org> wrote:
>
> So I'd like to determine first if we really need this. Then if so,
> either add a new global config option, and worst comes to worst
> figure out a way to do it per driver. I don't think we'd need it
> per driver.

I really don't think we need to have a config option for some small
alignment. Increasing the alignment unconditionally to 16 bytes won't
hurt anybody.

Now, whether there might be other firmware loaders that need even more
alignment, that might be an interesting question, and if such an
alignment would be _huge_ we might want to worry about actual memory
waste.

But 16-byte alignment for a fw blob? That's nothing.

                Linus

Re: Fix built-in early-load Intel microcode alignment

[Jari Ruusu]

On 1/13/20, Luis Chamberlain <mcgrof@kernel.org> wrote:
> So what happens with you use the built-in firmware loader for
> the Intel microcode at this time? I am surprised this issue
> wasn't reported earlier, so thanks for picking it up, but to
> be complete such a change requires a bit more information.
>
> What exactly happens now?

Before that 16-byte alignment patch was applied, my only one
microcode built-in BLOB was "accidentally" 16-byte aligned.

After that patch was applied, new kernel System.map file was
exactly same. So, for me that patch did not change anything.

Same 16-byte alignment before and after patch:

$  grep " _fw_.*_bin" System.map
ffffffff81f55e90 r _fw_intel_ucode_06_8e_09_bin

>> Fix this by forcing all built-in firmware BLOBs to 16-byte
>> alignment.
>
> That's a huge stretch, see below.

I understand and to some degree agree.

> So I'd like to determine first if we really need this.

We do need it. Violating Intel specs is not good. It may be that
some processor models require aligned and some accept less
aligned.

> If set as a global new config option, we can use the same logic and
> allow an architecture override if the user / architecture kconfig
> configures it such:
>
> config ARCH_DEFAULT_FIRMWARE_ALIGNMENT
> 	string "Default architecture firmware aligmnent"
> 	"4" if 64BIT
> 	"3" if !64BIT
>
> config FIRMWARE_BUILTIN_ALIGN
> 	string "Built in firmware aligment requirement"
> 	default ARCH_DEFAULT_FIRMWARE_ALIGNMENT if !ARCH_CUSTOM_FIRMWARE_ALIGNMENT
> 	default ARCH_CUSTOM_FIRMWARE_ALIGNMENT_VAL if
> ARCH_CUSTOM_FIRMWARE_ALIGNMENT
> 	  Some good description goes here
>
> Or something like that.

It doesn't have to user visible config option, only default align
changed when selected set of options are enabled.

My patch was intentionally minimal, without #ifdef spaghetti.

-- 
Jari Ruusu  4096R/8132F189 12D6 4C3A DCDA 0AA4 27BD  ACDF F073 3C80 8132 F189

Re: Fix built-in early-load Intel microcode alignment

[Borislav Petkov]

On Mon, Jan 13, 2020 at 09:58:25PM +0200, Jari Ruusu wrote:
> Before that 16-byte alignment patch was applied, my only one
> microcode built-in BLOB was "accidentally" 16-byte aligned.

Btw, just out of curiosity: why are you using built-in microcode and not
the initrd method?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: Fix built-in early-load Intel microcode alignment

[Jari Ruusu]

On 1/13/20, Borislav Petkov <bp@alien8.de> wrote:
> Btw, just out of curiosity: why are you using built-in microcode and not
> the initrd method?

Initrd method is better when it is a kernel intended to be booted
on many different computers. Built-in microcode method kernel is
tuned for one computer only. It is less hassle that way.

-- 
Jari Ruusu  4096R/8132F189 12D6 4C3A DCDA 0AA4 27BD  ACDF F073 3C80 8132 F189

Re: Fix built-in early-load Intel microcode alignment

[Borislav Petkov]

On Mon, Jan 13, 2020 at 10:30:13PM +0200, Jari Ruusu wrote:
> On 1/13/20, Borislav Petkov <bp@alien8.de> wrote:
> > Btw, just out of curiosity: why are you using built-in microcode and not
> > the initrd method?
> 
> Initrd method is better when it is a kernel intended to be booted
> on many different computers. Built-in microcode method kernel is
> tuned for one computer only. It is less hassle that way.

Oh well, you only need to do an initrd which is not that big of a deal.

The built-in method requires you to rebuild the kernel when there's
new microcode but new microcode is a relatively seldom occurrence
in practice. The last two years putting my statistics completely
out-of-whack.

But they should be coming back to normal because there should simply
be no more room for microcode patches anymore in the most x86 CPUs out
there. :)

So if you're building kernels often, it doesn't really matter if you do
initrd or builtin microcode.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: Fix built-in early-load Intel microcode alignment

[Luis Chamberlain]

On Mon, Jan 13, 2020 at 09:58:25PM +0200, Jari Ruusu wrote:
> On 1/13/20, Luis Chamberlain <mcgrof@kernel.org> wrote:
> > So what happens with you use the built-in firmware loader for
> > the Intel microcode at this time? I am surprised this issue
> > wasn't reported earlier, so thanks for picking it up, but to
> > be complete such a change requires a bit more information.
> >
> > What exactly happens now?
> 
> Before that 16-byte alignment patch was applied, my only one
> microcode built-in BLOB was "accidentally" 16-byte aligned.

How did it accidentially get 16-byte aligned?

Also, how do you *know* something is broken right now? I mean
you issued a patch for stable. I thought you hit a panic or
some issue while loading. If we are not sure this fixes a real
issue as of yet, I can't see the merit for propagating a fix
to stable.

> After that patch was applied, new kernel System.map file was
> exactly same. So, for me that patch did not change anything.
> 
> Same 16-byte alignment before and after patch:
> 
> $  grep " _fw_.*_bin" System.map
> ffffffff81f55e90 r _fw_intel_ucode_06_8e_09_bin
> 
> >> Fix this by forcing all built-in firmware BLOBs to 16-byte
> >> alignment.
> >
> > That's a huge stretch, see below.
> 
> I understand and to some degree agree.
> 
> > So I'd like to determine first if we really need this.
> 
> We do need it. Violating Intel specs is not good. It may be that
> some processor models require aligned and some accept less
> aligned.

Fair point. A fix to follow the spec is however different than to say
without it things don't work, and we need to propagate a fix to stable
kernels.

> > If set as a global new config option, we can use the same logic and
> > allow an architecture override if the user / architecture kconfig
> > configures it such:
> >
> > config ARCH_DEFAULT_FIRMWARE_ALIGNMENT
> > 	string "Default architecture firmware aligmnent"
> > 	"4" if 64BIT
> > 	"3" if !64BIT
> >
> > config FIRMWARE_BUILTIN_ALIGN
> > 	string "Built in firmware aligment requirement"
> > 	default ARCH_DEFAULT_FIRMWARE_ALIGNMENT if !ARCH_CUSTOM_FIRMWARE_ALIGNMENT
> > 	default ARCH_CUSTOM_FIRMWARE_ALIGNMENT_VAL if
> > ARCH_CUSTOM_FIRMWARE_ALIGNMENT
> > 	  Some good description goes here
> >
> > Or something like that.
> 
> It doesn't have to user visible config option, only default align
> changed when selected set of options are enabled.

Right, I didn't intend for it to be visible really. It was just an
example of kconfig magic how perhaps how to define this if we needed
something configurable per arch.

> My patch was intentionally minimal, without #ifdef spaghetti.

Thanks for it. We just need to dust it off a bit now.

  Luis

Re: Fix built-in early-load Intel microcode alignment

[Luis Chamberlain]

On Mon, Jan 13, 2020 at 11:44:25AM -0800, Linus Torvalds wrote:
> On Mon, Jan 13, 2020 at 7:47 AM Luis Chamberlain <mcgrof@kernel.org> wrote:
> >
> > So I'd like to determine first if we really need this. Then if so,
> > either add a new global config option, and worst comes to worst
> > figure out a way to do it per driver. I don't think we'd need it
> > per driver.
> 
> I really don't think we need to have a config option for some small
> alignment. Increasing the alignment unconditionally to 16 bytes won't
> hurt anybody.

Since you are confident in that, then simply bumping it to 16 bytes
seems fine by me.

> Now, whether there might be other firmware loaders that need even more
> alignment, that might be an interesting question, and if such an
> alignment would be _huge_ we might want to worry about actual memory
> waste.

I can only envision waste being considered due to alignent for remote
proc folks, who I *doubt* use the built-in stuff given the large size of
their blobs... but since you never know, better poke. So I've CC'd them.

> But 16-byte alignment for a fw blob? That's nothing.

Fine by me if we are sure it won't break anything and we hear no
complaints by remote proc folks.

  Luis

Re: Fix built-in early-load Intel microcode alignment

[Jari Ruusu]

On 1/15/20, Luis Chamberlain <mcgrof@kernel.org> wrote:
> On Mon, Jan 13, 2020 at 09:58:25PM +0200, Jari Ruusu wrote:
>> Before that 16-byte alignment patch was applied, my only one
>> microcode built-in BLOB was "accidentally" 16-byte aligned.
>
> How did it accidentially get 16-byte aligned?

Old code aligned it to 8-bytes.
There is 50/50-chance of it also being 16-byte aligned.
So it ended up being both 8-byte and 16-byte aligned.

> Also, how do you *know* something is broken right now?

I haven't spotted brokenness in Linux microcode loader other
than that small alignment issue.

However, I can confirm that there are 2 microcode updates newer
than what my laptop computer's latest BIOS includes. Both newer
ones (20191115 and 20191112) are unstable on my laptop computer
i5-7200U (fam 6 model 142 step 9 pf 0x80). Hard lockups with both
of them. Back to BIOS microcode for now.

-- 
Jari Ruusu  4096R/8132F189 12D6 4C3A DCDA 0AA4 27BD  ACDF F073 3C80 8132 F189

Re: Fix built-in early-load Intel microcode alignment

[Luis Chamberlain]

On Wed, Jan 15, 2020 at 08:46:04PM +0200, Jari Ruusu wrote:
> On 1/15/20, Luis Chamberlain <mcgrof@kernel.org> wrote:
> > On Mon, Jan 13, 2020 at 09:58:25PM +0200, Jari Ruusu wrote:
> >> Before that 16-byte alignment patch was applied, my only one
> >> microcode built-in BLOB was "accidentally" 16-byte aligned.
> >
> > How did it accidentially get 16-byte aligned?
> 
> Old code aligned it to 8-bytes.
> There is 50/50-chance of it also being 16-byte aligned.

But *how? Why is there a 50/50 chance of it being aligned to
16 bytes if 8 bytes are currently specified?

> So it ended up being both 8-byte and 16-byte aligned.

What do you mean both? How can it be aligned to both?

> > Also, how do you *know* something is broken right now?
> 
> I haven't spotted brokenness in Linux microcode loader other
> than that small alignment issue.
> 
> However, I can confirm that there are 2 microcode updates newer
> than what my laptop computer's latest BIOS includes. Both newer
> ones (20191115 and 20191112) are unstable on my laptop computer
> i5-7200U (fam 6 model 142 step 9 pf 0x80). Hard lockups with both
> of them. Back to BIOS microcode for now.

I was more interested in how you are *certain*, other than manualcode
inspection, and that a spec indicates we should use 16 bytes for Intel
microcode -- that the 8 byte alignment *does* not allow users to
currently update their Intel CPU microcode for built-in firmware.                                      

From what I gather so far we have no case yet reported where we know for
sure it fails right now with the 8 byte alignment on 64-bit.
										                                                                                
This information would just be useful for the commit log.

  Luis

Re: Fix built-in early-load Intel microcode alignment

[Andy Lutomirski]

> On Jan 15, 2020, at 10:46 AM, Jari Ruusu <jari.ruusu@gmail.com> wrote:
> 
> On 1/15/20, Luis Chamberlain <mcgrof@kernel.org> wrote:
>>> On Mon, Jan 13, 2020 at 09:58:25PM +0200, Jari Ruusu wrote:
>>> Before that 16-byte alignment patch was applied, my only one
>>> microcode built-in BLOB was "accidentally" 16-byte aligned.
>> 
>> How did it accidentially get 16-byte aligned?
> 
> Old code aligned it to 8-bytes.
> There is 50/50-chance of it also being 16-byte aligned.
> So it ended up being both 8-byte and 16-byte aligned.
> 
>> Also, how do you *know* something is broken right now?
> 
> I haven't spotted brokenness in Linux microcode loader other
> than that small alignment issue.
> 
> However, I can confirm that there are 2 microcode updates newer
> than what my laptop computer's latest BIOS includes. Both newer
> ones (20191115 and 20191112) are unstable on my laptop computer
> i5-7200U (fam 6 model 142 step 9 pf 0x80). Hard lockups with both
> of them. Back to BIOS microcode for now.
> 

Hard lockup when loaded or hard lockup after loading?

> -- 
> Jari Ruusu  4096R/8132F189 12D6 4C3A DCDA 0AA4 27BD  ACDF F073 3C80 8132 F189

Re: Fix built-in early-load Intel microcode alignment

[Jari Ruusu]

On 1/15/20, Andy Lutomirski <luto@amacapital.net> wrote:
> Hard lockup when loaded or hard lockup after loading?

No problem at microcode load time.
Hard lockup after 1-2 days of use.

-- 
Jari Ruusu  4096R/8132F189 12D6 4C3A DCDA 0AA4 27BD  ACDF F073 3C80 8132 F189

Re: Fix built-in early-load Intel microcode alignment

[Linus Torvalds]

On Wed, Jan 15, 2020 at 10:58 AM Luis Chamberlain <mcgrof@kernel.org> wrote:
>
> But *how? Why is there a 50/50 chance of it being aligned to
> 16 bytes if 8 bytes are currently specified?

What?

It's trivial.

Address 256 is 4-byte aligned. But it's also 8-byte aligned. And
16-byte aligned. And..

So if you ask for 8-byte alignment, and you already had that address
(or were just below it), you'll get 8-byte alignment. But it will
_also_ be 16-byte aligned just by happenstance.

And yes, exactly half of the addresses that are 8-byte aligned are
also 16-byte aligned, so you have a 50/50 chance of getting the bigger
alignment simply by random chance.

In fact, often you probably have a _better_ than 50/50 chance of
getting the bigger alignment, since many other things are aligned too,
and the starting address likely isn't very random. So it might have
started out with a bigger alignment even before you asked for just
8-byte aligned data from the linker.

(Of course, the reverse may be true too - there may be cases you were
coimpletely mis-aligned, and asking for 8-byte alignment will never
give you any more aligned memory, but I suspect aligned data is a lot
more common than unaligned data is)

              Linus

Re: Fix built-in early-load Intel microcode alignment

[Linus Torvalds]

On Wed, Jan 15, 2020 at 11:15 AM Jari Ruusu <jari.ruusu@gmail.com> wrote:
>
> No problem at microcode load time.
> Hard lockup after 1-2 days of use.

That is "interesting".

However, the most likely cause is that you have a borderline dodgy
system, and the microcode update then just triggers a pre-existing
problem.

Possibly because of how newer microcode will have things like "VERW
now flushes CPU buffers" etc.

But it might be worth it if the intel people could check up with their
microcode people on this anyway - if there is _one_ report of "my
system locks up with newer ucode", that's one thing. But if Jari isn't
alone...

I don't know who the right intel person would be. There's a couple of
Intel people on the cc (and I added one more at random), can you try
to see if somebody would be aware of or interested in that "ucode
problems with i5-7200U (fam 6 model 142 step 9 pf 0x80)"

               Linus

Re: Fix built-in early-load Intel microcode alignment

[Jari Ruusu]

On 1/15/20, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> However, the most likely cause is that you have a borderline dodgy
> system, and the microcode update then just triggers a pre-existing
> problem.

For that particular processor model, there appears to be microcode
updates for four steppings: 9 10 11 and 12. My model is stepping
9, so it appears to be early commercially sold version of that
model. Probably more problems on it than on later steppings.

> But it might be worth it if the intel people could check up with their
> microcode people on this anyway - if there is _one_ report of "my
> system locks up with newer ucode", that's one thing. But if Jari isn't
> alone...

I'm not alone with latest Intel microcode problems. Debian for
example reverted microcode to older microcode version on some
Intel processor models because of hangs on warm reboots. Those
reverts were not for same processor model as my processor, but
they do indicate "not everything OK" situation with latest Intel
microcodes.

https://lists.debian.org/debian-security-announce/2019/msg00237.html

My laptop computer was made by Dell, and Dell has been really good
at providing new BIOS updates (that don't require Microsoft OS to
update). More than once they have provided new BIOS to fix some
security flaw that was still embargoed. The information about that
security flaw then became publically known later after embargo
ended.

Now that I have learned about the instability of latest two
microcode updates for my laptop's processor, it isn't difficult to
connect the dots why Dell is still shipping 3rd latest microcode
in their latest BIOS update for that laptop computer.

-- 
Jari Ruusu  4096R/8132F189 12D6 4C3A DCDA 0AA4 27BD  ACDF F073 3C80 8132 F189

Re: Fix built-in early-load Intel microcode alignment

[Raj, Ashok]

Hi Jari

Sorry for the delay, just returned after a long vacation. 

On Thu, Jan 16, 2020 at 08:55:52AM +0200, Jari Ruusu wrote:
> On 1/15/20, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> > However, the most likely cause is that you have a borderline dodgy
> > system, and the microcode update then just triggers a pre-existing
> > problem.
> 
> For that particular processor model, there appears to be microcode
> updates for four steppings: 9 10 11 and 12. My model is stepping
> 9, so it appears to be early commercially sold version of that
> model. Probably more problems on it than on later steppings.

I don't suspect the alignment issue during microcode load to trigger
any hangs after couple days. Its only used to copy to some internal store.
So once the ucode is loaded and if you look at /proc/cpuinfo and it reflects
the latest microcode you are past the alignment issue. I suspect its
something else.

Can you please also document the OEM, BIOS versions, and also both the
old and new microcode versions after the update.

Would suggest logging the hang issue in public github for microcode issues.
This would let the product folks look at them.

https://github.com/intel/Intel-Linux-Processor-Microcode-Data-Files

> 
> > But it might be worth it if the intel people could check up with their
> > microcode people on this anyway - if there is _one_ report of "my
> > system locks up with newer ucode", that's one thing. But if Jari isn't
> > alone...
> 

Cheers,
Ashok

Re: Fix built-in early-load Intel microcode alignment

[Jari Ruusu]

On 1/16/20, Raj, Ashok <ashok.raj@intel.com> wrote:
> I don't suspect the alignment issue during microcode load to trigger
> any hangs after couple days.

Microcode was properly aligned when it was loaded to CPU.

> Can you please also document the OEM, BIOS versions, and also both the
> old and new microcode versions after the update.
>
> Would suggest logging the hang issue in public github for microcode issues.
> This would let the product folks look at them.
>
> https://github.com/intel/Intel-Linux-Processor-Microcode-Data-Files

I opened issue there.

https://github.com/intel/Intel-Linux-Processor-Microcode-Data-Files/issues/23

-- 
Jari Ruusu  4096R/8132F189 12D6 4C3A DCDA 0AA4 27BD  ACDF F073 3C80 8132 F189

Re: Fix built-in early-load Intel microcode alignment

[Bjorn Andersson]

On Tue 14 Jan 18:27 PST 2020, Luis Chamberlain wrote:

> On Mon, Jan 13, 2020 at 11:44:25AM -0800, Linus Torvalds wrote:
> > On Mon, Jan 13, 2020 at 7:47 AM Luis Chamberlain <mcgrof@kernel.org> wrote:
> > >
> > > So I'd like to determine first if we really need this. Then if so,
> > > either add a new global config option, and worst comes to worst
> > > figure out a way to do it per driver. I don't think we'd need it
> > > per driver.
> > 
> > I really don't think we need to have a config option for some small
> > alignment. Increasing the alignment unconditionally to 16 bytes won't
> > hurt anybody.
> 
> Since you are confident in that, then simply bumping it to 16 bytes
> seems fine by me.
> 
> > Now, whether there might be other firmware loaders that need even more
> > alignment, that might be an interesting question, and if such an
> > alignment would be _huge_ we might want to worry about actual memory
> > waste.
> 
> I can only envision waste being considered due to alignent for remote
> proc folks, who I *doubt* use the built-in stuff given the large size of
> their blobs... but since you never know, better poke. So I've CC'd them.
> 

I've not heard of anyone using built-in firmware with remoteproc, but as
you say firmware used with remoteproc is large. So I can't see there
being a problem of potentially wasting 8 bytes...

> > But 16-byte alignment for a fw blob? That's nothing.
> 
> Fine by me if we are sure it won't break anything and we hear no
> complaints by remote proc folks.
> 

Go for it.

Regards,
Bjorn

Re: Fix built-in early-load Intel microcode alignment

[Luis Chamberlain]

On Sun, Jan 12, 2020 at 03:00:53PM +0200, Jari Ruusu wrote:
> Intel Software Developer's Manual, volume 3, chapter 9.11.6 says:
> "Note that the microcode update must be aligned on a 16-byte
> boundary and the size of the microcode update must be 1-KByte
> granular"
> 
> When early-load Intel microcode is loaded from initramfs,
> userspace tool 'iucode_tool' has already 16-byte aligned those
> microcode bits in that initramfs image. Image that was created
> something like this:
> 
>  iucode_tool --write-earlyfw=FOO.cpio microcode-files...
> 
> However, when early-load Intel microcode is loaded from built-in
> firmware BLOB using CONFIG_EXTRA_FIRMWARE= kernel config option,
> that 16-byte alignment is not guaranteed.
> 
> Fix this by forcing all built-in firmware BLOBs to 16-byte
> alignment.
> 
> 
> Signed-off-by: Jari Ruusu <jari.ruusu@gmail.com>
> 
> --- a/drivers/base/firmware_loader/builtin/Makefile
> +++ b/drivers/base/firmware_loader/builtin/Makefile
> @@ -17,7 +17,7 @@
>  filechk_fwbin = \
>  	echo "/* Generated by $(src)/Makefile */"		;\
>  	echo "    .section .rodata"				;\
> -	echo "    .p2align $(ASM_ALIGN)"			;\
> +	echo "    .p2align 4"					;\

Why not just keep ASM_ALIGN and define it to 4, with a nice
comment explaining the ucode stuff. Now we have a raw 4 here
and still use ASM_ALIGN which will depend on 64-bit or not.

  Luis

>  	echo "_fw_$(FWSTR)_bin:"				;\
>  	echo "    .incbin \"$(fwdir)/$(FWNAME)\""		;\
>  	echo "_fw_end:"						;\
> 
> -- 
> Jari Ruusu  4096R/8132F189 12D6 4C3A DCDA 0AA4 27BD  ACDF F073 3C80 8132 F189