On Mon, Dec 12, 2016 at 05:29:43PM +0100, Radim Krčmář wrote:
> 2016-12-12 17:02+0300, Denis Plotnikov:
> > When processing KVM_REQ_EVENT, apic_update_ppr is called which may set
> > KVM_REQ_EVENT again if the recalculated value of PPR becomes smaller
> > than the previous one. This results in cancelling the guest entry and
> > reiterating in vcpu_enter_guest.
> >
> > However this is unnecessary because at this point KVM_REQ_EVENT is
> > already being processed and there are no other changes in the lapic
> > that may require full-fledged state recalculation.
> >
> > This situation is often hit on systems with TPR shadow, where the
> > TPR can be updated by the guest without a vmexit, so that the first
> > apic_update_ppr to notice it is exactly the one called while
> > processing KVM_REQ_EVENT.
> >
> > To avoid it, introduce a parameter in apic_update_ppr allowing to
> > suppress setting of KVM_REQ_EVENT, and use it on the paths called from
> > KVM_REQ_EVENT processing.
>
> We also call:
>
> kvm_cpu_get_interrupt() in nested_vmx_vmexit()
> - that path is intended without KVM_REQ_EVENT
> kvm_cpu_has_interrupt() in vmx_check_nested_events(),
> - I think it does no harm
> kvm_cpu_has_interrupt() in kvm_vcpu_has_events()
> kvm_cpu_has_interrupt() in kvm_vcpu_ready_for_interrupt_injection()
> - both seem safe as we should not have an interrupt between TPR
> threshold and the new PPR value, so the KVM_REQ_EVENT was useless.
>
> I would prefer we made sure that only callers from KVM_REQ_EVENT used
> the function we are changing -- it is really easy to make a hard-to-find
> mistake in interrupt delivery.
Indeed, that was my concern as well. How about introducing a parameter
to kvm_cpu_{has,get}_interrupt() with the same meaning, and pass it down
to apic_update_ppr()? Then only the call sites under KVM_REQ_EVENT
processing would pass "false" there, and the rest would remain with
"true"?
Roman.
--
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
