Re: [PATCH] KVM: PPC: Do not create debugfs if fail to create vcpu

2010-03-09 Thread Alexander Graf

On 09.03.2010, at 07:13, Wei Yongjun wrote:

 If fail to create the vcpu, we should not create the debugfs
 for it.
 
 Signed-off-by: Wei Yongjun yj...@cn.fujitsu.com

Good catch.

I guess a goto out kind of construct would be better, but for a single line of 
code this is enough. And whoever adds more lines can put the goto into place.

Acked-by: Alexander Graf ag...@suse.de


Alex--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 13/15] KVM: Add support for enabling capabilities per-vcpu

2010-03-09 Thread Avi Kivity

On 03/09/2010 03:01 PM, Alexander Graf wrote:

On 09.03.2010, at 13:56, Avi Kivity wrote:

   

On 03/08/2010 08:03 PM, Alexander Graf wrote:
 

Some times we don't want all capabilities to be available to all
our vcpus. One example for that is the OSI interface, implemented
in the next patch.

In order to have a generic mechanism in how to enable capabilities
individually, this patch introduces a new ioctl that can be used
for this purpose. That way features we don't want in all guests or
userspace configurations can just not be enabled and we're good.


diff --git a/Documentation/kvm/api.txt b/Documentation/kvm/api.txt
index d170cb4..6a19ab6 100644
--- a/Documentation/kvm/api.txt
+++ b/Documentation/kvm/api.txt
@@ -749,6 +749,21 @@ Writes debug registers into the vcpu.
  See KVM_GET_DEBUGREGS for the data structure. The flags field is unused
  yet and must be cleared on entry.

+4.34 KVM_ENABLE_CAP
+
+Capability: basic

   

Capability: basic means that the feature was present in 2.6.22.  Otherwise you 
need to specify the KVM_CAP_ that presents this feature.

 

+Architectures: all


   

But it's implemented for ppc only (other arches will get ENOTTY).
 

That was the whole idea behind it. if it fails it fails. Nothing we can do 
about it. If it succeeds - great.
   


If KVM_CAP_ENABLE_CAP is present, it means the KVM_ENABLE_CAP ioctl will 
not return ENOTTY (it may return EINVAL if wrong values are present).


ENOTTY means not implemented.  'Architectures: all' means implemented.


+Not all extensions are enabled by default. Using this ioctl the application
+can enable an extension, making it available to the guest.
+
+On systems that do not support this ioctl, it always fails. On systems that
+do support it, it only works for extensions that are supported for enablement.
+As of writing this the only enablement enabled extenion is KVM_CAP_PPC_OSI.

   

That needs to be documented.  It also needs to be discoverable separately - we 
can have a kernel with KVM_ENABLE_CAP but without KVM_CAP_PPC_OSI.

btw, KVM_CAP_PPC_OSI conflicts with the KVM_CAP_ namespace.  Please choose 
another namespace.
 

Well I figured it'd be slick to have capabilities get enabled or disabled. 
That's the whole idea behind making it generic. If I wanted a specific 
interface I'd go in and create an ioctl ENABLE_OSI_INTERFACE.
   


Ah, I see.  Well, that makes sense.  Please document it.


But this way the detection if a capability exists can be done using the 
existing CAP detection. It can then be enabled using ENABLE_CAP.
   


Okay, I agree.

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

--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 14/15] KVM: PPC: Add OSI hypercall interface

2010-03-09 Thread Alexander Graf

On 09.03.2010, at 14:11, Avi Kivity wrote:

 On 03/09/2010 03:04 PM, Alexander Graf wrote:
 
 +  /* KVM_EXIT_OSI */
 +  struct {
 +  __u64 gprs[32];
 +  } osi;
 +
 +MOL uses a special hypercall interface it calls 'OSI'. To enable it, we 
 catch
 +hypercalls and exit with this exit struct that contains all the guest 
 gprs.
 +
 +If exit_reason is KVM_EXIT_OSI, then the vcpu has triggered such a 
 hypercall.
 +Userspace can now handle the hypercall and when it's done modify the gprs 
 as
 +necessary. Upon guest entry all guest GPRs will then be replaced by the 
 values
 +in this struct.
 +
 
   
 That's migration unsafe.  There may not be next guest entry on this host.
 
 It's as unsafe as MMIO then.
 
   
 
 From api.txt:
 
 NOTE: For KVM_EXIT_IO and KVM_EXIT_MMIO, the corresponding operations
 are complete (and guest state is consistent) only after userspace has
 re-entered the kernel with KVM_RUN.  The kernel side will first finish
 incomplete operations and then check for pending signals.  Userspace
 can re-enter the guest with an unmasked signal pending to complete
 pending operations.
 

