Date:   Thu, 13 Jan 2022 11:43:30 +0000
Message-ID: <87r19b97z1.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     Mark Rutland <mark.rutland@arm.com>
Cc:     linux-kernel@vger.kernel.org, aleksandar.qemu.devel@gmail.com,
        alexandru.elisei@arm.com, anup.patel@wdc.com,
        aou@eecs.berkeley.edu, atish.patra@wdc.com,
        benh@kernel.crashing.org, borntraeger@linux.ibm.com, bp@alien8.de,
        catalin.marinas@arm.com, chenhuacai@kernel.org,
        dave.hansen@linux.intel.com, david@redhat.com,
        frankja@linux.ibm.com, frederic@kernel.org, gor@linux.ibm.com,
        hca@linux.ibm.com, imbrenda@linux.ibm.com, james.morse@arm.com,
        jmattson@google.com, joro@8bytes.org, kvm@vger.kernel.org,
        mingo@redhat.com, mpe@ellerman.id.au, nsaenzju@redhat.com,
        palmer@dabbelt.com, paulmck@kernel.org, paulus@samba.org,
        paul.walmsley@sifive.com, pbonzini@redhat.com, seanjc@google.com,
        suzuki.poulose@arm.com, tglx@linutronix.de,
        tsbogend@alpha.franken.de, vkuznets@redhat.com,
        wanpengli@tencent.com, will@kernel.org
Subject: Re: [PATCH 2/5] kvm/arm64: rework guest entry logic
In-Reply-To: <YeAKYUQcHc0+LJ/P@FVFF77S0Q05N>
References: <20220111153539.2532246-1-mark.rutland@arm.com>
        <20220111153539.2532246-3-mark.rutland@arm.com>
        <87tuearwc7.wl-maz@kernel.org>
        <YeAKYUQcHc0+LJ/P@FVFF77S0Q05N>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1
 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Thu, 13 Jan 2022 11:17:53 +0000,
Mark Rutland <mark.rutland@arm.com> wrote:
> 
> On Tue, Jan 11, 2022 at 05:55:20PM +0000, Marc Zyngier wrote:
> > On Tue, 11 Jan 2022 15:35:36 +0000,
> > Mark Rutland <mark.rutland@arm.com> wrote:

[...]

> > > @@ -891,26 +909,23 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
> > >  		kvm_arch_vcpu_ctxsync_fp(vcpu);
> > >  
> > >  		/*
> > > -		 * We may have taken a host interrupt in HYP mode (ie
> > > -		 * while executing the guest). This interrupt is still
> > > -		 * pending, as we haven't serviced it yet!
> > > +		 * We must ensure that any pending interrupts are taken before
> > > +		 * we exit guest timing so that timer ticks are accounted as
> > > +		 * guest time. Transiently unmask interrupts so that any
> > > +		 * pending interrupts are taken.
> > >  		 *
> > > -		 * We're now back in SVC mode, with interrupts
> > > -		 * disabled.  Enabling the interrupts now will have
> > > -		 * the effect of taking the interrupt again, in SVC
> > > -		 * mode this time.
> > > +		 * Per ARM DDI 0487G.b section D1.13.4, an ISB (or other
> > > +		 * context synchronization event) is necessary to ensure that
> > > +		 * pending interrupts are taken.
> > >  		 */
> > >  		local_irq_enable();
> > > +		isb();
> > > +		local_irq_disable();
> > 
> > Small nit: we may be able to elide this enable/isb/disable dance if a
> > read of ISR_EL1 returns 0.
> 
> Wouldn't that be broken when using GIC priority masking, since that
> can prevent IRQS being signalled ot the PE?

You're right. But this can be made even simpler. We already know if
we've exited the guest because of an IRQ (ret tells us that), and
that's true whether we're using priority masking or not. It could be
as simple as:

	if (ARM_EXCEPTION_CODE(ret) == ARM_EXCEPTION_IRQ) {
		// We exited because of an interrupt. Let's take
		// it now to account timer ticks to the guest.
	 	local_irq_enable();
 		isb();
 		local_irq_disable();
	}

and that would avoid accounting the interrupt to the guest if it fired
after the exit took place.

> I'm happy to rework this, but I'll need to think a bit harder about
> it. Would you be happy if we did that as a follow-up?

Oh, absolutely. I want the flow to be correct before we make it
fast(-ish).

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.