[PATCH v6 0/1] MR7339: ntdll: implement create_logical_proc_info on FreeBSD.

[Damjan Jovanovic (＠dj)]

Add Expat as a dependency, and use it to parse the XML returned from FreeBSD's kern.sched.topology_spec sysctl.

--
  v6: ntdll: implement create_logical_proc_info on FreeBSD.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7339

[PATCH v6 1/1] ntdll: implement create_logical_proc_info on FreeBSD.

[Damjan Jovanovic]

From: Damjan Jovanovic <damjan.jov(a)gmail.com>

Add Expat as a dependency, and use it to parse the XML returned from
FreeBSD's kern.sched.topology_spec sysctl.
---
 configure.ac             |  15 +++
 dlls/ntdll/Makefile.in   |   4 +-
 dlls/ntdll/unix/system.c | 233 +++++++++++++++++++++++++++++++++++++++
 include/config.h.in      |   3 +
 4 files changed, 253 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index 8192c067e4c..4c1856ab056 100644
--- a/configure.ac
+++ b/configure.ac
@@ -30,6 +30,7 @@ AC_ARG_WITH(capi,      AS_HELP_STRING([--without-capi],[do not use CAPI (ISDN su
 AC_ARG_WITH(coreaudio, AS_HELP_STRING([--without-coreaudio],[do not use the CoreAudio sound support]))
 AC_ARG_WITH(cups,      AS_HELP_STRING([--without-cups],[do not use CUPS]))
 AC_ARG_WITH(dbus,      AS_HELP_STRING([--without-dbus],[do not use DBus (dynamic device support)]))
+AC_ARG_WITH(expat,     AS_HELP_STRING([--without-expat],[do not use Expat]))
 AC_ARG_WITH(ffmpeg,    AS_HELP_STRING([--without-ffmpeg],[do not use the FFmpeg library]))
 AC_ARG_WITH(fontconfig,AS_HELP_STRING([--without-fontconfig],[do not use fontconfig]))
 AC_ARG_WITH(freetype,  AS_HELP_STRING([--without-freetype],[do not use the FreeType library]))
@@ -1483,6 +1484,20 @@ case $host_os in
                       [libdbus ${notice_platform}development files not found, no dynamic device support.]) ;;
 esac
 
+dnl **** Check for expat ****
+if test "x$with_expat" != "xno" -a `expr "$host_os" : '\(freebsd\).*'` = "freebsd"
+then
+    WINE_PACKAGE_FLAGS(EXPAT,[expat],,,,
+        [AC_CHECK_HEADER([expat.h],
+            [AC_CHECK_LIB(expat,XML_ParserCreate,
+                [AC_DEFINE(HAVE_LIBEXPAT, 1, [Define to 1 if you have the 'expat' library (-lexpat).])],
+                [EXPAT_LIBS=""],
+                [$EXPAT_LIBS])],
+            [EXPAT_LIBS=""])])
+    WINE_NOTICE_WITH(expat,[test "$ac_cv_lib_expat_XML_ParserCreate" != "yes"],
+                     [expat ${notice_platform}development files not found, detailed CPU info on FreeBSD won't be supported.])
+fi
+
 dnl **** Check for libgnutls ****
 if test "x$with_gnutls" != "xno"
 then
diff --git a/dlls/ntdll/Makefile.in b/dlls/ntdll/Makefile.in
index f7558bb5d86..9d698986592 100644
--- a/dlls/ntdll/Makefile.in
+++ b/dlls/ntdll/Makefile.in
@@ -4,8 +4,8 @@ UNIXLIB   = ntdll.so
 IMPORTLIB = ntdll
 IMPORTS   = $(TOMCRYPT_PE_LIBS) $(MUSL_PE_LIBS)
 EXTRAINCL = $(TOMCRYPT_PE_CFLAGS)
-UNIX_CFLAGS  = $(UNWIND_CFLAGS)
-UNIX_LIBS    = $(IOKIT_LIBS) $(COREFOUNDATION_LIBS) $(CORESERVICES_LIBS) $(RT_LIBS) $(PTHREAD_LIBS) $(UNWIND_LIBS) $(I386_LIBS) $(PROCSTAT_LIBS)
+UNIX_CFLAGS  = $(UNWIND_CFLAGS) $(EXPAT_CFLAGS)
+UNIX_LIBS    = $(IOKIT_LIBS) $(COREFOUNDATION_LIBS) $(CORESERVICES_LIBS) $(RT_LIBS) $(PTHREAD_LIBS) $(UNWIND_LIBS) $(I386_LIBS) $(PROCSTAT_LIBS) $(EXPAT_LIBS)
 
 EXTRADLLFLAGS = -nodefaultlibs
 i386_EXTRADLLFLAGS = -Wl,--image-base,0x7bc00000
diff --git a/dlls/ntdll/unix/system.c b/dlls/ntdll/unix/system.c
index cbd27d094d1..a569e2a79bc 100644
--- a/dlls/ntdll/unix/system.c
+++ b/dlls/ntdll/unix/system.c
@@ -69,6 +69,10 @@
 # include <mach/vm_map.h>
 #endif
 
+#if defined(HAVE_LIBEXPAT)
+# include <expat.h>
+#endif
+
 #include "ntstatus.h"
 #define WIN32_NO_STATUS
 #include "windef.h"
@@ -1332,6 +1336,235 @@ static NTSTATUS create_logical_proc_info(void)
     return STATUS_SUCCESS;
 }
 
+#elif defined(__FreeBSD__) || defined(__FreeBSD__kernel__)
+
+/*
+ * str is a comma-separated array of longs, in hexadecimal, in the kernel's
+ * bitness, eg: "f,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0" for 4 CPUs. With
+ * CPU_MAXSIZE==1024, and with (sizeof(long)*8)==64, there will be
+ * ceil(1024/64)==16 array entries. CPU IDs fill the longs from first to last,
+ * lowest to highest bit.
+ *
+ * Wine currently doesn't support processor groups, so we only care about
+ * the first ULONG_PTR size bits, others wouldn't fit in the mask.
+ */
+static BOOL parse_cpu_mask(const char *str, ULONG_PTR *mask)
+{
+    *mask = 0;
+    for (; *str && *str != ','; str++)
+    {
+        *mask <<= 4;
+        if ('0' <= *str && *str <= '9')
+            *mask |= *str - '0';
+        else if ('a' <= *str && *str <= 'f')
+            *mask |= 0xa + (*str - 'a');
+        else
+            return FALSE;
+    }
+    return TRUE;
+}
+
+struct group_state {
+    unsigned int cache_level;
+    ULONG_PTR cpu_mask;
+    BOOL is_numa_node;
+    BOOL has_children;
+};
+
+#define MAX_GROUP_LEVELS 10
+
+struct topology_parse_state {
+    NTSTATUS nt_status;
+    DWORD group_level;
+    struct group_state groups[1 + MAX_GROUP_LEVELS];
+
+    unsigned int package_count;
+    unsigned int core_count;
+    unsigned int numa_node_count;
+    ULONG_PTR all_cpus_mask;
+};
+
+static void topology_start_elem(void *user_data, const char *name, const char **attrs)
+{
+    struct group_state *group;
+    struct topology_parse_state *state = user_data;
+    int i;
+
+    if (state->nt_status != STATUS_SUCCESS)
+        return;
+    group = &state->groups[state->group_level];
+
+    if (!strcmp(name, "groups"))
+        ;
+    else if (!strcmp(name, "group"))
+    {
+        group->has_children = TRUE;
+        ++state->group_level;
+        if (state->group_level > MAX_GROUP_LEVELS)
+        {
+            FIXME("excessive <group> element nesting\n");
+            state->nt_status = STATUS_XML_PARSE_ERROR;
+            return;
+        }
+        group = &state->groups[state->group_level];
+        group->cache_level = 0;
+        group->cpu_mask = 0;
+        group->is_numa_node = FALSE;
+        group->has_children = FALSE;
+        for (i = 0; attrs[i]; i += 2)
+        {
+            if (!strcmp(attrs[i], "cache-level"))
+                group->cache_level = atoi(attrs[i+1]);
+        }
+    }
+    else if (!strcmp(name, "cpu"))
+    {
+        for (i = 0; attrs[i]; i += 2)
+        {
+            if (!strcmp(attrs[i], "mask"))
+                parse_cpu_mask(attrs[i+1], &group->cpu_mask);
+        }
+    }
+    else if (!strcmp(name, "flags"))
+        ;
+    else if (!strcmp(name, "flag"))
+    {
+        for (i = 0; attrs[i]; i += 2)
+        {
+            if (!strcmp(attrs[i], "name") && !strcmp(attrs[i+1], "NODE"))
+                group->is_numa_node = TRUE;
+        }
+    }
+    else if (!strcmp(name, "children"))
+        ;
+}
+
+static void topology_end_elem(void *user_data, const char *name)
+{
+    struct topology_parse_state *state = user_data;
+
+    if (state->nt_status != STATUS_SUCCESS)
+        return;
+
+    /* Treat top-level groups as packages, leaf groups as cores */
+    if (!strcmp(name, "group"))
+    {
+        struct group_state *group = &state->groups[state->group_level];
+        state->all_cpus_mask |= group->cpu_mask;
+        if (!group->has_children)
+        {
+            if (!logical_proc_info_add_by_id(RelationProcessorPackage, state->package_count, group->cpu_mask))
+            {
+                state->nt_status = STATUS_NO_MEMORY;
+                return;
+            }
+            TRACE("added processors 0x%016llx to package %u\n", (long long)group->cpu_mask, state->package_count);
+            if (!logical_proc_info_add_by_id(RelationProcessorCore, state->core_count, group->cpu_mask))
+            {
+                state->nt_status = STATUS_NO_MEMORY;
+                return;
+            }
+            TRACE("added processors 0x%016llx to core %u\n", (long long)group->cpu_mask, state->core_count);
+            ++state->core_count;
+        }
+        if (group->is_numa_node)
+        {
+            if (!logical_proc_info_add_numa_node(group->cpu_mask, state->numa_node_count))
+            {
+                state->nt_status = STATUS_NO_MEMORY;
+                return;
+            }
+            TRACE("added processors 0x%016llx to NUMA node %u\n", (long long)group->cpu_mask, state->numa_node_count);
+            ++state->numa_node_count;
+        }
+        /* FreeBSD provides almost no info about caching */
+        if (1 <= group->cache_level && group->cache_level <= 3)
+        {
+            CACHE_DESCRIPTOR cache;
+            cache.Level = group->cache_level;
+            cache.Associativity = 12; /* reasonable default */
+            cache.LineSize = 64; /* reasonable default */
+            cache.Size = 64 * 1024; /* reasonable default */
+            cache.Type = CacheUnified; /* reasonable default */
+            if (!logical_proc_info_add_cache(group->cpu_mask, &cache))
+            {
+                state->nt_status = STATUS_NO_MEMORY;
+                return;
+            }
+            TRACE("added processors 0x%016llx to cache level %u\n", (long long)group->cpu_mask, group->cache_level);
+        }
+        --state->group_level;
+        if (state->group_level == 0)
+            ++state->package_count;
+    }
+}
+
+static NTSTATUS create_logical_proc_info(void)
+{
+#if defined(HAVE_LIBEXPAT)
+    size_t size;
+    int ret;
+    NTSTATUS nt_status = STATUS_SUCCESS;
+    char *topology_spec = NULL;
+    XML_Parser parser = NULL;
+    struct topology_parse_state state = {};
+
+    /* See the smp(4) man page, and https://forums.freebsd.org/threads/number-of-cpus-and-cores.41299/ */
+    ret = sysctlbyname("kern.sched.topology_spec", NULL, &size, NULL, 0);
+    if (ret != 0)
+    {
+        if (errno == ENOENT || errno == ENOTDIR || errno == EISDIR)
+        {
+            FIXME("the kern.sched.topology_spec sysctl does not exist, you need to use sched_ule\n");
+            nt_status = STATUS_NOT_IMPLEMENTED;
+        }
+        else
+            nt_status = STATUS_NO_MEMORY;
+        goto end;
+    }
+    topology_spec = malloc(size);
+    if (topology_spec == NULL)
+    {
+        nt_status = STATUS_NO_MEMORY;
+        goto end;
+    }
+    ret = sysctlbyname("kern.sched.topology_spec", topology_spec, &size, NULL, 0);
+    if (ret != 0)
+    {
+        nt_status = STATUS_NO_MEMORY;
+        goto end;
+    }
+
+    parser = XML_ParserCreate("US-ASCII");
+    if (parser == NULL)
+    {
+        nt_status = STATUS_NO_MEMORY;
+        goto end;
+    }
+    state.nt_status = STATUS_SUCCESS;
+    XML_SetUserData(parser, &state);
+    XML_SetElementHandler(parser, topology_start_elem, topology_end_elem);
+    if (XML_Parse(parser, topology_spec, size-1, XML_TRUE) == XML_STATUS_OK)
+        nt_status = state.nt_status;
+    else
+    {
+        FIXME("failed to parse sysctl kern.sched.topology_spec, expat errorcode %d\n", XML_GetErrorCode(parser));
+        nt_status = STATUS_XML_PARSE_ERROR;
+    }
+    logical_proc_info_add_group(count_bits(state.all_cpus_mask), state.all_cpus_mask);
+
+end:
+    free(topology_spec);
+    if (parser != NULL)
+        XML_ParserFree(parser);
+    return nt_status;
+
+#else
+    FIXME("expat unavailable, cannot parse sysctl kern.sched.topology_spec\n");
+    return STATUS_NOT_IMPLEMENTED;
+#endif
+}
+
 #else
 
 static NTSTATUS create_logical_proc_info(void)
diff --git a/include/config.h.in b/include/config.h.in
index 7ac924a077c..773a8254a11 100644
--- a/include/config.h.in
+++ b/include/config.h.in
@@ -138,6 +138,9 @@
 /* Define to 1 if you have the <libavutil/avutil.h> header file. */
 #undef HAVE_LIBAVUTIL_AVUTIL_H
 
+/* Define to 1 if you have the 'expat' library (-lexpat). */
+#undef HAVE_LIBEXPAT
+
 /* Define to 1 if you have the 'gettextpo' library (-lgettextpo). */
 #undef HAVE_LIBGETTEXTPO
 
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/7339

[Damjan Jovanovic (＠dj)]

On Wed Mar 26 05:37:02 2025 +0000, Brendan Shanks wrote:
> Maybe this check should only happen on FreeBSD, so other platforms don't
> unnecessarily link against expat?
> I've now made that check FreeBSD-specific.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7339#note_99101

[Damjan Jovanovic (＠dj)]

On Mon Mar 24 18:44:48 2025 +0000, Brendan Shanks wrote:
> This works for me running
> [hwloc/lstopo](https://www.open-mpi.org/projects/hwloc/) on FreeBSD 14.2
> on an i7-14700 (24 threads between E- and hyper-threaded P-cores).
> fontconfig and mesa depend on expat, so it's typically installed anyway.
> Should we be using that hwloc/lstopo library instead of parsing kern.sched.topology_spec ourselves? It seems to provide more info, including cache sizes.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7339#note_99102

[Gerald Pfeifer (＠gerald)]

On Wed Mar 26 05:39:14 2025 +0000, Damjan Jovanovic wrote:
> Should we be using that hwloc/lstopo library instead of parsing
> kern.sched.topology_spec ourselves? It seems to provide more info,
> including cache sizes.
> From a FreeBSD (packaging) perspective I can see this is a fine alternative. It adds two additional dependencies - devel/hwloc and devel/libpciaccess - which appear to be reasonably
> light.

Happy to leave the choice to you. It just would be nice to either get the current patch
pushed or an alternate one that you hint at. 

@bshanks, for the time being, is there any obstacle to the former?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7339#note_107544

[Alastair Hogge (＠agh)]

Hello,

Great work, Damjan. This patch has restored wine on FreeBSD for myself, that I have not seen since 5.x or 6.x. I have a local modified copy of emulators/wine-devel and I sync with wine/main, and the patch has been stable for over 5 weeks now.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7339#note_113396

[Thibault Payet (＠thpayet)]

Hello,

Is this patch expected to be merged in wine, or something major is missing ?
Without this information, we can't easily use it downstream to be tested by a broader audience.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7339#note_117613