Re: [PATCH v2 1/4] KVM: arm64: nv: Keep reference on stage-2 MMU when scheduled out

[Sean Christopherson <seanjc@google.com>]

On Mon, Oct 07, 2024, Oliver Upton wrote:
> On Mon, Oct 07, 2024 at 11:39:37AM -0700, Sean Christopherson wrote:
> > > @@ -674,10 +681,18 @@ void kvm_vcpu_load_hw_mmu(struct kvm_vcpu *vcpu)
> > >  
> > >  void kvm_vcpu_put_hw_mmu(struct kvm_vcpu *vcpu)
> > >  {
> > > -	if (kvm_is_nested_s2_mmu(vcpu->kvm, vcpu->arch.hw_mmu)) {
> > > +	/*
> > > +	 * Keep a reference on the associated stage-2 MMU if the vCPU is
> > > +	 * scheduling out and not in WFI emulation, suggesting it is likely to
> > > +	 * reuse the MMU sometime soon.
> > > +	 */
> > > +	if (vcpu->scheduled_out && !vcpu_get_flag(vcpu, IN_WFI))
> > > +		return;
> > 
> > Assuming KVM arm64 supports halt-polling, I think it makes more sense to check
> > kvm_vcpu_is_blocking() instead of IN_WFI.  That way, KVM will keep the MMU resident
> > if the vCPU happens to be preempted and halt-polling is successful.
> > 
> > And somewhat of a side topic, calling kvm_vgic_put() from kvm_arch_vcpu_blocking()
> > instead of kvm_vcpu_wfi() would provide the same optimization for the GIC, as KVM
> > would only need to put/reload the vGIC if the vCPU is actually scheduled out.
> 
> That wouldn't be an optimization, it'd render the halt polling loop
> useless.
> 
> The most recent view of the guest's CPU interface is sitting in
> hardware at this point, so we need to synchronize KVM's view of
> the CPUIF with it to determine if a pending interrupt exceeds the
> priority mask.

Hmm, but what happens if the wakeup event arrives before IN_WFI is set?  Ah, IIUC,
GICR_VPENDBASER_PendingLast tracks if there's a pending event, KVM makes sure to
read PendingLast after making vPE non-resident, and hardware is required to ensure
either PendingLast=1 or a doorbell is signaled.

Niave question time: why not query GICR_VPENDBASER_PendingLast directly in
kvm_vgic_vcpu_pending_irq() instead of putting the vGIC?  E.g. if safely querying
PendingLast is "heavy", wouldn't it still make sense to do a slow check of
PendingLast in kvm_vgic_vcpu_pending_irq() if IN_WFI=1?  Or is putting the vGIC
as lightweight as things get when it comes to checking PendingLast?

> On top of that, we need to activate the doorbell IRQ for GICv4 to give
> KVM a kick when something arrives for the vPE.

Right, but KVM needs to "manually" detect pending events to account for events
that arrive before the doorbell is activated, so from a functional perspective,
it shouldn't matter when KVM activates the doorbell (with respect to halt-polling).