coreboot-gerrit December 2018

coreboot-gerrit@coreboot.org

1 participants
1832 discussions

Change in ...coreboot[master]: [WIP]arch/x86: Support x86_64 exceptions
by Patrick Rudolph (Code Review) 09 Dec '18

09 Dec '18

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30117 Change subject: [WIP]arch/x86: Support x86_64 exceptions ...................................................................... [WIP]arch/x86: Support x86_64 exceptions * Seems to have alignment issues Signed-off-by: Patrick Rudolph <siro(a)das-labor.org> Change-Id: Idd12c90a95cc2989eb9b2a718740a84222193f48 --- M src/arch/x86/exception.c M src/arch/x86/idt.S M src/arch/x86/include/arch/registers.h 3 files changed, 90 insertions(+), 12 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/30117/1 diff --git a/src/arch/x86/exception.c b/src/arch/x86/exception.c index 0f42fdf..e89215f 100644 --- a/src/arch/x86/exception.c +++ b/src/arch/x86/exception.c @@ -502,12 +502,31 @@ put_packet(out_buffer); } #else /* !CONFIG_GDB_STUB */ -#define MDUMP_SIZE 0x80 unsigned int logical_processor = 0; #if ENV_RAMSTAGE logical_processor = cpu_index(); #endif + u8 *code; +#ifdef __x86_64__ +#define MDUMP_SIZE 0x100 + printk(BIOS_EMERG, + "CPU Index %d - APIC %d Unexpected Exception:\n" + "%lld @ %02llx:%016llx - Halting\n" + "Code: %lld rflags: %016llx cr2: %016llx\n" + "rax: %016llx rbx: %016llx\n" + "rcx: %016llx rdx: %016llx\n" + "rdi: %016llx rsi: %016llx\n" + "rbp: %016llx rsp: %016llx\n", + logical_processor, (unsigned int)lapicid(), + info->vector, info->cs, info->rip, + info->error_code, info->rflags, read_cr2(), + info->rax, info->rbx, info->rcx, info->rdx, + info->rdi, info->rsi, info->rbp, info->rsp); + code = (u8 *)((uintptr_t)info->rip - (MDUMP_SIZE >> 2)); +#else +#define MDUMP_SIZE 0x80 + printk(BIOS_EMERG, "CPU Index %d - APIC %d Unexpected Exception:" "%d @ %02x:%08x - Halting\n" @@ -519,7 +538,8 @@ info->error_code, info->eflags, read_cr2(), info->eax, info->ebx, info->ecx, info->edx, info->edi, info->esi, info->ebp, info->esp); - u8 *code = (u8 *)((uintptr_t)info->eip - (MDUMP_SIZE >> 1)); + code = (u8 *)((uintptr_t)info->eip - (MDUMP_SIZE >> 1)); +#endif /* Align to 8-byte boundary please, and print eight bytes per row. * This is done to make DRAM burst timing/reordering errors more * evident from the looking at the dump */ diff --git a/src/arch/x86/idt.S b/src/arch/x86/idt.S index 9c36d81..8a3706d 100644 --- a/src/arch/x86/idt.S +++ b/src/arch/x86/idt.S @@ -120,26 +120,34 @@ .global int_hand int_hand: - /* At this point, on x86-32, on the stack there is: - * 0(%esp) vector - * 4(%esp) error code - * 8(%esp) eip - * 12(%esp) cs - * 16(%esp) eflags - */ #ifdef __x86_64__ + /* At this point, on x86-64, on the stack there is: + * 0(%rsp) vector + * 8(%rsp) error code + * 16(%rsp) rip + * 24(%rsp) cs + * 32(%rsp) rflags + * 40(%rsp) rsp + */ push %rdi push %rsi push %rbp /* Original stack pointer */ - lea 32(%rsp), %rbp + lea 64(%rsp), %rbp push %rbp push %rbx push %rdx push %rcx push %rax - push %rsp /* Pointer to structure on the stack */ + /* Save pointer to eregs structure */ + mov %rsp, %rbp + /* Align stack to 16 bytes. */ + and $(~0xf), %rsp + /* Save original stack pointer while keeping stack alignment. This + value is also the eregs argument x86_exception(). */ + sub $8, %rsp + push %rbp /* Pointer to structure on the stack */ call x86_exception pop %rax /* Drop the pointer */ @@ -152,8 +160,15 @@ pop %rsi pop %rdi - add $8, %rsp /* pop of the vector and error code */ + add $16, %rsp /* pop of the vector and error code */ #else + /* At this point, on x86-32, on the stack there is: + * 0(%esp) vector + * 4(%esp) error code + * 8(%esp) eip + * 12(%esp) cs + * 16(%esp) eflags + */ pushl %edi pushl %esi pushl %ebp diff --git a/src/arch/x86/include/arch/registers.h b/src/arch/x86/include/arch/registers.h index 1d3b90a..20fc683 100644 --- a/src/arch/x86/include/arch/registers.h +++ b/src/arch/x86/include/arch/registers.h @@ -19,6 +19,38 @@ #if !defined(__ASSEMBLER__) #include <stdint.h> +#ifdef __x86_64__ +#define DOWNTO8(A) \ + struct { \ + union { \ + struct { \ + union { \ + struct { \ + uint8_t A##l; \ + uint8_t A##h; \ + } __packed; \ + uint16_t A##x; \ + } __packed; \ + uint16_t h##A##x; \ + } __packed; \ + uint32_t e##A##x; \ + } __packed; \ + uint64_t r##A##x; \ + } __packed; + +#define DOWNTO16(A) \ + struct {\ + union { \ + struct { \ + uint16_t A; \ + uint16_t h##A; \ + } __packed; \ + uint32_t e##A; \ + } __packed; \ + uint64_t r##A; \ + } __packed; + +#else #define DOWNTO8(A) \ union { \ struct { \ @@ -43,6 +75,9 @@ uint32_t e##A; \ } __packed; +#endif + + struct eregs { DOWNTO8(a); DOWNTO8(c); @@ -52,11 +87,19 @@ DOWNTO16(bp); DOWNTO16(si); DOWNTO16(di); +#ifdef __x86_64__ + uint64_t vector; + uint64_t error_code; + uint64_t rip; + uint64_t cs; + uint64_t rflags; +#else uint32_t vector; uint32_t error_code; uint32_t eip; uint32_t cs; uint32_t eflags; +#endif }; #endif // !ASSEMBLER -- 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: 1 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