Alright - so I add KVM_EXIT_OSI there and be good? :)

 
 Is using KVM_[GS]ET_REGS problematic for some reason?
 
 It's two additional ioctls for no good reason. We know the interface, so we 
 can model towards it.
   
 
 But we need to be migration safe.  If the interface is not heavily used, 
 let's not add complications.

MOL uses OSI calls instead of MMIO. So yes, it is heavily used.


Alex--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 14/15] KVM: PPC: Add OSI hypercall interface

2010-03-09 Thread Avi Kivity

On 03/09/2010 03:04 PM, Alexander Graf wrote:



+   /* KVM_EXIT_OSI */
+   struct {
+   __u64 gprs[32];
+   } osi;
+
+MOL uses a special hypercall interface it calls 'OSI'. To enable it, we catch
+hypercalls and exit with this exit struct that contains all the guest gprs.
+
+If exit_reason is KVM_EXIT_OSI, then the vcpu has triggered such a hypercall.
+Userspace can now handle the hypercall and when it's done modify the gprs as
+necessary. Upon guest entry all guest GPRs will then be replaced by the values
+in this struct.
+

   

That's migration unsafe.  There may not be next guest entry on this host.
 

It's as unsafe as MMIO then.

   


From api.txt:


NOTE: For KVM_EXIT_IO and KVM_EXIT_MMIO, the corresponding operations
are complete (and guest state is consistent) only after userspace has
re-entered the kernel with KVM_RUN.  The kernel side will first finish
incomplete operations and then check for pending signals.  Userspace
can re-enter the guest with an unmasked signal pending to complete
pending operations.




Is using KVM_[GS]ET_REGS problematic for some reason?
 

It's two additional ioctls for no good reason. We know the interface, so we can 
model towards it.
   


But we need to be migration safe.  If the interface is not heavily used, 
let's not add complications.


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

--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 14/15] KVM: PPC: Add OSI hypercall interface

2010-03-09 Thread Alexander Graf

On 09.03.2010, at 14:19, Avi Kivity wrote:

 On 03/09/2010 03:12 PM, Alexander Graf wrote:
 On 09.03.2010, at 14:11, Avi Kivity wrote:
 
   
 On 03/09/2010 03:04 PM, Alexander Graf wrote:
 
   
 +/* KVM_EXIT_OSI */
 +struct {
 +__u64 gprs[32];
 +} osi;
 +
 +MOL uses a special hypercall interface it calls 'OSI'. To enable it, we 
 catch
 +hypercalls and exit with this exit struct that contains all the guest 
 gprs.
 +
 +If exit_reason is KVM_EXIT_OSI, then the vcpu has triggered such a 
 hypercall.
 +Userspace can now handle the hypercall and when it's done modify the 
 gprs as
 +necessary. Upon guest entry all guest GPRs will then be replaced by the 
 values
 +in this struct.
 +
 
 
   
 That's migration unsafe.  There may not be next guest entry on this host.
 
 
 It's as unsafe as MMIO then.
 
 
   
 From api.txt:
 
 
 NOTE: For KVM_EXIT_IO and KVM_EXIT_MMIO, the corresponding operations
 are complete (and guest state is consistent) only after userspace has
 re-entered the kernel with KVM_RUN.  The kernel side will first finish
 incomplete operations and then check for pending signals.  Userspace
 can re-enter the guest with an unmasked signal pending to complete
 pending operations.
   
 
 Alright - so I add KVM_EXIT_OSI there and be good? :)
   
 
 Sure, just verify that the note holds for that case too.

The handling of the hypercall write-back is in the same region as the mmio one. 
So whatever applies for MMIO entries applies for OSI entries too.

Alex--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 14/15] KVM: PPC: Add OSI hypercall interface

2010-03-09 Thread Avi Kivity

On 03/09/2010 03:12 PM, Alexander Graf wrote:

On 09.03.2010, at 14:11, Avi Kivity wrote:

   

On 03/09/2010 03:04 PM, Alexander Graf wrote:
 
   

+   /* KVM_EXIT_OSI */
+   struct {
+   __u64 gprs[32];
+   } osi;
+
+MOL uses a special hypercall interface it calls 'OSI'. To enable it, we catch
+hypercalls and exit with this exit struct that contains all the guest gprs.
+
+If exit_reason is KVM_EXIT_OSI, then the vcpu has triggered such a hypercall.
+Userspace can now handle the hypercall and when it's done modify the gprs as
+necessary. Upon guest entry all guest GPRs will then be replaced by the values
+in this struct.
+


   

That's migration unsafe.  There may not be next guest entry on this host.

 

It's as unsafe as MMIO then.


   

 From api.txt:

 

NOTE: For KVM_EXIT_IO and KVM_EXIT_MMIO, the corresponding operations
are complete (and guest state is consistent) only after userspace has
re-entered the kernel with KVM_RUN.  The kernel side will first finish
incomplete operations and then check for pending signals.  Userspace
can re-enter the guest with an unmasked signal pending to complete
pending operations.
   
 

