[coreboot] WG: What is the best way to treat warnings reported by checkpatch.pl

[Zeh, Werner]

>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
-- 
coreboot mailing list: coreboot@coreboot.org
https://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] What is the best way to treat warnings reported by checkpatch.pl

[ron minnich]

On Tue, Jul 26, 2016 at 7:15 PM Stefan Reinauer <
stefan.reina...@coreboot.org> wrote:

>
>
> > 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? ;)
>
>
>

but in this does not seem to be. It looks like a table. Is it really
registers?

ron
-- 
coreboot mailing list: coreboot@coreboot.org
https://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] Microcode problem with Braswell CPU

[Zoran Stojsavljevic]

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-with-j3060-j3160-and-j3710
 :
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  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.stojsavlje...@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 
>> 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.stojsavlje...@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-with-j3060-j3160-and-j3710

 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 

Re: [coreboot] What is the best way to treat warnings reported by checkpatch.pl

[Stefan Reinauer]

* ron minnich  [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


-- 
coreboot mailing list: coreboot@coreboot.org
https://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] What is the best way to treat warnings reported by checkpatch.pl

[ron minnich]

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  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@coreboot.org
> https://www.coreboot.org/mailman/listinfo/coreboot
>
-- 
coreboot mailing list: coreboot@coreboot.org
https://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] ASUS KFSN4-DRE (K8) Automated Test Failure [master]

[Timothy Pearson]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 07/26/2016 06:28 PM, Raptor Engineering Automated Coreboot Test Stand
wrote:
> 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

This is a genuine test failure.  The previous message suffered from an
unrelated failure of the serial adapter attached to this system,
corrupting the log.  In the process of verifying this failure the DIMMs
in the K8 test system were exchanged, but the machine check failure
remained.  Furthermore, there are no boot issues with the fallback ROM
on the same hardware.

- -- 
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645 (direct line)
+1 (512) 690-0200 (switchboard)
https://www.raptorengineering.com
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJXl/P2AAoJEK+E3vEXDOFbKccIAIaa3D6sgSJhrXMowF1u85Rc
Hs8ymHOCxTRfilvZrhkHiRcm2bmKAghkt2YUYHvOr/soc0/Vt7g8Pw6LtQkpAKEx
9rzXoz2ZJETQYqXPPdpJ/TQyC3+wnt7BgkMRWwRwW77On+mRHwKBounmXq1/10Rc
HqVYU02beESC6I9+O0pw0G4O/fSeruYpFsvz9m65cqFWss8lpjJhRZtRHUSNOpdn
+d3UIBgVlj7GoL1H2XYroO8nCr4sbkHvQtKdmM2pwAjxAr43W95FKkUyY8UZIh10
YjOAO0o4AV93UDLhzG8ZHAn+Ks/5kJccmYOP5IjRyFVpy+THcDASRBR008H0Pec=
=V9P4
-END PGP SIGNATURE-

-- 
coreboot mailing list: coreboot@coreboot.org
https://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] Jenkins started giving Code-Review -1 on gerrit

[Patrick Georgi via coreboot]

2016-07-23 15:57 GMT+02:00 Jonathan Neuschäfer :
> 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.
There was a difference between the "failed" (V-1) and "unstable"
(CR-1, V-1) states that could be signalled in this way. But it's not
like these are very meaningful in the first place, so I set them to
report CR=0 always.


Thanks,
Patrick
-- 
Google Germany GmbH, ABC-Str. 19, 20354 Hamburg
Registergericht und -nummer: Hamburg, HRB 86891, Sitz der Gesellschaft: Hamburg
Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle

-- 
coreboot mailing list: coreboot@coreboot.org
https://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] kexec of Xen hypervisor from a Linux payload

[Trammell Hudson]

On Tue, Jul 26, 2016 at 02:48:42PM -0400, Ward Vandewege wrote:
> Oh, wow, thank you! Sorry that I didn't spend time tracking that down
> properly back in 2008. I'd be interested to know if Xen takes the patch.

Thank *you* for isolating it to the change between 3.1.0 and 3.1.3 so
many years ago.  Without that hint it would have been a much longer
process to figure out what was wrong.

-- 
Trammell

-- 
coreboot mailing list: coreboot@coreboot.org
https://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] kexec of Xen hypervisor from a Linux payload

[Ward Vandewege]

On Tue, Jul 26, 2016 at 12:09:18PM -0600, Trammell Hudson wrote:
> On Tue, Jul 26, 2016 at 09:37:20AM -0600, Trammell Hudson wrote:
> > [...]
> > Unfortunately 3.1.3 is ancient; I'm going to build the more modern
> > Xen 4.6.x to see if I can repeat these fixes to boot into Qubes.
> 
> This required a few more hacks, but it works now.  The problem is not
> with Coreboot, but with Xen assuming that there is a BIOS it can call
> to setup the video display and also assuming that there is an EBDA
> that contains pointers to various structures.
> 
> My 4.6.3 Xen tree is hacked up right now with stuff copied from old
> versions of the tree, so I'll clean it up and see if they are interested
> in accepting patches.

Oh, wow, thank you! Sorry that I didn't spend time tracking that down
properly back in 2008. I'd be interested to know if Xen takes the patch.

Thanks,
Ward.

-- 
Ward Vandewege
GPG Key: 25F774AB

Do you use free software? Donate to join the FSF and support freedom at
 http://www.fsf.org/register_form?referrer=859

-- 
coreboot mailing list: coreboot@coreboot.org
https://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] kexec of Xen hypervisor from a Linux payload

[Trammell Hudson]

On Tue, Jul 26, 2016 at 09:37:20AM -0600, Trammell Hudson wrote:
> [...]
> Unfortunately 3.1.3 is ancient; I'm going to build the more modern
> Xen 4.6.x to see if I can repeat these fixes to boot into Qubes.

This required a few more hacks, but it works now.  The problem is not
with Coreboot, but with Xen assuming that there is a BIOS it can call
to setup the video display and also assuming that there is an EBDA
that contains pointers to various structures.

My 4.6.3 Xen tree is hacked up right now with stuff copied from old
versions of the tree, so I'll clean it up and see if they are interested
in accepting patches.

-- 
Trammell

-- 
coreboot mailing list: coreboot@coreboot.org
https://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] kexec of Xen hypervisor from a Linux payload

[Trammell Hudson]

On Mon, Jul 25, 2016 at 03:56:22PM -0600, Trammell Hudson wrote:
> # There seems to be a regression with regard to kexec'ing into
> # a Xen kernel between Xen 3.1.0 (confirmed working) and 3.1.3
> # (confirmed not working).

I was able to reproduce this in qemu, which allowed me to debug
it much faster.  The problem is that 3.1.3 is making BIOS calls
to try to initialize the VGA console in xen/arch/x86/boot/video.S,
while 3.1.0 had much of it hard coded in the xen/drivers/video/vga.c
file.

I nop'ed out the calls to do the real mode stuff in
trampoline_boot_cpu_entry, which avoids lots of things like:

# Set the 80x25 mode. If already set, do nothing.
set_80x25:
movw$0x5019, bootsym(force_size)  # Override possibly broken BIOS
use_80x25:
movw$0x1202, %ax# Force 400 scan lines
movb$0x30, %bl
int $0x10
movw$0x0003, %ax# Mode 3
int $0x10
stc
ret

I also replaced vga.c with the one from the 3.1.0 Xen tree to hardcode
the text mode 3 configuration.  Now it is able to kexec the Xen 3.1.3
kernel from my Coreboot+Linux payload with no BIOS.

Unfortunately 3.1.3 is ancient; I'm going to build the more modern
Xen 4.6.x to see if I can repeat these fixes to boot into Qubes.

-- 
Trammell
-- 
coreboot mailing list: coreboot@coreboot.org
https://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] Microcode problem with Braswell CPU

[cheng yichen]

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 23:11 GMT+08:00 cheng yichen :

> 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.stojsavlje...@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 
>> 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.stojsavlje...@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-with-j3060-j3160-and-j3710

 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.boec...@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 

Re: [coreboot] Microcode problem with Braswell CPU

[cheng yichen]

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.stojsavlje...@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-with-j3060-j3160-and-j3710
>
> 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.boec...@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.stojsavlje...@gmail.com]
>> *Gesendet:* Montag, 25. Juli 2016 22:08
>> *An:* Alexander Böcken
>> *Cc:* coreboot@coreboot.org; york.y...@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
>>
>> 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.boec...@junger-audio.com> wrote:
>>
>> Hello Zoran,
>>
>> 

