Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Freebsd #159

Closed
wants to merge 2 commits into from
Closed

Support Freebsd #159

wants to merge 2 commits into from

Conversation

mldulaney
Copy link

This patch introduces support for x86 on FreeBSD. I note that I added the check for FreeBSD on aarch64 to fail in case this gets built on an aarch64 FreeBSD system.

Signed-off-by: Mairi Dulaney <jdulaney@fedoraproject.org>
Signed-off-by: Mairi Dulaney <jdulaney@fedoraproject.org>
@vishwin
Copy link

vishwin commented Jun 11, 2021

Those sysctl names are probably wrong. On a Broadwell system, only hw.instruction_sse is present, yet the hardware actually supports much more. cpuinfo from linprocfs is correct, however, but FreeBSD binaries should not be reading from any Linuxulator interfaces.

@@ -1244,6 +1246,13 @@ static void DetectSseViaOs(X86Features* features) {
features->ssse3 = GetDarwinSysCtlByName("hw.optional.supplementalsse3");
features->sse4_1 = GetDarwinSysCtlByName("hw.optional.sse4_1");
features->sse4_2 = GetDarwinSysCtlByName("hw.optional.sse4_2");
#elif defined(CPU_FEATURES_OS_FREEBSD)
features->sse = sysctlbyname("hw.instruction_sse", NULL, NULL, NULL, 1);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I understand, this is not how sysctlbyname is supposed to be used.
https://www.freebsd.org/cgi/man.cgi?query=sysctlbyname&manpath=FreeBSD+13.0-RELEASE+and+Ports

One need to set proper arguments.
Also returned value is 0 upon success and -1 upon failures.
I believe we should rename GetDarwinSysCtlByName above and use it here.

@@ -80,6 +80,11 @@
#define DEFINE_TABLE_FEATURE_TYPE Aarch64Features
#include "define_tables.h"

#if defined(CPU_FEATURES_OS_FREEBSD)
#error "FreeBSD not yet supported on this arch"
#endif // CPU_FEATURES_OS
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// CPU_FEATURES_OS_FREEBSD

@@ -107,6 +107,8 @@
#error "Darwin needs support for sysctlbyname"
#endif
#include <sys/sysctl.h>
#elif defined(CPU_FEATURES_OS_FREEBSD)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be merged with the Darwin logic above.

@gchatelet
Copy link
Collaborator

I'm closing this PR, FreeBSD support will be handled by #163 which will be submitted as soon as we have FreeBSD support in our CI.

Thank you @mldulaney for the patch: it didn't make it to the repo but it got me interested in solving the issue so it's a worthwhile contribution! (also thank you @vishwin for the comment)

@gchatelet gchatelet closed this Jun 25, 2021
@mldulaney mldulaney deleted the freebsd branch June 26, 2021 15:54
@gchatelet gchatelet added this to the v0.7.0 milestone Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fails on FreeBSD: error: "Unsupported fallback detection of SSE OS support."
4 participants