coreboot-gerrit April 2022

coreboot-gerrit@coreboot.org

1 participants
5879 discussions

Change in coreboot[master]: soc/intel/common/xhci: Add function to reset the XHCI host controller
by Subrata Banik (Code Review) 11 Apr '22

11 Apr '22

Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/63551 ) Change subject: soc/intel/common/xhci: Add function to reset the XHCI host controller ...................................................................... soc/intel/common/xhci: Add function to reset the XHCI host controller This patch adds `xhci_host_reset()` to reset XHCI controller and the scope of this function is with SMM hence, compiling xhci.c for SMM as well. Also, refactored `xhci.c` code to keep PCI enumeration within the scope of `ramstage` alone hence, guarded with `ENV_RAMSTAGE` env_variable. BUG=b:227289581 TEST=Able to perform a call from `xhci_host_reset` from S5 smi handler. Signed-off-by: Subrata Banik <subratabanik(a)google.com> Change-Id: Ie0dc0a64044f291893931726d26c08c8b964a3cc --- M src/soc/intel/common/block/include/intelblocks/xhci.h M src/soc/intel/common/block/xhci/Makefile.inc M src/soc/intel/common/block/xhci/xhci.c 3 files changed, 28 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/63551/1 diff --git a/src/soc/intel/common/block/include/intelblocks/xhci.h b/src/soc/intel/common/block/include/intelblocks/xhci.h index 1adcbc0..7e2b76e 100644 --- a/src/soc/intel/common/block/include/intelblocks/xhci.h +++ b/src/soc/intel/common/block/include/intelblocks/xhci.h @@ -46,6 +46,8 @@ bool xhci_update_wake_event(const struct xhci_wake_info *wake_info, size_t wake_info_count); +/* xhci_host_reset() - Function to reset the host controller */ +void xhci_host_reset(void); void soc_xhci_init(struct device *dev); /* diff --git a/src/soc/intel/common/block/xhci/Makefile.inc b/src/soc/intel/common/block/xhci/Makefile.inc index d1c505e..831992a 100644 --- a/src/soc/intel/common/block/xhci/Makefile.inc +++ b/src/soc/intel/common/block/xhci/Makefile.inc @@ -1,4 +1,5 @@ ramstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_XHCI) += xhci.c ramstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_XHCI_ELOG) += elog.c +smm-$(CONFIG_SOC_INTEL_COMMON_BLOCK_XHCI) += xhci.c smm-$(CONFIG_SOC_INTEL_COMMON_BLOCK_XHCI_ELOG) += elog.c diff --git a/src/soc/intel/common/block/xhci/xhci.c b/src/soc/intel/common/block/xhci/xhci.c index 9317bb0..4933f84 100644 --- a/src/soc/intel/common/block/xhci/xhci.c +++ b/src/soc/intel/common/block/xhci/xhci.c @@ -13,9 +13,33 @@ #define XHCI_USB2 2 #define XHCI_USB3 3 +#define XHCI_USBCMD 0x80 +#define USBCMD_HCRST (1 << 1) + /* Current Connect Status */ #define XHCI_STATUS_CCS (1 << 0) +static uint8_t *xhci_mem_base(void) +{ + uint32_t mem_base = pci_read_config32(PCH_DEV_XHCI, PCI_BASE_ADDRESS_0); + + /* Check if the controller is disabled or not present */ + if (mem_base == 0 || mem_base == 0xffffffff) + return 0; + + return (uint8_t *)(mem_base & ~0xf); +} + +void xhci_host_reset(void) +{ + uint8_t *xhci_base = xhci_mem_base(); + if (!xhci_base) + return; + + setbits8(xhci_base + XHCI_USBCMD, USBCMD_HCRST); +} + +#if ENV_RAMSTAGE static bool is_usb_port_connected(const struct xhci_usb_info *info, unsigned int port_type, unsigned int port_id) { @@ -134,3 +158,4 @@ .vendor = PCI_VID_INTEL, .devices = pci_device_ids, }; +#endif -- To view, visit https://review.coreboot.org/c/coreboot/+/63551 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie0dc0a64044f291893931726d26c08c8b964a3cc Gerrit-Change-Number: 63551 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: soc/intel/fast_spi.c: Use smarter mtrr code in ramstage
by Subrata Banik (Code Review) 11 Apr '22

11 Apr '22