[coreboot] Support for USB auto switching in Intel FSP

[Naveed Ghori]

Hi all,

I wanted to know if anyone had gotten the Intel FSP to work with auto mode. It 
does not work for me in Seabios. I have not tried EDKII yet.
However I believe the FSP is not setup for it anyway out of the box.

Any help appreciated.
Naveed Ghori

Naveed Ghori | Lead Firmware & Driver Engineer

DTI Group Ltd | Transit Security & Surveillance

31 Affleck Road, Perth Airport, Western Australia 6105, AU

P +61 8 9373 2905,151 | F +61 8 9479 1190 | naveed.gh...@dti.com.au



Visit our website www.dti.com.au

The information contained in this email is confidential. If you receive this 
email in error, please inform DTI Group Ltd via the above contact details. If 
you are not the intended recipient, you may not use or disclose the information 
contained in this email or attachments.
-- 
coreboot mailing list: coreboot@coreboot.org
https://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] Where is the first instrucion?

[Zoran Stojsavljevic]

> Oh no! The corporate copyright bogeyman's gonna get ya!

Probably Intel Inside paranormal activities. ;-)

Zoran

On Tue, Jul 26, 2016 at 9:02 AM, Anthony Martin  wrote:

> Tony Marchini  once said:
> > I would like to start with, Do not disassemble copyrighted firmware and
> > CERTAINLY don't post your results to a public forum.
>
> Oh no! The corporate copyright bogeyman's gonna get ya!
>
> Keep learning, Rafael. Disassemble everything.
>
> Cheers,
>   Anthony
>
> --
> coreboot mailing list: coreboot@coreboot.org
> https://www.coreboot.org/mailman/listinfo/coreboot
>
-- 
coreboot mailing list: coreboot@coreboot.org
https://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] Microcode problem with Braswell CPU

[Zoran Stojsavljevic]

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-with-j3060-j3160-and-j3710

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.boec...@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.stojsavlje...@gmail.com]
> *Gesendet:* Montag, 25. Juli 2016 22:08
> *An:* Alexander Böcken
> *Cc:* coreboot@coreboot.org; york.y...@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
>
> 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.boec...@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 

Re: [coreboot] Where is the first instrucion?

[Anthony Martin]

Tony Marchini  once said:
> I would like to start with, Do not disassemble copyrighted firmware and
> CERTAINLY don't post your results to a public forum.

Oh no! The corporate copyright bogeyman's gonna get ya!

Keep learning, Rafael. Disassemble everything.

Cheers,
  Anthony

-- 
coreboot mailing list: coreboot@coreboot.org
https://www.coreboot.org/mailman/listinfo/coreboot

[coreboot] What is the best way to treat warnings reported by checkpatch.pl

[Zeh, Werner]

Hi community.

I recently ran into a blocked commit due to warnings reported by checkpatch.pl.
In this particular case I wanted to add the "ATSR" structure to the DMAR 
tables. To do that, I need to define a new structure
which reflects the per specification needed layout of this table entry.
I did that in the same style how other structures are defined in the same file 
(src/arch/x86/include/arch/acpi.h):

typedef struct dmar_atsr_entry {
u16 type;
u16 length;
u8 flags;
u8 reserved;
u16 segment;
} __attribute__ ((packed)) dmar_atsr_entry_t;

This change resulted in two warnings caused by util/lint/checkpatch.pl when I 
was trying to do a git commit:

WARNING: do not add new typedefs
#34: FILE: src/arch/x86/include/arch/acpi.h:288:
+typedef struct dmar_atsr_entry {

WARNING: __packed is preferred over __attribute__((packed))
#40: FILE: src/arch/x86/include/arch/acpi.h:294:
+} __attribute__ ((packed)) dmar_atsr_entry_t;

So the first warning tells me that I should not use "typedef" in my code.

The second one tells me not to use __attribute__ ((packed) but instead __packed!
If one take a closer look to the coreboot tree, one will not find a macro 
definition called __packed at all.

After a short discussion on IRC with Stefan it was clear that although we have 
this checkpatch.pl script in the tree and the git hook "pre-commit"
activates it, the script does not perfectly match current coreboot coding style.

So in the end there needs to be a change somewhere:

1. We can adapt the script to match current coreboot coding style

2. We can start to refactor the coreboot tree to match the demands of 
checkpatch.pl

3. We can align only new patches to the demands of the script

4. We can show warnings from the script but prevent them from aborting the 
git commit

While 1. would be the best, straight forward approach for the project, it might 
cause high effort for a person who knows the code base very well.
2. will lead to a lot of changes in the tree while the reason/goal is still 
questionable (at least to me).
3. will lead to different code style all over the tree (even in the same file) 
and in the end will result in fragmented code. I personally would not like it.
4. can be a short term solution while we are seeking for the best way to deal 
with this issue.

There may be other ways to go, too, which I have overseen now.

I just want to start this discussion officially as this is not the first time I 
ran into issues with checkpatch.pl.
What do you think about it, what would be the best way to handle this case?

Your ideas and thoughts are appreciated.

Werner
-- 
coreboot mailing list: coreboot@coreboot.org
https://www.coreboot.org/mailman/listinfo/coreboot

Re: [coreboot] Microcode problem with Braswell CPU

[Alexander Böcken]

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.stojsavlje...@gmail.com]
Gesendet: Montag, 25. Juli 2016 22:08
An: Alexander Böcken
Cc: coreboot@coreboot.org; york.y...@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
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 
> 
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@coreboot.org
https://www.coreboot.org/mailman/listinfo/coreboot