coreboot July 2016

coreboot@coreboot.org

47 participants
135 discussions

Re: [coreboot] What is the best way to treat warnings reported by checkpatch.pl
by ron minnich 27 Jul '16

27 Jul '16

On Tue, Jul 26, 2016 at 9:25 PM Zeh, Werner <werner.zeh(a)siemens.com> wrote: > > > > In the case of ACPI we need to provide a table which has constrains on the > used data types and alignment > as the contents of the table will be interpreted by the ACPI interpreter > of the OS. > So if we omit the usage of packed it may result in gaps in between the > single members of the data structure > which will in turn lead to errors while the OS interprets the contents. > > So in my point of view, we really need this structure to be packed in this > case. > > > no, please don't do this. We and others have made this mistake before and it took a lot of work to unwind it where we did it. Using packed structs to create aligned data in memory is a frequent source of bugs. If you're creating a desired data layout in memory, from a struct, using packed will fail badly should we ever have a big endian machine, but it can even have weird problems between gcc versions. When you pack data into memory with a specified alignment, byte order, and padding, you are converting it to an external data representation, i.e. XDR. You need to do that explicitly, not by using packed structs. We've got code in the repo which does this. Please look at util/cbfstool/ for example code, or see the mptable generation code. just git grep xdr to see some usage. But using packed structs to try to force layout of data in memory is almost always going to end badly. For a fun read, check this out https://sourceware.org/bugzilla/show_bug.cgi?id=5070 This one section is applicable: "Tom Hunter 2007-10-02 05:45:39 UTC ARM is one of the architectures supported by glibc. You may not like it, but it is a fact. Independently of the architecture, the padding done is not valid. You can't (and shouldn't) make any assumption about the alignment and associated padding done by the compiler for any architecture. GCC is free to change the alignment rules in any future versions. It seems rather silly to have the assert() which is meant to verify at runtime that your invalid assumption holds true. I would also suggest that you don't use structures to format packets for networking. Packets for networking should be treated as byte streams to avoid any alignment/padding/byte-order issues. A standard way of doing this is something like this: unsigned char buf[MaxPacketSize]; unsigned char *bp; int skt; size_t len; ... bp = buf; *bp++ = ...; *bp++ = ...; *bp++ = ...; ... len = send(skt, buf, bp - buf, 0); " The key realization is that Tom is right even when you're packing to memory. ron

1 0

Re: [coreboot] Where is the first instrucion?
by Rafael Machado 27 Jul '16

27 Jul '16

Hi Zoran, thanks a lot for expending your time to help! I'll take a look. (It may take some time, but I will! ) Rafael R. Machado Em qua, 27 de jul de 2016 às 08:17, Zoran Stojsavljevic < zoran.stojsavljevic(a)gmail.com> escreveu: > Hello Rafael, > > I am looking into disassembly pointed by you: > > > f000:0fcb 66b9ff020000 mov ecx, 0x2ff > > f000:0fd1 0f32 rdmsr ; read register > 0x2ff (IA32_MTRR_DEF_TYPE) > > f000:0fd3 0fbae80b bts ax, 0xb ; Enable bit 11 (MTRR > Enable). > > f000:0fd7 0fbae80a bts ax, 0xa ; Enable bit 10 (Fixed > MTRR Enable). > > f000:0fdb 0f30 wrmsr ; Write changes to > MTRR > > f000:0fdd 0f20c0 mov eax, cr0 > > f000:0fe0 660fbaf01e btr eax, 0x1e ; Bit 30 means CD - Cache > disabled. > > f000:0fe5 660fbaf01d btr eax, 0x1d ; Disable bit 29. NW - No > Write-through > > f000:0fea 0f22c0 mov cr0, eax ; Write changes to CR0 > > f000:0fed ffe7 jmp di > > f000:0fef 0f20c0 mov eax, cr0 > > f000:0ff2 660fbae81e bts eax, 0x1e > > f000:0ff7 660fbae81d bts eax, 0x1d > > f000:0ffc 0f22c0 mov cr0, eax > > Looks very similar like sequence in Coreboot ROMSTAGE ( > src/cpu/intel/car/cache_as_ram.inc): > > /* We don't need CAR from now on. */ > > /* Disable cache. */ > movl %cr0, %eax > orl $CR0_CacheDisable, %eax > movl %eax, %cr0 > > /* Clear sth. */ > movl $MTRR_FIX_4K_C8000, %ecx > xorl %edx, %edx > xorl %eax, %eax > wrmsr > > #if CONFIG_DCACHE_RAM_SIZE > 0x8000 > movl $MTRR_FIX_4K_C0000, %ecx > wrmsr > #endif > > /* > * Set the default memory type and disable fixed > * and enable variable MTRRs. > */ > movl $MTRR_DEF_TYPE_MSR, %ecx > xorl %edx, %edx > movl $MTRR_DEF_TYPE_EN, %eax /* Enable variable and disable fixed MTRRs. > */ > wrmsr > > /* Enable cache. */ > movl %cr0, %eax > andl $(~(CR0_CacheDisable | CR0_NoWriteThrough)), %eax > movl %eax, %cr0 > > __main: > > Please, check upon it. Please, pass to us your comments... If any? > > Best Regards, > Zoran > > On Tue, Jul 26, 2016 at 4:23 AM, Rafael Machado < > rafaelrodrigues.machado(a)gmail.com> wrote: > >> Thanks a lot Zoran. >> I will! >> >> Rafael >> >> Em seg, 25 de jul de 2016 14:13, Zoran Stojsavljevic < >> zoran.stojsavljevic(a)gmail.com> escreveu: >> >>> Hello Rafael, >>> >>> Let me try hard... ;-) >>> >>> Let us look into what actually we have here, in Coreboot: in bootblock >>> phase, at the very beginning. >>> Let me tell you what I am looking into (what cb tree): [zoran@localhost >>> coreboot-09.06.2016]$ git describe<CR> >>> 4.4-455-g538b324 >>> >>> Let us backtrace, to understand what is actual thread of execution: >>> src/arch/x86/prologue.inc >>> src/cpu/x86/16bit/entry16.inc >>> src/cpu/x86/16bit/reset16.inc >>> src/cpu/x86/32bit/entry32.inc >>> src/cpu/x86/sse_enable.inc >>> src/arch/x86/bootblock_simple.c >>> >>> Please, carefully examine what I pointed/presented here... And let us >>> know your thoughts. >>> >>> Best Regards, >>> Zoran >>> >>> On Mon, Jul 25, 2016 at 6:03 PM, Rafael Machado < >>> rafaelrodrigues.machado(a)gmail.com> wrote: >>> >>>> Hi guys. Long time since my last e-mail. >>>> >>>> It's hard to synchronize my day work with my firmware studies. Since my >>>> projects are more UEFI related I usually do not have to much time to study >>>> the legacy way, but It's really cool and Ill not give up :) >>>> >>>> Since the last talk I was doing what everyone kindly proposed. (by the >>>> way thank you all for the guidance.) >>>> >>>> Now I'm disassembly an old systems bios I have, but I cannot understand >>>> what is happening in a specific section of the code. (I'm using radare2 for >>>> my studies) >>>> >>>> The code is: >>>> >>>> f000:0fcb 66b9ff020000 mov ecx, 0x2ff >>>> f000:0fd1 0f32 rdmsr ; read register >>>> 0x2ff (IA32_MTRR_DEF_TYPE) >>>> f000:0fd3 0fbae80b bts ax, 0xb ; Enable bit 11 (MTRR >>>> Enable). >>>> f000:0fd7 0fbae80a bts ax, 0xa ; Enable bit 10 (Fixed >>>> MTRR Enable). >>>> f000:0fdb 0f30 wrmsr ; Write changes to >>>> MTRR >>>> f000:0fdd 0f20c0 mov eax, cr0 >>>> f000:0fe0 660fbaf01e btr eax, 0x1e ; Bit 30 means CD - >>>> Cache disabled. >>>> f000:0fe5 660fbaf01d btr eax, 0x1d ; Disable bit 29. NW - >>>> No Write-through >>>> f000:0fea 0f22c0 mov cr0, eax ; Write changes to CR0 >>>> f000:0fed ffe7 jmp di >>>> f000:0fef 0f20c0 mov eax, cr0 >>>> f000:0ff2 660fbae81e bts eax, 0x1e >>>> f000:0ff7 660fbae81d bts eax, 0x1d >>>> f000:0ffc 0f22c0 mov cr0, eax >>>> >>>> >>>> Here is the code with my notes. I understand that some MTRR were set, >>>> and now the processor will be "configured". >>>> We see at address f000:0fe0 and f000:0fe5 that the CR0 register is >>>> being changed and after that the changes are saved. >>>> >>>> Now I have two questions. >>>> >>>> 1 - After CR0 changes get completed there is a "jmp di" instruction. >>>> This does not make any sense to me. Does anyone know why this is >>>> needed ? As far as I could check di value is 0x0 at this point. I think >>>> >>>> 2 - After the "jmp di" a "CR0 Déjà vu" code is executed. Any idea why >>>> this is needed ? >>>> >>>> Thanks everyone >>>> Rafael R. Machado >>>> >>>> >>>> Em seg, 11 de jan de 2016 às 03:57, Alex G. <mr.nuke.me(a)gmail.com> >>>> escreveu: >>>> >>>>> On 01/10/2016 10:23 AM, ron minnich wrote: >>>>> > One thing I think you'd enjoy doing is building the qemu target, >>>>> setting >>>>> > up qemu with gdb, and just watching what happens, instruction by >>>>> > instruction, as the system boots. >>>>> >>>>> One exercise I liked doing was to rewrite the entire boot flow, from >>>>> reset vector to protected mode entry. Tested on qemu, put it on >>>>> hardware, nothing burned. >>>>> >>>>> Alex >>>>> >>>>> > ron >>>>> > >>>>> > On Sun, Jan 10, 2016 at 3:28 AM Rafael Machado >>>>> > <rafaelrodrigues.machado(a)gmail.com >>>>> > <mailto:rafaelrodrigues.machado@gmail.com>> wrote: >>>>> > >>>>> > Hi Peter and Rudolf. >>>>> > Thanks for the answers and tips. They are realy helpfull ! >>>>> > I'll take a look. >>>>> > >>>>> > Rafael R. Machado >>>>> > >>>>> > >>>>> > Em Sáb, 9 de jan de 2016 17:19, Rudolf Marek < >>>>> r.marek(a)assembler.cz >>>>> > <mailto:r.marek@assembler.cz>> escreveu: >>>>> > >>>>> > Hi, >>>>> > >>>>> > I guess your question is more general than the coreboot >>>>> related >>>>> > right? >>>>> > >>>>> > If you have a firmware image dump of the flash (not the file >>>>> you >>>>> > download from >>>>> > board vendor) then yes, first location to be executed is the >>>>> > instruction located >>>>> > 16 bytes before end of the image. >>>>> > >>>>> > In coreboot see in build/ bootblock_inc.S which also has >>>>> > reset16.inc and >>>>> > entry16.inc which is a real start. Consult the Intel or AMD >>>>> > manual to see the >>>>> > CPU state after reset. The CPU starts in real mode, but CS >>>>> base >>>>> > is shifted to >>>>> > last 64KB before end of 4GB address space. In general your >>>>> CPU >>>>> > starts in >>>>> > compatible mode with 8086 manufactured in 1978. >>>>> > >>>>> > Thanks >>>>> > Rudolf >>>>> > >>>>> > -- >>>>> > coreboot mailing list: coreboot(a)coreboot.org >>>>> > <mailto:coreboot@coreboot.org> >>>>> > http://www.coreboot.org/mailman/listinfo/coreboot >>>>> > >>>>> > >>>>> > >>>>> >>>>> -- >>>>> coreboot mailing list: coreboot(a)coreboot.org >>>>> http://www.coreboot.org/mailman/listinfo/coreboot >>>>> >>>> >>>> -- >>>> coreboot mailing list: coreboot(a)coreboot.org >>>> >>> https://www.coreboot.org/mailman/listinfo/coreboot >>>> >>> >>> >

1 0

Re: [coreboot] Where is the first instrucion?
by Zoran Stojsavljevic 27 Jul '16

27 Jul '16

Hello Rafael, I am looking into disassembly pointed by you: > f000:0fcb 66b9ff020000 mov ecx, 0x2ff > f000:0fd1 0f32 rdmsr ; read register 0x2ff (IA32_MTRR_DEF_TYPE) > f000:0fd3 0fbae80b bts ax, 0xb ; Enable bit 11 (MTRR Enable). > f000:0fd7 0fbae80a bts ax, 0xa ; Enable bit 10 (Fixed MTRR Enable). > f000:0fdb 0f30 wrmsr ; Write changes to MTRR > f000:0fdd 0f20c0 mov eax, cr0 > f000:0fe0 660fbaf01e btr eax, 0x1e ; Bit 30 means CD - Cache disabled. > f000:0fe5 660fbaf01d btr eax, 0x1d ; Disable bit 29. NW - No Write-through > f000:0fea 0f22c0 mov cr0, eax ; Write changes to CR0 > f000:0fed ffe7 jmp di > f000:0fef 0f20c0 mov eax, cr0 > f000:0ff2 660fbae81e bts eax, 0x1e > f000:0ff7 660fbae81d bts eax, 0x1d > f000:0ffc 0f22c0 mov cr0, eax Looks very similar like sequence in Coreboot ROMSTAGE ( src/cpu/intel/car/cache_as_ram.inc): /* We don't need CAR from now on. */ /* Disable cache. */ movl %cr0, %eax orl $CR0_CacheDisable, %eax movl %eax, %cr0 /* Clear sth. */ movl $MTRR_FIX_4K_C8000, %ecx xorl %edx, %edx xorl %eax, %eax wrmsr #if CONFIG_DCACHE_RAM_SIZE > 0x8000 movl $MTRR_FIX_4K_C0000, %ecx wrmsr #endif /* * Set the default memory type and disable fixed * and enable variable MTRRs. */ movl $MTRR_DEF_TYPE_MSR, %ecx xorl %edx, %edx movl $MTRR_DEF_TYPE_EN, %eax /* Enable variable and disable fixed MTRRs. */ wrmsr /* Enable cache. */ movl %cr0, %eax andl $(~(CR0_CacheDisable | CR0_NoWriteThrough)), %eax movl %eax, %cr0 __main: Please, check upon it. Please, pass to us your comments... If any? Best Regards, Zoran On Tue, Jul 26, 2016 at 4:23 AM, Rafael Machado < rafaelrodrigues.machado(a)gmail.com> wrote: > Thanks a lot Zoran. > I will! > > Rafael > > Em seg, 25 de jul de 2016 14:13, Zoran Stojsavljevic < > zoran.stojsavljevic(a)gmail.com> escreveu: > >> Hello Rafael, >> >> Let me try hard... ;-) >> >> Let us look into what actually we have here, in Coreboot: in bootblock >> phase, at the very beginning. >> Let me tell you what I am looking into (what cb tree): [zoran@localhost >> coreboot-09.06.2016]$ git describe<CR> >> 4.4-455-g538b324 >> >> Let us backtrace, to understand what is actual thread of execution: >> src/arch/x86/prologue.inc >> src/cpu/x86/16bit/entry16.inc >> src/cpu/x86/16bit/reset16.inc >> src/cpu/x86/32bit/entry32.inc >> src/cpu/x86/sse_enable.inc >> src/arch/x86/bootblock_simple.c >> >> Please, carefully examine what I pointed/presented here... And let us >> know your thoughts. >> >> Best Regards, >> Zoran >> >> On Mon, Jul 25, 2016 at 6:03 PM, Rafael Machado < >> rafaelrodrigues.machado(a)gmail.com> wrote: >> >>> Hi guys. Long time since my last e-mail. >>> >>> It's hard to synchronize my day work with my firmware studies. Since my >>> projects are more UEFI related I usually do not have to much time to study >>> the legacy way, but It's really cool and Ill not give up :) >>> >>> Since the last talk I was doing what everyone kindly proposed. (by the >>> way thank you all for the guidance.) >>> >>> Now I'm disassembly an old systems bios I have, but I cannot understand >>> what is happening in a specific section of the code. (I'm using radare2 for >>> my studies) >>> >>> The code is: >>> >>> f000:0fcb 66b9ff020000 mov ecx, 0x2ff >>> f000:0fd1 0f32 rdmsr ; read register >>> 0x2ff (IA32_MTRR_DEF_TYPE) >>> f000:0fd3 0fbae80b bts ax, 0xb ; Enable bit 11 (MTRR >>> Enable). >>> f000:0fd7 0fbae80a bts ax, 0xa ; Enable bit 10 (Fixed >>> MTRR Enable). >>> f000:0fdb 0f30 wrmsr ; Write changes to >>> MTRR >>> f000:0fdd 0f20c0 mov eax, cr0 >>> f000:0fe0 660fbaf01e btr eax, 0x1e ; Bit 30 means CD - Cache >>> disabled. >>> f000:0fe5 660fbaf01d btr eax, 0x1d ; Disable bit 29. NW - No >>> Write-through >>> f000:0fea 0f22c0 mov cr0, eax ; Write changes to CR0 >>> f000:0fed ffe7 jmp di >>> f000:0fef 0f20c0 mov eax, cr0 >>> f000:0ff2 660fbae81e bts eax, 0x1e >>> f000:0ff7 660fbae81d bts eax, 0x1d >>> f000:0ffc 0f22c0 mov cr0, eax >>> >>> >>> Here is the code with my notes. I understand that some MTRR were set, >>> and now the processor will be "configured". >>> We see at address f000:0fe0 and f000:0fe5 that the CR0 register is >>> being changed and after that the changes are saved. >>> >>> Now I have two questions. >>> >>> 1 - After CR0 changes get completed there is a "jmp di" instruction. >>> This does not make any sense to me. Does anyone know why this is needed >>> ? As far as I could check di value is 0x0 at this point. I think >>> >>> 2 - After the "jmp di" a "CR0 Déjà vu" code is executed. Any idea why >>> this is needed ? >>> >>> Thanks everyone >>> Rafael R. Machado >>> >>> >>> Em seg, 11 de jan de 2016 às 03:57, Alex G. <mr.nuke.me(a)gmail.com> >>> escreveu: >>> >>>> On 01/10/2016 10:23 AM, ron minnich wrote: >>>> > One thing I think you'd enjoy doing is building the qemu target, >>>> setting >>>> > up qemu with gdb, and just watching what happens, instruction by >>>> > instruction, as the system boots. >>>> >>>> One exercise I liked doing was to rewrite the entire boot flow, from >>>> reset vector to protected mode entry. Tested on qemu, put it on >>>> hardware, nothing burned. >>>> >>>> Alex >>>> >>>> > ron >>>> > >>>> > On Sun, Jan 10, 2016 at 3:28 AM Rafael Machado >>>> > <rafaelrodrigues.machado(a)gmail.com >>>> > <mailto:rafaelrodrigues.machado@gmail.com>> wrote: >>>> > >>>> > Hi Peter and Rudolf. >>>> > Thanks for the answers and tips. They are realy helpfull ! >>>> > I'll take a look. >>>> > >>>> > Rafael R. Machado >>>> > >>>> > >>>> > Em Sáb, 9 de jan de 2016 17:19, Rudolf Marek < >>>> r.marek(a)assembler.cz >>>> > <mailto:r.marek@assembler.cz>> escreveu: >>>> > >>>> > Hi, >>>> > >>>> > I guess your question is more general than the coreboot >>>> related >>>> > right? >>>> > >>>> > If you have a firmware image dump of the flash (not the file >>>> you >>>> > download from >>>> > board vendor) then yes, first location to be executed is the >>>> > instruction located >>>> > 16 bytes before end of the image. >>>> > >>>> > In coreboot see in build/ bootblock_inc.S which also has >>>> > reset16.inc and >>>> > entry16.inc which is a real start. Consult the Intel or AMD >>>> > manual to see the >>>> > CPU state after reset. The CPU starts in real mode, but CS >>>> base >>>> > is shifted to >>>> > last 64KB before end of 4GB address space. In general your CPU >>>> > starts in >>>> > compatible mode with 8086 manufactured in 1978. >>>> > >>>> > Thanks >>>> > Rudolf >>>> > >>>> > -- >>>> > coreboot mailing list: coreboot(a)coreboot.org >>>> > <mailto:coreboot@coreboot.org> >>>> > http://www.coreboot.org/mailman/listinfo/coreboot >>>> > >>>> > >>>> > >>>> >>>> -- >>>> coreboot mailing list: coreboot(a)coreboot.org >>>> http://www.coreboot.org/mailman/listinfo/coreboot >>>> >>> >>> -- >>> coreboot mailing list: coreboot(a)coreboot.org >>> >> https://www.coreboot.org/mailman/listinfo/coreboot >>> >> >>

1 0

WG: What is the best way to treat warnings reported by checkpatch.pl
by Zeh, Werner 27 Jul '16

27 Jul '16

>Does git commit --no-verify (or -n for short) allow you to commit? > Yes, one can go this way and I did it already earlier. But I just wanted to point it out here. We do not need a check script for every commit if we simply disable the check when it comes to issues. I wanted to start this discussion to improve the situation with this script we currently have. >I think we should try to do a little of option 1 within reason, not by forking the script but rather by disabling more check steps that don't match the coreboot code style (by >having our wrapper pass --ignore XXX flags to checkpatch) and possibly upstreaming checkpatch.pl patches with new features we need to the Linux kernel (to make the >detection more accurate or add a more specific --ignore flag we can turn on). In the Chromium fork of coreboot we're already using a bunch of those flags that we should >probably use in upstream coreboot as well: > >--ignore C99_COMMENTS >--ignore GLOBAL_INITIALISERS >--ignore INITIALISED_STATIC >--ignore LINE_SPACING >--ignore NEW_TYPEDEFS >--ignore PREFER_ALIGNED >--ignore PREFER_PACKED >--ignore PREFER_PRINTF >--ignore SPLIT_STRING > If we really did not touch the contents of the script, I totally agree with you. We can disable warnings that do not match our coding style while updating the script from its origin from time to time. If we have already started tailoring this script, than we should do it the right way and end this tailoring process to match our needs. I admit that the later one will end up in more work if one wants to synchronize this script with origin again one day. Werner

1 0

Re: [coreboot] What is the best way to treat warnings reported by checkpatch.pl
by Stefan Reinauer 27 Jul '16

27 Jul '16

* ron minnich <rminnich(a)gmail.com> [160727 02:26]: > A couple of questions preceded by the usual curmudgeonly comment :-) > > I'm not a big fan of checkpatch.pl. It's 5965 lines of dense perl spaghetti > code, continues to grow, and is attempting to achieve that which I understand > may be impossible: parsing C with REs and random blobs of PERL. Given that the > problem may be impossible, checkpatch finds it hard, and makes mistakes, as > experienced on the projects that use it. It also finds a lot of inconsistencies in our code base, which are (arguably) worth fixing. > It's not surprising it continues to grow, and not surprising it's got lots of > errors. You can find discussions on the various lists which boil down to "just > ignore checkpatch.pl in this case." This is not confidence-inspiring. I didn't see any errors reported since its use in coreboot, besides the current discussion of whether it is the right thing to abstract out gcc __attribute__(()) qualifiers or not. > I'd most prefer to move to clang-format for this sort of thing, which is what > we're doing on at least one other kernel project I work on. clang-format's scope is very different. It copes with whitespace, more like indent. > I continue to not understand the injunction on typedefs; operating systems > written by the inventors of C continued to use them until Bell Labs disbanded > the OS group in 2015 ... Agreed. > Finally, why the use of packed? In general you really only want to use packed > when you're using it to directly address data. Why not deserialize the data > from memory into an unpacked struct? That's how ACPICA seems to do it. Because in most cases, it is not data, but register space we are accessing. Are you suggesting to learn our coding style from ACPI? Who are you and what did you do to Ron Minnich? ;) > thanks > > ron Stefan

