Re: [PATCH v4 2/2] KVM: arm/arm64: Route vtimer events to user space

2016-09-22 Thread Marc Zyngier
On 22/09/16 13:32, Alexander Graf wrote:
> On 09/20/2016 11:21 AM, Marc Zyngier wrote:
>> On 19/09/16 18:39, Alexander Graf wrote:
>>>
>>> On 19.09.16 16:48, Marc Zyngier wrote:
>>>
> +
> + /*
> +  * So we can just explicitly mask or unmask the IRQ, gaining
> +  * more compatibility with oddball irq controllers.
> +  */
> + if (phys_active)
> + disable_percpu_irq(host_vtimer_irq);
> + else
> + enable_percpu_irq(host_vtimer_irq, 0);
 Since you are now targeting random irqchips (as opposed to a GIC
 specifically), what guarantees that the timer is a per-cpu IRQ?
>>> This is the host interrupt controller - and we're already using percpu
>>> irqs on it :). Also as it happens the RPi has them percpu (anything else
>>> wouldn't make sense...).
>> Not really. The RPi is faking percpu interrupts just to have some level
>> of compatibility with the host arch timer driver. But nonetheless, if
>> you're opening the code to something else than a GIC, then you should
>> check that the interrupt you're getting is percpu.
> 
> This should already be covered by request_percpu_irq() in 
> kvm_timer_hyp_init(), no?

Ah, true. Ignore me, then.

M.
-- 
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v4 2/2] KVM: arm/arm64: Route vtimer events to user space

2016-09-22 Thread Alexander Graf

On 09/20/2016 11:21 AM, Marc Zyngier wrote:

On 19/09/16 18:39, Alexander Graf wrote:


On 19.09.16 16:48, Marc Zyngier wrote:


+
+   /*
+* So we can just explicitly mask or unmask the IRQ, gaining
+* more compatibility with oddball irq controllers.
+*/
+   if (phys_active)
+   disable_percpu_irq(host_vtimer_irq);
+   else
+   enable_percpu_irq(host_vtimer_irq, 0);

Since you are now targeting random irqchips (as opposed to a GIC
specifically), what guarantees that the timer is a per-cpu IRQ?

This is the host interrupt controller - and we're already using percpu
irqs on it :). Also as it happens the RPi has them percpu (anything else
wouldn't make sense...).

Not really. The RPi is faking percpu interrupts just to have some level
of compatibility with the host arch timer driver. But nonetheless, if
you're opening the code to something else than a GIC, then you should
check that the interrupt you're getting is percpu.


This should already be covered by request_percpu_irq() in 
kvm_timer_hyp_init(), no?



Alex

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v4 2/2] KVM: arm/arm64: Route vtimer events to user space

2016-09-22 Thread Alexander Graf

On 09/20/2016 06:42 PM, Marc Zyngier wrote:

On 20/09/16 15:31, Alexander Graf wrote:

On 09/20/2016 02:37 PM, Marc Zyngier wrote:

[...]


We also need to know "timer line low + timer line masked", as otherwise
we might get spurious interrupts in the guest, no?

Yes. Though you can't really know about this one, and you'll have to
wait until the next natural exit to find out. As long as the spurious is

We can provoke a special exit for it, no?

How? The guest decides to disable its timer. That doesn't trigger any
exit whatsoever. You'll have to wait until the next exit from the guest
to notice it.

Before we inject a timer interrupt, we can check whether the pending
semantics of user space / kernel space match. If they don't match, we
can exit before we inject the interrupt and allow user space to disable
the pending state again.

Let's rewind a bit, because I've long lost track of what you're trying
to do to handle what.

You need two signals:

(1) TIMER_LEVEL: the output of the timer line, having accounted for the
IMASK bit. This is conveniently the value of timer->irq.level.

(2) TIMER_IRQ_MASK: an indication from userspace that a timer interrupt
is pending, and that the physical line should be masked.

You need a number of rules:

(a) On exit to userspace, the kernel always exposes the value of
TIMER_LEVEL.

(b) On kernel entry, userspace always exposes the required
TIMER_IRQ_MASK, depending on what has been exposed to it by TIMER_LEVEL.

(c) If on guest exit, TIMER_LEVEL==1 and TIMER_IRQ_MASK==0, perform a
userspace exit, because the emulated GIC needs to make the interrupt
pending.


This should be "before guest entry", because the timer might have 
expired in between.



(d) If on guest exit, TIMER_LEVEL==0 and TIMER_IRQ_MASK==1, perform a
userspace exit, because the guest has disabled its timer before taking
the interrupt, and the emulated GIC needs to retire the pending state.

and that's it. Nothing else. The kernel tells userspace the state of the
timer, and userspace drives the masking of the physical interrupt.
Conveniently, this matches what the current code does.


Yup. It seems to work. It also does feel slower than the previous code, 
but maybe that's just me. It definitely is way more correct.


I'll trace around a bit more to see whether I can spot any obviously low 
hanging performance fruits, then prettify the patches and send them out :).



Alex

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v4 2/2] KVM: arm/arm64: Route vtimer events to user space

2016-09-20 Thread Marc Zyngier
On 20/09/16 15:31, Alexander Graf wrote:
> On 09/20/2016 02:37 PM, Marc Zyngier wrote:

[...]

> We also need to know "timer line low + timer line masked", as otherwise
> we might get spurious interrupts in the guest, no?
 Yes. Though you can't really know about this one, and you'll have to
 wait until the next natural exit to find out. As long as the spurious is
>>> We can provoke a special exit for it, no?
>> How? The guest decides to disable its timer. That doesn't trigger any
>> exit whatsoever. You'll have to wait until the next exit from the guest
>> to notice it.
> 
> Before we inject a timer interrupt, we can check whether the pending 
> semantics of user space / kernel space match. If they don't match, we 
> can exit before we inject the interrupt and allow user space to disable 
> the pending state again.

Let's rewind a bit, because I've long lost track of what you're trying
to do to handle what.

You need two signals:

(1) TIMER_LEVEL: the output of the timer line, having accounted for the
IMASK bit. This is conveniently the value of timer->irq.level.

(2) TIMER_IRQ_MASK: an indication from userspace that a timer interrupt
is pending, and that the physical line should be masked.

You need a number of rules:

(a) On exit to userspace, the kernel always exposes the value of
TIMER_LEVEL.

(b) On kernel entry, userspace always exposes the required
TIMER_IRQ_MASK, depending on what has been exposed to it by TIMER_LEVEL.

(c) If on guest exit, TIMER_LEVEL==1 and TIMER_IRQ_MASK==0, perform a
userspace exit, because the emulated GIC needs to make the interrupt
pending.

(d) If on guest exit, TIMER_LEVEL==0 and TIMER_IRQ_MASK==1, perform a
userspace exit, because the guest has disabled its timer before taking
the interrupt, and the emulated GIC needs to retire the pending state.

and that's it. Nothing else. The kernel tells userspace the state of the
timer, and userspace drives the masking of the physical interrupt.
Conveniently, this matches what the current code does.

> 
>>
 "spurious in the GIC sense" (hence returning 0x3ff) and not a spurious
 timer interrupt being presented, you're fine.
>>> Imagine
>>>
>>> * guest configures timer to fire
>>> * kvm exits to user space because lines mismatch now
>> I suppose you mean timer fired + physical interrupt for the virtual enabled?
> 
> In this scheme, I would simply say "user space thinks it's pending" == 
> "host vtimer irq is masked". So because user space thinks that no timer 
> is pending and the guest managed to trigger a vtimer irq on the host, 
> the guest exited on a physical vtimer irq. Kvm then checks whether the 
> timer expired and sets the vcpu timer pending status to 1. User space 
> status is still 0, so kvm exits to user space to update the status.
> 
>>
>>> * user space sets gic line up, setting vcpu irq line high
>>> * guest runs, handles IRQ, calls EOI with IRQs disabled on the core
>> Same thing: you mean physical interrupt for the virtual timer disabled?
>> or something else?
> 
> I mean the guest sets pstate.I = 0.

