coreboot-gerrit April 2019

coreboot-gerrit@coreboot.org

1 participants
1219 discussions

Change in ...coreboot[master]: cbfstool: Add ifittool
by Nico Huber (Code Review) 30 Apr '19

30 Apr '19

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31493 ) Change subject: cbfstool: Add ifittool ...................................................................... Patch Set 11: (23 comments) There is some oddity around `fit_offset_converter_t`, otherwise the code looks pretty good :) Some inline comments were written before I realized that the code was only shuffled around. And I didn't get to `ifittool.c` yet. https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.h File util/cbfstool/fit.h: https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.h@49 PS11, Line 49: */ I honestly don't get it. This seems to sufficiently specify what the function does. If there is no room for alternative behavior, there is no need for a function pointer? Can somebody help me here? https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c File util/cbfstool/fit.c: https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@a132 PS11, Line 132: We can still assume that it contains at least the header. https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@45 PS11, Line 45: and must be set to 0 Only until a platform starts to use them. It seems at least APL does. https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@58 PS11, Line 58: * Bit 7 indicates whether component has valid checksum. mention C_V? as it's referred to as that below https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@182 PS11, Line 182: but only mentioned on page 22 of It is mentioned below "FIT Ordering Rules" in all specifications I know. https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@188 PS11, Line 188: if (fit_table_entries(fit) <= 1) : return; What difference does it make? https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@249 PS11, Line 249: fit_header It seems odd not to comment this function (but others with a self explanatory name). OTOH, maybe just give it a better name, e.g. fit_location_from_cbfs_header()? Generally, I would expect the output parameter(s) to come first. Shouldn't `ptr` be `const void *`? https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@256 PS11, Line 256: cbfs_file_get_header(&buf, &header); `buf.offset` (and `.name`) are still uninitialized https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@263 PS11, Line 263: ssize_t why not `size_t`? https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@264 PS11, Line 264: total_entries isn't this more like `max_entries`? https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@272 PS11, Line 272: mcode_file = cbfs_get_entry(image, blob_name); NB. while `struct cbfsfile` is used for a deserialized copy above, this seems to point into the serialized image... https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@300 PS11, Line 300: if (total_size < sizeof(*mcu_header)) Also check if it fits within the CBFS file? https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@309 PS11, Line 309: file_length -= mcus[num_mcus].size; Potential overflow (see above) https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@332 PS11, Line 332: zero or multiple or one? The existing comment comments the line directly below it. Your comment here and all the comments in the further added functions below would be better placed above or at the top of the functions. https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@443 PS11, Line 443: entires entries https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@443 PS11, Line 443: ERROR("Maximum of FIT entires reached.\n"); free(mcus); https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@450 PS11, Line 450: mcus[i].offset), NB. Looking through the code this still only works if the CBFS is memory mapped at the top of 4GiB (e.g. not with the usual APL layouts). Despite the annoying abstraction. I assume it wasn't noticed because on APL the CSME creates the effective FIT. https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@459 PS11, Line 459: sort? https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@480 PS11, Line 480: fit_pointer[1] != 0 Any hint that it's a 64-bit pointer? otherwise it wouldn't be specified what is at this location. https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@533 PS11, Line 533: break; `break` is usually indented. https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@571 PS11, Line 571: lx PRIx64 https://review.coreboot.org/#/c/31493/11/util/cbfstool/ifittool.c File util/cbfstool/ifittool.c: https://review.coreboot.org/#/c/31493/11/util/cbfstool/ifittool.c@118 PS11, Line 118: Cover the situation where a negative base address is given by the : * user. This doesn't seem to make sense in the new context. https://review.coreboot.org/#/c/31493/11/util/cbfstool/ifittool.c@123 PS11, Line 123: return region->size - offset; NB. All calls I've seen so far seem to run into this case. -- To view, visit https://review.coreboot.org/c/coreboot/+/31493 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0fe8cd70611d58823aca1147d5b830722ed72bd5 Gerrit-Change-Number: 31493 Gerrit-PatchSet: 11 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Amol N Sukerkar <amol.n.sukerkar(a)intel.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Jens Drenhaus <jens.drenhaus(a)9elements.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kai Michaelis <kai.michaelis(a)9elements.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Roy Wen <rgzwen(a)arista.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Patrick Rudolph <siro(a)das-labor.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Tue, 30 Apr 2019 14:09:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: sdm845: Add interface between CB & QCLib
by mturney mturney (Code Review) 30 Apr '19

