coreboot-gerrit March 2019

coreboot-gerrit@coreboot.org

1 participants
1542 discussions

Change in ...coreboot[master]: Documentation: Explain FMAP and FMD
by Hung-Te Lin (Code Review) 11 Apr '19

11 Apr '19

Hung-Te Lin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31766 Change subject: Documentation: Explain FMAP and FMD ...................................................................... Documentation: Explain FMAP and FMD The Flashmap (FMAP) was not clearly documented. The new flashmap.md explains where to find more details about that and how / why it was used in coreboot. Also explained what is FMD and how to use it. BUG=None TEST=None (only documentation). Change-Id: Ia389e56c632096d7c905ed221fd4f140dec382e6 Signed-off-by: Hung-Te Lin <hungte(a)chromium.org> --- A Documentation/flashmap.md M Documentation/index.md D util/cbfstool/README.fmaptool 3 files changed, 116 insertions(+), 69 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/31766/1 diff --git a/Documentation/flashmap.md b/Documentation/flashmap.md new file mode 100644 index 0000000..c6780b6 --- /dev/null +++ b/Documentation/flashmap.md @@ -0,0 +1,115 @@ +# Flashmap and Flashmap Descriptor in coreboot + +## Flashmap + +[Flashmap](https://code.google.com/p/flashmap) (FMAP) is a binary format +originally developed for Chromium OS to manipulate firmware image and to +represent the layout of flash chips. With Flashmap, the firmware image is split +into multiple `fmap_area` (may also be referred as 'regions', or 'sections'). +Each area has a name, offset, size and 16 bits of flags. + +coreboot puts most data in the CBFS structure. But many systems, especially +Chromium OS, need some area to be created in different format and layout. For +example, Chromium OS has defined a key-value style "Vital Product Data" (VPD) +that cannot live inside CBFS. Intel firmware also usually need a place to put +the firmware for management engine. Some ARM SOCs also need to preserve +calibration data in the fixed location. This is very helpful to manipulate +non-coreboot data. + +As a result, Flashmap is now directly supported by coreboot, and is the +de facto standard FMAP implementation. + +## Flashmap Descriptor + +Since coreboot is starting to use a "partition" of Flashmap to describe the +flash chip layout (both at runtime and when flashing a new image onto a +chip), the project needs a reasonably expressive plain text format for +representing such sections in the source tree. + +Flashmap Descriptor (FMD) is a [language and +compiler](https://chromium-review.googlesource.com/#/c/255031) inside coreboot +utility folder that can be used to generate final firmware images (i.e. +`coreboot.rom`) formatted by Flashmap. + +The FMD in coreboot `utils` folder contains a scanner, parser, semantic +analyzer, and Flashmap converter. Here's an informal language description: + +``` +# <line comment> +<image name>[@<memory-mapped address>] <image size> { + <section name>[(flags)][@<offset from start of image>] [<section size>] [{ + <subsection name>[@<offset from start of parent section>] [<subsection size>] [{ + # Sections can be nested as deeply as desired + <subsubsection name>[(flags)][@...] [...] [{...}] + }] + [<subsection name>[(flags)][@...] [...] [{...}]] + # There can be many subsections at each level of nesting: they will be inserted + # sequentially, and although gaps are allowed, any provided offsets are always + # relative to the closest parent node's and must be strictly increasing with neither + # overlapping nor degenerate-size sections. + }] +} +``` + +Note that the above example contains a few symbols that are actually meta +syntax, and therefore have neither meaning nor place in a real file. The `<.*>`s +indicate placeholders for parameters: + +* The names are strings, which are provided as single-word (no white space) + groups of syntactically unimportant symbols (i.e. every thing except `@`, `{`, + and `}`): they are not surrounded by quotes or any other form of delimiter. +* The other fields are non-negative integers, which may be given as decimal or + hexadecimal; in either case, a `K`, `M`, or `G` may be appended (without + intermediate white space) as a multiplier. +* Comments consist of anything one manages to enter, provided it doesn't start a + new line. + +The `[.*]`s indicate that a portion of the file could be omitted altogether: + +* Just because something is noted as optional doesn't mean it is in every case: + the answer might actually depend on which other information is---or + isn't---provided. +* The "flag" specifies the attribute or type for given section. The most + important supported flag is "CBFS", which indicates the section will contain + a CBFS structure. +* In particular, it is only legal to place a (CBFS) flag on a leaf section; that + is, choosing to add child sections excludes the possibility of putting a CBFS + in their parent. Such flags are only used to decide where CBFS empty file + headers should be created, and do not result in the storage of any additional + metadata in the resulting FMAP section. + +Additionally, it's important to note these properties of the overall file and +its values: + +* Other than within would-be strings and numbers, white space is ignored. It + goes without saying that such power comes with responsibility, which is why + this sentence is here. +* Although the `section name` must be globally unique, one of them may (but is + not required to) match the image name. +* It is a syntax error to supply a number (besides 0) that begins with the + character `0`, as there is no intention of adding octals to the mix. +* The image's memory address should be present on (and only on) layouts for + memory-mapped chips. +* Although it may be evident from above, all `section` offsets are relative only + to the immediate parent. There is no way to include an absolute offset (i.e. + from the beginning of flash), which means that it is "safe" to reorder the + sections within a particular level of nesting, as long as the change doesn't + cause their positions and sizes to necessitate overlap or zero sizes. +* A `section` with omitted offset is assumed to start at as low a position as + possible (with no consideration of alignment) and one with omitted size is + assumed to fill the remaining space until the next sibling or before the end + of its parent. +* It's fine to omit any `section`'s offset, size, or both, provided its position + and size are still unambiguous in the context of its *sibling* sections and + its parent's *size*. In particular, knowledge of one .*section 's children or + the `section`s' common parent's siblings will not be used for this purpose. +* Although `section`s are not required to have children, the flash chip as a + whole must have at least one. +* Though the braces after `section`s may be omitted for those that have no + children, if they are present, they must contain at least one child. + +PL people and sympathizers may wish to examine the formal abstract syntax and +context-free grammar, which are located in `fmd_scanner.l` and `fmd_scanner.y`, +respectively. Those interested in the algorithm used to infer omitted values +will feel at home in `fmd.c`, particularly near the definition of +`validate_and_complete_info()`. diff --git a/Documentation/index.md b/Documentation/index.md index dd8714c..9398458 100644 --- a/Documentation/index.md +++ b/Documentation/index.md @@ -186,3 +186,4 @@ * [Utilities](util.md) * [Release notes for past releases](releases/index.md) * [Flashing firmware tutorial](flash_tutorial/index.md) +* [Flashmap and Flashmap Descriptor](flashmap.md) diff --git a/util/cbfstool/README.fmaptool b/util/cbfstool/README.fmaptool deleted file mode 100644 index 86fc3b2..0000000 --- a/util/cbfstool/README.fmaptool +++ /dev/null @@ -1,69 +0,0 @@ -Flashmap descriptors in coreboot -================================ -Flashmap (https://code.google.com/p/flashmap) is a binary format for representing the layout of -flash chips. Since coreboot is starting to use a "partition" of this format to describe the flash -chip layout---both at runtime and when flashing a new image onto a chip---, the project needed a -reasonably expressive plaintext format for representing such sections in the source tree. Our -solution is the fmd ("flashmap descriptor") language, and the files in this directory contain a -scanner, parser, semantic analyser, and flashmap converter. Here's an informal language description: - -# <line comment> -<image name>[@<memory-mapped address>] <image size> { - <section name>[(flags)][@<offset from start of image>] [<section size>] [{ - <subsection name>[@<offset from start of parent section>] [<subsection size>] [{ - # Sections can be nested as deeply as desired - <subsubsection name>[(flags)][@...] [...] [{...}] - }] - [<subsection name>[(flags)][@...] [...] [{...}]] - # There can be many subsections at each level of nesting: they will be inserted - # sequentially, and although gaps are allowed, any provided offsets are always - # relative to the closest parent node's and must be strictly increasing with neither - # overlapping nor degenerate-size sections. - }] -} - -Note that the above example contains a few symbols that are actually metasyntax, and therefore have -neither meaning nor place in a real file. The <.*> s indicate placeholders for parameters: - - The names are strings, which are provided as single-word---no whitespace---groups of - syntactically unimportant symbols (i.e. everything except @, {, and }): they are not surrounded - by quotes or any other form of delimiter. - - The other fields are nonnegative integers, which may be given as decimal or hexadecimal; in - either case, a K, M, or G may be appended---without intermediate whitespace---as a multiplier. - - Comments consist of anything one manages to enter, provided it doesn't start a new line. -The [.*] s indicate that a portion of the file could be omitted altogether: - - Just because something is noted as optional doesn't mean it is in every case: the answer might - actually depend on which other information is---or isn't---provided. - - The "flag" specifies the attribute or type for given section. The most - important supported flag is "CBFS", which indicates the section will contain a CBFS structure. - - In particular, it is only legal to place a (CBFS) flag on a leaf section; that is, choosing - to add child sections excludes the possibility of putting a CBFS in their parent. Such - flags are only used to decide where CBFS empty file headers should be created, and do not - result in the storage of any additional metadata in the resulting FMAP section. -Additionally, it's important to note these properties of the overall file and its values: - - Other than within would-be strings and numbers, whitespace is ignored. It goes without saying - that such power comes with responsibility, which is why this sentence is here. - - Although the .*section names must be globally unique, one of them may---but is not required to--- - match the image name. - - It is a syntax error to supply a number---besides 0---that begins with the character 0, as there - is no intention of adding octals to the mix. - - The image's memory address should be present on---and only on---layouts for memory-mapped chips. - - Although it may be evident from above, all .*section offsets are relative only to the immediate - parent. There is no way to include an absolute offset (i.e. from the beginning of flash), which - means that it is "safe" to reorder the .*section s within a particular level of nesting, as long - as the change doesn't cause their positions and sizes to necessitate overlap or zero sizes. - - A .*section with omitted offset is assumed to start at as low a position as possible---with no - consideration of alignment---and one with omitted size is assumed to fill the remaining space - until the next sibling or before the end of its parent. - - It's fine to omit any .*section 's offset, size, or both, provided its position and size are - still unambiguous in the context of its *sibling* sections and its parent's *size*. In - particular, knowledge of one .*section 's children or the .*section s' common parent's siblings - will not be used for this purpose. - - Although .*section s are not required to have children, the flash chip as a whole must have at - least one. - - Though the braces after .*section s may be omitted for those that have no children, if they are - present, they must contain at least one child. - -PL people and sympathizers may wish to examine the formal abstract syntax and context-free grammar, -which are located in fmd_scanner.l and fmd_scanner.y, respectively. Those interested in the -algorithm used to infer omitted values will feel at home in fmd.c, particularly near the definition -of validate_and_complete_info(). -- To view, visit https://review.coreboot.org/c/coreboot/+/31766 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia389e56c632096d7c905ed221fd4f140dec382e6 Gerrit-Change-Number: 31766 Gerrit-PatchSet: 1 Gerrit-Owner: Hung-Te Lin <hungte(a)chromium.org> Gerrit-MessageType: newchange