The I bit is a mask, just like any other exception bit in PSTATE. So
you're *enabling* interrupts here. But let's assume the interrupts are
disabled, as they are in an interrupt handler.

> 
>>
>>> * user space handles EOI, but still leaves the vcpu line high
>> If there is another pending interrupt, sure. Otherwise, that's a bug.
> 
> Why is that a bug? The line really is still pending at this point.

So what is this "vcpu line"? HCR_EL2.VI? The timer interrupt through the
GIC? The timer output line?

> 
>>
>>> * guest runs, sets cval, enables IRQs on the core
>> What is that interrupt? Please name things, because that's very confusing.
> 
> The guest sets pstate.I=1.

And?

> 
>>
>>> * kvm injects a timer interrupt into the guest
>> Why? Has the timer fired?
> 
> cntv.ctl.imask = 0 here, but user space doesn't know that yet. So the 
> "pending" state is still active.

Rule (a) should apply, always. Whether it is pending or not depends on
what userspace decides to do. If userspace doesn't play ball, the guest
will keep exiting.

> 
>>
>>> At this point, the pending state from user space is bogus, as the line
>>> really isn't pending anymore. However, when the guest asks the GIC at
>>> that point what interrupt is pending, the line down state will have
>>> populated into user space before the mmio request gets processed. So you
>>> will see a 0x3ff return I guess.
>>>
>>> Is that reasonable?
>> I'm sorry, but you need to put names on things and precisely describe
>> the various events. because from the above script, I can derive twenty
>> different scenarios...
> 
> I'm sure one of them has to be racy? :)
> 
> (in fact, there are probably more than what I came up with that really 
> are...)

At this stage, I don't have any understanding of what you're trying to
do. I've given you what I think needs to be implemented, please let me
know if that matches your understandin

Re: [PATCH v4 2/2] KVM: arm/arm64: Route vtimer events to user space

2016-09-20 Thread Alexander Graf

On 09/20/2016 02:37 PM, Marc Zyngier wrote:

On 20/09/16 13:22, Alexander Graf wrote:

On 09/20/2016 12:28 PM, Marc Zyngier wrote:

On 20/09/16 11:05, Alexander Graf wrote:

On 09/20/2016 11:39 AM, Marc Zyngier wrote:

On 20/09/16 10:26, Alexander Graf wrote:

On 20.09.16 11:21, Marc Zyngier wrote:

On 19/09/16 18:39, Alexander Graf wrote:

On 19.09.16 16:48, Marc Zyngier wrote:

On 19/09/16 12:14, Alexander Graf wrote:

We have 2 modes for dealing with interrupts in the ARM world. We can either
handle them all using hardware acceleration through the vgic or we can emulate
a gic in user space and only drive CPU IRQ pins from there.

Unfortunately, when driving IRQs from user space, we never tell user space
about timer events that may result in interrupt line state changes, so we
lose out on timer events if we run with user space gic emulation.

This patch fixes that by routing vtimer expiration events to user space.
With this patch I can successfully run edk2 and Linux with user space gic
emulation.

Signed-off-by: Alexander Graf 

---

v1 -> v2:

 - Add back curly brace that got lost

v2 -> v3:

 - Split into patch set

v3 -> v4:

 - Improve documentation
---
Documentation/virtual/kvm/api.txt |  30 -
arch/arm/include/asm/kvm_host.h   |   3 +
arch/arm/kvm/arm.c|  22 ---
arch/arm64/include/asm/kvm_host.h |   3 +
include/uapi/linux/kvm.h  |  14 +
virt/kvm/arm/arch_timer.c | 125 
+++---
6 files changed, 155 insertions(+), 42 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt 
b/Documentation/virtual/kvm/api.txt
index 23937e0..1c0bd86 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -3202,9 +3202,14 @@ struct kvm_run {
/* in */
__u8 request_interrupt_window;

-Request that KVM_RUN return when it becomes possible to inject external

+[x86] Request that KVM_RUN return when it becomes possible to inject external
interrupts into the guest.  Useful in conjunction with KVM_INTERRUPT.

+[arm*] Bits set to 1 in here mask IRQ lines that would otherwise potentially

+trigger forever. These lines are available:
+
+KVM_IRQWINDOW_VTIMER  -  Masks hw virtual timer irq while in guest
+
__u8 padding1[7];

	/* out */

@@ -3519,6 +3524,18 @@ Hyper-V SynIC state change. Notification is used to 
remap SynIC
event/message pages and to enable/disable SynIC messages/events processing
in userspace.

+		/* KVM_EXIT_ARM_TIMER */

+   struct {
+   __u8 timesource;
+   } arm_timer;
+
+Indicates that a timer triggered that user space needs to handle and
+potentially mask with vcpu->run->request_interrupt_window to allow the
+guest to proceed. This only happens for timers that got enabled through
+KVM_CAP_ARM_TIMER. The following time sources are available:
+
+KVM_ARM_TIMER_VTIMER  - virtual cpu timer
+
/* Fix the size of the union. */
char padding[256];
};
@@ -3739,6 +3756,17 @@ Once this is done the KVM_REG_MIPS_VEC_* and 
KVM_REG_MIPS_MSA_* registers can be
accessed, and the Config5.MSAEn bit is accessible via the KVM API and also 
from
the guest.

+6.11 KVM_CAP_ARM_TIMER

+
+Architectures: arm, arm64
+Target: vcpu
+Parameters: args[0] contains a bitmap of timers to select (see 5.)
+
+This capability allows to route per-core timers into user space. When it's
+enabled and no in-kernel interrupt controller is in use, the timers selected
+by args[0] trigger KVM_EXIT_ARM_TIMER guest exits when they are pending,
+unless masked by vcpu->run->request_interrupt_window (see 5.).
+
7. Capabilities that can be enabled on VMs
--

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h

index de338d9..77d1f73 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -180,6 +180,9 @@ struct kvm_vcpu_arch {

	/* Detect first run of a vcpu */

bool has_run_once;
+
+   /* User space wants timer notifications */
+   bool user_space_arm_timers;

Please move this to the timer structure.

Sure.


};

struct kvm_vm_stat {

diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index c84b6ad..57bdb71 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -187,6 +187,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
case KVM_CAP_ARM_PSCI_0_2:
case KVM_CAP_READONLY_MEM:
case KVM_CAP_MP_STATE:
+   case KVM_CAP_ARM_TIMER:
r = 1;
break;
case KVM_CAP_COALESCED_MMIO:
@@ -474,13 +475,7 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu)
return ret;
}

