diff mbox series

[v1] Revert "x86: use invd instead of wbinvd in real mode start code"

Message ID 20200217153012.14989-1-andriy.shevchenko@linux.intel.com
State Accepted
Commit fa97ca161beb13a46a304f3c9824cdf0826dda0e
Delegated to: Bin Meng
Headers show
Series [v1] Revert "x86: use invd instead of wbinvd in real mode start code" | expand

Commit Message

Andy Shevchenko Feb. 17, 2020, 3:30 p.m. UTC
This reverts commit 0d67fac29f3187e67f4fd3ef15f73e91be2fad12.

As real hardware testing (*) shows the above mentioned commit
breaks U-Boot on it. Revert for the upcoming release. We may get
more information in the future and optimize the code accordingly.

(*) om Intel Edison board.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 arch/x86/cpu/start.S   | 2 +-
 arch/x86/cpu/start16.S | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Bin Meng Feb. 18, 2020, 12:24 a.m. UTC | #1
On Mon, Feb 17, 2020 at 11:30 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> This reverts commit 0d67fac29f3187e67f4fd3ef15f73e91be2fad12.
>
> As real hardware testing (*) shows the above mentioned commit
> breaks U-Boot on it. Revert for the upcoming release. We may get
> more information in the future and optimize the code accordingly.
>
> (*) om Intel Edison board.

on?

>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  arch/x86/cpu/start.S   | 2 +-
>  arch/x86/cpu/start16.S | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>

Acked-by: Bin Meng <bmeng.cn@gmail.com>
Masahiro Yamada Feb. 18, 2020, 12:45 a.m. UTC | #2
Hi Andy,

On Tue, Feb 18, 2020 at 12:30 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> This reverts commit 0d67fac29f3187e67f4fd3ef15f73e91be2fad12.
>
> As real hardware testing (*) shows the above mentioned commit
> breaks U-Boot on it. Revert for the upcoming release. We may get
> more information in the future and optimize the code accordingly.
>
> (*) om Intel Edison board.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  arch/x86/cpu/start.S   | 2 +-
>  arch/x86/cpu/start16.S | 2 +-


Reverting arch/x86/cpu/start.S is enough, isn't it?


arch/x86/cpu/start16.S is not compiled for the
Edison board.

start16.S is the start of the Real Mode,
and we are sure no program was running before
U-Boot.




>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> 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.
> diff --git a/arch/x86/cpu/start16.S b/arch/x86/cpu/start16.S
> index 292e750508..54f4ff6662 100644
> --- a/arch/x86/cpu/start16.S
> +++ b/arch/x86/cpu/start16.S
> @@ -28,7 +28,7 @@ start16:
>         movl    %cr0, %eax
>         orl     $(X86_CR0_NW | X86_CR0_CD), %eax
>         movl    %eax, %cr0
> -       invd
> +       wbinvd
>
>         /* load the temporary Global Descriptor Table */
>  data32 cs      lidt    idt_ptr
> --
> 2.25.0
>
Andy Shevchenko Feb. 18, 2020, 9:36 a.m. UTC | #3
On Tue, Feb 18, 2020 at 09:45:34AM +0900, Masahiro Yamada wrote:
> On Tue, Feb 18, 2020 at 12:30 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > This reverts commit 0d67fac29f3187e67f4fd3ef15f73e91be2fad12.
> >
> > As real hardware testing (*) shows the above mentioned commit
> > breaks U-Boot on it. Revert for the upcoming release. We may get
> > more information in the future and optimize the code accordingly.
> >
> > (*) om Intel Edison board.

> Reverting arch/x86/cpu/start.S is enough, isn't it?

Had you tested on real hardware?

Since Qemu ~= "no test" for the available hardware, I can't answer to this.

> arch/x86/cpu/start16.S is not compiled for the
> Edison board.
> 
> start16.S is the start of the Real Mode,
> and we are sure no program was running before
> U-Boot.

To be on the safer side I prefer to revert completely. Then you may come up
with a new patch with better commit message and testing. To me it sounds fair.
Andy Shevchenko Feb. 18, 2020, 9:37 a.m. UTC | #4
On Tue, Feb 18, 2020 at 08:24:03AM +0800, Bin Meng wrote:
> On Mon, Feb 17, 2020 at 11:30 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > This reverts commit 0d67fac29f3187e67f4fd3ef15f73e91be2fad12.
> >
> > As real hardware testing (*) shows the above mentioned commit
> > breaks U-Boot on it. Revert for the upcoming release. We may get
> > more information in the future and optimize the code accordingly.
> >
> > (*) om Intel Edison board.
> 
> on?

Right. Should I resend or you can fix when apply?

> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  arch/x86/cpu/start.S   | 2 +-
> >  arch/x86/cpu/start16.S | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> 
> Acked-by: Bin Meng <bmeng.cn@gmail.com>

Thanks!
Bin Meng Feb. 20, 2020, 1:43 p.m. UTC | #5
On Tue, Feb 18, 2020 at 5:37 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Feb 18, 2020 at 08:24:03AM +0800, Bin Meng wrote:
> > On Mon, Feb 17, 2020 at 11:30 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > This reverts commit 0d67fac29f3187e67f4fd3ef15f73e91be2fad12.
> > >
> > > As real hardware testing (*) shows the above mentioned commit
> > > breaks U-Boot on it. Revert for the upcoming release. We may get
> > > more information in the future and optimize the code accordingly.
> > >
> > > (*) om Intel Edison board.
> >
> > on?
>
> Right. Should I resend or you can fix when apply?

No, I fixed it when applying,

>
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > ---
> > >  arch/x86/cpu/start.S   | 2 +-
> > >  arch/x86/cpu/start16.S | 2 +-
> > >  2 files changed, 2 insertions(+), 2 deletions(-)
> > >
> >
> > Acked-by: Bin Meng <bmeng.cn@gmail.com>

applied to u-boot-x86, thanks!
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.
diff --git a/arch/x86/cpu/start16.S b/arch/x86/cpu/start16.S
index 292e750508..54f4ff6662 100644
--- a/arch/x86/cpu/start16.S
+++ b/arch/x86/cpu/start16.S
@@ -28,7 +28,7 @@  start16:
 	movl	%cr0, %eax
 	orl	$(X86_CR0_NW | X86_CR0_CD), %eax
 	movl	%eax, %cr0
-	invd
+	wbinvd
 
 	/* load the temporary Global Descriptor Table */
 data32 cs	lidt	idt_ptr