Attention is currently required from: Arthur Heymans. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63550 ) Change subject: soc/intel/fast_spi.c: Use smarter mtrr code in ramstage ...................................................................... Patch Set 1: -Code-Review (1 comment) Patchset: PS1: I will test this CL once again. -- To view, visit https://review.coreboot.org/c/coreboot/+/63550 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I283ba07fc12c410be39dfdc828657598237247c1 Gerrit-Change-Number: 63550 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Mon, 11 Apr 2022 12:34:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/fast_spi.c: Use smarter mtrr code in ramstage
by Subrata Banik (Code Review) 11 Apr '22

11 Apr '22

Attention is currently required from: Arthur Heymans. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63550 ) Change subject: soc/intel/fast_spi.c: Use smarter mtrr code in ramstage ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/63550 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I283ba07fc12c410be39dfdc828657598237247c1 Gerrit-Change-Number: 63550 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Mon, 11 Apr 2022 12:33:08 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: cpu/x86/mtrrlib: Use `need restore mtrr()` from set var mtrr function
by Arthur Heymans (Code Review) 11 Apr '22

11 Apr '22

Attention is currently required from: Subrata Banik, Tim Wawrzynczak, Nick Vaccaro, Eric Lai. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63492 ) Change subject: cpu/x86/mtrrlib: Use `need_restore_mtrr()` from set_var_mtrr function ...................................................................... Patch Set 1: (1 comment) File src/cpu/x86/mtrr/mtrrlib.c: https://review.coreboot.org/c/coreboot/+/63492/comment/56a429e6_d5e568d5 PS1, Line 52: /* Need to restore mtrr later using remove_temp_solution. */ : if (ENV_RAMSTAGE) : need_restore_mtrr(); > mostly mtrr_use_temp_range() is added for ramstage along using mtrr.c file. hence, last time I saw some compilation issue and decided to expand `set_var_mtrr` for such simple var mtrr setup. Let's see what jenkins makes of it. https://review.coreboot.org/c/coreboot/+/63550 worked fine locally. -- To view, visit https://review.coreboot.org/c/coreboot/+/63492 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I67fd36080b318ace53f3e34da53d747f9a3aa400 Gerrit-Change-Number: 63492 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Comment-Date: Mon, 11 Apr 2022 12:31:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/fast_spi.c: Use smarter mtrr code in ramstage
by Arthur Heymans (Code Review) 11 Apr '22

11 Apr '22

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/63550 ) Change subject: soc/intel/fast_spi.c: Use smarter mtrr code in ramstage ...................................................................... soc/intel/fast_spi.c: Use smarter mtrr code in ramstage mtrr_use_temp_range is a lot smarter than the plain set_var_mtrr. It will compute a new optimal solution with the temp ranges included while also taking care of the cleanup before loading the payload/s3 resume. Change-Id: I283ba07fc12c410be39dfdc828657598237247c1 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/soc/intel/common/block/fast_spi/fast_spi.c 1 file changed, 5 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/63550/1 diff --git a/src/soc/intel/common/block/fast_spi/fast_spi.c b/src/soc/intel/common/block/fast_spi/fast_spi.c index 5a76df3..20463d3 100644 --- a/src/soc/intel/common/block/fast_spi/fast_spi.c +++ b/src/soc/intel/common/block/fast_spi/fast_spi.c @@ -199,6 +199,11 @@ static void fast_spi_enable_cache_range(unsigned int base, unsigned int size) { + if (ENV_RAMSTAGE) { + mtrr_use_temp_range(base, size, MTRR_TYPE_WRPROT); + return; + } + const int type = MTRR_TYPE_WRPROT; int mtrr = get_free_var_mtrr(); if (mtrr == -1) { -- To view, visit https://review.coreboot.org/c/coreboot/+/63550 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I283ba07fc12c410be39dfdc828657598237247c1 Gerrit-Change-Number: 63550 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: mb/google/brask/variants/moli: update overridetree for moli
by Eric Lai (Code Review) 11 Apr '22

11 Apr '22

Attention is currently required from: YH Lin, Subrata Banik, Paul Menzel, Tim Wawrzynczak, Zhuohao Lee, Raihow Shi, Felix Held. Eric Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63080 ) Change subject: mb/google/brask/variants/moli: update overridetree for moli ...................................................................... Patch Set 13: (1 comment) Patchset: PS13: Please make another change which not relevant to the eMMC and NVME to another CL. And for NVMe and eMMC fw_config part is okay. -- To view, visit https://review.coreboot.org/c/coreboot/+/63080 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If83031edcd90ea746704590765102b9b0dee03c1 Gerrit-Change-Number: 63080 Gerrit-PatchSet: 13 Gerrit-Owner: Raihow Shi <raihow_shi(a)wistron.corp-partner.google.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: YH Lin <yueherngl(a)google.com> Gerrit-Reviewer: Zhuohao Lee <zhuohao(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-CC: Anfernee Chen <anfernee_chen(a)wistron.corp-partner.google.com> Gerrit-CC: Ariel Fang <ariel_fang(a)wistron.corp-partner.google.com> Gerrit-CC: Casper Chang <casper_chang(a)wistron.corp-partner.google.com> Gerrit-CC: Malik Hsu <malik_hsu(a)wistron.com> Gerrit-CC: Mark Hsieh <mark_hsieh(a)wistron.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Peter Chi <peter_chi(a)wistron.corp-partner.google.com> Gerrit-CC: Scott Chao <scott_chao(a)wistron.corp-partner.google.com> Gerrit-CC: Terry Chen <terry_chen(a)wistron.corp-partner.google.com> Gerrit-CC: Will Tsai <will_tsai(a)wistron.corp-partner.google.com> Gerrit-CC: Zoey Wu <zoey_wu(a)wistron.corp-partner.google.com> Gerrit-Attention: YH Lin <yueherngl(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Zhuohao Lee <zhuohao(a)google.com> Gerrit-Attention: Raihow Shi <raihow_shi(a)wistron.corp-partner.google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Mon, 11 Apr 2022 12:27:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: cpu/x86/mtrrlib: Use `need restore mtrr()` from set var mtrr function
by Arthur Heymans (Code Review) 11 Apr '22

11 Apr '22

Attention is currently required from: Tim Wawrzynczak, Arthur Heymans, Nick Vaccaro, Eric Lai. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63492 ) Change subject: cpu/x86/mtrrlib: Use `need_restore_mtrr()` from set_var_mtrr function ...................................................................... Patch Set 1: (1 comment) File src/cpu/x86/mtrr/mtrrlib.c: https://review.coreboot.org/c/coreboot/+/63492/comment/e66a1078_c93d9225 PS1, Line 52: /* Need to restore mtrr later using remove_temp_solution. */ : if (ENV_RAMSTAGE) : need_restore_mtrr(); > > > I don't like this. set_var_mtrr should do just one thing: set a variable mtrr... > > > I if you want a temp mtrr with this function, it should be the caller setting this up. > > > > Are you suggesting to make need_restore_mtrr() from the caller of `set_var_mtrr()` ? > > Yes. > > > I thought, after DRAM MTRR snapshot being implemented, if user called into `set_var_mtrr()` then set_var_mtrr() implement inherently sets > > put_back_original_solution variable. but I do see a value in your suggestion. I think try to call this call exclusive of set_var_mtrr(). > > What about a function that makes this temporary nature more explicit and calls both set_var_mtrr and calls need_restore_mtrr? > > Hmm it looks like I assumed that mtrr_use_temp_range() was used but in fact set_var_mtrr() is used in fast_spi.c. Given that the API's of mtrr_use_temp_range and set_var_mtrr are very similar it might be a good idea to have the if (ENV_RAMSTAGE) code in soc/intel/fast_spi.c instead to use mtrr_use_temp_range? That would allow to avoid using the more manual set_var_mtrr + there would be no need to expose need_restore_mtrr. A bonus of that approach is that disable_cache() is actually being called before changing MTRRs. I don't think it matters all that much but that's what "11.11.7.2 MemTypeSet() Function" recommends in the "Intel® 64 and IA-32 Architectures Software Developer’s Manual". -- To view, visit https://review.coreboot.org/c/coreboot/+/63492 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I67fd36080b318ace53f3e34da53d747f9a3aa400 Gerrit-Change-Number: 63492 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Comment-Date: Mon, 11 Apr 2022 12:24:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: cpu/x86/mtrrlib: Use `need restore mtrr()` from set var mtrr function
by Subrata Banik (Code Review) 11 Apr '22

11 Apr '22