2 1

Re: [coreboot] Microcode problem with Braswell CPU
by Zoran Stojsavljevic 27 Jul '16

27 Jul '16

Hello Cheng, What I am getting from your emails and the net is the following: CPUID 0x406C3 => 6 - Family, C - Model, Stepping - 3 (Si = C0) Following the article I have posted before: http://www.anandtech.com/show/9806/intel-introduces-new-braswell-stepping-w… : It is obvious that INTEL introduced new stepping Dx. In other words, CPUID 0x406C4 is stepping Dx (not sure which number x is for stepping 4). Certainly you have to have different MCUs for the different stepping. Now, I see the following from your last email: 2. In Coreboot I use microcode version M01406C440A N3060/N3160/x5-e800. version M01406C3363 for N3150. Here, you are correct, your N3150 is stepping C0, CPUID 0x406C3, MCU 0x363 N3060/N3160/x5-e800, they are all stepping Dx, CPUID 0x406C4, MCU 0x40A Only N3060 boots, and this is the lowest class (celeron) sku, maybe here there is a crucial difference why other skus do not boot. *Other things to be of significant importance is how many channels and which DDR3 memory you are using on your boards?* *POST 0x52 (maybe?) suggests problems with MRC?!* Best Regards, Zoran On Tue, Jul 26, 2016 at 5:11 PM, cheng yichen <blessyichen(a)gmail.com> wrote: > HI Zoran > > 1Cherry Hill CRB CPU is N3150(cpuid is 406c3) > 2.in coreboot I use microcode version M01406C440A N3060/N3160/x5-e800. > version M01406C3363 for N3150. > > > 2016-07-26 22:29 GMT+08:00 Zoran Stojsavljevic < > zoran.stojsavljevic(a)gmail.com>: > >> > The issue can't be duplicated in cherryhill CRB. >> >> Hello Cheng, >> >> What BSW SoC type (N3xxx), which CPUID, and which stepping do you have in >> Cherry Hill CRB (IOTG one, seems N3060)? And also what MCU do you have >> there?? >> >> Stepping is (at this point in time) very important (since you have to >> have correct MCU matching stepping)... I am not saying that this will >> anyhow solve the problem?! >> >> You can back-port IOTG BIOS, and read BIOS System Information page to >> find these info. And you (also, for sure) could open in Sales Force case >> #.... It will certainly put more >> pressure on INTEL IOTG support, so they'll try to cope with the situation >> (although two different GEOs)... .. . >> >> Zoran >> >> On Tue, Jul 26, 2016 at 10:48 AM, cheng yichen <blessyichen(a)gmail.com> >> wrote: >> >>> Hi all >>> I have the same issue and still can't solve it. I test those CPUs in my >>> mainboard and only N3060 is workable with FSP. >>> I confuse why the same cpuid have different result. The issue can't be >>> duplicated in cherryhill CRB. >>> >>> >>> >>> >>> N3060(cpuid:406C4) : boot successfully >>> N3160(cpuid:406C4) : hang up in check point 0x52 >>> N3150(cpuid:406C3) : hang up in check point 0x52 >>> x5-e8000(cpuid:406c4): hang up in check point 0x52 >>> >>> 2016-07-26 15:27 GMT+08:00 Zoran Stojsavljevic < >>> zoran.stojsavljevic(a)gmail.com>: >>> >>>> Hello Alex, >>>> >>>> I am not actively working for INTEL anymore (as my Linkedin profile >>>> suggests). For couple more months, my written agreement with them will come >>>> to the end, since we have some agreement in place for quite a while. I'll >>>> update my profile with the end date accordingly, when time comes. Here and >>>> everywhere else on the net, I speak only out of my IT experience/myself, so >>>> this has nothing to do with INTEL. In other words, opinions I write here >>>> are strictly mine, based on open net, open source, white paper documents >>>> and public data from INTEL, AMD and other companies. >>>> >>>> There are other INTEL people watching this thread, so they might >>>> expedite your IPS/Sales Force case #. Good luck with that. >>>> >>>> ATOM released wise, I was not too much involved with BSW (much more >>>> with BYT), so I have no idea if this what support suggested is correct, but >>>> it is (at least) worth trying. >>>> >>>> Please, do note the following: >>>> http://www.anandtech.com/show/9806/intel-introduces-new-braswell-stepping-w… >>>> >>>> Namely (excerpt from the article): *The Braswell update is a new >>>> stepping which adjusts the power consumption of the cores, raising the >>>> frequency, raising the TDP of the Pentium variants for a larger product >>>> separation, and renaming both the processor itself and the HD Graphics >>>> implementation. This change is referred to in the documentation as moving >>>> from the C-stepping to the D-stepping, which typically co-incides with a >>>> change in the way these processors are made (adjusted metal layer >>>> arrangement or lithography mask update).* >>>> >>>> Not sure how many D steppings are out there, you should ask/verify with >>>> support. >>>> >>>> I myself now inspected ./src/include/console/post_codes.h, and there >>>> is no 0x52 post code per say. This is why I asked several times PED FSP >>>> team to update/document non existent FSP post codes, so you all >>>> Coreboot-ers can have more clear picture what is going on with FSP boot, >>>> stages wise. :-) >>>> >>>> Considering the latest you wrote, there are two files you also need to >>>> inspect: >>>> src/cpu/intel/microcode/microcode.c >>>> src/include/cpu/intel/microcode.h >>>> >>>> Sincerely hope (some of) this helps, >>>> Zoran >>>> >>>> On Tue, Jul 26, 2016 at 8:04 AM, Alexander Böcken < >>>> Alexander.Boecken(a)junger-audio.com> wrote: >>>> >>>>> Hi Zoran, >>>>> >>>>> >>>>> >>>>> thanks for checking back. I’m still on the issue (next to some other >>>>> things), but haven’t made any progress yet. I also opened up a case at >>>>> Intel Premier Support and tried to follow their suggestions (Case 00053422). >>>>> >>>>> >>>>> >>>>> Anyway, I know the post_codes.h file. It defines >>>>> POST_FSP_TEMP_RAM_INIT (0x90) which is the post code shown by coreboot just >>>>> before it calls TempRamInit. Then TempRamInit shows 0x52. Intel suggested >>>>> that this is a microcode problem (i.e. the microcode doesn’t match the CPU >>>>> stepping or platform), however, I’m pretty sure that this is not the case. >>>>> At least I’ve taken a look at the CPUID signature (which is 0x406C4) and >>>>> the microcode header signature (which is 0x406C4). I also compared the >>>>> platform ID bits from MSR 0x17 (which are 000, i.e. 1 << 000 = 1) with the >>>>> platform ID field of the microcode (which is also 1). The microcode update >>>>> facilities are documented in Intel’s System Programming Guide (#325384). >>>>> >>>>> >>>>> >>>>> I’m currently checking if coreboot is able to update the microcode >>>>> while still in bootblock. There is a call to >>>>> intel_update_microcode_from_cbfs() in >>>>> /src/soc/intel/braswell/bootblock/bootblock.c. Maybe, there is something >>>>> sticking out… >>>>> >>>>> >>>>> >>>>> Regards, >>>>> >>>>> Alex >>>>> >>>>> >>>>> >>>>> *Von:* Zoran Stojsavljevic [mailto:zoran.stojsavljevic@gmail.com] >>>>> *Gesendet:* Montag, 25. Juli 2016 22:08 >>>>> *An:* Alexander Böcken >>>>> *Cc:* coreboot(a)coreboot.org; york.yang(a)intel.com >>>>> *Betreff:* Re: [coreboot] Microcode problem with Braswell CPU >>>>> >>>>> >>>>> >>>>> Hello Alex, >>>>> >>>>> >>>>> >>>>> It is awhile... Opportunity just did struck (suddenly/plotzlich), so I >>>>> am back! >>>>> >>>>> >>>>> >>>>> While lurking around in Coreboot, trying to solve some "Mystery of >>>>> digital Orga.ni.sms", I ran into very interesting file: >>>>> >>>>> ./src/include/console/post_codes.h >>>>> >>>>> >>>>> >>>>> Coreboot tree I am using: [zoran@localhost coreboot-09.06.2016]$ git >>>>> describe<CR> >>>>> >>>>> 4.4-455-g538b324 >>>>> >>>>> >>>>> >>>>> Maybe, it is worth looking into it. You tell us? >>>>> >>>>> >>>>> >>>>> Zoran >>>>> >>>>> >>>>> >>>>> On Tue, May 3, 2016 at 10:28 AM, Alexander Böcken < >>>>> Alexander.Boecken(a)junger-audio.com> wrote: >>>>> >>>>> Hello Zoran, >>>>> >>>>> again, thanks for your clues to this problem. I don't think post code >>>>> 0x52 is about memory configuration. The post code appears when I call >>>>> TempRamInit which is supposed to enable Cache-as-RAM. Real memory is >>>>> initialized at a later call to FspMemoryInit. coreboot supplies the >>>>> location of the microcode and a cachable region to TempRamInit. >>>>> Additionally, there are some settings that can be applied to the FSP image >>>>> with Intel's Binary Configuration Tool. I don't know if these are used >>>>> during TempRamInit, but I'll try and fiddle around with them. >>>>> >>>>> I agree, it would be helpful to have a list of post codes that can be >>>>> output by FSP. Otherwise it's all speculation as what is wrong. >>>>> >>>>> Regards, >>>>> Alex >>>>> >>>>> >>>>> >>>> >>>> >>>> -- >>>> coreboot mailing list: coreboot(a)coreboot.org >>>> https://www.coreboot.org/mailman/listinfo/coreboot >>>> >>> >>> >> >

