Nico Huber has submitted this change. ( https://review.coreboot.org/c/libgfxinit/+/83598?usp=email )
(
1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)Change subject: transcoder: Don't try to disable disabled DDI func
......................................................................
transcoder: Don't try to disable disabled DDI func
Tiger Lake makes some trouble on the `All_Off' path in case a trans-
coder is already disabled. It somehow seems to be able to seriously
lock things up inside the processor, causing Linux to hang, report
stalled CPUs, and the reset button not to work anymore. This might
be related to disabled power wells, that we do not keep track of on
the `All_Off' path.
As there shouldn't be no harm in skipping the register write for an
already disabled transcoder, just do that.
Tested with Kontron COMe bTL6.
Change-Id: Ia505422570d967b192fe2eb8cab10f305aff3dd7
Signed-off-by: Nico Huber <nico.huber(a)secunet.com>
Reviewed-on: https://review.coreboot.org/c/libgfxinit/+/83598
Reviewed-by: Angel Pons <th3fanbus(a)gmail.com>
Tested-by: Nico Huber <nico.h(a)gmx.de>
---
M common/hw-gfx-gma-transcoder.adb
1 file changed, 4 insertions(+), 1 deletion(-)
Approvals:
Nico Huber: Verified
Angel Pons: Looks good to me, approved
diff --git a/common/hw-gfx-gma-transcoder.adb b/common/hw-gfx-gma-transcoder.adb
index 9e85b75..6eea451 100644
--- a/common/hw-gfx-gma-transcoder.adb
+++ b/common/hw-gfx-gma-transcoder.adb
@@ -373,7 +373,10 @@
end if;
if Config.Has_Pipe_DDI_Func then
- Registers.Write (Trans.DDI_FUNC_CTL, 0);
+ Registers.Is_Set_Mask (Trans.DDI_FUNC_CTL, DDI_FUNC_CTL_ENABLE, Enabled);
+ if Enabled then
+ Registers.Write (Trans.DDI_FUNC_CTL, 0);
+ end if;
end if;
end Trans_Off;
--
To view, visit https://review.coreboot.org/c/libgfxinit/+/83598?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: libgfxinit
Gerrit-Branch: main
Gerrit-Change-Id: Ia505422570d967b192fe2eb8cab10f305aff3dd7
Gerrit-Change-Number: 83598
Gerrit-PatchSet: 4
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Attention is currently required from: Paul Menzel.
Nico Huber has posted comments on this change by Nico Huber. ( https://review.coreboot.org/c/libgfxinit/+/83598?usp=email )
Change subject: transcoder: Don't try to disable disabled DDI func
......................................................................
Patch Set 3: Verified+1
(1 comment)
Commit Message:
https://review.coreboot.org/c/libgfxinit/+/83598/comment/14fc6cad_1e61e8a0?… :
PS2, Line 18:
> What device do you test with?
Done
--
To view, visit https://review.coreboot.org/c/libgfxinit/+/83598?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: libgfxinit
Gerrit-Branch: main
Gerrit-Change-Id: Ia505422570d967b192fe2eb8cab10f305aff3dd7
Gerrit-Change-Number: 83598
Gerrit-PatchSet: 3
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Sat, 27 Jul 2024 16:00:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Attention is currently required from: Nico Huber.
Hello Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/libgfxinit/+/83598?usp=email
to look at the new patch set (#3).
Change subject: transcoder: Don't try to disable disabled DDI func
......................................................................
transcoder: Don't try to disable disabled DDI func
Tiger Lake makes some trouble on the `All_Off' path in case a trans-
coder is already disabled. It somehow seems to be able to seriously
lock things up inside the processor, causing Linux to hang, report
stalled CPUs, and the reset button not to work anymore. This might
be related to disabled power wells, that we do not keep track of on
the `All_Off' path.
As there shouldn't be no harm in skipping the register write for an
already disabled transcoder, just do that.
Tested with Kontron COMe bTL6.
Change-Id: Ia505422570d967b192fe2eb8cab10f305aff3dd7
Signed-off-by: Nico Huber <nico.huber(a)secunet.com>
---
M common/hw-gfx-gma-transcoder.adb
1 file changed, 4 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/libgfxinit refs/changes/98/83598/3
--
To view, visit https://review.coreboot.org/c/libgfxinit/+/83598?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: libgfxinit
Gerrit-Branch: main
Gerrit-Change-Id: Ia505422570d967b192fe2eb8cab10f305aff3dd7
Gerrit-Change-Number: 83598
Gerrit-PatchSet: 3
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Attention is currently required from: Arthur Heymans, Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Shuo Liu, Tim Chu.
Nico Huber has posted comments on this change by Shuo Liu. ( https://review.coreboot.org/c/coreboot/+/83538?usp=email )
Change subject: soc/intel/xeon_sp: Reserve FSP MMIO high window
......................................................................
Patch Set 2: Code-Review+1
(5 comments)
Patchset:
PS2:
Side note: This whole situation is becoming too messy for my taste. Especially
as there seems still no progress in documenting the FSP (at least none that
I've heard of). I think we should make it a requirement for future FSP versions
(GNR + 1) that FSP does *not* do any ressource assignments, and otherwise
not accept it upstream. I'm sure this would have saved us all a huge amount
of time by now.
That's just my opinion. I'm losing faith here because it feels like I'm not
giving the review the necessary attention. Attention only necessary because
of a botched blob design. Only option out seems to not review.
Commit Message:
https://review.coreboot.org/c/coreboot/+/83538/comment/7b281765_37d6b0db?us… :
PS1, Line 7: soc/intel/xeon_sp: Reserve FSP MMIO high window
> GNR and later SoC also fit.
I don't understand. I meant they are not covered by this change, so the
commit summary should mention SPR.
> There will be MMIO ranges not for PCI domain usage, and thus forming holes.
What does that mean "not for PCI domain usage"? Is it used for anything else?
https://review.coreboot.org/c/coreboot/+/83538/comment/650dbb46_e0d481c6?us… :
PS1, Line 19: is especially important
: on systems with 2 or more sockets, where each socket has multiple
: domains.
> On multiple socket system, each socket will have IIO stacks and PCI domains under them, and thus the […]
Well, you could also tell it to use WB default instead. But I get it, Linux
relies too much on the BIOS there (I very much would have expected it to use
PAT right away).
That Linux complains would be nice to have in the commit message (that's more
informative than "it's important").
File src/soc/intel/xeon_sp/uncore.c:
https://review.coreboot.org/c/coreboot/+/83538/comment/8a529726_b1e4003c?us… :
PS1, Line 375: index++;
> Yes, mmapvtd will have both anonymous static resources (starting from 0x0) and PCI resources (0x180, […]
No, I was not referring to any 0x180, but to the standard PCI BARs that
pci_dev_read_resources() would add if they exist (if not, why call it?).
Solution looks ok, though.
File src/soc/intel/xeon_sp/uncore.c:
https://review.coreboot.org/c/coreboot/+/83538/comment/55147469_e0bbca29?us… :
PS2, Line 371: * MMIO high window has to be added in set_resources instead of read_resources
: * due to that adding in read_resources will cause the whole window reserved
: * and cannot be used for resource allocation.
```suggestion
* The MMIO high window has to be added in set_resources() instead of
* read_resources(). Because adding in read_resources() would cause the
* whole window to be reserved, and it couldn't be used for resource
* allocation.
```
--
To view, visit https://review.coreboot.org/c/coreboot/+/83538?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ib2a0e1f1f13e797c1fab6aca589d060c4d3fa15b
Gerrit-Change-Number: 83538
Gerrit-PatchSet: 2
Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Sat, 27 Jul 2024 15:57:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com>
Attention is currently required from: Felix Singer, Jonathon Hall, Martin L Roth, Nigel Tao, Paul Menzel.
Nico Huber has posted comments on this change by Jonathon Hall. ( https://review.coreboot.org/c/coreboot/+/83476?usp=email )
Change subject: bootsplash: Increase heap from 1 MB to 4 MB when bootsplash is enabled
......................................................................
Patch Set 2:
(2 comments)
Patchset:
PS2:
> I don't have a problem with 4 MB. […]
I would have to ask "where would we stop?" If next time somebody runs into this
problem with a 4K image and wants to increase it to 16MiB, what would we do then?
For reference: The heap is kept during OS runtime by default because we'd need
it during S3 resume. And we still have Pentium II boards in the tree.
PS2:
NB. We should probably try to save the next person to run into this
issue the time to figure out what is going wrong. Maybe a warning
in the Kconfig prompt? or I guess some heuristic that even breaks
the build in case the file is >> HEAP_SIZE?
--
To view, visit https://review.coreboot.org/c/coreboot/+/83476?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ia4348d39effbc16c1b42ab01bcf1e4ec5d652fa9
Gerrit-Change-Number: 83476
Gerrit-PatchSet: 2
Gerrit-Owner: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Nigel Tao <nigeltao(a)golang.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Attention: Nigel Tao <nigeltao(a)golang.org>
Gerrit-Comment-Date: Sat, 27 Jul 2024 15:04:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Jonathon Hall <jonathon.hall(a)puri.sm>
Comment-In-Reply-To: Nigel Tao <nigeltao(a)golang.org>
Attention is currently required from: Angel Pons, Dinesh Gehlot, Eric Lai, Kapil Porwal, Matt DeVillier, Nick Vaccaro, Paul Menzel, Sean Rhodes, Subrata Banik.
Nico Huber has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/81638?usp=email )
Change subject: intel/alderlake: Add helper functions for Power Management
......................................................................
Patch Set 22: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/81638?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I9db18859f9a04ad4b7c0c3f7992b09e0f9484a81
Gerrit-Change-Number: 81638
Gerrit-PatchSet: 22
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Eric Lai <ericllai(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Sat, 27 Jul 2024 14:38:15 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Angel Pons, Dinesh Gehlot, Eric Lai, Kapil Porwal, Matt DeVillier, Nick Vaccaro, Paul Menzel, Sean Rhodes, Subrata Banik.
Nico Huber has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/81638?usp=email )
Change subject: intel/alderlake: Add helper functions for Power Management
......................................................................
Patch Set 22:
(9 comments)
Patchset:
PS22:
Thanks for keeping up the good work! and sorry for the long silence on my end.
Commit Message:
https://review.coreboot.org/c/coreboot/+/81638/comment/0c243ac4_f0295f9f?us… :
PS22, Line 14: Skylake, this is not the
: case for Alderlake, Raptorlake and Meteorlake.
I think what Paul tried to tell you but less verbose: These started
to be named after actual sites. Hence there is no consistency in the
spelling: It's Alder Lake, Raptor Lake, Meteor Lake but the exception:
Skylake.
File src/soc/intel/alderlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/81638/comment/868eb2c9_48365d3c?us… :
PS22, Line 494: adl_get_aspm_control
Could put something like `to_upd` or `to_fsp` in the helper names.
https://review.coreboot.org/c/coreboot/+/81638/comment/4f9522e1_ee6493e2?us… :
PS22, Line 494: unsigned int
`enum ASPM_control` to make it clear we're converting from that to something else.
(Actually `enum aspm_control` if we follow coreboot conventions.)
https://review.coreboot.org/c/coreboot/+/81638/comment/4bde1391_10d1e962?us… :
PS22, Line 507: unsigned int
`enum ...`
https://review.coreboot.org/c/coreboot/+/81638/comment/e3cfc261_5b369c53?us… :
PS22, Line 530: uint32_t
Just (`unsigned`)`int` for an index? Unless I miss something
File src/soc/intel/common/block/include/intelblocks/pcie_rp.h:
https://review.coreboot.org/c/coreboot/+/81638/comment/451a79d6_6c737fad?us… :
PS22, Line 40: are zero-indexed
To me this doesn't explain much, our enums are also zero-indexed. I'd rather
comment that our enums should be (left) compatible but off-by-1.
https://review.coreboot.org/c/coreboot/+/81638/comment/7f3771ea_c3e1e059?us… :
PS22, Line 43: /* This enum is for passing into an FSP UPD, typically PcieRpL1Substates */
Don't want to push to fix things that were already wrong before, but this is
not true: The enum is for the devicetree, the converted value is passed to FSP.
I guess the comment was always wrong.
https://review.coreboot.org/c/coreboot/+/81638/comment/405856f4_16a97795?us… :
PS22, Line 45: FSP_DEFAULT
Probably for another patch:
With our own rules like `/* Use auto unless overwritten */` this doesn't match
the meaning anymore. How about `L1_SS_UNSET`? Then we could also make the code
more explicit, e.g.
```
if (dt_l1ss == L1_SS_UNSET)
```
Should make it self-documenting.
--
To view, visit https://review.coreboot.org/c/coreboot/+/81638?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I9db18859f9a04ad4b7c0c3f7992b09e0f9484a81
Gerrit-Change-Number: 81638
Gerrit-PatchSet: 22
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Eric Lai <ericllai(a)google.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Sat, 27 Jul 2024 14:38:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Nico Huber has posted comments on this change by Rishika Raj. ( https://review.coreboot.org/c/coreboot/+/83540?usp=email )
Change subject: soc/intel/mtl: Increase CAR_STACK_SIZE by 31KB for coreboot compatibility
......................................................................
Patch Set 10:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83540/comment/dfd76e34_815bdc4e?us… :
PS5, Line 7: soc/intel/meteorlake: Increase CAR STACK_SIZE by 31KB to meet coreboot requirements
Sorry for the delay.
> I'm not sure how much static analysis can do there because coreboot does have a bunch of code paths that are affected by runtime values (and a few recursive functions), so I'm not sure if it's really possible to reliably determine stack use at build time. If you know some solution that can do that it would be great if you could integrate it into the CI, of course.
Right, I had only the romstage case in mind, hopefully that's without
recursion. Haven't found "the" tool yet, only did a quick search.
> We do use `-Wstack-usage=1536` on non-x86 boards and maybe we should do something like that on x86 as well, but I believe that only checks each function on its own.
GCC seems to provide enough information with `-fcallgraph-info=su` in
a somewhat parseable `.ci` file. Probably doable with a simple awk script,
but could take half a day and it seems worth to look for existing tools
first. (Saying "enough" information, because it'd be an overestimation:
it provides the maximum stack usage per function, not per path, i.e. if
you have separate paths through the function with different stack usages,
you'd get the sum of the local stack usage as if everything was allocated
at the top, AIUI.)
> There is code in `romstage_main()` to place and check stack canaries and I assume that it should fire if we have an overflow like we're suspecting, but it just writes a BIOS_DEBUG to the console and nothing else. My suspicion is that the FSP may only get into the path that really makes it use its whole 512K in very rare and unreproducible circumstances (e.g. related to memory training), and we've never managed to capture the boot that actually fails in our case on a UART. All we are seeing is corrupted vboot data structures persisted in the TPM that look like they would have been created by coreboot trying to write back vboot state from memory that has been overwritten with a bunch of 0xff.
For FSP I think something like one canary per 4KiB could make sense, so we
could quickly get an idea how much it's usually using. Of course that will
never tell us the worst case, but I think it's interesting enough to know.
I'll probably look into that.
>
> I am not against moving the UPD and other big things off the stack either if someone wants to do that. (I've also been thinking about doing some other things to harden against stack overflows, e.g. move the stack to the end of the CAR space and let it grow downwards into unallocated space, but I likely won't have time to finish that work any time soon unfortunately.) For now, we're just trying to do the most simple thing that should reasonably ensure we're not going to have a problem on the Meteorlake platform.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83540?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Iba3620b3b7c470176330f5e07989cd3f6238713e
Gerrit-Change-Number: 83540
Gerrit-PatchSet: 10
Gerrit-Owner: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Karthikeyan Ramasubramanian <kramasub(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun <tstuli(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Sat, 27 Jul 2024 11:46:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>