Re: [kvm-devel] s390 kvm_virtio.c build error

2008-05-04 Thread Heiko Carstens
On Sat, May 03, 2008 at 08:47:17PM +0300, Adrian Bunk wrote:
> Commit c45a6816c19dee67b8f725e6646d428901a6dc24
> (virtio: explicit advertisement of driver features)
> and commit e976a2b997fc4ad70ccc53acfe62811c4aaec851
> (s390: KVM guest: virtio device support, and kvm hypercalls)
> don't like each other:
> 
> <--  snip  -->
> 
> ...
>   CC  drivers/s390/kvm/kvm_virtio.o
> /home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/s390/kvm/kvm_virtio.c:224: 
> error: unknown field 'feature' specified in initializer
> /home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/s390/kvm/kvm_virtio.c:224: 
> warning: initialization from incompatible pointer type
> make[3]: *** [drivers/s390/kvm/kvm_virtio.o] Error 1
> 
> <--  snip  -->

Hmm... this should help:

---
 drivers/s390/kvm/kvm_virtio.c |   40 +++-
 1 file changed, 23 insertions(+), 17 deletions(-)

Index: linux-2.6/drivers/s390/kvm/kvm_virtio.c
===
--- linux-2.6.orig/drivers/s390/kvm/kvm_virtio.c
+++ linux-2.6/drivers/s390/kvm/kvm_virtio.c
@@ -78,27 +78,32 @@ static unsigned desc_size(const struct k
+ desc->config_len;
 }
 
-/*
- * This tests (and acknowleges) a feature bit.
- */
-static bool kvm_feature(struct virtio_device *vdev, unsigned fbit)
+/* This gets the device's feature bits. */
+static u32 kvm_get_features(struct virtio_device *vdev)
 {
+   unsigned int i;
+   u32 features = 0;
struct kvm_device_desc *desc = to_kvmdev(vdev)->desc;
-   u8 *features;
+   u8 *in_features = kvm_vq_features(desc);
 
-   if (fbit / 8 > desc->feature_len)
-   return false;
+   for (i = 0; i < min(desc->feature_len * 8, 32); i++)
+   if (in_features[i / 8] & (1 << (i % 8)))
+   features |= (1 << i);
+   return features;
+}
 
