On Tue, Jul 28, 2015 at 05:58:38PM +0200, Paolo Bonzini wrote: > > > On 28/07/2015 01:17, Steve Rutherford wrote: > > return kvm->arch.vpic; > > } > > > > +static inline int pic_in_kernel(struct kvm *kvm) > > +{ > > + int ret; > > + > > + ret = (pic_irqchip(kvm) != NULL); > > + smp_rmb(); > > What does this memory barrier pair with? I don't think it's necessary. To be honest, it's probably not necessary. I couldn't find why irqchip_in_kernel (which this function is more or less a copy of) needed it's memory barrier, so I cargo culted this one in. > > > + return ret; > > +} > > + > > static inline int irqchip_split(struct kvm *kvm) > > { > > return kvm->arch.irqchip_split; > > > > @@ -5819,13 +5828,24 @@ static void post_kvm_run_save(struct kvm_vcpu *vcpu) > > kvm_run->flags = is_smm(vcpu) ? KVM_RUN_X86_SMM : 0; > > kvm_run->cr8 = kvm_get_cr8(vcpu); > > kvm_run->apic_base = kvm_get_apic_base(vcpu); > > - if (irqchip_in_kernel(vcpu->kvm)) > > + if (irqchip_in_kernel(vcpu->kvm) && pic_in_kernel(vcpu->kvm)) > > kvm_run->ready_for_interrupt_injection = 1; > > - else > > + else if (irqchip_in_kernel(vcpu->kvm)) { > > + int ready_for_interrupt_injection = > > + kvm_apic_accept_pic_intr(vcpu); > > + > > + if (!kvm_run->ready_for_interrupt_injection && > > + ready_for_interrupt_injection) > > + kvm_make_request(KVM_REQ_PIC_UNMASK_EXIT, vcpu); > > + > > + kvm_run->ready_for_interrupt_injection = > > + ready_for_interrupt_injection; > > + } else { > > kvm_run->ready_for_interrupt_injection = > > kvm_arch_interrupt_allowed(vcpu) && > > !kvm_cpu_has_interrupt(vcpu) && > > !kvm_event_needs_reinjection(vcpu); > > + } > > } > > > > static void update_cr8_intercept(struct kvm_vcpu *vcpu) > > Why is this necessary? Could it just set > kvm_run->ready_for_interrupt_injection as in the pic_in_kernel case? The goal is to couple the interrupt ack cycle as closely as possible with the injection of the local interrupt (which occur more or less atomically on real hardware). The idea is to only ever attempt to inject local interrupts when the CPU/APIC is ready to immediately accept. If the CPU is ignoring the PIC, the interrupt acknowledge cycle should not be performed, even if the PIC is high. This patch uses the ready_for_interrupt_injection flag to let userspace whether or not the cpu is paying attention to the PIC at the moment. When the PIC is high and the CPU transitions from ignoring the PIC to paying attention to the PIC, it should (per real hardware) immediately trigger an interrupt acknowledge cycle (which requires bouncing up to userspace). Steve > > Paolo -- 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