Page MenuHomeFreeBSD

Linux ARM v2 2/9
AcceptedPublic

Authored by soutade_gmail.com on Sep 20 2016, 1:59 PM.

Details

Summary

Linux ARM : Add ARM architecture to compat files

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

soutade_gmail.com retitled this revision from to Linux ARM v2 2/9.
soutade_gmail.com updated this object.
soutade_gmail.com edited the test plan for this revision. (Show Details)
soutade_gmail.com set the repository for this revision to rS FreeBSD src repository.
imp accepted this revision.Sep 20 2016, 3:37 PM
imp added a reviewer: imp.

I wonder, though, if there might be a more generic way to say

#if defined(i386) || defined(arm) || (defined(amd64) && defined(COMPAT_LINUX32))

since it is repeated so often.

sys/compat/linux/linux_file.h
79

Is this x86 only? Or are there other emulations that use these values? If it is x86 only, I'd be tempted to have an #elsif defined(i386) || defined(amd64) here with a #else that's just an #error "No value defined for this architecture"

This revision is now accepted and ready to land.Sep 20 2016, 3:37 PM
In D7971#164955, @imp wrote:

I wonder, though, if there might be a more generic way to say
#if defined(i386) || defined(arm) || (defined(amd64) && defined(COMPAT_LINUX32))
since it is repeated so often.

Maybe we can do something like :
#if defined(i386) || defined(arm) || (defined(amd64) && defined(COMPAT_LINUX32))
#define _LINUX_COMPATIBLE 1
#else
#define _LINUX_COMPATIBLE 0
#endif

Then repeat :
#if _LINUX_COMPATIBLE
...

imp added a comment.EditedSep 20 2016, 4:11 PM
In D7971#164955, @imp wrote:

I wonder, though, if there might be a more generic way to say
#if defined(i386) || defined(arm) || (defined(amd64) && defined(COMPAT_LINUX32))
since it is repeated so often.

Maybe we can do something like :
#if defined(i386) || defined(arm) || (defined(amd64) && defined(COMPAT_LINUX32))
#define _LINUX_COMPATIBLE 1
#else
#define _LINUX_COMPATIBLE 0
#endif
Then repeat :
#if _LINUX_COMPATIBLE
...

These look to be only on platforms that support 32-bit linux emulation, so maybe there's a better name? maybe HAVE_LINUX32?

These look to be only on platforms that support 32-bit linux emulation, so maybe there's a better name? maybe HAVE_LINUX32?

Sounds good to me. I'll update the patch tomorrow.

sys/compat/linux/linux_file.h
79

From my point of view, these values should be the same for all architectures. I don't know why they changed them for ARM...
Could be checked for other architectures, but for now Linux emulation is only supported by x86.

imp added inline comments.Sep 20 2016, 7:34 PM
sys/compat/linux/linux_file.h
79

Right, but if I wanted to do linux emulation for powerpc or mips, I might use the wrong values unknowingly. These values are different for arm, arm64, and powerpc (among others). What's left are the generic ones it seems.

sys/compat/linux/linux_file.h
79

It's different for Alpha and PARISC (0100000). Blackfin, arm, arm64, PowerPC and m68k has the same value.

These look to be only on platforms that support 32-bit linux emulation, so maybe there's a better name? maybe HAVE_LINUX32?

Sounds good to me. I'll update the patch tomorrow.

After looking deeper in the code, all these defines are split into multiple files. There is no common include. For now, we can keep these defines as is, isn't it ?

imp added a comment.Sep 21 2016, 1:55 PM

These look to be only on platforms that support 32-bit linux emulation, so maybe there's a better name? maybe HAVE_LINUX32?

Sounds good to me. I'll update the patch tomorrow.

After looking deeper in the code, all these defines are split into multiple files. There is no common include. For now, we can keep these defines as is, isn't it ?

For now that's OK, but if you can prepare a follow on patch that fixes this, that would be great.

soutade_gmail.com edited edge metadata.

Remove old Linux syscalls (not presents in Linux calls.S)

This revision now requires review to proceed.Oct 4 2016, 8:42 AM
andrew added inline comments.Oct 4 2016, 9:42 AM
sys/compat/linux/linux_signal.c
94

Why the case? These types seem to be compatible.

sys/compat/linux/linux_signal.c
94

Right. It was my first definition of l_sigaction_t that was incorrect. I'll fix this.

soutade_gmail.com edited edge metadata.

Remove unnecessary cast. Remove obsolete syscalls. Add linux_send() and linux_recv()

emaste added a subscriber: emaste.Jun 17 2017, 9:03 PM
This revision is now accepted and ready to land.Jun 17 2017, 9:03 PM