Re: [kvm-devel] patch for review: kvm: implement guest rebooting

2007-01-22 Thread Avi Kivity
Leonard Norrgard wrote:
 I've been looking at the guest reboot problem, which currently reboots
 the host on svm when a Linux guest issues # shutdown -r now.
   

Obviously a major issue, which we've neglected.  Thanks for addressing it.

 The patch below stops the rebooting of the host and handles the request
 to reboot the guest in an orderly fashion, so it's a big step forward,
 but it doesn't yet succeed in rebooting of the guest, instead the VM exits.

 The basic approach is to intercept the shutdown. The patch includes some
 possible alternatives to reboot, while searching for the best approach:

 1) Call kvm_qemu_destroy(); and immediately after kvm_qemu_create_context();
   

I don't like this approach.  It means memory needs to be deallocated and 
reallocated.  There's some likelyhood of the reallocation failing, so a 
reboot can turn into a dead VM.  Memory contents is not preserved, while 
a real reset does preserve memory.

More fundamentally, a real cpu can undergo a reset, and so should a 
virtual cpu.


 2) New ioctl to reset a vcpu
   

Definitely the way to go.  It'll possibly be needed for guest smp too.

 3) I also tried calling init_vmcb from shutdown_intercept, but that had
 no noticable effect
   

I think shutdown in this context is processor shutdown, not a vcpu 
reset.  IIRC a processor shutdown can be caused by invalid state or 
hardware errors.

We should intercept it, but it's not that important IMO.  (We should 
also find out what it means exactly)

 Method 1 seems promising, but currently results in kvm exiting due to
 do_interrupt: unexpect errors.

 Method 2 currently results in vmrun returning KVM_EXIT_TYPE_FAIL_ENTRY.

   

Looks like not everything was initialized correctly...

 - Both of methods 1 and 2 need an API version bump
 - This code is safe to run without having to fear a host reboot
 - SVM only, at the moment
 - svm_reset_vcpu() was initially just the one-line call to init_vmcb(),
 there's some leftover cruft now...
 - A printf has been added to cpu_reset(), if this prints, qemu has
 called all registered reset handlers.

 Please comment. The Patch is relative to kvm-11.


   
 

 Index: kernel/kvm_main.c
 ===
 --- kernel/kvm_main.c (revision 2211)
 +++ kernel/kvm_main.c (working copy)
 @@ -1764,6 +1764,23 @@
   return r;
  }
  
 +static int kvm_dev_ioctl_reset_vcpu(struct kvm *kvm, struct kvm_reset_vcpu 
 *slot)
 +{
 + struct kvm_vcpu *vcpu;
 +
 + if (!valid_vcpu(slot-vcpu))
 + return -EINVAL;
 + vcpu = vcpu_load(kvm, slot-vcpu);
 + if (!vcpu)
 + return -ENOENT;
 +
 + kvm_arch_ops-reset_vcpu(vcpu);
 +
 + vcpu_put(vcpu);
 +
 + return 0;
 +}
 +
  static long kvm_dev_ioctl(struct file *filp,
 unsigned int ioctl, unsigned long arg)
  {
 @@ -1780,6 +1797,13 @@
   goto out;
   break;
   }
 + case KVM_RESET_VCPU: {
 + struct kvm_reset_vcpu kvm_reset_vcpu;
   

Empty line here.

 + if (copy_from_user(kvm_reset_vcpu, (void *)arg, sizeof 
 kvm_reset_vcpu))
 + goto out;
 + r = kvm_dev_ioctl_reset_vcpu(kvm, kvm_reset_vcpu);
 + break;
 + }
   case KVM_RUN: {
   struct kvm_run kvm_run;
  
 Index: kernel/include/linux/kvm.h
 ===
 --- kernel/include/linux/kvm.h(revision 2211)
 +++ kernel/include/linux/kvm.h(working copy)
 @@ -11,7 +11,7 @@
  #include asm/types.h
  #include linux/ioctl.h
  
 -#define KVM_API_VERSION 2
 +#define KVM_API_VERSION 3
  
  /*
   * Architectural interrupt line count, and the size of the bitmap needed
 @@ -46,6 +46,7 @@
   KVM_EXIT_HLT  = 5,
   KVM_EXIT_MMIO = 6,
   KVM_EXIT_IRQ_WINDOW_OPEN  = 7,
 + KVM_EXIT_SHUTDOWN = 8,
  };
  
  /* for KVM_RUN */
 @@ -218,6 +219,12 @@
   };
  };
  
 +/* for KVM_RESET_VCPU */
 +struct kvm_reset_vcpu {
 + /* in */
 + __u32 vcpu;
 +};
 +
   