6 28

Change in ...coreboot[master]: google/kukui: Use internal CR50_IRQ pull-up
by You-Cheng Syu (Code Review) 11 Apr '19

11 Apr '19

You-Cheng Syu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32116 Change subject: google/kukui: Use internal CR50_IRQ pull-up ...................................................................... google/kukui: Use internal CR50_IRQ pull-up For Kukui CR50_IRQ pin, we're going to replace external pull-up with internal pull-up. This change won't break older boards, so we can just always do that when setting up GPIOs. BUG=b:124821269 BRANCH=none TEST=Waveform looks correct. Change-Id: Ib1a90dce583a6aa0cec8ac8ba96d1362f50c16a8 Signed-off-by: You-Cheng Syu <youcheng(a)google.com> --- M src/mainboard/google/kukui/chromeos.c 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/32116/1 diff --git a/src/mainboard/google/kukui/chromeos.c b/src/mainboard/google/kukui/chromeos.c index 132f0d3..09c47ee 100644 --- a/src/mainboard/google/kukui/chromeos.c +++ b/src/mainboard/google/kukui/chromeos.c @@ -24,6 +24,7 @@ { gpio_input_pullup(EC_IN_RW); gpio_input_pullup(EC_IRQ); + gpio_input_pullup(CR50_IRQ); gpio_output(GPIO_RESET, 0); } -- To view, visit https://review.coreboot.org/c/coreboot/+/32116 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib1a90dce583a6aa0cec8ac8ba96d1362f50c16a8 Gerrit-Change-Number: 32116 Gerrit-PatchSet: 1 Gerrit-Owner: You-Cheng Syu <youcheng(a)google.com> Gerrit-MessageType: newchange

3 2

Change in ...coreboot[master]: nb/amd/amdfam10/util.c: Use "CONFIG" only when appropriate
by HAOUAS Elyes (Code Review) 11 Apr '19