-   features = kvm_vq_features(desc);
-   if (!(features[fbit / 8] & (1 << (fbit % 8
-   return false;
+static void kvm_set_features(struct virtio_device *vdev, u32 features)
+{
+   unsigned int i;
+   struct kvm_device_desc *desc = to_kvmdev(vdev)->desc;
+   /* Second half of bitmap is features we accept. */
+   u8 *out_features = kvm_vq_features(desc) + desc->feature_len;
 
-   /*
-* 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;
+   memset(out_features, 0, desc->feature_len);
+   for (i = 0; i < min(desc->feature_len * 8, 32); i++) {
+   if (features & (1 << i))
+   out_features[i / 8] |= (1 << (i % 8));
+   }
 }
 
 /*
@@ -221,7 +226,8 @@ static void kvm_del_vq(struct virtqueue 
  * The config ops structure as defined by virtio config
  */
 static struct virtio_config_ops kvm_vq_configspace_ops = {
-   .feature = kvm_feature,
+   .get_features = kvm_get_features,
+   .set_features = kvm_set_features,
.get = kvm_get,
.set = kvm_set,
.get_status = kvm_get_status,

-
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [RFC/PATCH 01/15] preparation: provide hook to enable pgstes in user pagetable

2008-03-22 Thread Heiko Carstens
> What you've done with dup_mm() is probably the brute-force way that I
> would have done it had I just been trying to make a proof of concept or
> something.  I'm worried that there are a bunch of corner cases that
> haven't been considered.
> 
> What if someone else is poking around with ptrace or something similar
> and they bump the mm_users:
> 
> +   if (tsk->mm->context.pgstes)
> +   return 0;
> +   if (!tsk->mm || atomic_read(&tsk->mm->mm_users) > 1 ||
> +   tsk->mm != tsk->active_mm || tsk->mm->ioctx_list)
> +   return -EINVAL;
> >HERE
> +   tsk->mm->context.pgstes = 1;/* dirty little tricks .. */
> +   mm = dup_mm(tsk);
> 
> It'll race, possibly fault in some other pages, and those faults will be
> lost during the dup_mm().  I think you need to be able to lock out all
> of the users of access_process_vm() before you go and do this.  You also
> need to make sure that anyone who has looked at task->mm doesn't go and
> get a reference to it and get confused later when it isn't the task->mm
> any more.
> 
> > Therefore, we need to reallocate the page table after fork() 
> > once we know that task is going to be a hypervisor. That's what this 
> > code does: reallocate a bigger page table to accomondate the extra 
> > information. The task needs to be single-threaded when calling for 
> > extended page tables.
> > 
> > Btw: at fork() time, we cannot tell whether or not the user's going to 
> > be a hypervisor. Therefore we cannot do this in fork.
> 
> Can you convert the page tables at a later time without doing a
> wholesale replacement of the mm?  It should be a bit easier to keep
> people off the pagetables than keep their grubby mitts off the mm
> itself.

Yes, as far as I can see you're right. And whatever we do in arch code,
after all it's just a work around to avoid a new clone flag.
If something like clone() with CLONE_KVM would be useful for more
architectures than just s390 then maybe we should try to get a flag.

Oh... there are just two unused clone flag bits left. Looks like the
namespace changes ate up a lot of them lately.

Well, we could still play dirty tricks like setting a bit in current
via whatever mechanism which indicates child-wants-extended-page-tables
and then just fork and be happy.

-
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 14/15] guest: detect when running on kvm

2008-03-22 Thread Heiko Carstens
On Fri, Mar 21, 2008 at 03:33:29PM +0100, Carsten Otte wrote:
> Am Freitag, den 21.03.2008, 15:06 +0100 schrieb Heiko Carstens:
> > Just introduce something like MACHINE_FLAG_KVM. The rest can be converted
> > later. Unless you're bored and feel like fiddling around with assembly code 
> > :)
> I've done that patch this morning already, see below. I agree with HCH
> that we should do that, but after the kvm merge. I don't want kvm-s390
> conflict with Martin's patches. This is just a beautification, and can
> safely wait a release cycle.

That's nice for a start. But you didn't convert the assembly files to use
the new defines. So there is still no connection between setting a bit
in asm code and the new defines.
That's the reason why I said something about fiddling around with asm code.

-
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 14/15] guest: detect when running on kvm

2008-03-21 Thread Heiko Carstens
On Fri, Mar 21, 2008 at 12:12:04PM +0100, Carsten Otte wrote:
> [EMAIL PROTECTED] wrote:
>> Since when do we have symbolic names for the bits?
>> It was always on my todo list to do a cleanup and replace the numbers
>> we use everywhere with names. Especially since we have clashes from time
>> to time... but that didn't hurt enough yet, obviously.
>> But now that you volunteered to take care of this... :)
> Right. We only have defines for (machine_flags & bit). Looks to me like 
> the bits really should have a name on them. I've created a patch that 
> does this, but I want to talk it over with Martin before sending that one 
> out.

Just introduce something like MACHINE_FLAG_KVM. The rest can be converted
later. Unless you're bored and feel like fiddling around with assembly code :)

-
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 14/15] guest: detect when running on kvm

2008-03-20 Thread Heiko Carstens
On Thu, Mar 20, 2008 at 09:59:32PM +0100, Carsten Otte wrote:
> Christoph Hellwig wrote:
> > On Thu, Mar 20, 2008 at 09:37:19PM +0100, Carsten Otte wrote:
> >> Christoph Hellwig wrote:
> >>> On Thu, Mar 20, 2008 at 05:25:26PM +0100, Carsten Otte wrote:
>  @@ -143,6 +143,10 @@ static noinline __init void detect_machi
>   /* Running on a P/390 ? */
>   if (cpuinfo->cpu_id.machine == 0x7490)
>   machine_flags |= 4;
>  +
>  +/* Running under KVM ? */
>  +if (cpuinfo->cpu_id.version == 0xfe)
>  +machine_flags |= 64;
> >>> Shouldn't these have symbolic names?
> >> You mean symbolics for machine_flags? Or symbolics for cpu ids?
> > 
> > Either.
> [...]
> The machine flags do have symbolic names, defined in 
> include/asm-s390/setup.h. And yea, they should be used here. Will 
> change that.

Since when do we have symbolic names for the bits?
It was always on my todo list to do a cleanup and replace the numbers
we use everywhere with names. Especially since we have clashes from time
to time... but that didn't hurt enough yet, obviously.
But now that you volunteered to take care of this... :)