-	/*

-* Enable the arch timers only if we have an in-kernel VGIC
-* and it has been properly initialized, sinc

Re: [PATCH v4 2/2] KVM: arm/arm64: Route vtimer events to user space

2016-09-20 Thread Marc Zyngier
On 20/09/16 13:22, Alexander Graf wrote:
> On 09/20/2016 12:28 PM, Marc Zyngier wrote:
>> On 20/09/16 11:05, Alexander Graf wrote:
>>> On 09/20/2016 11:39 AM, Marc Zyngier wrote:
 On 20/09/16 10:26, Alexander Graf wrote:
> On 20.09.16 11:21, Marc Zyngier wrote:
>> On 19/09/16 18:39, Alexander Graf wrote:
>>> On 19.09.16 16:48, Marc Zyngier wrote:
 On 19/09/16 12:14, Alexander Graf wrote:
> We have 2 modes for dealing with interrupts in the ARM world. We can 
> either
> handle them all using hardware acceleration through the vgic or we 
> can emulate
> a gic in user space and only drive CPU IRQ pins from there.
>
> Unfortunately, when driving IRQs from user space, we never tell user 
> space
> about timer events that may result in interrupt line state changes, 
> so we
> lose out on timer events if we run with user space gic emulation.
>
> This patch fixes that by routing vtimer expiration events to user 
> space.
> With this patch I can successfully run edk2 and Linux with user space 
> gic
> emulation.
>
> Signed-off-by: Alexander Graf 
>
> ---
>
> v1 -> v2:
>
> - Add back curly brace that got lost
>
> v2 -> v3:
>
> - Split into patch set
>
> v3 -> v4:
>
> - Improve documentation
> ---
>Documentation/virtual/kvm/api.txt |  30 -
>arch/arm/include/asm/kvm_host.h   |   3 +
>arch/arm/kvm/arm.c|  22 ---
>arch/arm64/include/asm/kvm_host.h |   3 +
>include/uapi/linux/kvm.h  |  14 +
>virt/kvm/arm/arch_timer.c | 125 
> +++---
>6 files changed, 155 insertions(+), 42 deletions(-)
>
> diff --git a/Documentation/virtual/kvm/api.txt 
> b/Documentation/virtual/kvm/api.txt
> index 23937e0..1c0bd86 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -3202,9 +3202,14 @@ struct kvm_run {
>   /* in */
>   __u8 request_interrupt_window;
>
> -Request that KVM_RUN return when it becomes possible to inject 
> external
> +[x86] Request that KVM_RUN return when it becomes possible to inject 
> external
>interrupts into the guest.  Useful in conjunction with 
> KVM_INTERRUPT.
>
> +[arm*] Bits set to 1 in here mask IRQ lines that would otherwise 
> potentially
> +trigger forever. These lines are available:
> +
> +KVM_IRQWINDOW_VTIMER  -  Masks hw virtual timer irq while in 
> guest
> +
>   __u8 padding1[7];
>
>   /* out */
> @@ -3519,6 +3524,18 @@ Hyper-V SynIC state change. Notification is 
> used to remap SynIC
>event/message pages and to enable/disable SynIC messages/events 
> processing
>in userspace.
>
> + /* KVM_EXIT_ARM_TIMER */
> + struct {
> + __u8 timesource;
> + } arm_timer;
> +
> +Indicates that a timer triggered that user space needs to handle and
> +potentially mask with vcpu->run->request_interrupt_window to allow 
> the
> +guest to proceed. This only happens for timers that got enabled 
> through
> +KVM_CAP_ARM_TIMER. The following time sources are available:
> +
> +KVM_ARM_TIMER_VTIMER  - virtual cpu timer
> +
>   /* Fix the size of the union. */
>   char padding[256];
>   };
> @@ -3739,6 +3756,17 @@ Once this is done the KVM_REG_MIPS_VEC_* and 
> KVM_REG_MIPS_MSA_* registers can be
>accessed, and the Config5.MSAEn bit is accessible via the KVM API 
> and also from
>the guest.
>
> +6.11 KVM_CAP_ARM_TIMER
> +
> +Architectures: arm, arm64
> +Target: vcpu
> +Parameters: args[0] contains a bitmap of timers to select (see 5.)
> +
> +This capability allows to route per-core timers into user space. 
> When it's
> +enabled and no in-kernel interrupt controller is in use, the timers 
> selected
> +by args[0] trigger KVM_EXIT_ARM_TIMER guest exits when they are 
> pending,
> +unless masked by vcpu->run->request_interrupt_window (see 5.).
> +
>7. Capabilities that can be enabled on VMs
>--
>
> diff --git a/arch/arm/include/asm/kvm_host.

Re: [PATCH v4 2/2] KVM: arm/arm64: Route vtimer events to user space

2016-09-20 Thread Alexander Graf

On 09/20/2016 12:28 PM, Marc Zyngier wrote:

On 20/09/16 11:05, Alexander Graf wrote:

On 09/20/2016 11:39 AM, Marc Zyngier wrote:

On 20/09/16 10:26, Alexander Graf wrote:

On 20.09.16 11:21, Marc Zyngier wrote:

On 19/09/16 18:39, Alexander Graf wrote:

On 19.09.16 16:48, Marc Zyngier wrote:

On 19/09/16 12:14, Alexander Graf wrote:

We have 2 modes for dealing with interrupts in the ARM world. We can either
handle them all using hardware acceleration through the vgic or we can emulate
a gic in user space and only drive CPU IRQ pins from there.

Unfortunately, when driving IRQs from user space, we never tell user space
about timer events that may result in interrupt line state changes, so we
lose out on timer events if we run with user space gic emulation.

This patch fixes that by routing vtimer expiration events to user space.
With this patch I can successfully run edk2 and Linux with user space gic
emulation.

Signed-off-by: Alexander Graf 

---

v1 -> v2:

- Add back curly brace that got lost

v2 -> v3:

- Split into patch set

v3 -> v4:

- Improve documentation
---
   Documentation/virtual/kvm/api.txt |  30 -
   arch/arm/include/asm/kvm_host.h   |   3 +
   arch/arm/kvm/arm.c|  22 ---
   arch/arm64/include/asm/kvm_host.h |   3 +
   include/uapi/linux/kvm.h  |  14 +
   virt/kvm/arm/arch_timer.c | 125 
+++---
   6 files changed, 155 insertions(+), 42 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt 
b/Documentation/virtual/kvm/api.txt
index 23937e0..1c0bd86 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -3202,9 +3202,14 @@ struct kvm_run {
/* in */
__u8 request_interrupt_window;
   
-Request that KVM_RUN return when it becomes possible to inject external

+[x86] Request that KVM_RUN return when it becomes possible to inject external
   interrupts into the guest.  Useful in conjunction with KVM_INTERRUPT.
   
+[arm*] Bits set to 1 in here mask IRQ lines that would otherwise potentially

+trigger forever. These lines are available:
+
+KVM_IRQWINDOW_VTIMER  -  Masks hw virtual timer irq while in guest
+
__u8 padding1[7];
   
   	/* out */

@@ -3519,6 +3524,18 @@ Hyper-V SynIC state change. Notification is used to 
remap SynIC
   event/message pages and to enable/disable SynIC messages/events processing
   in userspace.
   
+		/* KVM_EXIT_ARM_TIMER */

+   struct {
+   __u8 timesource;
+   } arm_timer;
+
+Indicates that a timer triggered that user space needs to handle and
+potentially mask with vcpu->run->request_interrupt_window to allow the
+guest to proceed. This only happens for timers that got enabled through
+KVM_CAP_ARM_TIMER. The following time sources are available:
+
+KVM_ARM_TIMER_VTIMER  - virtual cpu timer
+
/* Fix the size of the union. */
char padding[256];
};
@@ -3739,6 +3756,17 @@ Once this is done the KVM_REG_MIPS_VEC_* and 
KVM_REG_MIPS_MSA_* registers can be
   accessed, and the Config5.MSAEn bit is accessible via the KVM API and also 
from
   the guest.
   
+6.11 KVM_CAP_ARM_TIMER

+
+Architectures: arm, arm64
+Target: vcpu
+Parameters: args[0] contains a bitmap of timers to select (see 5.)
+
+This capability allows to route per-core timers into user space. When it's
+enabled and no in-kernel interrupt controller is in use, the timers selected
+by args[0] trigger KVM_EXIT_ARM_TIMER guest exits when they are pending,
+unless masked by vcpu->run->request_interrupt_window (see 5.).
+
   7. Capabilities that can be enabled on VMs
   --
   
diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h

