Re: CVS commit: src/sys

[Andrew Doran <ad%netbsd.org@localhost>]

On Mon, Feb 28, 2011 at 04:48:43PM +0000, Mindaugas Rasiukevicius wrote:
> "Jukka Ruohonen" <jruoho%netbsd.org@localhost> wrote:
> > Module Name:        src
> > Committed By:       jruoho
> > Date:               Fri Feb 25 09:16:00 UTC 2011
> > 
> > Modified Files:
> >     src/sys/arch/x86/acpi: acpi_cpu_md.c
> >     src/sys/dev/acpi: acpi_cpu.h acpi_cpu_pstate.c
> > 
> > Log Message:
> > Add preliminary support for the IA32_APERF and IA32_MPERF frequency
> > counters. These are not yet used for anything and only Intel is supported
> > at the moment.
> > 
> > ...
> 
> >From acpicpu_md_pstate_percent():
> 
> > +   x86_disable_intr();
> > +
> > +   aperf = sc->sc_pstate_aperf;
> > +   mperf = sc->sc_pstate_mperf;
> > +
> > +   xc = xc_unicast(0, acpicpu_md_pstate_percent_status, sc, NULL, ci);
> > +   xc_wait(xc);
> > +
> > +   x86_enable_intr();
> 
> Why interrupts are disabled here?  Calling xc_wait() with interrupts
> disabled is, at minimum, very expensive, and I would say it is wrong.
> Note that xc_wait(9) does not necessary spin - it can block.

You must not call these interfaces or any kind of MI synchronization
interface, spin mutexes included, with interrupts switched off at the
machine level!