-
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] Proposed new directory layout for kvm and virtualization

2007-12-11 Thread Heiko Carstens
On Tue, Dec 11, 2007 at 11:47:39AM +0200, Avi Kivity wrote:
> KVM is due to receive support for multiple architectures (ppc, ia64, and 
> s390, in addition to the existing x86), hopefully in time for the 2.6.25 
> merge window.  It is awkward to place the new arch support in drivers/kvm/, 
> so I'd like to propose the following new layout:
> ...
>  arch/*/kvm/   arch dependent kvm code

Maybe arch/*/virt/ ? No need to add an own directory for each hypervisor.

-
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH 0/2][KVM] guest time accounting

2007-08-13 Thread Heiko Carstens
>  The only thing remotely relevant in the list config is that 'Filter out 
>  duplicate messages to list members (if possible)' is set as a default for 
>  new members.  Maybe this means that if a cc is also part of the list, that 
>  cc is stripped (which seems a wierd implementation; I'd have expected that 
>  cc be kept and just one copy sent out).
> 
>  Anybody have a clue?

I got also removed when I was cc'ed. IIRC that started when I subscribed to
the list. So it looks like people who are subscribed to the list get
removed from the cc list.
That's very annoying btw. ;)

-
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH 4/7] SMP: Implement on_one_cpu()

2007-05-24 Thread Heiko Carstens
On Thu, May 24, 2007 at 03:10:12PM +0300, Avi Kivity wrote:
> This defines on_one_cpu() which is similar to smp_call_function_single()
> except that it works if cpu happens to be the current cpu.  Can also be
> seen as a complement to on_each_cpu() (which also doesn't treat the
> current cpu specially).
> 
> Signed-off-by: Avi Kivity <[EMAIL PROTECTED]>
> ---
>  include/linux/smp.h |   15 +++
>  kernel/softirq.c|   24 
>  2 files changed, 39 insertions(+), 0 deletions(-)
> 
> +/*
> + * Call a function on one processor
> + */
> +int on_one_cpu(int cpu, void (*func)(void *info), void *info,
> +int retry, int wait);
> 

Would you mind renaming that one to simply 'on_cpu'? It's even shorter and
clearly everybody will know what its purpose is. Also I doubt we will ever
have something like 'on_two_cpus'.

-
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH/PFC 0/2] s390 host support

2007-04-29 Thread Heiko Carstens
> Nested page tables/extended page tables also provide this facility, with 
> some caveats:
> 
> - on 32-bit hosts (or 64-bit hosts with 32-bit userspace), host 
> userspace virtual address space is not enough to contain the guest 
> physical address space.
> - there is no way to protect the host userspace from the guest

Sorry, but are you saying that it is currently possible to access
(read and/or write) host userspace address space from the guest?

-
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH/PFC 0/2] s390 host support

