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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YelcA/jl6YEfk7m4@FVFF77S0Q05N>
Date:   Thu, 20 Jan 2022 12:56:35 +0000
From:   Mark Rutland <mark.rutland@....com>
To:     Paolo Bonzini <pbonzini@...hat.com>, palmer@...belt.com
Cc:     linux-kernel@...r.kernel.org, aleksandar.qemu.devel@...il.com,
        alexandru.elisei@....com, anup.patel@....com,
        aou@...s.berkeley.edu, atish.patra@....com,
        borntraeger@...ux.ibm.com, bp@...en8.de, catalin.marinas@....com,
        chenhuacai@...nel.org, dave.hansen@...ux.intel.com,
        frankja@...ux.ibm.com, frederic@...nel.org, gor@...ux.ibm.com,
        hca@...ux.ibm.com, james.morse@....com, jmattson@...gle.com,
        joro@...tes.org, luto@...nel.org, maz@...nel.org, mingo@...hat.com,
        mpe@...erman.id.au, nsaenzju@...hat.com, paulmck@...nel.org,
        paul.walmsley@...ive.com, peterz@...radead.org, seanjc@...gle.com,
        suzuki.poulose@....com, svens@...ux.ibm.com, tglx@...utronix.de,
        tsbogend@...ha.franken.de, vkuznets@...hat.com,
        wanpengli@...cent.com, will@...nel.org
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.