Re: [PATCHv14 2/9] coresight: etm4x: Use asm-generic IO memory barriers

From: Suzuki K Poulose
Date: Fri May 06 2022 - 05:22:30 EST


On 06/05/2022 04:02, Sai Prakash Ranjan wrote:
Hi Suzuki,

On 5/6/2022 5:14 AM, Suzuki K Poulose wrote:
Hi,

On 04/05/2022 12:28, Sai Prakash Ranjan wrote:
Per discussion in [1], it was decided to move to using architecture
independent/asm-generic IO memory barriers to have just one set of
them and deprecate use of arm64 specific IO memory barriers in driver
code. So replace current usage of __io_rmb()/__iowmb() in drivers to
__io_ar()/__io_bw().

[1] https://lore.kernel.org/lkml/CAK8P3a0L2tLeF1Q0+0ijUxhGNaw+Z0fyPC1oW6_ELQfn0=i4iw@xxxxxxxxxxxxxx/


Looking at the dis-assembly it looks like in effect they are slightly
different for arm64.

i.e., before this patch we had

"dmb osh{ld/st}"

and after the patch we have :

"dsb {ld/st}"

Is this really what we want ? I don't think this is desirable.

Suzuki


No, this is not supposed to happen and I do not see how it could happen.
__io_ar() is defined as __iormb() and __io_bw() is __iowmb().

I checked the disassembly in both case with MMIO trace off/on with __etm4_cpu_save()
as below and saw the same number of "dmb"s.

aarch64-linux-gnu-gdb -batch -ex "disassemble/rs __etm4_cpu_save" vmlinux-without-mmio
aarch64-linux-gnu-gdb -batch -ex "disassemble/rs __etm4_cpu_save" vmlinux-with-mmio

Can you tell me how are you validating if I am missing something?

Apologies, I was missing the patch in this series, which adds
the arm64 definition for __io_ar/__io_bw. (hint: Please Cc
the affected subsystem in the Cover letter for the series, so
we know what the intention of the changes are).

With the patch1, yes this makes sense to me. Otherwise, __io_ar()
is default to rmb() which implies dsb ld.

With that said,

Reviewed-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>