Messages in this thread | | | Date | Thu, 20 Jan 2022 12:56:35 +0000 | From | Mark Rutland <> | Subject | Re: [PATCH v2 5/7] kvm/riscv: rework guest entry logic |
| |
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.
| |