| Message ID | 49C484B7.20100@goop.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Saturday 21 March 2009, Jeremy Fitzhardinge wrote: > Hi Len, > > I have a patch here ported from the linux2.6.18-xen tree to make host S3 > suspend work under Xen (attached). > > The salient part is this: > > --- a/drivers/acpi/acpica/hwsleep.c > +++ b/drivers/acpi/acpica/hwsleep.c > @@ -46,6 +46,9 @@ > #include "accommon.h" > #include "actables.h" > > +#include <xen/acpi.h> > +#include <asm/xen/hypervisor.h> > + > #define _COMPONENT ACPI_HARDWARE > ACPI_MODULE_NAME("hwsleep") > > @@ -337,14 +340,19 @@ acpi_status asmlinkage acpi_enter_sleep_state(u8 sleep_state) > > ACPI_FLUSH_CPU_CACHE(); > > - status = acpi_hw_register_write(ACPI_REGISTER_PM1A_CONTROL, > - PM1Acontrol); > - if (ACPI_FAILURE(status)) { > - return_ACPI_STATUS(status); > - } > + if (!xen_pv_domain()) { > + status = acpi_hw_register_write(ACPI_REGISTER_PM1A_CONTROL, > + PM1Acontrol); > + if (ACPI_FAILURE(status)) { > + return_ACPI_STATUS(status); > + } > + > + status = acpi_hw_register_write(ACPI_REGISTER_PM1B_CONTROL, > + PM1Bcontrol); > + } else > + status = acpi_notify_hypervisor_state(sleep_state, > + PM1Acontrol, PM1Bcontrol); > > - status = acpi_hw_register_write(ACPI_REGISTER_PM1B_CONTROL, > - PM1Bcontrol); > if (ACPI_FAILURE(status)) { > return_ACPI_STATUS(status); > } > > > where acpi_notify_hypervisor_state() more or less maps directly onto a > Xen hypercall, which in turn performs the corresponding acpi operation > from within Xen. > > I'm guessing you won't find this patch appealing as-is because it sticks > a great big if (xen) into an otherwise arch (and OS?) independent piece > of code. In this particular instance, its fairly easy to envisage > encapsulating these two register writes into their own function which > architectures can override, which makes it fairly easy for me to put a > Xen hook in somewhere on the arch/x86 side of the fence. > > But because Xen ends up doing the low-level cpu state save/restore, > several other parts of the S3 suspend path can be skipped on the Linux > side. I'm wondering if you have any thoughts about how that can be > done, or if putting the Xen code in as-is is acceptable? > > (BTW, xen_pv_domain() expands to a constant 0 when Xen isn't enabled in > the config, so the Xen-dependent bits will collapse down to nothing. > But it is defined in asm/xen/hypervisor.h, which is only present on x86 > and ia64 architectures; on the other hand, believe those are the only > architectures using acpi.) Well, why don't you implement the platform suspend operations for Xen? I guess you don't want ACPI _PTS to be executed during suspend as well. Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Rafael J. Wysocki wrote: > Well, why don't you implement the platform suspend operations for Xen? > I guess you don't want ACPI _PTS to be executed during suspend as well. > I don't know. What's _PTS? I think for the most part we want Linux to do most of the acpi work of bringing the machine into an idle state. Its just that Xen is responsible for the very low level cpu context save/restore, because the Linux kernel is still running on vcpus rather than the physical cpus. J -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sunday 22 March 2009, Jeremy Fitzhardinge wrote: > Rafael J. Wysocki wrote: > > Well, why don't you implement the platform suspend operations for Xen? > > I guess you don't want ACPI _PTS to be executed during suspend as well. > > > > I don't know. What's _PTS? It's an ACPI method called to prepare the platform to enter the sleep state (the name stands for "prepare to sleep"). Executing it may affect the hardware. > I think for the most part we want Linux to do most of the acpi work of > bringing the machine into an idle state. Its just that Xen is > responsible for the very low level cpu context save/restore, because the > Linux kernel is still running on vcpus rather than the physical cpus. I think you really should not execute any global ACPI methods to suspend a guest, because that may affect the host. That's why I think it's better to regard Xen as a platform and implement a separate set of suspend operations for it. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Rafael J. Wysocki <rjw@sisk.pl> wrote: > On Sunday 22 March 2009, Jeremy Fitzhardinge wrote: > > Rafael J. Wysocki wrote: > > > Well, why don't you implement the platform suspend operations for Xen? > > > I guess you don't want ACPI _PTS to be executed during suspend as well. > > > > > > > I don't know. What's _PTS? > > It's an ACPI method called to prepare the platform to enter the sleep state > (the name stands for "prepare to sleep"). Executing it may affect the > hardware. > > > I think for the most part we want Linux to do most of the acpi > > work of bringing the machine into an idle state. Its just that > > Xen is responsible for the very low level cpu context > > save/restore, because the Linux kernel is still running on vcpus > > rather than the physical cpus. > > I think you really should not execute any global ACPI methods to > suspend a guest, because that may affect the host. That's why I > think it's better to regard Xen as a platform and implement a > separate set of suspend operations for it. I'd agree with that. That also allows the reuse of existing callbacks, right? Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sunday 22 March 2009, Ingo Molnar wrote: > > * Rafael J. Wysocki <rjw@sisk.pl> wrote: > > > On Sunday 22 March 2009, Jeremy Fitzhardinge wrote: > > > Rafael J. Wysocki wrote: > > > > Well, why don't you implement the platform suspend operations for Xen? > > > > I guess you don't want ACPI _PTS to be executed during suspend as well. > > > > > > > > > > I don't know. What's _PTS? > > > > It's an ACPI method called to prepare the platform to enter the sleep state > > (the name stands for "prepare to sleep"). Executing it may affect the > > hardware. > > > > > I think for the most part we want Linux to do most of the acpi > > > work of bringing the machine into an idle state. Its just that > > > Xen is responsible for the very low level cpu context > > > save/restore, because the Linux kernel is still running on vcpus > > > rather than the physical cpus. > > > > I think you really should not execute any global ACPI methods to > > suspend a guest, because that may affect the host. That's why I > > think it's better to regard Xen as a platform and implement a > > separate set of suspend operations for it. > > I'd agree with that. That also allows the reuse of existing > callbacks, right? Yes, it does. Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Rafael J. Wysocki wrote: > On Sunday 22 March 2009, Jeremy Fitzhardinge wrote: > >> Rafael J. Wysocki wrote: >> >>> Well, why don't you implement the platform suspend operations for Xen? >>> I guess you don't want ACPI _PTS to be executed during suspend as well. >>> >>> >> I don't know. What's _PTS? >> > > It's an ACPI method called to prepare the platform to enter the sleep state > (the name stands for "prepare to sleep"). Executing it may affect the > hardware. > OK, that's what we want. Dom0 is the control domain which is responsible for the bulk of the hardware; Xen itself has very little hardware knowledge. > I think you really should not execute any global ACPI methods to suspend a > guest, because that may affect the host. That's why I think it's better to > regard Xen as a platform and implement a separate set of suspend operations for > it. > In this case we're talking about the special privileged domain which can be considered to be on the "host" side of the line. That said, I'd be interested in looking at a suspend operations-based approach if you think its the right way to go. But I'm concerned that we'd end up with a big set of very similar-looking parallel functions just to deal with some difference in detail near the bottom. Can you give me a pointer to where this gets put together for acpi? Thanks, J -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sunday 22 March 2009, Jeremy Fitzhardinge wrote: > Rafael J. Wysocki wrote: > > On Sunday 22 March 2009, Jeremy Fitzhardinge wrote: > > > >> Rafael J. Wysocki wrote: > >> > >>> Well, why don't you implement the platform suspend operations for Xen? > >>> I guess you don't want ACPI _PTS to be executed during suspend as well. > >>> > >>> > >> I don't know. What's _PTS? > >> > > > > It's an ACPI method called to prepare the platform to enter the sleep state > > (the name stands for "prepare to sleep"). Executing it may affect the > > hardware. > > > > OK, that's what we want. Dom0 is the control domain which is > responsible for the bulk of the hardware; Xen itself has very little > hardware knowledge. > > > I think you really should not execute any global ACPI methods to suspend a > > guest, because that may affect the host. That's why I think it's better to > > regard Xen as a platform and implement a separate set of suspend operations for > > it. > > > > In this case we're talking about the special privileged domain which can > be considered to be on the "host" side of the line. > > That said, I'd be interested in looking at a suspend operations-based > approach if you think its the right way to go. But I'm concerned that > we'd end up with a big set of very similar-looking parallel functions > just to deal with some difference in detail near the bottom. Can you > give me a pointer to where this gets put together for acpi? Please have a look at include/linux/suspend.h for the prototypes and drivers/acpi/sleep.c contains the ACPI implementation. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>From: Jeremy Fitzhardinge >Sent: Monday, March 23, 2009 1:08 AM > >Rafael J. Wysocki wrote: >> On Sunday 22 March 2009, Jeremy Fitzhardinge wrote: >> >>> Rafael J. Wysocki wrote: >>> >>>> Well, why don't you implement the platform suspend >operations for Xen? >>>> I guess you don't want ACPI _PTS to be executed during >suspend as well. >>>> >>>> >>> I don't know. What's _PTS? >>> >> >> It's an ACPI method called to prepare the platform to enter >the sleep state >> (the name stands for "prepare to sleep"). Executing it may >affect the >> hardware. >> > >OK, that's what we want. Dom0 is the control domain which is >responsible for the bulk of the hardware; Xen itself has very little >hardware knowledge. yes, Xen only takes care of several core system devices (pic, ioapic, serial, timer sources) and cpus, and let dom0 control all the rest. _PTS, imo, will not affect Xen controlled devices as even on native those devices are suspended after _PTS. Also Xen doesn't incorporate ACPI parser and thus can't evaluate any ACPI method on its own. > >> I think you really should not execute any global ACPI >methods to suspend a >> guest, because that may affect the host. That's why I think >it's better to >> regard Xen as a platform and implement a separate set of >suspend operations for >> it. >> > >In this case we're talking about the special privileged domain >which can >be considered to be on the "host" side of the line. > >That said, I'd be interested in looking at a suspend operations-based >approach if you think its the right way to go. But I'm concerned that >we'd end up with a big set of very similar-looking parallel functions >just to deal with some difference in detail near the bottom. Can you >give me a pointer to where this gets put together for acpi? > As Jeremy pointed out, dom0 is a special domain which cooperate with Xen to fulfill the whole S3 sequence, i.e. dom0 still carries 99% existing ACPI S3 flow, with several exceptions as below: a) No need to prepare wakeup stub (since it's Xen to be first waken up after resume), and no expectation that execution flow will be resumed from its wakeup stub (context is resumed at hypercall return) b) No need to save/restore bsp context and gear to Xen by hypercall at last step where originally hardware reg bits are written And then Xen jumps in to finish remaining steps. From this angle, Xen is not a completely new platform and, well, S3 is more like a 'S1' type from dom0's p.o.v with a different trigger method. Then is it overkilled to introduce a new set of ops with 99% content duplicated? Thanks, Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday 23 March 2009, Tian, Kevin wrote: > >From: Jeremy Fitzhardinge > >Sent: Monday, March 23, 2009 1:08 AM > > > >Rafael J. Wysocki wrote: > >> On Sunday 22 March 2009, Jeremy Fitzhardinge wrote: > >> > >>> Rafael J. Wysocki wrote: > >>> > >>>> Well, why don't you implement the platform suspend > >operations for Xen? > >>>> I guess you don't want ACPI _PTS to be executed during > >suspend as well. > >>>> > >>>> > >>> I don't know. What's _PTS? > >>> > >> > >> It's an ACPI method called to prepare the platform to enter > >the sleep state > >> (the name stands for "prepare to sleep"). Executing it may > >affect the > >> hardware. > >> > > > >OK, that's what we want. Dom0 is the control domain which is > >responsible for the bulk of the hardware; Xen itself has very little > >hardware knowledge. > > yes, Xen only takes care of several core system devices (pic, > ioapic, serial, timer sources) and cpus, and let dom0 control all > the rest. _PTS, imo, will not affect Xen controlled devices as > even on native those devices are suspended after _PTS. Also > Xen doesn't incorporate ACPI parser and thus can't evaluate any > ACPI method on its own. > > > > >> I think you really should not execute any global ACPI > >methods to suspend a > >> guest, because that may affect the host. That's why I think > >it's better to > >> regard Xen as a platform and implement a separate set of > >suspend operations for > >> it. > >> > > > >In this case we're talking about the special privileged domain > >which can > >be considered to be on the "host" side of the line. > > > >That said, I'd be interested in looking at a suspend operations-based > >approach if you think its the right way to go. But I'm concerned that > >we'd end up with a big set of very similar-looking parallel functions > >just to deal with some difference in detail near the bottom. Can you > >give me a pointer to where this gets put together for acpi? > > > > As Jeremy pointed out, dom0 is a special domain which cooperate > with Xen to fulfill the whole S3 sequence, i.e. dom0 still carries 99% > existing ACPI S3 flow, with several exceptions as below: > > a) No need to prepare wakeup stub (since it's Xen to be first waken > up after resume), and no expectation that execution flow will be > resumed from its wakeup stub (context is resumed at hypercall > return) > > b) No need to save/restore bsp context and gear to Xen by hypercall > at last step where originally hardware reg bits are written > > And then Xen jumps in to finish remaining steps. From this angle, > Xen is not a completely new platform and, well, S3 is more like a > 'S1' type from dom0's p.o.v with a different trigger method. Then is > it overkilled to introduce a new set of ops with 99% content > duplicated? IMO, no, it isn't. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Rafael J. Wysocki wrote: > On Monday 23 March 2009, Tian, Kevin wrote: > >> And then Xen jumps in to finish remaining steps. From this angle, >> Xen is not a completely new platform and, well, S3 is more like a >> 'S1' type from dom0's p.o.v with a different trigger method. Then is >> it overkilled to introduce a new set of ops with 99% content >> duplicated? >> > > IMO, no, it isn't. Hm. Well, lets take acpi_suspend_enter() as a specific example. The Xen change here is: @@ -240,11 +240,20 @@ static int acpi_suspend_enter(suspend_state_t pm_state) barrier(); status = acpi_enter_sleep_state(acpi_state); break; case ACPI_STATE_S3: - do_suspend_lowlevel(); + if (!xen_pv_domain()) + do_suspend_lowlevel(); + else { + /* + * Xen will save and restore CPU context, so + * we can skip that and just go straight to + * the suspend. + */ + acpi_enter_sleep_state(acpi_state); + } break; } /* If ACPI is not enabled by the BIOS, we need to enable it here. */ if (set_sci_en_on_resume) Which is, functionally, adding one if() and a new line of code, in the middle of a ~70 line function. Are you suggesting that it would be best to copy this whole function so that I can put one line of Xen-specific code in the middle, rather than just making this change? Some other functions, the Xen vs. non-Xen changes are larger; acpi_sleep_prepare() could reasonably have a Xen-specific variant because a big chunk of it is setting up the wakeup vector (which is unnecessary under Xen), and the rest can be easily pulled into common code. But unfortunately acpi_sleep_prepare isn't itself an operation, and is only called at the bottom of 2-3 level deep callchains. I think that rather than having a separate xen-acpi platform_suspend_ops, it would make more sense to have a acpi_ops within acpi/sleep.c and handle the differences that way. I'll see how it turns out. J -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 23, 2009 at 07:20:12PM +0100, Rafael J. Wysocki wrote: > On Monday 23 March 2009, Tian, Kevin wrote: > > And then Xen jumps in to finish remaining steps. From this angle, > > Xen is not a completely new platform and, well, S3 is more like a > > 'S1' type from dom0's p.o.v with a different trigger method. Then is > > it overkilled to introduce a new set of ops with 99% content > > duplicated? > > IMO, no, it isn't. I'd disagree. Duplicating the code means remembering to apply bugfixes to both parts.
On Monday 23 March 2009, Matthew Garrett wrote: > On Mon, Mar 23, 2009 at 07:20:12PM +0100, Rafael J. Wysocki wrote: > > On Monday 23 March 2009, Tian, Kevin wrote: > > > And then Xen jumps in to finish remaining steps. From this angle, > > > Xen is not a completely new platform and, well, S3 is more like a > > > 'S1' type from dom0's p.o.v with a different trigger method. Then is > > > it overkilled to introduce a new set of ops with 99% content > > > duplicated? > > > > IMO, no, it isn't. > > I'd disagree. Duplicating the code means remembering to apply bugfixes > to both parts. What code are you referring to exactly? The callbacks that are literally the same can be pointed to by the new set of operations just fine. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday 23 March 2009, Jeremy Fitzhardinge wrote: > Rafael J. Wysocki wrote: > > On Monday 23 March 2009, Tian, Kevin wrote: > > > >> And then Xen jumps in to finish remaining steps. From this angle, > >> Xen is not a completely new platform and, well, S3 is more like a > >> 'S1' type from dom0's p.o.v with a different trigger method. Then is > >> it overkilled to introduce a new set of ops with 99% content > >> duplicated? > >> > > > > IMO, no, it isn't. > > Hm. Well, lets take acpi_suspend_enter() as a specific example. The > Xen change here is: > > @@ -240,11 +240,20 @@ static int acpi_suspend_enter(suspend_state_t pm_state) > barrier(); > status = acpi_enter_sleep_state(acpi_state); > break; > > case ACPI_STATE_S3: > - do_suspend_lowlevel(); > + if (!xen_pv_domain()) > + do_suspend_lowlevel(); > + else { > + /* > + * Xen will save and restore CPU context, so > + * we can skip that and just go straight to > + * the suspend. > + */ > + acpi_enter_sleep_state(acpi_state); > + } > break; > } Hmm, in that case it may be more appropriate to modify do_suspend_lowlevel(). Have you considered doing that? > /* If ACPI is not enabled by the BIOS, we need to enable it here. */ > if (set_sci_en_on_resume) > > > Which is, functionally, adding one if() and a new line of code, in the > middle of a ~70 line function. > > Are you suggesting that it would be best to copy this whole function so > that I can put one line of Xen-specific code in the middle, rather than > just making this change? No. > Some other functions, the Xen vs. non-Xen changes are larger; > acpi_sleep_prepare() could reasonably have a Xen-specific variant > because a big chunk of it is setting up the wakeup vector (which is > unnecessary under Xen), and the rest can be easily pulled into common > code. But unfortunately acpi_sleep_prepare isn't itself an operation, > and is only called at the bottom of 2-3 level deep callchains. > > I think that rather than having a separate xen-acpi > platform_suspend_ops, it would make more sense to have a acpi_ops within > acpi/sleep.c and handle the differences that way. I'll see how it turns > out. Yes, that may be better. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Rafael J. Wysocki wrote: > Hmm, in that case it may be more appropriate to modify > do_suspend_lowlevel(). Have you considered doing that? > do_suspend_lowlevel is in asm with 32 and 64-bit variants, so it is a little awkward to deal with. But, yes, I was thinking of adding a do_suspend() with this logic in it, which calls do_suspend_lowlevel() as appropriate. J -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From 7bdec2cce7de8fb41207238c61f3075220c70a55 Mon Sep 17 00:00:00 2001 From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com> Date: Mon, 23 Feb 2009 00:06:06 -0800 Subject: [PATCH] xen: Enable ACPI sleep in Xen Open CONFIG_ACPI_SLEEP in xenlinux, to enable ACPI based power management. Basically, user can trigger power event now by "echo *** > /sys/power/state". Also gear to pm interface defined between xenlinux and Xen. Also sync to xen interface headers consequently [ From http://xenbits.xensource.com/linux-2.6.18-xen.hg change c68699484a65 ] Signed-off-by Ke Yu <ke.yu@intel.com> Signed-off-by Kevin Tian <kevin.tian@intel.com> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com> diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c index 7c243a2..a89de8d 100644 --- a/arch/x86/kernel/acpi/sleep.c +++ b/arch/x86/kernel/acpi/sleep.c @@ -12,6 +12,8 @@ #include <asm/segment.h> #include <asm/desc.h> +#include <asm/xen/hypervisor.h> + #include "realmode/wakeup.h" #include "sleep.h" @@ -37,6 +39,9 @@ int acpi_save_state_mem(void) { struct wakeup_header *header; + if (xen_pv_domain()) + return 0; /* Xen will do this for us */ + if (!acpi_realmode) { printk(KERN_ERR "Could not allocate memory during boot, " "S3 disabled\n"); diff --git a/drivers/acpi/acpica/hwsleep.c b/drivers/acpi/acpica/hwsleep.c index a2af2a4..bf719f1 100644 --- a/drivers/acpi/acpica/hwsleep.c +++ b/drivers/acpi/acpica/hwsleep.c @@ -46,6 +46,9 @@ #include "accommon.h" #include "actables.h" +#include <xen/acpi.h> +#include <asm/xen/hypervisor.h> + #define _COMPONENT ACPI_HARDWARE ACPI_MODULE_NAME("hwsleep") @@ -337,14 +340,19 @@ acpi_status asmlinkage acpi_enter_sleep_state(u8 sleep_state) ACPI_FLUSH_CPU_CACHE(); - status = acpi_hw_register_write(ACPI_REGISTER_PM1A_CONTROL, - PM1Acontrol); - if (ACPI_FAILURE(status)) { - return_ACPI_STATUS(status); - } + if (!xen_pv_domain()) { + status = acpi_hw_register_write(ACPI_REGISTER_PM1A_CONTROL, + PM1Acontrol); + if (ACPI_FAILURE(status)) { + return_ACPI_STATUS(status); + } + + status = acpi_hw_register_write(ACPI_REGISTER_PM1B_CONTROL, + PM1Bcontrol); + } else + status = acpi_notify_hypervisor_state(sleep_state, + PM1Acontrol, PM1Bcontrol); - status = acpi_hw_register_write(ACPI_REGISTER_PM1B_CONTROL, - PM1Bcontrol); if (ACPI_FAILURE(status)) { return_ACPI_STATUS(status); } diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c index 5192666..c855dec 100644 --- a/drivers/acpi/sleep.c +++ b/drivers/acpi/sleep.c @@ -59,7 +59,7 @@ static int acpi_sleep_prepare(u32 acpi_state) { #ifdef CONFIG_ACPI_SLEEP /* do we have a wakeup address for S2 and S3? */ - if (acpi_state == ACPI_STATE_S3) { + if (!xen_pv_domain() && acpi_state == ACPI_STATE_S3) { if (!acpi_wakeup_address) { return -EFAULT; } @@ -242,7 +242,16 @@ static int acpi_suspend_enter(suspend_state_t pm_state) break; case ACPI_STATE_S3: - do_suspend_lowlevel(); + if (!xen_pv_domain()) + do_suspend_lowlevel(); + else { + /* + * Xen will save and restore CPU context, so + * we can skip that and just go straight to + * the suspend. + */ + acpi_enter_sleep_state(acpi_state); + } break; } diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig index 51cbaa5..0138113 100644 --- a/drivers/xen/Kconfig +++ b/drivers/xen/Kconfig @@ -76,3 +76,7 @@ config XEN_COMPAT_XENFS config XEN_XENBUS_FRONTEND tristate + +config XEN_S3 + def_bool y + depends on XEN_DOM0 && ACPI \ No newline at end of file diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile index bb88673..4b01fc8 100644 --- a/drivers/xen/Makefile +++ b/drivers/xen/Makefile @@ -7,4 +7,6 @@ obj-$(CONFIG_XEN_BALLOON) += balloon.o obj-$(CONFIG_XEN_DEV_EVTCHN) += evtchn.o obj-$(CONFIG_XEN_BLKDEV_BACKEND) += blkback/ obj-$(CONFIG_XEN_NETDEV_BACKEND) += netback/ -obj-$(CONFIG_XENFS) += xenfs/ \ No newline at end of file +obj-$(CONFIG_XENFS) += xenfs/ + +obj-$(CONFIG_XEN_S3) += acpi.o \ No newline at end of file diff --git a/drivers/xen/acpi.c b/drivers/xen/acpi.c new file mode 100644 index 0000000..e6d3d0e --- /dev/null +++ b/drivers/xen/acpi.c @@ -0,0 +1,23 @@ +#include <xen/acpi.h> + +#include <xen/interface/platform.h> +#include <asm/xen/hypercall.h> +#include <asm/xen/hypervisor.h> + +int acpi_notify_hypervisor_state(u8 sleep_state, + u32 pm1a_cnt, u32 pm1b_cnt) +{ + struct xen_platform_op op = { + .cmd = XENPF_enter_acpi_sleep, + .interface_version = XENPF_INTERFACE_VERSION, + .u = { + .enter_acpi_sleep = { + .pm1a_cnt_val = (u16)pm1a_cnt, + .pm1b_cnt_val = (u16)pm1b_cnt, + .sleep_state = sleep_state, + }, + }, + }; + + return HYPERVISOR_dom0_op(&op); +} diff --git a/include/xen/acpi.h b/include/xen/acpi.h new file mode 100644 index 0000000..0d1e462 --- /dev/null +++ b/include/xen/acpi.h @@ -0,0 +1,9 @@ +#ifndef _XEN_ACPI_H +#define _XEN_ACPI_H + +#include <linux/types.h> + +int acpi_notify_hypervisor_state(u8 sleep_state, + u32 pm1a_cnt, u32 pm1b_cnd); + +#endif /* _XEN_ACPI_H */
Hi Len, I have a patch here ported from the linux2.6.18-xen tree to make host S3 suspend work under Xen (attached). The salient part is this: where acpi_notify_hypervisor_state() more or less maps directly onto a Xen hypercall, which in turn performs the corresponding acpi operation from within Xen. I'm guessing you won't find this patch appealing as-is because it sticks a great big if (xen) into an otherwise arch (and OS?) independent piece of code. In this particular instance, its fairly easy to envisage encapsulating these two register writes into their own function which architectures can override, which makes it fairly easy for me to put a Xen hook in somewhere on the arch/x86 side of the fence. But because Xen ends up doing the low-level cpu state save/restore, several other parts of the S3 suspend path can be skipped on the Linux side. I'm wondering if you have any thoughts about how that can be done, or if putting the Xen code in as-is is acceptable? (BTW, xen_pv_domain() expands to a constant 0 when Xen isn't enabled in the config, so the Xen-dependent bits will collapse down to nothing. But it is defined in asm/xen/hypervisor.h, which is only present on x86 and ia64 architectures; on the other hand, believe those are the only architectures using acpi.) Thanks, J