Opened 10 years ago

Closed 10 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:
Has a Patch: no 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)

about_and_pulse.png (39.8 KB) - added by j_freeman 10 years ago.
Screenshot of About this System and Pulse
sysinfo.txt (1.7 KB) - added by j_freeman 10 years ago.
Output of sysinfo command
haiku_ticket4427_3541.diff (1.4 KB) - added by v 10 years ago.
patch
haiku_ticket4427_3541_try2_r33411.diff (3.2 KB) - added by v 10 years ago.
patch. different approach
haiku_ticket4427_3541_try3c.diff (4.2 KB) - added by v 10 years ago.
patch. different approach

Download all attachments as: .zip

Change History (20)

Changed 10 years ago by j_freeman

Attachment: about_and_pulse.png added

Screenshot of About this System and Pulse

Changed 10 years ago by j_freeman

Attachment: sysinfo.txt added

Output of sysinfo command

comment:1 in reply to:  description ; Changed 10 years ago by v

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 in reply to:  1 Changed 10 years ago by v

Replying to v:

sysinfo was modified in hrev30231 (Timestamp: 04/17/2009 03:59:14 PM (5 months ago)) and should now report "model 17".

...should now report (1*16+7=23) "model 23". 17 would have been in hexadecimal.

Changed 10 years ago by v

Attachment: haiku_ticket4427_3541.diff added

patch

comment:3 Changed 10 years ago by v

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.

comment:4 Changed 10 years ago by stippi

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 in reply to:  4 Changed 10 years ago by v

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 Changed 10 years ago by bga

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 Changed 10 years ago by v

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.

comment:8 Changed 10 years ago by bga

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.

comment:9 in reply to:  8 ; Changed 10 years ago by 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?

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 in reply to:  9 Changed 10 years ago by mmadia

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

Changed 10 years ago by v

patch. different approach

comment:11 Changed 10 years ago by v

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 Changed 10 years ago by stippi

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.

Changed 10 years ago by v

patch. different approach

comment:13 Changed 10 years ago by v

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 Changed 10 years ago by stippi

Owner: changed from axeld to stippi
Status: newassigned

comment:15 Changed 10 years ago by stippi

Resolution: fixed
Status: assignedclosed

Applied in hrev33462. Thanks a bunch. Please reopen if any issues remain!

Note: See TracTickets for help on using tickets.