April 2022 - Wine-Devel - list.winehq.org

[PATCH v3 2/2] programs/sc: Allow using separate arguments for parameter name and value.

by Torge Matthies

In addition to using one argument for both parameter name and value. This fixes a regression from commit 8b38c91d83844dea882922055ced7cdeb79c1693. Signed-off-by: Torge Matthies <tmatthies(a)codeweavers.com> --- programs/sc/sc.c | 101 ++++++++++++++++++++++++++++------------- programs/sc/tests/sc.c | 95 ++++++++++++++++++++------------------ 2 files changed, 120 insertions(+), 76 deletions(-) diff --git a/programs/sc/sc.c b/programs/sc/sc.c index 43148ab99d60..bd7e093b61d2 100644 --- a/programs/sc/sc.c +++ b/programs/sc/sc.c @@ -27,6 +27,26 @@ WINE_DEFAULT_DEBUG_CHANNEL(sc); +static BOOL parse_string_param( int argc, const WCHAR *argv[], unsigned int *index, + const WCHAR *param_name, size_t name_len, const WCHAR **out ) +{ + if (!wcsnicmp( argv[*index], param_name, name_len )) + { + if (argv[*index][name_len]) + { + *out = &argv[*index][name_len]; + return TRUE; + } + else if (*index < argc - 1) + { + *index += 1; + *out = argv[*index]; + return TRUE; + } + } + return FALSE; +} + struct create_params { const WCHAR *displayname; @@ -56,48 +76,56 @@ static BOOL parse_create_params( int argc, const WCHAR *argv[], struct create_pa cp->obj = NULL; cp->password = NULL; +#define PARSE(x, y) parse_string_param( (argc), (argv), &(i), (x), ARRAY_SIZE(x) - 1, (y) ) for (i = 0; i < argc; i++) { - if (!wcsnicmp( argv[i], L"displayname=", 12 )) cp->displayname = argv[i] + 12; - if (!wcsnicmp( argv[i], L"binpath=", 8 )) cp->binpath = argv[i] + 8; - if (!wcsnicmp( argv[i], L"group=", 6 )) cp->group = argv[i] + 6; - if (!wcsnicmp( argv[i], L"depend=", 7 )) cp->depend = argv[i] + 7; - if (!wcsnicmp( argv[i], L"obj=", 4 )) cp->obj = argv[i] + 4; - if (!wcsnicmp( argv[i], L"password=", 9 )) cp->password = argv[i] + 9; - - if (!wcsnicmp( argv[i], L"tag=", 4 )) + const WCHAR *tag, *type, *start, *error; + + if (PARSE( L"displayname=", &cp->displayname )) continue; + if (PARSE( L"binpath=", &cp->binpath )) continue; + if (PARSE( L"group=", &cp->group )) continue; + if (PARSE( L"depend=", &cp->depend )) continue; + if (PARSE( L"obj=", &cp->obj )) continue; + if (PARSE( L"password=", &cp->password )) continue; + + if (PARSE( L"tag=", &tag )) { - if (!wcsicmp( argv[i] + 4, L"yes" )) + if (!wcsicmp( tag, L"yes" )) { WINE_FIXME("tag argument not supported\n"); cp->tag = TRUE; } + continue; } - if (!wcsnicmp( argv[i], L"type=", 5 )) + if (PARSE( L"type=", &type )) { - if (!wcsicmp( argv[i] + 5, L"own" )) cp->type = SERVICE_WIN32_OWN_PROCESS; - if (!wcsicmp( argv[i] + 5, L"share" )) cp->type = SERVICE_WIN32_SHARE_PROCESS; - if (!wcsicmp( argv[i] + 5, L"kernel" )) cp->type = SERVICE_KERNEL_DRIVER; - if (!wcsicmp( argv[i] + 5, L"filesys" )) cp->type = SERVICE_FILE_SYSTEM_DRIVER; - if (!wcsicmp( argv[i] + 5, L"rec" )) cp->type = SERVICE_RECOGNIZER_DRIVER; - if (!wcsicmp( argv[i] + 5, L"interact" )) cp->type |= SERVICE_INTERACTIVE_PROCESS; + if (!wcsicmp( type, L"own" )) cp->type = SERVICE_WIN32_OWN_PROCESS; + else if (!wcsicmp( type, L"share" )) cp->type = SERVICE_WIN32_SHARE_PROCESS; + else if (!wcsicmp( type, L"kernel" )) cp->type = SERVICE_KERNEL_DRIVER; + else if (!wcsicmp( type, L"filesys" )) cp->type = SERVICE_FILE_SYSTEM_DRIVER; + else if (!wcsicmp( type, L"rec" )) cp->type = SERVICE_RECOGNIZER_DRIVER; + else if (!wcsicmp( type, L"interact" )) cp->type |= SERVICE_INTERACTIVE_PROCESS; + continue; } - if (!wcsnicmp( argv[i], L"start=", 6 )) + if (PARSE( L"start=", &start )) { - if (!wcsicmp( argv[i] + 6, L"boot" )) cp->start = SERVICE_BOOT_START; - if (!wcsicmp( argv[i] + 6, L"system" )) cp->start = SERVICE_SYSTEM_START; - if (!wcsicmp( argv[i] + 6, L"auto" )) cp->start = SERVICE_AUTO_START; - if (!wcsicmp( argv[i] + 6, L"demand" )) cp->start = SERVICE_DEMAND_START; - if (!wcsicmp( argv[i] + 6, L"disabled" )) cp->start = SERVICE_DISABLED; + if (!wcsicmp( start, L"boot" )) cp->start = SERVICE_BOOT_START; + else if (!wcsicmp( start, L"system" )) cp->start = SERVICE_SYSTEM_START; + else if (!wcsicmp( start, L"auto" )) cp->start = SERVICE_AUTO_START; + else if (!wcsicmp( start, L"demand" )) cp->start = SERVICE_DEMAND_START; + else if (!wcsicmp( start, L"disabled" )) cp->start = SERVICE_DISABLED; + continue; } - if (!wcsnicmp( argv[i], L"error=", 6 )) + if (PARSE( L"error=", &error )) { - if (!wcsicmp( argv[i] + 6, L"normal" )) cp->error = SERVICE_ERROR_NORMAL; - if (!wcsicmp( argv[i] + 6, L"severe" )) cp->error = SERVICE_ERROR_SEVERE; - if (!wcsicmp( argv[i] + 6, L"critical" )) cp->error = SERVICE_ERROR_CRITICAL; - if (!wcsicmp( argv[i] + 6, L"ignore" )) cp->error = SERVICE_ERROR_IGNORE; + if (!wcsicmp( error, L"normal" )) cp->error = SERVICE_ERROR_NORMAL; + else if (!wcsicmp( error, L"severe" )) cp->error = SERVICE_ERROR_SEVERE; + else if (!wcsicmp( error, L"critical" )) cp->error = SERVICE_ERROR_CRITICAL; + else if (!wcsicmp( error, L"ignore" )) cp->error = SERVICE_ERROR_IGNORE; + continue; } } +#undef PARSE if (!cp->binpath) return FALSE; return TRUE; } @@ -156,16 +184,25 @@ static BOOL parse_failure_params( int argc, const WCHAR *argv[], SERVICE_FAILURE fa->cActions = 0; fa->lpsaActions = NULL; +#define PARSE(x, y) parse_string_param( (argc), (argv), &(i), (x), ARRAY_SIZE(x) - 1, (y) ) for (i = 0; i < argc; i++) { - if (!wcsnicmp( argv[i], L"reset=", 6 )) fa->dwResetPeriod = wcstol( argv[i] + 6, NULL, 10 ); - if (!wcsnicmp( argv[i], L"reboot=", 7 )) fa->lpRebootMsg = (WCHAR *)argv[i] + 7; - if (!wcsnicmp( argv[i], L"command=", 8 )) fa->lpCommand = (WCHAR *)argv[i] + 8; - if (!wcsnicmp( argv[i], L"actions=", 8 )) + const WCHAR *reset, *actions; + + if (PARSE( L"reset=", &reset )) + { + fa->dwResetPeriod = wcstol( reset, NULL, 10 ); + continue; + } + if (PARSE( L"reboot=", (const WCHAR **)&fa->lpRebootMsg )) continue; + if (PARSE( L"command=", (const WCHAR **)&fa->lpCommand )) continue; + if (PARSE( L"actions=", &actions )) { - if (!parse_failure_actions( argv[i] + 8, fa )) return FALSE; + if (!parse_failure_actions( actions, fa )) return FALSE; + continue; } } +#undef PARSE return TRUE; } diff --git a/programs/sc/tests/sc.c b/programs/sc/tests/sc.c index 66493ed578c7..57046905a4b0 100644 --- a/programs/sc/tests/sc.c +++ b/programs/sc/tests/sc.c @@ -168,13 +168,20 @@ static void test_create_service(BOOL elevated) DWORD expected_start_type; DWORD expected_service_type; const char * expected_binary_path; + DWORD broken; } start_types[] = { - { "boot type= kernel", SERVICE_BOOT_START, SERVICE_KERNEL_DRIVER, TEST_SERVICE_BINARY_START_BOOT }, - { "system type= kernel", SERVICE_SYSTEM_START, SERVICE_KERNEL_DRIVER, TEST_SERVICE_BINARY_START_SYSTEM }, - { "auto", SERVICE_AUTO_START, SERVICE_WIN32_OWN_PROCESS, TEST_SERVICE_BINARY }, - { "demand", SERVICE_DEMAND_START, SERVICE_WIN32_OWN_PROCESS, TEST_SERVICE_BINARY }, - { "disabled", SERVICE_DISABLED, SERVICE_WIN32_OWN_PROCESS, TEST_SERVICE_BINARY }, - { "delayed-auto", SERVICE_DELAYED_AUTO_START, SERVICE_WIN32_OWN_PROCESS, TEST_SERVICE_BINARY } + { "boot type= kernel", SERVICE_BOOT_START, SERVICE_KERNEL_DRIVER, TEST_SERVICE_BINARY_START_BOOT, + BROKEN_BINPATH | BROKEN_DISPLAY_NAME }, + { "system type= kernel", SERVICE_SYSTEM_START, SERVICE_KERNEL_DRIVER, TEST_SERVICE_BINARY_START_SYSTEM, + BROKEN_BINPATH | BROKEN_DISPLAY_NAME }, + { "auto", SERVICE_AUTO_START, SERVICE_WIN32_OWN_PROCESS, TEST_SERVICE_BINARY, + BROKEN_DISPLAY_NAME }, + { "demand", SERVICE_DEMAND_START, SERVICE_WIN32_OWN_PROCESS, TEST_SERVICE_BINARY, + BROKEN_DISPLAY_NAME }, + { "disabled", SERVICE_DISABLED, SERVICE_WIN32_OWN_PROCESS, TEST_SERVICE_BINARY, + BROKEN_DISPLAY_NAME }, + { "delayed-auto", SERVICE_DELAYED_AUTO_START, SERVICE_WIN32_OWN_PROCESS, TEST_SERVICE_BINARY, + BROKEN_START | BROKEN_DISPLAY_NAME | BROKEN_DELAYED_AUTO_START } }; static struct { const char *param; @@ -217,45 +224,45 @@ static void test_create_service(BOOL elevated) /* binpath= */ run_sc_exe("sc create " TEST_SERVICE_NAME " binpath= \"" TEST_SERVICE_BINARY "\"", &r); - todo_wine check_exit_code(SC_EXIT_SUCCESS); + check_exit_code(SC_EXIT_SUCCESS); check_test_service(SERVICE_WIN32_OWN_PROCESS, SERVICE_DEMAND_START, SERVICE_ERROR_NORMAL, "", NULL, - BROKEN_CREATE); + BROKEN_DISPLAY_NAME); /* existing service */ run_sc_exe("sc create " TEST_SERVICE_NAME " binpath= \"" TEST_SERVICE_BINARY "\" start= auto", &r); todo_wine check_exit_code(SC_EXIT_SERVICE_EXISTS); check_test_service(SERVICE_WIN32_OWN_PROCESS, SERVICE_DEMAND_START, SERVICE_ERROR_NORMAL, "", NULL, - BROKEN_CREATE); - delete_test_service(TRUE, TRUE); + BROKEN_DISPLAY_NAME); + delete_test_service(TRUE, FALSE); /* type= */ run_sc_exe("sc create " TEST_SERVICE_NAME " binpath= \"" TEST_SERVICE_BINARY "\" type= invalid", &r); todo_wine check_exit_code(SC_EXIT_INVALID_COMMAND_LINE); - delete_test_service(FALSE, FALSE); + delete_test_service(FALSE, TRUE); run_sc_exe("sc create " TEST_SERVICE_NAME " binpath= \"" TEST_SERVICE_BINARY "\" type= own", &r); - todo_wine check_exit_code(SC_EXIT_SUCCESS); + check_exit_code(SC_EXIT_SUCCESS); check_test_service(SERVICE_WIN32_OWN_PROCESS, SERVICE_DEMAND_START, SERVICE_ERROR_NORMAL, "", NULL, - BROKEN_CREATE); - delete_test_service(TRUE, TRUE); + BROKEN_DISPLAY_NAME); + delete_test_service(TRUE, FALSE); run_sc_exe("sc create " TEST_SERVICE_NAME " binpath= \"" TEST_SERVICE_BINARY "\" type= interact", &r); todo_wine check_exit_code(SC_EXIT_INVALID_PARAMETER); - delete_test_service(FALSE, FALSE); + delete_test_service(FALSE, TRUE); run_sc_exe("sc create " TEST_SERVICE_NAME " binpath= \"" TEST_SERVICE_BINARY "\" type= interact type= own", &r); - todo_wine check_exit_code(SC_EXIT_SUCCESS); + check_exit_code(SC_EXIT_SUCCESS); check_test_service(SERVICE_WIN32_OWN_PROCESS | SERVICE_INTERACTIVE_PROCESS, SERVICE_DEMAND_START, - SERVICE_ERROR_NORMAL, "", NULL, BROKEN_CREATE); - delete_test_service(TRUE, TRUE); + SERVICE_ERROR_NORMAL, "", NULL, BROKEN_TYPE | BROKEN_DISPLAY_NAME); + delete_test_service(TRUE, FALSE); /* start= */ run_sc_exe("sc create " TEST_SERVICE_NAME " binpath= \"" TEST_SERVICE_BINARY "\" start= invalid", &r); todo_wine check_exit_code(SC_EXIT_INVALID_COMMAND_LINE); - delete_test_service(FALSE, FALSE); + delete_test_service(FALSE, TRUE); for (i = 0; i < ARRAY_SIZE(start_types); i++) { @@ -264,11 +271,11 @@ static void test_create_service(BOOL elevated) strcpy(cmdline, "sc create " TEST_SERVICE_NAME " binpath= \"" TEST_SERVICE_BINARY "\" start= "); strcat(cmdline, start_types[i].param); run_sc_exe(cmdline, &r); - todo_wine check_exit_code(SC_EXIT_SUCCESS); + check_exit_code(SC_EXIT_SUCCESS); check_service_definition(TEST_SERVICE_NAME, start_types[i].expected_binary_path, start_types[i].expected_service_type, start_types[i].expected_start_type, - SERVICE_ERROR_NORMAL, "", TEST_SERVICE_NAME, BROKEN_CREATE); - delete_test_service(TRUE, TRUE); + SERVICE_ERROR_NORMAL, "", TEST_SERVICE_NAME, start_types[i].broken); + delete_test_service(TRUE, FALSE); } /* error= */ @@ -280,53 +287,53 @@ static void test_create_service(BOOL elevated) strcpy(cmdline, "sc create " TEST_SERVICE_NAME " binpath= \"" TEST_SERVICE_BINARY "\" error= "); strcat(cmdline, error_severities[i].param); run_sc_exe(cmdline, &r); - todo_wine check_exit_code(SC_EXIT_SUCCESS); + check_exit_code(SC_EXIT_SUCCESS); check_test_service(SERVICE_WIN32_OWN_PROCESS, SERVICE_DEMAND_START, - error_severities[i].expected_error_control, "", NULL, BROKEN_CREATE); - delete_test_service(TRUE, TRUE); + error_severities[i].expected_error_control, "", NULL, BROKEN_DISPLAY_NAME); + delete_test_service(TRUE, FALSE); } /* tag= */ run_sc_exe("sc create " TEST_SERVICE_NAME " binpath= \"" TEST_SERVICE_BINARY "\" tag= yes", &r); todo_wine check_exit_code(SC_EXIT_INVALID_PARAMETER); - delete_test_service(FALSE, FALSE); + delete_test_service(FALSE, TRUE); run_sc_exe("sc create " TEST_SERVICE_NAME " binpath= \"" TEST_SERVICE_BINARY "\" tag= no", &r); - todo_wine check_exit_code(SC_EXIT_SUCCESS); + check_exit_code(SC_EXIT_SUCCESS); check_test_service(SERVICE_WIN32_OWN_PROCESS, SERVICE_DEMAND_START, SERVICE_ERROR_NORMAL, "", NULL, - BROKEN_CREATE); - delete_test_service(TRUE, TRUE); + BROKEN_DISPLAY_NAME); + delete_test_service(TRUE, FALSE); /* depend= */ run_sc_exe("sc create " TEST_SERVICE_NAME " binpath= \"" TEST_SERVICE_BINARY "\" depend= " TEST_SERVICE_NAME, &r); todo_wine check_exit_code(SC_EXIT_CIRCULAR_DEPENDENCY); - delete_test_service(FALSE, FALSE); + delete_test_service(FALSE, TRUE); run_sc_exe("sc create " TEST_SERVICE_NAME2 " binpath= \"" TEST_SERVICE_BINARY "\" depend= " TEST_SERVICE_NAME, &r); - todo_wine check_exit_code(SC_EXIT_SUCCESS); + check_exit_code(SC_EXIT_SUCCESS); check_test_service2(SERVICE_WIN32_OWN_PROCESS, SERVICE_DEMAND_START, SERVICE_ERROR_NORMAL, - TEST_SERVICE_NAME, NULL, BROKEN_CREATE); - delete_test_service2(TRUE, TRUE); + TEST_SERVICE_NAME, NULL, BROKEN_DEPEND | BROKEN_DISPLAY_NAME); + delete_test_service2(TRUE, FALSE); run_sc_exe("sc create " TEST_SERVICE_NAME " binpath= " TEST_SERVICE_BINARY, &r); - todo_wine check_exit_code(SC_EXIT_SUCCESS); + check_exit_code(SC_EXIT_SUCCESS); run_sc_exe("sc create " TEST_SERVICE_NAME2 " binpath= \"" TEST_SERVICE_BINARY "\" depend= " TEST_SERVICE_NAME, &r); - todo_wine check_exit_code(SC_EXIT_SUCCESS); + check_exit_code(SC_EXIT_SUCCESS); check_test_service2(SERVICE_WIN32_OWN_PROCESS, SERVICE_DEMAND_START, SERVICE_ERROR_NORMAL, - TEST_SERVICE_NAME, NULL, BROKEN_CREATE); - delete_test_service2(TRUE, TRUE); - delete_test_service(TRUE, TRUE); + TEST_SERVICE_NAME, NULL, BROKEN_DEPEND | BROKEN_DISPLAY_NAME); + delete_test_service2(TRUE, FALSE); + delete_test_service(TRUE, FALSE); /* displayname= */ run_sc_exe("sc create " TEST_SERVICE_NAME " binpath= " TEST_SERVICE_BINARY " displayname= \"Wine Test Service\"", &r); - todo_wine check_exit_code(SC_EXIT_SUCCESS); + check_exit_code(SC_EXIT_SUCCESS); check_test_service(SERVICE_WIN32_OWN_PROCESS, SERVICE_DEMAND_START, SERVICE_ERROR_NORMAL, "", - "Wine Test Service", BROKEN_CREATE); - delete_test_service(TRUE, TRUE); + "Wine Test Service", 0); + delete_test_service(TRUE, FALSE); /* without spaces */ @@ -349,10 +356,10 @@ static void test_create_service(BOOL elevated) run_sc_exe("SC CREATE " TEST_SERVICE_NAME2 " BINPATH= \"" TEST_SERVICE_BINARY "\" TYPE= OWN START= AUTO" " ERROR= NORMAL TAG= NO DEPEND= " TEST_SERVICE_NAME " DISPLAYNAME= \"Wine Test Service\"", &r); - todo_wine check_exit_code(SC_EXIT_SUCCESS); + check_exit_code(SC_EXIT_SUCCESS); check_test_service2(SERVICE_WIN32_OWN_PROCESS, SERVICE_AUTO_START, SERVICE_ERROR_NORMAL, - TEST_SERVICE_NAME, "Wine Test Service", BROKEN_CREATE); - delete_test_service2(TRUE, TRUE); + TEST_SERVICE_NAME, "Wine Test Service", BROKEN_DEPEND); + delete_test_service2(TRUE, FALSE); #undef delete_test_service2 #undef check_test_service2 -- 2.36.0

April 29, 2022

[PATCH v3 1/2] programs/sc: Add tests.

by Torge Matthies

Signed-off-by: Torge Matthies <tmatthies(a)codeweavers.com> --- configure.ac | 1 + programs/sc/tests/Makefile.in | 5 + programs/sc/tests/sc.c | 402 ++++++++++++++++++++++++++++++++++ 3 files changed, 408 insertions(+) create mode 100644 programs/sc/tests/Makefile.in create mode 100644 programs/sc/tests/sc.c diff --git a/configure.ac b/configure.ac index 74c80fd7fa8c..98f56fc849fa 100644 --- a/configure.ac +++ b/configure.ac @@ -3322,6 +3322,7 @@ WINE_CONFIG_MAKEFILE(programs/rpcss) WINE_CONFIG_MAKEFILE(programs/rundll.exe16,enable_win16) WINE_CONFIG_MAKEFILE(programs/rundll32) WINE_CONFIG_MAKEFILE(programs/sc) +WINE_CONFIG_MAKEFILE(programs/sc/tests) WINE_CONFIG_MAKEFILE(programs/schtasks) WINE_CONFIG_MAKEFILE(programs/schtasks/tests) WINE_CONFIG_MAKEFILE(programs/sdbinst) diff --git a/programs/sc/tests/Makefile.in b/programs/sc/tests/Makefile.in new file mode 100644 index 000000000000..24a875fb9051 --- /dev/null +++ b/programs/sc/tests/Makefile.in @@ -0,0 +1,5 @@ +TESTDLL = sc.exe +IMPORTS = advapi32 + +C_SRCS = \ + sc.c diff --git a/programs/sc/tests/sc.c b/programs/sc/tests/sc.c new file mode 100644 index 000000000000..66493ed578c7 --- /dev/null +++ b/programs/sc/tests/sc.c @@ -0,0 +1,402 @@ +/* + * Copyright 2022 Torge Matthies for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include <windows.h> +#include <winsvc.h> +#include <stdio.h> +#include "wine/test.h" + +#define lok ok_(__FILE__,line) + +#define TEST_SERVICE_NAME "wine_test_svc" +#define TEST_SERVICE_NAME2 "wine_test_svc_2" +#define TEST_SERVICE_BINARY "c:\\windows\\system32\\cmd.exe" +#define TEST_SERVICE_BINARY_START_BOOT "\\SystemRoot\\system32\\cmd.exe" +#define TEST_SERVICE_BINARY_START_SYSTEM "\\??\\" TEST_SERVICE_BINARY + +#define SC_EXIT_SUCCESS ERROR_SUCCESS +#define SC_EXIT_INVALID_PARAMETER ERROR_INVALID_PARAMETER +#define SC_EXIT_CIRCULAR_DEPENDENCY ERROR_CIRCULAR_DEPENDENCY +#define SC_EXIT_SERVICE_DOES_NOT_EXIST ERROR_SERVICE_DOES_NOT_EXIST +#define SC_EXIT_SERVICE_EXISTS ERROR_SERVICE_EXISTS +#define SC_EXIT_INVALID_COMMAND_LINE ERROR_INVALID_COMMAND_LINE + +static HANDLE nul_file; +static SC_HANDLE scmgr; + +/* Copied and modified from the reg.exe tests */ +#define run_sc_exe(c,r) run_sc_exe_(__FILE__,__LINE__,c,r) +static BOOL run_sc_exe_(const char *file, unsigned line, const char *cmd, DWORD *rc) +{ + STARTUPINFOA si = {sizeof(STARTUPINFOA)}; + PROCESS_INFORMATION pi; + BOOL bret; + DWORD ret; + char cmdline[256]; + + si.dwFlags = STARTF_USESTDHANDLES; + si.hStdInput = nul_file; + si.hStdOutput = nul_file; + si.hStdError = nul_file; + + strcpy(cmdline, cmd); + if (!CreateProcessA(NULL, cmdline, NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi)) + return FALSE; + + ret = WaitForSingleObject(pi.hProcess, 10000); + if (ret == WAIT_TIMEOUT) + TerminateProcess(pi.hProcess, 1); + + bret = GetExitCodeProcess(pi.hProcess, rc); + lok(bret, "GetExitCodeProcess failed: %ld\n", GetLastError()); + + CloseHandle(pi.hThread); + CloseHandle(pi.hProcess); + return bret; +} + +#define BROKEN_CREATE 0x000000001UL +#define BROKEN_BINPATH 0x000000002UL +#define BROKEN_TYPE 0x000000004UL +#define BROKEN_START 0x000000008UL +#define BROKEN_ERROR 0x000000010UL +#define BROKEN_DEPEND 0x000000020UL +#define BROKEN_DISPLAY_NAME 0x000000040UL +#define BROKEN_DELAYED_AUTO_START 0x000000080UL +#define BROKEN_ALL ~0UL + +#define SERVICE_DELAYED_AUTO_START (SERVICE_AUTO_START | 0x80000000) + +#define check_service_definition(n,bi,t,s,e,de,di,br) check_service_definition_(__FILE__,__LINE__,n,bi,t,s,e,de,di,br) +static void check_service_definition_(const char *file, unsigned line, char const *name, + const char *binpath, DWORD type, DWORD start, DWORD error, + const char *depend, const char *display_name, DWORD broken) +{ + SERVICE_DELAYED_AUTO_START_INFO delayed_auto_info = {0}; + union { + char buffer[8192]; + QUERY_SERVICE_CONFIGA config; + } cfg; + BOOL delayed_auto; + SC_HANDLE svc; + DWORD needed; + BOOL ret; + + delayed_auto = !!(start & 0x80000000); + start &= ~0x80000000; + + if (!scmgr) + return; + + svc = OpenServiceA(scmgr, name, GENERIC_READ); + todo_wine_if(broken & BROKEN_CREATE) + lok(!!svc, "OpenServiceA failed: %ld\n", GetLastError()); + if (!svc) + return; + + ret = QueryServiceConfigA(svc, &cfg.config, sizeof(cfg.buffer), &needed); + lok(!!ret, "QueryServiceConfigA failed: %ld\n", GetLastError()); + if (!ret) + goto done; + + ret = QueryServiceConfig2A(svc, SERVICE_CONFIG_DELAYED_AUTO_START_INFO, (LPBYTE)&delayed_auto_info, + sizeof(delayed_auto_info), &needed); + todo_wine lok(!!ret, "QueryServiceConfig2A(SERVICE_CONFIG_DELAYED_AUTO_START_INFO) failed: %ld\n", + GetLastError()); + +#define check_str(a, b, msg) lok((a) && (b) && (a) != (b) && !strcmp((a), (b)), msg ": %s != %s\n", \ + debugstr_a((a)), debugstr_a((b))) +#define check_dw(a, b, msg) lok((a) == (b), msg ": 0x%lx != 0x%lx\n", a, b) + + todo_wine_if(broken & BROKEN_BINPATH) + check_str(cfg.config.lpBinaryPathName, binpath, "Wrong binary path"); + todo_wine_if(broken & BROKEN_TYPE) + check_dw(cfg.config.dwServiceType, type, "Wrong service type"); + todo_wine_if(broken & BROKEN_START) + check_dw(cfg.config.dwStartType, start, "Wrong start type"); + todo_wine_if(broken & BROKEN_ERROR) + check_dw(cfg.config.dwErrorControl, error, "Wrong error control"); + todo_wine_if(broken & BROKEN_DEPEND) + check_str(cfg.config.lpDependencies, depend, "Wrong dependencies"); + todo_wine_if(broken & BROKEN_DISPLAY_NAME) + check_str(cfg.config.lpDisplayName, display_name, "Wrong display name"); + todo_wine_if(broken & BROKEN_DELAYED_AUTO_START) + check_dw((DWORD)delayed_auto_info.fDelayedAutostart, (DWORD)delayed_auto, "Wrong delayed autostart value"); + +#undef check_dw +#undef check_str + +done: + CloseServiceHandle(svc); +} + +#define delete_service(n,e,b) delete_service_(__FILE__,__LINE__,n,e,b) +static void delete_service_(const char *file, unsigned line, const char *name, DWORD expected_status, BOOL broken) +{ + char command[256]; + BOOL bret; + DWORD r; + + strcpy(command, "sc delete "); + strcat(command, name); + bret = run_sc_exe_(file, line, command, &r); + lok(bret, "run_sc_exe failed\n"); + if (expected_status != SC_EXIT_SUCCESS && !strcmp(winetest_platform, "wine")) + expected_status = 1; + todo_wine_if(broken) lok(r == expected_status, "got exit code %ld, expected %ld\n", r, expected_status); +} + +static void test_create_service(BOOL elevated) +{ + static struct { + const char *param; + DWORD expected_start_type; + DWORD expected_service_type; + const char * expected_binary_path; + } start_types[] = { + { "boot type= kernel", SERVICE_BOOT_START, SERVICE_KERNEL_DRIVER, TEST_SERVICE_BINARY_START_BOOT }, + { "system type= kernel", SERVICE_SYSTEM_START, SERVICE_KERNEL_DRIVER, TEST_SERVICE_BINARY_START_SYSTEM }, + { "auto", SERVICE_AUTO_START, SERVICE_WIN32_OWN_PROCESS, TEST_SERVICE_BINARY }, + { "demand", SERVICE_DEMAND_START, SERVICE_WIN32_OWN_PROCESS, TEST_SERVICE_BINARY }, + { "disabled", SERVICE_DISABLED, SERVICE_WIN32_OWN_PROCESS, TEST_SERVICE_BINARY }, + { "delayed-auto", SERVICE_DELAYED_AUTO_START, SERVICE_WIN32_OWN_PROCESS, TEST_SERVICE_BINARY } + }; + static struct { + const char *param; + DWORD expected_error_control; + } error_severities[] = { + { "normal", SERVICE_ERROR_NORMAL }, + { "severe", SERVICE_ERROR_SEVERE }, + { "critical", SERVICE_ERROR_CRITICAL }, + { "ignore", SERVICE_ERROR_IGNORE } + }; + unsigned int i; + DWORD r; + + if (!elevated) + { + win_skip("\"sc create\" tests need elevated permissions\n"); + return; + } + +#define check_exit_code(x) ok(r == (x), "got exit code %ld, expected %d\n", r, (x)) +#define check_test_service(t,s,e,de,di,br) \ + check_service_definition(TEST_SERVICE_NAME, TEST_SERVICE_BINARY, t, s, e, de, di ? di : TEST_SERVICE_NAME, br) +#define delete_test_service(x, y) \ + delete_service(TEST_SERVICE_NAME, (x) ? SC_EXIT_SUCCESS : SC_EXIT_SERVICE_DOES_NOT_EXIST, (y)) +#define check_test_service2(t,s,e,de,di,br) \ + check_service_definition(TEST_SERVICE_NAME2, TEST_SERVICE_BINARY, t, s, e, de, di ? di : TEST_SERVICE_NAME2, br) +#define delete_test_service2(x, y) \ + delete_service(TEST_SERVICE_NAME2, (x) ? SC_EXIT_SUCCESS : SC_EXIT_SERVICE_DOES_NOT_EXIST, (y)) + + /* too few parameters */ + + run_sc_exe("sc create", &r); + todo_wine check_exit_code(SC_EXIT_INVALID_COMMAND_LINE); + delete_test_service(FALSE, FALSE); + + run_sc_exe("sc create " TEST_SERVICE_NAME, &r); + todo_wine check_exit_code(SC_EXIT_INVALID_COMMAND_LINE); + delete_test_service(FALSE, FALSE); + + /* binpath= */ + + run_sc_exe("sc create " TEST_SERVICE_NAME " binpath= \"" TEST_SERVICE_BINARY "\"", &r); + todo_wine check_exit_code(SC_EXIT_SUCCESS); + check_test_service(SERVICE_WIN32_OWN_PROCESS, SERVICE_DEMAND_START, SERVICE_ERROR_NORMAL, "", NULL, + BROKEN_CREATE); + + /* existing service */ + + run_sc_exe("sc create " TEST_SERVICE_NAME " binpath= \"" TEST_SERVICE_BINARY "\" start= auto", &r); + todo_wine check_exit_code(SC_EXIT_SERVICE_EXISTS); + check_test_service(SERVICE_WIN32_OWN_PROCESS, SERVICE_DEMAND_START, SERVICE_ERROR_NORMAL, "", NULL, + BROKEN_CREATE); + delete_test_service(TRUE, TRUE); + + /* type= */ + + run_sc_exe("sc create " TEST_SERVICE_NAME " binpath= \"" TEST_SERVICE_BINARY "\" type= invalid", &r); + todo_wine check_exit_code(SC_EXIT_INVALID_COMMAND_LINE); + delete_test_service(FALSE, FALSE); + + run_sc_exe("sc create " TEST_SERVICE_NAME " binpath= \"" TEST_SERVICE_BINARY "\" type= own", &r); + todo_wine check_exit_code(SC_EXIT_SUCCESS); + check_test_service(SERVICE_WIN32_OWN_PROCESS, SERVICE_DEMAND_START, SERVICE_ERROR_NORMAL, "", NULL, + BROKEN_CREATE); + delete_test_service(TRUE, TRUE); + + run_sc_exe("sc create " TEST_SERVICE_NAME " binpath= \"" TEST_SERVICE_BINARY "\" type= interact", &r); + todo_wine check_exit_code(SC_EXIT_INVALID_PARAMETER); + delete_test_service(FALSE, FALSE); + + run_sc_exe("sc create " TEST_SERVICE_NAME " binpath= \"" TEST_SERVICE_BINARY "\" type= interact type= own", &r); + todo_wine check_exit_code(SC_EXIT_SUCCESS); + check_test_service(SERVICE_WIN32_OWN_PROCESS | SERVICE_INTERACTIVE_PROCESS, SERVICE_DEMAND_START, + SERVICE_ERROR_NORMAL, "", NULL, BROKEN_CREATE); + delete_test_service(TRUE, TRUE); + + /* start= */ + + run_sc_exe("sc create " TEST_SERVICE_NAME " binpath= \"" TEST_SERVICE_BINARY "\" start= invalid", &r); + todo_wine check_exit_code(SC_EXIT_INVALID_COMMAND_LINE); + delete_test_service(FALSE, FALSE); + + for (i = 0; i < ARRAY_SIZE(start_types); i++) + { + char cmdline[256]; + + strcpy(cmdline, "sc create " TEST_SERVICE_NAME " binpath= \"" TEST_SERVICE_BINARY "\" start= "); + strcat(cmdline, start_types[i].param); + run_sc_exe(cmdline, &r); + todo_wine check_exit_code(SC_EXIT_SUCCESS); + check_service_definition(TEST_SERVICE_NAME, start_types[i].expected_binary_path, + start_types[i].expected_service_type, start_types[i].expected_start_type, + SERVICE_ERROR_NORMAL, "", TEST_SERVICE_NAME, BROKEN_CREATE); + delete_test_service(TRUE, TRUE); + } + + /* error= */ + + for (i = 0; i < ARRAY_SIZE(error_severities); i++) + { + char cmdline[256]; + + strcpy(cmdline, "sc create " TEST_SERVICE_NAME " binpath= \"" TEST_SERVICE_BINARY "\" error= "); + strcat(cmdline, error_severities[i].param); + run_sc_exe(cmdline, &r); + todo_wine check_exit_code(SC_EXIT_SUCCESS); + check_test_service(SERVICE_WIN32_OWN_PROCESS, SERVICE_DEMAND_START, + error_severities[i].expected_error_control, "", NULL, BROKEN_CREATE); + delete_test_service(TRUE, TRUE); + } + + /* tag= */ + + run_sc_exe("sc create " TEST_SERVICE_NAME " binpath= \"" TEST_SERVICE_BINARY "\" tag= yes", &r); + todo_wine check_exit_code(SC_EXIT_INVALID_PARAMETER); + delete_test_service(FALSE, FALSE); + + run_sc_exe("sc create " TEST_SERVICE_NAME " binpath= \"" TEST_SERVICE_BINARY "\" tag= no", &r); + todo_wine check_exit_code(SC_EXIT_SUCCESS); + check_test_service(SERVICE_WIN32_OWN_PROCESS, SERVICE_DEMAND_START, SERVICE_ERROR_NORMAL, "", NULL, + BROKEN_CREATE); + delete_test_service(TRUE, TRUE); + + /* depend= */ + + run_sc_exe("sc create " TEST_SERVICE_NAME " binpath= \"" TEST_SERVICE_BINARY "\" depend= " TEST_SERVICE_NAME, &r); + todo_wine check_exit_code(SC_EXIT_CIRCULAR_DEPENDENCY); + delete_test_service(FALSE, FALSE); + + run_sc_exe("sc create " TEST_SERVICE_NAME2 " binpath= \"" TEST_SERVICE_BINARY "\" depend= " TEST_SERVICE_NAME, &r); + todo_wine check_exit_code(SC_EXIT_SUCCESS); + check_test_service2(SERVICE_WIN32_OWN_PROCESS, SERVICE_DEMAND_START, SERVICE_ERROR_NORMAL, + TEST_SERVICE_NAME, NULL, BROKEN_CREATE); + delete_test_service2(TRUE, TRUE); + + run_sc_exe("sc create " TEST_SERVICE_NAME " binpath= " TEST_SERVICE_BINARY, &r); + todo_wine check_exit_code(SC_EXIT_SUCCESS); + run_sc_exe("sc create " TEST_SERVICE_NAME2 " binpath= \"" TEST_SERVICE_BINARY "\" depend= " TEST_SERVICE_NAME, &r); + todo_wine check_exit_code(SC_EXIT_SUCCESS); + check_test_service2(SERVICE_WIN32_OWN_PROCESS, SERVICE_DEMAND_START, SERVICE_ERROR_NORMAL, + TEST_SERVICE_NAME, NULL, BROKEN_CREATE); + delete_test_service2(TRUE, TRUE); + delete_test_service(TRUE, TRUE); + + /* displayname= */ + + run_sc_exe("sc create " TEST_SERVICE_NAME " binpath= " TEST_SERVICE_BINARY + " displayname= \"Wine Test Service\"", &r); + todo_wine check_exit_code(SC_EXIT_SUCCESS); + check_test_service(SERVICE_WIN32_OWN_PROCESS, SERVICE_DEMAND_START, SERVICE_ERROR_NORMAL, "", + "Wine Test Service", BROKEN_CREATE); + delete_test_service(TRUE, TRUE); + + /* without spaces */ + + run_sc_exe("sc create " TEST_SERVICE_NAME2 " binpath=\"" TEST_SERVICE_BINARY "\" type=own start=auto" + " error=normal tag=no depend=" TEST_SERVICE_NAME " displayname=\"Wine Test Service\"", &r); + ok(r == SC_EXIT_SUCCESS || broken(r == SC_EXIT_INVALID_COMMAND_LINE), "got exit code %ld, expected %d\n", + r, SC_EXIT_SUCCESS); + if (r == SC_EXIT_SUCCESS) + { + check_test_service2(SERVICE_WIN32_OWN_PROCESS, SERVICE_AUTO_START, SERVICE_ERROR_NORMAL, + TEST_SERVICE_NAME, "Wine Test Service", BROKEN_DEPEND); + delete_test_service2(TRUE, FALSE); + } + else + { + delete_test_service2(FALSE, FALSE); + } + + /* case-insensitive */ + + run_sc_exe("SC CREATE " TEST_SERVICE_NAME2 " BINPATH= \"" TEST_SERVICE_BINARY "\" TYPE= OWN START= AUTO" + " ERROR= NORMAL TAG= NO DEPEND= " TEST_SERVICE_NAME " DISPLAYNAME= \"Wine Test Service\"", &r); + todo_wine check_exit_code(SC_EXIT_SUCCESS); + check_test_service2(SERVICE_WIN32_OWN_PROCESS, SERVICE_AUTO_START, SERVICE_ERROR_NORMAL, + TEST_SERVICE_NAME, "Wine Test Service", BROKEN_CREATE); + delete_test_service2(TRUE, TRUE); + +#undef delete_test_service2 +#undef check_test_service2 +#undef delete_test_service +#undef check_test_service +#undef check_exit_code +} + +/* taken from winetest, only whitespace changes */ +static int running_elevated(void) +{ + HANDLE token; + TOKEN_ELEVATION elevation_info; + DWORD size; + + /* Get the process token */ + if (!OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &token)) + return -1; + + /* Get the elevation info from the token */ + if (!GetTokenInformation(token, TokenElevation, &elevation_info, sizeof(TOKEN_ELEVATION), &size)) + { + CloseHandle(token); + return -1; + } + CloseHandle(token); + + return elevation_info.TokenIsElevated; +} + + +START_TEST(sc) +{ + SECURITY_ATTRIBUTES secattr = {sizeof(SECURITY_ATTRIBUTES), NULL, TRUE}; + BOOL elevated = running_elevated(); + + nul_file = CreateFileA("NUL", GENERIC_READ | GENERIC_WRITE, 0, &secattr, OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, NULL); + + scmgr = OpenSCManagerA(NULL, NULL, GENERIC_READ); + ok(!!scmgr, "OpenSCManagerA failed: %ld\n", GetLastError()); + + test_create_service(elevated); + + CloseServiceHandle(scmgr); + CloseHandle(nul_file); +} -- 2.36.0

April 29, 2022

[PATCH vkd3d v2 4/4] vkd3d: Map timeline semaphore values to fence virtual values and submit commands in worker threads.

by Conor McCarthy

Correct fence behaviour requires: Map monotonically increasing timeline values to fence virtual values to avoid invalid use of Vulkan timeline semaphores. In particular, non- increasing values and value jumps of >= 4G are required for d3d12. Create a worker thread for each queue to handle queue commands. This allows blocking of wait submission until an unblocking signal is submitted, so out-of-order waits are handled correctly when two d3d12 queues are mapped to the same Vulkan queue. Threaded queue submission also fixes the old fence implementation so it is fully functional, though a bit less efficient than timeline semaphores. Based in part on vkd3d-proton patches by Hans-Kristian Arntzen. Signed-off-by: Conor McCarthy <cmccarthy(a)codeweavers.com> --- v2: Always broadcast on the null event condition after signaling. Threaded command queues on their own seem to bring out the problems with the existing fence implementation, which results in games crashing. Combining them with the fence changes avoids that issue. --- libs/vkd3d/command.c | 895 +++++++++++++++++++++---------------- libs/vkd3d/device.c | 79 ---- libs/vkd3d/vkd3d_private.h | 81 +++- tests/d3d12.c | 4 +- 4 files changed, 565 insertions(+), 494 deletions(-) diff --git a/libs/vkd3d/command.c b/libs/vkd3d/command.c index 55e6be58..4efe391c 100644 --- a/libs/vkd3d/command.c +++ b/libs/vkd3d/command.c @@ -23,6 +23,7 @@ static void d3d12_fence_incref(struct d3d12_fence *fence); static void d3d12_fence_decref(struct d3d12_fence *fence); static HRESULT d3d12_fence_signal(struct d3d12_fence *fence, uint64_t value, VkFence vk_fence); +static void d3d12_fence_signal_timeline_semaphore(struct d3d12_fence *fence, uint64_t timeline_value); HRESULT vkd3d_queue_create(struct d3d12_device *device, uint32_t family_index, const VkQueueFamilyProperties *properties, struct vkd3d_queue **queue) @@ -48,9 +49,6 @@ HRESULT vkd3d_queue_create(struct d3d12_device *device, object->vk_queue_flags = properties->queueFlags; object->timestamp_bits = properties->timestampValidBits; - object->wait_completion_semaphore = VK_NULL_HANDLE; - object->pending_wait_completion_value = 0; - object->semaphores = NULL; object->semaphores_size = 0; object->semaphore_count = 0; @@ -66,20 +64,6 @@ HRESULT vkd3d_queue_create(struct d3d12_device *device, return S_OK; } -bool vkd3d_queue_init_timeline_semaphore(struct vkd3d_queue *queue, struct d3d12_device *device) -{ - VkResult vr; - - if (!queue->wait_completion_semaphore - && (vr = vkd3d_create_timeline_semaphore(device, 0, &queue->wait_completion_semaphore)) < 0) - { - WARN("Failed to create timeline semaphore, vr %d.\n", vr); - return false; - } - - return true; -} - void vkd3d_queue_destroy(struct vkd3d_queue *queue, struct d3d12_device *device) { const struct vkd3d_vk_device_procs *vk_procs = &device->vk_procs; @@ -94,8 +78,6 @@ void vkd3d_queue_destroy(struct vkd3d_queue *queue, struct d3d12_device *device) vkd3d_free(queue->semaphores); - VK_CALL(vkDestroySemaphore(device->vk_device, queue->wait_completion_semaphore, NULL)); - for (i = 0; i < ARRAY_SIZE(queue->old_vk_semaphores); ++i) { if (queue->old_vk_semaphores[i]) @@ -265,7 +247,7 @@ static VkResult vkd3d_queue_create_vk_semaphore_locked(struct vkd3d_queue *queue } /* Fence worker thread */ -static HRESULT vkd3d_enqueue_gpu_fence(struct vkd3d_fence_worker *worker, +static bool vkd3d_enqueue_gpu_fence(struct vkd3d_fence_worker *worker, VkFence vk_fence, struct d3d12_fence *fence, uint64_t value, struct vkd3d_queue *queue, uint64_t queue_sequence_number) { @@ -277,7 +259,7 @@ static HRESULT vkd3d_enqueue_gpu_fence(struct vkd3d_fence_worker *worker, if ((rc = vkd3d_mutex_lock(&worker->mutex))) { ERR("Failed to lock mutex, error %d.\n", rc); - return hresult_from_errno(rc); + return false; } if (!vkd3d_array_reserve((void **)&worker->fences, &worker->fences_size, @@ -285,7 +267,7 @@ static HRESULT vkd3d_enqueue_gpu_fence(struct vkd3d_fence_worker *worker, { ERR("Failed to add GPU fence.\n"); vkd3d_mutex_unlock(&worker->mutex); - return E_OUTOFMEMORY; + return false; } waiting_fence = &worker->fences[worker->fence_count++]; @@ -299,7 +281,7 @@ static HRESULT vkd3d_enqueue_gpu_fence(struct vkd3d_fence_worker *worker, vkd3d_cond_signal(&worker->cond); vkd3d_mutex_unlock(&worker->mutex); - return S_OK; + return true; } static void vkd3d_wait_for_gpu_timeline_semaphore(struct vkd3d_fence_worker *worker, @@ -308,9 +290,7 @@ static void vkd3d_wait_for_gpu_timeline_semaphore(struct vkd3d_fence_worker *wor const struct d3d12_device *device = worker->device; const struct vkd3d_vk_device_procs *vk_procs = &device->vk_procs; VkSemaphoreWaitInfoKHR wait_info; - uint64_t counter_value; VkResult vr; - HRESULT hr; wait_info.sType = VK_STRUCTURE_TYPE_SEMAPHORE_WAIT_INFO_KHR; wait_info.pNext = NULL; @@ -328,19 +308,10 @@ static void vkd3d_wait_for_gpu_timeline_semaphore(struct vkd3d_fence_worker *wor return; } - if ((vr = VK_CALL(vkGetSemaphoreCounterValueKHR(device->vk_device, waiting_fence->u.vk_semaphore, - &counter_value))) < 0) - { - ERR("Failed to get Vulkan semaphore value, vr %d.\n", vr); - } - else - { - TRACE("Signaling fence %p value %#"PRIx64".\n", waiting_fence->fence, waiting_fence->value); - if (FAILED(hr = d3d12_fence_signal(waiting_fence->fence, counter_value, VK_NULL_HANDLE))) - ERR("Failed to signal D3D12 fence, hr %#x.\n", hr); + TRACE("Signaling fence %p value %#"PRIx64".\n", waiting_fence->fence, waiting_fence->value); + d3d12_fence_signal_timeline_semaphore(waiting_fence->fence, waiting_fence->value); - d3d12_fence_decref(waiting_fence->fence); - } + d3d12_fence_decref(waiting_fence->fence); } static void vkd3d_wait_for_gpu_fence(struct vkd3d_fence_worker *worker, @@ -434,7 +405,7 @@ static HRESULT vkd3d_fence_worker_start(struct vkd3d_fence_worker *worker, worker->fences = NULL; worker->fences_size = 0; - worker->wait_for_gpu_fence = device->use_timeline_semaphores + worker->wait_for_gpu_fence = device->vk_info.KHR_timeline_semaphore ? vkd3d_wait_for_gpu_timeline_semaphore : vkd3d_wait_for_gpu_fence; if ((rc = vkd3d_mutex_init(&worker->mutex))) @@ -606,17 +577,17 @@ static void d3d12_fence_garbage_collect_vk_semaphores_locked(struct d3d12_fence current = &fence->semaphores[i]; /* The semaphore doesn't have a pending signal operation if the fence * was signaled. */ - if ((current->vk_fence || current->is_acquired) && !destroy_all) + if ((current->u.binary.vk_fence || current->u.binary.is_acquired) && !destroy_all) { ++i; continue; } - if (current->vk_fence) + if (current->u.binary.vk_fence) WARN("Destroying potentially pending semaphore.\n"); - assert(!current->is_acquired); + assert(!current->u.binary.is_acquired); - VK_CALL(vkDestroySemaphore(device->vk_device, current->vk_semaphore, NULL)); + VK_CALL(vkDestroySemaphore(device->vk_device, current->u.binary.vk_semaphore, NULL)); fence->semaphores[i] = fence->semaphores[--fence->semaphore_count]; } @@ -652,23 +623,16 @@ static void d3d12_fence_destroy_vk_objects(struct d3d12_fence *fence) vkd3d_mutex_unlock(&fence->mutex); } -static struct vkd3d_signaled_semaphore *d3d12_fence_acquire_vk_semaphore(struct d3d12_fence *fence, +static struct vkd3d_signaled_semaphore *d3d12_fence_acquire_vk_semaphore_locked(struct d3d12_fence *fence, uint64_t value, uint64_t *completed_value) { struct vkd3d_signaled_semaphore *semaphore; struct vkd3d_signaled_semaphore *current; uint64_t semaphore_value; unsigned int i; - int rc; TRACE("fence %p, value %#"PRIx64".\n", fence, value); - if ((rc = vkd3d_mutex_lock(&fence->mutex))) - { - ERR("Failed to lock mutex, error %d.\n", rc); - return VK_NULL_HANDLE; - } - semaphore = NULL; semaphore_value = ~(uint64_t)0; @@ -676,7 +640,7 @@ static struct vkd3d_signaled_semaphore *d3d12_fence_acquire_vk_semaphore(struct { current = &fence->semaphores[i]; /* Prefer a semaphore with the smallest value. */ - if (!current->is_acquired && current->value >= value && semaphore_value >= current->value) + if (!current->u.binary.is_acquired && current->value >= value && semaphore_value >= current->value) { semaphore = current; semaphore_value = current->value; @@ -686,12 +650,10 @@ static struct vkd3d_signaled_semaphore *d3d12_fence_acquire_vk_semaphore(struct } if (semaphore) - semaphore->is_acquired = true; + semaphore->u.binary.is_acquired = true; *completed_value = fence->value; - vkd3d_mutex_unlock(&fence->mutex); - return semaphore; } @@ -705,7 +667,7 @@ static void d3d12_fence_remove_vk_semaphore(struct d3d12_fence *fence, struct vk return; } - assert(semaphore->is_acquired); + assert(semaphore->u.binary.is_acquired); *semaphore = fence->semaphores[--fence->semaphore_count]; @@ -722,32 +684,24 @@ static void d3d12_fence_release_vk_semaphore(struct d3d12_fence *fence, struct v return; } - assert(semaphore->is_acquired); - semaphore->is_acquired = false; + assert(semaphore->u.binary.is_acquired); + semaphore->u.binary.is_acquired = false; vkd3d_mutex_unlock(&fence->mutex); } -static HRESULT d3d12_fence_add_vk_semaphore(struct d3d12_fence *fence, - VkSemaphore vk_semaphore, VkFence vk_fence, uint64_t value) +static bool d3d12_fence_add_vk_semaphore(struct d3d12_fence *fence, VkSemaphore vk_semaphore, + VkFence vk_fence, uint64_t value, const struct vkd3d_queue *signalling_queue) { struct vkd3d_signaled_semaphore *semaphore; - HRESULT hr = S_OK; int rc; TRACE("fence %p, value %#"PRIx64".\n", fence, value); - if (!(semaphore = vkd3d_malloc(sizeof(*semaphore)))) - { - ERR("Failed to add semaphore.\n"); - return E_OUTOFMEMORY; - } - if ((rc = vkd3d_mutex_lock(&fence->mutex))) { ERR("Failed to lock mutex, error %d.\n", rc); - vkd3d_free(semaphore); - return E_FAIL; + return false; } d3d12_fence_garbage_collect_vk_semaphores_locked(fence, false); @@ -762,16 +716,17 @@ static HRESULT d3d12_fence_add_vk_semaphore(struct d3d12_fence *fence, semaphore = &fence->semaphores[fence->semaphore_count++]; semaphore->value = value; - semaphore->vk_semaphore = vk_semaphore; - semaphore->vk_fence = vk_fence; - semaphore->is_acquired = false; + semaphore->u.binary.vk_semaphore = vk_semaphore; + semaphore->u.binary.vk_fence = vk_fence; + semaphore->u.binary.is_acquired = false; + semaphore->signalling_queue = signalling_queue; vkd3d_mutex_unlock(&fence->mutex); - return hr; + return true; } -static bool d3d12_fence_signal_external_events_locked(struct d3d12_fence *fence) +static void d3d12_fence_signal_external_events_locked(struct d3d12_fence *fence) { struct d3d12_device *device = fence->device; bool signal_null_event_cond = false; @@ -803,7 +758,21 @@ static bool d3d12_fence_signal_external_events_locked(struct d3d12_fence *fence) fence->event_count = j; - return signal_null_event_cond; + if (signal_null_event_cond) + vkd3d_cond_broadcast(&fence->null_event_cond); +} + +static void d3d12_fence_update_pending_value_locked(struct d3d12_fence *fence) +{ + uint64_t new_max_pending_value = 0; + unsigned int i; + + for (i = 0; i < fence->semaphore_count; ++i) + new_max_pending_value = max(fence->semaphores[i].value, new_max_pending_value); + + fence->max_pending_value = max(fence->value, new_max_pending_value); + /* If we're signalling the fence, wake up any submission threads which can now safely kick work. */ + vkd3d_cond_broadcast(&fence->cond); } static HRESULT d3d12_fence_signal(struct d3d12_fence *fence, uint64_t value, VkFence vk_fence) @@ -821,8 +790,7 @@ static HRESULT d3d12_fence_signal(struct d3d12_fence *fence, uint64_t value, VkF fence->value = value; - if (d3d12_fence_signal_external_events_locked(fence)) - vkd3d_cond_broadcast(&fence->null_event_cond); + d3d12_fence_signal_external_events_locked(fence); if (vk_fence) { @@ -831,8 +799,8 @@ static HRESULT d3d12_fence_signal(struct d3d12_fence *fence, uint64_t value, VkF for (i = 0; i < fence->semaphore_count; ++i) { current = &fence->semaphores[i]; - if (current->vk_fence == vk_fence) - current->vk_fence = VK_NULL_HANDLE; + if (current->u.binary.vk_fence == vk_fence) + current->u.binary.vk_fence = VK_NULL_HANDLE; } for (i = 0; i < ARRAY_SIZE(fence->old_vk_fences); ++i) @@ -849,11 +817,135 @@ static HRESULT d3d12_fence_signal(struct d3d12_fence *fence, uint64_t value, VkF VK_CALL(vkDestroyFence(device->vk_device, vk_fence, NULL)); } + d3d12_fence_update_pending_value_locked(fence); + vkd3d_mutex_unlock(&fence->mutex); return S_OK; } +static void d3d12_fence_block_until_pending_value_reaches_locked(struct d3d12_fence *fence, uint64_t pending_value) +{ + while (pending_value > fence->max_pending_value) + { + TRACE("Blocking wait on fence %p until it reaches 0x%"PRIx64".\n", fence, pending_value); + vkd3d_cond_wait(&fence->cond, &fence->mutex); + } +} + +static bool d3d12_fence_can_elide_wait_semaphore_locked(const struct d3d12_fence *fence, + uint64_t wait_value, const struct vkd3d_queue *waiting_queue) +{ + unsigned int i; + + /* Relevant if the semaphore has been signalled already on host. + * We should not wait on the timeline semaphore directly, we can simply submit in-place. */ + if (fence->value >= wait_value) + return true; + + /* We can elide a wait if we can use the submission order guarantee. + * If there is a pending signal on this queue which will satisfy the wait, + * submission barrier will implicitly complete the wait, + * and we don't have to eat the overhead of submitting an extra wait on top. + * This will essentially always trigger on single-queue. + */ + for (i = 0; i < fence->semaphore_count; ++i) + { + if (fence->semaphores[i].signalling_queue == waiting_queue && fence->semaphores[i].value >= wait_value) + return true; + } + + return false; +} + +static uint64_t d3d12_fence_add_pending_signal_locked(struct d3d12_fence *fence, uint64_t virtual_value, + const struct vkd3d_queue *signalling_queue) +{ + struct vkd3d_signaled_semaphore *semaphore; + + if (!vkd3d_array_reserve((void**)&fence->semaphores, &fence->semaphores_size, + fence->semaphore_count + 1, sizeof(*fence->semaphores))) + { + return 0; + } + + semaphore = &fence->semaphores[fence->semaphore_count++]; + semaphore->value = virtual_value; + semaphore->u.timeline_value = ++fence->pending_timeline_value; + semaphore->signalling_queue = signalling_queue; + return fence->pending_timeline_value; +} + +static uint64_t d3d12_fence_get_timeline_wait_value_locked(struct d3d12_fence *fence, uint64_t virtual_value) +{ + uint64_t target_timeline_value = UINT64_MAX; + unsigned int i; + + /* This shouldn't happen, we will have elided the wait completely in can_elide_wait_semaphore_locked. */ + assert(virtual_value > fence->value); + + /* Find the smallest physical value which is at least the virtual value. */ + for (i = 0; i < fence->semaphore_count; ++i) + { + if (virtual_value <= fence->semaphores[i].value) + target_timeline_value = min(target_timeline_value, fence->semaphores[i].u.timeline_value); + } + + if (target_timeline_value == UINT64_MAX) + { + FIXME("Cannot find a pending timeline semaphore wait value. Emitting a noop wait.\n"); + return 0; + } + else + { + return target_timeline_value; + } +} + +static void d3d12_fence_signal_timeline_semaphore(struct d3d12_fence *fence, uint64_t timeline_value) +{ + bool did_signal; + unsigned int i; + int rc; + + if ((rc = vkd3d_mutex_lock(&fence->mutex))) + { + ERR("Failed to lock mutex, error %d.\n", rc); + return; + } + + /* With multiple fence workers, it is possible that signal calls are out of + * order. The physical value itself is monotonic, but we need to make sure + * that all signals happen in correct order if there are fence rewinds. + * We don't expect the loop to run more than once, but there might be + * extreme edge cases where we signal 2 or more. */ + while (fence->timeline_value < timeline_value) + { + ++fence->timeline_value; + did_signal = false; + + for (i = 0; i < fence->semaphore_count; ++i) + { + if (fence->timeline_value == fence->semaphores[i].u.timeline_value) + { + fence->value = fence->semaphores[i].value; + d3d12_fence_signal_external_events_locked(fence); + fence->semaphores[i] = fence->semaphores[--fence->semaphore_count]; + did_signal = true; + break; + } + } + + if (!did_signal) + FIXME("Did not signal a virtual value.\n"); + } + + /* In case we have a rewind signalled from GPU, we need to recompute the max pending timeline value. */ + d3d12_fence_update_pending_value_locked(fence); + + vkd3d_mutex_unlock(&fence->mutex); +} + static HRESULT STDMETHODCALLTYPE d3d12_fence_QueryInterface(ID3D12Fence *iface, REFIID riid, void **object) { @@ -921,6 +1013,8 @@ static void d3d12_fence_decref(struct d3d12_fence *fence) vkd3d_free(fence->semaphores); if ((rc = vkd3d_mutex_destroy(&fence->mutex))) ERR("Failed to destroy mutex, error %d.\n", rc); + if ((rc = vkd3d_cond_destroy(&fence->cond))) + ERR("Failed to destroy cond, error %d.\n", rc); vkd3d_cond_destroy(&fence->null_event_cond); vkd3d_free(fence); @@ -1060,100 +1154,8 @@ static HRESULT STDMETHODCALLTYPE d3d12_fence_SetEventOnCompletion(ID3D12Fence *i return S_OK; } -static inline bool d3d12_fence_gpu_wait_is_completed(const struct d3d12_fence *fence, unsigned int i) -{ - const struct d3d12_device *device = fence->device; - const struct vkd3d_vk_device_procs *vk_procs; - uint64_t value; - VkResult vr; - - vk_procs = &device->vk_procs; - - if ((vr = VK_CALL(vkGetSemaphoreCounterValueKHR(device->vk_device, - fence->gpu_waits[i].queue->wait_completion_semaphore, &value))) >= 0) - { - return value >= fence->gpu_waits[i].pending_value; - } - - ERR("Failed to get Vulkan semaphore status, vr %d.\n", vr); - return true; -} - -static inline bool d3d12_fence_has_pending_gpu_ops_locked(struct d3d12_fence *fence) -{ - const struct d3d12_device *device = fence->device; - const struct vkd3d_vk_device_procs *vk_procs; - uint64_t value; - unsigned int i; - VkResult vr; - - for (i = 0; i < fence->gpu_wait_count; ++i) - { - if (d3d12_fence_gpu_wait_is_completed(fence, i) && i < --fence->gpu_wait_count) - fence->gpu_waits[i] = fence->gpu_waits[fence->gpu_wait_count]; - } - if (fence->gpu_wait_count) - return true; - - /* Check for pending signals too. */ - if (fence->value >= fence->pending_timeline_value) - return false; - - vk_procs = &device->vk_procs; - - /* Check the actual semaphore value in case fence->value update is lagging. */ - if ((vr = VK_CALL(vkGetSemaphoreCounterValueKHR(device->vk_device, fence->timeline_semaphore, &value))) < 0) - { - ERR("Failed to get Vulkan semaphore status, vr %d.\n", vr); - return false; - } - - return value < fence->pending_timeline_value; -} - -/* Replace the VkSemaphore with a new one to allow a lower value to be set. Ideally apps will - * only use this to reset the fence when no operations are pending on the queue. */ -static HRESULT d3d12_fence_reinit_timeline_semaphore_locked(struct d3d12_fence *fence, uint64_t value) -{ - const struct d3d12_device *device = fence->device; - const struct vkd3d_vk_device_procs *vk_procs; - VkSemaphore timeline_semaphore; - VkResult vr; - - if (d3d12_fence_has_pending_gpu_ops_locked(fence)) - { - /* This situation is not very likely because it means a fence with pending waits and/or signals was - * signalled on the CPU to a lower value. For now, emit a fixme so it can be patched if necessary. - * A patch already exists for this but it's not pretty. */ - FIXME("Unable to re-initialise timeline semaphore to a lower value due to pending GPU ops.\n"); - return E_FAIL; - } - - if ((vr = vkd3d_create_timeline_semaphore(device, value, &timeline_semaphore)) < 0) - { - WARN("Failed to create timeline semaphore, vr %d.\n", vr); - return hresult_from_vk_result(vr); - } - - fence->value = value; - fence->pending_timeline_value = value; - - WARN("Replacing timeline semaphore with a new object.\n"); - - vk_procs = &device->vk_procs; - - VK_CALL(vkDestroySemaphore(device->vk_device, fence->timeline_semaphore, NULL)); - fence->timeline_semaphore = timeline_semaphore; - - return S_OK; -} - static HRESULT d3d12_fence_signal_cpu_timeline_semaphore(struct d3d12_fence *fence, uint64_t value) { - const struct d3d12_device *device = fence->device; - VkSemaphoreSignalInfoKHR info; - HRESULT hr = S_OK; - VkResult vr; int rc; if ((rc = vkd3d_mutex_lock(&fence->mutex))) @@ -1162,48 +1164,13 @@ static HRESULT d3d12_fence_signal_cpu_timeline_semaphore(struct d3d12_fence *fen return hresult_from_errno(rc); } - /* We must only signal a value which is greater than the current value. - * That value can be in the range of current known value (fence->value), or as large as pending_timeline_value. - * Pending timeline value signal might be blocked by another synchronization primitive, and thus statically - * cannot be that value, so the safest thing to do is to check the current value which is updated by the fence - * wait thread continuously. This check is technically racy since the value might be immediately out of date, - * but there is no way to avoid this. */ - if (value > fence->value) - { - const struct vkd3d_vk_device_procs *vk_procs = &device->vk_procs; - - /* Sanity check against the delta limit. */ - if (value - fence->value > device->vk_info.timeline_semaphore_properties.maxTimelineSemaphoreValueDifference) - { - FIXME("Timeline semaphore delta is %"PRIu64", but implementation only supports a delta of %"PRIu64".\n", - value - fence->value, device->vk_info.timeline_semaphore_properties.maxTimelineSemaphoreValueDifference); - } - - info.sType = VK_STRUCTURE_TYPE_SEMAPHORE_SIGNAL_INFO_KHR; - info.pNext = NULL; - info.semaphore = fence->timeline_semaphore; - info.value = value; - if ((vr = VK_CALL(vkSignalSemaphoreKHR(device->vk_device, &info))) >= 0) - { - fence->value = value; - if (value > fence->pending_timeline_value) - fence->pending_timeline_value = value; - } - else - { - ERR("Failed to signal timeline semaphore, vr %d.\n", vr); - hr = hresult_from_vk_result(vr); - } - } - else if (value < fence->value) - { - hr = d3d12_fence_reinit_timeline_semaphore_locked(fence, value); - } - + fence->value = value; d3d12_fence_signal_external_events_locked(fence); + d3d12_fence_update_pending_value_locked(fence); vkd3d_mutex_unlock(&fence->mutex); - return hr; + + return S_OK; } static HRESULT STDMETHODCALLTYPE d3d12_fence_Signal(ID3D12Fence *iface, UINT64 value) @@ -1256,6 +1223,7 @@ static HRESULT d3d12_fence_init(struct d3d12_fence *fence, struct d3d12_device * fence->refcount = 1; fence->value = initial_value; + fence->max_pending_value = initial_value; if ((rc = vkd3d_mutex_init(&fence->mutex))) { @@ -1263,11 +1231,18 @@ static HRESULT d3d12_fence_init(struct d3d12_fence *fence, struct d3d12_device * return hresult_from_errno(rc); } + if ((rc = vkd3d_cond_init(&fence->cond))) + { + ERR("Failed to initialize cond variable, error %d.\n", rc); + hr = hresult_from_errno(rc); + goto fail_destroy_mutex; + } + if ((rc = vkd3d_cond_init(&fence->null_event_cond))) { ERR("Failed to initialize cond variable, error %d.\n", rc); - vkd3d_mutex_destroy(&fence->mutex); - return hresult_from_errno(rc); + hr = hresult_from_errno(rc); + goto fail_destroy_cond; } if (flags) @@ -1278,14 +1253,15 @@ static HRESULT d3d12_fence_init(struct d3d12_fence *fence, struct d3d12_device * fence->event_count = 0; fence->timeline_semaphore = VK_NULL_HANDLE; - if (device->use_timeline_semaphores && (vr = vkd3d_create_timeline_semaphore(device, initial_value, + fence->timeline_value = 0; + fence->pending_timeline_value = 0; + if (device->vk_info.KHR_timeline_semaphore && (vr = vkd3d_create_timeline_semaphore(device, 0, &fence->timeline_semaphore)) < 0) { WARN("Failed to create timeline semaphore, vr %d.\n", vr); - return hresult_from_vk_result(vr); + hr = hresult_from_vk_result(vr); + goto fail_destroy_null_cond; } - fence->pending_timeline_value = initial_value; - fence->gpu_wait_count = 0; fence->semaphores = NULL; fence->semaphores_size = 0; @@ -1295,14 +1271,21 @@ static HRESULT d3d12_fence_init(struct d3d12_fence *fence, struct d3d12_device * if (FAILED(hr = vkd3d_private_store_init(&fence->private_store))) { - vkd3d_mutex_destroy(&fence->mutex); - vkd3d_cond_destroy(&fence->null_event_cond); - return hr; + goto fail_destroy_null_cond; } d3d12_device_add_ref(fence->device = device); return S_OK; + +fail_destroy_null_cond: + vkd3d_cond_destroy(&fence->null_event_cond); +fail_destroy_cond: + vkd3d_cond_destroy(&fence->cond); +fail_destroy_mutex: + vkd3d_mutex_destroy(&fence->mutex); + + return hr; } HRESULT d3d12_fence_create(struct d3d12_device *device, @@ -6076,6 +6059,41 @@ HRESULT d3d12_command_list_create(struct d3d12_device *device, return S_OK; } +static HRESULT d3d12_command_queue_add_submission_locked(struct d3d12_command_queue *queue, + const struct d3d12_command_queue_submission *sub) +{ + if (!vkd3d_array_reserve((void**)&queue->submissions, &queue->submissions_size, + queue->submissions_count + 1, sizeof(*queue->submissions))) + { + return E_OUTOFMEMORY; + } + + queue->submissions[queue->submissions_count++] = *sub; + vkd3d_cond_signal(&queue->submission_cond); + return S_OK; +} + +static HRESULT d3d12_command_queue_add_submission(struct d3d12_command_queue *queue, + const struct d3d12_command_queue_submission *sub) +{ + HRESULT hr; + + vkd3d_mutex_lock(&queue->submission_mutex); + hr = d3d12_command_queue_add_submission_locked(queue, sub); + vkd3d_mutex_unlock(&queue->submission_mutex); + return hr; +} + +static void d3d12_command_queue_submit_stop(struct d3d12_command_queue *queue) +{ + struct d3d12_command_queue_submission sub; + HRESULT hr; + + sub.type = VKD3D_SUBMISSION_STOP; + if (FAILED(hr = d3d12_command_queue_add_submission(queue, &sub))) + ERR("Failed to submit command, hr %#x.\n", hr); +} + /* ID3D12CommandQueue */ static inline struct d3d12_command_queue *impl_from_ID3D12CommandQueue(ID3D12CommandQueue *iface) { @@ -6126,7 +6144,12 @@ static ULONG STDMETHODCALLTYPE d3d12_command_queue_Release(ID3D12CommandQueue *i struct d3d12_device *device = command_queue->device; vkd3d_fence_worker_stop(&command_queue->fence_worker, device); + d3d12_command_queue_submit_stop(command_queue); + vkd3d_join_thread(device->vkd3d_instance, &command_queue->submission_thread); + vkd3d_mutex_destroy(&command_queue->submission_mutex); + vkd3d_cond_destroy(&command_queue->submission_cond); + vkd3d_free(command_queue->submissions); vkd3d_private_store_destroy(&command_queue->private_store); vkd3d_free(command_queue); @@ -6231,18 +6254,17 @@ static void STDMETHODCALLTYPE d3d12_command_queue_ExecuteCommandLists(ID3D12Comm UINT command_list_count, ID3D12CommandList * const *command_lists) { struct d3d12_command_queue *command_queue = impl_from_ID3D12CommandQueue(iface); - const struct vkd3d_vk_device_procs *vk_procs; + struct d3d12_command_queue_submission sub; struct d3d12_command_list *cmd_list; - struct VkSubmitInfo submit_desc; VkCommandBuffer *buffers; - VkQueue vk_queue; unsigned int i; - VkResult vr; + HRESULT hr; TRACE("iface %p, command_list_count %u, command_lists %p.\n", iface, command_list_count, command_lists); - vk_procs = &command_queue->device->vk_procs; + if (!command_list_count) + return; if (!(buffers = vkd3d_calloc(command_list_count, sizeof(*buffers)))) { @@ -6265,29 +6287,11 @@ static void STDMETHODCALLTYPE d3d12_command_queue_ExecuteCommandLists(ID3D12Comm buffers[i] = cmd_list->vk_command_buffer; } - submit_desc.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO; - submit_desc.pNext = NULL; - submit_desc.waitSemaphoreCount = 0; - submit_desc.pWaitSemaphores = NULL; - submit_desc.pWaitDstStageMask = NULL; - submit_desc.commandBufferCount = command_list_count; - submit_desc.pCommandBuffers = buffers; - submit_desc.signalSemaphoreCount = 0; - submit_desc.pSignalSemaphores = NULL; - - if (!(vk_queue = vkd3d_queue_acquire(command_queue->vkd3d_queue))) - { - ERR("Failed to acquire queue %p.\n", command_queue->vkd3d_queue); - vkd3d_free(buffers); - return; - } - - if ((vr = VK_CALL(vkQueueSubmit(vk_queue, 1, &submit_desc, VK_NULL_HANDLE))) < 0) - ERR("Failed to submit queue(s), vr %d.\n", vr); - - vkd3d_queue_release(command_queue->vkd3d_queue); - - vkd3d_free(buffers); + sub.type = VKD3D_SUBMISSION_EXECUTE; + sub.u.execute.cmd = buffers; + sub.u.execute.cmd_count = command_list_count; + if (FAILED(hr = d3d12_command_queue_add_submission(command_queue, &sub))) + ERR("Failed to submit command, hr %#x.\n", hr); } static void STDMETHODCALLTYPE d3d12_command_queue_SetMarker(ID3D12CommandQueue *iface, @@ -6309,39 +6313,7 @@ static void STDMETHODCALLTYPE d3d12_command_queue_EndEvent(ID3D12CommandQueue *i FIXME("iface %p stub!\n", iface); } -static HRESULT d3d12_fence_update_gpu_signal_timeline_semaphore(struct d3d12_fence *fence, uint64_t value) -{ - const struct d3d12_device *device = fence->device; - int rc; - - if ((rc = vkd3d_mutex_lock(&fence->mutex))) - { - ERR("Failed to lock mutex, error %d.\n", rc); - return hresult_from_errno(rc); - } - - /* If we're attempting to async signal a fence with a value which is not strictly increasing the payload value, - * warn about this case. Do not treat this as an error since it works at least with RADV and Nvidia drivers and - * there's no workaround on the GPU side. */ - if (value <= fence->pending_timeline_value) - { - WARN("Fence %p values are not strictly increasing. Pending values: old %"PRIu64", new %"PRIu64".\n", - fence, fence->pending_timeline_value, value); - } - /* Sanity check against the delta limit. Use the current fence value. */ - else if (value - fence->value > device->vk_info.timeline_semaphore_properties.maxTimelineSemaphoreValueDifference) - { - FIXME("Timeline semaphore delta is %"PRIu64", but implementation only supports a delta of %"PRIu64".\n", - value - fence->value, device->vk_info.timeline_semaphore_properties.maxTimelineSemaphoreValueDifference); - } - fence->pending_timeline_value = value; - - vkd3d_mutex_unlock(&fence->mutex); - - return S_OK; -} - -static HRESULT vkd3d_enqueue_timeline_semaphore(struct vkd3d_fence_worker *worker, VkSemaphore vk_semaphore, +static void vkd3d_enqueue_timeline_semaphore(struct vkd3d_fence_worker *worker, VkSemaphore vk_semaphore, struct d3d12_fence *fence, uint64_t value, struct vkd3d_queue *queue) { struct vkd3d_waiting_fence *waiting_fence; @@ -6352,7 +6324,7 @@ static HRESULT vkd3d_enqueue_timeline_semaphore(struct vkd3d_fence_worker *worke if ((rc = vkd3d_mutex_lock(&worker->mutex))) { ERR("Failed to lock mutex, error %d.\n", rc); - return hresult_from_errno(rc); + return; } if (!vkd3d_array_reserve((void **)&worker->fences, &worker->fences_size, @@ -6360,7 +6332,7 @@ static HRESULT vkd3d_enqueue_timeline_semaphore(struct vkd3d_fence_worker *worke { ERR("Failed to add GPU timeline semaphore.\n"); vkd3d_mutex_unlock(&worker->mutex); - return E_OUTOFMEMORY; + return; } waiting_fence = &worker->fences[worker->fence_count++]; @@ -6372,39 +6344,58 @@ static HRESULT vkd3d_enqueue_timeline_semaphore(struct vkd3d_fence_worker *worke vkd3d_cond_signal(&worker->cond); vkd3d_mutex_unlock(&worker->mutex); - - return S_OK; } static HRESULT STDMETHODCALLTYPE d3d12_command_queue_Signal(ID3D12CommandQueue *iface, ID3D12Fence *fence_iface, UINT64 value) { struct d3d12_command_queue *command_queue = impl_from_ID3D12CommandQueue(iface); + struct d3d12_fence *fence = unsafe_impl_from_ID3D12Fence(fence_iface); + struct d3d12_command_queue_submission sub; + + TRACE("iface %p, fence %p, value %#"PRIx64".\n", iface, fence_iface, value); + + d3d12_fence_incref(fence); + + sub.type = VKD3D_SUBMISSION_SIGNAL; + sub.u.signal.fence = fence; + sub.u.signal.value = value; + return d3d12_command_queue_add_submission(command_queue, &sub); +} + +static void d3d12_command_queue_signal(struct d3d12_command_queue *command_queue, + struct d3d12_fence *fence, uint64_t value) +{ VkTimelineSemaphoreSubmitInfoKHR timeline_submit_info; const struct vkd3d_vk_device_procs *vk_procs; VkSemaphore vk_semaphore = VK_NULL_HANDLE; VkFence vk_fence = VK_NULL_HANDLE; struct vkd3d_queue *vkd3d_queue; uint64_t sequence_number = 0; + uint64_t timeline_value = 0; struct d3d12_device *device; - struct d3d12_fence *fence; VkSubmitInfo submit_info; VkQueue vk_queue; + int rc = -1; VkResult vr; - HRESULT hr; - - TRACE("iface %p, fence %p, value %#"PRIx64".\n", iface, fence_iface, value); device = command_queue->device; vk_procs = &device->vk_procs; vkd3d_queue = command_queue->vkd3d_queue; - fence = unsafe_impl_from_ID3D12Fence(fence_iface); - - if (device->use_timeline_semaphores) + if (device->vk_info.KHR_timeline_semaphore) { - if (FAILED(hr = d3d12_fence_update_gpu_signal_timeline_semaphore(fence, value))) - return hr; + if ((rc = vkd3d_mutex_lock(&fence->mutex))) + { + ERR("Failed to lock mutex, error %d.\n", rc); + return; + } + if (!(timeline_value = d3d12_fence_add_pending_signal_locked(fence, value, vkd3d_queue))) + { + ERR("Failed to add pending signal.\n"); + vkd3d_mutex_unlock(&fence->mutex); + return; + } vk_semaphore = fence->timeline_semaphore; assert(vk_semaphore); @@ -6414,18 +6405,17 @@ static HRESULT STDMETHODCALLTYPE d3d12_command_queue_Signal(ID3D12CommandQueue * if ((vr = d3d12_fence_create_vk_fence(fence, &vk_fence)) < 0) { WARN("Failed to create Vulkan fence, vr %d.\n", vr); - goto fail_vkresult; + goto fail; } } if (!(vk_queue = vkd3d_queue_acquire(vkd3d_queue))) { ERR("Failed to acquire queue %p.\n", vkd3d_queue); - hr = E_FAIL; goto fail; } - if (!device->use_timeline_semaphores && (vr = vkd3d_queue_create_vk_semaphore_locked(vkd3d_queue, + if (!device->vk_info.KHR_timeline_semaphore && (vr = vkd3d_queue_create_vk_semaphore_locked(vkd3d_queue, device, &vk_semaphore)) < 0) { ERR("Failed to create Vulkan semaphore, vr %d.\n", vr); @@ -6442,11 +6432,11 @@ static HRESULT STDMETHODCALLTYPE d3d12_command_queue_Signal(ID3D12CommandQueue * submit_info.signalSemaphoreCount = vk_semaphore ? 1 : 0; submit_info.pSignalSemaphores = &vk_semaphore; - if (device->use_timeline_semaphores) + if (device->vk_info.KHR_timeline_semaphore) { timeline_submit_info.sType = VK_STRUCTURE_TYPE_TIMELINE_SEMAPHORE_SUBMIT_INFO_KHR; timeline_submit_info.pNext = NULL; - timeline_submit_info.pSignalSemaphoreValues = &value; + timeline_submit_info.pSignalSemaphoreValues = &timeline_value; timeline_submit_info.signalSemaphoreValueCount = submit_info.signalSemaphoreCount; timeline_submit_info.waitSemaphoreValueCount = 0; timeline_submit_info.pWaitSemaphoreValues = NULL; @@ -6454,7 +6444,7 @@ static HRESULT STDMETHODCALLTYPE d3d12_command_queue_Signal(ID3D12CommandQueue * } vr = VK_CALL(vkQueueSubmit(vk_queue, 1, &submit_info, vk_fence)); - if (!device->use_timeline_semaphores && vr >= 0) + if (!device->vk_info.KHR_timeline_semaphore && vr >= 0) { sequence_number = ++vkd3d_queue->submitted_sequence_number; @@ -6465,41 +6455,43 @@ static HRESULT STDMETHODCALLTYPE d3d12_command_queue_Signal(ID3D12CommandQueue * vkd3d_queue_release(vkd3d_queue); + if (!rc) + { + vkd3d_mutex_unlock(&fence->mutex); + rc = -1; + } + if (vr < 0) { WARN("Failed to submit signal operation, vr %d.\n", vr); - goto fail_vkresult; + goto fail; } - if (device->use_timeline_semaphores) + if (device->vk_info.KHR_timeline_semaphore) { return vkd3d_enqueue_timeline_semaphore(&command_queue->fence_worker, - vk_semaphore, fence, value, vkd3d_queue); + vk_semaphore, fence, timeline_value, vkd3d_queue); } - if (vk_semaphore && SUCCEEDED(hr = d3d12_fence_add_vk_semaphore(fence, vk_semaphore, vk_fence, value))) + if (vk_semaphore && d3d12_fence_add_vk_semaphore(fence, vk_semaphore, vk_fence, value, vkd3d_queue)) vk_semaphore = VK_NULL_HANDLE; vr = VK_CALL(vkGetFenceStatus(device->vk_device, vk_fence)); if (vr == VK_NOT_READY) { - if (SUCCEEDED(hr = vkd3d_enqueue_gpu_fence(&command_queue->fence_worker, - vk_fence, fence, value, vkd3d_queue, sequence_number))) - { + if (vkd3d_enqueue_gpu_fence(&command_queue->fence_worker, vk_fence, fence, value, vkd3d_queue, sequence_number)) vk_fence = VK_NULL_HANDLE; - } } else if (vr == VK_SUCCESS) { TRACE("Already signaled %p, value %#"PRIx64".\n", fence, value); - hr = d3d12_fence_signal(fence, value, vk_fence); + d3d12_fence_signal(fence, value, vk_fence); vk_fence = VK_NULL_HANDLE; vkd3d_queue_update_sequence_number(vkd3d_queue, sequence_number, device); } else { FIXME("Failed to get fence status, vr %d.\n", vr); - hr = hresult_from_vk_result(vr); } if (vk_fence || vk_semaphore) @@ -6509,18 +6501,17 @@ static HRESULT STDMETHODCALLTYPE d3d12_command_queue_Signal(ID3D12CommandQueue * goto fail; } - return hr; + return; -fail_vkresult: - hr = hresult_from_vk_result(vr); fail: + if (!rc) + vkd3d_mutex_unlock(&fence->mutex); VK_CALL(vkDestroyFence(device->vk_device, vk_fence, NULL)); - if (!device->use_timeline_semaphores) + if (!device->vk_info.KHR_timeline_semaphore) VK_CALL(vkDestroySemaphore(device->vk_device, vk_semaphore, NULL)); - return hr; } -static HRESULT d3d12_command_queue_wait_binary_semaphore(struct d3d12_command_queue *command_queue, +static void d3d12_command_queue_wait_binary_semaphore_locked(struct d3d12_command_queue *command_queue, struct d3d12_fence *fence, uint64_t value) { static const VkPipelineStageFlagBits wait_stage_mask = VK_PIPELINE_STAGE_ALL_COMMANDS_BIT; @@ -6531,23 +6522,24 @@ static HRESULT d3d12_command_queue_wait_binary_semaphore(struct d3d12_command_qu VkSubmitInfo submit_info; VkQueue vk_queue; VkResult vr; - HRESULT hr; vk_procs = &command_queue->device->vk_procs; queue = command_queue->vkd3d_queue; - semaphore = d3d12_fence_acquire_vk_semaphore(fence, value, &completed_value); + semaphore = d3d12_fence_acquire_vk_semaphore_locked(fence, value, &completed_value); + + vkd3d_mutex_unlock(&fence->mutex); + if (!semaphore && completed_value >= value) { /* We don't get a Vulkan semaphore if the fence was signaled on CPU. */ TRACE("Already signaled %p, value %#"PRIx64".\n", fence, completed_value); - return S_OK; + return; } if (!(vk_queue = vkd3d_queue_acquire(queue))) { ERR("Failed to acquire queue %p.\n", queue); - hr = E_FAIL; goto fail; } @@ -6564,13 +6556,13 @@ static HRESULT d3d12_command_queue_wait_binary_semaphore(struct d3d12_command_qu } vkd3d_queue_release(queue); - return S_OK; + return; } submit_info.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO; submit_info.pNext = NULL; submit_info.waitSemaphoreCount = 1; - submit_info.pWaitSemaphores = &semaphore->vk_semaphore; + submit_info.pWaitSemaphores = &semaphore->u.binary.vk_semaphore; submit_info.pWaitDstStageMask = &wait_stage_mask; submit_info.commandBufferCount = 0; submit_info.pCommandBuffers = NULL; @@ -6582,13 +6574,12 @@ static HRESULT d3d12_command_queue_wait_binary_semaphore(struct d3d12_command_qu { ERR("Failed to allocate memory for semaphore.\n"); vkd3d_queue_release(queue); - hr = E_OUTOFMEMORY; goto fail; } if ((vr = VK_CALL(vkQueueSubmit(vk_queue, 1, &submit_info, VK_NULL_HANDLE))) >= 0) { - queue->semaphores[queue->semaphore_count].vk_semaphore = semaphore->vk_semaphore; + queue->semaphores[queue->semaphore_count].vk_semaphore = semaphore->u.binary.vk_semaphore; queue->semaphores[queue->semaphore_count].sequence_number = queue->submitted_sequence_number + 1; ++queue->semaphore_count; @@ -6601,60 +6592,17 @@ static HRESULT d3d12_command_queue_wait_binary_semaphore(struct d3d12_command_qu if (vr < 0) { WARN("Failed to submit wait operation, vr %d.\n", vr); - hr = hresult_from_vk_result(vr); goto fail; } d3d12_fence_remove_vk_semaphore(fence, semaphore); - return S_OK; + return; fail: d3d12_fence_release_vk_semaphore(fence, semaphore); - return hr; } -static inline void d3d12_fence_update_gpu_wait(struct d3d12_fence *fence, const struct vkd3d_queue *queue) -{ - unsigned int i; - bool found; - int rc; - - if ((rc = vkd3d_mutex_lock(&fence->mutex))) - { - ERR("Failed to lock mutex, error %d.\n", rc); - return; - } - - for (i = 0, found = false; i < fence->gpu_wait_count; ++i) - { - if (fence->gpu_waits[i].queue == queue) - { - fence->gpu_waits[i].pending_value = queue->pending_wait_completion_value; - found = true; - } - else if (d3d12_fence_gpu_wait_is_completed(fence, i) && i < --fence->gpu_wait_count) - { - fence->gpu_waits[i] = fence->gpu_waits[fence->gpu_wait_count]; - } - } - - if (!found) - { - if (fence->gpu_wait_count < ARRAY_SIZE(fence->gpu_waits)) - { - fence->gpu_waits[fence->gpu_wait_count].queue = queue; - fence->gpu_waits[fence->gpu_wait_count++].pending_value = queue->pending_wait_completion_value; - } - else - { - FIXME("Unable to track GPU fence wait.\n"); - } - } - - vkd3d_mutex_unlock(&fence->mutex); -} - -static HRESULT d3d12_command_queue_wait_timeline_semaphore(struct d3d12_command_queue *command_queue, +static void d3d12_command_queue_wait(struct d3d12_command_queue *command_queue, struct d3d12_fence *fence, uint64_t value) { static const VkPipelineStageFlagBits wait_stage_mask = VK_PIPELINE_STAGE_ALL_COMMANDS_BIT; @@ -6662,25 +6610,47 @@ static HRESULT d3d12_command_queue_wait_timeline_semaphore(struct d3d12_command_ const struct vkd3d_vk_device_procs *vk_procs; struct vkd3d_queue *queue; VkSubmitInfo submit_info; + uint64_t wait_count; VkQueue vk_queue; VkResult vr; vk_procs = &command_queue->device->vk_procs; queue = command_queue->vkd3d_queue; + vkd3d_mutex_lock(&fence->mutex); + + /* This is the critical part required to support out-of-order signal. + * Normally we would be able to submit waits and signals out of order, but + * we don't have virtualized queues in Vulkan, so we need to handle the case + * where multiple queues alias over the same physical queue, so effectively, + * we need to manage out-of-order submits ourselves. */ + d3d12_fence_block_until_pending_value_reaches_locked(fence, value); + + /* If a host signal unblocked us, or we know that the fence has reached a specific value, there is no need + * to queue up a wait. */ + if (d3d12_fence_can_elide_wait_semaphore_locked(fence, value, queue)) + { + TRACE("Eliding wait on fence %p, value %#"PRIx64".\n", fence, value); + vkd3d_mutex_unlock(&fence->mutex); + return; + } + + if (!command_queue->device->vk_info.KHR_timeline_semaphore) + return d3d12_command_queue_wait_binary_semaphore_locked(command_queue, fence, value); + + wait_count = d3d12_fence_get_timeline_wait_value_locked(fence, value); + + /* We can unlock the fence here. The queue semaphore will not be signalled to signal_value + * until we have submitted, so the semaphore cannot be destroyed before the call to vkQueueSubmit. */ + vkd3d_mutex_unlock(&fence->mutex); + assert(fence->timeline_semaphore); timeline_submit_info.sType = VK_STRUCTURE_TYPE_TIMELINE_SEMAPHORE_SUBMIT_INFO_KHR; timeline_submit_info.pNext = NULL; + timeline_submit_info.waitSemaphoreValueCount = 1; + timeline_submit_info.pWaitSemaphoreValues = &wait_count; timeline_submit_info.signalSemaphoreValueCount = 0; timeline_submit_info.pSignalSemaphoreValues = NULL; - timeline_submit_info.waitSemaphoreValueCount = 1; - timeline_submit_info.pWaitSemaphoreValues = &value; - - if (!(vk_queue = vkd3d_queue_acquire(queue))) - { - ERR("Failed to acquire queue %p.\n", queue); - return E_FAIL; - } submit_info.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO; submit_info.pNext = &timeline_submit_info; @@ -6692,14 +6662,11 @@ static HRESULT d3d12_command_queue_wait_timeline_semaphore(struct d3d12_command_ submit_info.signalSemaphoreCount = 0; submit_info.pSignalSemaphores = NULL; - ++queue->pending_wait_completion_value; - - submit_info.signalSemaphoreCount = 1; - submit_info.pSignalSemaphores = &queue->wait_completion_semaphore; - timeline_submit_info.signalSemaphoreValueCount = 1; - timeline_submit_info.pSignalSemaphoreValues = &queue->pending_wait_completion_value; - - d3d12_fence_update_gpu_wait(fence, queue); + if (!(vk_queue = vkd3d_queue_acquire(queue))) + { + ERR("Failed to acquire queue %p.\n", queue); + return; + } vr = VK_CALL(vkQueueSubmit(vk_queue, 1, &submit_info, VK_NULL_HANDLE)); @@ -6708,10 +6675,7 @@ static HRESULT d3d12_command_queue_wait_timeline_semaphore(struct d3d12_command_ if (vr < 0) { WARN("Failed to submit wait operation, vr %d.\n", vr); - return hresult_from_vk_result(vr); } - - return S_OK; } static HRESULT STDMETHODCALLTYPE d3d12_command_queue_Wait(ID3D12CommandQueue *iface, @@ -6719,14 +6683,16 @@ static HRESULT STDMETHODCALLTYPE d3d12_command_queue_Wait(ID3D12CommandQueue *if { struct d3d12_command_queue *command_queue = impl_from_ID3D12CommandQueue(iface); struct d3d12_fence *fence = unsafe_impl_from_ID3D12Fence(fence_iface); + struct d3d12_command_queue_submission sub; TRACE("iface %p, fence %p, value %#"PRIx64".\n", iface, fence_iface, value); - if (command_queue->device->use_timeline_semaphores) - return d3d12_command_queue_wait_timeline_semaphore(command_queue, fence, value); + d3d12_fence_incref(fence); - FIXME_ONCE("KHR_timeline_semaphore is not available or incompatible. Some wait commands may be unsupported.\n"); - return d3d12_command_queue_wait_binary_semaphore(command_queue, fence, value); + sub.type = VKD3D_SUBMISSION_WAIT; + sub.u.wait.fence = fence; + sub.u.wait.value = value; + return d3d12_command_queue_add_submission(command_queue, &sub); } static HRESULT STDMETHODCALLTYPE d3d12_command_queue_GetTimestampFrequency(ID3D12CommandQueue *iface, @@ -6850,10 +6816,118 @@ static const struct ID3D12CommandQueueVtbl d3d12_command_queue_vtbl = d3d12_command_queue_GetDesc, }; +static void d3d12_command_queue_acquire_serialised(struct d3d12_command_queue *queue) +{ + /* In order to make sure all pending operations queued so far have been submitted, we build a drain + * task which will increment the queue_drain_count once the thread has finished all its work. */ + struct d3d12_command_queue_submission sub; + uint64_t current_drain; + HRESULT hr; + + sub.type = VKD3D_SUBMISSION_DRAIN; + + vkd3d_mutex_lock(&queue->submission_mutex); + + current_drain = ++queue->target_drain_count; + if (FAILED(hr = d3d12_command_queue_add_submission_locked(queue, &sub))) + ERR("Failed to submit command, hr %#x.\n", hr); + + while (current_drain != queue->queue_drain_count) + vkd3d_cond_wait(&queue->submission_cond, &queue->submission_mutex); +} + +static void d3d12_command_queue_release_serialised(struct d3d12_command_queue *queue) +{ + vkd3d_mutex_unlock(&queue->submission_mutex); +} + +static void d3d12_command_queue_execute(struct d3d12_command_queue *command_queue, + VkCommandBuffer *cmd, unsigned int count) +{ + const struct vkd3d_vk_device_procs *vk_procs = &command_queue->device->vk_procs; + struct vkd3d_queue *vkd3d_queue = command_queue->vkd3d_queue; + VkSubmitInfo submit_desc; + VkQueue vk_queue; + VkResult vr; + + TRACE("queue %p, command_list_count %u, command_lists %p.\n", + command_queue, count, cmd); + + memset(&submit_desc, 0, sizeof(submit_desc)); + + if (!(vk_queue = vkd3d_queue_acquire(vkd3d_queue))) + { + ERR("Failed to acquire queue %p.\n", vkd3d_queue); + return; + } + + submit_desc.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO; + submit_desc.commandBufferCount = count; + submit_desc.pCommandBuffers = cmd; + + if ((vr = VK_CALL(vkQueueSubmit(vk_queue, 1, &submit_desc, VK_NULL_HANDLE))) < 0) + ERR("Failed to submit queue(s), vr %d.\n", vr); + + vkd3d_queue_release(vkd3d_queue); +} + +static void *d3d12_command_queue_submission_worker_main(void *userdata) +{ + struct d3d12_command_queue_submission submission; + struct d3d12_command_queue *queue = userdata; + + vkd3d_set_thread_name("vkd3d_queue"); + + for (;;) + { + vkd3d_mutex_lock(&queue->submission_mutex); + while (!queue->submissions_count) + vkd3d_cond_wait(&queue->submission_cond, &queue->submission_mutex); + + submission = queue->submissions[0]; + memmove(queue->submissions, queue->submissions + 1, --queue->submissions_count * sizeof(submission)); + vkd3d_mutex_unlock(&queue->submission_mutex); + + switch (submission.type) + { + case VKD3D_SUBMISSION_WAIT: + d3d12_command_queue_wait(queue, submission.u.wait.fence, submission.u.wait.value); + d3d12_fence_decref(submission.u.wait.fence); + break; + + case VKD3D_SUBMISSION_SIGNAL: + d3d12_command_queue_signal(queue, submission.u.signal.fence, submission.u.signal.value); + d3d12_fence_decref(submission.u.signal.fence); + break; + + case VKD3D_SUBMISSION_EXECUTE: + d3d12_command_queue_execute(queue, submission.u.execute.cmd, submission.u.execute.cmd_count); + vkd3d_free(submission.u.execute.cmd); + break; + + case VKD3D_SUBMISSION_DRAIN: + vkd3d_mutex_lock(&queue->submission_mutex); + ++queue->queue_drain_count; + vkd3d_cond_signal(&queue->submission_cond); + vkd3d_mutex_unlock(&queue->submission_mutex); + break; + + case VKD3D_SUBMISSION_STOP: + TRACE("Stopping command queue %p.\n", queue); + return NULL; + + default: + FIXME("Unhandled submission type %u.\n", submission.type); + break; + } + } +} + static HRESULT d3d12_command_queue_init(struct d3d12_command_queue *queue, struct d3d12_device *device, const D3D12_COMMAND_QUEUE_DESC *desc) { HRESULT hr; + int rc; queue->ID3D12CommandQueue_iface.lpVtbl = &d3d12_command_queue_vtbl; queue->refcount = 1; @@ -6868,6 +6942,12 @@ static HRESULT d3d12_command_queue_init(struct d3d12_command_queue *queue, queue->last_waited_fence = NULL; queue->last_waited_fence_value = 0; + queue->submissions = NULL; + queue->submissions_count = 0; + queue->submissions_size = 0; + queue->target_drain_count = 0; + queue->queue_drain_count = 0; + if (desc->Priority == D3D12_COMMAND_QUEUE_PRIORITY_GLOBAL_REALTIME) { FIXME("Global realtime priority is not implemented.\n"); @@ -6882,15 +6962,40 @@ static HRESULT d3d12_command_queue_init(struct d3d12_command_queue *queue, if (FAILED(hr = vkd3d_private_store_init(&queue->private_store))) return hr; + if ((rc = vkd3d_mutex_init(&queue->submission_mutex)) < 0) + { + hr = hresult_from_errno(rc); + goto fail_submission_mutex; + } + + if ((rc = vkd3d_cond_init(&queue->submission_cond)) < 0) + { + hr = hresult_from_errno(rc); + goto fail_submission_cond; + } + if (FAILED(hr = vkd3d_fence_worker_start(&queue->fence_worker, queue->vkd3d_queue, device))) + goto fail_fence_worker_start; + + if ((rc = vkd3d_create_thread(device->vkd3d_instance, d3d12_command_queue_submission_worker_main, queue, &queue->submission_thread)) < 0) { - vkd3d_private_store_destroy(&queue->private_store); - return hr; + hr = hresult_from_errno(rc); + goto fail_pthread_create; } d3d12_device_add_ref(queue->device = device); return S_OK; + +fail_pthread_create: + vkd3d_fence_worker_stop(&queue->fence_worker, device); +fail_fence_worker_start: + vkd3d_cond_destroy(&queue->submission_cond); +fail_submission_cond: + vkd3d_mutex_destroy(&queue->submission_mutex); +fail_submission_mutex: + vkd3d_private_store_destroy(&queue->private_store); + return hr; } HRESULT d3d12_command_queue_create(struct d3d12_device *device, @@ -6926,6 +7031,9 @@ VkQueue vkd3d_acquire_vk_queue(ID3D12CommandQueue *queue) { struct d3d12_command_queue *d3d12_queue = impl_from_ID3D12CommandQueue(queue); + /* For external users of the Vulkan queue, we must ensure that the queue is drained + * so that submissions happen in the desired order. */ + d3d12_command_queue_acquire_serialised(d3d12_queue); return vkd3d_queue_acquire(d3d12_queue->vkd3d_queue); } @@ -6933,7 +7041,8 @@ void vkd3d_release_vk_queue(ID3D12CommandQueue *queue) { struct d3d12_command_queue *d3d12_queue = impl_from_ID3D12CommandQueue(queue); - return vkd3d_queue_release(d3d12_queue->vkd3d_queue); + vkd3d_queue_release(d3d12_queue->vkd3d_queue); + d3d12_command_queue_release_serialised(d3d12_queue); } /* ID3D12CommandSignature */ diff --git a/libs/vkd3d/device.c b/libs/vkd3d/device.c index 5f8108ec..49145797 100644 --- a/libs/vkd3d/device.c +++ b/libs/vkd3d/device.c @@ -747,7 +747,6 @@ struct vkd3d_physical_device_info VkPhysicalDeviceTexelBufferAlignmentPropertiesEXT texel_buffer_alignment_properties; VkPhysicalDeviceTransformFeedbackPropertiesEXT xfb_properties; VkPhysicalDeviceVertexAttributeDivisorPropertiesEXT vertex_divisor_properties; - VkPhysicalDeviceTimelineSemaphorePropertiesKHR timeline_semaphore_properties; VkPhysicalDeviceProperties2KHR properties2; @@ -772,7 +771,6 @@ static void vkd3d_physical_device_info_init(struct vkd3d_physical_device_info *i VkPhysicalDeviceDescriptorIndexingPropertiesEXT *descriptor_indexing_properties; VkPhysicalDeviceVertexAttributeDivisorPropertiesEXT *vertex_divisor_properties; VkPhysicalDeviceTexelBufferAlignmentPropertiesEXT *buffer_alignment_properties; - VkPhysicalDeviceTimelineSemaphorePropertiesKHR *timeline_semaphore_properties; VkPhysicalDeviceDescriptorIndexingFeaturesEXT *descriptor_indexing_features; VkPhysicalDeviceRobustness2FeaturesEXT *robustness2_features; VkPhysicalDeviceVertexAttributeDivisorFeaturesEXT *vertex_divisor_features; @@ -799,7 +797,6 @@ static void vkd3d_physical_device_info_init(struct vkd3d_physical_device_info *i vertex_divisor_features = &info->vertex_divisor_features; vertex_divisor_properties = &info->vertex_divisor_properties; timeline_semaphore_features = &info->timeline_semaphore_features; - timeline_semaphore_properties = &info->timeline_semaphore_properties; xfb_features = &info->xfb_features; xfb_properties = &info->xfb_properties; @@ -841,8 +838,6 @@ static void vkd3d_physical_device_info_init(struct vkd3d_physical_device_info *i vk_prepend_struct(&info->properties2, xfb_properties); vertex_divisor_properties->sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_VERTEX_ATTRIBUTE_DIVISOR_PROPERTIES_EXT; vk_prepend_struct(&info->properties2, vertex_divisor_properties); - timeline_semaphore_properties->sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_TIMELINE_SEMAPHORE_PROPERTIES_KHR; - vk_prepend_struct(&info->properties2, timeline_semaphore_properties); if (vulkan_info->KHR_get_physical_device_properties2) VK_CALL(vkGetPhysicalDeviceProperties2KHR(physical_device, &info->properties2)); @@ -1431,7 +1426,6 @@ static HRESULT vkd3d_init_device_caps(struct d3d12_device *device, vulkan_info->rasterization_stream = physical_device_info->xfb_properties.transformFeedbackRasterizationStreamSelect; vulkan_info->transform_feedback_queries = physical_device_info->xfb_properties.transformFeedbackQueries; vulkan_info->max_vertex_attrib_divisor = max(physical_device_info->vertex_divisor_properties.maxVertexAttribDivisor, 1); - vulkan_info->timeline_semaphore_properties = physical_device_info->timeline_semaphore_properties; device->feature_options.DoublePrecisionFloatShaderOps = features->shaderFloat64; device->feature_options.OutputMergerLogicOp = features->logicOp; @@ -1908,75 +1902,6 @@ static bool d3d12_is_64k_msaa_supported(struct d3d12_device *device) && info.Alignment <= 0x10000; } -/* A lower value can be signalled on a D3D12 fence. Vulkan timeline semaphores - * do not support this, but test if it works anyway. */ -static bool d3d12_is_timeline_semaphore_supported(const struct d3d12_device *device) -{ - const struct vkd3d_vk_device_procs *vk_procs = &device->vk_procs; - VkTimelineSemaphoreSubmitInfoKHR timeline_submit_info; - VkSemaphore timeline_semaphore; - VkSubmitInfo submit_info; - bool result = false; - uint64_t value = 0; - VkQueue vk_queue; - VkResult vr; - - if (!device->vk_info.KHR_timeline_semaphore) - return false; - - if ((vr = vkd3d_create_timeline_semaphore(device, 1, &timeline_semaphore)) < 0) - { - WARN("Failed to create timeline semaphore, vr %d.\n", vr); - return false; - } - - if (!(vk_queue = vkd3d_queue_acquire(device->direct_queue))) - { - ERR("Failed to acquire queue %p.\n", device->direct_queue); - VK_CALL(vkDestroySemaphore(device->vk_device, timeline_semaphore, NULL)); - return false; - } - - submit_info.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO; - submit_info.pNext = &timeline_submit_info; - submit_info.waitSemaphoreCount = 0; - submit_info.pWaitSemaphores = NULL; - submit_info.pWaitDstStageMask = NULL; - submit_info.commandBufferCount = 0; - submit_info.pCommandBuffers = NULL; - submit_info.signalSemaphoreCount = 1; - submit_info.pSignalSemaphores = &timeline_semaphore; - - timeline_submit_info.sType = VK_STRUCTURE_TYPE_TIMELINE_SEMAPHORE_SUBMIT_INFO_KHR; - timeline_submit_info.pNext = NULL; - timeline_submit_info.pSignalSemaphoreValues = &value; - timeline_submit_info.signalSemaphoreValueCount = 1; - timeline_submit_info.waitSemaphoreValueCount = 0; - timeline_submit_info.pWaitSemaphoreValues = NULL; - - vr = VK_CALL(vkQueueSubmit(vk_queue, 1, &submit_info, VK_NULL_HANDLE)); - - if (vr >= 0) - { - if ((vr = VK_CALL(vkQueueWaitIdle(vk_queue))) < 0) - WARN("Failed to wait for queue, vr %d.\n", vr); - - if ((vr = VK_CALL(vkGetSemaphoreCounterValueKHR(device->vk_device, timeline_semaphore, &value))) < 0) - ERR("Failed to get Vulkan semaphore status, vr %d.\n", vr); - else if (!(result = !value)) - WARN("Disabling timeline semaphore use due to incompatible behaviour.\n"); - } - else - { - WARN("Failed to submit signal operation, vr %d.\n", vr); - } - - vkd3d_queue_release(device->direct_queue); - VK_CALL(vkDestroySemaphore(device->vk_device, timeline_semaphore, NULL)); - - return result; -} - static HRESULT vkd3d_create_vk_device(struct d3d12_device *device, const struct vkd3d_device_create_info *create_info) { @@ -2075,10 +2000,6 @@ static HRESULT vkd3d_create_vk_device(struct d3d12_device *device, } device->feature_options4.MSAA64KBAlignedTextureSupported = d3d12_is_64k_msaa_supported(device); - device->use_timeline_semaphores = d3d12_is_timeline_semaphore_supported(device) - && vkd3d_queue_init_timeline_semaphore(device->direct_queue, device) - && vkd3d_queue_init_timeline_semaphore(device->compute_queue, device) - && vkd3d_queue_init_timeline_semaphore(device->copy_queue, device); TRACE("Created Vulkan device %p.\n", vk_device); diff --git a/libs/vkd3d/vkd3d_private.h b/libs/vkd3d/vkd3d_private.h index 4e03145d..9989f20a 100644 --- a/libs/vkd3d/vkd3d_private.h +++ b/libs/vkd3d/vkd3d_private.h @@ -59,7 +59,6 @@ #define VKD3D_MAX_SHADER_EXTENSIONS 3u #define VKD3D_MAX_SHADER_STAGES 5u #define VKD3D_MAX_VK_SYNC_OBJECTS 4u -#define VKD3D_MAX_FENCE_WAITING_QUEUES 4u #define VKD3D_MAX_DESCRIPTOR_SETS 64u /* D3D12 binding tier 3 has a limit of 2048 samplers. */ #define VKD3D_MAX_DESCRIPTOR_SET_SAMPLERS 2048u @@ -152,8 +151,6 @@ struct vkd3d_vulkan_info VkPhysicalDeviceTexelBufferAlignmentPropertiesEXT texel_buffer_alignment_properties; - VkPhysicalDeviceTimelineSemaphorePropertiesKHR timeline_semaphore_properties; - unsigned int shader_extension_count; enum vkd3d_shader_spirv_extension shader_extensions[VKD3D_MAX_SHADER_EXTENSIONS]; @@ -502,15 +499,17 @@ HRESULT vkd3d_set_private_data_interface(struct vkd3d_private_store *store, cons struct vkd3d_signaled_semaphore { uint64_t value; - VkSemaphore vk_semaphore; - VkFence vk_fence; - bool is_acquired; -}; - -struct vkd3d_pending_fence_wait -{ - const struct vkd3d_queue *queue; - uint64_t pending_value; + union + { + struct + { + VkSemaphore vk_semaphore; + VkFence vk_fence; + bool is_acquired; + } binary; + uint64_t timeline_value; + } u; + const struct vkd3d_queue *signalling_queue; }; /* ID3D12Fence */ @@ -521,7 +520,9 @@ struct d3d12_fence LONG refcount; uint64_t value; + uint64_t max_pending_value; struct vkd3d_mutex mutex; + struct vkd3d_cond cond; struct vkd3d_cond null_event_cond; struct vkd3d_waiting_event @@ -534,9 +535,8 @@ struct d3d12_fence size_t event_count; VkSemaphore timeline_semaphore; + uint64_t timeline_value; uint64_t pending_timeline_value; - struct vkd3d_pending_fence_wait gpu_waits[VKD3D_MAX_FENCE_WAITING_QUEUES]; - unsigned int gpu_wait_count; struct vkd3d_signaled_semaphore *semaphores; size_t semaphores_size; @@ -1294,9 +1294,6 @@ struct vkd3d_queue VkQueueFlags vk_queue_flags; uint32_t timestamp_bits; - VkSemaphore wait_completion_semaphore; - uint64_t pending_wait_completion_value; - struct { VkSemaphore vk_semaphore; @@ -1311,10 +1308,47 @@ struct vkd3d_queue VkQueue vkd3d_queue_acquire(struct vkd3d_queue *queue); HRESULT vkd3d_queue_create(struct d3d12_device *device, uint32_t family_index, const VkQueueFamilyProperties *properties, struct vkd3d_queue **queue); -bool vkd3d_queue_init_timeline_semaphore(struct vkd3d_queue *queue, struct d3d12_device *device); void vkd3d_queue_destroy(struct vkd3d_queue *queue, struct d3d12_device *device); void vkd3d_queue_release(struct vkd3d_queue *queue); +enum vkd3d_submission_type +{ + VKD3D_SUBMISSION_WAIT, + VKD3D_SUBMISSION_SIGNAL, + VKD3D_SUBMISSION_EXECUTE, + VKD3D_SUBMISSION_DRAIN, + VKD3D_SUBMISSION_STOP, +}; + +struct d3d12_command_queue_submission_wait +{ + struct d3d12_fence *fence; + uint64_t value; +}; + +struct d3d12_command_queue_submission_signal +{ + struct d3d12_fence *fence; + uint64_t value; +}; + +struct d3d12_command_queue_submission_execute +{ + VkCommandBuffer *cmd; + unsigned int cmd_count; +}; + +struct d3d12_command_queue_submission +{ + enum vkd3d_submission_type type; + union + { + struct d3d12_command_queue_submission_wait wait; + struct d3d12_command_queue_submission_signal signal; + struct d3d12_command_queue_submission_execute execute; + } u; +}; + /* ID3D12CommandQueue */ struct d3d12_command_queue { @@ -1331,6 +1365,16 @@ struct d3d12_command_queue struct d3d12_device *device; + struct vkd3d_mutex submission_mutex; + struct vkd3d_cond submission_cond; + union vkd3d_thread_handle submission_thread; + + struct d3d12_command_queue_submission *submissions; + size_t submissions_count; + size_t submissions_size; + uint64_t target_drain_count; + uint64_t queue_drain_count; + struct vkd3d_private_store private_store; }; @@ -1470,7 +1514,6 @@ struct d3d12_device VkDescriptorPoolSize vk_pool_sizes[VKD3D_DESCRIPTOR_POOL_COUNT]; struct vkd3d_vk_descriptor_heap_layout vk_descriptor_heap_layouts[VKD3D_SET_INDEX_COUNT]; bool use_vk_heaps; - bool use_timeline_semaphores; }; HRESULT d3d12_device_create(struct vkd3d_instance *instance, diff --git a/tests/d3d12.c b/tests/d3d12.c index 015c3122..5f83a373 100644 --- a/tests/d3d12.c +++ b/tests/d3d12.c @@ -33224,9 +33224,7 @@ static void test_queue_wait(void) command_list = context.list; queue = context.queue; - /* 'queue2' must not map to the same command queue as 'queue', or Wait() before GPU signal will fail. - * Using a compute queue fixes this on most hardware, but it may still fail on low spec hardware. */ - queue2 = create_command_queue(device, D3D12_COMMAND_LIST_TYPE_COMPUTE, D3D12_COMMAND_QUEUE_PRIORITY_NORMAL); + queue2 = create_command_queue(device, D3D12_COMMAND_LIST_TYPE_DIRECT, D3D12_COMMAND_QUEUE_PRIORITY_NORMAL); event = create_event(); ok(event, "Failed to create event.\n"); -- 2.35.1

April 29, 2022

[PATCH vkd3d v2 3/4] vkd3d: Replace the signaled semaphore list with a resizable array.

by Conor McCarthy

Order does not need to be preserved here, and another function will add to this array when mapped timeline semaphores are implemented. Signed-off-by: Conor McCarthy <cmccarthy(a)codeweavers.com> --- v2: Tidy up the semaphore garbage collection loop, and removal of a semaphore from a fence. --- libs/vkd3d/command.c | 44 +++++++++++++++++++++++--------------- libs/vkd3d/vkd3d_private.h | 4 ++-- 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/libs/vkd3d/command.c b/libs/vkd3d/command.c index 7690760c..55e6be58 100644 --- a/libs/vkd3d/command.c +++ b/libs/vkd3d/command.c @@ -590,32 +590,34 @@ static void d3d12_fence_garbage_collect_vk_semaphores_locked(struct d3d12_fence { struct d3d12_device *device = fence->device; const struct vkd3d_vk_device_procs *vk_procs = &device->vk_procs; - struct vkd3d_signaled_semaphore *current, *p; - unsigned int semaphore_count; + struct vkd3d_signaled_semaphore *current; + unsigned int i, semaphore_count; semaphore_count = fence->semaphore_count; if (!destroy_all && semaphore_count < VKD3D_MAX_VK_SYNC_OBJECTS) return; - LIST_FOR_EACH_ENTRY_SAFE(current, p, &fence->semaphores, struct vkd3d_signaled_semaphore, entry) + i = 0; + while (i < fence->semaphore_count) { if (!destroy_all && fence->semaphore_count < VKD3D_MAX_VK_SYNC_OBJECTS) break; + current = &fence->semaphores[i]; /* The semaphore doesn't have a pending signal operation if the fence * was signaled. */ if ((current->vk_fence || current->is_acquired) && !destroy_all) + { + ++i; continue; + } if (current->vk_fence) WARN("Destroying potentially pending semaphore.\n"); assert(!current->is_acquired); VK_CALL(vkDestroySemaphore(device->vk_device, current->vk_semaphore, NULL)); - list_remove(&current->entry); - vkd3d_free(current); - - --fence->semaphore_count; + fence->semaphores[i] = fence->semaphores[--fence->semaphore_count]; } if (semaphore_count != fence->semaphore_count) @@ -656,6 +658,7 @@ static struct vkd3d_signaled_semaphore *d3d12_fence_acquire_vk_semaphore(struct struct vkd3d_signaled_semaphore *semaphore; struct vkd3d_signaled_semaphore *current; uint64_t semaphore_value; + unsigned int i; int rc; TRACE("fence %p, value %#"PRIx64".\n", fence, value); @@ -669,8 +672,9 @@ static struct vkd3d_signaled_semaphore *d3d12_fence_acquire_vk_semaphore(struct semaphore = NULL; semaphore_value = ~(uint64_t)0; - LIST_FOR_EACH_ENTRY(current, &fence->semaphores, struct vkd3d_signaled_semaphore, entry) + for (i = 0; i < fence->semaphore_count; ++i) { + current = &fence->semaphores[i]; /* Prefer a semaphore with the smallest value. */ if (!current->is_acquired && current->value >= value && semaphore_value >= current->value) { @@ -703,10 +707,7 @@ static void d3d12_fence_remove_vk_semaphore(struct d3d12_fence *fence, struct vk assert(semaphore->is_acquired); - list_remove(&semaphore->entry); - vkd3d_free(semaphore); - - --fence->semaphore_count; + *semaphore = fence->semaphores[--fence->semaphore_count]; vkd3d_mutex_unlock(&fence->mutex); } @@ -751,14 +752,20 @@ static HRESULT d3d12_fence_add_vk_semaphore(struct d3d12_fence *fence, d3d12_fence_garbage_collect_vk_semaphores_locked(fence, false); + if (!vkd3d_array_reserve((void**)&fence->semaphores, &fence->semaphores_size, + fence->semaphore_count + 1, sizeof(*fence->semaphores))) + { + ERR("Failed to add semaphore.\n"); + vkd3d_mutex_unlock(&fence->mutex); + return false; + } + + semaphore = &fence->semaphores[fence->semaphore_count++]; semaphore->value = value; semaphore->vk_semaphore = vk_semaphore; semaphore->vk_fence = vk_fence; semaphore->is_acquired = false; - list_add_tail(&fence->semaphores, &semaphore->entry); - ++fence->semaphore_count; - vkd3d_mutex_unlock(&fence->mutex); return hr; @@ -821,8 +828,9 @@ static HRESULT d3d12_fence_signal(struct d3d12_fence *fence, uint64_t value, VkF { const struct vkd3d_vk_device_procs *vk_procs = &device->vk_procs; - LIST_FOR_EACH_ENTRY(current, &fence->semaphores, struct vkd3d_signaled_semaphore, entry) + for (i = 0; i < fence->semaphore_count; ++i) { + current = &fence->semaphores[i]; if (current->vk_fence == vk_fence) current->vk_fence = VK_NULL_HANDLE; } @@ -910,6 +918,7 @@ static void d3d12_fence_decref(struct d3d12_fence *fence) d3d12_fence_destroy_vk_objects(fence); vkd3d_free(fence->events); + vkd3d_free(fence->semaphores); if ((rc = vkd3d_mutex_destroy(&fence->mutex))) ERR("Failed to destroy mutex, error %d.\n", rc); vkd3d_cond_destroy(&fence->null_event_cond); @@ -1278,7 +1287,8 @@ static HRESULT d3d12_fence_init(struct d3d12_fence *fence, struct d3d12_device * fence->pending_timeline_value = initial_value; fence->gpu_wait_count = 0; - list_init(&fence->semaphores); + fence->semaphores = NULL; + fence->semaphores_size = 0; fence->semaphore_count = 0; memset(fence->old_vk_fences, 0, sizeof(fence->old_vk_fences)); diff --git a/libs/vkd3d/vkd3d_private.h b/libs/vkd3d/vkd3d_private.h index 350382cd..4e03145d 100644 --- a/libs/vkd3d/vkd3d_private.h +++ b/libs/vkd3d/vkd3d_private.h @@ -501,7 +501,6 @@ HRESULT vkd3d_set_private_data_interface(struct vkd3d_private_store *store, cons struct vkd3d_signaled_semaphore { - struct list entry; uint64_t value; VkSemaphore vk_semaphore; VkFence vk_fence; @@ -539,7 +538,8 @@ struct d3d12_fence struct vkd3d_pending_fence_wait gpu_waits[VKD3D_MAX_FENCE_WAITING_QUEUES]; unsigned int gpu_wait_count; - struct list semaphores; + struct vkd3d_signaled_semaphore *semaphores; + size_t semaphores_size; unsigned int semaphore_count; VkFence old_vk_fences[VKD3D_MAX_VK_SYNC_OBJECTS]; -- 2.35.1

April 29, 2022

[PATCH vkd3d v2 2/4] vkd3d: Create one fence worker thread per command queue.

by Conor McCarthy

Simplifies the handling of GPU waits, and in vkd3d-proton is reported to increase performance when support for multiple Vulkan queues is enabled, because it avoids the problem of fences being signaled while they sit in the pending buffer waiting to be moved to the wait buffer. Based on a vkd3d-proton patch by Philip Rebohle. Signed-off-by: Conor McCarthy <cmccarthy(a)codeweavers.com> --- v2: Do not replace device->use_timeline_semaphores --- libs/vkd3d/command.c | 274 ++++++++++++------------------------- libs/vkd3d/device.c | 8 +- libs/vkd3d/vkd3d_private.h | 29 ++-- 3 files changed, 96 insertions(+), 215 deletions(-) diff --git a/libs/vkd3d/command.c b/libs/vkd3d/command.c index 7a373b34..7690760c 100644 --- a/libs/vkd3d/command.c +++ b/libs/vkd3d/command.c @@ -280,22 +280,19 @@ static HRESULT vkd3d_enqueue_gpu_fence(struct vkd3d_fence_worker *worker, return hresult_from_errno(rc); } - if (!vkd3d_array_reserve((void **)&worker->enqueued_fences, &worker->enqueued_fences_size, - worker->enqueued_fence_count + 1, sizeof(*worker->enqueued_fences))) + if (!vkd3d_array_reserve((void **)&worker->fences, &worker->fences_size, + worker->fence_count + 1, sizeof(*worker->fences))) { ERR("Failed to add GPU fence.\n"); vkd3d_mutex_unlock(&worker->mutex); return E_OUTOFMEMORY; } - worker->enqueued_fences[worker->enqueued_fence_count].vk_fence = vk_fence; - worker->enqueued_fences[worker->enqueued_fence_count].vk_semaphore = VK_NULL_HANDLE; - waiting_fence = &worker->enqueued_fences[worker->enqueued_fence_count].waiting_fence; + waiting_fence = &worker->fences[worker->fence_count++]; waiting_fence->fence = fence; waiting_fence->value = value; - waiting_fence->queue = queue; + waiting_fence->u.vk_fence = vk_fence; waiting_fence->queue_sequence_number = queue_sequence_number; - ++worker->enqueued_fence_count; d3d12_fence_incref(fence); @@ -305,219 +302,124 @@ static HRESULT vkd3d_enqueue_gpu_fence(struct vkd3d_fence_worker *worker, return S_OK; } -static void vkd3d_fence_worker_move_enqueued_fences_locked(struct vkd3d_fence_worker *worker) -{ - unsigned int i; - bool timeline; - size_t count; - bool ret; - - if (!worker->enqueued_fence_count) - return; - - count = worker->fence_count + worker->enqueued_fence_count; - - if ((timeline = worker->device->use_timeline_semaphores)) - { - ret = vkd3d_array_reserve((void **) &worker->vk_semaphores, &worker->vk_semaphores_size, - count, sizeof(*worker->vk_semaphores)); - ret &= vkd3d_array_reserve((void **) &worker->semaphore_wait_values, &worker->semaphore_wait_values_size, - count, sizeof(*worker->semaphore_wait_values)); - } - else - { - ret = vkd3d_array_reserve((void **)&worker->vk_fences, &worker->vk_fences_size, - count, sizeof(*worker->vk_fences)); - } - ret &= vkd3d_array_reserve((void **)&worker->fences, &worker->fences_size, - count, sizeof(*worker->fences)); - if (!ret) - { - ERR("Failed to reserve memory.\n"); - return; - } - - for (i = 0; i < worker->enqueued_fence_count; ++i) - { - struct vkd3d_enqueued_fence *current = &worker->enqueued_fences[i]; - - if (timeline) - { - worker->vk_semaphores[worker->fence_count] = current->vk_semaphore; - worker->semaphore_wait_values[worker->fence_count] = current->waiting_fence.value; - } - else - { - worker->vk_fences[worker->fence_count] = current->vk_fence; - } - - worker->fences[worker->fence_count] = current->waiting_fence; - ++worker->fence_count; - } - assert(worker->fence_count == count); - worker->enqueued_fence_count = 0; -} - -static void vkd3d_wait_for_gpu_timeline_semaphores(struct vkd3d_fence_worker *worker) +static void vkd3d_wait_for_gpu_timeline_semaphore(struct vkd3d_fence_worker *worker, + const struct vkd3d_waiting_fence *waiting_fence) { const struct d3d12_device *device = worker->device; const struct vkd3d_vk_device_procs *vk_procs = &device->vk_procs; VkSemaphoreWaitInfoKHR wait_info; - VkSemaphore vk_semaphore; uint64_t counter_value; - unsigned int i, j; + VkResult vr; HRESULT hr; - int vr; - - if (!worker->fence_count) - return; wait_info.sType = VK_STRUCTURE_TYPE_SEMAPHORE_WAIT_INFO_KHR; wait_info.pNext = NULL; - wait_info.flags = VK_SEMAPHORE_WAIT_ANY_BIT_KHR; - wait_info.pSemaphores = worker->vk_semaphores; - wait_info.semaphoreCount = worker->fence_count; - wait_info.pValues = worker->semaphore_wait_values; + wait_info.flags = 0; + wait_info.semaphoreCount = 1; + wait_info.pSemaphores = &waiting_fence->u.vk_semaphore; + wait_info.pValues = &waiting_fence->value; vr = VK_CALL(vkWaitSemaphoresKHR(device->vk_device, &wait_info, ~(uint64_t)0)); if (vr == VK_TIMEOUT) return; if (vr != VK_SUCCESS) { - ERR("Failed to wait for Vulkan timeline semaphores, vr %d.\n", vr); + ERR("Failed to wait for Vulkan timeline semaphore, vr %d.\n", vr); return; } - for (i = 0, j = 0; i < worker->fence_count; ++i) + if ((vr = VK_CALL(vkGetSemaphoreCounterValueKHR(device->vk_device, waiting_fence->u.vk_semaphore, + &counter_value))) < 0) { - struct vkd3d_waiting_fence *current = &worker->fences[i]; - - vk_semaphore = worker->vk_semaphores[i]; - if ((vr = VK_CALL(vkGetSemaphoreCounterValueKHR(device->vk_device, vk_semaphore, &counter_value))) < 0) - { - ERR("Failed to get Vulkan semaphore value, vr %d.\n", vr); - } - else if (counter_value >= current->value) - { - TRACE("Signaling fence %p value %#"PRIx64".\n", current->fence, current->value); - if (FAILED(hr = d3d12_fence_signal(current->fence, counter_value, VK_NULL_HANDLE))) - ERR("Failed to signal D3D12 fence, hr %#x.\n", hr); - - d3d12_fence_decref(current->fence); - continue; - } + ERR("Failed to get Vulkan semaphore value, vr %d.\n", vr); + } + else + { + TRACE("Signaling fence %p value %#"PRIx64".\n", waiting_fence->fence, waiting_fence->value); + if (FAILED(hr = d3d12_fence_signal(waiting_fence->fence, counter_value, VK_NULL_HANDLE))) + ERR("Failed to signal D3D12 fence, hr %#x.\n", hr); - if (i != j) - { - worker->vk_semaphores[j] = worker->vk_semaphores[i]; - worker->semaphore_wait_values[j] = worker->semaphore_wait_values[i]; - worker->fences[j] = worker->fences[i]; - } - ++j; + d3d12_fence_decref(waiting_fence->fence); } - worker->fence_count = j; } -static void vkd3d_wait_for_gpu_fences(struct vkd3d_fence_worker *worker) +static void vkd3d_wait_for_gpu_fence(struct vkd3d_fence_worker *worker, + const struct vkd3d_waiting_fence *waiting_fence) { struct d3d12_device *device = worker->device; const struct vkd3d_vk_device_procs *vk_procs = &device->vk_procs; - unsigned int i, j; - VkFence vk_fence; HRESULT hr; int vr; - if (!worker->fence_count) - return; - - vr = VK_CALL(vkWaitForFences(device->vk_device, - worker->fence_count, worker->vk_fences, VK_FALSE, ~(uint64_t)0)); + vr = VK_CALL(vkWaitForFences(device->vk_device, 1, &waiting_fence->u.vk_fence, VK_FALSE, ~(uint64_t)0)); if (vr == VK_TIMEOUT) return; if (vr != VK_SUCCESS) { - ERR("Failed to wait for Vulkan fences, vr %d.\n", vr); + ERR("Failed to wait for Vulkan fence, vr %d.\n", vr); return; } - for (i = 0, j = 0; i < worker->fence_count; ++i) - { - vk_fence = worker->vk_fences[i]; - if (!(vr = VK_CALL(vkGetFenceStatus(device->vk_device, vk_fence)))) - { - struct vkd3d_waiting_fence *current = &worker->fences[i]; + TRACE("Signaling fence %p value %#"PRIx64".\n", waiting_fence->fence, waiting_fence->value); + if (FAILED(hr = d3d12_fence_signal(waiting_fence->fence, waiting_fence->value, waiting_fence->u.vk_fence))) + ERR("Failed to signal D3D12 fence, hr %#x.\n", hr); - TRACE("Signaling fence %p value %#"PRIx64".\n", current->fence, current->value); - if (FAILED(hr = d3d12_fence_signal(current->fence, current->value, vk_fence))) - ERR("Failed to signal D3D12 fence, hr %#x.\n", hr); + d3d12_fence_decref(waiting_fence->fence); - d3d12_fence_decref(current->fence); - - vkd3d_queue_update_sequence_number(current->queue, current->queue_sequence_number, device); - continue; - } - - if (vr != VK_NOT_READY) - ERR("Failed to get Vulkan fence status, vr %d.\n", vr); - - if (i != j) - { - worker->vk_fences[j] = worker->vk_fences[i]; - worker->fences[j] = worker->fences[i]; - } - ++j; - } - worker->fence_count = j; + vkd3d_queue_update_sequence_number(worker->queue, waiting_fence->queue_sequence_number, device); } static void *vkd3d_fence_worker_main(void *arg) { + size_t old_fences_size, cur_fences_size = 0, cur_fence_count = 0; + struct vkd3d_waiting_fence *old_fences, *cur_fences = NULL; struct vkd3d_fence_worker *worker = arg; + unsigned int i; int rc; vkd3d_set_thread_name("vkd3d_fence"); for (;;) { - worker->wait_for_gpu_fences(worker); + if ((rc = vkd3d_mutex_lock(&worker->mutex))) + { + ERR("Failed to lock mutex, error %d.\n", rc); + break; + } - if (!worker->fence_count || InterlockedAdd(&worker->enqueued_fence_count, 0)) + if (!worker->fence_count && !worker->should_exit && (rc = vkd3d_cond_wait(&worker->cond, &worker->mutex))) { - if ((rc = vkd3d_mutex_lock(&worker->mutex))) - { - ERR("Failed to lock mutex, error %d.\n", rc); - break; - } + ERR("Failed to wait on condition variable, error %d.\n", rc); + vkd3d_mutex_unlock(&worker->mutex); + break; + } - if (worker->enqueued_fence_count) - { - vkd3d_fence_worker_move_enqueued_fences_locked(worker); - } - else - { - if (worker->should_exit) - { - vkd3d_mutex_unlock(&worker->mutex); - break; - } + if (worker->should_exit) + break; - if ((rc = vkd3d_cond_wait(&worker->cond, &worker->mutex))) - { - ERR("Failed to wait on condition variable, error %d.\n", rc); - vkd3d_mutex_unlock(&worker->mutex); - break; - } - } + old_fences_size = cur_fences_size; + old_fences = cur_fences; - vkd3d_mutex_unlock(&worker->mutex); - } + cur_fence_count = worker->fence_count; + cur_fences_size = worker->fences_size; + cur_fences = worker->fences; + + worker->fence_count = 0; + worker->fences_size = old_fences_size; + worker->fences = old_fences; + + vkd3d_mutex_unlock(&worker->mutex); + + for (i = 0; i < cur_fence_count; ++i) + worker->wait_for_gpu_fence(worker, &cur_fences[i]); } + vkd3d_free(cur_fences); return NULL; } -HRESULT vkd3d_fence_worker_start(struct vkd3d_fence_worker *worker, - struct d3d12_device *device) +static HRESULT vkd3d_fence_worker_start(struct vkd3d_fence_worker *worker, + struct vkd3d_queue *queue, struct d3d12_device *device) { HRESULT hr; int rc; @@ -525,25 +427,15 @@ HRESULT vkd3d_fence_worker_start(struct vkd3d_fence_worker *worker, TRACE("worker %p.\n", worker); worker->should_exit = false; + worker->queue = queue; worker->device = device; - worker->enqueued_fence_count = 0; - worker->enqueued_fences = NULL; - worker->enqueued_fences_size = 0; - worker->fence_count = 0; - - worker->vk_fences = NULL; - worker->vk_fences_size = 0; worker->fences = NULL; worker->fences_size = 0; - worker->vk_semaphores = NULL; - worker->vk_semaphores_size = 0; - worker->semaphore_wait_values = NULL; - worker->semaphore_wait_values_size = 0; - worker->wait_for_gpu_fences = device->use_timeline_semaphores - ? vkd3d_wait_for_gpu_timeline_semaphores : vkd3d_wait_for_gpu_fences; + worker->wait_for_gpu_fence = device->use_timeline_semaphores + ? vkd3d_wait_for_gpu_timeline_semaphore : vkd3d_wait_for_gpu_fence; if ((rc = vkd3d_mutex_init(&worker->mutex))) { @@ -577,7 +469,7 @@ HRESULT vkd3d_fence_worker_start(struct vkd3d_fence_worker *worker, return hr; } -HRESULT vkd3d_fence_worker_stop(struct vkd3d_fence_worker *worker, +static HRESULT vkd3d_fence_worker_stop(struct vkd3d_fence_worker *worker, struct d3d12_device *device) { HRESULT hr; @@ -603,11 +495,7 @@ HRESULT vkd3d_fence_worker_stop(struct vkd3d_fence_worker *worker, vkd3d_cond_destroy(&worker->cond); vkd3d_cond_destroy(&worker->fence_destruction_cond); - vkd3d_free(worker->enqueued_fences); - vkd3d_free(worker->vk_fences); vkd3d_free(worker->fences); - vkd3d_free(worker->vk_semaphores); - vkd3d_free(worker->semaphore_wait_values); return S_OK; } @@ -6227,6 +6115,8 @@ static ULONG STDMETHODCALLTYPE d3d12_command_queue_Release(ID3D12CommandQueue *i { struct d3d12_device *device = command_queue->device; + vkd3d_fence_worker_stop(&command_queue->fence_worker, device); + vkd3d_private_store_destroy(&command_queue->private_store); vkd3d_free(command_queue); @@ -6455,20 +6345,18 @@ static HRESULT vkd3d_enqueue_timeline_semaphore(struct vkd3d_fence_worker *worke return hresult_from_errno(rc); } - if (!vkd3d_array_reserve((void **)&worker->enqueued_fences, &worker->enqueued_fences_size, - worker->enqueued_fence_count + 1, sizeof(*worker->enqueued_fences))) + if (!vkd3d_array_reserve((void **)&worker->fences, &worker->fences_size, + worker->fence_count + 1, sizeof(*worker->fences))) { ERR("Failed to add GPU timeline semaphore.\n"); vkd3d_mutex_unlock(&worker->mutex); return E_OUTOFMEMORY; } - worker->enqueued_fences[worker->enqueued_fence_count].vk_semaphore = vk_semaphore; - waiting_fence = &worker->enqueued_fences[worker->enqueued_fence_count].waiting_fence; + waiting_fence = &worker->fences[worker->fence_count++]; waiting_fence->fence = fence; waiting_fence->value = value; - waiting_fence->queue = queue; - ++worker->enqueued_fence_count; + waiting_fence->u.vk_semaphore = vk_semaphore; d3d12_fence_incref(fence); @@ -6574,7 +6462,10 @@ static HRESULT STDMETHODCALLTYPE d3d12_command_queue_Signal(ID3D12CommandQueue * } if (device->use_timeline_semaphores) - return vkd3d_enqueue_timeline_semaphore(&device->fence_worker, vk_semaphore, fence, value, vkd3d_queue); + { + return vkd3d_enqueue_timeline_semaphore(&command_queue->fence_worker, + vk_semaphore, fence, value, vkd3d_queue); + } if (vk_semaphore && SUCCEEDED(hr = d3d12_fence_add_vk_semaphore(fence, vk_semaphore, vk_fence, value))) vk_semaphore = VK_NULL_HANDLE; @@ -6582,8 +6473,11 @@ static HRESULT STDMETHODCALLTYPE d3d12_command_queue_Signal(ID3D12CommandQueue * vr = VK_CALL(vkGetFenceStatus(device->vk_device, vk_fence)); if (vr == VK_NOT_READY) { - if (SUCCEEDED(hr = vkd3d_enqueue_gpu_fence(&device->fence_worker, vk_fence, fence, value, vkd3d_queue, sequence_number))) + if (SUCCEEDED(hr = vkd3d_enqueue_gpu_fence(&command_queue->fence_worker, + vk_fence, fence, value, vkd3d_queue, sequence_number))) + { vk_fence = VK_NULL_HANDLE; + } } else if (vr == VK_SUCCESS) { @@ -6978,6 +6872,12 @@ static HRESULT d3d12_command_queue_init(struct d3d12_command_queue *queue, if (FAILED(hr = vkd3d_private_store_init(&queue->private_store))) return hr; + if (FAILED(hr = vkd3d_fence_worker_start(&queue->fence_worker, queue->vkd3d_queue, device))) + { + vkd3d_private_store_destroy(&queue->private_store); + return hr; + } + d3d12_device_add_ref(queue->device = device); return S_OK; diff --git a/libs/vkd3d/device.c b/libs/vkd3d/device.c index 1522065f..5f8108ec 100644 --- a/libs/vkd3d/device.c +++ b/libs/vkd3d/device.c @@ -2711,7 +2711,6 @@ static ULONG STDMETHODCALLTYPE d3d12_device_Release(ID3D12Device *iface) vkd3d_gpu_va_allocator_cleanup(&device->gpu_va_allocator); vkd3d_gpu_descriptor_allocator_cleanup(&device->gpu_descriptor_allocator); vkd3d_render_pass_cache_cleanup(&device->render_pass_cache, device); - vkd3d_fence_worker_stop(&device->fence_worker, device); d3d12_device_destroy_pipeline_cache(device); d3d12_device_destroy_vkd3d_queues(device); for (i = 0; i < ARRAY_SIZE(device->desc_mutex); ++i) @@ -4346,11 +4345,8 @@ static HRESULT d3d12_device_init(struct d3d12_device *device, if (FAILED(hr = vkd3d_private_store_init(&device->private_store))) goto out_free_pipeline_cache; - if (FAILED(hr = vkd3d_fence_worker_start(&device->fence_worker, device))) - goto out_free_private_store; - if (FAILED(hr = vkd3d_init_format_info(device))) - goto out_stop_fence_worker; + goto out_free_private_store; if (FAILED(hr = vkd3d_init_null_resources(&device->null_resources, device))) goto out_cleanup_format_info; @@ -4382,8 +4378,6 @@ out_destroy_null_resources: vkd3d_destroy_null_resources(&device->null_resources, device); out_cleanup_format_info: vkd3d_cleanup_format_info(device); -out_stop_fence_worker: - vkd3d_fence_worker_stop(&device->fence_worker, device); out_free_private_store: vkd3d_private_store_destroy(&device->private_store); out_free_pipeline_cache: diff --git a/libs/vkd3d/vkd3d_private.h b/libs/vkd3d/vkd3d_private.h index a0163c8d..350382cd 100644 --- a/libs/vkd3d/vkd3d_private.h +++ b/libs/vkd3d/vkd3d_private.h @@ -335,7 +335,11 @@ struct vkd3d_waiting_fence { struct d3d12_fence *fence; uint64_t value; - struct vkd3d_queue *queue; + union + { + VkFence vk_fence; + VkSemaphore vk_semaphore; + } u; uint64_t queue_sequence_number; }; @@ -347,33 +351,16 @@ struct vkd3d_fence_worker struct vkd3d_cond fence_destruction_cond; bool should_exit; - LONG enqueued_fence_count; - struct vkd3d_enqueued_fence - { - VkFence vk_fence; - VkSemaphore vk_semaphore; - struct vkd3d_waiting_fence waiting_fence; - } *enqueued_fences; - size_t enqueued_fences_size; - size_t fence_count; - VkFence *vk_fences; - size_t vk_fences_size; struct vkd3d_waiting_fence *fences; size_t fences_size; - VkSemaphore *vk_semaphores; - size_t vk_semaphores_size; - uint64_t *semaphore_wait_values; - size_t semaphore_wait_values_size; - void (*wait_for_gpu_fences)(struct vkd3d_fence_worker *worker); + void (*wait_for_gpu_fence)(struct vkd3d_fence_worker *worker, const struct vkd3d_waiting_fence *enqueued_fence); + struct vkd3d_queue *queue; struct d3d12_device *device; }; -HRESULT vkd3d_fence_worker_start(struct vkd3d_fence_worker *worker, struct d3d12_device *device); -HRESULT vkd3d_fence_worker_stop(struct vkd3d_fence_worker *worker, struct d3d12_device *device); - struct vkd3d_gpu_va_allocation { D3D12_GPU_VIRTUAL_ADDRESS base; @@ -1338,6 +1325,7 @@ struct d3d12_command_queue struct vkd3d_queue *vkd3d_queue; + struct vkd3d_fence_worker fence_worker; const struct d3d12_fence *last_waited_fence; uint64_t last_waited_fence_value; @@ -1440,7 +1428,6 @@ struct d3d12_device struct vkd3d_gpu_descriptor_allocator gpu_descriptor_allocator; struct vkd3d_gpu_va_allocator gpu_va_allocator; - struct vkd3d_fence_worker fence_worker; struct vkd3d_mutex mutex; struct vkd3d_mutex desc_mutex[8]; -- 2.35.1

April 29, 2022

[PATCH vkd3d 1/4] vkd3d: Introduce an internal refcount to d3d12_fence to replace the thread waiting mechanism.

by Conor McCarthy

Simplifies the preservation of fence objects until worker threads are done with them, and will be needed when threaded queue submission is added. Signed-off-by: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d/command.c | 72 ++++++++++++-------------------------- libs/vkd3d/vkd3d_private.h | 4 +-- 2 files changed, 24 insertions(+), 52 deletions(-) diff --git a/libs/vkd3d/command.c b/libs/vkd3d/command.c index 09171fe4..7a373b34 100644 --- a/libs/vkd3d/command.c +++ b/libs/vkd3d/command.c @@ -20,6 +20,8 @@ #include "vkd3d_private.h" +static void d3d12_fence_incref(struct d3d12_fence *fence); +static void d3d12_fence_decref(struct d3d12_fence *fence); static HRESULT d3d12_fence_signal(struct d3d12_fence *fence, uint64_t value, VkFence vk_fence); HRESULT vkd3d_queue_create(struct d3d12_device *device, @@ -295,7 +297,7 @@ static HRESULT vkd3d_enqueue_gpu_fence(struct vkd3d_fence_worker *worker, waiting_fence->queue_sequence_number = queue_sequence_number; ++worker->enqueued_fence_count; - InterlockedIncrement(&fence->pending_worker_operation_count); + d3d12_fence_incref(fence); vkd3d_cond_signal(&worker->cond); vkd3d_mutex_unlock(&worker->mutex); @@ -303,37 +305,6 @@ static HRESULT vkd3d_enqueue_gpu_fence(struct vkd3d_fence_worker *worker, return S_OK; } -static void vkd3d_fence_worker_remove_fence(struct vkd3d_fence_worker *worker, struct d3d12_fence *fence) -{ - LONG count; - int rc; - - if (!(count = InterlockedAdd(&fence->pending_worker_operation_count, 0))) - return; - - WARN("Waiting for %u pending fence operations (fence %p).\n", count, fence); - - if ((rc = vkd3d_mutex_lock(&worker->mutex))) - { - ERR("Failed to lock mutex, error %d.\n", rc); - return; - } - - while ((count = InterlockedAdd(&fence->pending_worker_operation_count, 0))) - { - TRACE("Still waiting for %u pending fence operations (fence %p).\n", count, fence); - - worker->pending_fence_destruction = true; - vkd3d_cond_signal(&worker->cond); - - vkd3d_cond_wait(&worker->fence_destruction_cond, &worker->mutex); - } - - TRACE("Removed fence %p.\n", fence); - - vkd3d_mutex_unlock(&worker->mutex); -} - static void vkd3d_fence_worker_move_enqueued_fences_locked(struct vkd3d_fence_worker *worker) { unsigned int i; @@ -432,7 +403,7 @@ static void vkd3d_wait_for_gpu_timeline_semaphores(struct vkd3d_fence_worker *wo if (FAILED(hr = d3d12_fence_signal(current->fence, counter_value, VK_NULL_HANDLE))) ERR("Failed to signal D3D12 fence, hr %#x.\n", hr); - InterlockedDecrement(&current->fence->pending_worker_operation_count); + d3d12_fence_decref(current->fence); continue; } @@ -480,7 +451,7 @@ static void vkd3d_wait_for_gpu_fences(struct vkd3d_fence_worker *worker) if (FAILED(hr = d3d12_fence_signal(current->fence, current->value, vk_fence))) ERR("Failed to signal D3D12 fence, hr %#x.\n", hr); - InterlockedDecrement(&current->fence->pending_worker_operation_count); + d3d12_fence_decref(current->fence); vkd3d_queue_update_sequence_number(current->queue, current->queue_sequence_number, device); continue; @@ -518,12 +489,6 @@ static void *vkd3d_fence_worker_main(void *arg) break; } - if (worker->pending_fence_destruction) - { - vkd3d_cond_broadcast(&worker->fence_destruction_cond); - worker->pending_fence_destruction = false; - } - if (worker->enqueued_fence_count) { vkd3d_fence_worker_move_enqueued_fences_locked(worker); @@ -560,7 +525,6 @@ HRESULT vkd3d_fence_worker_start(struct vkd3d_fence_worker *worker, TRACE("worker %p.\n", worker); worker->should_exit = false; - worker->pending_fence_destruction = false; worker->device = device; worker->enqueued_fence_count = 0; @@ -1026,22 +990,35 @@ static ULONG STDMETHODCALLTYPE d3d12_fence_AddRef(ID3D12Fence *iface) return refcount; } +static void d3d12_fence_incref(struct d3d12_fence *fence) +{ + InterlockedIncrement(&fence->internal_refcount); +} + static ULONG STDMETHODCALLTYPE d3d12_fence_Release(ID3D12Fence *iface) { struct d3d12_fence *fence = impl_from_ID3D12Fence(iface); ULONG refcount = InterlockedDecrement(&fence->refcount); - int rc; TRACE("%p decreasing refcount to %u.\n", fence, refcount); if (!refcount) + d3d12_fence_decref(fence); + + return refcount; +} + +static void d3d12_fence_decref(struct d3d12_fence *fence) +{ + ULONG internal_refcount = InterlockedDecrement(&fence->internal_refcount); + int rc; + + if (!internal_refcount) { struct d3d12_device *device = fence->device; vkd3d_private_store_destroy(&fence->private_store); - vkd3d_fence_worker_remove_fence(&device->fence_worker, fence); - d3d12_fence_destroy_vk_objects(fence); vkd3d_free(fence->events); @@ -1052,8 +1029,6 @@ static ULONG STDMETHODCALLTYPE d3d12_fence_Release(ID3D12Fence *iface) d3d12_device_release(device); } - - return refcount; } static HRESULT STDMETHODCALLTYPE d3d12_fence_GetPrivateData(ID3D12Fence *iface, @@ -1380,6 +1355,7 @@ static HRESULT d3d12_fence_init(struct d3d12_fence *fence, struct d3d12_device * int rc; fence->ID3D12Fence_iface.lpVtbl = &d3d12_fence_vtbl; + fence->internal_refcount = 1; fence->refcount = 1; fence->value = initial_value; @@ -1419,8 +1395,6 @@ static HRESULT d3d12_fence_init(struct d3d12_fence *fence, struct d3d12_device * memset(fence->old_vk_fences, 0, sizeof(fence->old_vk_fences)); - fence->pending_worker_operation_count = 0; - if (FAILED(hr = vkd3d_private_store_init(&fence->private_store))) { vkd3d_mutex_destroy(&fence->mutex); @@ -6496,7 +6470,7 @@ static HRESULT vkd3d_enqueue_timeline_semaphore(struct vkd3d_fence_worker *worke waiting_fence->queue = queue; ++worker->enqueued_fence_count; - InterlockedIncrement(&fence->pending_worker_operation_count); + d3d12_fence_incref(fence); vkd3d_cond_signal(&worker->cond); vkd3d_mutex_unlock(&worker->mutex); diff --git a/libs/vkd3d/vkd3d_private.h b/libs/vkd3d/vkd3d_private.h index 56060b6d..a0163c8d 100644 --- a/libs/vkd3d/vkd3d_private.h +++ b/libs/vkd3d/vkd3d_private.h @@ -346,7 +346,6 @@ struct vkd3d_fence_worker struct vkd3d_cond cond; struct vkd3d_cond fence_destruction_cond; bool should_exit; - bool pending_fence_destruction; LONG enqueued_fence_count; struct vkd3d_enqueued_fence @@ -532,6 +531,7 @@ struct vkd3d_pending_fence_wait struct d3d12_fence { ID3D12Fence ID3D12Fence_iface; + LONG internal_refcount; LONG refcount; uint64_t value; @@ -555,8 +555,6 @@ struct d3d12_fence struct list semaphores; unsigned int semaphore_count; - LONG pending_worker_operation_count; - VkFence old_vk_fences[VKD3D_MAX_VK_SYNC_OBJECTS]; struct d3d12_device *device; -- 2.35.1

April 29, 2022

[PATCH 2/2] programs/cmd: skip too long paths

by Eric Pouech

From: Eric Pouech <eric.pouech(a)gmail.com> Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- programs/cmd/builtins.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index dd3ebf76d50..5ae5a129d90 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -1355,6 +1355,11 @@ static BOOL WCMD_delete_one (const WCHAR *thisArg) { DIRECTORY_STACK *nextDir; WCHAR subParm[MAX_PATH]; + if (wcslen(thisDir) + wcslen(fd.cFileName) + 1 + wcslen(fname) + wcslen(ext) >= MAX_PATH) + { + WINE_TRACE("Skipping path too long %ls%ls\\%ls%ls\n", thisDir, fd.cFileName, fname, ext); + continue; + } /* Work out search parameter in sub dir */ lstrcpyW (subParm, thisDir); lstrcatW (subParm, fd.cFileName); @@ -1761,7 +1766,13 @@ static void WCMD_add_dirstowalk(DIRECTORY_STACK *dirsToWalk) { (lstrcmpW(fd.cFileName, L"..") != 0) && (lstrcmpW(fd.cFileName, L".") != 0)) { /* Allocate memory, add to list */ - DIRECTORY_STACK *toWalk = heap_xalloc(sizeof(DIRECTORY_STACK)); + DIRECTORY_STACK *toWalk; + if (wcslen(dirsToWalk->dirName) + 1 + wcslen(fd.cFileName) >= MAX_PATH) + { + WINE_TRACE("Skipping too long path %ls\\%ls\n", dirsToWalk->dirName, fd.cFileName); + continue; + } + toWalk = heap_xalloc(sizeof(DIRECTORY_STACK)); WINE_TRACE("(%p->%p)\n", remainingDirs, remainingDirs->next); toWalk->next = remainingDirs->next; remainingDirs->next = toWalk; @@ -2321,6 +2332,11 @@ void WCMD_for (WCHAR *p, CMD_LIST **cmdList) { WINE_TRACE("Processing FOR filename %s\n", wine_dbgstr_w(fd.cFileName)); if (doRecurse) { + if (wcslen(dirsToWalk->dirName) + 1 + wcslen(fd.cFileName) >= MAX_PATH) + { + WINE_TRACE("Skipping too long path %ls\\%ls\n", dirsToWalk->dirName, fd.cFileName); + continue; + } lstrcpyW(fullitem, dirsToWalk->dirName); lstrcatW(fullitem, L"\\"); lstrcatW(fullitem, fd.cFileName); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/17

April 29, 2022

[PATCH 1/2] programs/cmd: test that read is successful before using its result

by Eric Pouech

From: Eric Pouech <eric.pouech(a)gmail.com> There are cases where the read can fail (not attached to a console, input stream mapped to /dev/null...) Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- programs/cmd/builtins.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index 963a9eaf361..dd3ebf76d50 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -193,7 +193,8 @@ static BOOL WCMD_ask_confirm (const WCHAR *message, BOOL showSureText, if (showSureText) WCMD_output_asis (confirm); WCMD_output_asis (options); - WCMD_ReadFile(GetStdHandle(STD_INPUT_HANDLE), answer, ARRAY_SIZE(answer), &count); + if (!WCMD_ReadFile(GetStdHandle(STD_INPUT_HANDLE), answer, ARRAY_SIZE(answer), &count)) + return FALSE; answer[0] = towupper(answer[0]); if (answer[0] == Ybuffer[0]) return TRUE; @@ -383,7 +384,12 @@ void WCMD_choice (const WCHAR * args) { /* FIXME: Add support for option /T */ answer[1] = 0; /* terminate single character string */ - WCMD_ReadFile(GetStdHandle(STD_INPUT_HANDLE), answer, 1, &count); + if (!WCMD_ReadFile(GetStdHandle(STD_INPUT_HANDLE), answer, 1, &count)) + { + heap_free(my_command); + errorlevel = 0; + return; + } if (!opt_s) answer[0] = towupper(answer[0]); @@ -3506,8 +3512,8 @@ void WCMD_setshow_date (void) { WCMD_output (WCMD_LoadMessage(WCMD_CURRENTDATE), curdate); if (wcsstr(quals, L"/T") == NULL) { WCMD_output (WCMD_LoadMessage(WCMD_NEWDATE)); - WCMD_ReadFile(GetStdHandle(STD_INPUT_HANDLE), buffer, ARRAY_SIZE(buffer), &count); - if (count > 2) { + if (WCMD_ReadFile(GetStdHandle(STD_INPUT_HANDLE), buffer, ARRAY_SIZE(buffer), &count) && + count > 2) { WCMD_output_stderr (WCMD_LoadMessage(WCMD_NYI)); } } @@ -4142,8 +4148,7 @@ void WCMD_setshow_env (WCHAR *s) { if (*p) WCMD_output_asis(p); /* Read the reply */ - WCMD_ReadFile(GetStdHandle(STD_INPUT_HANDLE), string, ARRAY_SIZE(string), &count); - if (count > 1) { + if (WCMD_ReadFile(GetStdHandle(STD_INPUT_HANDLE), string, ARRAY_SIZE(string), &count) && count > 1) { string[count-1] = '\0'; /* ReadFile output is not null-terminated! */ if (string[count-2] == '\r') string[count-2] = '\0'; /* Under Windoze we get CRLF! */ WINE_TRACE("set /p: Setting var '%s' to '%s'\n", wine_dbgstr_w(s), @@ -4295,8 +4300,8 @@ void WCMD_setshow_time (void) { WCMD_output (WCMD_LoadMessage(WCMD_CURRENTTIME), curtime); if (wcsstr(quals, L"/T") == NULL) { WCMD_output (WCMD_LoadMessage(WCMD_NEWTIME)); - WCMD_ReadFile(GetStdHandle(STD_INPUT_HANDLE), buffer, ARRAY_SIZE(buffer), &count); - if (count > 2) { + if (WCMD_ReadFile(GetStdHandle(STD_INPUT_HANDLE), buffer, ARRAY_SIZE(buffer), &count) && + count > 2) { WCMD_output_stderr (WCMD_LoadMessage(WCMD_NYI)); } } @@ -4717,8 +4722,8 @@ int WCMD_volume(BOOL set_label, const WCHAR *path) HIWORD(serial), LOWORD(serial)); if (set_label) { WCMD_output (WCMD_LoadMessage(WCMD_VOLUMEPROMPT)); - WCMD_ReadFile(GetStdHandle(STD_INPUT_HANDLE), string, ARRAY_SIZE(string), &count); - if (count > 1) { + if (WCMD_ReadFile(GetStdHandle(STD_INPUT_HANDLE), string, ARRAY_SIZE(string), &count) && + count > 1) { string[count-1] = '\0'; /* ReadFile output is not null-terminated! */ if (string[count-2] == '\r') string[count-2] = '\0'; /* Under Windoze we get CRLF! */ } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/17

April 29, 2022

[PATCH 0/2] MR17: programs/cmd: test that read is successful before using its result

by eric pouech (＠epo)

There are cases where the read can fail (not attached to a console, input stream mapped to /dev/null...) Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/17

April 29, 2022

[PATCH 1/1] wined3d: Reduce the size of staging BO's for texture uploads.

by Jan Sikorski

From: Jan Sikorski <jsikorski(a)codeweavers.com> Signed-off-by: Jan Sikorski <jsikorski(a)codeweavers.com> --- dlls/wined3d/texture.c | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/dlls/wined3d/texture.c b/dlls/wined3d/texture.c index 5fd38b49132..c7086f03a42 100644 --- a/dlls/wined3d/texture.c +++ b/dlls/wined3d/texture.c @@ -4876,6 +4876,7 @@ static void wined3d_texture_vk_upload_data(struct wined3d_context *context, struct wined3d_context_vk *context_vk = wined3d_context_vk(context); unsigned int dst_level, dst_row_pitch, dst_slice_pitch; struct wined3d_texture_sub_resource *sub_resource; + unsigned int src_width, src_height, src_depth; struct wined3d_bo_address staging_bo_addr; VkPipelineStageFlags bo_stage_flags = 0; const struct wined3d_vk_info *vk_info; @@ -4933,6 +4934,10 @@ static void wined3d_texture_vk_upload_data(struct wined3d_context *context, sub_resource = &dst_texture_vk->t.sub_resources[dst_sub_resource_idx]; vk_info = context_vk->vk_info; + src_width = src_box->right - src_box->left; + src_height = src_box->bottom - src_box->top; + src_depth = src_box->back - src_box->front; + src_offset = src_box->front * src_slice_pitch + (src_box->top / src_format->block_height) * src_row_pitch + (src_box->left / src_format->block_width) * src_format->block_byte_count; @@ -4945,10 +4950,15 @@ static void wined3d_texture_vk_upload_data(struct wined3d_context *context, /* We need to be outside of a render pass for vkCmdPipelineBarrier() and vkCmdCopyBufferToImage() calls below. */ wined3d_context_vk_end_current_render_pass(context_vk); - if (!src_bo_addr->buffer_object) { - if (!wined3d_context_vk_create_bo(context_vk, sub_resource->size, + unsigned int staging_row_pitch, staging_slice_pitch, staging_size; + + wined3d_format_calculate_pitch(src_format, context->device->surface_alignment, src_width, src_height, + &staging_row_pitch, &staging_slice_pitch); + staging_size = staging_slice_pitch * src_depth; + + if (!wined3d_context_vk_create_bo(context_vk, staging_size, VK_BUFFER_USAGE_TRANSFER_SRC_BIT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, &staging_bo)) { ERR("Failed to create staging bo.\n"); @@ -4958,26 +4968,25 @@ static void wined3d_texture_vk_upload_data(struct wined3d_context *context, staging_bo_addr.buffer_object = &staging_bo.b; staging_bo_addr.addr = NULL; if (!(map_ptr = wined3d_context_map_bo_address(context, &staging_bo_addr, - sub_resource->size, WINED3D_MAP_DISCARD | WINED3D_MAP_WRITE))) + staging_size, WINED3D_MAP_DISCARD | WINED3D_MAP_WRITE))) { ERR("Failed to map staging bo.\n"); wined3d_context_vk_destroy_bo(context_vk, &staging_bo); return; } - wined3d_format_copy_data(src_format, src_bo_addr->addr + src_offset, src_row_pitch, - src_slice_pitch, map_ptr, dst_row_pitch, dst_slice_pitch, src_box->right - src_box->left, - src_box->bottom - src_box->top, src_box->back - src_box->front); + wined3d_format_copy_data(src_format, src_bo_addr->addr, src_row_pitch, src_slice_pitch, + map_ptr, staging_row_pitch, staging_slice_pitch, src_width, src_height, src_depth); range.offset = 0; - range.size = sub_resource->size; + range.size = staging_size; wined3d_context_unmap_bo_address(context, &staging_bo_addr, 1, &range); src_bo = &staging_bo; src_offset = 0; - src_row_pitch = dst_row_pitch; - src_slice_pitch = dst_slice_pitch; + src_row_pitch = staging_row_pitch; + src_slice_pitch = staging_slice_pitch; } else { @@ -5027,9 +5036,9 @@ static void wined3d_texture_vk_upload_data(struct wined3d_context *context, region.imageOffset.x = dst_x; region.imageOffset.y = dst_y; region.imageOffset.z = dst_z; - region.imageExtent.width = src_box->right - src_box->left; - region.imageExtent.height = src_box->bottom - src_box->top; - region.imageExtent.depth = src_box->back - src_box->front; + region.imageExtent.width = src_width; + region.imageExtent.height = src_height; + region.imageExtent.depth = src_depth; VK_CALL(vkCmdCopyBufferToImage(vk_command_buffer, src_bo->vk_buffer, dst_texture_vk->image.vk_image, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, 1, &region)); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/16

April 29, 2022