2007-04-29 Thread Heiko Carstens
> > We intend to move to a common arch-independent kernel interface and
> > userspace with kvm.
> The address space and vcpu management are rather different from kvm's,
> however your approach is better and we'll want to move kvm in your
> direction rather than the other way round (specifically the tight vcpu
> <-> task coupling; mmu is more diffcult).

How do we continue from here? Adding new architectures to the ioctl based
approach or change kvm to a syscall interface? Also IMHO it would be better
to move the code away from drivers and to kernel/ or virt/ with arch
dependent backends.

-
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH/PFC 0/2] s390 host support

2007-04-29 Thread Heiko Carstens
> To investigate PowerPC support I did some work to refactor the KVM code
> into x86 vs shared bits. It was based on the in-kernel KVM code, so I need
> to rebase that work on kvm.git. I guess you guys have the same
> problem; have you done any work in that area?

We haven't done anything yet to make kvm non-x86 friendly. If you have
patches please send them.

-
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH/RFC 2/2] s390 virtualization interface.

2007-04-29 Thread Heiko Carstens
On Fri, Apr 27, 2007 at 06:53:40PM +0200, Arnd Bergmann wrote:
> On Friday 27 April 2007, Carsten Otte wrote:
> > Add interface which allows a process to start a virtual machine.
> 
> Looks pretty good to me already.
> 
> It seems a lot closer to lguest than to kvm at the moment concerning
> the kernel interfaces (guest real address == host process, state
> is attached to the thread_info, ...).

Guess I should take a look at lguest then :)

