[PATCH v3 0/1] MR2710: ntdll: Implement efficiency class reporting for intel hybrid CPUs.
Signed-off-by: Torge Matthies <tmatthies(a)codeweavers.com> -- v3: ntdll: Implement efficiency class reporting for intel hybrid CPUs. https://gitlab.winehq.org/wine/wine/-/merge_requests/2710
From: Torge Matthies <tmatthies(a)codeweavers.com> Signed-off-by: Torge Matthies <tmatthies(a)codeweavers.com> --- dlls/ntdll/unix/system.c | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/dlls/ntdll/unix/system.c b/dlls/ntdll/unix/system.c index 04755fac5ea..087b4b3894a 100644 --- a/dlls/ntdll/unix/system.c +++ b/dlls/ntdll/unix/system.c @@ -231,6 +231,7 @@ struct smbios_chassis_args SYSTEM_CPU_INFORMATION cpu_info = { 0 }; static SYSTEM_PROCESSOR_FEATURES_INFORMATION cpu_features; static char cpu_name[49]; +static ULONG performance_cores[1024]; static SYSTEM_LOGICAL_PROCESSOR_INFORMATION *logical_proc_info; static unsigned int logical_proc_info_len, logical_proc_info_alloc_len; static SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX *logical_proc_info_ex; @@ -643,7 +644,10 @@ static BOOL logical_proc_info_ex_add_by_id( LOGICAL_PROCESSOR_RELATIONSHIP rel, dataex->u.Processor.Flags = count_bits( mask ) > 1 ? LTP_PC_SMT : 0; else dataex->u.Processor.Flags = 0; - dataex->u.Processor.EfficiencyClass = 0; + if (rel == RelationProcessorCore && id < ARRAY_SIZE(performance_cores) * 32) + dataex->u.Processor.EfficiencyClass = (performance_cores[id / 32] >> (id % 32)) & 1; + else + dataex->u.Processor.EfficiencyClass = 0; dataex->u.Processor.GroupCount = 1; dataex->u.Processor.GroupMask[0].Mask = mask; dataex->u.Processor.GroupMask[0].Group = 0; @@ -846,6 +850,28 @@ static BOOL sysfs_count_list_elements(const char *filename, unsigned int *result return TRUE; } +static void fill_performance_core_info(void) +{ + FILE *fpcore_list; + unsigned int beg, end, i; + char op = ','; + + if (access("/sys/devices/cpu_atom/cpus", R_OK)) return; + + fpcore_list = fopen("/sys/devices/cpu_core/cpus", "r"); + if (!fpcore_list) return; + + while (!feof(fpcore_list) && op == ',') + { + if (!fscanf(fpcore_list, "%u %c ", &beg, &op)) break; + if (op == '-') fscanf(fpcore_list, "%u %c ", &end, &op); + else end = beg; + + for(i = beg; i <= end && i < ARRAY_SIZE(performance_cores) * 32; i++) + performance_cores[i / 32] |= 1 << (i % 32); + } +} + /* for 'data', max_len is the array count. for 'dataex', max_len is in bytes */ static NTSTATUS create_logical_proc_info(void) { @@ -872,6 +898,8 @@ static NTSTATUS create_logical_proc_info(void) max_cpus, MAXIMUM_PROCESSORS); } + fill_performance_core_info(); + fcpu_list = fopen("/sys/devices/system/cpu/online", "r"); if (!fcpu_list) return STATUS_NOT_IMPLEMENTED; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2710
oops (fixed some /tmp paths that I used for testing) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2710#note_31195
Zebediah Figura (@zfigura) commented about dlls/ntdll/unix/system.c:
return TRUE; }
+static void fill_performance_core_info(void) +{ + FILE *fpcore_list; + unsigned int beg, end, i; + char op = ','; + + if (access("/sys/devices/cpu_atom/cpus", R_OK)) return;
What's the point of this access() call? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2710#note_31270
Zebediah Figura (@zfigura) commented about dlls/ntdll/unix/system.c:
SYSTEM_CPU_INFORMATION cpu_info = { 0 }; static SYSTEM_PROCESSOR_FEATURES_INFORMATION cpu_features; static char cpu_name[49]; +static ULONG performance_cores[1024]; Does this need to take up a whole page? Should we allocate this dynamically?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2710#note_31271
On Wed Apr 26 19:57:29 2023 +0000, Zebediah Figura wrote:
What's the point of this access() call? Just sanity-checking that this processor actually has e-cores, I could remove this if you want that
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2710#note_31273
On Wed Apr 26 19:59:22 2023 +0000, Zebediah Figura wrote:
Does this need to take up a whole page? Should we allocate this dynamically? I was debating whether I want to dynamically allocate and realloc memory for this or just support a max. of 4096 CPUs (probably enough for anyone but statements like that have famously been wrong before). Do you want me to use `realloc` or do you want me to decrease the size of this array?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2710#note_31274
On Wed Apr 26 20:46:27 2023 +0000, Torge Matthies wrote:
Just sanity-checking that this processor actually has e-cores, I could remove this if you want that Well, isn't it redundant with the following fopen()?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2710#note_31275
On Wed Apr 26 20:49:10 2023 +0000, Torge Matthies wrote:
I was debating whether I want to dynamically allocate and realloc memory for this or just support a max. of 4096 CPUs (probably enough for anyone but statements like that have famously been wrong before). Do you want me to use `realloc` or do you want me to decrease the size of this array? I don't think this has anything to do with what I want, but it seems unfortunate that we're using up a lot of memory when in many cases we won't need it, and also unfortunate that we might (eventually) exceed an artificial limit.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2710#note_31276
On Wed Apr 26 20:54:24 2023 +0000, Zebediah Figura wrote:
Well, isn't it redundant with the following fopen()? they're different paths
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2710#note_31300
On Wed Apr 26 20:55:20 2023 +0000, Zebediah Figura wrote:
I don't think this has anything to do with what I want, but it seems unfortunate that we're using up a lot of memory when in many cases we won't need it, and also unfortunate that we might (eventually) exceed an artificial limit. I could allocate the memory while the processor infos are being created and then free it afterwards
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2710#note_31301
On Wed Apr 26 21:24:47 2023 +0000, Torge Matthies wrote:
they're different paths Oh indeed, I missed that.
Still, that does raise the question, under what circumstances would "cpu_core" exist but the CPU not be hybrid? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2710#note_31302
On Wed Apr 26 21:55:13 2023 +0000, Zebediah Figura wrote:
Oh indeed, I missed that. Still, that does raise the question, under what circumstances would "cpu_core" exist but the CPU not be hybrid? I don't think that can ever happen with the current kernel code, but who knows, maybe in the future it could? I just added this check because "this is supposed to exist too so may as well check that".
Another alternative would be to also parse that file and `memset(performance_cores, 0, sizeof(performance_cores));` if that fails. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2710#note_31310
participants (3)
-
Torge Matthies -
Torge Matthies (＠tmatthies)
-
Zebediah Figura (＠zfigura)