v8: workaround to arm v6l detection in newer kernel versions as described in Issue 8589

[wilson0x4d]

v8: workaround to arm v6l detection in newer kernel versions as described in Issue 8589

See: nodejs/node#283
See: #7222
See: #8589
See: https://code.google.com/p/v8/issues/detail?id=3112

Also reflected in v8 pull: v8/v8#18

[silverwind]

@wilson0x4d did the compiled binary work for you? I wasn't having any luck with a very similar patch so far, with either node or iojs (they SIGILL on movw/movt instructions). Also I'd be interested on which kernel and model you tested.

Here's my data for comparison:

$ uname -a
Linux pi 3.12.36-1-ARCH #1 PREEMPT Thu Jan 15 21:54:44 MST 2015 armv6l GNU/Linux
$ cat /proc/cpuinfo
processor   : 0
model name  : ARMv6-compatible processor rev 7 (v6l)
Features    : swp half thumb fastmult vfp edsp java tls
CPU implementer : 0x41
CPU architecture: 7
CPU variant : 0x0
CPU part    : 0xb76
CPU revision    : 7

Hardware    : BCM2708
Revision    : 000d
Serial      : 0000000085415c8b

[wilson0x4d]

yes, and no.

I also had previously edited configure to force the use of "vfpv2" instead of "vfpv3" by changing:

o['variables']['arm_fpu'] = 'vfpv3' # V8 3.18 no longer supports VFP2.
to
o['variables']['arm_fpu'] = 'vfpv2' # V8 3.18 no longer supports VFP2.

In addition to the armv6 change to cpu.cc.

Since "vfpv2" isn't considered supported in "v8" I have not generated a patch (I'm really not sure what the correct action is wrt the configure script tbh.) My limited understanding is that 'vfp' are floating point extensions, and my assumption here is that by specifying vfpv2 the node build is omitting said extensions (as the VFP3=0 from --v8-options suggests.) Thus, someone will probably want to generate an appropriate patch to configure, or perhaps give me some insight into what scenarios it is acceptable (or unacceptable) to opt out of VFPv3. (@benjamingram, I don't suppose this is something you're familiar with?)

I have seen a similar patch floating around that checked for v7 before interrogating 'model name', however, this patch is taken from @sxa555's Issue #8589 which simply determines a failure to read v6l from "Processor" field and then blindly attempts to pull 'v6l' from the "model name" field. It avoids problems where a version OTHER than v7 is being detected (for whatever reason, now or in the future.)

After updating local source to detect armv6 I am able to run node successfully again, here is my build command-line (for completeness):

sudo make clean; sudo ./configure --without-snapshot; sudo make; sudo make install

and also of interest:

me@mypi ~/node-build $ uname -a
Linux mypi 3.12.35+ #730 PREEMPT Fri Dec 19 18:31:24 GMT 2014 armv6l GNU/Linux

me@mypi ~/node-build $ node --v8-options | more -2
target arm v6 vfp2 hard
ARMv7=0 VFP3=0 VFP32DREGS=0 NEON=0 SUDIV=0 UNALIGNED_ACCESSES=0 MOVW_MOVT_IMMEDIATE_LOADS=0 USE_EABI_HARDFLOAT=1

pi@mypi ~/node-build $ cat /proc/cpuinfo
processor       : 0
model name      : ARMv6-compatible processor rev 7 (v6l)
Features        : swp half thumb fastmult vfp edsp java tls
CPU implementer : 0x41
CPU architecture: 7
CPU variant     : 0x0
CPU part        : 0xb76
CPU revision    : 7
Hardware        : BCM2708
Revision        : 000e
Serial          : 000000009b70a753

and lastly (within node):

> this.process.version
'v0.13.0-pre'

Hope this helps someone else, I probably burned a total of 4 hours getting node up and running again after upgrading my pi last night :)

[silverwind]

Great job @wilson0x4d, I've just compiled a working binary with both patches from nodejs/node#283

[wilson0x4d]

the only problem I have with the chromium fix applied to bnoordhuis/io.js is that it's still hardcoding a version check, it will necessarily break again with ARMv8.

that aside, would be nice to get similar patches pushed into joyent/node so we can get updated ARMv6 packages :) and I don't deserve all the credit, this was solved by multiple people, I simply gathered all the info available. I'm surprised nobody else fixed it sooner...

[wilson0x4d]

@silverwind @bnoordhuis

additional commits based on those submitted to bnoordhuis/io.js

1. do not assume ARMv7 is the version we incorrectly detect in all cases
2. do not assume non-ARMv7 has VFPv2 support
3. assume ARMpre6 supports VFPv1 (e.g. ARMv5, which I believe is the earliest version ever supported)

Maintainers should expect ARMv8 will require an additional conditional to configure in the future, and ideally no change to cpu.cc, based on these commits.

HTH someone else ;) I'll keep my fork live until there's a merge or equivalent fix put into place.

[bnoordhuis]

@wilson0x4d I'll be happy to take a pull request. I wonder though, does V8 still work on ARMv5 / with VFP < 2? V8 generally only supports what AOSP and Chrome support.

[wilson0x4d]

I'll be honest and note that I'm not sure (ARMv5 on v8), I found conflicting info on the net with respect to ARM cores, and do not have an ARMv5 based device to test.

I based the conditional on info from wikipedia, which lists 2 of 3 official ARMv5 based cores as supporting VFPv2, and the other supporting VFP (no specific version that I could find), and basically went with the LCD of the three (that they all supported at least the original VFP instruction set), but again this was an assumption without having access to an ARMv5 device to test.