> > +static DEFINE_MUTEX(s390host_init_mutex);
> > +
> > +static void s390host_get_data(struct s390host_data *data)
> > +{
> > +   atomic_inc(&data->count);
> > +}
> > +
> > +void s390host_put_data(struct s390host_data *data)
> > +{
> > +   int cpu;
> > +
> > +   if (atomic_dec_return(&data->count))
> > +   return;
> 
> These should probably use a struct kref instead of an atomic_t
> to count the references.

That makes sense.

> 
> > +static unsigned long
> > +s390host_create_io_area(unsigned long addr, unsigned long flags,
> > +   unsigned long io_addr, struct s390host_data *data)
> > +{
[...]
> > +   down_write(&mm->mmap_sem);
> > +   ret = insert_vm_struct(mm, vma);
> > +   if (ret) {
> > +   kmem_cache_free(vm_area_cachep, vma);
> > +   goto out;
> > +   }
> > +   s390host_get_data(data);
> > +   mm->total_vm += 2;
> > +   vm_insert_page(vma, addr, virt_to_page(io_addr));
> > +
> > +   ret = split_vma(mm, vma, addr + PAGE_SIZE, 0);
> > +   if (ret)
> > +   goto out;
> 
> This open-coded mmap looks a bit scary. I can't see anything wrong in it,
> but it may conflict with a user application that tries to do other strange
> things with memory maps.

Well, there is at least already one bug in it. If the call to split_vma fails
there is no cleanup of the vm_insert_page that has already happened.

> Maybe you can either make this a real call to mmap(), or have it automatically
> mapped using the vdso mechanism.

So, yes we need to find something better, since this is the code that I
don't like at all.

> > +   __u16 cpu;
> 
> __u16 is a type for user space interfaces. In the kernel, use u16.

Yes.

> > +   if (current_thread_info()->sie_cpu != -1)
> > +   return -EINVAL;
> 
> -EBUSY?

Yes.

> > +   if (copy_from_user(&cpu, &sie_template->icpua, sizeof(u16)))
> > +   return -EFAULT;
> 
> get_user()?

Yes.

> > +   sca_block = data->sca_block;
> > +   if (sca_block->mcn & (1UL << (S390HOST_MAX_CPUS - 1 - cpu))) {
> 
> __test_bit()?

Yes, but I think we should probably add some function to access the mcn
bits. Each access looks a bit complicated.
 
> > +   ret = -EINVAL;
> 
> -EBUSY?

Yes.

> > +int sys_s390host_remove_cpu(void)
> > +{
> > +   struct sca_block *sca_block;
> > +   int cpu;
> > +
> > +   cpu = current_thread_info()->sie_cpu;
> > +   if (cpu == -1)
> > +   return -EINVAL;
> > +
> > +   mutex_lock(&s390host_init_mutex);
> > +   sca_block = current_thread_info()->s390host_data->sca_block;
> > +   sca_block->mcn &= ~(1UL << (S390HOST_MAX_CPUS - 1 - cpu));
> > +   current_thread_info()->sie_cpu = -1;
> > +   mutex_unlock(&s390host_init_mutex);
> > +   return 0;
> > +}
> 
> Shouldn't this free the resources that were allocated by add_cpu?

Yes, it was supposed to do that. But since it was only a prototype I was
too lazy to code it up and so I decided to free things on exit.

> If not, this syscall seems pretty useless and I'd simply not
> do it at all -- just wait until exit() to clean up.

This reveals that we are not clearing the mcn bit on exit without
pior call to this syscall. BUG().

> > +   if (signal_pending(current)) {
> > +   ret = -EINTR;
> > +   goto out;
> > +   }
> 
> Can't you handle interrupted system calls? I think it would be nicer
> to return -ERESTARTSYS here so you don't have to go back to your
> user space after a signal.

That should work and it is better.

> > +struct sie_kernel {
> > +   struct sie_blocksie_block;
> > +   s390_fp_regshost_fpregs;
> > +   int host_acrs[NUM_ACRS];
> > +} __attribute__((packed,aligned(4096)));
> 
> Why packed?

Not needed.

> > +#define SIE_UPDATE_PSW (1UL << 0)
> > +#define SIE_FLUSH_TLB  (1UL << 1)
> > +#define SIE_ISKE   (1UL << 2)
> > +#define SIE_SSKE   (1UL << 3)
> > +#define SIE_BLOCK_UPDATE   (1UL << 4)
> > +#define SIE_VSMXM_LOCAL_UPDATE (1UL << 5)
> > +#define SIE_VSMXM_DIST_UPDATE   (1UL << 6)
> 
> I find it much more readable to define these like
> 
> #define SIE_UPDATE_PSW1
> #define SIE_FLUSH_TLB 2
> #define SIE_ISKE  4
> 
> especially when trying to interpret values in memory.

Hmm... don't know. Both make sense, but I have no preference on this.
 
> > +struct sie_user {
> > +   struct sie_blocksie_block;
> > +   psw_t   psw;
> > +   unsigned long   gprs[NUM_GPRS];
> > +   s390_fp_regsguest_fpregs;
> > +   int guest_acrs[NUM_ACRS];
> > 

Re: [kvm-devel] memory hotplug for guests?

2007-04-05 Thread Heiko Carstens
> >For example, lets says you are running several guests, and would like
> to
> >start yet another one for a while - but have no free memory left.
> >
> 
> We have another solution for it that will soon be pushed into the
> kernel:
> It is the balloon driver solution.
> Each guest runs a balloon driver, when the host needs to free up memory
> a daemon with certain policy asks some of the guests to inflate their
> balloon,
> KVM frees their ballooned pages and the host free memory increases.
> When the memory pressure relives, the balloons get deflate command.

You probably want to have a look at arch/s390/mm/cmm.c which is
exactly doing what you want. Of course the message interface to the
hypervisor is different to what you want to do.

-
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.php&p=sourceforge&CID=DEVDEV
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH 0/15] KVM userspace interface updates

2007-03-19 Thread Heiko Carstens
On Mon, Mar 19, 2007 at 06:02:57PM +0200, Avi Kivity wrote:
> Heiko Carstens wrote:
> I agree with all of the above, and in addition, integration to the
> scheduler will allow us to reduce vcpu migration rate, and maybe do
> things like gang scheduling.
> 
> But that doesn't mean it can be done now: we really need to see how it
> works out with smp and with an additional arch, and then we can stabilize
> it.  Meanwhile I'd like a stable ABI so distros can start shipping
> kvm without worrying about upgrade headaches.
> 
> So the plan is:
> - get the /dev/kvm ABI into 2.6.22
> - implement smp
> - add another arch
> - move to a syscall based interface in parallel; userspace should work with 
> both
> - deprecate the old ABI and external modules.

I wasn't asking to change the ABI right now, just wanted to make sure that
it is not the 'final' interface. So I agree with your plan.

-
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.php&p=sourceforge&CID=DEVDEV
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH 0/15] KVM userspace interface updates

2007-03-19 Thread Heiko Carstens
On Sun, Mar 18, 2007 at 12:42:00PM +0200, Avi Kivity wrote:
> Heiko Carstens wrote:
> >In addition, if we would port kvm to s390, then we would need to
> >make sure that each virtual cpu only gets executed from the thread
> >that created it. That is simply because the upper half of our page
> >tables contain information about the guest page states. This is yet
> >another thing that would be strange to do via an ioctl based interface.
> 
> Right.  I agree it's more natural to associate a vcpu with a task
> instead of a vcpu being an independent entry.  We'd still need a
> handle for it, and in Linux that's an fd (pid doesn't cut it as it's
> racy, and probably slower too as it has to go through a global structure).

If you go for: only one VM per thread group and only one vcpu per thread
you don't need any identifier.
All relevant information or a pointer to it would be saved in the
thread_info structure.
That would give you two system calls to add/remove cpus which implicitely
create a VM if none is present. This add_vcpu syscall would also map
a memory range to user space which would be used to communicate between
user/kernel space to avoid frequent copy_to/from_user just like your
latest patches for KVM_RUN do.

We implemented a prototype on s390 based on a system call interface
and which does have full smp support.

This is a simplified version of how a add_cpu system call would look like.
Please note that I left out all error checkings etc. E.g. checking if the
vcpu already exists in the VM.

asmlinkage long sys_kvm_add_cpu(int vcpu, unsigned long addr)
{
struct kvm *kvm;

if (current_thread_info()->vcpu != -1)
return -EINVAL;

mutex_lock(&kvm_mutex);

write_lock_bh(&tasklist_lock);
/*
 * Check all thread_infos in thread group if a VM context
 * was already created.
 */
kvm = search_threads_for_kvm();
write_unlock_bh(&tasklist_lock);

if (!kvm) {
kvm = create_kvm_context();
current_thread_info()->kvm = kvm;
}

arch_add_cpu(vcpu);
current_thread_info()->vcpu = vcpu;

/*
 * Map vcpu data to userspace at addr.
 */
arch_create_kvm_area(addr);

mutex_unlock(&kvm_mutex);

return 0;
}

asmlinkage long sys_kvm_remove_cpu(void)
{
int vcpu;

vcpu = current_thread_info()->vcpu;
if (cpu == -1)
return -EINVAL;

mutex_lock(&kvm_mutex);
arch_remove_cpu(vcpu);
current_thread_info()->vcpu = -1;
mutex_unlock(&kvm_mutex);
return 0;
}

The interesting part with this is that you don't need any locking
for a kvm_run system call, simply because only the thread itself can
create/remove the vcpu:

asmlinkage long sys_kvm_run(void)
{
int vcpu;

vcpu = current_thread_info()->vcpu;
if (vcpu == -1)
return -EINVAL;
return arch_kvm_run();
}

Of course all this is rather simplified, but should give a good idea
why I think that a syscall based interface should be the way to go.

-
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.php&p=sourceforge&CID=DEVDEV
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH 0/15] KVM userspace interface updates