30 Apr '19

mturney mturney has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/27360 ) Change subject: sdm845: Add interface between CB & QCLib ...................................................................... Patch Set 35: > Patch Set 35: > > > After looking at how the whole codebase uses CONFIG_CBFS_PREFIX, this request would make sdm845 code different. In fact to fit in with the codebase, the #define for the name, e.g. pmic or aop should be removed... > > Yes, having the string inline is certainly acceptable as long as it's only used once in the code (which should be the case for all here). Feel free to do that if you want, but defining a named constant for it isn't "wrong" either. (But if we do have a constant, I think the CBFS prefix should be part of it.) PMIC_NAME replaced with "/pmiccfg" DCB_NAME replaced with "/dcb" Macro replaced with single instance string which agrees with rest of codebase. -- To view, visit https://review.coreboot.org/c/coreboot/+/27360 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8d86f360d9e5f1a50f052ab99e821a587e091946 Gerrit-Change-Number: 27360 Gerrit-PatchSet: 35 Gerrit-Owner: T.Michael Turney <tturne(a)codeaurora.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: T.Michael Turney <tturne(a)codeaurora.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: mturney mturney <mturney(a)codeaurora.org> Gerrit-Comment-Date: Tue, 30 Apr 2019 13:33:55 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...libhwbase[master]: Makefile.proof: Add --no-inlining to SPARKFLAGS
by Reto Buerki (Code Review) 30 Apr '19

30 Apr '19