FWIW; The VFPv2-compatible listing included boards/devices such as Atmel's Make Controller, Lego's Mindstorms NXT, "most" older Nokia devices, an unlisted number of automobiles, etc. The VFPv1 based ARMv5 chip appeared to only be in use as a microcontroller of sorts (as opposed to the main CPU for anything) for example video decoding on the Wii. I'm only really interested in Pi support, and perhaps a few older phones (which are not ARMv5), but it seemed appropriate to have an explicit conditional in the off chance an ARMv5 device was detected so that an ARMv6 architecture wasn't specified to any build tools. (for example, there's at least one guy attempting to build nodejs for the OLinuXino, an ARMv5-based SBC.)

As an aside, I just read that ARM's official tool chain appears to treat vfpv1 and vfpv2 as synonymous, so it likely makes sense that armv5 detection use vfpv2 instead.

I'll make a few more edits.

[wilson0x4d]

i think this version of configure better conveys intent, where arch specific overrides can be added/modified as needed to ensure a proper build. if someone were to come back to this and edit it in the future they would be coerced into writing code that conveys their intent more clearly (e.g. for version X the tool chain requires Y.)

[trevnorris]

@tjfontaine @misterdjules Thoughts on this?

[misterdjules]

Why is this change different than the one landed upstream?

[misterdjules]

@trevnorris Besides my inline comment, the change seems to make sense. Thank you @wilson0x4d!

That would make us float another patch on top of V8 though, which makes me wonder how @jasnell's work on having V8 built from vanilla source code + patches queue is going :)

[misterdjules]

@trevnorris @wilson0x4d Also, shouldn't we land that in v0.12 instead of master?

[trevnorris]

@misterdjules It's possible that's just how the PR was created, instead of actually intended. I'm fine landing this on v0.12.

[WarheadsSE]

@wilson0x4d There are very very few ARMv5 generation processors with VFP units, and they were all essentially custom. You're far better assuming no VFP then any VFP below ARMv6. Even in ARMv6, it was an option, albeit a standard one. The only reason for is proliferation of code with vfpv2 is because of the RPi. I have several ARMv6 boards with no VFP at all.

I do entirely agree that hard-coding is a Bad Thing ™ but a proper functional solution does need found.

If you ever need test platforms, let me know. I have access to the full range of low-high ARMv5-ARMv7 boards and should soon have an ARMv8.

[misterdjules]

@WarheadsSE Thanks for your input! It seems that this PR doesn't fix the vfp detection on all ARM platforms, but it seems it fixes a lot of issues with common platforms, and thus would bring some improvements while not causing any regression. I have the impression that we could land this PR, and then work on a more robust and broader ARM support later. What do you think?

@trevnorris I'm also fine on landing this, but I would rather split it into two commits as done in this branch: https://github.com/misterdjules/node/commits/fix-armv6-detection (see the top 2 commits). This preserves all the information on the V8 fix upstream and land the configure changes in a separate commit.

@wilson0x4d What do you think?

[WarheadsSE]

@misterdjules Spot on with my meaning regarding detection. I know that a chunk of this is faults within v8, but I would say that it is safe to call ARMv6 vfpv2, since 90% of all ARMv6 boards running targeted toolchains (read not a generic set from random distro) are /sigh/ RPi's.

Please keep in mind also, that ARMv7 does not mean neon, and often when there is neon, there might be vpfpv4! (This is how Cortex-A15/A7 pairs in big.LITTLE work)

I also agree with splitting the commits as you suggested.

[misterdjules]

@WarheadsSE Excellent.

@wilson0x4d @WarheadsSE Do you have some time to test that https://github.com/misterdjules/node/commits/fix-armv6-detection fixes the build on the ARM platforms it's supposed to fix (Raspberry PI B+, but probably others)?

Thank you!

[WarheadsSE]

@silverwind can confirm that this does work. I know this, because he directed me to the chain of issues that leads here.

[misterdjules]

@WarheadsSE Yes, I just want to make sure that the changes in my branch, which are slightly different than the changes in this PR, also work as expected. They should work as they seem to be semantically equivalent, but I would like to be certain :)

[silverwind]

@misterdjules: I can start off a build on my Pi for it. Does your branch include the v8 fix? (These builds usually take ~5 hours)

[silverwind]

Nevermind, I see it's included. ETA on build 5 hours.

[misterdjules]

@silverwind Thank you!

[silverwind]

@misterdjules build done, and it works!

[misterdjules]

@silverwind That's great, thank you so much for your help! We'll land https://github.com/misterdjules/node/commits/fix-armv6-detection as soon as possible.

[misterdjules]

Actually, we'll probably want to just integrate nodejs/node#559 since it's the exact same changes as my branch, plus the removal of arm_neon, which is effectively not used by the current V8 code in v0.12.

[meshpoint]

Y U NO USE UNAME?

#include <stdio.h>
#include <sys/utsname.h>
int main () {
        struct utsname u;
        if (uname (&u)<0) {
                perror ("uname: ");
                return 1;
        }
        printf ("machine: %s\n", u.machine);
        return 0;
}

machine: armv6l

[misterdjules]

Closing in favor of #14193 as discussed previously. Hopefully we'll be able to release that soon. Maybe for the upcoming v0.12.2, probably for v0.12.3 at the latest.

[wilson0x4d]

finally catching up on this thread, /agree /agree /agree. code changes look great! thanks for the focus on this issue by everyone, hopefully this means we'll be able to deploy a 'vanilla' node to our devices soon. :)

[misterdjules]

@wilson0x4d Thank you for all your help 👍

[magic890]

Here is the official patch from V8 team: https://github.com/v8/v8-git-mirror/blob/master/src/base/cpu.cc#L483-L499

I have created a pull request to solve this issue: #25625