cvs commit: src/sys/dev/acpica acpi.c

[John Baldwin]

On Wednesday 15 June 2005 09:14 pm, Nate Lawson wrote:
> John Baldwin wrote:
> > On Wednesday 15 June 2005 04:11 pm, Nate Lawson wrote:
> >>Eric Anderson wrote:
> >>>>>>Ok - I've narrowed it down.  A GENERIC kernel will go into S3 just
> >>>>>>fine on this laptop.  Removing apic from the kernel will break that.
> >>
> >>It is interesting that the suspend path without the apic support causes
> >>a hang for you.  This should be investigated.  Does it work if you have
> >>apic compiled in but boot with hint.apic.0.disabled="1" ? Any ideas
> >>where to look John?
> >
> > Not for the !APIC case, no.  It would probably be good to get that
> > working first before trying to work on the APIC case.  Perhaps the ioapic
> > resume code is using locks when it shouldn't though.  Is it not safe to
> > grab a spin lock when intr_resume() is called?
>
> Without apic compiled in (still waiting on his reply to the "compiled in
> but disabled" case), his system hangs when writing to PM1A, which is an
> IO port in the ICH that indicates the requested sleep state.  I haven't
> checked errata for the ICH6 which I think he has.  Perhaps there is a
> PIC mode erratum for S3.
>
> I don't think spinlocks will be a problem.  However, you are executing
> intr_resume() before several things have been cleaned up:
>
> } else {
> 		/* Execute Wakeup */
> 		intr_resume();
>
> 		if (bootverbose) {
> 			acpi_savecpu();
> 			acpi_printcpu();
> 		}
> 	}
>
> out:
> 	pmap_remove(pm, sc->acpi_wakephys, sc->acpi_wakephys + PAGE_SIZE);
> 	load_cr3(cr3);
> 	write_eflags(ef);
>
> Maybe you should probably wait until after cr3 and eflags are restored?
>   Except I think the eflags write might reenable interrupts.

You can always work around eflags and restore PSL_I later.  I only put 
intr_resume() where we used to reprogram the PIC.  If it needs to happen 
later, feel free to move it to a safer place.  To restore PSL_I later, just 
do 'write_eflags(ef & ~PSL_I)' and later on when you want to turn interrupts 
back on do 'if (ef & PSL_I) enable_intr();'

> >>>>>ioapic_suspend: not implemented!
> >>>>>ioapic_suspend: not implemented!
> >>
> >>I still think this needs to be implemented although it's not likely to
> >>be your problem.
> >
> > Actually, we already reprogram all the APIC intpins on resume in
> > ioapic_resume() from saved state.  There's actually not anything for
> > ioapic_suspend() to do, so I've mostly left this as a marker until the
> > current resume code is tested.
>
> Shouldn't suspend be masking any interrupts in the apic?  We do a cli in
> the suspend path, but I thought the apic might need to be programmed to
> unhook interrupts before reprogramming in resume.  In most systems, apic
> should be powered down completely in S3 since lapic is part of the CPU
> and ioapic is part of the chipset.  Only thing that should be powered in
> S3 is the dram refresh of the MCH and the device wake capabilities of
> the ICH.  I guess I'm saying that I'm wondering if more needs to be done
> in the suspend path.

We don't do anything like that for the normal PICs and never have.  We just 
let PSL_I mask it on the CPU and reprogram the PICs on resume.

-- 
John Baldwin <jhb at FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve"  =  http://www.FreeBSD.org