Change in ...coreboot[master]: arch/x86: Move checking for MTRR's as a proxy for proper CPU reset
Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32319
Change subject: arch/x86: Move checking for MTRR's as a proxy for proper CPU reset ......................................................................
arch/x86: Move checking for MTRR's as a proxy for proper CPU reset
Checking for empty MTRR_DEF_TYPE_MSR as a proxy for proper CPU reset is common across multiple platforms. Therefore place it in a common location.
Change-Id: I81d82fb9fe27cd9de6085251fe1a5685cdd651fc Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- A src/cpu/x86/early_reset.S M src/soc/intel/common/block/cpu/Makefile.inc M src/soc/intel/common/block/cpu/car/cache_as_ram.S 3 files changed, 44 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/32319/1
diff --git a/src/cpu/x86/early_reset.S b/src/cpu/x86/early_reset.S new file mode 100644 index 0000000..a35775c --- /dev/null +++ b/src/cpu/x86/early_reset.S @@ -0,0 +1,41 @@ +/* + * This file is part of the coreboot project. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +/* + * input %esp: return address (not pointer to return address!) + * clobber the content of eax, ecx, edx + */ + +#include <cpu/x86/mtrr.h> + +.global check_mtrr +check_mtrr: + + post_code(0x20) + + /* Use the MTRR default type MSR as a proxy for detecting INIT#. + * Reset the system if any known bits are set in that MSR. That is + * an indication of the CPU not being properly reset. */ + +check_for_clean_reset: + movl $MTRR_DEF_TYPE_MSR, %ecx + rdmsr + andl $(MTRR_DEF_TYPE_EN | MTRR_DEF_TYPE_FIX_EN), %eax + cmp $0, %eax + jz *%esp + /* perform warm reset */ + movw $0xcf9, %dx + movb $0x06, %al + outb %al, %dx + diff --git a/src/soc/intel/common/block/cpu/Makefile.inc b/src/soc/intel/common/block/cpu/Makefile.inc index 5207227..a6c4f37 100644 --- a/src/soc/intel/common/block/cpu/Makefile.inc +++ b/src/soc/intel/common/block/cpu/Makefile.inc @@ -1,4 +1,5 @@ bootblock-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CAR) += car/cache_as_ram.S +bootblock-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CAR) += ../../../../../cpu/x86/early_reset.S bootblock-$(CONFIG_FSP_CAR)+= car/cache_as_ram_fsp.S bootblock-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CPU) += cpulib.c
diff --git a/src/soc/intel/common/block/cpu/car/cache_as_ram.S b/src/soc/intel/common/block/cpu/car/cache_as_ram.S index d3ee671..1160b09 100644 --- a/src/soc/intel/common/block/cpu/car/cache_as_ram.S +++ b/src/soc/intel/common/block/cpu/car/cache_as_ram.S @@ -26,23 +26,8 @@ .global bootblock_pre_c_entry bootblock_pre_c_entry:
- post_code(0x20) - - /* - * Use the MTRR default type MSR as a proxy for detecting INIT#. - * Reset the system if any known bits are set in that MSR. That is - * an indication of the CPU not being properly reset. - */ -check_for_clean_reset: - mov $MTRR_DEF_TYPE_MSR, %ecx - rdmsr - and $(MTRR_DEF_TYPE_EN | MTRR_DEF_TYPE_FIX_EN), %eax - cmp $0, %eax - jz no_reset - /* perform warm reset */ - movw $0xcf9, %dx - movb $0x06, %al - outb %al, %dx + movl $no_reset, %esp /* return address */ + jmp check_mtrr /* Check if CPU properly reset */
no_reset: post_code(0x21)
Hello Patrick Rudolph, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32319
to look at the new patch set (#2).
Change subject: arch/x86: Move checking for MTRR's as a proxy for proper CPU reset ......................................................................
arch/x86: Move checking for MTRR's as a proxy for proper CPU reset
Checking for empty MTRR_DEF_TYPE_MSR as a proxy for proper CPU reset is common across multiple platforms. Therefore place it in a common location.
Change-Id: I81d82fb9fe27cd9de6085251fe1a5685cdd651fc Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- A src/cpu/x86/early_reset.S M src/soc/intel/common/block/cpu/Makefile.inc M src/soc/intel/common/block/cpu/car/cache_as_ram.S 3 files changed, 43 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/32319/2
Hello Kyösti Mälkki, Patrick Rudolph, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32319
to look at the new patch set (#3).
Change subject: cpu/x86: Move checking for MTRR's as a proxy for proper CPU reset ......................................................................
cpu/x86: Move checking for MTRR's as a proxy for proper CPU reset
Checking for empty MTRR_DEF_TYPE_MSR as a proxy for proper CPU reset is common across multiple platforms. Therefore place it in a common location.
Change-Id: I81d82fb9fe27cd9de6085251fe1a5685cdd651fc Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- A src/cpu/x86/early_reset.S M src/soc/intel/common/block/cpu/Makefile.inc M src/soc/intel/common/block/cpu/car/cache_as_ram.S 3 files changed, 43 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/32319/3
Hello Kyösti Mälkki, Patrick Rudolph, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32319
to look at the new patch set (#4).
Change subject: cpu/x86: Move checking for MTRR's as a proxy for proper CPU reset ......................................................................
cpu/x86: Move checking for MTRR's as a proxy for proper CPU reset
Checking for empty MTRR_DEF_TYPE_MSR as a proxy for proper CPU reset is common across multiple platforms. Therefore place it in a common location.
Change-Id: I81d82fb9fe27cd9de6085251fe1a5685cdd651fc Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- A src/cpu/x86/early_reset.S M src/soc/intel/common/block/cpu/Makefile.inc M src/soc/intel/common/block/cpu/car/cache_as_ram.S 3 files changed, 40 insertions(+), 15 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/32319/4
Hello Kyösti Mälkki, Patrick Rudolph, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32319
to look at the new patch set (#5).
Change subject: cpu/x86: Move checking for MTRR's as a proxy for proper CPU reset ......................................................................
cpu/x86: Move checking for MTRR's as a proxy for proper CPU reset
Checking for empty MTRR_DEF_TYPE_MSR as a proxy for proper CPU reset is common across multiple platforms. Therefore place it in a common location.
Change-Id: I81d82fb9fe27cd9de6085251fe1a5685cdd651fc Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- A src/cpu/x86/early_reset.S M src/soc/intel/common/block/cpu/Makefile.inc M src/soc/intel/common/block/cpu/car/cache_as_ram.S 3 files changed, 44 insertions(+), 15 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/32319/5
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32319 )
Change subject: cpu/x86: Move checking for MTRR's as a proxy for proper CPU reset ......................................................................
Patch Set 5: Code-Review+1
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32319 )
Change subject: cpu/x86: Move checking for MTRR's as a proxy for proper CPU reset ......................................................................
Patch Set 5: Code-Review+1
(1 comment)
https://review.coreboot.org/#/c/32319/5/src/cpu/x86/early_reset.S File src/cpu/x86/early_reset.S:
https://review.coreboot.org/#/c/32319/5/src/cpu/x86/early_reset.S@41 PS5, Line 41: outb %al, %dx Add infinite loop after outb ?
Hello Kyösti Mälkki, Patrick Rudolph, Subrata Banik, Aamir Bohra, Patrick Rudolph, Duncan Laurie, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32319
to look at the new patch set (#6).
Change subject: cpu/x86: Move checking for MTRR's as a proxy for proper CPU reset ......................................................................
cpu/x86: Move checking for MTRR's as a proxy for proper CPU reset
Checking for empty MTRR_DEF_TYPE_MSR as a proxy for proper CPU reset is common across multiple platforms. Therefore place it in a common location.
Change-Id: I81d82fb9fe27cd9de6085251fe1a5685cdd651fc Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- A src/cpu/x86/early_reset.S M src/soc/intel/common/block/cpu/Makefile.inc M src/soc/intel/common/block/cpu/car/cache_as_ram.S 3 files changed, 48 insertions(+), 15 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/32319/6
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32319 )
Change subject: cpu/x86: Move checking for MTRR's as a proxy for proper CPU reset ......................................................................
Patch Set 6: Code-Review+2
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32319 )
Change subject: cpu/x86: Move checking for MTRR's as a proxy for proper CPU reset ......................................................................
Patch Set 6: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32319 )
Change subject: cpu/x86: Move checking for MTRR's as a proxy for proper CPU reset ......................................................................
Patch Set 6: Code-Review+2
Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32319 )
Change subject: cpu/x86: Move checking for MTRR's as a proxy for proper CPU reset ......................................................................
cpu/x86: Move checking for MTRR's as a proxy for proper CPU reset
Checking for empty MTRR_DEF_TYPE_MSR as a proxy for proper CPU reset is common across multiple platforms. Therefore place it in a common location.
Change-Id: I81d82fb9fe27cd9de6085251fe1a5685cdd651fc Signed-off-by: Arthur Heymans arthur@aheymans.xyz Reviewed-on: https://review.coreboot.org/c/coreboot/+/32319 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Duncan Laurie dlaurie@chromium.org Reviewed-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-by: Furquan Shaikh furquan@google.com --- A src/cpu/x86/early_reset.S M src/soc/intel/common/block/cpu/Makefile.inc M src/soc/intel/common/block/cpu/car/cache_as_ram.S 3 files changed, 48 insertions(+), 15 deletions(-)
Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved Duncan Laurie: Looks good to me, approved Furquan Shaikh: Looks good to me, approved
diff --git a/src/cpu/x86/early_reset.S b/src/cpu/x86/early_reset.S new file mode 100644 index 0000000..ec015abe --- /dev/null +++ b/src/cpu/x86/early_reset.S @@ -0,0 +1,45 @@ +/* + * This file is part of the coreboot project. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +/* + * input %esp: return address (not pointer to return address!) + * clobber the content of eax, ecx, edx + */ + +#include <cpu/x86/mtrr.h> + +.section .text +.global check_mtrr + +check_mtrr: + /* Use the MTRR default type MSR as a proxy for detecting INIT#. + * Reset the system if any known bits are set in that MSR. That is + * an indication of the CPU not being properly reset. */ + +check_for_clean_reset: + movl $MTRR_DEF_TYPE_MSR, %ecx + rdmsr + andl $(MTRR_DEF_TYPE_EN | MTRR_DEF_TYPE_FIX_EN), %eax + cmp $0, %eax + jnz warm_reset + jmp *%esp + /* perform warm reset */ +warm_reset: + movw $0xcf9, %dx + movb $0x06, %al + outb %al, %dx + /* Should not reach this*/ +.Lhlt: + hlt + jmp .Lhlt diff --git a/src/soc/intel/common/block/cpu/Makefile.inc b/src/soc/intel/common/block/cpu/Makefile.inc index 5207227..a6c4f37 100644 --- a/src/soc/intel/common/block/cpu/Makefile.inc +++ b/src/soc/intel/common/block/cpu/Makefile.inc @@ -1,4 +1,5 @@ bootblock-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CAR) += car/cache_as_ram.S +bootblock-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CAR) += ../../../../../cpu/x86/early_reset.S bootblock-$(CONFIG_FSP_CAR)+= car/cache_as_ram_fsp.S bootblock-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CPU) += cpulib.c
diff --git a/src/soc/intel/common/block/cpu/car/cache_as_ram.S b/src/soc/intel/common/block/cpu/car/cache_as_ram.S index d3ee671..b1648e8 100644 --- a/src/soc/intel/common/block/cpu/car/cache_as_ram.S +++ b/src/soc/intel/common/block/cpu/car/cache_as_ram.S @@ -28,21 +28,8 @@
post_code(0x20)
- /* - * Use the MTRR default type MSR as a proxy for detecting INIT#. - * Reset the system if any known bits are set in that MSR. That is - * an indication of the CPU not being properly reset. - */ -check_for_clean_reset: - mov $MTRR_DEF_TYPE_MSR, %ecx - rdmsr - and $(MTRR_DEF_TYPE_EN | MTRR_DEF_TYPE_FIX_EN), %eax - cmp $0, %eax - jz no_reset - /* perform warm reset */ - movw $0xcf9, %dx - movb $0x06, %al - outb %al, %dx + movl $no_reset, %esp /* return address */ + jmp check_mtrr /* Check if CPU properly reset */
no_reset: post_code(0x21)
-
Arthur Heymans (Code Review) -
Duncan Laurie (Code Review)
-
Furquan Shaikh (Code Review)
-
Kyösti Mälkki (Code Review)
-
Martin Roth (Code Review)
-
Patrick Rudolph (Code Review)