1 0

Change in ...coreboot[master]: [WIP]pci_drivers/cpu_drivers: Fix constructed arrays on 64bit platforms
by Patrick Rudolph (Code Review) 09 Dec '18

09 Dec '18

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30116 Change subject: [WIP]pci_drivers/cpu_drivers: Fix constructed arrays on 64bit platforms ...................................................................... [WIP]pci_drivers/cpu_drivers: Fix constructed arrays on 64bit platforms The __pci_driver and __cpu_driver uses variable length arrays which are constructed by the linker at build-time. The linker always place the structs at 16-byte boundary, as per "System V ABI". That's not a problem on x86, as the struct is exactly 16 Bytes in size. On other platforms, like x86_64 it breaks, because the runtime iterator isn't aware of that alignment. Make both linker and compiler aware of the used alignment. Fixes broken __pci_driver and __cpu_driver on x86_64 (and possibly aarch64). * Introduce ARCH_VLA_ALIGN_SIZE which defaults to 16 * Set ARCH_VLA_ALIGN_SIZE to 32 on x86_64 and aarch64 * Align struct pci_driver and struct cpu_driver to ARCH_VLA_ALIGN_SIZE * Align __pci_driver and __cpu_driver to ARCH_VLA_ALIGN_SIZE Change-Id: I2491d47ed03dcfd8db110dfb181b2c5281449591 Signed-off-by: Patrick Rudolph <siro(a)das-labor.org> --- M src/arch/arm/include/armv7/arch/cpu.h M src/arch/arm64/include/arch/memlayout.h M src/arch/arm64/include/armv8/arch/cpu.h M src/arch/mips/include/arch/cpu.h M src/arch/ppc64/include/arch/cpu.h M src/arch/riscv/include/arch/cpu.h M src/arch/x86/include/arch/cpu.h M src/arch/x86/include/arch/memlayout.h M src/include/device/pci.h M src/include/memlayout.h M src/lib/program.ld 11 files changed, 28 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/30116/1 diff --git a/src/arch/arm/include/armv7/arch/cpu.h b/src/arch/arm/include/armv7/arch/cpu.h index 22b3fb4..e0948cf 100644 --- a/src/arch/arm/include/armv7/arch/cpu.h +++ b/src/arch/arm/include/armv7/arch/cpu.h @@ -22,11 +22,12 @@ #if !defined(__PRE_RAM__) #include <device/device.h> +#include <memlayout.h> struct cpu_driver { struct device_operations *ops; const struct cpu_device_id *id_table; -}; +} __attribute__ ((aligned (ARCH_VLA_ALIGN_SIZE))); struct cpuinfo_arm { uint8_t arm; /* CPU family */ diff --git a/src/arch/arm64/include/arch/memlayout.h b/src/arch/arm64/include/arch/memlayout.h index 7fce9aa..a7977d3 100644 --- a/src/arch/arm64/include/arch/memlayout.h +++ b/src/arch/arm64/include/arch/memlayout.h @@ -37,4 +37,6 @@ REGION(stack, addr, size, 16) \ _ = ASSERT(size >= 2K, "stack should be >= 2K, see toolchain.inc"); +#define ARCH_VLA_ALIGN_SIZE 32 + #endif /* __ARCH_MEMLAYOUT_H */ diff --git a/src/arch/arm64/include/armv8/arch/cpu.h b/src/arch/arm64/include/armv8/arch/cpu.h index 6e096cc..6c9498c 100644 --- a/src/arch/arm64/include/armv8/arch/cpu.h +++ b/src/arch/arm64/include/armv8/arch/cpu.h @@ -22,7 +22,9 @@ #if !defined(__PRE_RAM__) -struct cpu_driver { }; +#include <memlayout.h> + +struct cpu_driver { } __attribute__ ((aligned (ARCH_VLA_ALIGN_SIZE))); #endif #endif /* __ARCH_CPU_H__ */ diff --git a/src/arch/mips/include/arch/cpu.h b/src/arch/mips/include/arch/cpu.h index 8e35908..16244cb 100644 --- a/src/arch/mips/include/arch/cpu.h +++ b/src/arch/mips/include/arch/cpu.h @@ -21,11 +21,12 @@ #ifndef __PRE_RAM__ #include <device/device.h> +#include <memlayout.h> struct cpu_driver { struct device_operations *ops; const struct cpu_device_id *id_table; -}; +} __attribute__ ((aligned (ARCH_VLA_ALIGN_SIZE))); struct thread; diff --git a/src/arch/ppc64/include/arch/cpu.h b/src/arch/ppc64/include/arch/cpu.h index 3238bfb..bd9b8ca 100644 --- a/src/arch/ppc64/include/arch/cpu.h +++ b/src/arch/ppc64/include/arch/cpu.h @@ -20,11 +20,12 @@ #if !defined(__PRE_RAM__) #include <device/device.h> +#include <memlayout.h> struct cpu_driver { struct device_operations *ops; const struct cpu_device_id *id_table; -}; +} __attribute__ ((aligned (ARCH_VLA_ALIGN_SIZE))); struct thread; diff --git a/src/arch/riscv/include/arch/cpu.h b/src/arch/riscv/include/arch/cpu.h index 4ee580c..db3c6b2 100644 --- a/src/arch/riscv/include/arch/cpu.h +++ b/src/arch/riscv/include/arch/cpu.h @@ -22,11 +22,12 @@ #if !defined(__PRE_RAM__) #include <device/device.h> +#include <memlayout.h> struct cpu_driver { struct device_operations *ops; const struct cpu_device_id *id_table; -}; +} __attribute__ ((aligned (ARCH_VLA_ALIGN_SIZE))); struct thread; diff --git a/src/arch/x86/include/arch/cpu.h b/src/arch/x86/include/arch/cpu.h index 99d1000..00c7b7f 100644 --- a/src/arch/x86/include/arch/cpu.h +++ b/src/arch/x86/include/arch/cpu.h @@ -17,6 +17,7 @@ #include <stdint.h> #include <stddef.h> #include <rules.h> +#include <memlayout.h> /* * EFLAGS bits @@ -180,7 +181,7 @@ struct device_operations *ops; const struct cpu_device_id *id_table; struct acpi_cstate *cstates; -}; +} __attribute__ ((aligned (ARCH_VLA_ALIGN_SIZE))); struct cpu_driver *find_cpu_driver(struct device *cpu); diff --git a/src/arch/x86/include/arch/memlayout.h b/src/arch/x86/include/arch/memlayout.h index 83e5b90..fa4db03 100644 --- a/src/arch/x86/include/arch/memlayout.h +++ b/src/arch/x86/include/arch/memlayout.h @@ -28,4 +28,8 @@ # error "CONFIG_RAMTOP not configured" #endif +#ifdef __ARCH_x86_64__ +#define ARCH_VLA_ALIGN_SIZE 32 +#endif + #endif /* __ARCH_MEMLAYOUT_H */ diff --git a/src/include/device/pci.h b/src/include/device/pci.h index f1ab91b..32e79ac 100644 --- a/src/include/device/pci.h +++ b/src/include/device/pci.h @@ -26,6 +26,7 @@ #include <device/device.h> #include <device/pci_ops.h> #include <device/pci_rom.h> +#include <memlayout.h> /* Common pci operations without a standard interface */ @@ -54,7 +55,7 @@ unsigned short vendor; unsigned short device; const unsigned short *devices; -}; +} __attribute__ ((aligned (ARCH_VLA_ALIGN_SIZE))); struct msix_entry { union { diff --git a/src/include/memlayout.h b/src/include/memlayout.h index 5de2370..f02f97d 100644 --- a/src/include/memlayout.h +++ b/src/include/memlayout.h @@ -26,6 +26,11 @@ #define ARCH_POINTER_ALIGN_SIZE 8 #endif +/* System V ABI alignment of Variable length arrays */ +#ifndef ARCH_VLA_ALIGN_SIZE +#define ARCH_VLA_ALIGN_SIZE 16 +#endif + #ifndef ARCH_CACHELINE_ALIGN_SIZE #define ARCH_CACHELINE_ALIGN_SIZE 64 #endif diff --git a/src/lib/program.ld b/src/lib/program.ld index 156b862..4e28958 100644 --- a/src/lib/program.ld +++ b/src/lib/program.ld @@ -56,11 +56,11 @@ _ersbe_init_begin = .; #if ENV_RAMSTAGE - . = ALIGN(ARCH_POINTER_ALIGN_SIZE); + . = ALIGN(ARCH_VLA_ALIGN_SIZE); _pci_drivers = .; KEEP(*(.rodata.pci_driver)); _epci_drivers = .; - . = ALIGN(ARCH_POINTER_ALIGN_SIZE); + . = ALIGN(ARCH_VLA_ALIGN_SIZE); _cpu_drivers = .; KEEP(*(.rodata.cpu_driver)); _ecpu_drivers = .; -- To view, visit https://review.coreboot.org/c/coreboot/+/30116 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2491d47ed03dcfd8db110dfb181b2c5281449591 Gerrit-Change-Number: 30116 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-MessageType: newchange

