Opened 16 years ago
Closed 15 years ago
#3541 closed bug (fixed)
Core 2 Duo shows as Core 2 Extreme
Reported by: | j_freeman | Owned by: | stippi |
---|---|---|---|
Priority: | normal | Milestone: | R1 |
Component: | - General | Version: | R1/pre-alpha1 |
Keywords: | Cc: | ||
Blocked By: | Blocking: | ||
Platform: | All |
Description
In About This System, Pulse, and part of sysinfo, the processor shows up as an Intel Core 2 Extreme when in fact it is a Core 2 Duo.
Attachments (5)
Change History (20)
by , 16 years ago
Attachment: | about_and_pulse.png added |
---|
follow-up: 2 comment:1 by , 15 years ago
sysinfo was modified in hrev30231 (Timestamp: 04/17/2009 03:59:14 PM (5 months ago)) and should now report "model 17".
"Intel® Processor Identification and the CPUID Instruction, Application Note 485" http://www.intel.com/Assets/PDF/appnote/241618.pdf dated August 2009 reports that family 6, model 17 (or rather model 7 extended model 1) identifies "Intel CoreTM2 Extreme processor, Intel Xeon processor, model 17h." and that all those "processors are manufactured using the 45 nm process."
For the Core 2 Duo's in that appnote is said: "All processors are manufactured using the 65 nm process."
Wikipedia mentions: "See also: Versions of the same Wolfdale core in an LGA 771 are available under the Dual-Core Xeon brand." and lists the "Core 2 Duo E8600" as a Wolfdale CPU. http://en.wikipedia.org/wiki/List_of_Intel_Core_2_microprocessors#.22Wolfdale.22_.2845_nm.29
I don't know a good solution yet.
I'd say this bug is somewhat related to ticket:2002
comment:2 by , 15 years ago
comment:3 by , 15 years ago
Attached patch defaults all 45nm Core2's to Core 2 instead of Core 2 Extreme. This includes Core 2 Duo, Core 2 Quad, Core 2 Extreme and the Xeon-variant.
follow-up: 5 comment:4 by , 15 years ago
Hm, but the original code was perhaps correct for "some" Core 2 Extreme models? I wouldn't know, but that being the case, the patch would now mess it up for those. If memory does not fail me, the original code was actually added to show some Extreme models correctly.
comment:5 by , 15 years ago
True, the original code is more correct for the Extreme models. But those are still Core 2's.
The issue with the Extreme models was that the extended model information wasn't taken into account (and only a hack prevented it from being identified as a Rise processor). And at that time, I don't think there were any regular Duo's or Quad's (or nobody complained then) with the same (extended)model.
Besides:
- the result of get_cpu_model_string() is just a pretty name
- and Extreme is incorrect for the Duo's and Quad's, but Core 2 is not incorrect for the Extreme's (just less specific).
I sent a email to the haiku-development with a suggestion on how to become more specific (for Extreme's and Xeon's).
comment:6 by , 15 years ago
Yes, the problem with this patch is exactly this. It is not fixing anything and, instead, is simply changing the problem to a different one. At least from me, it would be a -1 for this specific patch (because of what I just said) and +1 to a complete fix.
comment:7 by , 15 years ago
Alright. (I should've known BGA wouldn't want to lose his 'Extreme' :-) )
I'll copy-paste my idea for a complete fix from haiku-development.
my idea would be to add a call to get_cpu_struct()
(found in: headers/private/kernel/cpu.h )
in get_cpu_model_string()
(in: headers/private/shared/cpu_type.h )
and searching the arch.model_name field of the result for xeon and extreme and based on the result:
- add nothing to 'Core 2' (for the Core 2 Duo's and Quad's)
- add ' (Xeon)'
- add ' (Extreme)'
I could then do the same for the Xeon from ticket 2002 and any other Xeon.
follow-up: 9 comment:8 by , 15 years ago
Ahahahahahaha... You got me. :)
But, really, this is the second time a patch like this is proposed and although I agree your reasoning is valid (they are all Core 2) a fix like this will end up in delaying a correct fix because it would appear good enough (except for myself, of course, as I would lose my "Extreme" ;) ).
I still see a problem with your new proposed solution as it seems a bit hackish. Of course if there is no other way to get the correct model though CPU ID, then Ithink this is a valid compromise.
follow-up: 10 comment:9 by , 15 years ago
Replying to bga:
But, really, this is the second time a patch like this is proposed
I'd be interested to read the first one and the comments on it. Do you remember anything more about it that would help me locate it?
I still see a problem with your new proposed solution as it seems a bit hackish.
Agreed.
Of course if there is no other way to get the correct model though CPU ID, then I think this is a valid compromise.
I don't think there is another way. The caches (would be a different hack) seem to be the same size. And Intels Application Note 485, Table 2-3 even has a note:
"To differentiate between the processors with the same processor Vendor ID, software should execute the Brand String functions and parse the Brand String"
BGA, could you tell me the string sysinfo gives you for your Core 2 Extreme? If I go by http://www.cpu-world.com/cgi-bin/CPUID.pl?CPUID=1549 (perhaps unreliable) I'd have to exclude the other possibilities before concluding it's a Core 2 Extreme
or look for model numbers like X6800 :(
Perhaps a cleaner solution would be to use the arch.model_name completely and only use the current method if that's unavailable. Would give pretty inconsistent and ugly results though between AMD & Intel, Intels older CPUs & newer, and Intels (R)'s and (TM)'s. :(
comment:10 by , 15 years ago
Replying to v:
Replying to bga:
But, really, this is the second time a patch like this is proposed
I'd be interested to read the first one and the comments on it. Do you remember anything more about it that would help me locate it?
IIRC, http://www.freelists.org/post/haiku-development/Fixing-get-cpu-model-string-Ticket-3541
by , 15 years ago
Attachment: | haiku_ticket4427_3541_try2_r33411.diff added |
---|
patch. different approach
comment:11 by , 15 years ago
re: new patch It still needs work, but shows approach I've chosen. Instead of using the stored model string from the kernel, cpuid is read again as this does not seem to warrant introducing a new syscall.
I've tested the principle on my PC, but as its cpu doesn't identify as a Core 2 Extreme, I'd appreciate it if it's tested on one of the 45nm Core2-based cpu's. I haven't tested it on hrev33411 as that resulted in build failure.
get_cpuid_model_string() was in part taken from src/bin/sysinfo.c
Still left to do: -I will have to compare it to the coding style guide. -I'm not happy with introducing 3 free(cpu) calls in get_cpu_model_string(system_info *info). -It's quite bulky so I'll try to rework it into a reusable function or macro, but at least shows the principle of it and the direction I'm taking.
comment:12 by , 15 years ago
Hi v, thanks for the new patch! I cannot really comment on whether this is the best approach (it looks to me like it gets the job done). Just thought I'd mention that it may be a better approach to pass the new function a pre-allocated string, that way you avoid a lot of potential errors (allocation failure, forgetting to free the string, ...). The code would become cleaner. Just document the expected maximum length of the string above the function and make sure it is honored where it is called.
comment:13 by , 15 years ago
Thank you stippi! That cleaned it up a lot. I had considered passing a pointer to the function, but hadn't thought it through to avoiding *alloc/free.
haiku_ticket4427_3541_try3c.diff has taken stippi's suggestion on board and applies the same approach to post-Pentium3 Xeons and Celerons. Xeons based on Pentium3s and before need to check "cache descriptors" and/or what Intel calls a "Brand ID".
comment:14 by , 15 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:15 by , 15 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Applied in hrev33462. Thanks a bunch. Please reopen if any issues remain!
Screenshot of About this System and Pulse