1 0

Re: [coreboot] What is the best way to treat warnings reported by checkpatch.pl
by ron minnich 27 Jul '16

27 Jul '16

A couple of questions preceded by the usual curmudgeonly comment :-) I'm not a big fan of checkpatch.pl. It's 5965 lines of dense perl spaghetti code, continues to grow, and is attempting to achieve that which I understand may be impossible: parsing C with REs and random blobs of PERL. Given that the problem may be impossible, checkpatch finds it hard, and makes mistakes, as experienced on the projects that use it. It's not surprising it continues to grow, and not surprising it's got lots of errors. You can find discussions on the various lists which boil down to "just ignore checkpatch.pl in this case." This is not confidence-inspiring. I'd most prefer to move to clang-format for this sort of thing, which is what we're doing on at least one other kernel project I work on. I continue to not understand the injunction on typedefs; operating systems written by the inventors of C continued to use them until Bell Labs disbanded the OS group in 2015 ... Finally, why the use of packed? In general you really only want to use packed when you're using it to directly address data. Why not deserialize the data from memory into an unpacked struct? That's how ACPICA seems to do it. thanks ron On Tue, Jul 26, 2016 at 5:05 PM Julius Werner <jwerner(a)chromium.org> wrote: > Does git commit --no-verify (or -n for short) allow you to commit? > > I think we should try to do a little of option 1 within reason, not by > forking the script but rather by disabling more check steps that don't > match the coreboot code style (by having our wrapper pass --ignore XXX > flags to checkpatch) and possibly upstreaming checkpatch.pl patches > with new features we need to the Linux kernel (to make the detection > more accurate or add a more specific --ignore flag we can turn on). In > the Chromium fork of coreboot we're already using a bunch of those > flags that we should probably use in upstream coreboot as well: > > --ignore C99_COMMENTS > --ignore GLOBAL_INITIALISERS > --ignore INITIALISED_STATIC > --ignore LINE_SPACING > --ignore NEW_TYPEDEFS > --ignore PREFER_ALIGNED > --ignore PREFER_PACKED > --ignore PREFER_PRINTF > --ignore SPLIT_STRING > > However, in my experience there will always be edge cases checkpatch > doesn't get right, no matter how well you tweak it. It should always > be up to the discretion of the committer/reviewer which warnings they > act upon, so we need to retain a mechanism (like --no-verify) to work > around it if necessary. Maybe we should update the pre-commit hook to > print an explanation to this effect. (Ideally, I think it would be > awesome if Gerrit could separately run checkpatch and make it explicit > in the interface if someone circumvented a warning this way so it can > be discussed, but that's probably not and easy thing to add.) > > -- > coreboot mailing list: coreboot(a)coreboot.org > https://www.coreboot.org/mailman/listinfo/coreboot >