2007-03-18 Thread Heiko Carstens
On Sun, Mar 18, 2007 at 07:20:57AM +0200, Avi Kivity wrote:
> Heiko Carstens wrote:
> >On Sun, Mar 11, 2007 at 03:53:12PM +0200, Avi Kivity wrote:
> >  
> >>This patchset updates the kvm userspace interface to what I hope will
> >>be the long-term stable interface.  Provisions are included for extending
> >>the interface later.  The patches address performance and cleanliness
> >>concerns.
> > [...]
> >But the general question is: do you still plan to switch to a syscall
> >interface?
> 
> I don't have any present plans for that.  Maybe when the interface starts
> to evolve at a slower pace, or if it is shown to be significantly faster.
> 
> Not that interface stabilization here doesn't mean a freeze; it means that
> backwards compatibility starts when this gets merged.

If the interface is considered to be stable you can get rid of the
KVM_GET_API_VERSION ioctl, since the version can't change anymore, right?

-
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.php&p=sourceforge&CID=DEVDEV
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH 08/15] KVM: Add method to check for backwards-compatible API extensions

2007-03-16 Thread Heiko Carstens
> diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
> index 747966e..376538c 100644
> --- a/drivers/kvm/kvm_main.c
> +++ b/drivers/kvm/kvm_main.c
> @@ -2416,6 +2416,12 @@ static long kvm_dev_ioctl(struct file *filp,
>   r = 0;
>   break;
>   }
> + case KVM_CHECK_EXTENSION:
> + /*
> +  * No extensions defined at present.
> +  */
> + r = 0;
> + break;
>   default:

