Better cpuid support

[Robert Lunnon]

On Thursday 15 January 2004 15:53, Alexandre Julliard wrote:
> Robert Lunnon <bobl at optushome.com.au> writes:
> > 1. The original cpuid and i386 detect are merged into a single assembly
> > call for convenience.
>
> Much less readable IMO.

A little less readable in my opinion, this assembly code is documented in the 
intel/AMD handbooks as implemented. It performs a single logical function by 
determining CPUID information from 386 through Athlon.
>
> > 2. The data is then interpreted into a structure for use. Understanding
> > CPUID means knowing the cpuid_t structure and CPUID_IsFeatureAvailable
> > call. All the specifics are isolated into GetId for maintainablity.
>
> There's no reason to introduce that structure, we can set the Windows
> fields directly. We don't need the specifics of CPUID anywhere else
> than for setting the corresponding bits which already have well
> defined names.
>

We disagree, I think this is much easier to maintain, the names I use relating 
to registers are reflected in the cpuid documentation from intel and AMD so 
the actuall cpuid details are clearer. Because the feature names are used in 
adding the equivalent features to the Windows Structures it is easy to see 
where this information comes from.

> > 3. The OS Specifics for testing OS support of multiple CPUs and SSE were
> > moved into their own subroutines. This abstracts these functions to
> > permit the code in the FREEBSD section to be OS independent. Note that I
> > copied/added code to each of these subroutines to support all the
> > platforms that were already supported for that feature. Much of this code
> > is currently redundant unless someone wants to change the existing linux
> > or NETBSD sections (Which currently use other methods). The redundant
> > code was added to allow this to happen.
>
> On linux your new code will cause multiple opening and parsing of
> /proc/cpuinfo (and leak file descriptors...)

OOPs, yes a bug, I will fix that if you like (or you can add the close call). 
And yes this re-reading is a side effect of making the cpuid code OS 
independent (hopefully the linux buffer-cache is good) ... There may be a 
better way to implement this under linux and avoid the re-reading but without 
knowing linux I wanted to avoid adding too many bugs that I can't necessarily 
debug (You already found one !)

>
> > 3. The information collection and feature tests in the FREEBSD code were
> > changed to use cpuid_t and IsFeatureAvailable in a way exactly equivalent
> > to the original adding support for detection of PAE and SSE in the
> > process (Though OS_SSESupported will need a test for SSE for FREEBSD to
> > work)  There is no test for Solaris SSE test since SSE isn't supported at
> > all there (Yet).
>
> The new IsFeatureAvailable thing results in calling cpuid and parsing
> the results again and again at least a dozen times for no good reason.
>
This is true, but it is not very time consuming.  I had considered caching the 
cpuid results which would eliminate the multiple handling but didn't feel it 
was worth the effort. This function does not tend to get called often in 
windows programs (Usually once) but If you like I'll add the caching.

> Please consider working with the existing code instead of throwing it
> all away and replacing it by your own favorite routines which simply
> don't fit in with the rest of the code.

I threw very little away, I just abstracted the interface to the cpuid 
instruction and OS specifics. The change in the assembly code is incidental.



Bob