On 29 October 2016 at 22:10, Alexander Graf <agraf@xxxxxxx> wrote:
> When running with KVM enabled, you can choose between emulating the
> gic in kernel or user space. If the kernel supports in-kernel virtualization
> of the interrupt controller, it will default to that. If not, if will
> default to user space emulation.
>
> Unfortunately when running in user mode gic emulation, we miss out on
> timer events which are only available from kernel space. This patch leverages
> the new kernel/user space pending line synchronization for those timer events.
>
> Signed-off-by: Alexander Graf <agraf@xxxxxxx>
> ---
> hw/arm/virt.c | 10 ++++++++++
> target-arm/cpu.h | 3 +++
> target-arm/kvm.c | 19 +++++++++++++++++++
> 3 files changed, 32 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 070bbf8..8ac81e3 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -622,6 +622,16 @@ static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic, int type,
> } else if (type == 2) {
> create_v2m(vbi, pic);
> }
> +
> +#ifdef CONFIG_KVM
> + if (kvm_enabled() && !kvm_irqchip_in_kernel()) {
> + if (!kvm_check_extension(kvm_state, KVM_CAP_ARM_TIMER)) {
> + error_report("KVM with user space irqchip only works when the "
> + "host kernel supports KVM_CAP_ARM_TIMER");
> + exit(1);
> + }
> + }
> +#endif
I think this belongs somewhere in target-arm/kvm.c rather
than in hw/arm/virt.c -- it's not the only board model that
supports KVM.
> }
>
> static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic, int uart,
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 19d967b..7686082 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -659,6 +659,9 @@ struct ARMCPU {
>
> ARMELChangeHook *el_change_hook;
> void *el_change_hook_opaque;
> +
> + /* Used to synchronize KVM and QEMU timer levels */
> + uint8_t timer_irq_level;
> };
>
> static inline ARMCPU *arm_env_get_cpu(CPUARMState *env)
> diff --git a/target-arm/kvm.c b/target-arm/kvm.c
> index c00b94e..0d8b642 100644
> --- a/target-arm/kvm.c
> +++ b/target-arm/kvm.c
> @@ -527,6 +527,25 @@ void kvm_arch_pre_run(CPUState *cs, struct kvm_run *run)
>
> MemTxAttrs kvm_arch_post_run(CPUState *cs, struct kvm_run *run)
> {
> + ARMCPU *cpu;
> +
> + if (kvm_irqchip_in_kernel()) {
> + /*
> + * We only need to sync timer states with user-space interrupt
> + * controllers, so return early and save cycles if we don't.
> + */
> + return MEMTXATTRS_UNSPECIFIED;
> + }
> +
> + cpu = ARM_CPU(cs);
> +
> + /* Synchronize our internal vtimer irq line with the kvm one */
> + if (run->s.regs.timer_irq_level != cpu->timer_irq_level) {
> + qemu_set_irq(ARM_CPU(cs)->gt_timer_outputs[GTIMER_VIRT],
You just set up a local variable, so you don't need to inline "ARM_CPU(cs)".
> + run->s.regs.timer_irq_level & KVM_ARM_TIMER_VTIMER);
This is setting a bear trap for the person who comes along later
to add the next interrupt, because the level argument to qemu_set_irq()
should be 0 or 1. That happens to be true for the KVM_ARM_TIMER_VTIMER
bit but won't be for the cut-n-pasted version with the next bit name...
> + cpu->timer_irq_level = run->s.regs.timer_irq_level;
> + }
> +
> return MEMTXATTRS_UNSPECIFIED;
> }
Does this code do the right thing across a vcpu reset or
a full-system reset?
>
> --
> 1.8.5.6
thanks
-- PMM
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
