Re: CVS commit: src/sys/arch/i386/i386

["Julio M. Merino Vidal" <jmmv84%gmail.com@localhost>]

On 25/07/2007, at 15:41, Juan Romero Pardines wrote:

> Module Name:    src
> Committed By:   xtraeme
> Date:           Wed Jul 25 13:41:54 UTC 2007
>
> Modified Files:
>         src/sys/arch/i386/i386: identcpu.c
>
> Log Message:
> There's no need to check for cpu_vendor before calling est_init(),  
> just
> pass cpu_vendor to it.

Are you sure that is correct?  The original code was only calling  
est_init when the cpu_vendor was one of CPUVENDOR_INTEL or  
CPUVENDOR_IDT.  Now it calls est_init with any vendor.  According to  
the est_init code, I see:

        if (CPUID2FAMILY(curcpu()->ci_signature) == 15)
                bus_clock = p4_get_bus_clock(curcpu());
        else if (CPUID2FAMILY(curcpu()->ci_signature) == 6) {
                if (vendor == CPUVENDOR_IDT)
                        bus_clock = via_get_bus_clock(curcpu());
                else
                        bus_clock = p3_get_bus_clock(curcpu());
        }

So when cpu_vendor != CPUVENDOR_IDT, it calls p3_get_bus_clock.  And  
that function only seems to recognize Intel CPUs.  So with your  
change, this function will get called with any kind of CPU and return  
invalid values.

(It seems that the error paths are graceful and that there won't be  
serious problems, but this feels wrong.  Furthermore the user will  
likely get extra error messages during boot.)

--
Julio M. Merino Vidal <jmmv84%gmail.com@localhost>