[RFC][PATCH] allow dom0 to power off machine using ACPI on shutdown.

[RFC][PATCH] allow dom0 to power off machine using ACPI on shutdown.

[Harry Butterworth]

[-- Attachment #1: Type: text/plain, Size: 1329 bytes --]

This is an evil little patch which allows my dom0 to power off the
machine on shutdown successfully and doesn't break my domU shutdown
behaviour.

The #if bits are ugly and are present because pm_power_off is declared
above in the same #if context, maybe they are not necessary.  I'm not
sure about the solution for ia64.

This powers down the machine without bothering to tell the hypervisor.

I thought this kind of solution was better than trying to import a lot
more ACPI cruft into Xen.  Any thoughts?

Harry.

Signed-off-by: Harry Butterworth <butterwo@uk.ibm.com>

# HG changeset patch
# User harry@localhost.localdomain
# Node ID 5c85b5dd3f8eda21cbcf529941dc41c72a7c6e09
# Parent  67de34c062b5897d1e82995b171be081f976558e
Allow a domain to call native pm_power_off if present. Fixes dom0
failure to power off on shutdown.

diff -r 67de34c062b5 -r 5c85b5dd3f8e
linux-2.6-xen-sparse/drivers/xen/core/reboot.c
--- a/linux-2.6-xen-sparse/drivers/xen/core/reboot.c	Thu Apr  6 11:03:53
2006
+++ b/linux-2.6-xen-sparse/drivers/xen/core/reboot.c	Thu Apr  6 18:25:23
2006
@@ -59,6 +59,10 @@
 {
 	/* We really want to get pending console data out before we die. */
 	xencons_force_flush();
+#if defined(__i386__) || defined(__x86_64__)
+	if (pm_power_off)
+		pm_power_off();
+#endif
 	HYPERVISOR_shutdown(SHUTDOWN_poweroff);
 }
 


[-- Attachment #2: poweroff.patch --]
[-- Type: text/x-patch, Size: 756 bytes --]

# HG changeset patch
# User harry@localhost.localdomain
# Node ID 5c85b5dd3f8eda21cbcf529941dc41c72a7c6e09
# Parent  67de34c062b5897d1e82995b171be081f976558e
Allow a domain to call native pm_power_off if present. Fixes dom0 failure to power off on shutdown.

diff -r 67de34c062b5 -r 5c85b5dd3f8e linux-2.6-xen-sparse/drivers/xen/core/reboot.c
--- a/linux-2.6-xen-sparse/drivers/xen/core/reboot.c	Thu Apr  6 11:03:53 2006
+++ b/linux-2.6-xen-sparse/drivers/xen/core/reboot.c	Thu Apr  6 18:25:23 2006
@@ -59,6 +59,10 @@
 {
 	/* We really want to get pending console data out before we die. */
 	xencons_force_flush();
+#if defined(__i386__) || defined(__x86_64__)
+	if (pm_power_off)
+		pm_power_off();
+#endif
 	HYPERVISOR_shutdown(SHUTDOWN_poweroff);
 }
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [RFC][PATCH] allow dom0 to power off machine using ACPI on shutdown.

[Muli Ben-Yehuda]

On Thu, Apr 06, 2006 at 07:36:10PM +0100, Harry Butterworth wrote:

> This is an evil little patch which allows my dom0 to power off the
> machine on shutdown successfully and doesn't break my domU shutdown
> behaviour.
> 
> The #if bits are ugly and are present because pm_power_off is declared
> above in the same #if context, maybe they are not necessary.  I'm not
> sure about the solution for ia64.
> 
> This powers down the machine without bothering to tell the
> hypervisor.

I wonder why we go to the hypervisor for shutdown at the moment. I
don't see anything interesting being done for x86, and we could always
have dom0 notifying Xen that it's going to shut down, and then
shutting down whe the hypercall return.

> I thought this kind of solution was better than trying to import a lot
> more ACPI cruft into Xen.  Any thoughts?

I think you want kernel_power_off() here, which should do the right
thing for all architectures.

Cheers,
Muli
-- 
Muli Ben-Yehuda
http://www.mulix.org | http://mulix.livejournal.com/

Re: [RFC][PATCH] allow dom0 to power off machine using ACPI on shutdown.

[Harry Butterworth]

On Thu, 2006-04-06 at 21:46 +0300, Muli Ben-Yehuda wrote:
> On Thu, Apr 06, 2006 at 07:36:10PM +0100, Harry Butterworth wrote:
> 
> > This is an evil little patch which allows my dom0 to power off the
> > machine on shutdown successfully and doesn't break my domU shutdown
> > behaviour.
> > 
> > The #if bits are ugly and are present because pm_power_off is declared
> > above in the same #if context, maybe they are not necessary.  I'm not
> > sure about the solution for ia64.
> > 
> > This powers down the machine without bothering to tell the
> > hypervisor.
> 
> I wonder why we go to the hypervisor for shutdown at the moment.

The kernel is for both dom0 and domU.  I think in the domU case the
hypervisor call is the correct mechanism to power off the virtual
machine.

> I
> don't see anything interesting being done for x86, and we could always
> have dom0 notifying Xen that it's going to shut down, and then
> shutting down whe the hypercall return.

This might be better for dom0 but currently I don't think the hypervisor
returns from the call in the dom0 case.  Not sure about that though.

> 
> > I thought this kind of solution was better than trying to import a lot
> > more ACPI cruft into Xen.  Any thoughts?
> 
> I think you want kernel_power_off() here, which should do the right
> thing for all architectures.

No. kernel_power_off is currently already being called and doesn't work
because it calls machine_power_off which in current unstable calls the
hypervisor which doesn't power off the hardware.

My patch modifies machine_power_off to call the native pm power off if
present (i.e. if dom0) or the hypervisor if not (i.e. domU).

> 
> Cheers,
> Muli

Re: [RFC][PATCH] allow dom0 to power off machine using ACPI on shutdown.

[Muli Ben-Yehuda]

On Thu, Apr 06, 2006 at 07:58:50PM +0100, Harry Butterworth wrote:

> The kernel is for both dom0 and domU.  I think in the domU case the
> hypervisor call is the correct mechanism to power off the virtual
> machine.

I meant in the dom0 case, of course.

> > > I thought this kind of solution was better than trying to import a lot
> > > more ACPI cruft into Xen.  Any thoughts?
> > 
> > I think you want kernel_power_off() here, which should do the right
> > thing for all architectures.
> 
> No. kernel_power_off is currently already being called and doesn't work
> because it calls machine_power_off which in current unstable calls the
> hypervisor which doesn't power off the hardware.
> 
> My patch modifies machine_power_off to call the native pm power off if
> present (i.e. if dom0) or the hypervisor if not (i.e. domU).

I see... Xen provides its own machine_power_off() in
drivers/xen/core/reboot.c. If we go with dom0 shutting the machine
down, the right fix would be to get rid of it and / or make it call
the arch specific machine_power_off().

Cheers,
Muli
-- 
Muli Ben-Yehuda
http://www.mulix.org | http://mulix.livejournal.com/

Re: [RFC][PATCH] allow dom0 to power off machine using ACPI on shutdown.

[Harry Butterworth]

On Thu, 2006-04-06 at 22:06 +0300, Muli Ben-Yehuda wrote:
> On Thu, Apr 06, 2006 at 07:58:50PM +0100, Harry Butterworth wrote:
> 
> > The kernel is for both dom0 and domU.  I think in the domU case the
> > hypervisor call is the correct mechanism to power off the virtual
> > machine.
> 
> I meant in the dom0 case, of course.
> 
> > > > I thought this kind of solution was better than trying to import a lot
> > > > more ACPI cruft into Xen.  Any thoughts?
> > > 
> > > I think you want kernel_power_off() here, which should do the right
> > > thing for all architectures.
> > 
> > No. kernel_power_off is currently already being called and doesn't work
> > because it calls machine_power_off which in current unstable calls the
> > hypervisor which doesn't power off the hardware.
> > 
> > My patch modifies machine_power_off to call the native pm power off if
> > present (i.e. if dom0) or the hypervisor if not (i.e. domU).
> 
> I see... Xen provides its own machine_power_off() in
> drivers/xen/core/reboot.c. If we go with dom0 shutting the machine
> down, the right fix would be to get rid of it

...can't get rid of the hypervisor call entirely because we want the
domU case to call the hypervisor to power off...

>  and / or make it call
> the arch specific machine_power_off().

...and I think it xen/core/reboot.c replaced the arch specific
machine_power_off which is no longer there to be called.

It might be better to install the hypervisor call in pm_power_off and
let it get replaced by the acpi_power_off call if ACPI is detected.
That would seem to fit in better with the way the x86 arch handles
multiple power off strategies.

I haven't looked to see how the other architectures do power off yet.

Was hoping for some comment about whether to do power off in xen or in
domU and whether the hypervisor needed notification.

-- 
Harry Butterworth <harry@hebutterworth.freeserve.co.uk>

Re: [RFC][PATCH] allow dom0 to power off machine using ACPI on shutdown.

[Tristan Gingold]

Le Jeudi 06 Avril 2006 20:36, Harry Butterworth a écrit :
> This is an evil little patch which allows my dom0 to power off the
> machine on shutdown successfully and doesn't break my domU shutdown
> behaviour.
>
> The #if bits are ugly and are present because pm_power_off is declared
> above in the same #if context, maybe they are not necessary.  I'm not
> sure about the solution for ia64.
On ia64, we don't xen/core/reboot.c.
The hypervisor is able to power off the machine.  The reason is simple: there 
is a call for that in EFI.

Tristan.

Re: [RFC][PATCH] allow dom0 to power off machine using ACPI on shutdown.

[Keir Fraser]

On 6 Apr 2006, at 22:22, Harry Butterworth wrote:

> It might be better to install the hypervisor call in pm_power_off and
> let it get replaced by the acpi_power_off call if ACPI is detected.
> That would seem to fit in better with the way the x86 arch handles
> multiple power off strategies.
>
> I haven't looked to see how the other architectures do power off yet.
>
> Was hoping for some comment about whether to do power off in xen or in
> domU and whether the hypervisor needed notification.

I think your patch looked okay for now, actually. Alternatively our 
machine_restart() could be moved into arch-specific code files. Doing 
the power off in dom0 is fine -- as you say, Xen doesn't do anyhting 
special before shutdown.

  -- Keir

Re: [RFC][PATCH] allow dom0 to power off machine using ACPI on shutdown.

[Muli Ben-Yehuda]

On Fri, Apr 07, 2006 at 08:27:50AM +0100, Keir Fraser wrote:

> I think your patch looked okay for now, actually. Alternatively our 
> machine_restart() could be moved into arch-specific code files.

The patch might be ok as a short term stopgap measure, but if we
consider mainstream inclusion, it has to make use of the arch specific
machine_power_off, not have ifdefs in core code. There's just no way
such a thing will be accepted. I think it's better to get it right now
rather than have one more thing to go back and fix when we submit it.

Cheers,
Muli
-- 
Muli Ben-Yehuda
http://www.mulix.org | http://mulix.livejournal.com/