index de338d9..77d1f73 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -180,6 +180,9 @@ struct kvm_vcpu_arch {
   
   	/* Detect first run of a vcpu */

bool has_run_once;
+
+   /* User space wants timer notifications */
+   bool user_space_arm_timers;

Please move this to the timer structure.

Sure.


   };
   
   struct kvm_vm_stat {

diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index c84b6ad..57bdb71 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -187,6 +187,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
case KVM_CAP_ARM_PSCI_0_2:
case KVM_CAP_READONLY_MEM:
case KVM_CAP_MP_STATE:
+   case KVM_CAP_ARM_TIMER:
r = 1;
break;
case KVM_CAP_COALESCED_MMIO:
@@ -474,13 +475,7 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu)
return ret;
}
   
-	/*

-* Enable the arch timers only if we have an in-kernel VGIC
-* and it has been properly initialized, since we cannot handle
-* interrupts from the virtual timer with a userspace gic.
-*/
-   if (irqchip

Re: [PATCH v4 2/2] KVM: arm/arm64: Route vtimer events to user space

2016-09-20 Thread Marc Zyngier
On 20/09/16 11:05, Alexander Graf wrote:
> On 09/20/2016 11:39 AM, Marc Zyngier wrote:
>> On 20/09/16 10:26, Alexander Graf wrote:
>>>
>>> On 20.09.16 11:21, Marc Zyngier wrote:
 On 19/09/16 18:39, Alexander Graf wrote:
>
> On 19.09.16 16:48, Marc Zyngier wrote:
>> On 19/09/16 12:14, Alexander Graf wrote:
>>> We have 2 modes for dealing with interrupts in the ARM world. We can 
>>> either
>>> handle them all using hardware acceleration through the vgic or we can 
>>> emulate
>>> a gic in user space and only drive CPU IRQ pins from there.
>>>
>>> Unfortunately, when driving IRQs from user space, we never tell user 
>>> space
>>> about timer events that may result in interrupt line state changes, so 
>>> we
>>> lose out on timer events if we run with user space gic emulation.
>>>
>>> This patch fixes that by routing vtimer expiration events to user space.
>>> With this patch I can successfully run edk2 and Linux with user space 
>>> gic
>>> emulation.
>>>
>>> Signed-off-by: Alexander Graf 
>>>
>>> ---
>>>
>>> v1 -> v2:
>>>
>>>- Add back curly brace that got lost
>>>
>>> v2 -> v3:
>>>
>>>- Split into patch set
>>>
>>> v3 -> v4:
>>>
>>>- Improve documentation
>>> ---
>>>   Documentation/virtual/kvm/api.txt |  30 -
>>>   arch/arm/include/asm/kvm_host.h   |   3 +
>>>   arch/arm/kvm/arm.c|  22 ---
>>>   arch/arm64/include/asm/kvm_host.h |   3 +
>>>   include/uapi/linux/kvm.h  |  14 +
>>>   virt/kvm/arm/arch_timer.c | 125 
>>> +++---
>>>   6 files changed, 155 insertions(+), 42 deletions(-)
>>>
>>> diff --git a/Documentation/virtual/kvm/api.txt 
>>> b/Documentation/virtual/kvm/api.txt
>>> index 23937e0..1c0bd86 100644
>>> --- a/Documentation/virtual/kvm/api.txt
>>> +++ b/Documentation/virtual/kvm/api.txt
>>> @@ -3202,9 +3202,14 @@ struct kvm_run {
>>> /* in */
>>> __u8 request_interrupt_window;
>>>   
>>> -Request that KVM_RUN return when it becomes possible to inject external
>>> +[x86] Request that KVM_RUN return when it becomes possible to inject 
>>> external
>>>   interrupts into the guest.  Useful in conjunction with KVM_INTERRUPT.
>>>   
>>> +[arm*] Bits set to 1 in here mask IRQ lines that would otherwise 
>>> potentially
>>> +trigger forever. These lines are available:
>>> +
>>> +KVM_IRQWINDOW_VTIMER  -  Masks hw virtual timer irq while in guest
>>> +
>>> __u8 padding1[7];
>>>   
>>> /* out */
>>> @@ -3519,6 +3524,18 @@ Hyper-V SynIC state change. Notification is used 
>>> to remap SynIC
>>>   event/message pages and to enable/disable SynIC messages/events 
>>> processing
>>>   in userspace.
>>>   
>>> +   /* KVM_EXIT_ARM_TIMER */
>>> +   struct {
>>> +   __u8 timesource;
>>> +   } arm_timer;
>>> +
>>> +Indicates that a timer triggered that user space needs to handle and
>>> +potentially mask with vcpu->run->request_interrupt_window to allow the
>>> +guest to proceed. This only happens for timers that got enabled through
>>> +KVM_CAP_ARM_TIMER. The following time sources are available:
>>> +
>>> +KVM_ARM_TIMER_VTIMER  - virtual cpu timer
>>> +
>>> /* Fix the size of the union. */
>>> char padding[256];
>>> };
>>> @@ -3739,6 +3756,17 @@ Once this is done the KVM_REG_MIPS_VEC_* and 
>>> KVM_REG_MIPS_MSA_* registers can be
>>>   accessed, and the Config5.MSAEn bit is accessible via the KVM API and 
>>> also from
>>>   the guest.
>>>   
>>> +6.11 KVM_CAP_ARM_TIMER
>>> +
>>> +Architectures: arm, arm64
>>> +Target: vcpu
>>> +Parameters: args[0] contains a bitmap of timers to select (see 5.)
>>> +
>>> +This capability allows to route per-core timers into user space. When 
>>> it's
>>> +enabled and no in-kernel interrupt controller is in use, the timers 
>>> selected
>>> +by args[0] trigger KVM_EXIT_ARM_TIMER guest exits when they are 
>>> pending,
>>> +unless masked by vcpu->run->request_interrupt_window (see 5.).
>>> +
>>>   7. Capabilities that can be enabled on VMs
>>>   --
>>>   
>>> diff --git a/arch/arm/include/asm/kvm_host.h 
>>> b/arch/arm/include/asm/kvm_host.h
>>> index de338d9..77d1f73 100644
>>> --- a/arch/arm/include/asm/kvm_host.h
>>> +++ b/arch/arm/include/asm/kvm_host.h
>>> @@ -180,6 +180,9 @@ struct kvm_vcpu_arch {
>>>   
>>> /* Detect first run of a vcpu */
>>> bool has_run_once;
>>> +
>>> +   /* User space wants timer notifi

Re: [PATCH v4 2/2] KVM: arm/arm64: Route vtimer events to user space

2016-09-20 Thread Alexander Graf

On 09/20/2016 11:39 AM, Marc Zyngier wrote:

On 20/09/16 10:26, Alexander Graf wrote:


On 20.09.16 11:21, Marc Zyngier wrote:

On 19/09/16 18:39, Alexander Graf wrote:


On 19.09.16 16:48, Marc Zyngier wrote:

On 19/09/16 12:14, Alexander Graf wrote:

We have 2 modes for dealing with interrupts in the ARM world. We can either
handle them all using hardware acceleration through the vgic or we can emulate
a gic in user space and only drive CPU IRQ pins from there.

Unfortunately, when driving IRQs from user space, we never tell user space
about timer events that may result in interrupt line state changes, so we
lose out on timer events if we run with user space gic emulation.

This patch fixes that by routing vtimer expiration events to user space.
With this patch I can successfully run edk2 and Linux with user space gic
emulation.

Signed-off-by: Alexander Graf 

---

v1 -> v2:

   - Add back curly brace that got lost

v2 -> v3:

   - Split into patch set

v3 -> v4:

   - Improve documentation
---
  Documentation/virtual/kvm/api.txt |  30 -
  arch/arm/include/asm/kvm_host.h   |   3 +
  arch/arm/kvm/arm.c|  22 ---
  arch/arm64/include/asm/kvm_host.h |   3 +
  include/uapi/linux/kvm.h  |  14 +
  virt/kvm/arm/arch_timer.c | 125 +++---
  6 files changed, 155 insertions(+), 42 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt 
b/Documentation/virtual/kvm/api.txt
index 23937e0..1c0bd86 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -3202,9 +3202,14 @@ struct kvm_run {
/* in */
__u8 request_interrupt_window;
  
-Request that KVM_RUN return when it becomes possible to inject external

+[x86] Request that KVM_RUN return when it becomes possible to inject external
  interrupts into the guest.  Useful in conjunction with KVM_INTERRUPT.
  
+[arm*] Bits set to 1 in here mask IRQ lines that would otherwise potentially

+trigger forever. These lines are available:
+
+KVM_IRQWINDOW_VTIMER  -  Masks hw virtual timer irq while in guest
+
__u8 padding1[7];
  
  	/* out */

@@ -3519,6 +3524,18 @@ Hyper-V SynIC state change. Notification is used to 
remap SynIC
  event/message pages and to enable/disable SynIC messages/events processing
  in userspace.
  
+		/* KVM_EXIT_ARM_TIMER */

+   struct {
+   __u8 timesource;
+   } arm_timer;
+
+Indicates that a timer triggered that user space needs to handle and
+potentially mask with vcpu->run->request_interrupt_window to allow the
+guest to proceed. This only happens for timers that got enabled through
+KVM_CAP_ARM_TIMER. The following time sources are available:
+
+KVM_ARM_TIMER_VTIMER  - virtual cpu timer
+
/* Fix the size of the union. */
char padding[256];
};
@@ -3739,6 +3756,17 @@ Once this is done the KVM_REG_MIPS_VEC_* and 
KVM_REG_MIPS_MSA_* registers can be
  accessed, and the Config5.MSAEn bit is accessible via the KVM API and also 
from
  the guest.
  
+6.11 KVM_CAP_ARM_TIMER

+
+Architectures: arm, arm64
+Target: vcpu
+Parameters: args[0] contains a bitmap of timers to select (see 5.)
+
+This capability allows to route per-core timers into user space. When it's
+enabled and no in-kernel interrupt controller is in use, the timers selected
+by args[0] trigger KVM_EXIT_ARM_TIMER guest exits when they are pending,
+unless masked by vcpu->run->request_interrupt_window (see 5.).
+
  7. Capabilities that can be enabled on VMs
  --
  
diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h

index de338d9..77d1f73 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -180,6 +180,9 @@ struct kvm_vcpu_arch {
  
  	/* Detect first run of a vcpu */

bool has_run_once;
+
+   /* User space wants timer notifications */
+   bool user_space_arm_timers;

Please move this to the timer structure.

Sure.


  };
  
  struct kvm_vm_stat {

diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index c84b6ad..57bdb71 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -187,6 +187,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
case KVM_CAP_ARM_PSCI_0_2:
case KVM_CAP_READONLY_MEM:
case KVM_CAP_MP_STATE:
+   case KVM_CAP_ARM_TIMER:
r = 1;
break;
case KVM_CAP_COALESCED_MMIO:
@@ -474,13 +475,7 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu)
return ret;
}
  