Alright - so I add KVM_EXIT_OSI there and be good? :)
   


Sure, just verify that the note holds for that case too.


But we need to be migration safe.  If the interface is not heavily used, let's 
not add complications.
 

MOL uses OSI calls instead of MMIO. So yes, it is heavily used.

   


Ok.

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

--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 02/15] KVM: PPC: Allow userspace to unset the IRQ line

2010-03-09 Thread Avi Kivity

On 03/08/2010 08:03 PM, Alexander Graf wrote:

Userspace can tell us that it wants to trigger an interrupt. But
so far it can't tell us that it wants to stop triggering one.

So let's interpret the parameter to the ioctl that we have anyways
to tell us if we want to raise or lower the interrupt line.
   


I asked for a KVM_CAP_ for this.  What was the conclusion of that thread?

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

--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 13/15] KVM: Add support for enabling capabilities per-vcpu

2010-03-09 Thread Avi Kivity

On 03/08/2010 08:03 PM, Alexander Graf wrote:

Some times we don't want all capabilities to be available to all
our vcpus. One example for that is the OSI interface, implemented
in the next patch.

In order to have a generic mechanism in how to enable capabilities
individually, this patch introduces a new ioctl that can be used
for this purpose. That way features we don't want in all guests or
userspace configurations can just not be enabled and we're good.


diff --git a/Documentation/kvm/api.txt b/Documentation/kvm/api.txt
index d170cb4..6a19ab6 100644
--- a/Documentation/kvm/api.txt
+++ b/Documentation/kvm/api.txt
@@ -749,6 +749,21 @@ Writes debug registers into the vcpu.
  See KVM_GET_DEBUGREGS for the data structure. The flags field is unused
  yet and must be cleared on entry.

+4.34 KVM_ENABLE_CAP
+
+Capability: basic
   


Capability: basic means that the feature was present in 2.6.22.  
Otherwise you need to specify the KVM_CAP_ that presents this feature.



+Architectures: all

   


But it's implemented for ppc only (other arches will get ENOTTY).


+Not all extensions are enabled by default. Using this ioctl the application
+can enable an extension, making it available to the guest.
+
+On systems that do not support this ioctl, it always fails. On systems that
+do support it, it only works for extensions that are supported for enablement.
+As of writing this the only enablement enabled extenion is KVM_CAP_PPC_OSI.
   


That needs to be documented.  It also needs to be discoverable 
separately - we can have a kernel with KVM_ENABLE_CAP but without 
KVM_CAP_PPC_OSI.


btw, KVM_CAP_PPC_OSI conflicts with the KVM_CAP_ namespace.  Please 
choose another namespace.


Need to document the structure fields.



  /*
@@ -696,6 +705,8 @@ struct kvm_clock_data {
  /* Available with KVM_CAP_DEBUGREGS */
  #define KVM_GET_DEBUGREGS _IOR(KVMIO,  0xa1, struct kvm_debugregs)
  #define KVM_SET_DEBUGREGS _IOW(KVMIO,  0xa2, struct kvm_debugregs)
+/* No need for CAP, because then it just always fails */
+#define KVM_ENABLE_CAP_IOW(KVMIO,  0xa3, struct kvm_enable_cap)
   

The CAPs are needed so you can discover what you have without running guests.



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

--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 02/15] KVM: PPC: Allow userspace to unset the IRQ line

2010-03-09 Thread Avi Kivity

On 03/09/2010 02:54 PM, Alexander Graf wrote:

On 09.03.2010, at 13:50, Avi Kivity wrote:

   

On 03/08/2010 08:03 PM, Alexander Graf wrote:
 

Userspace can tell us that it wants to trigger an interrupt. But
so far it can't tell us that it wants to stop triggering one.

So let's interpret the parameter to the ioctl that we have anyways
to tell us if we want to raise or lower the interrupt line.

   

I asked for a KVM_CAP_ for this.  What was the conclusion of that thread?
 

Uh - did we come to one?

The last thing you said about it was:

   

Having individual capabilities makes backporting a lot easier (otherwise you 
have to backport the whole thing).  If the changes are logically separate, I 
prefer 500 separate capabilities.

However, for a platform bringup, it's okay to have just one capability, 
assuming none of the changes are applicable to other platforms.
 

So I assumed it'd be ok to not have one. If you like I can send an additional 
patch adding the CAP.

   


Well, what's the capability for this patchset?

Things like if you have KVM_CAP_OSI you can assume you have 
KVM_INTERRUPT_LOWER don't work for me.  A platform cap would be called 
KVM_CAP_MOL and explicitly document everything in there.


And it commits you to not deprecating things individually.  Really, 
individual caps are better.


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

--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html