Subject: Re: Patches for EST and SMP
To: Juan RP <juan@xtrarom.org>
From: Juraj Hercek <nbsd@hck.sk>
List: tech-kern
Date: 03/18/2007 11:10:00
Juan RP wrote:
> On Saturday 17 March 2007, Juan RP wrote:
>
>> Hi,
>>
>> I'm posting patches for review and test. The patches do:
>>
>
> I fixed two things that were wrong right now:
>
> * I was increasing atomically the counter in curcpu() two times, before
> x86_broadcast_ipi().
> * After a write with msr_cpu_broadcast_write(), ci->ci_msr_rvalue was not
> updated with the new value, so it always had previous value.
>
> A NetBSD user tested it for me with Enhanced Speedstep, and I tested it
> with p4tcc, I can't see any problem and the value is read or written in all
> CPUs correctly.
>
> http://www.netbsd.org/~xtraeme/msr_ipi_handlers.diff
> http://www.netbsd.org/~xtraeme/est_smp.diff
> http://www.netbsd.org/~xtraeme/powernow_k8_smp.diff
>
> Are there any problems with this code now?
> ...
>
I've looked at the changes to est.c and seen an #ifdefs for
multiprocessor support. Wouldn't be possible to hide it behind the
wrmsr/rdmsr functions? (I don't know if it is/isn't against some policy
or whatever - I'm still newbie regarding netbsd kernel...).
I'm writing this because I've a separate patch for est (will post it
soon) which implements undervolting (the new undervolting version is as
much as possible detached from original est code and is a sort of layer
above it), where I have couple of lines with wrmsr/rdmsr which I need
to adjust for multiprocessor according to your changes in est.c.
If it's not viable, It might be useful to have some sort of api to
detach processor specific things at est level. However, this is probably
an issue for a separate mailing thread (if it makes sense to deal with
this at all).
-- Juraj