Reto Buerki has posted comments on this change. ( https://review.coreboot.org/c/libhwbase/+/26850 ) Change subject: Makefile.proof: Add --no-inlining to SPARKFLAGS ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/libhwbase/+/26850 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: libhwbase Gerrit-Branch: master Gerrit-Change-Id: Ia6055da326901c1c25b9d0c757767891284bc515 Gerrit-Change-Number: 26850 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Adrian-Ken Rueegsegger <ken(a)codelabs.ch> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Reto Buerki <reet(a)codelabs.ch> Gerrit-Comment-Date: Tue, 30 Apr 2019 13:32:15 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: sdm845: Add QCLib to RomStage to perform IP init
by T.Michael Turney (Code Review) 30 Apr '19

30 Apr '19

Hello mturney mturney, Julius Werner, Paul Menzel, build bot (Jenkins), Patrick Georgi, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/25208 to look at the new patch set (#76). Change subject: sdm845: Add QCLib to RomStage to perform IP init ...................................................................... sdm845: Add QCLib to RomStage to perform IP init CB acts as I/O handler for QCLib (e.g. DDR training data) This interface allows bi-directional data flow between CB and QCLib Tested and working interfaces: DDR Training data QCLib serial console output DDR Information (base & size) limits cfg data TEST=build & run Change-Id: I073186674a1a593547d1ee1d15c7cd4fd8ad5bc1 Signed-off-by: T Michael Turney <mturney(a)codeaurora.org> --- M src/mainboard/google/cheza/chromeos.fmd M src/mainboard/google/cheza/romstage.c M src/soc/qualcomm/sdm845/Makefile.inc M src/soc/qualcomm/sdm845/include/soc/memlayout.ld M src/soc/qualcomm/sdm845/include/soc/mmu.h M src/soc/qualcomm/sdm845/include/soc/symbols.h M src/soc/qualcomm/sdm845/mmu.c A src/soc/qualcomm/sdm845/qclib.c M src/soc/qualcomm/sdm845/soc.c 9 files changed, 83 insertions(+), 22 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/25208/76 -- To view, visit https://review.coreboot.org/c/coreboot/+/25208 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I073186674a1a593547d1ee1d15c7cd4fd8ad5bc1 Gerrit-Change-Number: 25208 Gerrit-PatchSet: 76 Gerrit-Owner: T.Michael Turney <tturne(a)codeaurora.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: T.Michael Turney <tturne(a)codeaurora.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: mturney mturney <mturney(a)codeaurora.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newpatchset

1 0

Change in ...coreboot[master]: arch/x86/boot: Call payload in protected mode
by Patrick Rudolph (Code Review) 30 Apr '19

30 Apr '19

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30118 ) Change subject: arch/x86/boot: Call payload in protected mode ...................................................................... Patch Set 14: (1 comment) https://review.coreboot.org/#/c/30118/14/src/arch/x86/c_start.S File src/arch/x86/c_start.S: https://review.coreboot.org/#/c/30118/14/src/arch/x86/c_start.S@a220 PS14, Line 220: assuming this comment is wrong, can I replace the whole SetCodeSelector64 with the new implementation below? -- To view, visit https://review.coreboot.org/c/coreboot/+/30118 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6552ac30f1b6205e08e16d251328e01ce3fbfd14 Gerrit-Change-Number: 30118 Gerrit-PatchSet: 14 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Tue, 30 Apr 2019 11:30:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: arch/x86/boot: Call payload in protected mode
by Krystian Hebel (Code Review) 30 Apr '19

30 Apr '19

Krystian Hebel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30118 ) Change subject: arch/x86/boot: Call payload in protected mode ...................................................................... Patch Set 14: Code-Review+1 (1 comment) https://review.coreboot.org/#/c/30118/8/src/arch/x86/c_start.S File src/arch/x86/c_start.S: https://review.coreboot.org/#/c/30118/8/src/arch/x86/c_start.S@214 PS8, Line 214: SetCodeSelector64: I guess the changes from SetCodeSelector32 could be applied here too, but other than that looks good. -- To view, visit https://review.coreboot.org/c/coreboot/+/30118 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6552ac30f1b6205e08e16d251328e01ce3fbfd14 Gerrit-Change-Number: 30118 Gerrit-PatchSet: 14 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Tue, 30 Apr 2019 11:00:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: mainboard/intel/icelake_rvp: Add support to read board ID from EC
by Maulik V Vaghela (Code Review) 30 Apr '19

30 Apr '19

Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32523 ) Change subject: mainboard/intel/icelake_rvp: Add support to read board ID from EC ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/32523 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0082e04ef1b21d533e40d232209ee630f748aec6 Gerrit-Change-Number: 32523 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Comment-Date: Tue, 30 Apr 2019 10:58:20 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: arch/x86/assembly_entry: Enable long mode on x86_64
by Krystian Hebel (Code Review) 30 Apr '19

30 Apr '19

