diff mbox series

x86: Cache write back at 32-bit entry point

Message ID MWHPR11MB2048E2275A7815ED79E4E0628EE80@MWHPR11MB2048.namprd11.prod.outlook.com
State Superseded
Delegated to: Bin Meng
Headers show
Series x86: Cache write back at 32-bit entry point | expand

Commit Message

Park, Aiden Feb. 28, 2020, 4:54 a.m. UTC
In a certain condition, invd causes cache coherence issue.
  1. Pre-stage boot code passes memory address to U-Boot
  2. The data of the memory address is still in data cache line
  3. The invd marks data cache line as invalid without write back
  4. U-Boot accesses the memory address
  5. Data is invalid

Therefore, wbinvd is recommended at the 32-bit entry point even though
it consumes extra cpu clock cycles.

Signed-off-by: Aiden Park <aiden.park@intel.com>
---
 arch/x86/cpu/start.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Bin Meng Feb. 28, 2020, 5:26 a.m. UTC | #1
Hi Aiden,

On Fri, Feb 28, 2020 at 12:54 PM Park, Aiden <aiden.park@intel.com> wrote:
>
> In a certain condition, invd causes cache coherence issue.
>   1. Pre-stage boot code passes memory address to U-Boot
>   2. The data of the memory address is still in data cache line
>   3. The invd marks data cache line as invalid without write back
>   4. U-Boot accesses the memory address
>   5. Data is invalid
>
> Therefore, wbinvd is recommended at the 32-bit entry point even though
> it consumes extra cpu clock cycles.
>
> Signed-off-by: Aiden Park <aiden.park@intel.com>
> ---
>  arch/x86/cpu/start.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

This is already fixed (reverted) in the x86 tree. Would you please double check?

Regards,
Bin
Park, Aiden Feb. 28, 2020, 5:37 a.m. UTC | #2
Hi Bin,

> -----Original Message-----
> From: Bin Meng <bmeng.cn@gmail.com>
> Sent: Thursday, February 27, 2020 9:27 PM
> To: Park, Aiden <aiden.park@intel.com>
> Cc: masahiroy@kernel.org; sjg@chromium.org; sr@denx.de; agraf@suse.de; u-
> boot@lists.denx.de
> Subject: Re: [PATCH] x86: Cache write back at 32-bit entry point
> 
> Hi Aiden,
> 
> On Fri, Feb 28, 2020 at 12:54 PM Park, Aiden <aiden.park@intel.com> wrote:
> >
> > In a certain condition, invd causes cache coherence issue.
> >   1. Pre-stage boot code passes memory address to U-Boot
> >   2. The data of the memory address is still in data cache line
> >   3. The invd marks data cache line as invalid without write back
> >   4. U-Boot accesses the memory address
> >   5. Data is invalid
> >
> > Therefore, wbinvd is recommended at the 32-bit entry point even though
> > it consumes extra cpu clock cycles.
> >
> > Signed-off-by: Aiden Park <aiden.park@intel.com>
> > ---
> >  arch/x86/cpu/start.S | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> 
> This is already fixed (reverted) in the x86 tree. Would you please double check?
> 
Thank you for letting me know that. I think I missed some emails. Thanks again.
Andy, appreciate your commit. I had the same issue on Slim Bootloader due to this cache coherence.

> Regards,
> Bin

Best Regards,
Aiden
diff mbox series

Patch

diff --git a/arch/x86/cpu/start.S b/arch/x86/cpu/start.S
index 26cf995db2..01524635e9 100644
--- a/arch/x86/cpu/start.S
+++ b/arch/x86/cpu/start.S
@@ -50,7 +50,7 @@  _x86boot_start:
 	movl	%cr0, %eax
 	orl	$(X86_CR0_NW | X86_CR0_CD), %eax
 	movl	%eax, %cr0
-	invd
+	wbinvd
 
 	/*
 	 * Zero the BIST (Built-In Self Test) value since we don't have it.