[coreboot-gerrit] Change in ...coreboot[master]: [WIP]x86|mb/emulation/qemu-q35: 64bit ramstage support

Wed Nov 21 09:38:26 CET 2018

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29667 )

Change subject: [WIP]x86|mb/emulation/qemu-q35: 64bit ramstage support
......................................................................

Patch Set 5:

(27 comments)

https://review.coreboot.org/#/c/29667/5/src/arch/x86/boot.c 
File src/arch/x86/boot.c:

https://review.coreboot.org/#/c/29667/5/src/arch/x86/boot.c@36 
PS5, Line 36: 	printk(BIOS_DEBUG, "Jumping to %p\n", (void*)prog_entry(prog));
"(foo*)" should be "(foo *)"

https://review.coreboot.org/#/c/29667/5/src/arch/x86/boot.c@38 
PS5, Line 38: 	if (ENV_RAMSTAGE && IS_ENABLED(CONFIG_ARCH_RAMSTAGE_X86_64) && IS_ENABLED(CONFIG_ARCH_PAYLOAD_X86_32))
line over 80 characters

https://review.coreboot.org/#/c/29667/5/src/arch/x86/exception.c 
File src/arch/x86/exception.c:

https://review.coreboot.org/#/c/29667/5/src/arch/x86/exception.c@513 
PS5, Line 513: 	for (int i = 0; i < 16; i++)
suspect code indent for conditional statements (8, 8)

https://review.coreboot.org/#/c/29667/5/src/arch/x86/include/arch/mmu.h 
File src/arch/x86/include/arch/mmu.h:

https://review.coreboot.org/#/c/29667/5/src/arch/x86/include/arch/mmu.h@78 
PS5, Line 78: #define L0_ADDR_MASK     (((1ULL << BITS_RESOLVED_PER_LVL) - 1) << L0_ADDR_SHIFT)
line over 80 characters

https://review.coreboot.org/#/c/29667/5/src/arch/x86/include/arch/mmu.h@79 
PS5, Line 79: #define L1_ADDR_MASK     (((1ULL << BITS_RESOLVED_PER_LVL) - 1) << L1_ADDR_SHIFT)
line over 80 characters

https://review.coreboot.org/#/c/29667/5/src/arch/x86/include/arch/mmu.h@80 
PS5, Line 80: #define L2_ADDR_MASK     (((1ULL << BITS_RESOLVED_PER_LVL) - 1) << L2_ADDR_SHIFT)
line over 80 characters

https://review.coreboot.org/#/c/29667/5/src/arch/x86/include/arch/mmu.h@81 
PS5, Line 81: #define L3_ADDR_MASK     (((1ULL << BITS_RESOLVED_PER_LVL) - 1) << L3_ADDR_SHIFT)
line over 80 characters

https://review.coreboot.org/#/c/29667/5/src/arch/x86/include/arch/registers.h 
File src/arch/x86/include/arch/registers.h:

https://review.coreboot.org/#/c/29667/5/src/arch/x86/include/arch/registers.h@23 
PS5, Line 23: #define DOWNTO8(A) \
macros should not use a trailing semicolon

https://review.coreboot.org/#/c/29667/5/src/arch/x86/include/arch/registers.h@41 
PS5, Line 41: #define DOWNTO16(A) \
macros should not use a trailing semicolon

https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c 
File src/arch/x86/mmu.c:

https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@214 
PS5, Line 214: 	if ((size >= L1_XLAT_SIZE) &&
suspect code indent for conditional statements (8, 24)

https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@274 
PS5, Line 274: 		switch ((pte[index] & AVAIL_MASK) >> AVAIL_SHIFT) {
switch and case should be at the same indent

https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@276 
PS5, Line 276: 				printk(BIOS_DEBUG, "INVAL ");break;
space required after that ';' (ctx:VxV)

https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@278 
PS5, Line 278: 				printk(BIOS_DEBUG, "TABLE @ %llx\n", pte[index] & XLAT_ADDR_MASK);break;
line over 80 characters

https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@278 
PS5, Line 278: 				printk(BIOS_DEBUG, "TABLE @ %llx\n", pte[index] & XLAT_ADDR_MASK);break;
space required after that ';' (ctx:VxV)

https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@280 
PS5, Line 280: 				printk(BIOS_DEBUG, "PAGE %llx, %llx %llx\n", pte[index] & ((XLAT_ADDR_MASK<<1)),
line over 80 characters

https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@281 
PS5, Line 281: 			(pte[index] & ((XLAT_ADDR_MASK<<1))) + (1ULL << shift) -1,pte[index] & 0xfff);break;
line over 80 characters

https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@281 
PS5, Line 281: 			(pte[index] & ((XLAT_ADDR_MASK<<1))) + (1ULL << shift) -1,pte[index] & 0xfff);break;
need consistent spacing around '-' (ctx:WxV)

https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@281 
PS5, Line 281: 			(pte[index] & ((XLAT_ADDR_MASK<<1))) + (1ULL << shift) -1,pte[index] & 0xfff);break;
space required after that ',' (ctx:VxV)

https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@281 
PS5, Line 281: 			(pte[index] & ((XLAT_ADDR_MASK<<1))) + (1ULL << shift) -1,pte[index] & 0xfff);break;
space required after that ';' (ctx:VxV)

https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@284 
PS5, Line 284: 			printk(BIOS_DEBUG, "BLOCK %llx, %llx %llx\n", pte[index] & ((XLAT_ADDR_MASK<<1)),
line over 80 characters

https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@285 
PS5, Line 285: 			(pte[index] & ((XLAT_ADDR_MASK<<1))) + (1ULL << shift) -1,pte[index] & 0xfff);break;
line over 80 characters

https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@285 
PS5, Line 285: 			(pte[index] & ((XLAT_ADDR_MASK<<1))) + (1ULL << shift) -1,pte[index] & 0xfff);break;
need consistent spacing around '-' (ctx:WxV)

https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@285 
PS5, Line 285: 			(pte[index] & ((XLAT_ADDR_MASK<<1))) + (1ULL << shift) -1,pte[index] & 0xfff);break;
space required after that ',' (ctx:VxV)

https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@285 
PS5, Line 285: 			(pte[index] & ((XLAT_ADDR_MASK<<1))) + (1ULL << shift) -1,pte[index] & 0xfff);break;
space required after that ';' (ctx:VxV)

https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@335 
PS5, Line 335:  * The page tables generated are ment for long mode and do not work in
'ment' may be misspelled - perhaps 'meant'?

https://review.coreboot.org/#/c/29667/5/src/arch/x86/mmu.c@352 
PS5, Line 352: 		next_free_table = cbmem_add(CBMEM_ID_TTB, CONFIG_TTB_SIZE_KB * 1024);
line over 80 characters

https://review.coreboot.org/#/c/29667/5/src/include/device/pci.h 
File src/include/device/pci.h:

https://review.coreboot.org/#/c/29667/5/src/include/device/pci.h@59 
PS5, Line 59: } __attribute__(( aligned(sizeof(void *) * 4)));
space prohibited after that open parenthesis '('

-- 
To view, visit https://review.coreboot.org/c/coreboot/+/29667
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If2f02a95b2f91ab51043d4e81054354f4a6eb5d5
Gerrit-Change-Number: 29667
Gerrit-PatchSet: 5
Gerrit-Owner: Patrick Rudolph <patrick.rudolph at 9elements.com>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner at chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth at google.com>
Gerrit-Reviewer: Patrick Rudolph <siro at das-labor.org>
Gerrit-Reviewer: Ronald G. Minnich <rminnich at gmail.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer at coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-CC: Nico Huber <nico.h at gmx.de>
Gerrit-Comment-Date: Wed, 21 Nov 2018 08:38:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20181121/bce626f7/attachment.html>