On 11/16/21 18:57, David Woodhouse wrote:
+ read_lock(&gpc->lock);
+ if (!kvm_gfn_to_pfn_cache_check(vcpu->kvm, gpc, gpc->gpa, PAGE_SIZE)) {
+ read_unlock(&gpc->lock);
goto mmio_needed;
+ }
+
+ vapic_page = gpc->khva;
If we know this gpc is of the synchronous kind, I think we can skip the
read_lock/read_unlock here?!?
Er... this one was OUTSIDE_GUEST_MODE and is the atomic kind, which
means it needs to hold the lock for the duration of the access in order
to prevent (preemption and) racing with the invalidate?
It's the IN_GUEST_MODE one (in my check_guest_maps()) where we might
get away without the lock, perhaps?
Ah, this is check_nested_events which is mostly IN_GUEST_MODE but not
always (and that sucks for other reasons). I'll think a bit more about
it when I actually do the work.
__kvm_apic_update_irr(vmx->nested.pi_desc->pir,
vapic_page, &max_irr);
@@ -3749,6 +3783,7 @@ static int vmx_complete_nested_posted_interrupt(struct kvm_vcpu *vcpu)
status |= (u8)max_irr;
vmcs_write16(GUEST_INTR_STATUS, status);
}
+ read_unlock(&gpc->lock);
}
I just realised that the mark_page_dirty() on invalidation and when the
the irqfd workqueue refreshes the gpc might fall foul of the same
dirty_ring problem that I belatedly just spotted with the Xen shinfo
clock write. I'll fix it up to*always* require a vcpu (to be
associated with the writes), and reinstate the guest_uses_pa flag since
that can no longer in implicit in (vcpu!=NULL).
Okay.
I may leave the actual task of fixing nesting to you, if that's OK, as
long as we consider the new gfn_to_pfn_cache sufficient to address the
problem? I think it's mostly down to how we*use* it now, rather than
the fundamental design of cache itself?
Yes, I agree. Just stick whatever you have for nesting as an extra
patch at the end, and I'll take it from there.
Paolo
