Re: [EXTERNAL] [PATCH] target/ppc: Fix mtmsr(d) L=1 variant that loses interrupts

[Nicholas Piggin]

Excerpts from Cédric Le Goater's message of April 15, 2020 4:49 pm:
> On 4/14/20 1:11 PM, Nicholas Piggin wrote:
>> 
>> The confusion arises from L=0 being "context synchronizing" whereas L=1
>> is "execution synchronizing", which is a weaker semantic. However this
>> is not a relaxation of the requirement that these exceptions cause
>> interrupts when MSR[EE]=1 (e.g., when mtmsr executes to completion as
>> TCG is doing here), rather it specifies how a pipelined processor can
>> have multiple instructions in flight where one may influence how another
>> behaves.
> 
> I was expecting more changes but this looks fine. 

It _seems_ to just be these, from what I could see, but could quite 
easily be other issues I missed.

There is at least one other "funny" thing with this synchronization, 
which is the TLB flushing. I don't think it has a bug, but comments
are a bit suspect. tlbie/l doesn't have anything to do with context
/ execution synchronization, so it's a bit interesting to check them
in isync and rfi etc.

ptesync is required because the page table walkers are not necessarily 
coherent with the main CPU's memory pipeline, so you store a new value 
to a PTE then do a tlbiel, you can't have the MMU reload the TLB with 
the old PTE because the store was sitting in the store queue that 
doesn't forward to the table walker. This condition can persist after
the store instruction itself has completed so no amount of this
context synchronization would help.

It does kind of make sense to check the tlb flush in rfi, so you catch 
stray ones that didn't have the right ptesync/tlbsync, but it would 
almost be a condition you could catch and add a warn for. isync doesn't
make a lot of sense though, as far as I can see.

Thanks,
Nick

> Reviewed-by: Cédric Le Goater <address@hidden>

Sorry I always get your email wrong, phantom address book entry.