-	/*

-* Enable the arch timers only if we have an in-kernel VGIC
-* and it has been properly initialized, since we cannot handle
-* interrupts from the virtual timer with a userspace gic.
-*/
-   if (irqchip_in_kernel(kvm) && vgic_initialized(kvm))
-   ret = kvm_timer_enable(vcpu);
+   ret = kvm_timer_enab

Re: [PATCH v4 2/2] KVM: arm/arm64: Route vtimer events to user space

2016-09-20 Thread Marc Zyngier
On 20/09/16 10:26, Alexander Graf wrote:
> 
> 
> On 20.09.16 11:21, Marc Zyngier wrote:
>> On 19/09/16 18:39, Alexander Graf wrote:
>>>
>>>
>>> On 19.09.16 16:48, Marc Zyngier wrote:
 On 19/09/16 12:14, Alexander Graf wrote:
> We have 2 modes for dealing with interrupts in the ARM world. We can 
> either
> handle them all using hardware acceleration through the vgic or we can 
> emulate
> a gic in user space and only drive CPU IRQ pins from there.
>
> Unfortunately, when driving IRQs from user space, we never tell user space
> about timer events that may result in interrupt line state changes, so we
> lose out on timer events if we run with user space gic emulation.
>
> This patch fixes that by routing vtimer expiration events to user space.
> With this patch I can successfully run edk2 and Linux with user space gic
> emulation.
>
> Signed-off-by: Alexander Graf 
>
> ---
>
> v1 -> v2:
>
>   - Add back curly brace that got lost
>
> v2 -> v3:
>
>   - Split into patch set
>
> v3 -> v4:
>
>   - Improve documentation
> ---
>  Documentation/virtual/kvm/api.txt |  30 -
>  arch/arm/include/asm/kvm_host.h   |   3 +
>  arch/arm/kvm/arm.c|  22 ---
>  arch/arm64/include/asm/kvm_host.h |   3 +
>  include/uapi/linux/kvm.h  |  14 +
>  virt/kvm/arm/arch_timer.c | 125 
> +++---
>  6 files changed, 155 insertions(+), 42 deletions(-)
>
> diff --git a/Documentation/virtual/kvm/api.txt 
> b/Documentation/virtual/kvm/api.txt
> index 23937e0..1c0bd86 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -3202,9 +3202,14 @@ struct kvm_run {
>   /* in */
>   __u8 request_interrupt_window;
>  
> -Request that KVM_RUN return when it becomes possible to inject external
> +[x86] Request that KVM_RUN return when it becomes possible to inject 
> external
>  interrupts into the guest.  Useful in conjunction with KVM_INTERRUPT.
>  
> +[arm*] Bits set to 1 in here mask IRQ lines that would otherwise 
> potentially
> +trigger forever. These lines are available:
> +
> +KVM_IRQWINDOW_VTIMER  -  Masks hw virtual timer irq while in guest
> +
>   __u8 padding1[7];
>  
>   /* out */
> @@ -3519,6 +3524,18 @@ Hyper-V SynIC state change. Notification is used 
> to remap SynIC
>  event/message pages and to enable/disable SynIC messages/events 
> processing
>  in userspace.
>  
> + /* KVM_EXIT_ARM_TIMER */
> + struct {
> + __u8 timesource;
> + } arm_timer;
> +
> +Indicates that a timer triggered that user space needs to handle and
> +potentially mask with vcpu->run->request_interrupt_window to allow the
> +guest to proceed. This only happens for timers that got enabled through
> +KVM_CAP_ARM_TIMER. The following time sources are available:
> +
> +KVM_ARM_TIMER_VTIMER  - virtual cpu timer
> +
>   /* Fix the size of the union. */
>   char padding[256];
>   };
> @@ -3739,6 +3756,17 @@ Once this is done the KVM_REG_MIPS_VEC_* and 
> KVM_REG_MIPS_MSA_* registers can be
>  accessed, and the Config5.MSAEn bit is accessible via the KVM API and 
> also from
>  the guest.
>  
> +6.11 KVM_CAP_ARM_TIMER
> +
> +Architectures: arm, arm64
> +Target: vcpu
> +Parameters: args[0] contains a bitmap of timers to select (see 5.)
> +
> +This capability allows to route per-core timers into user space. When 
> it's
> +enabled and no in-kernel interrupt controller is in use, the timers 
> selected
> +by args[0] trigger KVM_EXIT_ARM_TIMER guest exits when they are pending,
> +unless masked by vcpu->run->request_interrupt_window (see 5.).
> +
>  7. Capabilities that can be enabled on VMs
>  --
>  
> diff --git a/arch/arm/include/asm/kvm_host.h 
> b/arch/arm/include/asm/kvm_host.h
> index de338d9..77d1f73 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -180,6 +180,9 @@ struct kvm_vcpu_arch {
>  
>   /* Detect first run of a vcpu */
>   bool has_run_once;
> +
> + /* User space wants timer notifications */
> + bool user_space_arm_timers;

 Please move this to the timer structure.
>>>
>>> Sure.
>>>

>  };
>  
>  struct kvm_vm_stat {
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index c84b6ad..57bdb71 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -187,6 +187,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, 
> long ext)
>   case KVM_CAP_ARM_PSCI_0_2:
>   case KVM_C

