Re: [kvm-devel] [RFC/PATCH 15/15] guest: virtio device support, and kvm hypercalls

2008-03-22 Thread Carsten Otte
Rusty Russell wrote:
 On Friday 21 March 2008 19:15:47 Christian Borntraeger wrote:
 Am Freitag, 21. März 2008 schrieb Rusty Russell:
 Hmm, panic on device_register fail, but -ENOMEM on add_shared_memory
 fail? My theory was that since this is boot time, panic() is the right
 thing.
 Good spot, but I agree with Carsten. Drivers should not panic. I have
 module load/unload capability on my long term todo list, but I can change
 the panic now.
 
 Yep, that makes sense.  For lguest, we panic: it's always at boot time so if 
 it fails we should die early to make it easier to diagnose (and that makes 
 sure it happens before we lose our early console).
Diangnostic is easy here at any time during the boot process: we've 
got our store status ioctl that userspace calls after guest execution 
has ended. It causes all cpus to store their register content and such 
into the cpu's lowcore area. Then it writes out our memory to a dump 
image, which lkcdutils and/or crash can read.


-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [RFC/PATCH 15/15] guest: virtio device support, and kvm hypercalls

2008-03-21 Thread Carsten Otte
Rusty Russell wrote:
 +static int __init kvm_devices_init(void)
 +{
 +if (!MACHINE_IS_KVM)
 +return -ENODEV;
 +
 +if (device_register(kvm_root) != 0)
 +panic(Could not register kvm root);
 +
 +if (add_shared_memory((max_pfn)  PAGE_SHIFT, PAGE_SIZE)) {
 +device_unregister(kvm_root);
 +return -ENOMEM;
 +}
 
 Hmm, panic on device_register fail, but -ENOMEM on add_shared_memory fail?
 My theory was that since this is boot time, panic() is the right thing.
We can't tell whether or not this is an important device or not. Maybe 
the guest is running with ramdisk as rootfs and can have a happy life 
if we don't kill it here. Return the rc from device register seems to 
be the right thing to me, if it was an important device we'll see 
panic: cannot mount rootfs or something later.

-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [RFC/PATCH 15/15] guest: virtio device support, and kvm hypercalls

2008-03-21 Thread Christian Borntraeger
Am Freitag, 21. März 2008 schrieb Rusty Russell:
 I'd recommend a hypercall after set_status, as well as reset.  The
 reason lguest doesn't do this is that we don't do feature negotiation
 (assuming guest kernel matches host kernel).  In general, the host
 needs to know when the VIRTIO_CONFIG_S_DRIVER_OK is set so it can see
 what features the guest driver accepted.

Right. Will have a look.
 
 Overloading the notify hypercall is kind of a hack too, but it works so
 no real need to change that.
 
  + * The root device for the kvm virtio devices.
  + * This makes them appear as /sys/devices/kvm/0,1,2 
not /sys/devices/0,1,2.
  + */ 
  +static struct device kvm_root = {
  +   .parent = NULL,
  +   .bus_id = kvm_s390,
  +};
 
 You mean /sys/devices/kvm_s390/0,1,2?

Yes, thanks.
 
  +static int __init kvm_devices_init(void)
  +{
  +   if (!MACHINE_IS_KVM)
  +   return -ENODEV;
  +
  +   if (device_register(kvm_root) != 0)
  +   panic(Could not register kvm root);
  +
  +   if (add_shared_memory((max_pfn)  PAGE_SHIFT, PAGE_SIZE)) {
  +   device_unregister(kvm_root);
  +   return -ENOMEM;
  +   }
 
 Hmm, panic on device_register fail, but -ENOMEM on add_shared_memory fail?
 My theory was that since this is boot time, panic() is the right thing.

Good spot, but I agree with Carsten. Drivers should not panic. I have module 
load/unload capability on my long term todo list, but I can change the 
panic now.

Christian



-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [RFC/PATCH 15/15] guest: virtio device support, and kvm hypercalls

2008-03-21 Thread Avi Kivity
Carsten Otte wrote:
 Currently we dont have PCI on s390. Making virtio_pci usable for s390 seems
 more complicated that providing an own stub. This virtio stub is similar to
 the lguest one, the memory for the descriptors and the device detection is 
 made
 via additional mapped memory on top of the guest storage. We use an external
 interrupt with extint code 1237 for host-guest notification. 
   

So, sanity won in the end.

-- 
Any sufficiently difficult bug is indistinguishable from a feature.


-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [RFC/PATCH 15/15] guest: virtio device support, and kvm hypercalls

2008-03-21 Thread Rusty Russell
On Friday 21 March 2008 19:15:47 Christian Borntraeger wrote:
 Am Freitag, 21. März 2008 schrieb Rusty Russell:
  Hmm, panic on device_register fail, but -ENOMEM on add_shared_memory
  fail? My theory was that since this is boot time, panic() is the right
  thing.

 Good spot, but I agree with Carsten. Drivers should not panic. I have
 module load/unload capability on my long term todo list, but I can change
 the panic now.

Yep, that makes sense.  For lguest, we panic: it's always at boot time so if 
it fails we should die early to make it easier to diagnose (and that makes 
sure it happens before we lose our early console).

Cheers,
Rusty.

-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


[kvm-devel] [RFC/PATCH 15/15] guest: virtio device support, and kvm hypercalls

2008-03-20 Thread Carsten Otte
From: Christian Borntraeger [EMAIL PROTECTED]

This patch implements kvm guest kernel support for paravirtualized devices
and contains two parts:
o a basic virtio stub using virtio_ring and external interrupts and hypercalls
o full hypercall implementation in kvm_para.h

Currently we dont have PCI on s390. Making virtio_pci usable for s390 seems
more complicated that providing an own stub. This virtio stub is similar to
the lguest one, the memory for the descriptors and the device detection is made
via additional mapped memory on top of the guest storage. We use an external
interrupt with extint code 1237 for host-guest notification. 

The hypercall definition uses the diag instruction for issuing a hypercall. The
parameters are written in R2-R7, the hypercall number is written in R1. This is
similar to the system call ABI (svc) which can use R1 for the number and R2-R6 
for the parameters.


Signed-off-by: Christian Borntraeger [EMAIL PROTECTED]
Acked-by: Martin Schwidefsky [EMAIL PROTECTED]
Signed-off-by: Carsten Otte [EMAIL PROTECTED]
---
 drivers/s390/Makefile |2 
 drivers/s390/kvm/Makefile |9 +
 drivers/s390/kvm/kvm_virtio.c |  326 ++
 drivers/s390/kvm/kvm_virtio.h |   47 ++
 include/asm-s390/kvm_para.h   |  124 +++
 5 files changed, 505 insertions(+), 3 deletions(-)

Index: kvm/drivers/s390/Makefile
===
--- kvm.orig/drivers/s390/Makefile
+++ kvm/drivers/s390/Makefile
@@ -5,7 +5,7 @@
 CFLAGS_sysinfo.o += -Iinclude/math-emu -Iarch/s390/math-emu -w
 
 obj-y += s390mach.o sysinfo.o s390_rdev.o
-obj-y += cio/ block/ char/ crypto/ net/ scsi/
+obj-y += cio/ block/ char/ crypto/ net/ scsi/ kvm/
 
 drivers-y += drivers/s390/built-in.o
 
Index: kvm/drivers/s390/kvm/Makefile
===
--- /dev/null
+++ kvm/drivers/s390/kvm/Makefile
@@ -0,0 +1,9 @@
+# Makefile for kvm guest drivers on s390
+#
+# Copyright IBM Corp. 2008
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License (version 2 only)
+# as published by the Free Software Foundation.
+
+obj-$(CONFIG_VIRTIO) += kvm_virtio.o
Index: kvm/drivers/s390/kvm/kvm_virtio.c
===
--- /dev/null
+++ kvm/drivers/s390/kvm/kvm_virtio.c
@@ -0,0 +1,326 @@
+/*
+ * kvm_virtio.c - virtio for kvm on s390
+ *
+ * Copyright IBM Corp. 2008
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License (version 2 only)
+ * as published by the Free Software Foundation.
+ *
+ *Author(s): Christian Borntraeger [EMAIL PROTECTED]
+ */
+
+#include linux/init.h
+#include linux/bootmem.h
+#include linux/err.h
+#include linux/virtio.h
+#include linux/virtio_config.h
+#include linux/interrupt.h
+#include linux/virtio_ring.h
+#include asm/io.h
+#include asm/kvm_para.h
+#include asm/setup.h
+#include asm/s390_ext.h
+
+#include kvm_virtio.h
+
+/*
+ * The pointer to our (page) of device descriptions.
+ */
+static void *kvm_devices;
+
+/*
+ * Unique numbering for kvm devices.
+ */
+static unsigned int dev_index;
+
+struct kvm_device {
+   struct virtio_device vdev;
+   struct kvm_device_desc *desc;
+};
+
+#define to_kvmdev(vd) container_of(vd, struct kvm_device, vdev)
+
+/*
+ * memory layout:
+ * - kvm_device_descriptor
+ *struct kvm_device_desc
+ * - configuration
+ *struct kvm_vqconfig
+ * - feature bits
+ * - config space
+ */
+static struct kvm_vqconfig *kvm_vq_config(const struct kvm_device_desc *desc)
+{
+   return (struct kvm_vqconfig *)(desc + 1);
+}
+
+static u8 *kvm_vq_features(const struct kvm_device_desc *desc)
+{
+   return (u8 *)(kvm_vq_config(desc) + desc-num_vq);
+}
+
+static u8 *kvm_vq_configspace(const struct kvm_device_desc *desc)
+{
+   return kvm_vq_features(desc) + desc-feature_len * 2;
+}
+
+/*
+ * The total size of the config page used by this device (incl. desc)
+ */
+static unsigned desc_size(const struct kvm_device_desc *desc)
+{
+   return sizeof(*desc)
+   + desc-num_vq * sizeof(struct kvm_vqconfig)
+   + desc-feature_len * 2
+   + desc-config_len;
+}
+
+/*
+ * This tests (and acknowleges) a feature bit.
+ */
+static bool kvm_feature(struct virtio_device *vdev, unsigned fbit)
+{
+   struct kvm_device_desc *desc = to_kvmdev(vdev)-desc;
+   u8 *features;
+
+   if (fbit / 8  desc-feature_len)
+   return false;
+
+   features = kvm_vq_features(desc);
+   if (!(features[fbit / 8]  (1  (fbit % 8
+   return false;
+
+   /*
+* We set the matching bit in the other half of the bitmap to tell the
+* Host we want to use this feature.
+*/
+   features[desc-feature_len + fbit / 8] |= (1  (fbit % 8));
+   return true;
+}
+

Re: [kvm-devel] [RFC/PATCH 15/15] guest: virtio device support, and kvm hypercalls

2008-03-20 Thread Rusty Russell
On Friday 21 March 2008 03:25:28 Carsten Otte wrote:
 +static void kvm_set_status(struct virtio_device *vdev, u8 status)
 +{
 + BUG_ON(!status);
 + to_kvmdev(vdev)-desc-status = status;
 +}
 +
 +/*
 + * To reset the device, we (ab)use the NOTIFY hypercall, with the descriptor
 + * address of the device.  The Host will zero the status and all the 
 + * features. 
 + */
 +static void kvm_reset(struct virtio_device *vdev)
 +{
 + unsigned long offset = (void *)to_kvmdev(vdev)-desc - kvm_devices;
 +
 + kvm_hypercall1(1237, (max_pfnPAGE_SHIFT) + offset);
 +}

I'd recommend a hypercall after set_status, as well as reset.  The
reason lguest doesn't do this is that we don't do feature negotiation
(assuming guest kernel matches host kernel).  In general, the host
needs to know when the VIRTIO_CONFIG_S_DRIVER_OK is set so it can see
what features the guest driver accepted.

Overloading the notify hypercall is kind of a hack too, but it works so
no real need to change that.

 + * The root device for the kvm virtio devices.
 + * This makes them appear as /sys/devices/kvm/0,1,2 not /sys/devices/0,1,2.
 + */ 
 +static struct device kvm_root = {
 + .parent = NULL,
 + .bus_id = kvm_s390,
 +};

You mean /sys/devices/kvm_s390/0,1,2?

 +static int __init kvm_devices_init(void)
 +{
 + if (!MACHINE_IS_KVM)
 + return -ENODEV;
 +
 + if (device_register(kvm_root) != 0)
 + panic(Could not register kvm root);
 +
 + if (add_shared_memory((max_pfn)  PAGE_SHIFT, PAGE_SIZE)) {
 + device_unregister(kvm_root);
 + return -ENOMEM;
 + }

Hmm, panic on device_register fail, but -ENOMEM on add_shared_memory fail?
My theory was that since this is boot time, panic() is the right thing.

Cheers,
Rusty.

-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel