On 25/11/20 19:32, Sean Christopherson wrote:
I'm pretty sure the exiting vCPU needs to wait
for all senders to finish their sequence, otherwise pi_pending could be left
set, but spinning on pi_pending is wrong.
What if you set it before?
static int vmx_deliver_nested_posted_interrupt(struct kvm_vcpu *vcpu,
int vector)
{
struct vcpu_vmx *vmx = to_vmx(vcpu);
if (is_guest_mode(vcpu) &&
vector == vmx->nested.posted_intr_nv) {
/*
* Set pi_pending after ON.
*/
smp_store_release(&vmx->nested.pi_pending, true);
if (!kvm_vcpu_trigger_posted_interrupt(vcpu, true)) {
/*
* The guest was not running, let's try again
* on the next vmentry.
*/
<set PINV in L1 vIRR>
kvm_make_request(KVM_REQ_EVENT, vcpu);
kvm_vcpu_kick(vcpu);
vmx->nested.pi_pending = false;
}
write_seqcount_end(&vmx->nested.pi_pending_sc);
return 0;
}
return -1;
}
On top of this:
- kvm_x86_ops.hwapic_irr_update can be deleted. It is already done
unconditionally by vmx_sync_pir_to_irr before every vmentry. This gives
more freedom in changing vmx_sync_pir_to_irr and vmx_hwapic_irr_update.
- VCPU entry must check if max_irr == vmx->nested.posted_intr_nv, and if
so send a POSTED_INTR_NESTED_VECTOR self-IPI.
Combining both (and considering that AMD doesn't do anything interesting
in vmx_sync_pir_to_irr), I would move the whole call to
vmx_sync_pir_to_irr from x86.c to vmx/vmx.c, so that we know that
vmx_hwapic_irr_update is called with interrupts disabled and right
before vmentry:
static int vmx_sync_pir_to_irr(struct kvm_vcpu *vcpu)
{
...
- vmx_hwapic_irr_update(vcpu, max_irr);
return max_irr;
}
-static void vmx_hwapic_irr_update(struct kvm_vcpu *vcpu, int max_irr)
+static void vmx_hwapic_irr_update(struct kvm_vcpu *vcpu)
{
+ int max_irr;
+
+ WARN_ON(!irqs_disabled());
+ max_irr = vmx_sync_pir_to_irr(vcpu);
if (!is_guest_mode(vcpu))
vmx_set_rvi(max_irr);
+ else if (max_irr == vmx->nested.posted_intr_nv) {
+ ...
+ }
}
and in vmx_vcpu_run:
+ if (kvm_lapic_enabled(vcpu) && vcpu->arch.apicv_active)
+ vmx_hwapic_irr_update(vcpu);
If you agree, feel free to send this (without the else of course) as a
separate cleanup patch immediately.
Paolo