1 0

Re: [coreboot] What is the best way to treat warnings reported by checkpatch.pl
by Julius Werner 27 Jul '16

27 Jul '16

Does git commit --no-verify (or -n for short) allow you to commit? I think we should try to do a little of option 1 within reason, not by forking the script but rather by disabling more check steps that don't match the coreboot code style (by having our wrapper pass --ignore XXX flags to checkpatch) and possibly upstreaming checkpatch.pl patches with new features we need to the Linux kernel (to make the detection more accurate or add a more specific --ignore flag we can turn on). In the Chromium fork of coreboot we're already using a bunch of those flags that we should probably use in upstream coreboot as well: --ignore C99_COMMENTS --ignore GLOBAL_INITIALISERS --ignore INITIALISED_STATIC --ignore LINE_SPACING --ignore NEW_TYPEDEFS --ignore PREFER_ALIGNED --ignore PREFER_PACKED --ignore PREFER_PRINTF --ignore SPLIT_STRING However, in my experience there will always be edge cases checkpatch doesn't get right, no matter how well you tweak it. It should always be up to the discretion of the committer/reviewer which warnings they act upon, so we need to retain a mechanism (like --no-verify) to work around it if necessary. Maybe we should update the pre-commit hook to print an explanation to this effect. (Ideally, I think it would be awesome if Gerrit could separately run checkpatch and make it explicit in the interface if someone circumvented a warning this way so it can be discussed, but that's probably not and easy thing to add.)

