Xen devel - Aug 2006 - [PATCH][LINUX] use machine_emergency_restart() from the generic header.

This patch uses the standard generic header for the definition of
machine_emergency_restart() and reverses the call path for XenoLinux.

Signed-off-by: Jimi Xenidis <jimix@watson.ibm.com>

---

diff -r 7680b29c03a6 drivers/xen/core/reboot.c
--- a/drivers/xen/core/reboot.c	Sat Aug 05 11:33:24 2006 -0400
+++ b/drivers/xen/core/reboot.c	Mon Aug 07 09:50:33 2006 -0400
@@ -9,6 +9,7 @@
 #include <linux/stringify.h>
 #include <asm/irq.h>
 #include <asm/mmu_context.h>
+#include <asm/emergency-restart.h>
 #include <xen/evtchn.h>
 #include <asm/hypervisor.h>
 #include <xen/interface/dom0_ops.h>
@@ -39,16 +40,11 @@ extern void ctrl_alt_del(void);
  */
 #define SHUTDOWN_HALT      4
 
-void machine_emergency_restart(void)
+void machine_restart(char * __unused)
 {
 	/* We really want to get pending console data out before we die. */
 	xencons_force_flush();
 	HYPERVISOR_shutdown(SHUTDOWN_reboot);
-}
-
-void machine_restart(char * __unused)
-{
-	machine_emergency_restart();
 }
 
 void machine_halt(void)

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

On 7/8/06 6:20 pm, "Jimi Xenidis" <jimix@watson.ibm.com> wrote:
> 
> This patch uses the standard generic header for the definition of
> machine_emergency_restart() and reverses the call path for XenoLinux.
It doesn''t reverse the call path, it removes
machine_emergency_restart()
entirely from drivers/xen/core/reboot.c. Hence the kernel no longer builds.

What is wrong with the code as it is? It follows the call order of the
arch-specific reboot.c (e.g., arch/i386/kernel/reboot.c).

 -- Keir



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

First, I apologize for not seeing the link error.

On Aug 8, 2006, at 4:46 AM, Keir Fraser wrote:
>
> On 7/8/06 6:20 pm, "Jimi Xenidis" <jimix@watson.ibm.com>
wrote:
>
>>
>> This patch uses the standard generic header for the definition of
>> machine_emergency_restart() and reverses the call path for XenoLinux.
>
> It doesn''t reverse the call path, it removes  
> machine_emergency_restart()
> entirely from drivers/xen/core/reboot.c. Hence the kernel no longer  
> builds.
File:
   include/asm-generic/emergency-restart.h
Defines:
   static inline void machine_emergency_restart(void)
   {
	machine_restart(NULL);
   }

But asm-i386 has a def that is not the same.
If fact all arch''s accept for i386 and x86_64 define it like:

   #ifndef _ASM_EMERGENCY_RESTART_H
   #define _ASM_EMERGENCY_RESTART_H

   #include <asm-generic/emergency-restart.h>

   #endif /* _ASM_EMERGENCY_RESTART_H */
>
> What is wrong with the code as it is? It follows the call order of the
> arch-specific reboot.c (e.g., arch/i386/kernel/reboot.c).
I have a linker problem building this file in POWERPC as you may  
suspect.
The ./driver/xen/ file is more like the other archs than x86, and  
should be more portable.

Not sure what header magik would be needed here, if you think its  
worthwhile I''ll pursue it.

-JX







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

On 8/8/06 12:49 pm, "Jimi Xenidis" <jimix@watson.ibm.com> wrote:
> I have a linker problem building this file in POWERPC as you may
> suspect.
> The ./driver/xen/ file is more like the other archs than x86, and
> should be more portable.
> 
> Not sure what header magik would be needed here, if you think its
> worthwhile I''ll pursue it.
You could switch the call order in drivers/xen/core/reboot.c so that
machine_emergency_restart calls machine_restart, and then conditionally
compile the former only for x86. Not super nice but it''s at least
straightforward. I''d take a patch that did that.

 -- Keir



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

On Aug 8, 2006, at 8:26 AM, Keir Fraser wrote:
>
> On 8/8/06 12:49 pm, "Jimi Xenidis" <jimix@watson.ibm.com>
wrote:
>
>> I have a linker problem building this file in POWERPC as you may
>> suspect.
>> The ./driver/xen/ file is more like the other archs than x86, and
>> should be more portable.
>>
>> Not sure what header magik would be needed here, if you think its
>> worthwhile I''ll pursue it.
>
> You could switch the call order in drivers/xen/core/reboot.c so that
> machine_emergency_restart calls machine_restart, and then  
> conditionally
> compile the former only for x86. Not super nice but it''s at least
> straightforward. I''d take a patch that did that.
hmm, there are a lot of x86 isms in this file.. I think having all  
these ifdefs in ./drivers will be problematic to the Linux  
integration cause.
At some point I could see us trying to remove/abstract them anyway.

Perhaps we could prefix all the methods with "xen_" and have arch  
specific code _in_ arch specific directories call it accordingly?

I would also like to separate the suspend code from the reboot/halt  
code into different files since thats is cleaner to select that  
support by CONFIG_, thoughts?

-JX

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

On 8/8/06 1:46 pm, "Jimi Xenidis" <jimix@watson.ibm.com> wrote:
> hmm, there are a lot of x86 isms in this file.. I think having all
> these ifdefs in ./drivers will be problematic to the Linux
> integration cause.
> At some point I could see us trying to remove/abstract them anyway.
> 
> Perhaps we could prefix all the methods with "xen_" and have arch
> specific code _in_ arch specific directories call it accordingly?
> 
> I would also like to separate the suspend code from the reboot/halt
> code into different files since thats is cleaner to select that
> support by CONFIG_, thoughts?
All sound like good suggestions. It''ll be necessary for upstream
merging
anyway, as it''ll need to hook under arch-specific paravirt_ops.

I''d take patches to split into separate source files, certainly. And
the
xen_ prefixing probably too.

 -- Keir



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