11 Apr '19

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31848 Change subject: nb/amd/amdfam10/util.c: Use "CONFIG" only when appropriate ...................................................................... nb/amd/amdfam10/util.c: Use "CONFIG" only when appropriate Change-Id: Idcdbbfa883c906db1ebb8d9bc7c9e277e7c0c949 Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr> --- M src/northbridge/amd/amdfam10/util.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/31848/1 diff --git a/src/northbridge/amd/amdfam10/util.c b/src/northbridge/amd/amdfam10/util.c index 41f4deb..9941aec 100644 --- a/src/northbridge/amd/amdfam10/util.c +++ b/src/northbridge/amd/amdfam10/util.c @@ -135,7 +135,7 @@ static void showconfig(int level, u8 which, u32 reg) { /* Don't use r_node() and r_link() here. */ - printk(level, "CONFIG(%02x)%02x-%02x ->(%d,%d),%s %s (%s numbers)\n", + printk(level, "Config(%02x)%02x-%02x ->(%d,%d),%s %s (%s numbers)\n", which, BITS(reg, 16, 0xff), BITS(reg, 24, 0xff), BITS(reg, 4, 0x7), BITS(reg, 8, 0x3), re(reg), we(reg), -- To view, visit https://review.coreboot.org/c/coreboot/+/31848 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idcdbbfa883c906db1ebb8d9bc7c9e277e7c0c949 Gerrit-Change-Number: 31848 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-MessageType: newchange

3 3

Change in ...coreboot[master]: Documentation: Add coreboot architecture
by Patrick Rudolph (Code Review) 11 Apr '19

11 Apr '19

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32123 Change subject: Documentation: Add coreboot architecture ...................................................................... Documentation: Add coreboot architecture Describe the coreboot stages, given a short introduction what is done and add a chart for coreboot's vs EDK II bootflow. TODO: Describe stages and payloads in detail Change-Id: I98cb61b1d0d29ac9d03f5ef3644d51a8e14bad74 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- A Documentation/getting_started/architecture.md A Documentation/getting_started/comparision_coreboot_uefi.svg M Documentation/getting_started/index.md 3 files changed, 287 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/32123/1 diff --git a/Documentation/getting_started/architecture.md b/Documentation/getting_started/architecture.md new file mode 100644 index 0000000..f8f8b3d --- /dev/null +++ b/Documentation/getting_started/architecture.md @@ -0,0 +1,110 @@ +# coreboot architecture + +## Overwiew +![][architecture] + +[architecture]: comparision_coreboot_uefi.svg + +## Stages +The stages are compiled as seperate binaries and are inserted into the CBFS with +custom compression. The bootblock usually doesn't have compression, while the +ramstage and payload are compressed with LZMA. + +Each stage loads the next stage at given address (possibly decompressing it). + +Some stages are relocatable and can be placed anywhere in DRAM. Those stages are +usually cached in CBMEM for faster loading times on ACPI S3 resume. + +## bootblock +The bootblock is the first stage executed after CPU reset. It is written in +assembly language and its main task is to setup everything for a C-environment: + +Common tasks: + +* Cache-As-RAM for heap and stack +* Set stack pointer +* Clear memory for BSS +* Decompress and load the next stage + +On x86 platforms that includes: + +* Microcode updates +* Timer init +* Switching from 16-bit real-mode to 32-bit protected mode + +The bootblock loads the romstage or the verstage if verified boot is enabled. + +### Cache-As-Ram +The *Cache-As-Ram*, also called Non-Eviction mode, or *CAR* allows to use the +CPU cache like regular SRAM. This is particullary usefull for high level +languages like `C`, which need RAM for heap and stack. + +The CAR needs to be actived using vendor specific CPU instructions. + +The following stages run when Cache-As-Ram is active: +* bootblock +* romstage +* verstage +* postcar + +## verstage +The verstage is where the root-of-trust starts. It's assumed that +it cannot be overwritten in-field (together with the public key) and +that starts at the very beginning of the boot process. +The verstage installs a hook to verify a file before it's loaded from +CBFS or a partition before it's accessed. + +The verified boot mechanism allows trusted in-field firmware updates +combined with a fail-safe recovery mode. + +## romstage +The romstage initialized the DRAM and prepares everything for device init. + +Common tasks: + +* DRAM init +* Early device init + +## postcar +To leave the CAR setup and run code from regular DRAM the postcar-stage tears +down CAR and loads the ramstage. Compared to other stages it's minimal in size. + +## ramstage + +The ramstage does the main device init: + +* PCI device init +* On-chip device init +* TPM init +* Graphics init (optional) +* CPU init (like set up SMM) + +After initialization tables are written to inform the payload about the current +hardware existance and state. That includes: + +* ACPI tables (x86 specific) +* SMBIOS tables (x86 specific) +* coreboot tables +* devicetree updates (ARM specific) + +It also does hardware and firmware lockdown: +* Write-protection of boot media +* Lock security related registers +* Lock SMM mode (x86 specific) + +## payload +The payload is the software that is run after coreboot is done. It resides in +the CBFS and there's no possibility to choose it at runtime. +Some payloads, like *SeaBios*, offer to load `secondary payloads` at runtime. + +Supported payloads are: + +* [SeaBIOS](https://www.seabios.org/SeaBIOS) +* [Tianocore](https://www.tianocore.org/) +* [GRUB](https://www.gnu.org/software/grub/) +* [Depthcharge](https://chromium.googlesource.com/chromiumos/platform/depthcha… +* ELF binaries +* [UBoot](https://www.denx.de/wiki/U-Boot) +* [Linux kernel](https://www.kernel.org/) +* [LinuxBoot](https://www.linuxboot.org/) + diff --git a/Documentation/getting_started/comparision_coreboot_uefi.svg b/Documentation/getting_started/comparision_coreboot_uefi.svg new file mode 100644 index 0000000..3ea2fa6 --- /dev/null +++ b/Documentation/getting_started/comparision_coreboot_uefi.svg @@ -0,0 +1,176 @@ +<?xml version="1.0" encoding="UTF-8" standalone="no"?> +<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.0//EN" "http://www.w3.org/TR/2001/PR-SVG-20010719/DTD/svg10.dtd"> +<svg width="55cm" height="28cm" viewBox="62 37 1088 559" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"> + <g> + <rect style="fill: #ffffff" x="63.296" y="74.0258" width="1085.8" height="520.893"/> + <rect style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #ffffff" x="63.296" y="74.0258" width="1085.8" height="520.893"/> + </g> + <line style="fill: none; fill-opacity:0; stroke-width: 4; stroke: #000000" x1="242.613" y1="107.463" x2="242.698" y2="492.591"/> + <g> + <line style="fill: none; fill-opacity:0; stroke-width: 4; stroke: #000000" x1="234.964" y1="477.053" x2="1135.15" y2="478.109"/> + <polyline style="fill: none; fill-opacity:0; stroke-width: 4; stroke: #000000" points="1124.61,485.597 1139.62,478.114 1124.63,470.597 "/> + </g> + <text font-size="22.5778" style="fill: #000000;text-anchor:start;font-family:sans-serif;font-style:normal;font-weight:normal" x="482.342" y="58.1574"> + <tspan x="482.342" y="58.1574">Platform Initialization Firmware Phases</tspan> + </text> + <text font-size="16.9333" style="fill: #000000;text-anchor:start;font-family:sans-serif;font-style:normal;font-weight:normal" x="98.4514" y="435.714"> + <tspan x="98.4514" y="435.714">EDK II - stages</tspan> + </text> + <text font-size="12.8" style="fill: #000000;text-anchor:start;font-family:sans-serif;font-style:normal;font-weight:normal" x="1073.49" y="499.998"> + <tspan x="1073.49" y="499.998">time</tspan> + </text> + <text font-size="16.9333" style="fill: #000000;text-anchor:start;font-family:sans-serif;font-style:normal;font-weight:normal" x="82.8266" y="330.476"> + <tspan x="82.8266" y="330.476">coreboot - stages</tspan> + </text> + <g> + <rect style="fill: #faff94" x="250.501" y="404.247" width="130.432" height="69.1471"/> + <rect style="fill: none; fill-opacity:0; stroke-width: 1; stroke: #000000" x="250.501" y="404.247" width="130.432" height="69.1471"/> + <text font-size="12.8" style="fill: #000000;text-anchor:middle;font-family:sans-serif;font-style:normal;font-weight:700" x="315.718" y="434.72"> + <tspan x="315.718" y="434.72">Security</tspan> + <tspan x="315.718" y="450.72">(SEC)</tspan> + </text> + </g> + <g> + <rect style="fill: #faff94" x="383.033" y="404.781" width="282.702" height="69"/> + <rect style="fill: none; fill-opacity:0; stroke-width: 1; stroke: #000000" x="383.033" y="404.781" width="282.702" height="69"/> + <text font-size="12.8" style="fill: #000000;text-anchor:middle;font-family:sans-serif;font-style:normal;font-weight:700" x="524.384" y="427.181"> + <tspan x="524.384" y="427.181">Pre-EFI</tspan> + <tspan x="524.384" y="443.181">Initialization Environment</tspan> + <tspan x="524.384" y="459.181">(PEI)</tspan> + </text> + </g> + <g> + <rect style="fill: #faff94" x="668.027" y="405.317" width="269.244" height="69"/> + <rect style="fill: none; fill-opacity:0; stroke-width: 1; stroke: #000000" x="668.027" y="405.317" width="269.244" height="69"/> + <text font-size="12.8" style="fill: #000000;text-anchor:middle;font-family:sans-serif;font-style:normal;font-weight:700" x="802.649" y="427.717"> + <tspan x="802.649" y="427.717">Driver Execution</tspan> + <tspan x="802.649" y="443.717">Environment</tspan> + <tspan x="802.649" y="459.717">(DXE)</tspan> + </text> + </g> + <g> + <rect style="fill: #faff94" x="939.541" y="405.727" width="178.75" height="69"/> + <rect style="fill: none; fill-opacity:0; stroke-width: 1; stroke: #000000" x="939.541" y="405.727" width="178.75" height="69"/> + <text font-size="12.8" style="fill: #000000;text-anchor:middle;font-family:sans-serif;font-style:normal;font-weight:700" x="1028.92" y="436.127"> + <tspan x="1028.92" y="436.127">Boot Device Selection</tspan> + <tspan x="1028.92" y="452.127">(BDS)</tspan> + </text> + </g> + <g> + <rect style="fill: #90c9ff" x="254.747" y="291.309" width="125.314" height="69.1471"/> + <rect style="fill: none; fill-opacity:0; stroke-width: 1; stroke: #000000" x="254.747" y="291.309" width="125.314" height="69.1471"/> + <text font-size="12.8" style="fill: #000000;text-anchor:middle;font-family:sans-serif;font-style:normal;font-weight:700" x="317.404" y="329.782"> + <tspan x="317.404" y="329.782">bootblock</tspan> + </text> + </g> + <g> + <rect style="fill: #90c9ff" x="476.354" y="290.735" width="89.65" height="69.1471"/> + <rect style="fill: none; fill-opacity:0; stroke-width: 1; stroke: #000000" x="476.354" y="290.735" width="89.65" height="69.1471"/> + <text font-size="12.8" style="fill: #000000;text-anchor:middle;font-family:sans-serif;font-style:normal;font-weight:700" x="521.179" y="329.209"> + <tspan x="521.179" y="329.209">romstage</tspan> + </text> + </g> + <g> + <rect style="fill: #90c9ff" x="382.317" y="291.011" width="92.1" height="69.1471"/> + <rect style="fill: none; fill-opacity:0; stroke-width: 1; stroke: #000000" x="382.317" y="291.011" width="92.1" height="69.1471"/> + <text font-size="12.8" style="fill: #000000;text-anchor:middle;font-family:sans-serif;font-style:normal;font-weight:700" x="428.367" y="321.485"> + <tspan x="428.367" y="321.485">verstage</tspan> + <tspan x="428.367" y="337.485">(optional)</tspan> + </text> + </g> + <g> + <rect style="fill: #90c9ff" x="567.853" y="290.99" width="98.5152" height="69.1471"/> + <rect style="fill: none; fill-opacity:0; stroke-width: 1; stroke: #000000" x="567.853" y="290.99" width="98.5152" height="69.1471"/> + <text font-size="12.8" style="fill: #000000;text-anchor:middle;font-family:sans-serif;font-style:normal;font-weight:700" x="617.11" y="321.464"> + <tspan x="617.11" y="321.464">postcar</tspan> + <tspan x="617.11" y="337.464">(x86 only)</tspan> + </text> + </g> + <g> + <rect style="fill: #90c9ff" x="667.529" y="281.527" width="168.747" height="37"/> + <rect style="fill: none; fill-opacity:0; stroke-width: 1; stroke: #000000" x="667.529" y="281.527" width="168.747" height="37"/> + <text font-size="12.8" style="fill: #000000;text-anchor:middle;font-family:sans-serif;font-style:normal;font-weight:700" x="751.903" y="303.927"> + <tspan x="751.903" y="303.927">ramstage</tspan> + </text> + </g> + <g> + <rect style="fill: #90c9ff" x="667.84" y="321.487" width="167.519" height="53"/> + <rect style="fill: none; fill-opacity:0; stroke-width: 1; stroke: #000000" x="667.84" y="321.487" width="167.519" height="53"/> + <text font-size="12.8" style="fill: #000000;text-anchor:middle;font-family:sans-serif;font-style:normal;font-weight:700" x="751.6" y="343.887"> + <tspan x="751.6" y="343.887">SMM</tspan> + <tspan x="751.6" y="359.887">(x86 only)</tspan> + </text> + </g> + <g> + <rect style="fill: #90c9ff" x="941.841" y="283.151" width="171.98" height="69.1471"/> + <rect style="fill: none; fill-opacity:0; stroke-width: 1; stroke: #000000" x="941.841" y="283.151" width="171.98" height="69.1471"/> + <text font-size="12.8" style="fill: #000000;text-anchor:middle;font-family:sans-serif;font-style:normal;font-weight:700" x="1027.83" y="321.624"> + <tspan x="1027.83" y="321.624">payload</tspan> + </text> + </g> + <g> + <rect style="fill: #d8e5e5" x="253.112" y="209.178" width="82.7" height="27"/> + <rect style="fill: none; fill-opacity:0; stroke-width: 1; stroke: #000000" x="253.112" y="209.178" width="82.7" height="27"/> + <text font-size="12.8" style="fill: #000000;text-anchor:middle;font-family:sans-serif;font-style:normal;font-weight:700" x="294.462" y="226.578"> + <tspan x="294.462" y="226.578">Assembly</tspan> + </text> + </g> + <g> + <rect style="fill: #00c800" x="318.155" y="129.267" width="283.43" height="27"/> + <rect style="fill: none; fill-opacity:0; stroke-width: 1; stroke: #000000" x="318.155" y="129.267" width="283.43" height="27"/> + <text font-size="12.8" style="fill: #000000;text-anchor:middle;font-family:sans-serif;font-style:normal;font-weight:700" x="459.87" y="146.667"> + <tspan x="459.87" y="146.667">Cache-As-RAM</tspan> + </text> + </g> + <g> + <rect style="fill: #ff8484" x="506.676" y="159.67" width="599.421" height="27"/> + <rect style="fill: none; fill-opacity:0; stroke-width: 1; stroke: #000000" x="506.676" y="159.67" width="599.421" height="27"/> + <text font-size="12.8" style="fill: #000000;text-anchor:middle;font-family:sans-serif;font-style:normal;font-weight:700" x="806.387" y="177.07"> + <tspan x="806.387" y="177.07">DRAM</tspan> + </text> + </g> + <line style="fill: none; fill-opacity:0; stroke-width: 1; stroke-dasharray: 4; stroke: #000000" x1="175.046" y1="392.926" x2="1113.82" y2="391.893"/> + <text font-size="12.7998" style="fill: #000000;text-anchor:start;font-family:sans-serif;font-style:normal;font-weight:normal" x="387.045" y="241.637"> + <tspan x="387.045" y="241.637"></tspan> + </text> + <g> + <rect style="fill: #ffffff" x="337.438" y="209.383" width="618.831" height="27"/> + <rect style="fill: none; fill-opacity:0; stroke-width: 1; stroke: #000000" x="337.438" y="209.383" width="618.831" height="27"/> + <text font-size="12.8" style="fill: #000000;text-anchor:middle;font-family:sans-serif;font-style:normal;font-weight:700" x="646.853" y="226.783"> + <tspan x="646.853" y="226.783">C</tspan> + </text> + </g> + <g> + <rect style="fill: #f6c7c7" x="667.35" y="238.912" width="170.3" height="27"/> + <rect style="fill: none; fill-opacity:0; stroke-width: 1; stroke: #000000" x="667.35" y="238.912" width="170.3" height="27"/> + <text font-size="12.8" style="fill: #000000;text-anchor:middle;font-family:sans-serif;font-style:normal;font-weight:700" x="752.5" y="256.312"> + <tspan x="752.5" y="256.312">ADA SPARK (x86 only)</tspan> + </text> + </g> + <text font-size="16.9333" style="fill: #000000;text-anchor:start;font-family:sans-serif;font-style:normal;font-weight:normal" x="84.2481" y="233.28"> + <tspan x="84.2481" y="233.28">coreboot</tspan> + <tspan x="84.2481" y="254.446">source languages</tspan> + </text> + <text font-size="16.9333" style="fill: #000000;text-anchor:start;font-family:sans-serif;font-style:normal;font-weight:normal" x="86.5008" y="153.786"> + <tspan x="86.5008" y="153.786">code/heap</tspan> + <tspan x="86.5008" y="174.953">memory location </tspan> + </text> + <line style="fill: none; fill-opacity:0; stroke-width: 1; stroke-dasharray: 4; stroke: #000000" x1="175.483" y1="273.35" x2="1109.07" y2="273.582"/> + <line style="fill: none; fill-opacity:0; stroke-width: 1; stroke-dasharray: 4; stroke: #000000" x1="176.24" y1="192.463" x2="1109.66" y2="192.132"/> + <g> + <rect style="fill: #90c9ff" x="838.583" y="281.963" width="100.3" height="53"/> + <rect style="fill: none; fill-opacity:0; stroke-width: 1; stroke: #000000" x="838.583" y="281.963" width="100.3" height="53"/> + <text font-size="12.8" style="fill: #000000;text-anchor:middle;font-family:sans-serif;font-style:normal;font-weight:700" x="888.733" y="304.363"> + <tspan x="888.733" y="304.363">BL31</tspan> + <tspan x="888.733" y="320.363">(ARM only)</tspan> + </text> + </g> + <text font-size="12.7998" style="fill: #000000;text-anchor:start;font-family:sans-serif;font-style:normal;font-weight:normal" x="209.772" y="508.772"> + <tspan x="209.772" y="508.772">Power on</tspan> + </text> + <g> + <rect style="fill: #ffffff" x="941.939" y="210.26" width="22.4641" height="25.1384"/> + <rect style="fill: none; fill-opacity:0; stroke-width: 1; stroke: #ffffff" x="941.939" y="210.26" width="22.4641" height="25.1384"/> + </g> + <path style="fill: none; fill-opacity:0; stroke-width: 1; stroke: #000000" d="M 955.029 209.941 C 967.678,210.1 946.349,230.772 955.598,237.021"/> +</svg> diff --git a/Documentation/getting_started/index.md b/Documentation/getting_started/index.md index 8f2a58e..52d873e 100644 --- a/Documentation/getting_started/index.md +++ b/Documentation/getting_started/index.md @@ -1,5 +1,6 @@ # Getting Started +* [coreboot architecture](architecture.md) * [Build System](build_system.md) * [Submodules](submodules.md) * [Kconfig](kconfig.md) -- To view, visit https://review.coreboot.org/c/coreboot/+/32123 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I98cb61b1d0d29ac9d03f5ef3644d51a8e14bad74 Gerrit-Change-Number: 32123 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newchange

5 12

Change in ...coreboot[master]: mb/google/hatch: Add overridetree to hatch variant
by Shelley Chen (Code Review) 11 Apr '19

11 Apr '19

Shelley Chen has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32003 Change subject: mb/google/hatch: Add overridetree to hatch variant ...................................................................... mb/google/hatch: Add overridetree to hatch variant Mimic the serialio settings from cml_u variant of coffeelake rvp BUG=b:128347800 BRANCH=None TEST=abuild Change-Id: I6a9ec778d74cd48a2e1c79f8e669a9a6a6a9477d Signed-off-by: Shelley Chen <shchen(a)google.com> --- M src/mainboard/google/hatch/Kconfig A src/mainboard/google/hatch/variants/hatch/overridetree.cb 2 files changed, 22 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/32003/1 diff --git a/src/mainboard/google/hatch/Kconfig b/src/mainboard/google/hatch/Kconfig index 629b125..d88c795 100644 --- a/src/mainboard/google/hatch/Kconfig +++ b/src/mainboard/google/hatch/Kconfig @@ -38,6 +38,10 @@ string default "variants/baseboard/devicetree.cb" +config OVERRIDE_DEVICETREE + string + default "variants/$(CONFIG_VARIANT_DIR)/overridetree.cb" if !BOARD_GOOGLE_HATCH_WHL + config DIMM_MAX int default 2 diff --git a/src/mainboard/google/hatch/variants/hatch/overridetree.cb b/src/mainboard/google/hatch/variants/hatch/overridetree.cb new file mode 100644 index 0000000..62da635 --- /dev/null +++ b/src/mainboard/google/hatch/variants/hatch/overridetree.cb @@ -0,0 +1,18 @@ +chip soc/intel/cannonlake + + register "SerialIoDevMode" = "{ + [PchSerialIoIndexI2C0] = PchSerialIoPci, + [PchSerialIoIndexI2C1] = PchSerialIoPci, + [PchSerialIoIndexI2C2] = PchSerialIoPci, + [PchSerialIoIndexI2C3] = PchSerialIoPci, + [PchSerialIoIndexI2C4] = PchSerialIoDisabled, + [PchSerialIoIndexI2C5] = PchSerialIoPci, + [PchSerialIoIndexSPI0] = PchSerialIoPci, + [PchSerialIoIndexSPI1] = PchSerialIoPci, + [PchSerialIoIndexSPI2] = PchSerialIoDisabled, + [PchSerialIoIndexUART0] = PchSerialIoSkipInit, + [PchSerialIoIndexUART1] = PchSerialIoDisabled, + [PchSerialIoIndexUART2] = PchSerialIoSkipInit, + }" + +end \ No newline at end of file -- To view, visit https://review.coreboot.org/c/coreboot/+/32003 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6a9ec778d74cd48a2e1c79f8e669a9a6a6a9477d Gerrit-Change-Number: 32003 Gerrit-PatchSet: 1 Gerrit-Owner: Shelley Chen <shchen(a)google.com> Gerrit-MessageType: newchange

4 9

Change in ...coreboot[master]: Doc/mb/asrock/h110m: update info about PEG
by Maxim Polyakov (Code Review) 10 Apr '19

10 Apr '19

Maxim Polyakov has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31949 Change subject: Doc/mb/asrock/h110m: update info about PEG ...................................................................... Doc/mb/asrock/h110m: update info about PEG - Now there is no need to additionally configure the FSP before building; - PEG works with high link speed 8 GT/s (Gen 3); - external GPU supported, but dynamic switching between iGPU and PEG is not yet supported. Change-Id: Ie0f9db47c0b88052b090cba139f0ae821758935d Signed-off-by: Maxim Polyakov <max.senia.poliak(a)gmail.com> --- M Documentation/mainboard/asrock/h110m-dvs.md 1 file changed, 6 insertions(+), 21 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/31949/1 diff --git a/Documentation/mainboard/asrock/h110m-dvs.md b/Documentation/mainboard/asrock/h110m-dvs.md index 7bc38ff..c9be798 100644 --- a/Documentation/mainboard/asrock/h110m-dvs.md +++ b/Documentation/mainboard/asrock/h110m-dvs.md @@ -23,22 +23,10 @@ Please take FSP from the directory `3rdparty/fsp/KabylakeFspBinPkg/` in the coreboot or download the latest version from [github][FSP github]. - -You must use [Intel Binary Configuration Tool] BCT to set the following -parameters in FSP.fd to initialize the PEG x16 port: - -```eval_rst - Peg0Enable = Enable - Peg0MaxLinkSpeed = Gen3 - Peg0MaxLinkWidth = Auto -``` - -BCT creates Fsp_M.fd, Fsp_S.fd and Fsp_T.fd. These files are integrated -into the coreboot image. If PEG port is not used, you can get these files -without BTC: +You must prepare the FSP for integration into the coreboot image: ```bash -# split FSP.fd +# split FSP.fd: Fsp_M.fd and Fsp_S.fd should be used in building the image python 3rdparty/fsp/Tools/SplitFspBin.py split -f 3rdparty/fsp/KabylakeFspBinPkg/Fsp.fd ``` @@ -97,10 +85,9 @@ ## Known issues -- The VGA port doesn't work. - -- PEG x16 port training correctly runs only at link speed of 2.5GT/s(gen1). - It takes more time to research the schematic of this board. +- The VGA port doesn't work. Discrete graphic card is used as primary + device for display output (if CONFIG_ONBOARD_VGA_IS_PRIMARY is not + set). Dynamic switching between iGPU and PEG is not yet supported. - SuperIO GPIO pin is used to reset Realtek chip. However, since the Logical Device 7 (GPIO6, GPIO7, GPIO8) is not initialized, the network @@ -121,7 +108,7 @@ - integrated graphics init with libgfxinit (see [Known issues](#known-issues)) - PCIe x1 -- PEG x16 Gen1 (see [Known issues](#known-issues)) +- PEG x16 Gen3 - SATA - USB - serial port @@ -131,7 +118,6 @@ ## TODO -- PEG x16 Gen3 - NCT6791D GPIOs - onboard network (see [Known issues](#known-issues)) - S3 suspend/resume @@ -156,7 +142,6 @@ [ASRock H110M-DVS]: https://www.asrock.com/mb/Intel/H110M-DVS%20R2.0/ [FSP github]: https://github.com/IntelFsp/FSP -[Intel Binary Configuration Tool]: https://github.com/IntelFsp/BCT [MX25L6473E]: http://www.macronix.com/Lists/Datasheet/Attachments/7380/MX25L6473E,%203V,%… [flashrom]: https://flashrom.org/Flashrom [H110M-DVS manual]: http://asrock.pc.cdn.bitgravity.com/Manual/H110M-DVS%20R2.0.pdf -- To view, visit https://review.coreboot.org/c/coreboot/+/31949 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie0f9db47c0b88052b090cba139f0ae821758935d Gerrit-Change-Number: 31949 Gerrit-PatchSet: 1 Gerrit-Owner: Maxim Polyakov <max.senia.poliak(a)gmail.com> Gerrit-MessageType: newchange

4 14

Change in ...coreboot[master]: soc/skl: Use devtree options to set primary GPU
by Maxim Polyakov (Code Review) 10 Apr '19

10 Apr '19

Maxim Polyakov has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32044 Change subject: soc/skl: Use devtree options to set primary GPU ...................................................................... soc/skl: Use devtree options to set primary GPU List of options: - InternalGfx, - PrimaryDisplay, - SkipExtGfxScan. Change-Id: Ie3f9362676105e41c69139a094dbb9e8b865689f Signed-off-by: Maxim Polyakov <max.senia.poliak(a)gmail.com> --- M src/soc/intel/skylake/romstage/romstage_fsp20.c 1 file changed, 10 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/32044/1 diff --git a/src/soc/intel/skylake/romstage/romstage_fsp20.c b/src/soc/intel/skylake/romstage/romstage_fsp20.c index 43ba9c9..f38a775 100644 --- a/src/soc/intel/skylake/romstage/romstage_fsp20.c +++ b/src/soc/intel/skylake/romstage/romstage_fsp20.c @@ -274,6 +274,16 @@ /* Enable SMBus controller based on config */ m_cfg->SmbusEnable = config->SmbusEnable; + /* Set primary graphic device */ + if (CONFIG(ONBOARD_VGA_IS_PRIMARY)) { + m_cfg->PrimaryDisplay = 0; + m_cfg->InternalGfx = 1; + } else { + m_cfg->PrimaryDisplay = config->PrimaryDisplay; + m_cfg->InternalGfx = config->InternalGfx; + } + m_t_cfg->SkipExtGfxScan = config->SkipExtGfxScan; + mainboard_memory_init_params(mupd); } -- To view, visit https://review.coreboot.org/c/coreboot/+/32044 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie3f9362676105e41c69139a094dbb9e8b865689f Gerrit-Change-Number: 32044 Gerrit-PatchSet: 1 Gerrit-Owner: Maxim Polyakov <max.senia.poliak(a)gmail.com> Gerrit-Reviewer: Maxim Polyakov <max.senia.poliak(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

5 39

Change in ...coreboot[master]: libpayload: Deduplicate strtol and strtoull
by Julius Werner (Code Review) 10 Apr '19

10 Apr '19

Hello Nico Huber, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/32086 to review the following change. Change subject: libpayload: Deduplicate strtol and strtoull ...................................................................... libpayload: Deduplicate strtol and strtoull Our strtol() and strtoull() function contain almost exactly the same code. This is a) bad in general and b) may cause the code to get out of sync, such as it recently happened with CB:32029. This patch changes strtol() to be based on strtoull() so that the main parsing code exists only once, and also adds a strtoll() to round off the library. Also fix the bounds imposed by strtoul() to be based on the actual length of a 'long', not hardcoded to 32-bits (which is not equivalent on all architectures). Change-Id: I919c65a773cecdb11739c3f22dd0d182ed50c07f Signed-off-by: Julius Werner <jwerner(a)chromium.org> --- M payloads/libpayload/include/stdlib.h M payloads/libpayload/libc/string.c 2 files changed, 26 insertions(+), 40 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/32086/1 diff --git a/payloads/libpayload/include/stdlib.h b/payloads/libpayload/include/stdlib.h index aaacc87..d4a7844 100644 --- a/payloads/libpayload/include/stdlib.h +++ b/payloads/libpayload/include/stdlib.h @@ -196,6 +196,7 @@ * @{ */ long int strtol(const char *s, char **nptr, int base); +long long int strtoll(const char *s, char **nptr, int base); unsigned long int strtoul(const char *s, char **nptr, int base); unsigned long long int strtoull(const char *s, char **nptr, int base); long atol(const char *nptr); diff --git a/payloads/libpayload/libc/string.c b/payloads/libpayload/libc/string.c index f563f63..6c257cb 100644 --- a/payloads/libpayload/libc/string.c +++ b/payloads/libpayload/libc/string.c @@ -33,6 +33,7 @@ #include <string.h> #include <ctype.h> #include <inttypes.h> +#include <limits.h> #include <errno.h> /** @@ -419,59 +420,43 @@ * @return A signed integer representation of the string */ -long int strtol(const char *ptr, char **endptr, int base) +long long int strtoll(const char *orig_ptr, char **endptr, int base) { - int ret = 0; - int negative = 1; - - if (endptr != NULL) - *endptr = (char *) ptr; + const char *ptr = orig_ptr; + int is_negative = 0; /* Purge whitespace */ for( ; *ptr && isspace(*ptr); ptr++); if (ptr[0] == '-') { - negative = -1; + is_negative = 1; ptr++; } - if (!*ptr) - return 0; + unsigned long long uval = strtoull(ptr, endptr, base); - /* Determine the base */ + /* If the whole string is unparseable, endptr should point to start. */ + if (endptr && *endptr == ptr) + *endptr = (char *)orig_ptr; - if (base == 0) { - if (ptr[0] == '0' && (ptr[1] == 'x' || ptr[1] == 'X')) - base = 16; - else if (ptr[0] == '0') { - base = 8; - ptr++; - } - else - base = 10; - } + if (uval > (unsigned long long)LLONG_MAX + !!is_negative) + uval = (unsigned long long)LLONG_MAX + !!is_negative; - /* Base 16 allows the 0x on front - so skip over it */ + if (is_negative) + return -uval; + else + return uval; +} - if (base == 16) { - if (ptr[0] == '0' && (ptr[1] == 'x' || ptr[1] == 'X')) - ptr += 2; - } - - /* If the first character isn't valid, then don't - * bother */ - - if (!*ptr || !_valid(*ptr, base)) - return 0; - - for( ; *ptr && _valid(*ptr, base); ptr++) - ret = (ret * base) + _offset(*ptr, base); - - if (endptr != NULL) - *endptr = (char *) ptr; - - return ret * negative; +long int strtol(const char *ptr, char **endptr, int base) +{ + long long int val = strtoll(ptr, endptr, base); + if (val > LONG_MAX) + return LONG_MAX; + if (val < LONG_MIN) + return LONG_MIN; + return val; } long atol(const char *nptr) @@ -534,7 +519,7 @@ unsigned long int strtoul(const char *ptr, char **endptr, int base) { unsigned long long val = strtoull(ptr, endptr, base); - if (val > UINT32_MAX) return UINT32_MAX; + if (val > ULONG_MAX) return ULONG_MAX; return val; } -- To view, visit https://review.coreboot.org/c/coreboot/+/32086 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I919c65a773cecdb11739c3f22dd0d182ed50c07f Gerrit-Change-Number: 32086 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newchange

3 5

Change in ...coreboot[master]: libpayload: limits.h: Provide reliable definitions for all XXX_MAX/MIN
by Julius Werner (Code Review) 10 Apr '19

10 Apr '19

Hello Nico Huber, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/32085 to review the following change. Change subject: libpayload: limits.h: Provide reliable definitions for all XXX_MAX/MIN ...................................................................... libpayload: limits.h: Provide reliable definitions for all XXX_MAX/MIN Our current limits.h only provides (U)INT_MAX constants. This patch adds most others expected by POSIX. Since some of these may be different depending on architecture (e.g. 'long' is 32-bit on x86 and 64-bit on arm64), provide a definition that will automatically figure out the right value for the data model the compiler is using (as long as it's using two's complement for signed integers, which I think we can assume these days). Change-Id: I1124a41279abd4f53d208270e392e590ca8eaada Signed-off-by: Julius Werner <jwerner(a)chromium.org> --- M payloads/libpayload/include/limits.h 1 file changed, 15 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/32085/1 diff --git a/payloads/libpayload/include/limits.h b/payloads/libpayload/include/limits.h index 2fecf23..4238e0e 100644 --- a/payloads/libpayload/include/limits.h +++ b/payloads/libpayload/include/limits.h @@ -40,7 +40,20 @@ # endif #endif -#define UINT_MAX (unsigned int)0xffffffff -#define INT_MAX (unsigned int)0x7fffffff +#define USHRT_MAX ((unsigned short int)~0U) +#define SHRT_MIN ((short int)(USHRT_MAX & ~(USHRT_MAX >> 1))) +#define SHRT_MAX ((short int)(USHRT_MAX >> 1)) + +#define UINT_MAX ((unsigned int)~0U) +#define INT_MIN ((int)(UINT_MAX & ~(UINT_MAX >> 1))) +#define INT_MAX ((int)(UINT_MAX >> 1)) + +#define ULONG_MAX ((unsigned long int)~0UL) +#define LONG_MIN ((long int)(ULONG_MAX & ~(ULONG_MAX >> 1))) +#define LONG_MAX ((long int)(ULONG_MAX >> 1)) + +#define ULLONG_MAX ((unsigned long long int)~0UL) +#define LLONG_MIN ((long long int)(ULLONG_MAX & ~(ULLONG_MAX >> 1))) +#define LLONG_MAX ((long long int)(ULLONG_MAX >> 1)) #endif -- To view, visit https://review.coreboot.org/c/coreboot/+/32085 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1124a41279abd4f53d208270e392e590ca8eaada Gerrit-Change-Number: 32085 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newchange

2 3

Change in ...coreboot[master]: payload/libpayload: This is a fixe for potential NULL pointer derefer...
by Francois Toguo Fotso (Code Review) 09 Apr '19

09 Apr '19

Francois Toguo Fotso has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32083 Change subject: payload/libpayload: This is a fixe for potential NULL pointer dereferences ...................................................................... payload/libpayload: This is a fixe for potential NULL pointer dereferences These potential issues are flagged in Klockwork reports BUG=NONE TEST=Boot to OS on GLK Sparky Signed-off-by: Francois Toguo <francois.toguo.fotso(a)intel.com> Change-Id: I9d4636f0429de829e746909492c2f543026a02ac --- M payloads/libpayload/drivers/usb/ehci.c M payloads/libpayload/drivers/usb/ohci.c M payloads/libpayload/drivers/usb/uhci.c 3 files changed, 76 insertions(+), 19 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/32083/1 diff --git a/payloads/libpayload/drivers/usb/ehci.c b/payloads/libpayload/drivers/usb/ehci.c index 30cf191..61a39a7 100644 --- a/payloads/libpayload/drivers/usb/ehci.c +++ b/payloads/libpayload/drivers/usb/ehci.c @@ -634,6 +634,8 @@ /* create #reqcount transfer descriptors (qTDs) */ intrq->head = (intr_qtd_t *)dma_memalign(64, sizeof(intr_qtd_t)); + if (!intrq->head) + fatal("Not enough memory to create #reqcount transfer descriptors. \n"); intr_qtd_t *cur_td = intrq->head; for (i = 0; i < reqcount; ++i) { fill_intr_queue_td(intrq, cur_td, data); @@ -642,6 +644,11 @@ /* create one more qTD */ intr_qtd_t *const next_td = (intr_qtd_t *)dma_memalign(64, sizeof(intr_qtd_t)); + if (!next_td) { + usb_debug("Not enough memory to add transfer descriptor. \n"); + free(next_td); + continue; + } cur_td->td.next_qtd = virt_to_phys(&next_td->td); cur_td->next = next_td; cur_td = next_td; @@ -651,6 +658,8 @@ /* create spare qTD */ intrq->spare = (intr_qtd_t *)dma_memalign(64, sizeof(intr_qtd_t)); + if (!intrq->spare) + fatal("Not enough memory to create spare transfer descriptors.\n"); intrq->spare->data = data; /* initialize QH */ @@ -824,15 +833,20 @@ * and doesn't violate the standard. */ EHCI_INST(controller)->dummy_qh = (ehci_qh_t *)dma_memalign(64, sizeof(ehci_qh_t)); - memset((void *)EHCI_INST(controller)->dummy_qh, 0, - sizeof(*EHCI_INST(controller)->dummy_qh)); - EHCI_INST(controller)->dummy_qh->horiz_link_ptr = QH_TERMINATE; - EHCI_INST(controller)->dummy_qh->td.next_qtd = QH_TERMINATE; - EHCI_INST(controller)->dummy_qh->td.alt_next_qtd = QH_TERMINATE; - for (i = 0; i < 1024; ++i) - periodic_list[i] = virt_to_phys(EHCI_INST(controller)->dummy_qh) - | PS_TYPE_QH; - + if (EHCI_INST(controller)->dummy_qh) { + memset((void *)EHCI_INST(controller)->dummy_qh, 0, + sizeof(*EHCI_INST(controller)->dummy_qh)); + EHCI_INST(controller)->dummy_qh->horiz_link_ptr = QH_TERMINATE; + EHCI_INST(controller)->dummy_qh->td.next_qtd = QH_TERMINATE; + EHCI_INST(controller)->dummy_qh->td.alt_next_qtd = QH_TERMINATE; + for (i = 0; i < 1024; ++i) + periodic_list[i] = virt_to_phys(EHCI_INST(controller)->dummy_qh) + | PS_TYPE_QH; + } else { + usb_debug("Not enough DMA memory for EHCI dummy QH!\n"); + free(EHCI_INST(controller)->dummy_qh); + } + /* Make sure periodic schedule is disabled */ ehci_set_periodic_schedule(EHCI_INST(controller), 0); /* Set periodic frame list pointer */ diff --git a/payloads/libpayload/drivers/usb/ohci.c b/payloads/libpayload/drivers/usb/ohci.c index 1964689..045bb7a 100644 --- a/payloads/libpayload/drivers/usb/ohci.c +++ b/payloads/libpayload/drivers/usb/ohci.c @@ -212,6 +212,8 @@ udelay (10); /* at most 10us for reset to complete. State must be set to Operational within 2ms (5.1.1.4) */ OHCI_INST (controller)->opreg->HcFmInterval = interval; OHCI_INST (controller)->hcca = dma_memalign(256, 256); + if (!OHCI_INST(controller)->hcca) + fatal("Not enough DMA memory for OHCI hcca.\n"); memset((void*)OHCI_INST (controller)->hcca, 0, 256); if (dma_initialized()) { @@ -223,6 +225,8 @@ /* Initialize interrupt table. */ u32 *const intr_table = OHCI_INST(controller)->hcca->HccaInterruptTable; ed_t *const periodic_ed = dma_memalign(sizeof(ed_t), sizeof(ed_t)); + if (!periodic_ed) + fatal("Not enough DMA memory for OHCI interrupt table .\n"); memset((void *)periodic_ed, 0, sizeof(*periodic_ed)); for (i = 0; i < 32; ++i) intr_table[i] = virt_to_phys(periodic_ed); @@ -378,6 +382,8 @@ /* First TD. */ td_t *const first_td = (td_t *)dma_memalign(sizeof(td_t), sizeof(td_t)); + if (!first_td) + fatal("Not enough DMA memory for OHCI first TD in buffer.\n"); memset((void *)first_td, 0, sizeof(*first_td)); cur = first_td; @@ -392,6 +398,8 @@ while (pages > 0) { /* One more TD. */ td_t *const next = (td_t *)dma_memalign(sizeof(td_t), sizeof(td_t)); + if (!next) + fatal("Not enough DMA memory for OHCI new page.\n"); memset((void *)next, 0, sizeof(*next)); /* Linked to the previous. */ cur->next_td = virt_to_phys(next); @@ -426,6 +434,8 @@ /* One more TD. */ td_t *const next_td = (td_t *)dma_memalign(sizeof(td_t), sizeof(td_t)); + if (!next_td) + fatal("Not enough DMA memory for OHCI additional TD.\n"); memset((void *)next_td, 0, sizeof(*next_td)); /* Linked to the previous. */ cur->next_td = virt_to_phys(next_td); @@ -441,12 +451,18 @@ /* Final dummy TD. */ td_t *const final_td = (td_t *)dma_memalign(sizeof(td_t), sizeof(td_t)); - memset((void *)final_td, 0, sizeof(*final_td)); - /* Linked to the previous. */ - cur->next_td = virt_to_phys(final_td); - + if (final_td) { + memset((void *)final_td, 0, sizeof(*final_td)); + /* Linked to the previous. */ + cur->next_td = virt_to_phys(final_td); + } else { + usb_debug("Not enough DMA memory for OHCI dummy TD!\n"); + free(final_td); + } /* Data structures */ ed_t *head = dma_memalign(sizeof(ed_t), sizeof(ed_t)); + if (!head) + fatal("Not enough DMA memory for OHCI data structures.\n"); memset((void*)head, 0, sizeof(*head)); head->config = (dev->address << ED_FUNC_SHIFT) | (0 << ED_EP_SHIFT) | @@ -519,6 +535,8 @@ /* First TD. */ td_t *const first_td = (td_t *)dma_memalign(sizeof(td_t), sizeof(td_t)); + if (!first_td) + fatal("Not enough DMA memory for OHCI first TD of bulk transfer.\n"); memset((void *)first_td, 0, sizeof(*first_td)); cur = next = first_td; @@ -557,6 +575,10 @@ } /* One more TD. */ next = (td_t *)dma_memalign(sizeof(td_t), sizeof(td_t)); + if (!next) { + usb_debug("Not enough DMA memory for OHCI addition TD in bulk transfer.\n"); + break; + } memset((void *)next, 0, sizeof(*next)); /* Linked to the previous. */ cur->next_td = virt_to_phys(next); @@ -569,6 +591,8 @@ /* Data structures */ ed_t *head = dma_memalign(sizeof(ed_t), sizeof(ed_t)); + if (!head) + fatal("Not enough DMA memory for OHCI bulk transfer's head.\n"); memset((void*)head, 0, sizeof(*head)); head->config = (ep->dev->address << ED_FUNC_SHIFT) | ((ep->endpoint & 0xf) << ED_EP_SHIFT) | @@ -665,6 +689,11 @@ intr_queue_t *const intrq = (intr_queue_t *)dma_memalign(sizeof(intrq->ed), sizeof(*intrq)); + if (!intrq) { + usb_debug("Not enough DMA memory for intr queue \n"); + free(final_td); + return NULL; + } memset(intrq, 0, sizeof(*intrq)); intrq->data = (u8 *)dma_malloc(reqcount * reqsize); intrq->reqsize = reqsize; @@ -674,6 +703,11 @@ u8 *cur_data = intrq->data; for (i = 0; i < reqcount; ++i) { intrq_td_t *const td = dma_memalign(sizeof(td->td), sizeof(*td)); + if (!td) { + usb_debug("Not enough memory to add transfer descriptor. \n"); + free(td); + continue; + } ++intrq->remaining_tds; ohci_fill_intrq_td(td, intrq, cur_data); cur_data += reqsize; @@ -686,12 +720,17 @@ /* Create last, dummy TD. */ intrq_td_t *dummy_td = dma_memalign(sizeof(dummy_td->td), sizeof(*dummy_td)); - memset(dummy_td, 0, sizeof(*dummy_td)); - dummy_td->intrq = intrq; - if (last_td) - last_td->td.next_td = virt_to_phys(&dummy_td->td); - last_td = dummy_td; - + if (dummy_td) { + memset(dummy_td, 0, sizeof(*dummy_td)); + dummy_td->intrq = intrq; + if (last_td) + last_td->td.next_td = virt_to_phys(&dummy_td->td); + last_td = dummy_td; + } else { + usb_debug("Not enough memory to add dummy TD. \n"); + free(dummy_td); + } + /* Initialize ED. */ intrq->ed.config = (ep->dev->address << ED_FUNC_SHIFT) | ((ep->endpoint & 0xf) << ED_EP_SHIFT) | diff --git a/payloads/libpayload/drivers/usb/uhci.c b/payloads/libpayload/drivers/usb/uhci.c index 70071eb..e62fd25 100644 --- a/payloads/libpayload/drivers/usb/uhci.c +++ b/payloads/libpayload/drivers/usb/uhci.c @@ -317,6 +317,8 @@ unsigned short req = ((unsigned short *) devreq)[0]; int i; td_t *tds = memalign (16, sizeof (td_t) * count); + if (!tds) + fatal("Not enough memory for uhci control.\n"); memset (tds, 0, sizeof (td_t) * count); count--; /* to compensate for 0-indexed array */ for (i = 0; i < count; i++) { @@ -386,6 +388,8 @@ if (numpackets == 0) return 0; td_t *tds = memalign (16, sizeof (td_t) * numpackets); + if (!tds) + fatal("Not enough memory for packets scheduling.\n"); memset (tds, 0, sizeof (td_t) * numpackets); int i; for (i = 0; i < numpackets; i++) { -- To view, visit https://review.coreboot.org/c/coreboot/+/32083 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9d4636f0429de829e746909492c2f543026a02ac Gerrit-Change-Number: 32083 Gerrit-PatchSet: 1 Gerrit-Owner: Francois Toguo Fotso <francois.toguo.fotso(a)intel.com> Gerrit-MessageType: newchange

7 35

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2019