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

2010-03-08 Thread Avi Kivity

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

Avi Kivity wrote:
   

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

Avi Kivity wrote:

   

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

 


   

How does userspace know they exist?


 

#ifdef KVM_INTERRUPT_SET? MOL is the only user of this so far. And
that
won't work without the hypervisor call anyways.


   

We generally compile on one machine, and run on another.

 

So? Then IRQ unsetting doesn't work. Without this series you won't get
much further than booting the kernel anyways because XER is broken, TLB
flushes are broken and FPU loading is broken. So not being able to unset
an IRQ line is the least of your problems :).

   

There's a difference between an error message telling you to upgrade
to a kernel with KVM_CAP_BLAH and a failure.  It's the difference
between a bug report and silence.
 

I see. So we can check for KVM_CAP_PPC_OSI and know that it's in the
same patch series, also making KVM_INTERRUPT_XXX work, right? Or do you
really want to have 500 capabilities for every single patch?
   


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.


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

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


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

2010-03-08 Thread Alexander Graf
Avi Kivity wrote:
> On 03/08/2010 03:55 PM, Alexander Graf wrote:
>> Avi Kivity wrote:
>>   
>>> On 03/08/2010 03:48 PM, Alexander Graf wrote:
>>> 

   
> How does userspace know they exist?
>
>  
 #ifdef KVM_INTERRUPT_SET? MOL is the only user of this so far. And
 that
 won't work without the hypervisor call anyways.


>>> We generally compile on one machine, and run on another.
>>>  
>> So? Then IRQ unsetting doesn't work. Without this series you won't get
>> much further than booting the kernel anyways because XER is broken, TLB
>> flushes are broken and FPU loading is broken. So not being able to unset
>> an IRQ line is the least of your problems :).
>>
>
> There's a difference between an error message telling you to upgrade
> to a kernel with KVM_CAP_BLAH and a failure.  It's the difference
> between a bug report and silence.

I see. So we can check for KVM_CAP_PPC_OSI and know that it's in the
same patch series, also making KVM_INTERRUPT_XXX work, right? Or do you
really want to have 500 capabilities for every single patch?


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


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

2010-03-08 Thread Avi Kivity

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

Avi Kivity wrote:
   

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


   

How does userspace know they exist?

 

#ifdef KVM_INTERRUPT_SET? MOL is the only user of this so far. And that
won't work without the hypervisor call anyways.

   

We generally compile on one machine, and run on another.
 

So? Then IRQ unsetting doesn't work. Without this series you won't get
much further than booting the kernel anyways because XER is broken, TLB
flushes are broken and FPU loading is broken. So not being able to unset
an IRQ line is the least of your problems :).
   


There's a difference between an error message telling you to upgrade to 
a kernel with KVM_CAP_BLAH and a failure.  It's the difference between a 
bug report and silence.


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

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


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

2010-03-08 Thread Alexander Graf
Avi Kivity wrote:
> On 03/08/2010 03:48 PM, Alexander Graf wrote:
>>
>>
>>> How does userspace know they exist?
>>>  
>> #ifdef KVM_INTERRUPT_SET? MOL is the only user of this so far. And that
>> won't work without the hypervisor call anyways.
>>
>
> We generally compile on one machine, and run on another.

So? Then IRQ unsetting doesn't work. Without this series you won't get
much further than booting the kernel anyways because XER is broken, TLB
flushes are broken and FPU loading is broken. So not being able to unset
an IRQ line is the least of your problems :).


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


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

2010-03-08 Thread Avi Kivity

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




How does userspace know they exist?
 

#ifdef KVM_INTERRUPT_SET? MOL is the only user of this so far. And that
won't work without the hypervisor call anyways.
   


We generally compile on one machine, and run on another.


Can you use KVM_IRQ_LINE?
 

I'd rather like to keep that around for when we get an in-kernel-mpic,
which is what we probably ultimately want for qemu.
   


Yes.

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

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


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

