Le 19/06/2021 à 17:02, Segher Boessenkool a écrit :
On Sat, Jun 19, 2021 at 11:35:34AM +0200, Christophe Leroy wrote:
Le 18/06/2021 à 19:26, Mathieu Desnoyers a écrit :
----- On Jun 18, 2021, at 1:13 PM, Christophe Leroy
christophe.leroy@xxxxxxxxxx wrote:
[...]
I don't understand all that complexity to just replace a simple
'smp_mb__after_unlock_lock()'.
#define smp_mb__after_unlock_lock() smp_mb()
#define smp_mb() barrier()
# define barrier() __asm__ __volatile__("": : :"memory")
Am I missing some subtility ?
On powerpc CONFIG_SMP, smp_mb() is actually defined as:
#define smp_mb() __smp_mb()
#define __smp_mb() mb()
#define mb() __asm__ __volatile__ ("sync" : : : "memory")
So the original motivation here was to skip a "sync" instruction whenever
switching between threads which are part of the same process. But based on
recent discussions, I suspect my implementation may be inaccurately doing
so though.
I see.
Then, if you think a 'sync' is a concern, shouldn't we try and remove the
forest of 'sync' in the I/O accessors ?
I can't really understand why we need all those 'sync' and 'isync' and
'twi' around the accesses whereas I/O memory is usually mapped as 'Guarded'
so memory access ordering is already garantied.
I'm sure we'll save a lot with that.
The point of the twi in the I/O accessors was to make things easier to
debug if the accesses fail: for the twi insn to complete the load will
have to have completed as well. On a correctly working system you never
should need this (until something fails ;-) )
Without the twi you might need to enforce ordering in some cases still.
The twi is a very heavy hammer, but some of that that gives us is no
doubt actually needed.
Well, I've always been quite perplex about that. According to the documentation of the 8xx, if a bus
error or something happens on an I/O access, the exception will be accounted on the instruction
which does the access. But based on the following function, I understand that some version of
powerpc do generate the trap on the instruction which was being executed at the time the I/O access
failed, not the instruction that does the access itself ?
/*
* I/O accesses can cause machine checks on powermacs.
* Check if the NIP corresponds to the address of a sync
* instruction for which there is an entry in the exception
* table.
* -- paulus.
*/
static inline int check_io_access(struct pt_regs *regs)
{
#ifdef CONFIG_PPC32
unsigned long msr = regs->msr;
const struct exception_table_entry *entry;
unsigned int *nip = (unsigned int *)regs->nip;
if (((msr & 0xffff0000) == 0 || (msr & (0x80000 | 0x40000)))
&& (entry = search_exception_tables(regs->nip)) != NULL) {
/*
* Check that it's a sync instruction, or somewhere
* in the twi; isync; nop sequence that inb/inw/inl uses.
* As the address is in the exception table
* we should be able to read the instr there.
* For the debug message, we look at the preceding
* load or store.
*/
if (*nip == PPC_INST_NOP)
nip -= 2;
else if (*nip == PPC_INST_ISYNC)
--nip;
if (*nip == PPC_INST_SYNC || (*nip >> 26) == OP_TRAP) {
unsigned int rb;
--nip;
rb = (*nip >> 11) & 0x1f;
printk(KERN_DEBUG "%s bad port %lx at %p\n",
(*nip & 0x100)? "OUT to": "IN from",
regs->gpr[rb] - _IO_BASE, nip);
regs->msr |= MSR_RI;
regs->nip = extable_fixup(entry);
return 1;
}
}
#endif /* CONFIG_PPC32 */
return 0;
}
Am I right ?
It is not only the twi which bother's me in the I/O accessors but also the sync/isync and stuff.
A write typically is
sync
stw
A read is
sync
lwz
twi
isync
Taking into account that HW ordering is garanteed by the fact that __iomem is guarded, isn't the
'memory' clobber enough as a barrier ?
Thanks
Christophe