Krystian Hebel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30114 ) Change subject: arch/x86/assembly_entry: Enable long mode on x86_64 ...................................................................... Patch Set 12: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/30114 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I57974a55f3b778c90b3587f39e86e4eb8692ad48 Gerrit-Change-Number: 30114 Gerrit-PatchSet: 12 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: David Guckian <david.guckian(a)intel.com> Gerrit-Reviewer: Huang Jin <huang.jin(a)intel.com> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: York Yang <yyang024(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-CC: Lijian Zhao <lijian.zhao(a)intel.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Tue, 30 Apr 2019 10:52:43 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: soc/intel/braswell/chip.c: Configure LPSS devices in correct mode
by Frans Hendriks (Code Review) 30 Apr '19

30 Apr '19

Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29284 ) Change subject: soc/intel/braswell/chip.c: Configure LPSS devices in correct mode ...................................................................... Patch Set 9: > Patch Set 9: > > > > > If LPSS (SIO DMA1) pci device 0/30/0 is configured in ACPI mode, PWM, HS2UARTS, and SPI controller needs to be configured in ACPI mode. > > > > If LPSS (SIO DMA2) pci device 0/24/0 is configured in ACPI mode, I2C controller needs to be configured in ACPI mode. > > > > In other words: If function > 1 in PCI mode, function 0 must be PCI mode to be PCI-enumerated. > > > > (See 33.1 of Intel Braswell BIOS Writes Guide) > > > > (See 24.3 of Intel Braswell External Design Guide Volume 2) > > > > > > > > This is NOT a bug in FSP. I expect it's responsibility of coreboot/developer to provide correct parameters API of FSP. > > > > > > IMO it is still a BUG in FSP docs. The FSP docs do not document ACPI mode for DMAs and the dependency for child devices following the parent device setting. How a developer can integrate the binary correctly when the documentation and API have bugs? Not everyone has access to FSP source code, let's keep that in mind. > > > > I (now) realize that not everyone has access to all Intel document/FSP source code. > > > > On the other hand providing a patch and having code-review score of -1, I don't feel much support for supplying for this kit of fixes/update to coreboot. > > A -1 merely states that somebody doesn't like the current > state of a patch, not the patch itself (it should automa- > tically vanish once you push a new version). Your help here > is much appreciated. > > When you write some code that conflicts an API documentation, > that's ok when you know better. But most developers (no matter > if they have access to FSP sources) will probably trust the API > documentation and think that your code is wrong. So please add > a code comment in such cases (or try to get the documentation > fixed). > > But before you continue, I would like to know in detail how you > tested this. The interaction of coreboot and FSP is most fragile, > especially when coreboot and FSP both offer options to configure > the same thing (ACPI mode in this case). I fear, in case FSP > hides the PCI device too early, dev_enable_acpi_mode() in core- > boot might not work correctly anymore (e.g. fail to report the > correct BARs). Thanks for clarification. (My assumption was that having a -1 prevent other reviewers from reviewing) This 'issue' is discovered during bringup on the Intel CherryHill (CRB). It has not influence on actuel Braswell platforms where the LPSS is configured in ACPI mode. This patch will be 'used' when board configuration is incorrect only. We noticed that PCI devices were not visible on the PCI bus, since 'lpss_mode' was set to ACPI. Can not describe details of testing, since PCI/ACPI modes has been corrected in devicetree.cb. If all devicetree.cb setting are correct this fix would not influence behavoir of actual code. So there should not be any fear about other influences. If this influence exist, it exist in actual code already. -- To view, visit https://review.coreboot.org/c/coreboot/+/29284 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie271d8cb9f30f0c0ba538f1530cfb82f1306fea8 Gerrit-Change-Number: 29284 Gerrit-PatchSet: 9 Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Comment-Date: Tue, 30 Apr 2019 10:52:20 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: arch/x86: Support x86_64 exceptions
by Krystian Hebel (Code Review) 30 Apr '19

30 Apr '19

Krystian Hebel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30117 ) Change subject: arch/x86: Support x86_64 exceptions ...................................................................... Patch Set 14: Code-Review+1 (1 comment) https://review.coreboot.org/#/c/30117/13/src/arch/x86/idt.S File src/arch/x86/idt.S: https://review.coreboot.org/#/c/30117/13/src/arch/x86/idt.S@145 PS13, Line 145: lea 64(%rsp), %rbp > I removed this code as RSP is pushed by interrupt on stack. Right, that works too. -- To view, visit https://review.coreboot.org/c/coreboot/+/30117 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idd12c90a95cc2989eb9b2a718740a84222193f48 Gerrit-Change-Number: 30117 Gerrit-PatchSet: 14 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Tue, 30 Apr 2019 10:45:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Krystian Hebel <krystian.hebel(a)3mdeb.com> Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2019