2010-03-08 Thread Alexander Graf
Avi Kivity wrote:
> On 03/05/2010 06:50 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.
>>
>> Signed-off-by: Alexander Graf
>> ---
>>   arch/powerpc/include/asm/kvm.h |3 +++
>>   arch/powerpc/include/asm/kvm_ppc.h |2 ++
>>   arch/powerpc/kvm/book3s.c  |6 ++
>>   arch/powerpc/kvm/powerpc.c |5 -
>>   4 files changed, 15 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/kvm.h
>> b/arch/powerpc/include/asm/kvm.h
>> index 19bae31..6c5547d 100644
>> --- a/arch/powerpc/include/asm/kvm.h
>> +++ b/arch/powerpc/include/asm/kvm.h
>> @@ -84,4 +84,7 @@ struct kvm_guest_debug_arch {
>>   #define KVM_REG_QPR0x0040
>>   #define KVM_REG_FQPR0x0060
>>
>> +#define KVM_INTERRUPT_SET-1U
>> +#define KVM_INTERRUPT_UNSET-2U
>>
>
> Funny choice of numbers.

Qemu currently does explicitly set -1U and is the only user.

> How does userspace know they exist?

#ifdef KVM_INTERRUPT_SET? MOL is the only user of this so far. And that
won't work without the hypervisor call anyways.

> Can you use KVM_IRQ_LINE?

I'd rather like to keep that around for when we get an in-kernel-mpic,
which is what we probably ultimately want for qemu.



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


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

2010-03-08 Thread Avi Kivity

On 03/05/2010 06:50 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.

Signed-off-by: Alexander Graf
---
  arch/powerpc/include/asm/kvm.h |3 +++
  arch/powerpc/include/asm/kvm_ppc.h |2 ++
  arch/powerpc/kvm/book3s.c  |6 ++
  arch/powerpc/kvm/powerpc.c |5 -
  4 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm.h b/arch/powerpc/include/asm/kvm.h
index 19bae31..6c5547d 100644
--- a/arch/powerpc/include/asm/kvm.h
+++ b/arch/powerpc/include/asm/kvm.h
@@ -84,4 +84,7 @@ struct kvm_guest_debug_arch {
  #define KVM_REG_QPR   0x0040
  #define KVM_REG_FQPR  0x0060

+#define KVM_INTERRUPT_SET  -1U
+#define KVM_INTERRUPT_UNSET-2U
   


Funny choice of numbers.

How does userspace know they exist?

Can you use KVM_IRQ_LINE?



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

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


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

2010-03-05 Thread Alexander Graf
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.

Signed-off-by: Alexander Graf 
---
 arch/powerpc/include/asm/kvm.h |3 +++
 arch/powerpc/include/asm/kvm_ppc.h |2 ++
 arch/powerpc/kvm/book3s.c  |6 ++
 arch/powerpc/kvm/powerpc.c |5 -
 4 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm.h b/arch/powerpc/include/asm/kvm.h
index 19bae31..6c5547d 100644
--- a/arch/powerpc/include/asm/kvm.h
+++ b/arch/powerpc/include/asm/kvm.h
@@ -84,4 +84,7 @@ struct kvm_guest_debug_arch {
 #define KVM_REG_QPR0x0040
 #define KVM_REG_FQPR   0x0060
 
+#define KVM_INTERRUPT_SET  -1U
+#define KVM_INTERRUPT_UNSET-2U
+
 #endif /* __LINUX_KVM_POWERPC_H */
diff --git a/arch/powerpc/include/asm/kvm_ppc.h 
b/arch/powerpc/include/asm/kvm_ppc.h
index c3912e9..6f92a70 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -92,6 +92,8 @@ extern void kvmppc_core_queue_dec(struct kvm_vcpu *vcpu);
 extern void kvmppc_core_dequeue_dec(struct kvm_vcpu *vcpu);
 extern void kvmppc_core_queue_external(struct kvm_vcpu *vcpu,
struct kvm_interrupt *irq);
+extern void kvmppc_core_dequeue_external(struct kvm_vcpu *vcpu,
+ struct kvm_interrupt *irq);
 
 extern int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
   unsigned int op, int *advance);
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index 3f29959..1830414 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -232,6 +232,12 @@ void kvmppc_core_queue_external(struct kvm_vcpu *vcpu,
kvmppc_book3s_queue_irqprio(vcpu, BOOK3S_INTERRUPT_EXTERNAL);
 }
 
+void kvmppc_core_dequeue_external(struct kvm_vcpu *vcpu,
+  struct kvm_interrupt *irq)
+{
+   kvmppc_book3s_dequeue_irqprio(vcpu, BOOK3S_INTERRUPT_EXTERNAL);
+}
+
 int kvmppc_book3s_irqprio_deliver(struct kvm_vcpu *vcpu, unsigned int priority)
 {
int deliver = 1;
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 5a8eb95..a28a512 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -449,7 +449,10 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct 
kvm_run *run)
 
 int kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu, struct kvm_interrupt *irq)
 {
-   kvmppc_core_queue_external(vcpu, irq);
+   if (irq->irq == KVM_INTERRUPT_UNSET)
+   kvmppc_core_dequeue_external(vcpu, irq);
+   else
+   kvmppc_core_queue_external(vcpu, irq);
 
if (waitqueue_active(&vcpu->wq)) {
wake_up_interruptible(&vcpu->wq);
-- 
1.6.0.2

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