Signed-off-by: Paul Gofman pgofman@codeweavers.com --- dlls/ntdll/tests/virtual.c | 6 ++++++ programs/wineboot/wineboot.c | 2 ++ 2 files changed, 8 insertions(+)
diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index 9c2cf824018..df5d601927a 100644 --- a/dlls/ntdll/tests/virtual.c +++ b/dlls/ntdll/tests/virtual.c @@ -579,6 +579,12 @@ static void test_user_shared_data(void) ok(user_shared_data->ActiveProcessorCount == NtCurrentTeb()->Peb->NumberOfProcessors || broken(!user_shared_data->ActiveProcessorCount) /* before Win7 */, "Got unexpected ActiveProcessorCount %u.\n", user_shared_data->ActiveProcessorCount); + if (user_shared_data->ActiveProcessorCount) + ok(user_shared_data->ActiveProcessorAffinity == (user_shared_data->ActiveProcessorCount >= 64 + ? ~(ULONG_PTR)0 : ((ULONG_PTR)1 << user_shared_data->ActiveProcessorCount) - 1) + || broken(!user_shared_data->ActiveProcessorAffinity) /* before Win7 */, + "Got unexpected ActiveProcessorAffinity %s.\n", + wine_dbgstr_longlong(user_shared_data->ActiveProcessorAffinity)); ok(user_shared_data->ActiveGroupCount == 1 || broken(!user_shared_data->ActiveGroupCount) /* before Win7 */, "Got unexpected ActiveGroupCount %u.\n", user_shared_data->ActiveGroupCount); diff --git a/programs/wineboot/wineboot.c b/programs/wineboot/wineboot.c index 05a5ee6aa62..b3a28444a06 100644 --- a/programs/wineboot/wineboot.c +++ b/programs/wineboot/wineboot.c @@ -332,6 +332,8 @@ static void create_user_shared_data(void) break; } data->ActiveProcessorCount = NtCurrentTeb()->Peb->NumberOfProcessors; + data->ActiveProcessorAffinity = data->ActiveProcessorCount >= 64 + ? ~(ULONG_PTR)0 : ((ULONG_PTR)1 << data->ActiveProcessorCount) - 1; data->ActiveGroupCount = 1;
initialize_xstate_features( data );
On Fri, Nov 20, 2020 at 02:55:48PM +0300, Paul Gofman wrote:
diff --git a/programs/wineboot/wineboot.c b/programs/wineboot/wineboot.c index 05a5ee6aa62..b3a28444a06 100644 --- a/programs/wineboot/wineboot.c +++ b/programs/wineboot/wineboot.c @@ -332,6 +332,8 @@ static void create_user_shared_data(void) break; } data->ActiveProcessorCount = NtCurrentTeb()->Peb->NumberOfProcessors;
- data->ActiveProcessorAffinity = data->ActiveProcessorCount >= 64
Shouldn't this be >= 8 * sizeof(ULONG_PTR) ?
? ~(ULONG_PTR)0 : ((ULONG_PTR)1 << data->ActiveProcessorCount) - 1;data->ActiveGroupCount = 1;
initialize_xstate_features( data );
Also ActiveProcessorAffinity's type is currently ULONG, not ULONG_PTR. Perhaps it should actually be ULONGLONG?
Huw.
On 11/20/20 15:54, Huw Davies wrote:
On Fri, Nov 20, 2020 at 02:55:48PM +0300, Paul Gofman wrote:
diff --git a/programs/wineboot/wineboot.c b/programs/wineboot/wineboot.c index 05a5ee6aa62..b3a28444a06 100644 --- a/programs/wineboot/wineboot.c +++ b/programs/wineboot/wineboot.c @@ -332,6 +332,8 @@ static void create_user_shared_data(void) break; } data->ActiveProcessorCount = NtCurrentTeb()->Peb->NumberOfProcessors;
- data->ActiveProcessorAffinity = data->ActiveProcessorCount >= 64
Shouldn't this be >= 8 * sizeof(ULONG_PTR) ?
Yes, I will update it.
? ~(ULONG_PTR)0 : ((ULONG_PTR)1 << data->ActiveProcessorCount) - 1;data->ActiveGroupCount = 1;
initialize_xstate_features( data );
Also ActiveProcessorAffinity's type is currently ULONG, not ULONG_PTR. Perhaps it should actually be ULONGLONG?
Yeah, indeed, thanks. The structure's consequen the next field. the next field. Geoff Chappel defines this field as:
union { ULONGLONG AffinityPad; ULONG ActiveProcessorAffinity; };
But perhaps we can just make ActiveProcessorAffinity ULONGLONG. This is defined as ULONGLONG Reserved4; in Win DDK 10.0.19041.
Yeah, indeed, thanks. The structure's consequen the next field. the next field.
Sorry, the text meant to be here is: "The structure's consequent field has the correct offset only because of the next field's alignment."
On 11/20/20 16:20, Paul Gofman wrote:
On 11/20/20 15:54, Huw Davies wrote:
On Fri, Nov 20, 2020 at 02:55:48PM +0300, Paul Gofman wrote:
diff --git a/programs/wineboot/wineboot.c b/programs/wineboot/wineboot.c index 05a5ee6aa62..b3a28444a06 100644 --- a/programs/wineboot/wineboot.c +++ b/programs/wineboot/wineboot.c @@ -332,6 +332,8 @@ static void create_user_shared_data(void) break; } data->ActiveProcessorCount = NtCurrentTeb()->Peb->NumberOfProcessors;
- data->ActiveProcessorAffinity = data->ActiveProcessorCount >= 64
Shouldn't this be >= 8 * sizeof(ULONG_PTR) ?
Yes, I will update it.
I actually missed _PTR part. I actually suggest we better simplify things and use sizeof(ULONGLONG). Yes, there is a processor count limit of 32 on Win32, but in this case NtCurrentTeb()->Peb->NumberOfProcessors should already obey the limit (and MAXIMUM_PROCESSORS defined as 32 on Win32), so for that case this check should be just redundant.
On Fri, Nov 20, 2020 at 04:27:15PM +0300, Paul Gofman wrote:
On 11/20/20 16:20, Paul Gofman wrote:
On 11/20/20 15:54, Huw Davies wrote:
On Fri, Nov 20, 2020 at 02:55:48PM +0300, Paul Gofman wrote:
diff --git a/programs/wineboot/wineboot.c b/programs/wineboot/wineboot.c index 05a5ee6aa62..b3a28444a06 100644 --- a/programs/wineboot/wineboot.c +++ b/programs/wineboot/wineboot.c @@ -332,6 +332,8 @@ static void create_user_shared_data(void) break; } data->ActiveProcessorCount = NtCurrentTeb()->Peb->NumberOfProcessors;
- data->ActiveProcessorAffinity = data->ActiveProcessorCount >= 64
Shouldn't this be >= 8 * sizeof(ULONG_PTR) ?
Yes, I will update it.
I actually missed _PTR part. I actually suggest we better simplify things and use sizeof(ULONGLONG). Yes, there is a processor count limit of 32 on Win32, but in this case NtCurrentTeb()->Peb->NumberOfProcessors should already obey the limit (and MAXIMUM_PROCESSORS defined as 32 on Win32), so for that case this check should be just redundant.
Sounds good to me.
Huw.
-
Huw Davies -
Paul Gofman