Re: [PATCH v4 2/2] KVM: arm/arm64: Route vtimer events to user space

2016-09-20 Thread Alexander Graf


On 20.09.16 11:21, Marc Zyngier wrote:
> On 19/09/16 18:39, Alexander Graf wrote:
>>
>>
>> On 19.09.16 16:48, Marc Zyngier wrote:
>>> On 19/09/16 12:14, Alexander Graf wrote:
 We have 2 modes for dealing with interrupts in the ARM world. We can either
 handle them all using hardware acceleration through the vgic or we can 
 emulate
 a gic in user space and only drive CPU IRQ pins from there.

 Unfortunately, when driving IRQs from user space, we never tell user space
 about timer events that may result in interrupt line state changes, so we
 lose out on timer events if we run with user space gic emulation.

 This patch fixes that by routing vtimer expiration events to user space.
 With this patch I can successfully run edk2 and Linux with user space gic
 emulation.

 Signed-off-by: Alexander Graf 

 ---

 v1 -> v2:

   - Add back curly brace that got lost

 v2 -> v3:

   - Split into patch set

 v3 -> v4:

   - Improve documentation
 ---
  Documentation/virtual/kvm/api.txt |  30 -
  arch/arm/include/asm/kvm_host.h   |   3 +
  arch/arm/kvm/arm.c|  22 ---
  arch/arm64/include/asm/kvm_host.h |   3 +
  include/uapi/linux/kvm.h  |  14 +
  virt/kvm/arm/arch_timer.c | 125 
 +++---
  6 files changed, 155 insertions(+), 42 deletions(-)

 diff --git a/Documentation/virtual/kvm/api.txt 
 b/Documentation/virtual/kvm/api.txt
 index 23937e0..1c0bd86 100644
 --- a/Documentation/virtual/kvm/api.txt
 +++ b/Documentation/virtual/kvm/api.txt
 @@ -3202,9 +3202,14 @@ struct kvm_run {
/* in */
__u8 request_interrupt_window;
  
 -Request that KVM_RUN return when it becomes possible to inject external
 +[x86] Request that KVM_RUN return when it becomes possible to inject 
 external
  interrupts into the guest.  Useful in conjunction with KVM_INTERRUPT.
  
 +[arm*] Bits set to 1 in here mask IRQ lines that would otherwise 
 potentially
 +trigger forever. These lines are available:
 +
 +KVM_IRQWINDOW_VTIMER  -  Masks hw virtual timer irq while in guest
 +
__u8 padding1[7];
  
/* out */
 @@ -3519,6 +3524,18 @@ Hyper-V SynIC state change. Notification is used to 
 remap SynIC
  event/message pages and to enable/disable SynIC messages/events processing
  in userspace.
  
 +  /* KVM_EXIT_ARM_TIMER */
 +  struct {
 +  __u8 timesource;
 +  } arm_timer;
 +
 +Indicates that a timer triggered that user space needs to handle and
 +potentially mask with vcpu->run->request_interrupt_window to allow the
 +guest to proceed. This only happens for timers that got enabled through
 +KVM_CAP_ARM_TIMER. The following time sources are available:
 +
 +KVM_ARM_TIMER_VTIMER  - virtual cpu timer
 +
/* Fix the size of the union. */
char padding[256];
};
 @@ -3739,6 +3756,17 @@ Once this is done the KVM_REG_MIPS_VEC_* and 
 KVM_REG_MIPS_MSA_* registers can be
  accessed, and the Config5.MSAEn bit is accessible via the KVM API and 
 also from
  the guest.
  
 +6.11 KVM_CAP_ARM_TIMER
 +
 +Architectures: arm, arm64
 +Target: vcpu
 +Parameters: args[0] contains a bitmap of timers to select (see 5.)
 +
 +This capability allows to route per-core timers into user space. When it's
 +enabled and no in-kernel interrupt controller is in use, the timers 
 selected
 +by args[0] trigger KVM_EXIT_ARM_TIMER guest exits when they are pending,
 +unless masked by vcpu->run->request_interrupt_window (see 5.).
 +
  7. Capabilities that can be enabled on VMs
  --
  
 diff --git a/arch/arm/include/asm/kvm_host.h 
 b/arch/arm/include/asm/kvm_host.h
 index de338d9..77d1f73 100644
 --- a/arch/arm/include/asm/kvm_host.h
 +++ b/arch/arm/include/asm/kvm_host.h
 @@ -180,6 +180,9 @@ struct kvm_vcpu_arch {
  
/* Detect first run of a vcpu */
bool has_run_once;
 +
 +  /* User space wants timer notifications */
 +  bool user_space_arm_timers;
>>>
>>> Please move this to the timer structure.
>>
>> Sure.
>>
>>>
  };
  
  struct kvm_vm_stat {
 diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
 index c84b6ad..57bdb71 100644
 --- a/arch/arm/kvm/arm.c
 +++ b/arch/arm/kvm/arm.c
 @@ -187,6 +187,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long 
 ext)
case KVM_CAP_ARM_PSCI_0_2:
case KVM_CAP_READONLY_MEM:
case KVM_CAP_MP_STATE:
 +  case KVM_CAP_ARM_TIMER:
r = 1;
break;
case KVM_CAP_COALESCED_MMIO:
 @@ -474,13 +475,7 @

Re: [PATCH v4 2/2] KVM: arm/arm64: Route vtimer events to user space

