Re: ACPI fundamental locking problems

Jeff Garzik (jgarzik@mandrakesoft.com)
Tue, 03 Jul 2001 15:20:10 -0400


"Grover, Andrew" wrote:
>
> > From: Jeff Garzik [mailto:jgarzik@mandrakesoft.com]
> > events/evxface.c:610:acpi_acquire_global_lock ->
> > events/evmisc.c:337:acpi_ev_acquire_global_lock ->
> > include/platform/acgcc.h:52:ACPI_ACQUIRE_GLOBAL_LOCK
> >
> > My immediate objections are,
> > (a) acgcc.h is re-implementing spinlocks in a non-standard,
> > non-portable, and expensive way, and (b) this entire code path is
> > incredibly expensive.
>
> Well, when I look at other Linux code, I assume that if it does something
> weird, it does it for a reason.
>
> That is the case here. The Global Lock is for synchronizing accesses between
> the OS (that's us) and the firmware (SMI). Normal spinlocks are for intra-OS
> locking. Here, we're synchronizing access with the BIOS. It's different.

I realize what the purpose of the global lock is...

How is the asm code in ACPI_ACQUIRE_GLOBAL_LOCK specific to interaction
between OS and SMI?

> All this code has been working for as long as I can remember.

;-) Under Windows? Irrelevant. Linux uses the hardware totally
differently from Windows.
Under Linux? You cannot come close to saying CONFIG_ACPI is used by a
large number of users... I don't think that claim can really be made
yet.

> You mention twice that it is "expensive". Keeping in mind the 80-20 rule, in
> what way do you find this code costly?

Compare your x86 asm code with the spinlock asm code. More importantly,
why is a spinlock or other kernel intrinsic avoided in acgcc.h? It's
much less portable this way.

> > But my fundamental objection is,
> > we are depending on vendors to get locking right in their
> > ACPI tables.
> > And assume by some magic or design that said locking works
> > with whatever
> > unrelated kernel locking is going on.
>
> By design, it works. We get to slipstream Windows a little here in that
> vendors need to have a working Global Lock interface to pass WHQL.

(1) A working lock interface does not imply that -users- of the lock
interface are correct
(2) Nobody here puts stock in Windows tests, which I'm assuming the WHQL
is.

> > It is my initial belief that a smaller info query interface that can
> > parse useful ACPI would be more effective. For times like
> > suspend/resume where we would want to execute control methods, well,
> > there's always the disasm -> write-small-driver cycle. Who knows if
> > this is a realistics proposed solution. But it really scares me to
> > depend on vendors to get locking right.
>
> We're depending on vendors (aka the BIOS) for all the ACPI tables, as well
> as every other piece of a priori data we need to boot the OS.

The difference with ACPI is that vendors can write code that is executed
in the kernel's context (instead of what you can consider the BIOS's
context). That is a whole new can of worms.

> Could I please ask that you at least show me a system where this is a
> problem before throwing out all the work we've done on ACPI because of this
> technical nit?

ACPI is so new I do not think this is necessary. I am reading straight
from the spec, the same thing system implementors will read.

Anyway, for the main point, which you missed:

The global lock is not ONLY for SMM type stuff. The vendor can use it
in control methods all over the place. And the vendor can just as
easily screw up it up. I do not trust a bios vendor to get runtime OS
locking correct... and that is -totally- different from trusting the
BIOS to provide us with a tiny bit of information.

Look at the Linux boot sequence, which Randy Dunlap documented. We
collect as much information as is reasonable from BIOS at startup, so we
won't have to talk to it again at runtime.

Jeff

-- 
Jeff Garzik      | "I respect faith, but doubt is
Building 1024    |  what gives you an education."
MandrakeSoft     |           -- Wilson Mizner
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/