What exactly is this good for?

-
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.php&p=sourceforge&CID=DEVDEV
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH 0/15] KVM userspace interface updates

2007-03-16 Thread Heiko Carstens
On Fri, Mar 16, 2007 at 09:03:08AM -0500, Anthony Liguori wrote:
> Heiko Carstens wrote:
> >On Sun, Mar 11, 2007 at 03:53:12PM +0200, Avi Kivity wrote:
> >  
> >>This patchset updates the kvm userspace interface to what I hope will
> >>be the long-term stable interface.  Provisions are included for extending
> >>the interface later.  The patches address performance and cleanliness
> >>concerns.
> >>
> >
> >Searching the mailing list I figured that as soons as the interface seems
> >to be stable, kvm should/would switch to a system call based interface.
> >I assume the userspace interface might still change a lot, especially if
> >kvm is ported to new architectures.
> >But the general question is: do you still plan to switch to a syscall
> >interface?
> >  
> 
> What benefit would a syscall interface have?

First of all: it's faster and doesn't burn a bunch of additional cpu
cycles like sys_ioctl and the large switch statements do.

Another thing is that this patch set already introduces a way to pass a
sigset. Passing a sigset to a device node is sort of strange.

In addition, if we would port kvm to s390, then we would need to
make sure that each virtual cpu only gets executed from the thread
that created it. That is simply because the upper half of our page
tables contain information about the guest page states. This is yet
another thing that would be strange to do via an ioctl based interface.

Of course everthing can be done via an iotcl interface too, but IMHO
that's just the wrong interface.

-
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.php&p=sourceforge&CID=DEVDEV
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH 0/15] KVM userspace interface updates

2007-03-16 Thread Heiko Carstens
On Sun, Mar 11, 2007 at 03:53:12PM +0200, Avi Kivity wrote:
> This patchset updates the kvm userspace interface to what I hope will
> be the long-term stable interface.  Provisions are included for extending
> the interface later.  The patches address performance and cleanliness
> concerns.

Searching the mailing list I figured that as soons as the interface seems
to be stable, kvm should/would switch to a system call based interface.
I assume the userspace interface might still change a lot, especially if
kvm is ported to new architectures.
But the general question is: do you still plan to switch to a syscall
interface?

-
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.php&p=sourceforge&CID=DEVDEV
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel