From: Ralf Baechle <ralf@linux-mips.org>
To: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Cc: "Steven J. Hill" <Steven.Hill@imgtec.com>,
linux-mips@linux-mips.org,
Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
Subject: Re: [PATCH] MIPS: Add missing hazard barrier in TLB load handler.
Date: Thu, 20 Jun 2013 15:32:18 +0200 [thread overview]
Message-ID: <20130620133218.GA25622@linux-mips.org> (raw)
In-Reply-To: <51C2E3F5.7040507@cogentembedded.com>
On Thu, Jun 20, 2013 at 03:13:57PM +0400, Sergei Shtylyov wrote:
> >+#if !defined(CONFIG_CPU_CAVIUM_OCTEON)
>
> Suggesting to use if (!IS_ENABLED(CONFIG_CPU_CAVIUM_OCTEON)) to
> avoid #ifdef...
An ifdef in disguise. IS_ENABLED() gives better coverage in build tests
but that's about it, it allows more readable formatting. But that's
about it's advantages.
Never use CONFIG_CPU_* to conditionally execute code only on certain
CPU types. It's not consistently used like this everywhere but it's meant
to select optimization for a particular processor type.
For example on the Malta, a system that's notorious for supporting
particularly man processor types CONFIG_CPU_R4X00 might be set but the
actual core core be a lowly R4Kc or the latest 666Kfc MIPS64 R6 core from
hell.
CONFIG_SYS_HAS_CPU_* only expresses that a system _might_ be equipped
with a particular processor, that is support for this core should be
compiled in. An arbitrary number of CONFIG_SYS_HAS_CPU_* may be enabled.
If you really need to test for a processor at runtime, use current_cpu_type().
Typically that's done in a switch and there are plenty of example scattered
around arch/mips/mm/.
It's possible to replace the current current_cpu_type() with a platform-
specific version that looks (example for Cavium Octeon) like:
static inline int __pure current_cpu_type(void)
{
const int cpu_type = current_cpu_data.cputype;
switch (cpu_type) {
case CPU_CAVIUM_OCTEON:
case CPU_CAVIUM_OCTEON_PLUS:
case CPU_CAVIUM_OCTEON2:
break;
default:
unreachable();
}
return cpu_type;
}
Modern GCC will then notice that current_cpu_type() may only ever return
the values CPU_CAVIUM_OCTEON, CPU_CAVIUM_OCTEON_PLUS and CPU_CAVIUM_OCTEON2
and will discard all the non-Octeon code from a switch construct like this:
switch (current_cpu_type()) {
case CPU_R2000:
case CPU_R3000:
...
break;
case CPU_CAVIUM_OCTEON:
case CPU_CAVIUM_OCTEON_PLUS:
case CPU_CAVIUM_OCTEON2:
break;
}
Result: sane, clean code.
Ralf
MIPS: Fix TLBR-use hazards for R2 cores in the TLB reload handlers
MIPS R2 documents state that an execution hazard barrier is needed
after a TLBR before reading EntryLo.
Original patch by Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>.
Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
---
arch/mips/mm/tlbex.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
index bfff8fe..34c882c 100644
--- a/arch/mips/mm/tlbex.c
+++ b/arch/mips/mm/tlbex.c
@@ -1932,6 +1932,19 @@ static void __cpuinit build_r4000_tlb_load_handler(void)
uasm_i_nop(&p);
uasm_i_tlbr(&p);
+
+ switch (current_cpu_type()) {
+ default:
+ if (cpu_has_mips_r2) {
+ uasm_i_ehb(&p);
+
+ case CPU_CAVIUM_OCTEON:
+ case CPU_CAVIUM_OCTEON_PLUS:
+ case CPU_CAVIUM_OCTEON2:
+ break;
+ }
+ }
+
/* Examine entrylo 0 or 1 based on ptr. */
if (use_bbit_insns()) {
uasm_i_bbit0(&p, wr.r2, ilog2(sizeof(pte_t)), 8);
@@ -1986,6 +1999,19 @@ static void __cpuinit build_r4000_tlb_load_handler(void)
uasm_i_nop(&p);
uasm_i_tlbr(&p);
+
+ switch (current_cpu_type()) {
+ default:
+ if (cpu_has_mips_r2) {
+ uasm_i_ehb(&p);
+
+ case CPU_CAVIUM_OCTEON:
+ case CPU_CAVIUM_OCTEON_PLUS:
+ case CPU_CAVIUM_OCTEON2:
+ break;
+ }
+ }
+
/* Examine entrylo 0 or 1 based on ptr. */
if (use_bbit_insns()) {
uasm_i_bbit0(&p, wr.r2, ilog2(sizeof(pte_t)), 8);
prev parent reply other threads:[~2013-06-20 13:32 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-20 5:57 [PATCH] MIPS: Add missing hazard barrier in TLB load handler Steven J. Hill
2013-06-20 11:07 ` Ralf Baechle
2013-06-20 11:13 ` Sergei Shtylyov
2013-06-20 13:32 ` Ralf Baechle [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130620133218.GA25622@linux-mips.org \
--to=ralf@linux-mips.org \
--cc=Leonid.Yegoshin@imgtec.com \
--cc=Steven.Hill@imgtec.com \
--cc=linux-mips@linux-mips.org \
--cc=sergei.shtylyov@cogentembedded.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.