Re: [PATCH v2 5/7] kvm/riscv: rework guest entry logic

[Mark Rutland <mark.rutland@arm.com>]

On Thu, Jan 20, 2022 at 12:18:04PM +0100, Paolo Bonzini wrote:
> On 1/19/22 11:58, Mark Rutland wrote:
> > +		 * There's no barrier which ensures that pending interrupts are
> > +		 * recognised, so we just hope that the CPU takes any pending
> > +		 * interrupts between the enable and disable.
> >   		 */
> >   		local_irq_enable();
> > +		local_irq_disable();
> 
> This should be the required architectural behavior: "a CSR access is
> performed after the execution of any prior instructions in program order
> whose behavior modifies or is modified by the CSR state and before the
> execution of any subsequent instructions in program order whose behavior
> modifies or is modified by the CSR state" (Zicsr spec, paragraph "CSR Access
> Ordering", available at
> https://www.five-embeddev.com/riscv-isa-manual/latest/csr.html#csrinsts).

I think that's necessary, but not sufficient.

IIUC that wording means that writes to the CSR state occur in program order
without requiring additional barriers to take effect. The current value of the
CSR determines whether interrupts *can* be taken, but that doesn't say that
pending interrrupts *must* be taken immediately when unmasked in the CSR.

For comparison, ARMv8 has similar wording that writes to PSTATE.I via the
DAIF/DAIFSet/DAIFClr registers occur in program order and do not require
additional barriers. However, there's also wording that to ensure that a
pending interrupt is taken, a context-synchronization-event (e.g. an ISB
instruction) is necessary. Without that, a back-to-back enable->disable will
not necessarily result in a pending interrupt being taken.

The arm64 patch in this series has references to the documentation.

I had asked Palmer about this on IRC, and he didn't seem to think the
architecture mandated this (or at least, was unsure).

Palmer, thoughts?

Thanks,
Mark.