Attention is currently required from: Tim Wawrzynczak, Arthur Heymans, Nick Vaccaro, Eric Lai. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63492 ) Change subject: cpu/x86/mtrrlib: Use `need_restore_mtrr()` from set_var_mtrr function ...................................................................... Patch Set 1: (1 comment) File src/cpu/x86/mtrr/mtrrlib.c: https://review.coreboot.org/c/coreboot/+/63492/comment/50c00568_6111ecb1 PS1, Line 52: /* Need to restore mtrr later using remove_temp_solution. */ : if (ENV_RAMSTAGE) : need_restore_mtrr(); > > > I don't like this. set_var_mtrr should do just one thing: set a variable mtrr... > > > I if you want a temp mtrr with this function, it should be the caller setting this up. > > > > Are you suggesting to make need_restore_mtrr() from the caller of `set_var_mtrr()` ? > > Yes. > > > I thought, after DRAM MTRR snapshot being implemented, if user called into `set_var_mtrr()` then set_var_mtrr() implement inherently sets > > put_back_original_solution variable. but I do see a value in your suggestion. I think try to call this call exclusive of set_var_mtrr(). > > What about a function that makes this temporary nature more explicit and calls both set_var_mtrr and calls need_restore_mtrr? > > Hmm it looks like I assumed that mtrr_use_temp_range() was used but in fact set_var_mtrr() is used in fast_spi.c. Given that the API's of mtrr_use_temp_range and set_var_mtrr are very similar it might be a good idea to have the if (ENV_RAMSTAGE) code in soc/intel/fast_spi.c instead to use mtrr_use_temp_range? That would allow to avoid using the more manual set_var_mtrr + there would be no need to expose need_restore_mtrr. mostly mtrr_use_temp_range() is added for ramstage along using mtrr.c file. hence, last time I saw some compilation issue and decided to expand `set_var_mtrr` for such simple var mtrr setup. -- To view, visit https://review.coreboot.org/c/coreboot/+/63492 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I67fd36080b318ace53f3e34da53d747f9a3aa400 Gerrit-Change-Number: 63492 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Comment-Date: Mon, 11 Apr 2022 12:23:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: cpu/x86/mtrrlib: Use `need restore mtrr()` from set var mtrr function
by Arthur Heymans (Code Review) 11 Apr '22

11 Apr '22

Attention is currently required from: Subrata Banik, Tim Wawrzynczak, Nick Vaccaro, Eric Lai. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63492 ) Change subject: cpu/x86/mtrrlib: Use `need_restore_mtrr()` from set_var_mtrr function ...................................................................... Patch Set 1: (1 comment) File src/cpu/x86/mtrr/mtrrlib.c: https://review.coreboot.org/c/coreboot/+/63492/comment/2db6890f_342fdc75 PS1, Line 52: /* Need to restore mtrr later using remove_temp_solution. */ : if (ENV_RAMSTAGE) : need_restore_mtrr(); > > I don't like this. set_var_mtrr should do just one thing: set a variable mtrr... > > I if you want a temp mtrr with this function, it should be the caller setting this up. > > Are you suggesting to make need_restore_mtrr() from the caller of `set_var_mtrr()` ? Yes. > I thought, after DRAM MTRR snapshot being implemented, if user called into `set_var_mtrr()` then set_var_mtrr() implement inherently sets > put_back_original_solution variable. but I do see a value in your suggestion. I think try to call this call exclusive of set_var_mtrr(). What about a function that makes this temporary nature more explicit and calls both set_var_mtrr and calls need_restore_mtrr? Hmm it looks like I assumed that mtrr_use_temp_range() was used but in fact set_var_mtrr() is used in fast_spi.c. Given that the API's of mtrr_use_temp_range and set_var_mtrr are very similar it might be a good idea to have the if (ENV_RAMSTAGE) code in soc/intel/fast_spi.c instead to use mtrr_use_temp_range? That would allow to avoid using the more manual set_var_mtrr + there would be no need to expose need_restore_mtrr. -- To view, visit https://review.coreboot.org/c/coreboot/+/63492 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I67fd36080b318ace53f3e34da53d747f9a3aa400 Gerrit-Change-Number: 63492 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Comment-Date: Mon, 11 Apr 2022 12:16:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: coreboot_tables: Replace 'struct lb_uint64' with lb_uint64_t
by Jianjun Wang (Code Review) 11 Apr '22

11 Apr '22

Attention is currently required from: Hung-Te Lin, Jakub Czapiga, Julius Werner, Arthur Heymans, Yu-Ping Wu. Jianjun Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63494 ) Change subject: coreboot_tables: Replace 'struct lb_uint64' with lb_uint64_t ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: > Cool! I learned a new thing today as I didn't know this was possible :-) Thanks to the suggestion from Julius! -- To view, visit https://review.coreboot.org/c/coreboot/+/63494 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If6b037e4403a8000625f4a5fb8d20311fe76200a Gerrit-Change-Number: 63494 Gerrit-PatchSet: 4 Gerrit-Owner: Jianjun Wang <jianjun.wang(a)mediatek.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Mon, 11 Apr 2022 12:01:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2022