Messages in this thread | | | Date | Wed, 4 Jan 2023 08:43:51 +0100 | Subject | Re: [PATCH] x86: also disable FSRM if ERMS is disabled | From | Jiri Slaby <> |
| |
On 07. 10. 22, 20:08, Daniel Verkamp wrote: > On Fri, Sep 23, 2022 at 10:51 AM Borislav Petkov <bp@alien8.de> wrote: >> >> On Fri, Sep 23, 2022 at 10:25:05AM -0700, Daniel Verkamp wrote: >>> Yes, we hit this in crosvm when booting the guest kernel with either >>> OVMF or u-boot on an Intel 12th Gen CPU. The guest kernel boots fine >>> when loaded directly (using the crosvm kernel loader and not running >>> any firmware setup in the guest), but it crashes when booting with >>> firmware inside the first forward memmove() after alternatives are set >>> up (which happens to be in printk). I haven't gotten to the bottom of >>> why exactly using firmware is causing this to be set up in an >>> inconsistent way, but this is a real-world situation, not just a >>> hypothetical. >> >> Sounds like broken virt firmware or so. And if that is not an issue on >> baremetal, then the virt stack should be fixed - not the kernel. >> >>> Now that I look at it with fresh eyes again, maybe we should instead >>> directly patch the memmove FSRM alternative so that the flag-set >>> version just does the same jmp as the ERMS one. I can prepare a patch >>> for that instead of (or in addition to) this one if that sounds >>> better. >> >> So, if the virt firmware deviates from how the real hardware behaves, >> then the kernel needs no fixing. >> >> So you'd have to figure out why is the virt firmware causing this and >> not baremetal. >> >> Then we can talk about fixes. > > Hi Borislav, > > We found that the IA32_MISC_ENABLE MSR setup was missing in the crosvm > firmware boot path (but not when directly booting a kernel, which is > why it did not get noticed for a while). Setting the fast string bit > in the MSR avoids the issue. > > However, I still think it would be appropriate to apply this patch or > something like it, since there could be a CPU, microcode update, BIOS, > etc. that clears this bit while still having the CPUID flags for FSRM > and ERMS.
Let me resurrect this thread... Our customer has an AMD CPU which has indeed both capabilities under normal circumstances. But they have a cool UEFI BIOS too. They say:
""" In AMD platform, while disalbe ERMS(Enhanced Rep MOVSB/STOSB) in UEFI (system setup -> processor -> Enhanced Rep MOVSB/STOSB), the OS can't boot normally. """
That is exactly the case here. So can we have the patch (the original one, the one below or a better one) to fix this?
> The Intel SDM says: "Software can disable fast-string > operation by clearing the fast-string-enable bit (bit 0) of > IA32_MISC_ENABLE MSR", so it's not an invalid configuration for this > bit to be unset. > > Additionally, something like this avoids the problem by making the > FSRM case jump directly to the REP MOVSB rather than falling through > to the ERMS jump in the next instruction, which seems like basically > free insurance (but if the FSRM flag gets used somewhere else in the > future, having it set consistently with ERMS is probably still a good > idea, per the original patch): > > diff --git a/arch/x86/lib/memmove_64.S b/arch/x86/lib/memmove_64.S > index 724bbf83eb5b..8ac557409c7d 100644 > --- a/arch/x86/lib/memmove_64.S > +++ b/arch/x86/lib/memmove_64.S > @@ -38,7 +38,7 @@ SYM_FUNC_START(__memmove) > > /* FSRM implies ERMS => no length checks, do the copy directly */ > .Lmemmove_begin_forward: > - ALTERNATIVE "cmp $0x20, %rdx; jb 1f", "", X86_FEATURE_FSRM > + ALTERNATIVE "cmp $0x20, %rdx; jb 1f", "jmp .Lmemmove_erms", > X86_FEATURE_FSRM > ALTERNATIVE "", "jmp .Lmemmove_erms", X86_FEATURE_ERMS > > And hey, this means one less instruction to execute in the FSRM path. :)
thanks, -- js suse labs
| |