Why define the structure?  Since ioctl() passes one argument, it can be 
the vcpu number.


  #define KVMIO 0xAE
  
  #define KVM_GET_API_VERSION   _IO(KVMIO, 1)
 @@ -235,5 +242,6 @@
  #define KVM_GET_MSRS  _IOWR(KVMIO, 13, struct kvm_msrs)
  #define KVM_SET_MSRS  _IOWR(KVMIO, 14, struct kvm_msrs)
  #define KVM_GET_MSR_INDEX_LIST_IOWR(KVMIO, 15, struct kvm_msr_list)
 +#define KVM_RESET_VCPU_IO(KVMIO, 16)
   

If you're passing a structure, this needs to be _IOR(KVMIO, 16, struct 
kvm_reset_vcpu).


 @@ -582,6 +583,21 @@
   return r;
  }
  
 +static void svm_reset_vcpu(struct kvm_vcpu *vcpu)
 +{
 + printk(svm_reset_vcpu\n);
 + memset(vcpu-svm-vmcb, 0, PAGE_SIZE);
 + //vcpu-svm-vmcb_pa = page_to_pfn(page)  PAGE_SHIFT;
 + vcpu-svm-cr0 = 

Re: [kvm-devel] KVM Wiki

2007-01-22 Thread Arnd Bergmann
On Monday 22 January 2007 11:21, Klaas van Gend wrote:
 Maybe this can be done without much setup costs, by asking the
 maintainer of kernel.org.
 
 At least rt.wiki.kernel.org exists and is being used by the community.
 As KVM has rather similar usage patterns, I can easily imagine that we
 could setup kvm.wiki.kernel.org , not requiring Avi or anyone else at
 Qumranet to setup a wiki server.
 
Actually, you don't even need to ask anyone to do it, because the
wiki.kernel.org domain uses virtual hosting that automatically creates
new wikis when you use them. Just go to 
http://kvm.wiki.kernel.org/index.php/Main_Page and start editing.

Arnd 

-
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT  business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.phpp=sourceforgeCID=DEVDEV
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] KVM Wiki

2007-01-22 Thread Avi Kivity
Klaas van Gend wrote:
 Hi all,

 I noticed at least two promises by Avi on this list to start a wiki for KVM.

 Maybe this can be done without much setup costs, by asking the
 maintainer of kernel.org.

 At least rt.wiki.kernel.org exists and is being used by the community.
 As KVM has rather similar usage patterns, I can easily imagine that we
 could setup kvm.wiki.kernel.org , not requiring Avi or anyone else at
 Qumranet to setup a wiki server.
   

The IT here has promised to bring the wiki up no later than the end of 
this week, hopefully sooner.


-- 
error compiling committee.c: too many arguments to function


-
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT  business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.phpp=sourceforgeCID=DEVDEV
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH 4/5] KVM: Fix asm constraints with CONFIG_FRAME_POINTER=n

2007-01-22 Thread Herbert Xu
Avi Kivity [EMAIL PROTECTED] wrote:
 A g constraint may place a local variable in an %rsp-relative memory 
 operand.
 but if your assembly changes %rsp, the operand points to the wrong location.
 
 An r constraint fixes that.
 
 Thanks to Ingo Molnar for neatly bisecting the problem.
 
 Signed-off-by: Avi Kivity [EMAIL PROTECTED]
 
 Index: linux-2.6/drivers/kvm/vmx.c
 ===
 --- linux-2.6.orig/drivers/kvm/vmx.c
 +++ linux-2.6/drivers/kvm/vmx.c
 @@ -1825,7 +1825,7 @@ again:
 #endif
setbe %0 \n\t
popf \n\t
 - : =g (fail)
 + : =r (fail)
  : r(vcpu-launched), d((unsigned long)HOST_RSP),
c(vcpu),
[rax]i(offsetof(struct kvm_vcpu, regs[VCPU_REGS_RAX])),

We need the following fix for 2.6.20.

[KVM] vmx: Fix register constraint in launch code

Both =r and =g breaks my build on i386:

$ make
  CC [M]  drivers/kvm/vmx.o
{standard input}: Assembler messages:
{standard input}:3318: Error: bad register name `%sil'
make[1]: *** [drivers/kvm/vmx.o] Error 1
make: *** [_module_drivers/kvm] Error 2

The reason is that setbe requires an 8-bit register but =r does not
constrain the target register to be one that has an 8-bit version on
i386.

According to

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=10153

the correct constraint is =q.

Signed-off-by: Herbert Xu [EMAIL PROTECTED]

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
index ce219e3..0aa2659 100644
--- a/drivers/kvm/vmx.c
+++ b/drivers/kvm/vmx.c
@@ -1824,7 +1824,7 @@ again:
 #endif
setbe %0 \n\t
popf \n\t
- : =g (fail)
+ : =q (fail)
  : r(vcpu-launched), d((unsigned long)HOST_RSP),
c(vcpu),
[rax]i(offsetof(struct kvm_vcpu, regs[VCPU_REGS_RAX])),

-
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT  business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.phpp=sourceforgeCID=DEVDEV
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel