* [PATCH v2 1/5] KVM: arm/arm64: Cleanup the arch timer code's irqchip checking
2017-02-03 14:56 [PATCH v2 0/5] Support userspace irqchip with arch timers Christoffer Dall
@ 2017-02-03 14:56 ` Christoffer Dall
2017-02-03 14:56 ` [PATCH v2 2/5] KVM: arm/arm64: Add ARM user space interrupt signaling ABI Christoffer Dall
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Christoffer Dall @ 2017-02-03 14:56 UTC (permalink / raw)
To: linux-arm-kernel
From: Christoffer Dall <christoffer.dall@linaro.org>
Currently we check if we have an in-kernel irqchip and if the vgic was
properly implemented several places in the arch timer code. But, we
already predicate our enablement of the arm timers on having a valid
and initialized gic, so we can simply check if the timers are enabled or
not.
This also gets rid of the ugly "error that's not an error but used to
signal that the timer shouldn't poke the gic" construct we have.
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
virt/kvm/arm/arch_timer.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
index 35d7100..5099b30 100644
--- a/virt/kvm/arm/arch_timer.c
+++ b/virt/kvm/arm/arch_timer.c
@@ -189,8 +189,6 @@ static void kvm_timer_update_irq(struct kvm_vcpu *vcpu, bool new_level,
{
int ret;
- BUG_ON(!vgic_initialized(vcpu->kvm));
-
timer_ctx->active_cleared_last = false;
timer_ctx->irq.level = new_level;
trace_kvm_timer_update_irq(vcpu->vcpu_id, timer_ctx->irq.irq,
@@ -205,7 +203,7 @@ static void kvm_timer_update_irq(struct kvm_vcpu *vcpu, bool new_level,
* Check if there was a change in the timer state (should we raise or lower
* the line level to the GIC).
*/
-static int kvm_timer_update_state(struct kvm_vcpu *vcpu)
+static void kvm_timer_update_state(struct kvm_vcpu *vcpu)
{
struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
@@ -217,16 +215,14 @@ static int kvm_timer_update_state(struct kvm_vcpu *vcpu)
* because the guest would never see the interrupt. Instead wait
* until we call this function from kvm_timer_flush_hwstate.
*/
- if (!vgic_initialized(vcpu->kvm) || !timer->enabled)
- return -ENODEV;
+ if (!timer->enabled)
+ return;
if (kvm_timer_should_fire(vtimer) != vtimer->irq.level)
kvm_timer_update_irq(vcpu, !vtimer->irq.level, vtimer);
if (kvm_timer_should_fire(ptimer) != ptimer->irq.level)
kvm_timer_update_irq(vcpu, !ptimer->irq.level, ptimer);
-
- return 0;
}
/* Schedule the background timer for the emulated timer. */
@@ -299,9 +295,11 @@ void kvm_timer_flush_hwstate(struct kvm_vcpu *vcpu)
bool phys_active;
int ret;
- if (kvm_timer_update_state(vcpu))
+ if (unlikely(!timer->enabled))
return;
+ kvm_timer_update_state(vcpu);
+
/* Set the background timer for the physical timer emulation. */
kvm_timer_emulate(vcpu, vcpu_ptimer(vcpu));
--
2.9.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 2/5] KVM: arm/arm64: Add ARM user space interrupt signaling ABI
2017-02-03 14:56 [PATCH v2 0/5] Support userspace irqchip with arch timers Christoffer Dall
2017-02-03 14:56 ` [PATCH v2 1/5] KVM: arm/arm64: Cleanup the arch timer code's irqchip checking Christoffer Dall
@ 2017-02-03 14:56 ` Christoffer Dall
2017-02-03 17:51 ` Peter Maydell
2017-02-03 14:56 ` [PATCH v2 3/5] KVM: arm/arm64: Support arch timers with a userspace gic Christoffer Dall
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Christoffer Dall @ 2017-02-03 14:56 UTC (permalink / raw)
To: linux-arm-kernel
From: Christoffer Dall <christoffer.dall@linaro.org>
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 events from devices emulated inside the kernel, which may
result in interrupt line state changes, so we lose out on for example
timer and PMU events if we run with user space gic emulation.
Define an ABI to publish such device output levels to userspace.
Signed-off-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
Documentation/virtual/kvm/api.txt | 41 +++++++++++++++++++++++++++++++++++++++
arch/arm/include/uapi/asm/kvm.h | 2 ++
arch/arm64/include/uapi/asm/kvm.h | 2 ++
include/uapi/linux/kvm.h | 8 ++++++++
4 files changed, 53 insertions(+)
diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 03145b7..76a5ebc 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -3942,3 +3942,44 @@ In order to use SynIC, it has to be activated by setting this
capability via KVM_ENABLE_CAP ioctl on the vcpu fd. Note that this
will disable the use of APIC hardware virtualization even if supported
by the CPU, as it's incompatible with SynIC auto-EOI behavior.
+
+8.3 KVM_CAP_ARM_USER_IRQ
+
+Architectures: arm, arm64
+This capability, if KVM_CHECK_EXTENSION indicates that it is available, means
+that if userspace creates a VM without an in-kernel interrupt controller, it
+will be notified of changes to the output level of in-kernel emulated devices,
+which can generate virtual interrupts, presented to the VM.
+For such VMs, on every return to userspace, the kernel
+updates the vcpu's run->s.regs.device_irq_level field to represent the actual
+output level of the device.
+
+Whenever kvm detects a change in the device output level, kvm guarantees at
+least one return to userspace before running the VM. This exit could either
+be a KVM_EXIT_INTR or any other exit event, like KVM_EXIT_MMIO. This way,
+userspace can always sample the device output level and re-compute the state of
+the userspace interrupt controller. Userspace should always check the state
+of run->s.regs.device_irq_level on every kvm exit.
+The value in run->s.regs.device_irq_level can represent both level and edge
+triggered interrupt signals, depending on the device. Edge triggered interrupt
+signals will exit to userspace with the bit in run->s.regs.device_irq_level
+set exactly once per edge signal.
+
+The field run->s.regs.device_irq_level is available independent of
+run->kvm_valid_regs or run->kvm_dirty_regs bits.
+
+If KVM_CAP_ARM_USER_IRQ is supported, the KVM_CHECK_EXTENSION ioctl returns a
+number larger than 0 indicating the version of this capability is implemented
+and thereby which bits in in run->s.regs.device_irq_level can signal values.
+
+Currently the following bits are defined for the device_irq_level bitmap:
+
+ KVM_CAP_ARM_USER_IRQ >= 1:
+
+ KVM_ARM_DEV_EL1_VTIMER - EL1 virtual timer
+ KVM_ARM_DEV_EL1_PTIMER - EL1 physical timer
+ KVM_ARM_DEV_PMU - ARM PMU overflow interrupt signal
+
+Future versions of kvm may implement additional events. These will get
+indicated by returning a higher number from KVM_CHECK_EXTENSION and will be
+listed above.
diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
index 6ebd3e6..c8af4d8 100644
--- a/arch/arm/include/uapi/asm/kvm.h
+++ b/arch/arm/include/uapi/asm/kvm.h
@@ -114,6 +114,8 @@ struct kvm_debug_exit_arch {
};
struct kvm_sync_regs {
+ /* Used with KVM_CAP_ARM_USER_IRQ */
+ u64 device_irq_level;
};
struct kvm_arch_memory_slot {
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index c286035..4197f51 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -143,6 +143,8 @@ struct kvm_debug_exit_arch {
#define KVM_GUESTDBG_USE_HW (1 << 17)
struct kvm_sync_regs {
+ /* Used with KVM_CAP_ARM_USER_IRQ */
+ u64 device_irq_level;
};
struct kvm_arch_memory_slot {
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index cac48ed..21e4cb9 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -871,6 +871,7 @@ struct kvm_ppc_smmu_info {
#define KVM_CAP_S390_USER_INSTR0 130
#define KVM_CAP_MSI_DEVID 131
#define KVM_CAP_PPC_HTM 132
+#define KVM_CAP_ARM_USER_IRQ 133
#ifdef KVM_CAP_IRQ_ROUTING
@@ -1335,4 +1336,11 @@ struct kvm_assigned_msix_entry {
#define KVM_X2APIC_API_USE_32BIT_IDS (1ULL << 0)
#define KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK (1ULL << 1)
+/* Available with KVM_CAP_ARM_USER_IRQ */
+
+/* Bits for run->s.regs.device_irq_level */
+#define KVM_ARM_DEV_EL1_VTIMER (1 << 0)
+#define KVM_ARM_DEV_EL1_PTIMER (1 << 1)
+#define KVM_ARM_DEV_PMU (1 << 2)
+
#endif /* __LINUX_KVM_H */
--
2.9.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 2/5] KVM: arm/arm64: Add ARM user space interrupt signaling ABI
2017-02-03 14:56 ` [PATCH v2 2/5] KVM: arm/arm64: Add ARM user space interrupt signaling ABI Christoffer Dall
@ 2017-02-03 17:51 ` Peter Maydell
2017-02-21 11:41 ` Christoffer Dall
0 siblings, 1 reply; 10+ messages in thread
From: Peter Maydell @ 2017-02-03 17:51 UTC (permalink / raw)
To: linux-arm-kernel
On 3 February 2017 at 14:56, Christoffer Dall <cdall@linaro.org> wrote:
> From: Christoffer Dall <christoffer.dall@linaro.org>
>
> 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 events from devices emulated inside the kernel, which may
> result in interrupt line state changes, so we lose out on for example
> timer and PMU events if we run with user space gic emulation.
>
> Define an ABI to publish such device output levels to userspace.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
Acked-by: Peter Maydell <peter.maydell@linaro.org>
for the userspace ABI/docs part.
thanks
-- PMM
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/5] KVM: arm/arm64: Add ARM user space interrupt signaling ABI
2017-02-03 17:51 ` Peter Maydell
@ 2017-02-21 11:41 ` Christoffer Dall
2017-04-05 0:14 ` Alexander Graf
0 siblings, 1 reply; 10+ messages in thread
From: Christoffer Dall @ 2017-02-21 11:41 UTC (permalink / raw)
To: linux-arm-kernel
Hi Alex,
On Fri, Feb 03, 2017 at 05:51:18PM +0000, Peter Maydell wrote:
> On 3 February 2017 at 14:56, Christoffer Dall <cdall@linaro.org> wrote:
> > From: Christoffer Dall <christoffer.dall@linaro.org>
> >
> > 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 events from devices emulated inside the kernel, which may
> > result in interrupt line state changes, so we lose out on for example
> > timer and PMU events if we run with user space gic emulation.
> >
> > Define an ABI to publish such device output levels to userspace.
> >
> > Signed-off-by: Alexander Graf <agraf@suse.de>
> > Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
>
>
> Acked-by: Peter Maydell <peter.maydell@linaro.org>
>
> for the userspace ABI/docs part.
>
Given Peter's ack on this ABI, any chance you could take a look at
fixing the userspace SMP issue and respinning the userspace patches for
this series?
Thanks,
-Christoffer
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/5] KVM: arm/arm64: Add ARM user space interrupt signaling ABI
2017-02-21 11:41 ` Christoffer Dall
@ 2017-04-05 0:14 ` Alexander Graf
2017-04-05 8:12 ` Christoffer Dall
0 siblings, 1 reply; 10+ messages in thread
From: Alexander Graf @ 2017-04-05 0:14 UTC (permalink / raw)
To: linux-arm-kernel
On 21.02.17 12:41, Christoffer Dall wrote:
> Hi Alex,
>
> On Fri, Feb 03, 2017 at 05:51:18PM +0000, Peter Maydell wrote:
>> On 3 February 2017 at 14:56, Christoffer Dall <cdall@linaro.org> wrote:
>>> From: Christoffer Dall <christoffer.dall@linaro.org>
>>>
>>> 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 events from devices emulated inside the kernel, which may
>>> result in interrupt line state changes, so we lose out on for example
>>> timer and PMU events if we run with user space gic emulation.
>>>
>>> Define an ABI to publish such device output levels to userspace.
>>>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
>>
>>
>> Acked-by: Peter Maydell <peter.maydell@linaro.org>
>>
>> for the userspace ABI/docs part.
>>
>
> Given Peter's ack on this ABI, any chance you could take a look at
> fixing the userspace SMP issue and respinning the userspace patches for
> this series?
The problem with user space SMP was simply a missing lock:
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index a66845f..1b33895 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -548,10 +548,14 @@ MemTxAttrs kvm_arch_post_run(CPUState *cs, struct
kvm_run *run)
/* Synchronize our internal vtimer irq line with the kvm one */
if (run->s.regs.device_irq_level != cpu->device_irq_level) {
- qemu_set_irq(ARM_CPU(cs)->gt_timer_outputs[GTIMER_VIRT],
+ qemu_mutex_lock_iothread();
+ qemu_set_irq(cpu->gt_timer_outputs[GTIMER_VIRT],
run->s.regs.device_irq_level &
KVM_ARM_DEV_EL1_VTIMER);
+ qemu_set_irq(cpu->gt_timer_outputs[GTIMER_PHYS],
+ run->s.regs.device_irq_level &
KVM_ARM_DEV_EL1_PTIMER);
/* TODO: Handle changes in PTIMER and PMU as well */
cpu->device_irq_level = run->s.regs.device_irq_level;
+ qemu_mutex_unlock_iothread();
}
return MEMTXATTRS_UNSPECIFIED;
----
I also wasn't able to use your series out of the box. There are several
places in the code that check whether a timer is enabled (for register
save/restore for example) and without vgic we never ended up setting
that to true.
I don't know how safe it is to set it to true regardless. It feels to me
like someone has to put more thought into how to switch from an
initialized user space timer state to a vgic one. For reference, my test
patch is below:
diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
index 891a725..56a5d51 100644
--- a/virt/kvm/arm/arch_timer.c
+++ b/virt/kvm/arm/arch_timer.c
@@ -629,8 +629,11 @@ int kvm_timer_enable(struct kvm_vcpu *vcpu)
return 0;
/* Without a VGIC we do not map virtual IRQs to physical IRQs */
- if (!irqchip_in_kernel(vcpu->kvm))
+ if (!irqchip_in_kernel(vcpu->kvm)) {
+ /* Enable timer for now, may be racy? */
+ timer->enabled = 1;
return 0;
+ }
if (!vgic_initialized(vcpu->kvm))
return -ENODEV;
----
Once we solved that piece, I'll happily respin my user space patch.
Alex
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 2/5] KVM: arm/arm64: Add ARM user space interrupt signaling ABI
2017-04-05 0:14 ` Alexander Graf
@ 2017-04-05 8:12 ` Christoffer Dall
0 siblings, 0 replies; 10+ messages in thread
From: Christoffer Dall @ 2017-04-05 8:12 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Apr 05, 2017 at 02:14:11AM +0200, Alexander Graf wrote:
>
>
> On 21.02.17 12:41, Christoffer Dall wrote:
> >Hi Alex,
> >
> >On Fri, Feb 03, 2017 at 05:51:18PM +0000, Peter Maydell wrote:
> >>On 3 February 2017 at 14:56, Christoffer Dall <cdall@linaro.org> wrote:
> >>>From: Christoffer Dall <christoffer.dall@linaro.org>
> >>>
> >>>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 events from devices emulated inside the kernel, which may
> >>>result in interrupt line state changes, so we lose out on for example
> >>>timer and PMU events if we run with user space gic emulation.
> >>>
> >>>Define an ABI to publish such device output levels to userspace.
> >>>
> >>>Signed-off-by: Alexander Graf <agraf@suse.de>
> >>>Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> >>
> >>
> >>Acked-by: Peter Maydell <peter.maydell@linaro.org>
> >>
> >>for the userspace ABI/docs part.
> >>
> >
> >Given Peter's ack on this ABI, any chance you could take a look at
> >fixing the userspace SMP issue and respinning the userspace patches for
> >this series?
>
> The problem with user space SMP was simply a missing lock:
>
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index a66845f..1b33895 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -548,10 +548,14 @@ MemTxAttrs kvm_arch_post_run(CPUState *cs,
> struct kvm_run *run)
>
> /* Synchronize our internal vtimer irq line with the kvm one */
> if (run->s.regs.device_irq_level != cpu->device_irq_level) {
> - qemu_set_irq(ARM_CPU(cs)->gt_timer_outputs[GTIMER_VIRT],
> + qemu_mutex_lock_iothread();
> + qemu_set_irq(cpu->gt_timer_outputs[GTIMER_VIRT],
> run->s.regs.device_irq_level &
> KVM_ARM_DEV_EL1_VTIMER);
> + qemu_set_irq(cpu->gt_timer_outputs[GTIMER_PHYS],
> + run->s.regs.device_irq_level &
> KVM_ARM_DEV_EL1_PTIMER);
> /* TODO: Handle changes in PTIMER and PMU as well */
> cpu->device_irq_level = run->s.regs.device_irq_level;
> + qemu_mutex_unlock_iothread();
> }
>
> return MEMTXATTRS_UNSPECIFIED;
>
> ----
>
>
> I also wasn't able to use your series out of the box. There are
> several places in the code that check whether a timer is enabled
> (for register save/restore for example) and without vgic we never
> ended up setting that to true.
>
> I don't know how safe it is to set it to true regardless. It feels
> to me like someone has to put more thought into how to switch from
> an initialized user space timer state to a vgic one. For reference,
> my test patch is below:
>
> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> index 891a725..56a5d51 100644
> --- a/virt/kvm/arm/arch_timer.c
> +++ b/virt/kvm/arm/arch_timer.c
> @@ -629,8 +629,11 @@ int kvm_timer_enable(struct kvm_vcpu *vcpu)
> return 0;
>
> /* Without a VGIC we do not map virtual IRQs to physical IRQs */
> - if (!irqchip_in_kernel(vcpu->kvm))
> + if (!irqchip_in_kernel(vcpu->kvm)) {
> + /* Enable timer for now, may be racy? */
> + timer->enabled = 1;
> return 0;
> + }
>
> if (!vgic_initialized(vcpu->kvm))
> return -ENODEV;
>
Heh, sorry about this, I accidentally pushed the wrong content to my git
repo, so I'm guessing you pulled from there instead of applying these
patches directly. If you want a branch with these patches as intended,
try this:
git://git.kernel.org/pub/scm/linux/kernel/git/cdall/linux.git irqs-to-user-v2-for-real
I have a v3 ready based on v4.11-rc1 plus other KVM fixes and I'll send
that out later today once I've tested with your qemu fix above.
Thanks,
-Christoffer
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 3/5] KVM: arm/arm64: Support arch timers with a userspace gic
2017-02-03 14:56 [PATCH v2 0/5] Support userspace irqchip with arch timers Christoffer Dall
2017-02-03 14:56 ` [PATCH v2 1/5] KVM: arm/arm64: Cleanup the arch timer code's irqchip checking Christoffer Dall
2017-02-03 14:56 ` [PATCH v2 2/5] KVM: arm/arm64: Add ARM user space interrupt signaling ABI Christoffer Dall
@ 2017-02-03 14:56 ` Christoffer Dall
2017-02-03 14:56 ` [PATCH v2 4/5] KVM: arm/arm64: Report PMU overflow interrupts to userspace irqchip Christoffer Dall
2017-02-03 14:56 ` [PATCH v2 5/5] KVM: arm/arm64: Advertise support for KVM_CAP_ARM_USER_IRQ Christoffer Dall
4 siblings, 0 replies; 10+ messages in thread
From: Christoffer Dall @ 2017-02-03 14:56 UTC (permalink / raw)
To: linux-arm-kernel
If you're running with a userspace gic or other interrupt constroller
(that is no vgic in the kernel), then you have so far not been able to
use the architected timers, because the output of the architected
timers, which are driven inside the kernel, was a kernel-only construct
between the arch timer code and the vgic.
This patch implements the new KVM_CAP_ARM_USER_IRQ feature, where we use a
side channel on the kvm_run structure, run->s.regs.device_irq_level, to
always notify userspace of the timer output levels when using a userspace
irqchip.
This works by ensureing that before we enter the guest, if the timer
output level has changed compared to what we last told userspace, we
don't enter the guest, but instead return to userspace to notify it of
the new level. If we are exiting, because of an MMIO for example, and
the level changed at the same time, the value is also updated and
userspace can sample the line as it needs. This is nicely achieved
simply always updating the timer_irq_level field after the main run
loop.
Note that the kvm_timer_update_irq trace event is changed to show the
host IRQ number for the timer instead of the guest IRQ number, because
the kernel no longer know which IRQ userspace wires up the timer signal
to.
Also note that this patch implements all required functionality but does
not yet advertise the capability.
Signed-off-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
arch/arm/kvm/arm.c | 18 +++----
include/kvm/arm_arch_timer.h | 2 +
virt/kvm/arm/arch_timer.c | 121 +++++++++++++++++++++++++++++++++++--------
3 files changed, 110 insertions(+), 31 deletions(-)
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 21c493a..505f928 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -513,13 +513,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);
return ret;
}
@@ -633,9 +627,12 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
local_irq_disable();
/*
- * Re-check atomic conditions
+ * If we have a singal pending, or need to notify a userspace
+ * irqchip about timer level changes, then we exit (and update
+ * the timer level state in kvm_timer_update_run below).
*/
- if (signal_pending(current)) {
+ if (signal_pending(current) ||
+ kvm_timer_should_notify_user(vcpu)) {
ret = -EINTR;
run->exit_reason = KVM_EXIT_INTR;
}
@@ -707,6 +704,9 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
ret = handle_exit(vcpu, run, ret);
}
+ /* Tell userspace about in-kernel device output levels */
+ kvm_timer_update_run(vcpu);
+
if (vcpu->sigset_active)
sigprocmask(SIG_SETMASK, &sigsaved, NULL);
return ret;
diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
index fe797d6..295584f 100644
--- a/include/kvm/arm_arch_timer.h
+++ b/include/kvm/arm_arch_timer.h
@@ -63,6 +63,8 @@ int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
void kvm_timer_vcpu_init(struct kvm_vcpu *vcpu);
void kvm_timer_flush_hwstate(struct kvm_vcpu *vcpu);
void kvm_timer_sync_hwstate(struct kvm_vcpu *vcpu);
+bool kvm_timer_should_notify_user(struct kvm_vcpu *vcpu);
+void kvm_timer_update_run(struct kvm_vcpu *vcpu);
void kvm_timer_vcpu_terminate(struct kvm_vcpu *vcpu);
u64 kvm_arm_timer_get_reg(struct kvm_vcpu *, u64 regid);
diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
index 5099b30..5dc2167 100644
--- a/virt/kvm/arm/arch_timer.c
+++ b/virt/kvm/arm/arch_timer.c
@@ -184,6 +184,27 @@ bool kvm_timer_should_fire(struct arch_timer_context *timer_ctx)
return cval <= now;
}
+/*
+ * Reflect the timer output level into the kvm_run structure
+ */
+void kvm_timer_update_run(struct kvm_vcpu *vcpu)
+{
+ struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
+ struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
+ struct kvm_sync_regs *regs = &vcpu->run->s.regs;
+
+ if (likely(irqchip_in_kernel(vcpu->kvm)))
+ return;
+
+ /* Populate the device bitmap with the timer states */
+ regs->device_irq_level &= ~(KVM_ARM_DEV_EL1_VTIMER |
+ KVM_ARM_DEV_EL1_PTIMER);
+ if (vtimer->irq.level)
+ regs->device_irq_level |= KVM_ARM_DEV_EL1_VTIMER;
+ if (ptimer->irq.level)
+ regs->device_irq_level |= KVM_ARM_DEV_EL1_PTIMER;
+}
+
static void kvm_timer_update_irq(struct kvm_vcpu *vcpu, bool new_level,
struct arch_timer_context *timer_ctx)
{
@@ -194,9 +215,12 @@ static void kvm_timer_update_irq(struct kvm_vcpu *vcpu, bool new_level,
trace_kvm_timer_update_irq(vcpu->vcpu_id, timer_ctx->irq.irq,
timer_ctx->irq.level);
- ret = kvm_vgic_inject_irq(vcpu->kvm, vcpu->vcpu_id, timer_ctx->irq.irq,
- timer_ctx->irq.level);
- WARN_ON(ret);
+ if (likely(irqchip_in_kernel(vcpu->kvm))) {
+ ret = kvm_vgic_inject_irq(vcpu->kvm, vcpu->vcpu_id,
+ timer_ctx->irq.irq,
+ timer_ctx->irq.level);
+ WARN_ON(ret);
+ }
}
/*
@@ -215,7 +239,7 @@ static void kvm_timer_update_state(struct kvm_vcpu *vcpu)
* because the guest would never see the interrupt. Instead wait
* until we call this function from kvm_timer_flush_hwstate.
*/
- if (!timer->enabled)
+ if (unlikely(!timer->enabled))
return;
if (kvm_timer_should_fire(vtimer) != vtimer->irq.level)
@@ -282,27 +306,12 @@ void kvm_timer_unschedule(struct kvm_vcpu *vcpu)
timer_disarm(timer);
}
-/**
- * kvm_timer_flush_hwstate - prepare to move the virt timer to the cpu
- * @vcpu: The vcpu pointer
- *
- * Check if the virtual timer has expired while we were running in the host,
- * and inject an interrupt if that was the case.
- */
-void kvm_timer_flush_hwstate(struct kvm_vcpu *vcpu)
+static void kvm_timer_flush_hwstate_vgic(struct kvm_vcpu *vcpu)
{
struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
bool phys_active;
int ret;
- if (unlikely(!timer->enabled))
- return;
-
- kvm_timer_update_state(vcpu);
-
- /* Set the background timer for the physical timer emulation. */
- kvm_timer_emulate(vcpu, vcpu_ptimer(vcpu));
-
/*
* If we enter the guest with the virtual input level to the VGIC
* asserted, then we have already told the VGIC what we need to, and
@@ -354,11 +363,72 @@ void kvm_timer_flush_hwstate(struct kvm_vcpu *vcpu)
vtimer->active_cleared_last = !phys_active;
}
+bool kvm_timer_should_notify_user(struct kvm_vcpu *vcpu)
+{
+ struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
+ struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
+ struct kvm_sync_regs *sregs = &vcpu->run->s.regs;
+ bool vlevel, plevel;
+
+ if (likely(irqchip_in_kernel(vcpu->kvm)))
+ return false;
+
+ vlevel = sregs->device_irq_level & KVM_ARM_DEV_EL1_VTIMER;
+ plevel = sregs->device_irq_level & KVM_ARM_DEV_EL1_PTIMER;
+
+ return vtimer->irq.level != vlevel ||
+ ptimer->irq.level != plevel;
+}
+
+static void kvm_timer_flush_hwstate_user(struct kvm_vcpu *vcpu)
+{
+ struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
+
+ /*
+ * To prevent continuously exiting from the guest, we mask the
+ * physical interrupt such that the guest can make forward progress.
+ * Once we detect the output level being deasserted, we unmask the
+ * interrupt again so that we exit from the guest when the timer
+ * fires.
+ */
+ if (vtimer->irq.level)
+ disable_percpu_irq(host_vtimer_irq);
+ else
+ enable_percpu_irq(host_vtimer_irq, 0);
+}
+
+/**
+ * kvm_timer_flush_hwstate - prepare timers before running the vcpu
+ * @vcpu: The vcpu pointer
+ *
+ * Check if the virtual timer has expired while we were running in the host,
+ * and inject an interrupt if that was the case, making sure the timer is
+ * masked or disabled on the host so that we keep executing. Also schedule a
+ * software timer for the physical timer if it is enabled.
+ */
+void kvm_timer_flush_hwstate(struct kvm_vcpu *vcpu)
+{
+ struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
+
+ if (unlikely(!timer->enabled))
+ return;
+
+ kvm_timer_update_state(vcpu);
+
+ /* Set the background timer for the physical timer emulation. */
+ kvm_timer_emulate(vcpu, vcpu_ptimer(vcpu));
+
+ if (unlikely(!irqchip_in_kernel(vcpu->kvm)))
+ kvm_timer_flush_hwstate_user(vcpu);
+ else
+ kvm_timer_flush_hwstate_vgic(vcpu);
+}
+
/**
* kvm_timer_sync_hwstate - sync timer state from cpu
* @vcpu: The vcpu pointer
*
- * Check if the virtual timer has expired while we were running in the guest,
+ * Check if any of the timers have expired while we were running in the guest,
* and inject an interrupt if that was the case.
*/
void kvm_timer_sync_hwstate(struct kvm_vcpu *vcpu)
@@ -558,6 +628,13 @@ int kvm_timer_enable(struct kvm_vcpu *vcpu)
if (timer->enabled)
return 0;
+ /* Without a VGIC we do not map virtual IRQs to physical IRQs */
+ if (!irqchip_in_kernel(vcpu->kvm))
+ goto no_vgic;
+
+ if (!vgic_initialized(vcpu->kvm))
+ return -ENODEV;
+
/*
* Find the physical IRQ number corresponding to the host_vtimer_irq
*/
@@ -581,8 +658,8 @@ int kvm_timer_enable(struct kvm_vcpu *vcpu)
if (ret)
return ret;
+no_vgic:
timer->enabled = 1;
-
return 0;
}
--
2.9.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 4/5] KVM: arm/arm64: Report PMU overflow interrupts to userspace irqchip
2017-02-03 14:56 [PATCH v2 0/5] Support userspace irqchip with arch timers Christoffer Dall
` (2 preceding siblings ...)
2017-02-03 14:56 ` [PATCH v2 3/5] KVM: arm/arm64: Support arch timers with a userspace gic Christoffer Dall
@ 2017-02-03 14:56 ` Christoffer Dall
2017-02-03 14:56 ` [PATCH v2 5/5] KVM: arm/arm64: Advertise support for KVM_CAP_ARM_USER_IRQ Christoffer Dall
4 siblings, 0 replies; 10+ messages in thread
From: Christoffer Dall @ 2017-02-03 14:56 UTC (permalink / raw)
To: linux-arm-kernel
From: Christoffer Dall <christoffer.dall@linaro.org>
When not using an in-kernel VGIC, but instead emulating an interrupt
controller in userspace, we should report the PMU overflow status to
that userspace interrupt controller using the KVM_CAP_ARM_USER_IRQ
feature.
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
arch/arm/kvm/arm.c | 9 ++++++---
include/kvm/arm_pmu.h | 7 +++++++
virt/kvm/arm/pmu.c | 42 ++++++++++++++++++++++++++++++++++++++----
3 files changed, 51 insertions(+), 7 deletions(-)
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 505f928..92f38f6 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -628,11 +628,13 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
/*
* If we have a singal pending, or need to notify a userspace
- * irqchip about timer level changes, then we exit (and update
- * the timer level state in kvm_timer_update_run below).
+ * irqchip about timer or PMU level changes, then we exit (and
+ * update the timer level state in kvm_timer_update_run
+ * below).
*/
if (signal_pending(current) ||
- kvm_timer_should_notify_user(vcpu)) {
+ kvm_timer_should_notify_user(vcpu) ||
+ kvm_pmu_should_notify_user(vcpu)) {
ret = -EINTR;
run->exit_reason = KVM_EXIT_INTR;
}
@@ -706,6 +708,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
/* Tell userspace about in-kernel device output levels */
kvm_timer_update_run(vcpu);
+ kvm_pmu_update_run(vcpu);
if (vcpu->sigset_active)
sigprocmask(SIG_SETMASK, &sigsaved, NULL);
diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
index 92e7e97..1ab4633 100644
--- a/include/kvm/arm_pmu.h
+++ b/include/kvm/arm_pmu.h
@@ -50,6 +50,8 @@ void kvm_pmu_enable_counter(struct kvm_vcpu *vcpu, u64 val);
void kvm_pmu_overflow_set(struct kvm_vcpu *vcpu, u64 val);
void kvm_pmu_flush_hwstate(struct kvm_vcpu *vcpu);
void kvm_pmu_sync_hwstate(struct kvm_vcpu *vcpu);
+bool kvm_pmu_should_notify_user(struct kvm_vcpu *vcpu);
+void kvm_pmu_update_run(struct kvm_vcpu *vcpu);
void kvm_pmu_software_increment(struct kvm_vcpu *vcpu, u64 val);
void kvm_pmu_handle_pmcr(struct kvm_vcpu *vcpu, u64 val);
void kvm_pmu_set_counter_event_type(struct kvm_vcpu *vcpu, u64 data,
@@ -85,6 +87,11 @@ static inline void kvm_pmu_enable_counter(struct kvm_vcpu *vcpu, u64 val) {}
static inline void kvm_pmu_overflow_set(struct kvm_vcpu *vcpu, u64 val) {}
static inline void kvm_pmu_flush_hwstate(struct kvm_vcpu *vcpu) {}
static inline void kvm_pmu_sync_hwstate(struct kvm_vcpu *vcpu) {}
+static inline bool kvm_pmu_should_notify_user(struct kvm_vcpu *vcpu)
+{
+ return false;
+}
+static inline void kvm_pmu_update_run(struct kvm_vcpu *vcpu) {}
static inline void kvm_pmu_software_increment(struct kvm_vcpu *vcpu, u64 val) {}
static inline void kvm_pmu_handle_pmcr(struct kvm_vcpu *vcpu, u64 val) {}
static inline void kvm_pmu_set_counter_event_type(struct kvm_vcpu *vcpu,
diff --git a/virt/kvm/arm/pmu.c b/virt/kvm/arm/pmu.c
index 69ccce3..51218be 100644
--- a/virt/kvm/arm/pmu.c
+++ b/virt/kvm/arm/pmu.c
@@ -230,13 +230,47 @@ static void kvm_pmu_update_state(struct kvm_vcpu *vcpu)
return;
overflow = !!kvm_pmu_overflow_status(vcpu);
- if (pmu->irq_level != overflow) {
- pmu->irq_level = overflow;
- kvm_vgic_inject_irq(vcpu->kvm, vcpu->vcpu_id,
- pmu->irq_num, overflow);
+ if (pmu->irq_level == overflow)
+ return;
+
+ pmu->irq_level = overflow;
+
+ if (likely(irqchip_in_kernel(vcpu->kvm))) {
+ int ret;
+ ret = kvm_vgic_inject_irq(vcpu->kvm, vcpu->vcpu_id,
+ pmu->irq_num, overflow);
+ WARN_ON(ret);
}
}
+bool kvm_pmu_should_notify_user(struct kvm_vcpu *vcpu)
+{
+ struct kvm_pmu *pmu = &vcpu->arch.pmu;
+ struct kvm_sync_regs *sregs = &vcpu->run->s.regs;
+ bool run_level = sregs->device_irq_level & KVM_ARM_DEV_PMU;
+
+ if (likely(irqchip_in_kernel(vcpu->kvm)))
+ return false;
+
+ return pmu->irq_level != run_level;
+}
+
+/*
+ * Reflect the PMU overflow interrupt output level into the kvm_run structure
+ */
+void kvm_pmu_update_run(struct kvm_vcpu *vcpu)
+{
+ struct kvm_sync_regs *regs = &vcpu->run->s.regs;
+
+ if (likely(irqchip_in_kernel(vcpu->kvm)))
+ return;
+
+ /* Populate the timer bitmap for user space */
+ regs->device_irq_level &= ~KVM_ARM_DEV_PMU;
+ if (vcpu->arch.pmu.irq_level)
+ regs->device_irq_level |= KVM_ARM_DEV_PMU;
+}
+
/**
* kvm_pmu_flush_hwstate - flush pmu state to cpu
* @vcpu: The vcpu pointer
--
2.9.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 5/5] KVM: arm/arm64: Advertise support for KVM_CAP_ARM_USER_IRQ
2017-02-03 14:56 [PATCH v2 0/5] Support userspace irqchip with arch timers Christoffer Dall
` (3 preceding siblings ...)
2017-02-03 14:56 ` [PATCH v2 4/5] KVM: arm/arm64: Report PMU overflow interrupts to userspace irqchip Christoffer Dall
@ 2017-02-03 14:56 ` Christoffer Dall
4 siblings, 0 replies; 10+ messages in thread
From: Christoffer Dall @ 2017-02-03 14:56 UTC (permalink / raw)
To: linux-arm-kernel
From: Christoffer Dall <christoffer.dall@linaro.org>
Now when we support both the virtual timer and PMU reporting interrupts
to userspace, we can advertise this support.
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
arch/arm/kvm/arm.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 92f38f6..71cf78e 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -226,6 +226,13 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
else
r = kvm->arch.vgic.msis_require_devid;
break;
+ case KVM_CAP_ARM_USER_IRQ:
+ /*
+ * 1: EL1_VTIMER, EL1_PTIMER, and PMU.
+ * (bump this number if adding more devices)
+ */
+ r = 1;
+ break;
default:
r = kvm_arch_dev_ioctl_check_extension(kvm, ext);
break;
--
2.9.0
^ permalink raw reply related [flat|nested] 10+ messages in thread