<p><a href="https://review.coreboot.org/c/coreboot/+/29667">View Change</a></p><p>27 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/boot.c">File src/arch/x86/boot.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/boot.c@36">Patch Set #5, Line 36:</a> <code style="font-family:monospace,monospace"> printk(BIOS_DEBUG, "Jumping to %p\n", (void*)prog_entry(prog));</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">"(foo*)" should be "(foo *)"</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/boot.c@38">Patch Set #5, Line 38:</a> <code style="font-family:monospace,monospace"> if (ENV_RAMSTAGE && IS_ENABLED(CONFIG_ARCH_RAMSTAGE_X86_64) && IS_ENABLED(CONFIG_ARCH_PAYLOAD_X86_32))</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">line over 80 characters</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/exception.c">File src/arch/x86/exception.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/exception.c@513">Patch Set #5, Line 513:</a> <code style="font-family:monospace,monospace"> for (int i = 0; i < 16; i++)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">suspect code indent for conditional statements (8, 8)</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/include/arch/mmu.h">File src/arch/x86/include/arch/mmu.h:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/include/arch/mmu.h@78">Patch Set #5, Line 78:</a> <code style="font-family:monospace,monospace">#define L0_ADDR_MASK (((1ULL << BITS_RESOLVED_PER_LVL) - 1) << L0_ADDR_SHIFT)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">line over 80 characters</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/include/arch/mmu.h@79">Patch Set #5, Line 79:</a> <code style="font-family:monospace,monospace">#define L1_ADDR_MASK (((1ULL << BITS_RESOLVED_PER_LVL) - 1) << L1_ADDR_SHIFT)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">line over 80 characters</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/include/arch/mmu.h@80">Patch Set #5, Line 80:</a> <code style="font-family:monospace,monospace">#define L2_ADDR_MASK (((1ULL << BITS_RESOLVED_PER_LVL) - 1) << L2_ADDR_SHIFT)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">line over 80 characters</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/include/arch/mmu.h@81">Patch Set #5, Line 81:</a> <code style="font-family:monospace,monospace">#define L3_ADDR_MASK (((1ULL << BITS_RESOLVED_PER_LVL) - 1) << L3_ADDR_SHIFT)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">line over 80 characters</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/include/arch/registers.h">File src/arch/x86/include/arch/registers.h:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/include/arch/registers.h@23">Patch Set #5, Line 23:</a> <code style="font-family:monospace,monospace">#define DOWNTO8(A) \</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">macros should not use a trailing semicolon</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/include/arch/registers.h@41">Patch Set #5, Line 41:</a> <code style="font-family:monospace,monospace">#define DOWNTO16(A) \</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">macros should not use a trailing semicolon</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c">File src/arch/x86/mmu.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@214">Patch Set #5, Line 214:</a> <code style="font-family:monospace,monospace"> if ((size >= L1_XLAT_SIZE) &&</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">suspect code indent for conditional statements (8, 24)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@274">Patch Set #5, Line 274:</a> <code style="font-family:monospace,monospace"> switch ((pte[index] & AVAIL_MASK) >> AVAIL_SHIFT) {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">switch and case should be at the same indent</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@276">Patch Set #5, Line 276:</a> <code style="font-family:monospace,monospace"> printk(BIOS_DEBUG, "INVAL ");break;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">space required after that ';' (ctx:VxV)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@278">Patch Set #5, Line 278:</a> <code style="font-family:monospace,monospace"> printk(BIOS_DEBUG, "TABLE @ %llx\n", pte[index] & XLAT_ADDR_MASK);break;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">line over 80 characters</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@278">Patch Set #5, Line 278:</a> <code style="font-family:monospace,monospace"> printk(BIOS_DEBUG, "TABLE @ %llx\n", pte[index] & XLAT_ADDR_MASK);break;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">space required after that ';' (ctx:VxV)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@280">Patch Set #5, Line 280:</a> <code style="font-family:monospace,monospace"> printk(BIOS_DEBUG, "PAGE %llx, %llx %llx\n", pte[index] & ((XLAT_ADDR_MASK<<1)),</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">line over 80 characters</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@281">Patch Set #5, Line 281:</a> <code style="font-family:monospace,monospace"> (pte[index] & ((XLAT_ADDR_MASK<<1))) + (1ULL << shift) -1,pte[index] & 0xfff);break;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">line over 80 characters</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@281">Patch Set #5, Line 281:</a> <code style="font-family:monospace,monospace"> (pte[index] & ((XLAT_ADDR_MASK<<1))) + (1ULL << shift) -1,pte[index] & 0xfff);break;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">need consistent spacing around '-' (ctx:WxV)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@281">Patch Set #5, Line 281:</a> <code style="font-family:monospace,monospace"> (pte[index] & ((XLAT_ADDR_MASK<<1))) + (1ULL << shift) -1,pte[index] & 0xfff);break;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">space required after that ',' (ctx:VxV)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@281">Patch Set #5, Line 281:</a> <code style="font-family:monospace,monospace"> (pte[index] & ((XLAT_ADDR_MASK<<1))) + (1ULL << shift) -1,pte[index] & 0xfff);break;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">space required after that ';' (ctx:VxV)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@284">Patch Set #5, Line 284:</a> <code style="font-family:monospace,monospace"> printk(BIOS_DEBUG, "BLOCK %llx, %llx %llx\n", pte[index] & ((XLAT_ADDR_MASK<<1)),</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">line over 80 characters</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@285">Patch Set #5, Line 285:</a> <code style="font-family:monospace,monospace"> (pte[index] & ((XLAT_ADDR_MASK<<1))) + (1ULL << shift) -1,pte[index] & 0xfff);break;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">line over 80 characters</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@285">Patch Set #5, Line 285:</a> <code style="font-family:monospace,monospace"> (pte[index] & ((XLAT_ADDR_MASK<<1))) + (1ULL << shift) -1,pte[index] & 0xfff);break;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">need consistent spacing around '-' (ctx:WxV)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@285">Patch Set #5, Line 285:</a> <code style="font-family:monospace,monospace"> (pte[index] & ((XLAT_ADDR_MASK<<1))) + (1ULL << shift) -1,pte[index] & 0xfff);break;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">space required after that ',' (ctx:VxV)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@285">Patch Set #5, Line 285:</a> <code style="font-family:monospace,monospace"> (pte[index] & ((XLAT_ADDR_MASK<<1))) + (1ULL << shift) -1,pte[index] & 0xfff);break;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">space required after that ';' (ctx:VxV)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@335">Patch Set #5, Line 335:</a> <code style="font-family:monospace,monospace"> * The page tables generated are ment for long mode and do not work in</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">'ment' may be misspelled - perhaps 'meant'?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@352">Patch Set #5, Line 352:</a> <code style="font-family:monospace,monospace"> next_free_table = cbmem_add(CBMEM_ID_TTB, CONFIG_TTB_SIZE_KB * 1024);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">line over 80 characters</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://review.coreboot.org/#/c/29667/5/src/include/device/pci.h">File src/include/device/pci.h:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/29667/5/src/include/device/pci.h@59">Patch Set #5, Line 59:</a> <code style="font-family:monospace,monospace">} __attribute__(( aligned(sizeof(void *) * 4)));</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">space prohibited after that open parenthesis '('</p></li></ul></li></ul><p>To view, visit <a href="https://review.coreboot.org/c/coreboot/+/29667">change 29667</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://review.coreboot.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://review.coreboot.org/c/coreboot/+/29667"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: coreboot </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: If2f02a95b2f91ab51043d4e81054354f4a6eb5d5 </div>
<div style="display:none"> Gerrit-Change-Number: 29667 </div>
<div style="display:none"> Gerrit-PatchSet: 5 </div>
<div style="display:none"> Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com> </div>
<div style="display:none"> Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org> </div>
<div style="display:none"> Gerrit-Reviewer: Julius Werner <jwerner@chromium.org> </div>
<div style="display:none"> Gerrit-Reviewer: Martin Roth <martinroth@google.com> </div>
<div style="display:none"> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> </div>
<div style="display:none"> Gerrit-Reviewer: Ronald G. Minnich <rminnich@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org> </div>
<div style="display:none"> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> </div>
<div style="display:none"> Gerrit-CC: Nico Huber <nico.h@gmx.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 21 Nov 2018 08:38:26 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Gerrit-MessageType: comment </div>