On Tue, Nov 14, 2017 at 05:28:48PM +0100, Luc Michel wrote: > On 11/06/2017 07:16 AM, David Gibson wrote: > > On Thu, Nov 02, 2017 at 11:35:59AM +0100, Luc MICHEL wrote: > >> When overwritting a valid TLB entry with a new one, the previous page > >> were not flushed in QEMU TLB, leading to incoherent mapping. This commit > >> fixes this. > > > > I don't think this is right. As a rule, overwriting a TLB entry > > doesn't necessarily invalidate the previous entry, even on real > > hardware. I don't know exactly what the situation is on the various > > FSL BookE chips, but I know various other models have other caches > > ahead of the main TLB which can cache mappings that have been removed > > from it (e.g. the ERAT on server chips and the shadow TLBs on 4xx). > Indeed, e500 cores have a two-level TLB. tlbwe writes in L2, and L1 is > handled by the hardware and not visible to the user. > > > > > To invalidate those other caches requires something other than simply > > a tlbwe (tlbie for the ERAT and an isync for the shadow TLBs). > Yes you are right. From "EREF 2.0: A Programmer’s Reference Manual for > Freescale Power Architecture Processors, Rev. 0": > > "A context synchronizing instruction is required after a tlbwe > instruction to ensure any subsequent instructions that will use the > updated TLB values execute in the new context." > > Linux executes a msync followed by a isync after a tlbwe for BookE MMU > machines. > > > > > The current behaviour won't exactly match what hardware does (and it's > > probably not practical to do so), but it should be within what's > > permitted by the architecture - and therefore good enough for correct > > guests. > > > > It's possible that we do need this for the BookE chips, but it'll need > > a more detailed rationale. > The one sentence from the "PowerPC e500 Core Family Reference Manual, > Rev. 1" document that makes my fix kind of correct is this one: > > In 12.4.2 TLB Write Entry (tlbwe) Instruction: > > "Note that when an L2 TLB entry is written, it may be displacing an > already valid entry in the same L2 TLB location (a victim). If a valid > L1 TLB entry corresponds to the L2 MMU victim entry, that L1 TLB entry > is automatically invalidated."
That does seem fairly clear. If you resend the patch with that
paragraph cited in a comment, I'll apply it. A link to the reference
manual would also be appreciated.
> At least, it is as correct as the current tlb_flush in
> "helper_booke206_tlbwe" function, since it does not wait for isync to
> effectively invalidate the page.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
