On Sat, Jul 12, 2008 at 5:49 AM, Bill Holler <Bill.Holler at sun.com> wrote:
> Dana H. Myers wrote:
>> Bill Holler wrote:
>>> Hi Aubrey,
>>>
>>> I suspect we should raise the interrupt priority level high enough
>>> for mutex_enter() to treat AcpiGbl_HardwareLock as a spin lock.
> I was thinking of something else. Raising the spl will not help.
>
>
>>>
>>> AcpiGbl_HardwareLock is declared as an ACPI_SPINLOCK.
>>> AcpiOsCreateLock() initializes AcpiGbl_HardwareLock as a
>>> MUTEX_DRIVER. The mutex must be an ADAPTIVE lock.
>>>
>>>
>>> What is the idle code doing with interrupts while it places the
>>> cpu into deeper c-states with the ACPI interface?
>>> I assume it will be like the current cpu_halt() function masks
>>> interrupts before halting the processor?
>>>
>> I'd like to understand better the exposure here. It's true
>> that AcpiOsCreateLock() uses adaptive mutexes; this hasn't
>> appeared to present a problem in the past and I'd like
>> to understand the specific cases where a spin mutex
>> is truly required. Perhaps the problem is better addressed
>> at a higher algorithmic level, for example.
>
> The ACPI interfaces need to be used to halt the processor into
> ACPI C2 and C3. The idle thread cannot block on a mutex
> because the idle thread must always be dispatch-able.
> It would be much simpler if the idle thread can use the ACPI
> interface instead of alternatives such as creating a new block-able
> thread to do this.
>
> AcpiGbl_HardwareLock is defined as an ACPI_"SPIN"LOCK. :-)
> On the other hand, changing AcpiGbl_HardwareLock does
> sound risky.
>
>
>>> We need interrupts to be masked when the processor wakes
>>> up, so we can fixup the local APIC state.
>> It sounds like there's a local CPU vs. other CPU issue
>> with respect to masking interrupts then?
>
> This is a related but different issue than the above idle thread
> blocking in the ACPI interfaces.
>
> The current idle loop services interrupts from the HALT state
> before continuing with the idle loop. CPUs in deep C-state
> will have to execute some code to cleanup cpu state before
> servicing interrupts.
>
> One option is to have the hardware return control to the idle
> thread with interrupts masked. Another option is to check
> for deep c-state cleanup in the interrupt path similar to
> tlb_service().
>
I think the issue might be different.
The following is what idle code doing.
cpu_acpi_idle()
{
value = AcpiGetRegister(); /* need a lock */
using value to determine next idle type;
cli();
place the processor into Cx;
sti();
}
The problem occurs in AcpiGetRegister(), it uses AcpiGbl_HardwareLock.
I suspect AcpiGbl_HardwareLock really works as a spin lock. Otherwise it
will not go into turnstiles block.
Thanks,
-Aubrey