1 0

Change in ...coreboot[master]: util/cbfstool/cbfs-mkstage: Support x86_64
by Patrick Rudolph (Code Review) 09 Dec '18

09 Dec '18

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30115 Change subject: util/cbfstool/cbfs-mkstage: Support x86_64 ...................................................................... util/cbfstool/cbfs-mkstage: Support x86_64 Add support for relocations on x86_64. Required for 64bit romstage. Change-Id: I1ff223d3476776297b501813a953356dd6fa9d67 Signed-off-by: Patrick Rudolph <siro(a)das-labor.org> --- M util/cbfstool/cbfs-mkstage.c 1 file changed, 7 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/30115/1 diff --git a/util/cbfstool/cbfs-mkstage.c b/util/cbfstool/cbfs-mkstage.c index 85c3e4f..6071437 100644 --- a/util/cbfstool/cbfs-mkstage.c +++ b/util/cbfstool/cbfs-mkstage.c @@ -362,7 +362,9 @@ /* If there is any relocation to the ignored section that isn't * absolute fail as current assumptions are that all relocations * are absolute. */ - if (reloc_type != R_386_32) { + if ((reloc_type != R_386_32) && + (reloc_type != R_AMD64_64) && + (reloc_type != R_AMD64_32)) { ERROR("Invalid reloc to ignored section: %x\n", reloc_type); return -1; } @@ -392,9 +394,10 @@ if (rmodule_init(rmodctx, input)) return -1; - /* Only support x86 XIP currently. */ - if (rmodctx->pelf.ehdr.e_machine != EM_386) { - ERROR("Only support XIP stages for x86\n"); + /* Only support x86 / x86_64 XIP currently. */ + if ((rmodctx->pelf.ehdr.e_machine != EM_386) && + (rmodctx->pelf.ehdr.e_machine != EM_X86_64)) { + ERROR("Only support XIP stages for x86/x86_64\n"); goto out; } -- To view, visit https://review.coreboot.org/c/coreboot/+/30115 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1ff223d3476776297b501813a953356dd6fa9d67 Gerrit-Change-Number: 30115 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

1 0

Change in ...coreboot[master]: arch/x86/assembly_entry: Enable long mode in romstage
by Patrick Rudolph (Code Review) 09 Dec '18

09 Dec '18

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30114 Change subject: arch/x86/assembly_entry: Enable long mode in romstage ...................................................................... arch/x86/assembly_entry: Enable long mode in romstage * Install page tables for long mode * Activate long mode Change-Id: I57974a55f3b778c90b3587f39e86e4eb8692ad48 Signed-off-by: Patrick Rudolph <siro(a)das-labor.org> --- M src/arch/x86/assembly_entry.S 1 file changed, 58 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/30114/1 diff --git a/src/arch/x86/assembly_entry.S b/src/arch/x86/assembly_entry.S index 02f492c..08e693b 100644 --- a/src/arch/x86/assembly_entry.S +++ b/src/arch/x86/assembly_entry.S @@ -26,6 +26,7 @@ * variables that are stage specific. */ .section ".text._start", "ax", @progbits +.code32 .global _start _start: @@ -35,6 +36,15 @@ /* reset stack pointer to CAR stack */ mov $_car_stack_end, %esp +#ifdef __x86_64__ +#if IS_ENABLED(CONFIG_PAGING_IN_CACHE_AS_RAM) + # disable paging, in case page table were in CAR + mov %cr0, %eax + btr $31, %eax + mov %eax, %cr0 +#endif +#endif + /* clear CAR_GLOBAL area as it is not shared */ cld xor %eax, %eax @@ -43,6 +53,54 @@ sub %edi, %ecx rep stosl +#ifdef __x86_64__ +#if IS_ENABLED(CONFIG_PAGING_IN_CACHE_AS_RAM) + /* Setup long mode: identity map 0-4GiB */ + movl $(_pagetables), %ecx + xor %eax, %eax + /* Generate one PM4LE entry */ + lea (0x1000)(%ecx), %edi + or $0x00000623, %edi + mov %edi, (%ecx) + mov %eax, 4(%ecx) + /* Generate 4 PDPE entries */ + lea (0x1000)(%ecx), %edi + movl $0x000000e3, 0x00(%edi) + movl %eax, 0x04(%edi) + movl $0x400000e3, 0x08(%edi) + movl %eax, 0x0c(%edi) + movl $0x800000e3, 0x10(%edi) + movl %eax, 0x14(%edi) + movl $0xc00000e3, 0x18(%edi) + movl %eax, 0x1c(%edi) + + # load ROM based identity mapped page tables + mov %ecx, %eax + mov %eax, %cr3 + + # enable PAE + mov %cr4, %eax + bts $5, %eax + mov %eax, %cr4 + + # enable long mode + mov $0xC0000080, %ecx + rdmsr + bts $8, %eax + wrmsr + + # enable paging + mov %cr0, %eax + bts $31, %eax + mov %eax, %cr0 + + # use call far to switch to 64-bit code segment + ljmp $0x18, $1f +.code64 +1: +#endif +#endif + #if ((ENV_VERSTAGE && IS_ENABLED(CONFIG_VERSTAGE_DEBUG_SPINLOOP)) \ || (ENV_ROMSTAGE && IS_ENABLED(CONFIG_ROMSTAGE_DEBUG_SPINLOOP))) -- 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: 1 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

1 0

Change in ...coreboot[master]: cpu/amd/fam10: Enable relocatable ramstage
by Arthur Heymans (Code Review) 09 Dec '18

09 Dec '18

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30063 ) Change subject: cpu/amd/fam10: Enable relocatable ramstage ...................................................................... Patch Set 2: Code-Review-2 Romstage sets TOP_MEM on AP's. When initialising AP this results in a hang. -- To view, visit https://review.coreboot.org/c/coreboot/+/30063 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6fe4cf797a42621b3a9a3d939cd31d16ebbf9f53 Gerrit-Change-Number: 30063 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 09 Dec 2018 13:31:08 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: src/cpu/intel: Set get_ia32_fsb function common
by build bot (Jenkins) (Code Review) 09 Dec '18

09 Dec '18

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30103 ) Change subject: src/cpu/intel: Set get_ia32_fsb function common ...................................................................... Patch Set 4: (1 comment) https://review.coreboot.org/#/c/30103/4/src/cpu/intel/common/fsb.c File src/cpu/intel/common/fsb.c: https://review.coreboot.org/#/c/30103/4/src/cpu/intel/common/fsb.c@24 PS4, Line 24: static const short core2_fsb[8] = { 266, 133, 200, 166, 333, 100, 400, -1 }; line over 80 characters -- To view, visit https://review.coreboot.org/c/coreboot/+/30103 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I232bf88de7ebba6ac5865db046ce79e9b2f3ed28 Gerrit-Change-Number: 30103 Gerrit-PatchSet: 4 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 09 Dec 2018 12:57:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: src/cpu/intel: Set get_ia32_fsb function common
by HAOUAS Elyes (Code Review) 09 Dec '18

