This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


Re: [Xen-devel] Re: Paravirtualizing bits of acpi access

To: Len Brown <lenb@xxxxxxxxxx>
Subject: Re: [Xen-devel] Re: Paravirtualizing bits of acpi access
From: Jeremy Fitzhardinge <jeremy@xxxxxxxx>
Date: Fri, 27 Mar 2009 20:19:39 -0700
Cc: "Brown, Len" <len.brown@xxxxxxxxx>, "Tian, Kevin" <kevin.tian@xxxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, "Wang, Shane" <shane.wang@xxxxxxxxx>, "Cihula, Joseph" <joseph.cihula@xxxxxxxxx>, the arch/x86 maintainers <x86@xxxxxxxxxx>, "Rafael J. Wysocki" <rjw@xxxxxxx>, "linux-acpi@xxxxxxxxxxxxxxx" <linux-acpi@xxxxxxxxxxxxxxx>, Bjorn Helgaas <bjorn.helgaas@xxxxxx>
Delivery-date: Fri, 27 Mar 2009 20:20:06 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <alpine.LFD.2.00.0903272049230.26419@xxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <49C484B7.20100@xxxxxxxx> <0A882F4D99BBF6449D58E61AAFD7EDD60E5E877B@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <49C88647.8080404@xxxxxxxx> <200903241045.19194.bjorn.helgaas@xxxxxx> <49C91832.8090300@xxxxxxxx> <4F65016F6CB04E49BFFA15D4F7B798D9944E2524@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <alpine.LFD.2.00.0903271744340.26419@xxxxxxxxxxxxxxxxxxxxx> <49CD5F33.7050100@xxxxxxxx> <alpine.LFD.2.00.0903272049230.26419@xxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Thunderbird (X11/20090320)
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

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() 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.


Xen-devel mailing list

<Prev in Thread] Current Thread [Next in Thread>