Message ID | 156395684219.48058.9459603098282979575.stgit@lep8c.aus.stglabs.ibm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v5] ppc: remove the idle_timer logic | expand |
On Wed, Jul 24, 2019 at 03:28:28AM -0500, Shivaprasad G Bhat wrote: > The KVM_CAP_PPC_IRQ_LEVEL is part of the kernel now since 2.6.37. > Drop the redundant logic which is not excercised on new the kernels anymore. > > Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com> I don't think we want to support such ancient kernels any more, but it would be good to print a useful error message just in case someoone does. So I'd suggest.. > --- > v4: https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg04456.html > Changes from v4: > - it was discussed to drop the idle_timer logic instead of fixing the > leak and keeping the redundant logic around. So, the patch does that. > > target/ppc/kvm.c | 74 +----------------------------------------------------- > 1 file changed, 2 insertions(+), 72 deletions(-) > > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c > index 8a06d3171e..1c8f2319a0 100644 > --- a/target/ppc/kvm.c > +++ b/target/ppc/kvm.c > @@ -56,7 +56,6 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = { > }; > > static int cap_interrupt_unset; > -static int cap_interrupt_level; > static int cap_segstate; > static int cap_booke_sregs; > static int cap_ppc_smt; > @@ -87,25 +86,6 @@ static int cap_large_decr; > > static uint32_t debug_inst_opcode; > > -/* > - * XXX We have a race condition where we actually have a level triggered > - * interrupt, but the infrastructure can't expose that yet, so the guest > - * takes but ignores it, goes to sleep and never gets notified that there's > - * still an interrupt pending. > - * > - * As a quick workaround, let's just wake up again 20 ms after we injected > - * an interrupt. That way we can assure that we're always reinjecting > - * interrupts in case the guest swallowed them. > - */ > -static QEMUTimer *idle_timer; > - > -static void kvm_kick_cpu(void *opaque) > -{ > - PowerPCCPU *cpu = opaque; > - > - qemu_cpu_kick(CPU(cpu)); > -} > - > /* > * Check whether we are running with KVM-PR (instead of KVM-HV). This > * should only be used for fallback tests - generally we should use > @@ -125,7 +105,6 @@ static int kvmppc_get_dec_bits(void); > int kvm_arch_init(MachineState *ms, KVMState *s) > { > cap_interrupt_unset = kvm_check_extension(s, KVM_CAP_PPC_UNSET_IRQ); > - cap_interrupt_level = kvm_check_extension(s, KVM_CAP_PPC_IRQ_LEVEL); > cap_segstate = kvm_check_extension(s, KVM_CAP_PPC_SEGSTATE); > cap_booke_sregs = kvm_check_extension(s, KVM_CAP_PPC_BOOKE_SREGS); > cap_ppc_smt_possible = kvm_vm_check_extension(s, KVM_CAP_PPC_SMT_POSSIBLE); > @@ -161,11 +140,6 @@ int kvm_arch_init(MachineState *ms, KVMState *s) > */ > cap_ppc_pvr_compat = false; > > - if (!cap_interrupt_level) { > - fprintf(stderr, "KVM: Couldn't find level irq capability. Expect the " > - "VM to stall at times!\n"); > - } > - ..changing this to print such an error and exit(1) if the extension isn't found. > kvm_ppc_register_host_cpu_type(ms); > > return 0; > @@ -491,8 +465,6 @@ int kvm_arch_init_vcpu(CPUState *cs) > return ret; > } > > - idle_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, kvm_kick_cpu, cpu); > - > switch (cenv->mmu_model) { > case POWERPC_MMU_BOOKE206: > /* This target supports access to KVM's guest TLB */ > @@ -1332,7 +1304,7 @@ int kvmppc_set_interrupt(PowerPCCPU *cpu, int irq, int level) > return 0; > } > > - if (!kvm_enabled() || !cap_interrupt_unset || !cap_interrupt_level) { > + if (!kvm_enabled() || !cap_interrupt_unset) { > return 0; > } > > @@ -1349,49 +1321,7 @@ int kvmppc_set_interrupt(PowerPCCPU *cpu, int irq, int level) > > void kvm_arch_pre_run(CPUState *cs, struct kvm_run *run) > { > - PowerPCCPU *cpu = POWERPC_CPU(cs); > - CPUPPCState *env = &cpu->env; > - int r; > - unsigned irq; > - > - qemu_mutex_lock_iothread(); > - > - /* > - * PowerPC QEMU tracks the various core input pins (interrupt, > - * critical interrupt, reset, etc) in PPC-specific > - * env->irq_input_state. > - */ > - if (!cap_interrupt_level && > - run->ready_for_interrupt_injection && > - (cs->interrupt_request & CPU_INTERRUPT_HARD) && > - (env->irq_input_state & (1 << PPC_INPUT_INT))) > - { > - /* > - * For now KVM disregards the 'irq' argument. However, in the > - * future KVM could cache it in-kernel to avoid a heavyweight > - * exit when reading the UIC. > - */ > - irq = KVM_INTERRUPT_SET; > - > - trace_kvm_injected_interrupt(irq); > - r = kvm_vcpu_ioctl(cs, KVM_INTERRUPT, &irq); > - if (r < 0) { > - printf("cpu %d fail inject %x\n", cs->cpu_index, irq); > - } > - > - /* Always wake up soon in case the interrupt was level based */ > - timer_mod(idle_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + > - (NANOSECONDS_PER_SECOND / 50)); > - } > - > - /* > - * We don't know if there are more interrupts pending after > - * this. However, the guest will return to userspace in the course > - * of handling this one anyways, so we will get a chance to > - * deliver the rest. > - */ > - > - qemu_mutex_unlock_iothread(); > + return; > } > > MemTxAttrs kvm_arch_post_run(CPUState *cs, struct kvm_run *run) >
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c index 8a06d3171e..1c8f2319a0 100644 --- a/target/ppc/kvm.c +++ b/target/ppc/kvm.c @@ -56,7 +56,6 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = { }; static int cap_interrupt_unset; -static int cap_interrupt_level; static int cap_segstate; static int cap_booke_sregs; static int cap_ppc_smt; @@ -87,25 +86,6 @@ static int cap_large_decr; static uint32_t debug_inst_opcode; -/* - * XXX We have a race condition where we actually have a level triggered - * interrupt, but the infrastructure can't expose that yet, so the guest - * takes but ignores it, goes to sleep and never gets notified that there's - * still an interrupt pending. - * - * As a quick workaround, let's just wake up again 20 ms after we injected - * an interrupt. That way we can assure that we're always reinjecting - * interrupts in case the guest swallowed them. - */ -static QEMUTimer *idle_timer; - -static void kvm_kick_cpu(void *opaque) -{ - PowerPCCPU *cpu = opaque; - - qemu_cpu_kick(CPU(cpu)); -} - /* * Check whether we are running with KVM-PR (instead of KVM-HV). This * should only be used for fallback tests - generally we should use @@ -125,7 +105,6 @@ static int kvmppc_get_dec_bits(void); int kvm_arch_init(MachineState *ms, KVMState *s) { cap_interrupt_unset = kvm_check_extension(s, KVM_CAP_PPC_UNSET_IRQ); - cap_interrupt_level = kvm_check_extension(s, KVM_CAP_PPC_IRQ_LEVEL); cap_segstate = kvm_check_extension(s, KVM_CAP_PPC_SEGSTATE); cap_booke_sregs = kvm_check_extension(s, KVM_CAP_PPC_BOOKE_SREGS); cap_ppc_smt_possible = kvm_vm_check_extension(s, KVM_CAP_PPC_SMT_POSSIBLE); @@ -161,11 +140,6 @@ int kvm_arch_init(MachineState *ms, KVMState *s) */ cap_ppc_pvr_compat = false; - if (!cap_interrupt_level) { - fprintf(stderr, "KVM: Couldn't find level irq capability. Expect the " - "VM to stall at times!\n"); - } - kvm_ppc_register_host_cpu_type(ms); return 0; @@ -491,8 +465,6 @@ int kvm_arch_init_vcpu(CPUState *cs) return ret; } - idle_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, kvm_kick_cpu, cpu); - switch (cenv->mmu_model) { case POWERPC_MMU_BOOKE206: /* This target supports access to KVM's guest TLB */ @@ -1332,7 +1304,7 @@ int kvmppc_set_interrupt(PowerPCCPU *cpu, int irq, int level) return 0; } - if (!kvm_enabled() || !cap_interrupt_unset || !cap_interrupt_level) { + if (!kvm_enabled() || !cap_interrupt_unset) { return 0; } @@ -1349,49 +1321,7 @@ int kvmppc_set_interrupt(PowerPCCPU *cpu, int irq, int level) void kvm_arch_pre_run(CPUState *cs, struct kvm_run *run) { - PowerPCCPU *cpu = POWERPC_CPU(cs); - CPUPPCState *env = &cpu->env; - int r; - unsigned irq; - - qemu_mutex_lock_iothread(); - - /* - * PowerPC QEMU tracks the various core input pins (interrupt, - * critical interrupt, reset, etc) in PPC-specific - * env->irq_input_state. - */ - if (!cap_interrupt_level && - run->ready_for_interrupt_injection && - (cs->interrupt_request & CPU_INTERRUPT_HARD) && - (env->irq_input_state & (1 << PPC_INPUT_INT))) - { - /* - * For now KVM disregards the 'irq' argument. However, in the - * future KVM could cache it in-kernel to avoid a heavyweight - * exit when reading the UIC. - */ - irq = KVM_INTERRUPT_SET; - - trace_kvm_injected_interrupt(irq); - r = kvm_vcpu_ioctl(cs, KVM_INTERRUPT, &irq); - if (r < 0) { - printf("cpu %d fail inject %x\n", cs->cpu_index, irq); - } - - /* Always wake up soon in case the interrupt was level based */ - timer_mod(idle_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + - (NANOSECONDS_PER_SECOND / 50)); - } - - /* - * We don't know if there are more interrupts pending after - * this. However, the guest will return to userspace in the course - * of handling this one anyways, so we will get a chance to - * deliver the rest. - */ - - qemu_mutex_unlock_iothread(); + return; } MemTxAttrs kvm_arch_post_run(CPUState *cs, struct kvm_run *run)
The KVM_CAP_PPC_IRQ_LEVEL is part of the kernel now since 2.6.37. Drop the redundant logic which is not excercised on new the kernels anymore. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com> --- v4: https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg04456.html Changes from v4: - it was discussed to drop the idle_timer logic instead of fixing the leak and keeping the redundant logic around. So, the patch does that. target/ppc/kvm.c | 74 +----------------------------------------------------- 1 file changed, 2 insertions(+), 72 deletions(-)