09 Dec '18

Hello Patrick Rudolph, Arthur Heymans, build bot (Jenkins), Patrick Georgi, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/30103 to look at the new patch set (#4). Change subject: src/cpu/intel: Set get_ia32_fsb function common ...................................................................... src/cpu/intel: Set get_ia32_fsb function common get_ia32_fsb returns FSB values in MHz of intel's CPUs. Change-Id: I232bf88de7ebba6ac5865db046ce79e9b2f3ed28 Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr> --- M src/cpu/intel/common/Makefile.inc A src/cpu/intel/common/fsb.c M src/cpu/x86/lapic/apic_timer.c A src/include/cpu/intel/fsb.h 4 files changed, 109 insertions(+), 47 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/30103/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/30103 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I232bf88de7ebba6ac5865db046ce79e9b2f3ed28 Gerrit-Change-Number: 30103 Gerrit-PatchSet: 4 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in ...coreboot[master]: src/cpu/intel: Set get_ia32_fsb function common
by build bot (Jenkins) (Code Review) 09 Dec '18

09 Dec '18

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30103 ) Change subject: src/cpu/intel: Set get_ia32_fsb function common ...................................................................... Patch Set 3: (1 comment) https://review.coreboot.org/#/c/30103/3/src/cpu/intel/common/fsb.c File src/cpu/intel/common/fsb.c: https://review.coreboot.org/#/c/30103/3/src/cpu/intel/common/fsb.c@24 PS3, Line 24: static const short core2_fsb[8] = { 266, 133, 200, 166, 333, 100, 400, -1 }; line over 80 characters -- To view, visit https://review.coreboot.org/c/coreboot/+/30103 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I232bf88de7ebba6ac5865db046ce79e9b2f3ed28 Gerrit-Change-Number: 30103 Gerrit-PatchSet: 3 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 09 Dec 2018 12:15:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: src/cpu/intel: Set get_ia32_fsb function common
by HAOUAS Elyes (Code Review) 09 Dec '18

