Len Brown wrote:
Jeremy, I'm not excited about a proposed change to acpixf.h --
this is the API to ACPICA...
Do you have an issue with the mechanism (using weak function, etc), or just
the placement of the prototypes in that header? Would there be a better
header to put them in? Or would you prefer some other mechanism?
It certainly seems like Xen and tboot should be able to share the same hook,
given that they're doing similar things for similar reasons.
(I don't really understand the structure of all the acpi stuff; I'm just
wading in and making a mess of things until I can close the lid of laptop
successfully.)
Everything in acpi/acpica/ is source code that we share with BSD
via the ACPICA project http://acpica.org/
ACPICA also supplies a couple of the headers outside that directory,
eg. acpixf.h, which is the API between the kernel and ACPICA.
We can, and do, change that API when it makes sense.
However, we want to think carefully before changing it,
for we either cause Linux to diverge, or we have to sell
the same change to several other operating systems.
So ideally we wouuld need to make no Linux-specific, or Xen-specific
changes in that directory.
One possibility is to have this called via
function pointer from ASM and scribble over the default
function pointer for the Xen case. In that case, the Xen
version of the routine would live someplace other than acpi/acpica/ -
someplace with the word xen in the path.
Yes, that would be easy enough to do; we could overwrite it only when
actually running under Xen.
However, we don't need to replace the whole of acpi_enter_sleep_state();
there are two options: we could duplicate the early part of the function
in the Xen version of it, or break just the differing part out via
function pointer. The former has the disadvantage of duplicating code,
but it does allow the same function pointer to be used by the tboot version.
If using _weak can effectively
do that at link time, then fine, if we can do it w/o changing the API --
a _weak annotation is an easy ACPICA/Linux divergencen to manage.
The weak approach is what my proposed patch does:
* the default native-hardware version of the sleep-entry register
writes is broken out into default_acpi_enter_sleep_state()
* it introduces a weak arch_acpi_enter_sleep_state() which just
calls the default case, leaving the normal function unchanged
* in arch/x86/kernel/acpi/sleep.c, it adds an override
arch_acpi_enter_sleep_state(), which checks to see if we're
running under Xen; if not, then it simply calls
default_acpi_enter_sleep_state() as usual; if it does, it calls
xen_acpi_enter_sleep_state()
* xen_acpi_enter_sleep-state() is defined in arch/x86/xen/acpi.c
(Actually it didn't break the Xen version out separately, but it easily
could.)
On the whole, using a function pointer would be a bit cleaner, as it
removes the need for the weak glue functions, at the cost of some
(possible) code duplication.
I don't know if Xen and TXT are exclusive, or if we'd ever need
to handle both cases at the same time. I guess that will influence
if the TXT and Xen use the same approach or something different.
As Kevin said, they're exclusive, so they could share the same function
pointer.
J
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