1 0

ASUS KFSN4-DRE (K8) Automated Test Failure [master]
by Raptor Engineering Automated Coreboot Test Stand 27 Jul '16

27 Jul '16

The ASUS KFSN4-DRE (K8) fails verification for branch master as of commit aa3e8a8124fe82aa13eb828b0da69966e4d03cd1 The following tests failed: BOOT_FAILURE Commits since last successful test: aa3e8a8 drivers/intel/fsp2_0/header_util: Convert UPD headers c7dfbe2 google/oak: dsi: set mipi pin driving control on 700b039 meditek/mt8173: dsi: set mipi pin driving control on 4cfde2a arch/x86: Generate a map file for the postcar stage 99f1b2f arch/x86: Organize ramstage to match other stages e82b505 arch/x86: Move romstage files into romstage section a749150 arch/x86: Move postcar stage commands into place See attached log for details This message was automatically generated from Raptor Engineering's ASUS KFSN4-DRE (K8) test stand Want to test on your own equipment? Check out https://www.raptorengineeringinc.com/content/REACTS/intro.html Raptor Engineering also offers coreboot consulting services! Please visit https://www.raptorengineeringinc.com for more information Please contact Timothy Pearson at Raptor Engineering <tpearson(a)raptorengineeringinc.com> regarding any issues stemming from this notification

2 1

Jenkins started giving Code-Review -1 on gerrit
by Jonathan Neuschäfer 26 Jul '16

26 Jul '16

Hi, I noticed that the "build bot (Jenkins)" account on gerrit now gives a Code-Review -1 mark for patches that don't build cleanly, in addition to Verified -1. Is this intended? I find it confusing, because I thought Code-Review was reserved for human reviewers, and we already have Verified for automated tests. Best regards, Jonathan Neuschäfer

2 1

← Newer
1
2
3
4
5
6
7
...
14
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

2003

2002

coreboot July 2016