09 Dec '18

Hello Patrick Rudolph, Arthur Heymans, build bot (Jenkins), Patrick Georgi, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/30103 to look at the new patch set (#3). Change subject: src/cpu/intel: Set get_ia32_fsb function common ...................................................................... src/cpu/intel: Set get_ia32_fsb function common get_ia32_fsb returns FSB values in MHz of intel's CPUs. Change-Id: I232bf88de7ebba6ac5865db046ce79e9b2f3ed28 Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr> --- M src/cpu/intel/common/Makefile.inc A src/cpu/intel/common/fsb.c M src/cpu/x86/lapic/apic_timer.c A src/include/cpu/intel/fsb.h 4 files changed, 108 insertions(+), 47 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/30103/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/30103 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I232bf88de7ebba6ac5865db046ce79e9b2f3ed28 Gerrit-Change-Number: 30103 Gerrit-PatchSet: 3 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in ...coreboot[master]: src/cpu/intel: Set get_ia32_fsb function common
by HAOUAS Elyes (Code Review) 09 Dec '18

09 Dec '18

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30103 ) Change subject: src/cpu/intel: Set get_ia32_fsb function common ...................................................................... Patch Set 2: (3 comments) https://review.coreboot.org/#/c/30103/2/src/cpu/intel/common/fsb.c File src/cpu/intel/common/fsb.c: https://review.coreboot.org/#/c/30103/2/src/cpu/intel/common/fsb.c@54 PS2, Line 54: 0x4 > Do you want to add names here too ? I think it is not useful to add "Netburst Model 4" here. (same for the other cases) https://review.coreboot.org/#/c/30103/2/src/cpu/x86/lapic/apic_timer.c File src/cpu/x86/lapic/apic_timer.c: https://review.coreboot.org/#/c/30103/2/src/cpu/x86/lapic/apic_timer.c@46 PS2, Line 46: static int set_timer_fsb(void) : { : car_set_var(g_timer_fsb, get_ia32_fsb()); : return 0; > return value must depend on whether g_timer_fsb is sane. thank you so much for the review. I'll remove "return 0". https://review.coreboot.org/#/c/30103/2/src/include/cpu/intel/fsb.h File src/include/cpu/intel/fsb.h: https://review.coreboot.org/#/c/30103/2/src/include/cpu/intel/fsb.h@17 PS2, Line 17: get_ia32_fsb(void) > Add description. […] Ok, I'll do. thank you so much for the review -- To view, visit https://review.coreboot.org/c/coreboot/+/30103 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I232bf88de7ebba6ac5865db046ce79e9b2f3ed28 Gerrit-Change-Number: 30103 Gerrit-PatchSet: 2 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 09 Dec 2018 11:36:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <siro(a)das-labor.org> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit December 2018