2016-09-20 Thread Marc Zyngier
On 19/09/16 18:39, Alexander Graf wrote:
> 
> 
> On 19.09.16 16:48, Marc Zyngier wrote:
>> On 19/09/16 12:14, Alexander Graf wrote:
>>> We have 2 modes for dealing with interrupts in the ARM world. We can either
>>> handle them all using hardware acceleration through the vgic or we can 
>>> emulate
>>> a gic in user space and only drive CPU IRQ pins from there.
>>>
>>> Unfortunately, when driving IRQs from user space, we never tell user space
>>> about timer events that may result in interrupt line state changes, so we
>>> lose out on timer events if we run with user space gic emulation.
>>>
>>> This patch fixes that by routing vtimer expiration events to user space.
>>> With this patch I can successfully run edk2 and Linux with user space gic
>>> emulation.
>>>
>>> Signed-off-by: Alexander Graf 
>>>
>>> ---
>>>
>>> v1 -> v2:
>>>
>>>   - Add back curly brace that got lost
>>>
>>> v2 -> v3:
>>>
>>>   - Split into patch set
>>>
>>> v3 -> v4:
>>>
>>>   - Improve documentation
>>> ---
>>>  Documentation/virtual/kvm/api.txt |  30 -
>>>  arch/arm/include/asm/kvm_host.h   |   3 +
>>>  arch/arm/kvm/arm.c|  22 ---
>>>  arch/arm64/include/asm/kvm_host.h |   3 +
>>>  include/uapi/linux/kvm.h  |  14 +
>>>  virt/kvm/arm/arch_timer.c | 125 
>>> +++---
>>>  6 files changed, 155 insertions(+), 42 deletions(-)
>>>
>>> diff --git a/Documentation/virtual/kvm/api.txt 
>>> b/Documentation/virtual/kvm/api.txt
>>> index 23937e0..1c0bd86 100644
>>> --- a/Documentation/virtual/kvm/api.txt
>>> +++ b/Documentation/virtual/kvm/api.txt
>>> @@ -3202,9 +3202,14 @@ struct kvm_run {
>>> /* in */
>>> __u8 request_interrupt_window;
>>>  
>>> -Request that KVM_RUN return when it becomes possible to inject external
>>> +[x86] Request that KVM_RUN return when it becomes possible to inject 
>>> external
>>>  interrupts into the guest.  Useful in conjunction with KVM_INTERRUPT.
>>>  
>>> +[arm*] Bits set to 1 in here mask IRQ lines that would otherwise 
>>> potentially
>>> +trigger forever. These lines are available:
>>> +
>>> +KVM_IRQWINDOW_VTIMER  -  Masks hw virtual timer irq while in guest
>>> +
>>> __u8 padding1[7];
>>>  
>>> /* out */
>>> @@ -3519,6 +3524,18 @@ Hyper-V SynIC state change. Notification is used to 
>>> remap SynIC
>>>  event/message pages and to enable/disable SynIC messages/events processing
>>>  in userspace.
>>>  
>>> +   /* KVM_EXIT_ARM_TIMER */
>>> +   struct {
>>> +   __u8 timesource;
>>> +   } arm_timer;
>>> +
>>> +Indicates that a timer triggered that user space needs to handle and
>>> +potentially mask with vcpu->run->request_interrupt_window to allow the
>>> +guest to proceed. This only happens for timers that got enabled through
>>> +KVM_CAP_ARM_TIMER. The following time sources are available:
>>> +
>>> +KVM_ARM_TIMER_VTIMER  - virtual cpu timer
>>> +
>>> /* Fix the size of the union. */
>>> char padding[256];
>>> };
>>> @@ -3739,6 +3756,17 @@ Once this is done the KVM_REG_MIPS_VEC_* and 
>>> KVM_REG_MIPS_MSA_* registers can be
>>>  accessed, and the Config5.MSAEn bit is accessible via the KVM API and also 
>>> from
>>>  the guest.
>>>  
>>> +6.11 KVM_CAP_ARM_TIMER
>>> +
>>> +Architectures: arm, arm64
>>> +Target: vcpu
>>> +Parameters: args[0] contains a bitmap of timers to select (see 5.)
>>> +
>>> +This capability allows to route per-core timers into user space. When it's
>>> +enabled and no in-kernel interrupt controller is in use, the timers 
>>> selected
>>> +by args[0] trigger KVM_EXIT_ARM_TIMER guest exits when they are pending,
>>> +unless masked by vcpu->run->request_interrupt_window (see 5.).
>>> +
>>>  7. Capabilities that can be enabled on VMs
>>>  --
>>>  
>>> diff --git a/arch/arm/include/asm/kvm_host.h 
>>> b/arch/arm/include/asm/kvm_host.h
>>> index de338d9..77d1f73 100644
>>> --- a/arch/arm/include/asm/kvm_host.h
>>> +++ b/arch/arm/include/asm/kvm_host.h
>>> @@ -180,6 +180,9 @@ struct kvm_vcpu_arch {
>>>  
>>> /* Detect first run of a vcpu */
>>> bool has_run_once;
>>> +
>>> +   /* User space wants timer notifications */
>>> +   bool user_space_arm_timers;
>>
>> Please move this to the timer structure.
> 
> Sure.
> 
>>
>>>  };
>>>  
>>>  struct kvm_vm_stat {
>>> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
>>> index c84b6ad..57bdb71 100644
>>> --- a/arch/arm/kvm/arm.c
>>> +++ b/arch/arm/kvm/arm.c
>>> @@ -187,6 +187,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long 
>>> ext)
>>> case KVM_CAP_ARM_PSCI_0_2:
>>> case KVM_CAP_READONLY_MEM:
>>> case KVM_CAP_MP_STATE:
>>> +   case KVM_CAP_ARM_TIMER:
>>> r = 1;
>>> break;
>>> case KVM_CAP_COALESCED_MMIO:
>>> @@ -474,13 +475,7 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu 
>>> *vcpu)
>>> return ret;
>>> }
>>>  
>>> -   /*
>>> -* Enable the arch tim

Re: [PATCH v4 2/2] KVM: arm/arm64: Route vtimer events to user space

2016-09-19 Thread Alexander Graf


On 19.09.16 16:48, Marc Zyngier wrote:
> On 19/09/16 12:14, Alexander Graf wrote:
>> We have 2 modes for dealing with interrupts in the ARM world. We can either
>> handle them all using hardware acceleration through the vgic or we can 
>> emulate
>> a gic in user space and only drive CPU IRQ pins from there.
>>
>> Unfortunately, when driving IRQs from user space, we never tell user space
>> about timer events that may result in interrupt line state changes, so we
>> lose out on timer events if we run with user space gic emulation.
>>
>> This patch fixes that by routing vtimer expiration events to user space.
>> With this patch I can successfully run edk2 and Linux with user space gic
>> emulation.
>>
>> Signed-off-by: Alexander Graf 
>>
>> ---
>>
>> v1 -> v2:
>>
>>   - Add back curly brace that got lost
>>
>> v2 -> v3:
>>
>>   - Split into patch set
>>
>> v3 -> v4:
>>
>>   - Improve documentation
>> ---
>>  Documentation/virtual/kvm/api.txt |  30 -
>>  arch/arm/include/asm/kvm_host.h   |   3 +
>>  arch/arm/kvm/arm.c|  22 ---
>>  arch/arm64/include/asm/kvm_host.h |   3 +
>>  include/uapi/linux/kvm.h  |  14 +
>>  virt/kvm/arm/arch_timer.c | 125 
>> +++---
>>  6 files changed, 155 insertions(+), 42 deletions(-)
>>
>> diff --git a/Documentation/virtual/kvm/api.txt 
>> b/Documentation/virtual/kvm/api.txt
>> index 23937e0..1c0bd86 100644
>> --- a/Documentation/virtual/kvm/api.txt
>> +++ b/Documentation/virtual/kvm/api.txt
>> @@ -3202,9 +3202,14 @@ struct kvm_run {
>>  /* in */
>>  __u8 request_interrupt_window;
>>  
>> -Request that KVM_RUN return when it becomes possible to inject external
>> +[x86] Request that KVM_RUN return when it becomes possible to inject 
>> external
>>  interrupts into the guest.  Useful in conjunction with KVM_INTERRUPT.
>>  
>> +[arm*] Bits set to 1 in here mask IRQ lines that would otherwise potentially
>> +trigger forever. These lines are available:
>> +
>> +KVM_IRQWINDOW_VTIMER  -  Masks hw virtual timer irq while in guest
>> +
>>  __u8 padding1[7];
>>  
>>  /* out */
>> @@ -3519,6 +3524,18 @@ Hyper-V SynIC state change. Notification is used to 
>> remap SynIC
>>  event/message pages and to enable/disable SynIC messages/events processing
>>  in userspace.
>>  
>> +/* KVM_EXIT_ARM_TIMER */
>> +struct {
>> +__u8 timesource;
>> +} arm_timer;
>> +
>> +Indicates that a timer triggered that user space needs to handle and
>> +potentially mask with vcpu->run->request_interrupt_window to allow the
>> +guest to proceed. This only happens for timers that got enabled through
>> +KVM_CAP_ARM_TIMER. The following time sources are available:
>> +
>> +KVM_ARM_TIMER_VTIMER  - virtual cpu timer
>> +
>>  /* Fix the size of the union. */
>>  char padding[256];
>>  };
>> @@ -3739,6 +3756,17 @@ Once this is done the KVM_REG_MIPS_VEC_* and 
>> KVM_REG_MIPS_MSA_* registers can be
>>  accessed, and the Config5.MSAEn bit is accessible via the KVM API and also 
>> from
>>  the guest.
>>  
>> +6.11 KVM_CAP_ARM_TIMER
>> +
>> +Architectures: arm, arm64
>> +Target: vcpu
>> +Parameters: args[0] contains a bitmap of timers to select (see 5.)
>> +
>> +This capability allows to route per-core timers into user space. When it's
>> +enabled and no in-kernel interrupt controller is in use, the timers selected
>> +by args[0] trigger KVM_EXIT_ARM_TIMER guest exits when they are pending,
>> +unless masked by vcpu->run->request_interrupt_window (see 5.).
>> +
>>  7. Capabilities that can be enabled on VMs
>>  --
>>  
>> diff --git a/arch/arm/include/asm/kvm_host.h 
>> b/arch/arm/include/asm/kvm_host.h
>> index de338d9..77d1f73 100644
>> --- a/arch/arm/include/asm/kvm_host.h
>> +++ b/arch/arm/include/asm/kvm_host.h
>> @@ -180,6 +180,9 @@ struct kvm_vcpu_arch {
>>  
>>  /* Detect first run of a vcpu */
>>  bool has_run_once;
>> +
>> +/* User space wants timer notifications */
>> +bool user_space_arm_timers;
> 
> Please move this to the timer structure.

Sure.

> 
>>  };
>>  
>>  struct kvm_vm_stat {
>> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
>> index c84b6ad..57bdb71 100644
>> --- a/arch/arm/kvm/arm.c
>> +++ b/arch/arm/kvm/arm.c
>> @@ -187,6 +187,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long 
>> ext)
>>  case KVM_CAP_ARM_PSCI_0_2:
>>  case KVM_CAP_READONLY_MEM:
>>  case KVM_CAP_MP_STATE:
>> +case KVM_CAP_ARM_TIMER:
>>  r = 1;
>>  break;
>>  case KVM_CAP_COALESCED_MMIO:
>> @@ -474,13 +475,7 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu 
>> *vcpu)
>>  return ret;
>>  }
>>  
>> -/*
>> - * Enable the arch timers only if we have an in-kernel VGIC
>> - * and it has been properly initialized, since we cannot handle
>> - * interrupts from the virtual timer with a userspace gi

Re: [PATCH v4 2/2] KVM: arm/arm64: Route vtimer events to user space

2016-09-19 Thread Marc Zyngier
On 19/09/16 12:14, Alexander Graf wrote:
> We have 2 modes for dealing with interrupts in the ARM world. We can either
> handle them all using hardware acceleration through the vgic or we can emulate
> a gic in user space and only drive CPU IRQ pins from there.
> 
> Unfortunately, when driving IRQs from user space, we never tell user space
> about timer events that may result in interrupt line state changes, so we
> lose out on timer events if we run with user space gic emulation.
> 
> This patch fixes that by routing vtimer expiration events to user space.
> With this patch I can successfully run edk2 and Linux with user space gic
> emulation.
> 
> Signed-off-by: Alexander Graf 
> 
> ---
> 
> v1 -> v2:
> 
>   - Add back curly brace that got lost
> 
> v2 -> v3:
> 
>   - Split into patch set
> 
> v3 -> v4:
> 
>   - Improve documentation
> ---
>  Documentation/virtual/kvm/api.txt |  30 -
>  arch/arm/include/asm/kvm_host.h   |   3 +
>  arch/arm/kvm/arm.c|  22 ---
>  arch/arm64/include/asm/kvm_host.h |   3 +
>  include/uapi/linux/kvm.h  |  14 +
>  virt/kvm/arm/arch_timer.c | 125 
> +++---
>  6 files changed, 155 insertions(+), 42 deletions(-)
> 
> diff --git a/Documentation/virtual/kvm/api.txt 
> b/Documentation/virtual/kvm/api.txt
> index 23937e0..1c0bd86 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -3202,9 +3202,14 @@ struct kvm_run {
>   /* in */
>   __u8 request_interrupt_window;
>  
> -Request that KVM_RUN return when it becomes possible to inject external
> +[x86] Request that KVM_RUN return when it becomes possible to inject external
>  interrupts into the guest.  Useful in conjunction with KVM_INTERRUPT.
>  
> +[arm*] Bits set to 1 in here mask IRQ lines that would otherwise potentially
> +trigger forever. These lines are available:
> +
> +KVM_IRQWINDOW_VTIMER  -  Masks hw virtual timer irq while in guest
> +
>   __u8 padding1[7];
>  
>   /* out */
> @@ -3519,6 +3524,18 @@ Hyper-V SynIC state change. Notification is used to 
> remap SynIC
>  event/message pages and to enable/disable SynIC messages/events processing
>  in userspace.
>  
> + /* KVM_EXIT_ARM_TIMER */
> + struct {
> + __u8 timesource;
> + } arm_timer;
> +
> +Indicates that a timer triggered that user space needs to handle and
> +potentially mask with vcpu->run->request_interrupt_window to allow the
> +guest to proceed. This only happens for timers that got enabled through
> +KVM_CAP_ARM_TIMER. The following time sources are available:
> +
> +KVM_ARM_TIMER_VTIMER  - virtual cpu timer
> +
>   /* Fix the size of the union. */
>   char padding[256];
>   };
> @@ -3739,6 +3756,17 @@ Once this is done the KVM_REG_MIPS_VEC_* and 
> KVM_REG_MIPS_MSA_* registers can be
>  accessed, and the Config5.MSAEn bit is accessible via the KVM API and also 
> from
>  the guest.
>  
> +6.11 KVM_CAP_ARM_TIMER
> +
> +Architectures: arm, arm64
> +Target: vcpu
> +Parameters: args[0] contains a bitmap of timers to select (see 5.)
> +
> +This capability allows to route per-core timers into user space. When it's
> +enabled and no in-kernel interrupt controller is in use, the timers selected
> +by args[0] trigger KVM_EXIT_ARM_TIMER guest exits when they are pending,
> +unless masked by vcpu->run->request_interrupt_window (see 5.).
> +
>  7. Capabilities that can be enabled on VMs
>  --
>  
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index de338d9..77d1f73 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -180,6 +180,9 @@ struct kvm_vcpu_arch {
>  
>   /* Detect first run of a vcpu */
>   bool has_run_once;
> +
> + /* User space wants timer notifications */
> + bool user_space_arm_timers;

Please move this to the timer structure.

>  };
>  
>  struct kvm_vm_stat {
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index c84b6ad..57bdb71 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -187,6 +187,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long 
> ext)
>   case KVM_CAP_ARM_PSCI_0_2:
>   case KVM_CAP_READONLY_MEM:
>   case KVM_CAP_MP_STATE:
> + case KVM_CAP_ARM_TIMER:
>   r = 1;
>   break;
>   case KVM_CAP_COALESCED_MMIO:
> @@ -474,13 +475,7 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu)
>   return ret;
>   }
>  
> - /*
> -  * Enable the arch timers only if we have an in-kernel VGIC
> -  * and it has been properly initialized, since we cannot handle
> -  * interrupts from the virtual timer with a userspace gic.
> -  */
> - if (irqchip_in_kernel(kvm) && vgic_initialized(kvm))
> - ret = kvm_timer_enable(vcpu);
> + ret = kvm_timer_enable(vcpu);
>  
>