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.... File src/arch/x86/include/arch/registers.h:
https://review.coreboot.org/#/c/29667/5/src/arch/x86/include/arch/registers.... 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.... 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 '('