[PATCH 0/6] MIPS: BMIPS: RIXI and workarounds support

[PATCH 0/6] MIPS: BMIPS: RIXI and workarounds support

[Florian Fainelli]

Hi all,

This patch series contains some workarounds for some bug with pref30 on
Broadcom BMIPS5000 CPUs in 7344, 7346 and 7425 chips, and some other changes
to allow the use of RIXI/rotr on BMIPS4380 and BMIPS5000.

Finally, the last patch adds a debugfs entry for current_cpu_data.options since
it might be useful to debug that at a time where we set on the final CPU
options.

This is on top of mips-for-linux-next as of
a13d2abd8e617a96d235c0a528a742b347650853 ("MIPS: highmem: Turn
flush_cache_kmaps into a no-op.")

Thanks!

Florian Fainelli (6):
  MIPS: BMIPS: Disable pref 30 for buggy CPUs
  MIPS: BMIPS: Add early CPU initialization code
  MIPS: Allow RIXI to be used on non-R2 or R6 cores
  MIPS: Move RIXI exception enabling after vendor-specific cpu_probe
  MIPS: BMIPS: BMIPS4380 and BMIPS5000 support RIXI
  MIPS: Expose current_cpu_data.options through debugfs

 arch/mips/Kconfig                    |  7 +++
 arch/mips/bmips/setup.c              | 17 +++++++
 arch/mips/include/asm/bmips.h        |  1 +
 arch/mips/include/asm/pgtable-bits.h | 11 ++---
 arch/mips/kernel/cpu-probe.c         | 41 ++++++++++++-----
 arch/mips/kernel/smp-bmips.c         | 87 ++++++++++++++++++++++++++++++++++++
 arch/mips/mm/tlbex.c                 |  2 +-
 7 files changed, 150 insertions(+), 16 deletions(-)

-- 
2.1.0

[PATCH 1/6] MIPS: BMIPS: Disable pref 30 for buggy CPUs

[Florian Fainelli]

Disable pref 30 by utilizing the standard quirk method and matching the
affected SoCs: 7344, 7436, 7425.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/mips/bmips/setup.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/mips/bmips/setup.c b/arch/mips/bmips/setup.c
index 35535284b39e..9c8f15daf5e9 100644
--- a/arch/mips/bmips/setup.c
+++ b/arch/mips/bmips/setup.c
@@ -100,12 +100,28 @@ static void bcm6368_quirks(void)
 	bcm63xx_fixup_cpu1();
 }
 
+static void bmips5000_pref30_quirk(void)
+{
+	__asm__ __volatile__(
+	"	.word	0x4008b008\n"	/* mfc0 $8, $22, 8 */
+	"	lui	$9, 0x0100\n"
+	"	or	$8, $9\n"
+	/* disable "pref 30" on buggy CPUs */
+	"	lui	$9, 0x0800\n"
+	"	or	$8, $9\n"
+	"	.word	0x4088b008\n"	/* mtc0 $8, $22, 8 */
+	: : : "$8", "$9");
+}
+
 static const struct bmips_quirk bmips_quirk_list[] = {
 	{ "brcm,bcm3384-viper",		&bcm3384_viper_quirks		},
 	{ "brcm,bcm33843-viper",	&bcm3384_viper_quirks		},
 	{ "brcm,bcm6328",		&bcm6328_quirks			},
 	{ "brcm,bcm6368",		&bcm6368_quirks			},
 	{ "brcm,bcm63168",		&bcm6368_quirks			},
+	{ "brcm,bcm7344",		&bmips5000_pref30_quirk		},
+	{ "brcm,bcm7346",		&bmips5000_pref30_quirk		},
+	{ "brcm,bcm7425",		&bmips5000_pref30_quirk		},
 	{ },
 };
 
-- 
2.1.0

[PATCH 2/6] MIPS: BMIPS: Add early CPU initialization code

[Florian Fainelli]

Port the stblinux-3.3 code to perform a bunch of CPU-specific initialization,
make it compatible with run-time detection of the CPU, and unroll the
brcmstb-specific macros: BDEV_RB(), BDEV_UNSET.

The "pref 30" disabling is done as a quirk. This is a preliminary change to
allow the use of the "rotr" instruction gated by cpu_has_rixi.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/mips/bmips/setup.c       |  1 +
 arch/mips/include/asm/bmips.h |  1 +
 arch/mips/kernel/smp-bmips.c  | 87 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 89 insertions(+)

diff --git a/arch/mips/bmips/setup.c b/arch/mips/bmips/setup.c
index 9c8f15daf5e9..de31559f6f72 100644
--- a/arch/mips/bmips/setup.c
+++ b/arch/mips/bmips/setup.c
@@ -127,6 +127,7 @@ static const struct bmips_quirk bmips_quirk_list[] = {
 
 void __init prom_init(void)
 {
+	bmips_cpu_setup();
 	register_bmips_smp_ops();
 }
 
diff --git a/arch/mips/include/asm/bmips.h b/arch/mips/include/asm/bmips.h
index 6d25ad33ec78..a92aee7b977a 100644
--- a/arch/mips/include/asm/bmips.h
+++ b/arch/mips/include/asm/bmips.h
@@ -88,6 +88,7 @@ extern unsigned long bmips_tp1_irqs;
 
 extern void bmips_ebase_setup(void);
 extern asmlinkage void plat_wired_tlb_setup(void);
+extern void bmips_cpu_setup(void);
 
 static inline unsigned long bmips_read_zscm_reg(unsigned int offset)
 {
diff --git a/arch/mips/kernel/smp-bmips.c b/arch/mips/kernel/smp-bmips.c
index 78cf8c2f1de0..6835cb13ea9e 100644
--- a/arch/mips/kernel/smp-bmips.c
+++ b/arch/mips/kernel/smp-bmips.c
@@ -565,3 +565,90 @@ asmlinkage void __weak plat_wired_tlb_setup(void)
 	 * once the wired entries are present.
 	 */
 }
+
+void __init bmips_cpu_setup(void)
+{
+	void __iomem __maybe_unused *cbr = BMIPS_GET_CBR();
+	u32 __maybe_unused cfg;
+
+	switch (current_cpu_type()) {
+	case CPU_BMIPS3300:
+		/* Set BIU to async mode */
+		set_c0_brcm_bus_pll(BIT(22));
+		__sync();
+
+		/* put the BIU back in sync mode */
+		clear_c0_brcm_bus_pll(BIT(22));
+
+		/* clear BHTD to enable branch history table */
+		clear_c0_brcm_reset(BIT(16));
+
+		/* Flush and enable RAC */
+		cfg = __raw_readl(cbr + BMIPS_RAC_CONFIG);
+		__raw_writel(cfg | 0x100, BMIPS_RAC_CONFIG);
+		__raw_readl(cbr + BMIPS_RAC_CONFIG);
+
+		cfg = __raw_readl(cbr + BMIPS_RAC_CONFIG);
+		__raw_writel(cfg | 0xf, BMIPS_RAC_CONFIG);
+		__raw_readl(cbr + BMIPS_RAC_CONFIG);
+
+		cfg = __raw_readl(cbr + BMIPS_RAC_ADDRESS_RANGE);
+		__raw_writel(cfg | 0x0fff0000, cbr + BMIPS_RAC_ADDRESS_RANGE);
+		__raw_readl(cbr + BMIPS_RAC_ADDRESS_RANGE);
+		break;
+
+	case CPU_BMIPS4380:
+		/* CBG workaround for early BMIPS4380 CPUs */
+		switch (read_c0_prid()) {
+		case 0x2a040:
+		case 0x2a042:
+		case 0x2a044:
+		case 0x2a060:
+			cfg = __raw_readl(cbr + BMIPS_L2_CONFIG);
+			__raw_writel(cfg & ~0x07000000, cbr + BMIPS_L2_CONFIG);
+			__raw_readl(cbr + BMIPS_L2_CONFIG);
+		}
+
+		/* clear BHTD to enable branch history table */
+		clear_c0_brcm_config_0(BIT(21));
+
+		/* XI/ROTR enable */
+		set_c0_brcm_config_0(BIT(23));
+		set_c0_brcm_cmt_ctrl(BIT(15));
+		break;
+
+	case CPU_BMIPS5000:
+		/* enable RDHWR, BRDHWR */
+		set_c0_brcm_config(BIT(17) | BIT(21));
+
+		/* Disable JTB */
+		__asm__ __volatile__(
+		"	.set	noreorder\n"
+		"	li	$8, 0x5a455048\n"
+		"	.word	0x4088b00f\n"	/* mtc0	t0, $22, 15 */
+		"	.word	0x4008b008\n"	/* mfc0	t0, $22, 8 */
+		"	li	$9, 0x00008000\n"
+		"	or	$8, $8, $9\n"
+		"	.word	0x4088b008\n"	/* mtc0	t0, $22, 8 */
+		"	sync\n"
+		"	li	$8, 0x0\n"
+		"	.word	0x4088b00f\n"	/* mtc0	t0, $22, 15 */
+		"	.set	reorder\n"
+		: : : "$8", "$9");
+
+		/* XI enable */
+		set_c0_brcm_config(BIT(27));
+
+		/* enable MIPS32R2 ROR instruction for XI TLB handlers */
+		__asm__ __volatile__(
+		"	li	$8, 0x5a455048\n"
+		"	.word	0x4088b00f\n"	/* mtc0 $8, $22, 15 */
+		"	nop; nop; nop\n"
+		"	.word	0x4008b008\n"	/* mfc0 $8, $22, 8 */
+		"	lui	$9, 0x0100\n"
+		"	or	$8, $9\n"
+		"	.word	0x4088b008\n"	/* mtc0 $8, $22, 8 */
+		: : : "$8", "$9");
+		break;
+	}
+}
-- 
2.1.0

[PATCH 3/6] MIPS: Allow RIXI to be used on non-R2 or R6 cores

[Florian Fainelli]

Some processors, like Broadcom's BMIPS4380 and BMIPS5000 support RIXI and the
"rotr" instruction, which can be used to get a slightly more efficient page
table layout.

Introduce a CONFIG_CPU_HAS_RIXI such that those cores can benefit from this
feature. Perform the conditional check updates where relevant.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/mips/Kconfig                    |  5 +++++
 arch/mips/include/asm/pgtable-bits.h | 11 ++++++-----
 arch/mips/mm/tlbex.c                 |  2 +-
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 0f6b20a702fe..29f5b3138d6b 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -1963,11 +1963,13 @@ config CPU_MIPSR2
 	bool
 	default y if CPU_MIPS32_R2 || CPU_MIPS64_R2 || CPU_CAVIUM_OCTEON
 	select MIPS_SPRAM
+	select CPU_HAS_RIXI
 
 config CPU_MIPSR6
 	bool
 	default y if CPU_MIPS32_R6 || CPU_MIPS64_R6
 	select MIPS_SPRAM
+	select CPU_HAS_RIXI
 
 config EVA
 	bool
@@ -2378,6 +2380,9 @@ config CPU_HAS_WB
 config XKS01
 	bool
 
+config CPU_HAS_RIXI
+	bool
+
 #
 # Vectored interrupt mode is an R2 feature
 #
diff --git a/arch/mips/include/asm/pgtable-bits.h b/arch/mips/include/asm/pgtable-bits.h
index 97b313882678..0712b9b2e13d 100644
--- a/arch/mips/include/asm/pgtable-bits.h
+++ b/arch/mips/include/asm/pgtable-bits.h
@@ -112,8 +112,9 @@
  */
 #define _PAGE_PRESENT_SHIFT	0
 #define _PAGE_PRESENT		(1 << _PAGE_PRESENT_SHIFT)
-/* R2 or later cores check for RI/XI support to determine _PAGE_READ */
-#if defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_MIPSR6)
+/* R2, specific processors or later cores check for RI/XI support to determine
+ * _PAGE_READ */
+#ifdef CONFIG_CPU_HAS_RIXI
 #define _PAGE_WRITE_SHIFT	(_PAGE_PRESENT_SHIFT + 1)
 #define _PAGE_WRITE		(1 << _PAGE_WRITE_SHIFT)
 #else
@@ -133,7 +134,7 @@
 #define _PAGE_HUGE		(1 << _PAGE_HUGE_SHIFT)
 #endif	/* CONFIG_64BIT && CONFIG_MIPS_HUGE_TLB_SUPPORT */
 
-#if defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_MIPSR6)
+#ifdef CONFIG_CPU_HAS_RIXI
 /* XI - page cannot be executed */
 #ifdef _PAGE_HUGE_SHIFT
 #define _PAGE_NO_EXEC_SHIFT	(_PAGE_HUGE_SHIFT + 1)
@@ -147,7 +148,7 @@
 #define _PAGE_READ		(cpu_has_rixi ? 0 : (1 << _PAGE_READ_SHIFT))
 #define _PAGE_NO_READ_SHIFT	_PAGE_READ_SHIFT
 #define _PAGE_NO_READ		(cpu_has_rixi ? (1 << _PAGE_READ_SHIFT) : 0)
-#endif	/* defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_MIPSR6) */
+#endif	/* CONFIG_CPU_HAS_RIXI */
 
 #if defined(_PAGE_NO_READ_SHIFT)
 #define _PAGE_GLOBAL_SHIFT	(_PAGE_NO_READ_SHIFT + 1)
@@ -198,7 +199,7 @@
  */
 static inline uint64_t pte_to_entrylo(unsigned long pte_val)
 {
-#if defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_MIPSR6)
+#ifdef CONFIG_CPU_HAS_RIXI
 	if (cpu_has_rixi) {
 		int sa;
 #ifdef CONFIG_32BIT
diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
index 5a04b6f5c6fb..2efb85cdfa72 100644
--- a/arch/mips/mm/tlbex.c
+++ b/arch/mips/mm/tlbex.c
@@ -241,7 +241,7 @@ static void output_pgtable_bits_defines(void)
 #ifdef CONFIG_MIPS_HUGE_TLB_SUPPORT
 	pr_define("_PAGE_HUGE_SHIFT %d\n", _PAGE_HUGE_SHIFT);
 #endif
-#if defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_MIPSR6)
+#ifdef CONFIG_CPU_HAS_RIXI
 	if (cpu_has_rixi) {
 #ifdef _PAGE_NO_EXEC_SHIFT
 		pr_define("_PAGE_NO_EXEC_SHIFT %d\n", _PAGE_NO_EXEC_SHIFT);
-- 
2.1.0

[PATCH 4/6] MIPS: Move RIXI exception enabling after vendor-specific cpu_probe

[Florian Fainelli]

Some processors may not have the RIXI bit advertised in the Config3 register,
not being a MIPS32R2 or R6 core, yet, they might be supporting it through a
different way, which is overriden during vendor-specific cpu_probe().

Move the RIXI exceptions enabling after the vendor-specific cpu_probe()
function has had a change to run and override the current CPU's options with
MIPS_CPU_RIXI.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/mips/kernel/cpu-probe.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
index b725b713b9f8..a2df0357b453 100644
--- a/arch/mips/kernel/cpu-probe.c
+++ b/arch/mips/kernel/cpu-probe.c
@@ -828,15 +828,6 @@ static void decode_configs(struct cpuinfo_mips *c)
 
 	mips_probe_watch_registers(c);
 
-	if (cpu_has_rixi) {
-		/* Enable the RIXI exceptions */
-		set_c0_pagegrain(PG_IEC);
-		back_to_back_c0_hazard();
-		/* Verify the IEC bit is set */
-		if (read_c0_pagegrain() & PG_IEC)
-			c->options |= MIPS_CPU_RIXIEX;
-	}
-
 #ifndef CONFIG_MIPS_CPS
 	if (cpu_has_mips_r2_r6) {
 		c->core = get_ebase_cpunum();
@@ -1660,6 +1651,15 @@ void cpu_probe(void)
 	 */
 	BUG_ON(current_cpu_type() != c->cputype);
 
+	if (cpu_has_rixi) {
+		/* Enable the RIXI exceptions */
+		set_c0_pagegrain(PG_IEC);
+		back_to_back_c0_hazard();
+		/* Verify the IEC bit is set */
+		if (read_c0_pagegrain() & PG_IEC)
+			c->options |= MIPS_CPU_RIXIEX;
+	}
+
 	if (mips_fpu_disabled)
 		c->options &= ~MIPS_CPU_FPU;
 
-- 
2.1.0

[PATCH 5/6] MIPS: BMIPS: BMIPS4380 and BMIPS5000 support RIXI

[Florian Fainelli]

Make BMIPS4380 and BMIPS5000 advertise support for RIXI through
cpu_probe_broadcom(). bmips_cpu_setup() needs to be called shortly after that,
during prom_init() in order to enable the proper Broadcom-specific register to
turn on RIXI and the "rotr" instruction.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/mips/Kconfig            | 2 ++
 arch/mips/kernel/cpu-probe.c | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 29f5b3138d6b..b03a68879b6a 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -1799,6 +1799,7 @@ config CPU_BMIPS4380
 	select MIPS_L1_CACHE_SHIFT_6
 	select SYS_SUPPORTS_SMP
 	select SYS_SUPPORTS_HOTPLUG_CPU
+	select CPU_HAS_RIXI
 
 config CPU_BMIPS5000
 	bool
@@ -1806,6 +1807,7 @@ config CPU_BMIPS5000
 	select MIPS_L1_CACHE_SHIFT_7
 	select SYS_SUPPORTS_SMP
 	select SYS_SUPPORTS_HOTPLUG_CPU
+	select CPU_HAS_RIXI
 
 config SYS_HAS_CPU_LOONGSON3
 	bool
diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
index a2df0357b453..cf9869d15c9f 100644
--- a/arch/mips/kernel/cpu-probe.c
+++ b/arch/mips/kernel/cpu-probe.c
@@ -1426,6 +1426,7 @@ static inline void cpu_probe_broadcom(struct cpuinfo_mips *c, unsigned int cpu)
 			c->cputype = CPU_BMIPS4380;
 			__cpu_name[cpu] = "Broadcom BMIPS4380";
 			set_elf_platform(cpu, "bmips4380");
+			c->options |= MIPS_CPU_RIXI;
 		} else {
 			c->cputype = CPU_BMIPS4350;
 			__cpu_name[cpu] = "Broadcom BMIPS4350";
@@ -1438,7 +1439,7 @@ static inline void cpu_probe_broadcom(struct cpuinfo_mips *c, unsigned int cpu)
 		c->cputype = CPU_BMIPS5000;
 		__cpu_name[cpu] = "Broadcom BMIPS5000";
 		set_elf_platform(cpu, "bmips5000");
-		c->options |= MIPS_CPU_ULRI;
+		c->options |= MIPS_CPU_ULRI | MIPS_CPU_RIXI;
 		break;
 	}
 }
-- 
2.1.0

[PATCH 6/6] MIPS: Expose current_cpu_data.options through debugfs

[Florian Fainelli]

Debugging a missing features in cpu-features-override.h, or a runtime feature
set/clear in the vendor specific cpu_probe() function can be a little tedious,
ease that by providing a debugfs entry representing the
current_cpu_data.options bitmask.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/mips/kernel/cpu-probe.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
index cf9869d15c9f..4b569e13f727 100644
--- a/arch/mips/kernel/cpu-probe.c
+++ b/arch/mips/kernel/cpu-probe.c
@@ -17,6 +17,7 @@
 #include <linux/smp.h>
 #include <linux/stddef.h>
 #include <linux/export.h>
+#include <linux/debugfs.h>
 
 #include <asm/bugs.h>
 #include <asm/cpu.h>
@@ -1719,3 +1720,22 @@ void cpu_report(void)
 	if (cpu_has_msa)
 		pr_info("MSA revision is: %08x\n", c->msa_id);
 }
+
+#ifdef CONFIG_DEBUG_FS
+extern struct dentry *mips_debugfs_dir;
+static int __init debugfs_cpu_options(void)
+{
+	struct dentry *d;
+
+	if (!mips_debugfs_dir)
+		return -ENODEV;
+
+	d = debugfs_create_x64("cpu_options", S_IRUGO,
+			       mips_debugfs_dir, &current_cpu_data.options);
+	if (!d)
+		return -ENOMEM;
+
+	return 0;
+}
+__initcall(debugfs_cpu_options);
+#endif
-- 
2.1.0

Re: [PATCH 1/6] MIPS: BMIPS: Disable pref 30 for buggy CPUs

[Florian Fainelli]

On 09/02/16 12:55, Florian Fainelli wrote:
> Disable pref 30 by utilizing the standard quirk method and matching the
> affected SoCs: 7344, 7436, 7425.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  arch/mips/bmips/setup.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/arch/mips/bmips/setup.c b/arch/mips/bmips/setup.c
> index 35535284b39e..9c8f15daf5e9 100644
> --- a/arch/mips/bmips/setup.c
> +++ b/arch/mips/bmips/setup.c
> @@ -100,12 +100,28 @@ static void bcm6368_quirks(void)
>  	bcm63xx_fixup_cpu1();
>  }
>  
> +static void bmips5000_pref30_quirk(void)
> +{
> +	__asm__ __volatile__(
> +	"	.word	0x4008b008\n"	/* mfc0 $8, $22, 8 */
> +	"	lui	$9, 0x0100\n"
> +	"	or	$8, $9\n"
> +	/* disable "pref 30" on buggy CPUs */
> +	"	lui	$9, 0x0800\n"
> +	"	or	$8, $9\n"
> +	"	.word	0x4088b008\n"	/* mtc0 $8, $22, 8 */
> +	: : : "$8", "$9");

We are missing an additional load here to $8, I will respin this patch,
but would appreciate feedback on the other patches of the series so I
can address everything at once. Thanks!

> +}
> +
>  static const struct bmips_quirk bmips_quirk_list[] = {
>  	{ "brcm,bcm3384-viper",		&bcm3384_viper_quirks		},
>  	{ "brcm,bcm33843-viper",	&bcm3384_viper_quirks		},
>  	{ "brcm,bcm6328",		&bcm6328_quirks			},
>  	{ "brcm,bcm6368",		&bcm6368_quirks			},
>  	{ "brcm,bcm63168",		&bcm6368_quirks			},
> +	{ "brcm,bcm7344",		&bmips5000_pref30_quirk		},
> +	{ "brcm,bcm7346",		&bmips5000_pref30_quirk		},
> +	{ "brcm,bcm7425",		&bmips5000_pref30_quirk		},
>  	{ },
>  };
>  
> 


-- 
Florian

Re: [PATCH 1/6] MIPS: BMIPS: Disable pref 30 for buggy CPUs

[Maciej W. Rozycki]

On Tue, 9 Feb 2016, Florian Fainelli wrote:

> +static void bmips5000_pref30_quirk(void)
> +{
> +	__asm__ __volatile__(
> +	"	.word	0x4008b008\n"	/* mfc0 $8, $22, 8 */
> +	"	lui	$9, 0x0100\n"
> +	"	or	$8, $9\n"
> +	/* disable "pref 30" on buggy CPUs */
> +	"	lui	$9, 0x0800\n"
> +	"	or	$8, $9\n"
> +	"	.word	0x4088b008\n"	/* mtc0 $8, $22, 8 */
> +	: : : "$8", "$9");
> +}

 Ouch, why not using our standard accessors and avoid magic numbers, e.g.:

#define read_c0_brcm_whateverthisiscalled() \
	__read_32bit_c0_register($22, 8)
#define write_c0_brcm_whateverthisiscalled(val) \
	__write_32bit_c0_register($22, 8, val)

#define BMIPS5000_WHATEVERTHISISCALLED_BIT_X 0x0100
#define BMIPS5000_WHATEVERTHISISCALLED_BIT_Y 0x0800

static void bmips5000_pref30_quirk(void)
{
	unsigned int whateverthisiscalled;

	whateverthisiscalled = read_c0_brcm_whateverthisiscalled();
	whateverthisiscalled |= BMIPS_WHATEVERTHISISCALLED_BIT_X;
	/* disable "pref 30" on buggy CPUs */
	whateverthisiscalled |= BMIPS_WHATEVERTHISISCALLED_BIT_Y;
	write_c0_brcm_whateverthisiscalled(whateverthisiscalled);
}

?  I'm leaving it up to you to select the right names here -- just about 
anything will be better than the halfway-binary piece you have proposed.

 FYI, we already have BMIPS5000 accessors defined up to ($22, 7) in 
<asm/mipsregs.h> so it will be the right place for the new ones as well.

  Maciej

Re: [PATCH 1/6] MIPS: BMIPS: Disable pref 30 for buggy CPUs

[Maciej W. Rozycki]

On Tue, 9 Feb 2016, Florian Fainelli wrote:

> +static void bmips5000_pref30_quirk(void)
> +{
> +	__asm__ __volatile__(
> +	"	.word	0x4008b008\n"	/* mfc0 $8, $22, 8 */
> +	"	lui	$9, 0x0100\n"
> +	"	or	$8, $9\n"
> +	/* disable "pref 30" on buggy CPUs */
> +	"	lui	$9, 0x0800\n"
> +	"	or	$8, $9\n"
> +	"	.word	0x4088b008\n"	/* mtc0 $8, $22, 8 */
> +	: : : "$8", "$9");
> +}

 Ouch, why not using our standard accessors and avoid magic numbers, e.g.:

#define read_c0_brcm_whateverthisiscalled() \
	__read_32bit_c0_register($22, 8)
#define write_c0_brcm_whateverthisiscalled(val) \
	__write_32bit_c0_register($22, 8, val)

#define BMIPS5000_WHATEVERTHISISCALLED_BIT_X 0x0100
#define BMIPS5000_WHATEVERTHISISCALLED_BIT_Y 0x0800

static void bmips5000_pref30_quirk(void)
{
	unsigned int whateverthisiscalled;

	whateverthisiscalled = read_c0_brcm_whateverthisiscalled();
	whateverthisiscalled |= BMIPS_WHATEVERTHISISCALLED_BIT_X;
	/* disable "pref 30" on buggy CPUs */
	whateverthisiscalled |= BMIPS_WHATEVERTHISISCALLED_BIT_Y;
	write_c0_brcm_whateverthisiscalled(whateverthisiscalled);
}

?  I'm leaving it up to you to select the right names here -- just about 
anything will be better than the halfway-binary piece you have proposed.

 FYI, we already have BMIPS5000 accessors defined up to ($22, 7) in 
<asm/mipsregs.h> so it will be the right place for the new ones as well.

  Maciej

Re: [PATCH 1/6] MIPS: BMIPS: Disable pref 30 for buggy CPUs

[Florian Fainelli]

On 09/02/16 13:19, Maciej W. Rozycki wrote:
> On Tue, 9 Feb 2016, Florian Fainelli wrote:
> 
>> +static void bmips5000_pref30_quirk(void)
>> +{
>> +	__asm__ __volatile__(
>> +	"	.word	0x4008b008\n"	/* mfc0 $8, $22, 8 */
>> +	"	lui	$9, 0x0100\n"
>> +	"	or	$8, $9\n"
>> +	/* disable "pref 30" on buggy CPUs */
>> +	"	lui	$9, 0x0800\n"
>> +	"	or	$8, $9\n"
>> +	"	.word	0x4088b008\n"	/* mtc0 $8, $22, 8 */
>> +	: : : "$8", "$9");
>> +}
> 
>  Ouch, why not using our standard accessors and avoid magic numbers, e.g.:

Are you positive the assembler will not barf on these custom cp0 reg 22
selectors?

> 
> #define read_c0_brcm_whateverthisiscalled() \
> 	__read_32bit_c0_register($22, 8)
> #define write_c0_brcm_whateverthisiscalled(val) \
> 	__write_32bit_c0_register($22, 8, val)
> 
> #define BMIPS5000_WHATEVERTHISISCALLED_BIT_X 0x0100
> #define BMIPS5000_WHATEVERTHISISCALLED_BIT_Y 0x0800
> 
> static void bmips5000_pref30_quirk(void)
> {
> 	unsigned int whateverthisiscalled;
> 
> 	whateverthisiscalled = read_c0_brcm_whateverthisiscalled();
> 	whateverthisiscalled |= BMIPS_WHATEVERTHISISCALLED_BIT_X;
> 	/* disable "pref 30" on buggy CPUs */
> 	whateverthisiscalled |= BMIPS_WHATEVERTHISISCALLED_BIT_Y;
> 	write_c0_brcm_whateverthisiscalled(whateverthisiscalled);
> }
> 
> ?  I'm leaving it up to you to select the right names here -- just about 
> anything will be better than the halfway-binary piece you have proposed.

These are not standardized registers in any form, which is why these are
in a halfway-binary form, they are not meant to be re-used outside of
two known places: disabling pref30 and enabling "rotr".

> 
>  FYI, we already have BMIPS5000 accessors defined up to ($22, 7) in 
> <asm/mipsregs.h> so it will be the right place for the new ones as well.
> 
>   Maciej
> 


-- 
Florian

Re: [PATCH 1/6] MIPS: BMIPS: Disable pref 30 for buggy CPUs

[Petri Gynther]

On Tue, Feb 9, 2016 at 1:01 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 09/02/16 12:55, Florian Fainelli wrote:
>> Disable pref 30 by utilizing the standard quirk method and matching the
>> affected SoCs: 7344, 7436, 7425.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>>  arch/mips/bmips/setup.c | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/arch/mips/bmips/setup.c b/arch/mips/bmips/setup.c
>> index 35535284b39e..9c8f15daf5e9 100644
>> --- a/arch/mips/bmips/setup.c
>> +++ b/arch/mips/bmips/setup.c
>> @@ -100,12 +100,28 @@ static void bcm6368_quirks(void)
>>       bcm63xx_fixup_cpu1();
>>  }
>>
>> +static void bmips5000_pref30_quirk(void)
>> +{
>> +     __asm__ __volatile__(
>> +     "       .word   0x4008b008\n"   /* mfc0 $8, $22, 8 */
>> +     "       lui     $9, 0x0100\n"
>> +     "       or      $8, $9\n"
>> +     /* disable "pref 30" on buggy CPUs */
>> +     "       lui     $9, 0x0800\n"
>> +     "       or      $8, $9\n"
>> +     "       .word   0x4088b008\n"   /* mtc0 $8, $22, 8 */
>> +     : : : "$8", "$9");
>
> We are missing an additional load here to $8, I will respin this patch,
> but would appreciate feedback on the other patches of the series so I
> can address everything at once. Thanks!
>

In our codebase, we currently have the following workarounds:
LEAF(set_zephyr)
        .set    noreorder

        /* enable read/write of CP0 #22 sel. 8 */
        li      t0, 0x5a455048
        .word   0x4088b00f      /* mtc0    t0, $22, 15 */

        .word   0x4008b008      /* mfc0    t0, $22, 8 */
        li      t1, 0x09000000  /* turn off pref */
        or      t0, t0, t1
        .word   0x4088b008      /* mtc0    t0, $22, 8 */
        sync

        /* disable read/write of CP0 #22 sel 8 */
        li      t0, 0x0
        .word   0x4088b00f      /* mtc0    t0, $22, 15 */


        jr      ra
        nop
        .set reorder

END(set_zephyr)


/* enable MIPS32R2 ROR instruction for XI TLB handlers */
        __asm__ __volatile__(
        "       li      $8, 0x5a455048\n"
        "       .word   0x4088b00f\n"   /* mtc0 $8, $22, 15 */
        "       nop; nop; nop\n"
        "       .word   0x4008b008\n"   /* mfc0 $8, $22, 8 */
        "       lui     $9, 0x0100\n"
        "       or      $8, $9\n"
        "       .word   0x4088b008\n"   /* mtc0 $8, $22, 8 */
        "       sync\n"
        "       li      $8, 0x0\n"
        "       .word   0x4088b00f\n"
        "       nop; nop; nop\n"
        : : : "$8", "$9");

#if defined(CONFIG_BCM7425)
        /* Disable PREF 30 */
        __asm__ __volatile__(
        "       li      $8, 0x5a455048\n"
        "       .word   0x4088b00f\n"
        "       nop; nop; nop\n"
        "       .word   0x4008b008\n"
        "       lui     $9, 0x0800\n"
        "       or      $8, $8, $9\n"
        "       .word   0x4088b008\n"
        "       sync\n"
        "       li      $8, 0x0\n"
        "       .word   0x4088b00f\n"
        "       nop; nop; nop\n"
        : : : "$8", "$9");
#endif

#if defined(CONFIG_BCM7425) || defined(CONFIG_BCM7429)
        /* Disable JTB and CRS */
        __asm__ __volatile__(
        "       li      $8, 0x5a455048\n"
        "       .word   0x4088b00f\n"
        "       nop; nop; nop\n"
        "       .word   0x4008b008\n"
        "       li      $9, 0xfbffffff\n"
        "       and     $8, $8, $9\n"
        "       li      $9, 0x0400c000\n"
        "       or      $8, $8, $9\n"
        "       .word   0x4088b008\n"
        "       sync\n"
        "       li      $8, 0x0\n"
        "       .word   0x4088b00f\n"
        "       nop; nop; nop\n"
        : : : "$8", "$9");
#endif


It looks like set_zephyr() does the same as the next two ("enable
MIPS32R2 ROR instruction for XI TLB handlers" + "Disable PREF 30")
combined?

Are you planning to add the JTB and CRS workaround?


>> +}
>> +
>>  static const struct bmips_quirk bmips_quirk_list[] = {
>>       { "brcm,bcm3384-viper",         &bcm3384_viper_quirks           },
>>       { "brcm,bcm33843-viper",        &bcm3384_viper_quirks           },
>>       { "brcm,bcm6328",               &bcm6328_quirks                 },
>>       { "brcm,bcm6368",               &bcm6368_quirks                 },
>>       { "brcm,bcm63168",              &bcm6368_quirks                 },
>> +     { "brcm,bcm7344",               &bmips5000_pref30_quirk         },
>> +     { "brcm,bcm7346",               &bmips5000_pref30_quirk         },
>> +     { "brcm,bcm7425",               &bmips5000_pref30_quirk         },
>>       { },
>>  };
>>
>>
>
>
> --
> Florian

Re: [PATCH 1/6] MIPS: BMIPS: Disable pref 30 for buggy CPUs

[Florian Fainelli]

On 09/02/16 15:42, Petri Gynther wrote:
> On Tue, Feb 9, 2016 at 1:01 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>> On 09/02/16 12:55, Florian Fainelli wrote:
>>> Disable pref 30 by utilizing the standard quirk method and matching the
>>> affected SoCs: 7344, 7436, 7425.
>>>
>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>>> ---
>>>  arch/mips/bmips/setup.c | 16 ++++++++++++++++
>>>  1 file changed, 16 insertions(+)
>>>
>>> diff --git a/arch/mips/bmips/setup.c b/arch/mips/bmips/setup.c
>>> index 35535284b39e..9c8f15daf5e9 100644
>>> --- a/arch/mips/bmips/setup.c
>>> +++ b/arch/mips/bmips/setup.c
>>> @@ -100,12 +100,28 @@ static void bcm6368_quirks(void)
>>>       bcm63xx_fixup_cpu1();
>>>  }
>>>
>>> +static void bmips5000_pref30_quirk(void)
>>> +{
>>> +     __asm__ __volatile__(
>>> +     "       .word   0x4008b008\n"   /* mfc0 $8, $22, 8 */
>>> +     "       lui     $9, 0x0100\n"
>>> +     "       or      $8, $9\n"
>>> +     /* disable "pref 30" on buggy CPUs */
>>> +     "       lui     $9, 0x0800\n"
>>> +     "       or      $8, $9\n"
>>> +     "       .word   0x4088b008\n"   /* mtc0 $8, $22, 8 */
>>> +     : : : "$8", "$9");
>>
>> We are missing an additional load here to $8, I will respin this patch,
>> but would appreciate feedback on the other patches of the series so I
>> can address everything at once. Thanks!
>>
> 
> In our codebase, we currently have the following workarounds:
> LEAF(set_zephyr)
>         .set    noreorder
> 
>         /* enable read/write of CP0 #22 sel. 8 */
>         li      t0, 0x5a455048
>         .word   0x4088b00f      /* mtc0    t0, $22, 15 */
> 
>         .word   0x4008b008      /* mfc0    t0, $22, 8 */
>         li      t1, 0x09000000  /* turn off pref */
>         or      t0, t0, t1
>         .word   0x4088b008      /* mtc0    t0, $22, 8 */
>         sync
> 
>         /* disable read/write of CP0 #22 sel 8 */
>         li      t0, 0x0
>         .word   0x4088b00f      /* mtc0    t0, $22, 15 */
> 
> 
>         jr      ra
>         nop
>         .set reorder
> 
> END(set_zephyr)
> 
> 
> /* enable MIPS32R2 ROR instruction for XI TLB handlers */
>         __asm__ __volatile__(
>         "       li      $8, 0x5a455048\n"
>         "       .word   0x4088b00f\n"   /* mtc0 $8, $22, 15 */
>         "       nop; nop; nop\n"
>         "       .word   0x4008b008\n"   /* mfc0 $8, $22, 8 */
>         "       lui     $9, 0x0100\n"
>         "       or      $8, $9\n"
>         "       .word   0x4088b008\n"   /* mtc0 $8, $22, 8 */
>         "       sync\n"
>         "       li      $8, 0x0\n"
>         "       .word   0x4088b00f\n"
>         "       nop; nop; nop\n"
>         : : : "$8", "$9");
> 
> #if defined(CONFIG_BCM7425)
>         /* Disable PREF 30 */
>         __asm__ __volatile__(
>         "       li      $8, 0x5a455048\n"
>         "       .word   0x4088b00f\n"
>         "       nop; nop; nop\n"
>         "       .word   0x4008b008\n"
>         "       lui     $9, 0x0800\n"
>         "       or      $8, $8, $9\n"
>         "       .word   0x4088b008\n"
>         "       sync\n"
>         "       li      $8, 0x0\n"
>         "       .word   0x4088b00f\n"
>         "       nop; nop; nop\n"
>         : : : "$8", "$9");
> #endif
> 
> #if defined(CONFIG_BCM7425) || defined(CONFIG_BCM7429)
>         /* Disable JTB and CRS */
>         __asm__ __volatile__(
>         "       li      $8, 0x5a455048\n"
>         "       .word   0x4088b00f\n"
>         "       nop; nop; nop\n"
>         "       .word   0x4008b008\n"
>         "       li      $9, 0xfbffffff\n"
>         "       and     $8, $8, $9\n"
>         "       li      $9, 0x0400c000\n"
>         "       or      $8, $8, $9\n"
>         "       .word   0x4088b008\n"
>         "       sync\n"
>         "       li      $8, 0x0\n"
>         "       .word   0x4088b00f\n"
>         "       nop; nop; nop\n"
>         : : : "$8", "$9");
> #endif
> 
> 
> It looks like set_zephyr() does the same as the next two ("enable
> MIPS32R2 ROR instruction for XI TLB handlers" + "Disable PREF 30")
> combined?

The downstream stblinux kernel has separate build options for each SoC
(which I am trying to move away from), so we cannot just fold the two
writes into the same function, the point here is to use the
BMIPS_GENERIC quirk list to disable pref 30, which is per-chip, and do
the common Zephyr initialization during bmips_cpu_setup().

> 
> Are you planning to add the JTB and CRS workaround?

They should be in patch 2 AFAICT.
-- 
Florian

Re: [PATCH 1/6] MIPS: BMIPS: Disable pref 30 for buggy CPUs

[Maciej W. Rozycki]

On Tue, 9 Feb 2016, Florian Fainelli wrote:

> >> +static void bmips5000_pref30_quirk(void)
> >> +{
> >> +	__asm__ __volatile__(
> >> +	"	.word	0x4008b008\n"	/* mfc0 $8, $22, 8 */
> >> +	"	lui	$9, 0x0100\n"
> >> +	"	or	$8, $9\n"
> >> +	/* disable "pref 30" on buggy CPUs */
> >> +	"	lui	$9, 0x0800\n"
> >> +	"	or	$8, $9\n"
> >> +	"	.word	0x4088b008\n"	/* mtc0 $8, $22, 8 */
> >> +	: : : "$8", "$9");
> >> +}
> > 
> >  Ouch, why not using our standard accessors and avoid magic numbers, e.g.:
> 
> Are you positive the assembler will not barf on these custom cp0 reg 22
> selectors?

 Indeed, I missed that this is beyond the usual select range of 0-7.  
Sorry about that.

 That does not mean it shouldn't be done in a cleaner way, stashing the 
obscurity in one place only.  I did notice a similar piece in one of your 
other patches, which is a strong argument for abstracting it.

 So first, are you aware of support for these Broadcom instruction 
encoding extensions being considered for inclusion in binutils?  I'll be 
happy to accept a patch and AFAICT it's a simple extension of the `sel' 
field within the existing MFC0/MTC0 instruction definitions.

 Second, regardless we need to abstract this in a reusable way while we 
don't have such support in the assembler.  So here:

> > #define read_c0_brcm_whateverthisiscalled() \
> > 	__read_32bit_c0_register($22, 8)
> > #define write_c0_brcm_whateverthisiscalled(val) \
> > 	__write_32bit_c0_register($22, 8, val)

rather than using `__read_32bit_c0_register' and 
`__write_32bit_c0_register' we can define special 
`__read_32bit_c0_brcm_register' and `__write_32bit_c0_brcm_register' 
helpers like:

#define __read_32bit_c0_brcm_register(reg, sel)				\
({									\
	register unsigned int __res asm("t0");				\
									\
	__asm__ __volatile__(						\
		/* mfc0 t0, $reg, sel */				\
		".word	0x40080000 | ((%1 & 0x1f) << 11) | (%2 & 0xf)"	\
		: "=r" (__res) : "i" (reg), "i" (sel));			\
	__res;								\
})

#define __write_32bit_c0_brcm_register(reg, sel, value)			\
do {									\
	register unsigned int __val asm("t0") = value;			\
									\
	__asm__ __volatile__(						\
		/* mtc0 t0, $reg, sel */				\
		".word  0x40880000 | ((%1 & 0x1f) << 11) | (%2 & 0xf)"	\
		: : "r" (__val), "i" (reg), "i" (sel));			\
} while (0)

(if 0xf is indeed the mask for `sel').  This is untested, but should work, 
perhaps with a minor fix somewhere if I made a typo.  It should also 
produce a little bit better code, although I realise this is a corner case 
hardly worth optimising for.  What is important is maintainability.

> > #define BMIPS5000_WHATEVERTHISISCALLED_BIT_X 0x0100
> > #define BMIPS5000_WHATEVERTHISISCALLED_BIT_Y 0x0800
> > 
> > static void bmips5000_pref30_quirk(void)
> > {
> > 	unsigned int whateverthisiscalled;
> > 
> > 	whateverthisiscalled = read_c0_brcm_whateverthisiscalled();
> > 	whateverthisiscalled |= BMIPS_WHATEVERTHISISCALLED_BIT_X;
> > 	/* disable "pref 30" on buggy CPUs */
> > 	whateverthisiscalled |= BMIPS_WHATEVERTHISISCALLED_BIT_Y;
> > 	write_c0_brcm_whateverthisiscalled(whateverthisiscalled);
> > }
> > 
> > ?  I'm leaving it up to you to select the right names here -- just about 
> > anything will be better than the halfway-binary piece you have proposed.
> 
> These are not standardized registers in any form, which is why these are
> in a halfway-binary form, they are not meant to be re-used outside of
> two known places: disabling pref30 and enabling "rotr".

 Well, if Broadcom didn't give this register any name, then `reg22_sel8' 
will be as good as any.  We don't need to invent things here, just to keep 
them maintainable.  If you call something `8', then you can't easily 
search through the tree to find references.  If you use something uniquely 
identifiable, then you can.  So:

#define read_c0_brcm_reg22_sel8()					\
	__read_32bit_c0_brcm_register(22, 8)
#define write_c0_brcm_reg22_sel8(val)					\
	__write_32bit_c0_brcm_register(22, 8, val)

(note the dropped `$' because we can't pass it along in this form).

 As to the bit names -- you already gave them: NO_PREF30 (since this is 
negated) and ROTR will be just fine, with a BMIPS5000_REG22_SEL8_ prefix.  
So:

#define BMIPS5000_REG22_SEL8_ROTR	0x0100
#define BMIPS5000_REG22_SEL8_NO_PREF30	0x0800

(why is ROTR set along NO_PREF30 BTW? -- it does not seem related).

 I hope you agree this all is reasonable from a maintainer's point of 
view.  I'll leave it up to you to make a patch out of it.  You'll then be 
able to use this stuff in 2/6 too.

 Please try to give meaningful names to the other magic numbers you 
introduce too.

  Maciej

Re: [PATCH 1/6] MIPS: BMIPS: Disable pref 30 for buggy CPUs

[Maciej W. Rozycki]

On Tue, 9 Feb 2016, Florian Fainelli wrote:

> >> +static void bmips5000_pref30_quirk(void)
> >> +{
> >> +	__asm__ __volatile__(
> >> +	"	.word	0x4008b008\n"	/* mfc0 $8, $22, 8 */
> >> +	"	lui	$9, 0x0100\n"
> >> +	"	or	$8, $9\n"
> >> +	/* disable "pref 30" on buggy CPUs */
> >> +	"	lui	$9, 0x0800\n"
> >> +	"	or	$8, $9\n"
> >> +	"	.word	0x4088b008\n"	/* mtc0 $8, $22, 8 */
> >> +	: : : "$8", "$9");
> >> +}
> > 
> >  Ouch, why not using our standard accessors and avoid magic numbers, e.g.:
> 
> Are you positive the assembler will not barf on these custom cp0 reg 22
> selectors?

 Indeed, I missed that this is beyond the usual select range of 0-7.  
Sorry about that.

 That does not mean it shouldn't be done in a cleaner way, stashing the 
obscurity in one place only.  I did notice a similar piece in one of your 
other patches, which is a strong argument for abstracting it.

 So first, are you aware of support for these Broadcom instruction 
encoding extensions being considered for inclusion in binutils?  I'll be 
happy to accept a patch and AFAICT it's a simple extension of the `sel' 
field within the existing MFC0/MTC0 instruction definitions.

 Second, regardless we need to abstract this in a reusable way while we 
don't have such support in the assembler.  So here:

> > #define read_c0_brcm_whateverthisiscalled() \
> > 	__read_32bit_c0_register($22, 8)
> > #define write_c0_brcm_whateverthisiscalled(val) \
> > 	__write_32bit_c0_register($22, 8, val)

rather than using `__read_32bit_c0_register' and 
`__write_32bit_c0_register' we can define special 
`__read_32bit_c0_brcm_register' and `__write_32bit_c0_brcm_register' 
helpers like:

#define __read_32bit_c0_brcm_register(reg, sel)				\
({									\
	register unsigned int __res asm("t0");				\
									\
	__asm__ __volatile__(						\
		/* mfc0 t0, $reg, sel */				\
		".word	0x40080000 | ((%1 & 0x1f) << 11) | (%2 & 0xf)"	\
		: "=r" (__res) : "i" (reg), "i" (sel));			\
	__res;								\
})

#define __write_32bit_c0_brcm_register(reg, sel, value)			\
do {									\
	register unsigned int __val asm("t0") = value;			\
									\
	__asm__ __volatile__(						\
		/* mtc0 t0, $reg, sel */				\
		".word  0x40880000 | ((%1 & 0x1f) << 11) | (%2 & 0xf)"	\
		: : "r" (__val), "i" (reg), "i" (sel));			\
} while (0)

(if 0xf is indeed the mask for `sel').  This is untested, but should work, 
perhaps with a minor fix somewhere if I made a typo.  It should also 
produce a little bit better code, although I realise this is a corner case 
hardly worth optimising for.  What is important is maintainability.

> > #define BMIPS5000_WHATEVERTHISISCALLED_BIT_X 0x0100
> > #define BMIPS5000_WHATEVERTHISISCALLED_BIT_Y 0x0800
> > 
> > static void bmips5000_pref30_quirk(void)
> > {
> > 	unsigned int whateverthisiscalled;
> > 
> > 	whateverthisiscalled = read_c0_brcm_whateverthisiscalled();
> > 	whateverthisiscalled |= BMIPS_WHATEVERTHISISCALLED_BIT_X;
> > 	/* disable "pref 30" on buggy CPUs */
> > 	whateverthisiscalled |= BMIPS_WHATEVERTHISISCALLED_BIT_Y;
> > 	write_c0_brcm_whateverthisiscalled(whateverthisiscalled);
> > }
> > 
> > ?  I'm leaving it up to you to select the right names here -- just about 
> > anything will be better than the halfway-binary piece you have proposed.
> 
> These are not standardized registers in any form, which is why these are
> in a halfway-binary form, they are not meant to be re-used outside of
> two known places: disabling pref30 and enabling "rotr".

 Well, if Broadcom didn't give this register any name, then `reg22_sel8' 
will be as good as any.  We don't need to invent things here, just to keep 
them maintainable.  If you call something `8', then you can't easily 
search through the tree to find references.  If you use something uniquely 
identifiable, then you can.  So:

#define read_c0_brcm_reg22_sel8()					\
	__read_32bit_c0_brcm_register(22, 8)
#define write_c0_brcm_reg22_sel8(val)					\
	__write_32bit_c0_brcm_register(22, 8, val)

(note the dropped `$' because we can't pass it along in this form).

 As to the bit names -- you already gave them: NO_PREF30 (since this is 
negated) and ROTR will be just fine, with a BMIPS5000_REG22_SEL8_ prefix.  
So:

#define BMIPS5000_REG22_SEL8_ROTR	0x0100
#define BMIPS5000_REG22_SEL8_NO_PREF30	0x0800

(why is ROTR set along NO_PREF30 BTW? -- it does not seem related).

 I hope you agree this all is reasonable from a maintainer's point of 
view.  I'll leave it up to you to make a patch out of it.  You'll then be 
able to use this stuff in 2/6 too.

 Please try to give meaningful names to the other magic numbers you 
introduce too.

  Maciej

Re: [PATCH 1/6] MIPS: BMIPS: Disable pref 30 for buggy CPUs

[Florian Fainelli]

On 09/02/16 15:52, Maciej W. Rozycki wrote:
> On Tue, 9 Feb 2016, Florian Fainelli wrote:
> 
>>>> +static void bmips5000_pref30_quirk(void)
>>>> +{
>>>> +	__asm__ __volatile__(
>>>> +	"	.word	0x4008b008\n"	/* mfc0 $8, $22, 8 */
>>>> +	"	lui	$9, 0x0100\n"
>>>> +	"	or	$8, $9\n"
>>>> +	/* disable "pref 30" on buggy CPUs */
>>>> +	"	lui	$9, 0x0800\n"
>>>> +	"	or	$8, $9\n"
>>>> +	"	.word	0x4088b008\n"	/* mtc0 $8, $22, 8 */
>>>> +	: : : "$8", "$9");
>>>> +}
>>>
>>>  Ouch, why not using our standard accessors and avoid magic numbers, e.g.:
>>
>> Are you positive the assembler will not barf on these custom cp0 reg 22
>> selectors?
> 
>  Indeed, I missed that this is beyond the usual select range of 0-7.  
> Sorry about that.
> 
>  That does not mean it shouldn't be done in a cleaner way, stashing the 
> obscurity in one place only.  I did notice a similar piece in one of your 
> other patches, which is a strong argument for abstracting it.
> 
>  So first, are you aware of support for these Broadcom instruction 
> encoding extensions being considered for inclusion in binutils?  I'll be 
> happy to accept a patch and AFAICT it's a simple extension of the `sel' 
> field within the existing MFC0/MTC0 instruction definitions.

That's a bit of a stretch for something that a) nobody is likely to
benefit from except these two patches, b) introducing a 3 way dependency
with: getting these current patches accepted, wait for the new binutils
to be widely spread we can count on the assembler to know about these
additional selectors, update the kernel again...

> 
>  Second, regardless we need to abstract this in a reusable way while we 
> don't have such support in the assembler.  So here:
> 
>>> #define read_c0_brcm_whateverthisiscalled() \
>>> 	__read_32bit_c0_register($22, 8)
>>> #define write_c0_brcm_whateverthisiscalled(val) \
>>> 	__write_32bit_c0_register($22, 8, val)
> 
> rather than using `__read_32bit_c0_register' and 
> `__write_32bit_c0_register' we can define special 
> `__read_32bit_c0_brcm_register' and `__write_32bit_c0_brcm_register' 
> helpers like:
> 
> #define __read_32bit_c0_brcm_register(reg, sel)				\
> ({									\
> 	register unsigned int __res asm("t0");				\
> 									\
> 	__asm__ __volatile__(						\
> 		/* mfc0 t0, $reg, sel */				\
> 		".word	0x40080000 | ((%1 & 0x1f) << 11) | (%2 & 0xf)"	\
> 		: "=r" (__res) : "i" (reg), "i" (sel));			\
> 	__res;								\
> })
> 
> #define __write_32bit_c0_brcm_register(reg, sel, value)			\
> do {									\
> 	register unsigned int __val asm("t0") = value;			\
> 									\
> 	__asm__ __volatile__(						\
> 		/* mtc0 t0, $reg, sel */				\
> 		".word  0x40880000 | ((%1 & 0x1f) << 11) | (%2 & 0xf)"	\
> 		: : "r" (__val), "i" (reg), "i" (sel));			\
> } while (0)
> 
> (if 0xf is indeed the mask for `sel').  This is untested, but should work, 
> perhaps with a minor fix somewhere if I made a typo.  It should also 
> produce a little bit better code, although I realise this is a corner case 
> hardly worth optimising for.  What is important is maintainability.

Maintainability works if these things are going to dramatically change
in the future, as the code stands right now, I grant it to you it is
doing some funky voodoo, but it is documented enough you could
understand what that does.

> 
>>> #define BMIPS5000_WHATEVERTHISISCALLED_BIT_X 0x0100
>>> #define BMIPS5000_WHATEVERTHISISCALLED_BIT_Y 0x0800
>>>
>>> static void bmips5000_pref30_quirk(void)
>>> {
>>> 	unsigned int whateverthisiscalled;
>>>
>>> 	whateverthisiscalled = read_c0_brcm_whateverthisiscalled();
>>> 	whateverthisiscalled |= BMIPS_WHATEVERTHISISCALLED_BIT_X;
>>> 	/* disable "pref 30" on buggy CPUs */
>>> 	whateverthisiscalled |= BMIPS_WHATEVERTHISISCALLED_BIT_Y;
>>> 	write_c0_brcm_whateverthisiscalled(whateverthisiscalled);
>>> }
>>>
>>> ?  I'm leaving it up to you to select the right names here -- just about 
>>> anything will be better than the halfway-binary piece you have proposed.
>>
>> These are not standardized registers in any form, which is why these are
>> in a halfway-binary form, they are not meant to be re-used outside of
>> two known places: disabling pref30 and enabling "rotr".
> 
>  Well, if Broadcom didn't give this register any name, then `reg22_sel8' 
> will be as good as any.  We don't need to invent things here, just to keep 
> them maintainable.  If you call something `8', then you can't easily 
> search through the tree to find references.  If you use something uniquely 
> identifiable, then you can.  So:
> 
> #define read_c0_brcm_reg22_sel8()					\
> 	__read_32bit_c0_brcm_register(22, 8)
> #define write_c0_brcm_reg22_sel8(val)					\
> 	__write_32bit_c0_brcm_register(22, 8, val)
> 
> (note the dropped `$' because we can't pass it along in this form).
> 
>  As to the bit names -- you already gave them: NO_PREF30 (since this is 
> negated) and ROTR will be just fine, with a BMIPS5000_REG22_SEL8_ prefix.  
> So:
> 
> #define BMIPS5000_REG22_SEL8_ROTR	0x0100
> #define BMIPS5000_REG22_SEL8_NO_PREF30	0x0800

I like your suggestion.

> 
> (why is ROTR set along NO_PREF30 BTW? -- it does not seem related).

They are not, Petri is quoting the downstream kernel which does thing
completely differently because it has separate build options per SoC.
The upstream kernel does not, it can run the sam binary on multiple SoCs
and perform run-time detection, hence the two patches which changes
things at two different spots because, unrelated.
> 
>  I hope you agree this all is reasonable from a maintainer's point of 
> view.  I'll leave it up to you to make a patch out of it.  You'll then be 
> able to use this stuff in 2/6 too.

It seems reasonable to adopt your suggestion, but I will certainly drop
the binutils suggestion, that's just way out of the scope of this patch,
and creates a dependency issue that I do not want to deal with, as
history showed recently, testing correctly for a proper ld version
turned out to be interesting, so pardon my skepticism here.

> 
>  Please try to give meaningful names to the other magic numbers you 
> introduce too.

These are actually *magic* register values, I swear to you, I had to dig
in the RTL to understand what this does, there is zero documentation
besides the code.
-- 
Florian

Re: [PATCH 1/6] MIPS: BMIPS: Disable pref 30 for buggy CPUs

[Maciej W. Rozycki]

On Wed, 10 Feb 2016, Florian Fainelli wrote:

> >  So first, are you aware of support for these Broadcom instruction 
> > encoding extensions being considered for inclusion in binutils?  I'll be 
> > happy to accept a patch and AFAICT it's a simple extension of the `sel' 
> > field within the existing MFC0/MTC0 instruction definitions.
> 
> That's a bit of a stretch for something that a) nobody is likely to
> benefit from except these two patches, b) introducing a 3 way dependency
> with: getting these current patches accepted, wait for the new binutils
> to be widely spread we can count on the assembler to know about these
> additional selectors, update the kernel again...

 No need to wait, we have all the build tools at hand, that is we could 
conditionalise it on $(as-instr), formulated appropriately.  See 
Documentation/kbuild/makefiles.txt and examples in our tree.

 Overall I think the toolchain should be complete, even if for an exotic 
case here or there.  If a processor has an instruction, then the assembler 
should support it.  As I say this addition looks very simple to me and I 
could (and, if nobody beats me to, probably will, now that I'm aware) make 
such a change to binutils myself, but I'd prefer to get a change from 
someone actually having a documentation reference for the instruction set 
extension.

 An immediate benefit will be that you'll get a proper disassembly in 
`objdump' so the unaware poor soul debugging an odd issue won't have to 
scratch their head wondering what the heck 0x4008b008 in disassembly is, 
and if this is toolchain bug or some corruption (though it would likely 
still require telling the disassembler to use BMIPS5000 as the processor 
to disassemble for).

> > #define __read_32bit_c0_brcm_register(reg, sel)				\
> > ({									\
> > 	register unsigned int __res asm("t0");				\

 Actually on second thoughts explicit asm("$8") will be better here.  For 
GCC both variants are equivalent, unlike GAS the compiler does not have a 
concept of ABI register names, so no `a4' in place of `t0', etc., but 0x08 
is embedded in the instruction word used, so better have it consistent.

> > (why is ROTR set along NO_PREF30 BTW? -- it does not seem related).
> 
> They are not, Petri is quoting the downstream kernel which does thing
> completely differently because it has separate build options per SoC.
> The upstream kernel does not, it can run the sam binary on multiple SoCs
> and perform run-time detection, hence the two patches which changes
> things at two different spots because, unrelated.

 Hmm, maybe call the quirk `bmips5000_pref30_rotr_quirk' or suchlike then?

> >  I hope you agree this all is reasonable from a maintainer's point of 
> > view.  I'll leave it up to you to make a patch out of it.  You'll then be 
> > able to use this stuff in 2/6 too.
> 
> It seems reasonable to adopt your suggestion, but I will certainly drop
> the binutils suggestion, that's just way out of the scope of this patch,
> and creates a dependency issue that I do not want to deal with, as
> history showed recently, testing correctly for a proper ld version
> turned out to be interesting, so pardon my skepticism here.

 Fine with me, I just wanted to have a full picture and also hoped a 
little a patch might have already been in the works.

 The LD version case?  Well, the lack of experience with the toolchain has 
certainly contributed here (not a blame, not especially for kernel hackers 
who need not necessarily go down to such details, just an observation -- 
though I think a question how to correctly interpret binutils versioning 
posted to <binutils@sourceware.org> would have certainly been addressed 
with a correct answer).  We've had an LD version check sorted out since 
forever in glibc's build machinery for example, by using prefix matching, 
essentially. :)

> >  Please try to give meaningful names to the other magic numbers you 
> > introduce too.
> 
> These are actually *magic* register values, I swear to you, I had to dig
> in the RTL to understand what this does, there is zero documentation
> besides the code.

 OK, fair enough.  Thanks for your patience in explaining the details!

  Maciej

Re: [PATCH 1/6] MIPS: BMIPS: Disable pref 30 for buggy CPUs

[Maciej W. Rozycki]

On Wed, 10 Feb 2016, Florian Fainelli wrote:

> >  So first, are you aware of support for these Broadcom instruction 
> > encoding extensions being considered for inclusion in binutils?  I'll be 
> > happy to accept a patch and AFAICT it's a simple extension of the `sel' 
> > field within the existing MFC0/MTC0 instruction definitions.
> 
> That's a bit of a stretch for something that a) nobody is likely to
> benefit from except these two patches, b) introducing a 3 way dependency
> with: getting these current patches accepted, wait for the new binutils
> to be widely spread we can count on the assembler to know about these
> additional selectors, update the kernel again...

 No need to wait, we have all the build tools at hand, that is we could 
conditionalise it on $(as-instr), formulated appropriately.  See 
Documentation/kbuild/makefiles.txt and examples in our tree.

 Overall I think the toolchain should be complete, even if for an exotic 
case here or there.  If a processor has an instruction, then the assembler 
should support it.  As I say this addition looks very simple to me and I 
could (and, if nobody beats me to, probably will, now that I'm aware) make 
such a change to binutils myself, but I'd prefer to get a change from 
someone actually having a documentation reference for the instruction set 
extension.

 An immediate benefit will be that you'll get a proper disassembly in 
`objdump' so the unaware poor soul debugging an odd issue won't have to 
scratch their head wondering what the heck 0x4008b008 in disassembly is, 
and if this is toolchain bug or some corruption (though it would likely 
still require telling the disassembler to use BMIPS5000 as the processor 
to disassemble for).

> > #define __read_32bit_c0_brcm_register(reg, sel)				\
> > ({									\
> > 	register unsigned int __res asm("t0");				\

 Actually on second thoughts explicit asm("$8") will be better here.  For 
GCC both variants are equivalent, unlike GAS the compiler does not have a 
concept of ABI register names, so no `a4' in place of `t0', etc., but 0x08 
is embedded in the instruction word used, so better have it consistent.

> > (why is ROTR set along NO_PREF30 BTW? -- it does not seem related).
> 
> They are not, Petri is quoting the downstream kernel which does thing
> completely differently because it has separate build options per SoC.
> The upstream kernel does not, it can run the sam binary on multiple SoCs
> and perform run-time detection, hence the two patches which changes
> things at two different spots because, unrelated.

 Hmm, maybe call the quirk `bmips5000_pref30_rotr_quirk' or suchlike then?

> >  I hope you agree this all is reasonable from a maintainer's point of 
> > view.  I'll leave it up to you to make a patch out of it.  You'll then be 
> > able to use this stuff in 2/6 too.
> 
> It seems reasonable to adopt your suggestion, but I will certainly drop
> the binutils suggestion, that's just way out of the scope of this patch,
> and creates a dependency issue that I do not want to deal with, as
> history showed recently, testing correctly for a proper ld version
> turned out to be interesting, so pardon my skepticism here.

 Fine with me, I just wanted to have a full picture and also hoped a 
little a patch might have already been in the works.

 The LD version case?  Well, the lack of experience with the toolchain has 
certainly contributed here (not a blame, not especially for kernel hackers 
who need not necessarily go down to such details, just an observation -- 
though I think a question how to correctly interpret binutils versioning 
posted to <binutils@sourceware.org> would have certainly been addressed 
with a correct answer).  We've had an LD version check sorted out since 
forever in glibc's build machinery for example, by using prefix matching, 
essentially. :)

> >  Please try to give meaningful names to the other magic numbers you 
> > introduce too.
> 
> These are actually *magic* register values, I swear to you, I had to dig
> in the RTL to understand what this does, there is zero documentation
> besides the code.

 OK, fair enough.  Thanks for your patience in explaining the details!

  Maciej

Re: [PATCH 1/6] MIPS: BMIPS: Disable pref 30 for buggy CPUs

[Ralf Baechle]

On Tue, Feb 09, 2016 at 12:55:49PM -0800, Florian Fainelli wrote:

> +static void bmips5000_pref30_quirk(void)
> +{
> +	__asm__ __volatile__(
> +	"	.word	0x4008b008\n"	/* mfc0 $8, $22, 8 */
> +	"	lui	$9, 0x0100\n"
> +	"	or	$8, $9\n"
> +	/* disable "pref 30" on buggy CPUs */
> +	"	lui	$9, 0x0800\n"
> +	"	or	$8, $9\n"
> +	"	.word	0x4088b008\n"	/* mtc0 $8, $22, 8 */
> +	: : : "$8", "$9");
> +}

Simpler:

#define read_c0_horse_with_no_name(val)  __read_32bit_c0_register($22, 8, val)
#define write_c0_horse_with_no_name(val) __write_32bit_c0_register($22, 8)

...

write_c0_horse_with_no_name(read_c0_horse_with_no_name() | 0x123);

And why do both MFC0 and MTC0 instructions above have the same opcode?
Also the selector number used above for both instructions is 8 - but the
architecture only allows for 8 selectors 0..7.

  Ralf

Re: [PATCH 1/6] MIPS: BMIPS: Disable pref 30 for buggy CPUs

[Ralf Baechle]

On Wed, Feb 10, 2016 at 10:20:33AM +0100, Ralf Baechle wrote:

> And why do both MFC0 and MTC0 instructions above have the same opcode?

Forget this one, I should occasionally open my eyes in the morning ;)

  Ralf

Re: [PATCH 6/6] MIPS: Expose current_cpu_data.options through debugfs

[Ralf Baechle]

On Tue, Feb 09, 2016 at 12:55:54PM -0800, Florian Fainelli wrote:

> Debugging a missing features in cpu-features-override.h, or a runtime feature
> set/clear in the vendor specific cpu_probe() function can be a little tedious,
> ease that by providing a debugfs entry representing the
> current_cpu_data.options bitmask.

Hm..  Bits in the options bitmaps are not an ABI, they come and sometimes
they go as well and manual decoding can be tedious to humans.  so I'm
wondering if something in /sys/devices/system/cpu would be more suitable.
It'd depend on just sysfs, not debugfs which is disabled in production
kernels.

Thoughts?

  Ralf

Re: [PATCH 1/6] MIPS: BMIPS: Disable pref 30 for buggy CPUs

[Maciej W. Rozycki]

On Wed, 10 Feb 2016, Ralf Baechle wrote:

> > And why do both MFC0 and MTC0 instructions above have the same opcode?
> 
> Forget this one, I should occasionally open my eyes in the morning ;)

 Yeah, I think we've beaten it with Florian all to death last night 
already.

  Maciej

Re: [PATCH 1/6] MIPS: BMIPS: Disable pref 30 for buggy CPUs

[Maciej W. Rozycki]

On Wed, 10 Feb 2016, Ralf Baechle wrote:

> > And why do both MFC0 and MTC0 instructions above have the same opcode?
> 
> Forget this one, I should occasionally open my eyes in the morning ;)

 Yeah, I think we've beaten it with Florian all to death last night 
already.

  Maciej

Re: [PATCH 6/6] MIPS: Expose current_cpu_data.options through debugfs

[Florian Fainelli]

On 10/02/16 02:46, Ralf Baechle wrote:
> On Tue, Feb 09, 2016 at 12:55:54PM -0800, Florian Fainelli wrote:
> 
>> Debugging a missing features in cpu-features-override.h, or a runtime feature
>> set/clear in the vendor specific cpu_probe() function can be a little tedious,
>> ease that by providing a debugfs entry representing the
>> current_cpu_data.options bitmask.
> 
> Hm..  Bits in the options bitmaps are not an ABI, they come and sometimes
> they go as well and manual decoding can be tedious to humans.  so I'm
> wondering if something in /sys/devices/system/cpu would be more suitable.

Not sure, you need this while bringing up systems and/or debugging why a
kernel does not have this or that enabled as you think it should,
outside of that, not so much probably?. More standard interfaces like
/proc/cpuinfo should be used, but that does not give you the full picture.

> It'd depend on just sysfs, not debugfs which is disabled in production
> kernels.
> 
> Thoughts?

I suppose this is fine, in that case, we would probably want to go with
a text-based approach to make the interface more stable.

NB: I also have a patch that adds cache info reporting to MIPS, since it
seems useful for people dealing with user-space cache flushes (graphics,
JIT, etc.), Russell King rejected adding that for ARM, but ARM64 and x86
have it (arch/arm64/kernel/cacheinfo.c) would you oppose to having that
for MIPS?
-- 
Florian

Re: [PATCH 0/6] MIPS: BMIPS: RIXI and workarounds support

[Florian Fainelli]

Le 09/02/2016 12:55, Florian Fainelli a écrit :
> Hi all,
> 
> This patch series contains some workarounds for some bug with pref30 on
> Broadcom BMIPS5000 CPUs in 7344, 7346 and 7425 chips, and some other changes
> to allow the use of RIXI/rotr on BMIPS4380 and BMIPS5000.
> 
> Finally, the last patch adds a debugfs entry for current_cpu_data.options since
> it might be useful to debug that at a time where we set on the final CPU
> options.
> 
> This is on top of mips-for-linux-next as of
> a13d2abd8e617a96d235c0a528a742b347650853 ("MIPS: highmem: Turn
> flush_cache_kmaps into a no-op.")

Ralf, patches 2-5 have been marked accepted in patchwork, but I am not
able to find them in mips-for-linux-next, have they been merged some
place else? Thanks

> 
> Thanks!
> 
> Florian Fainelli (6):
>   MIPS: BMIPS: Disable pref 30 for buggy CPUs
>   MIPS: BMIPS: Add early CPU initialization code
>   MIPS: Allow RIXI to be used on non-R2 or R6 cores
>   MIPS: Move RIXI exception enabling after vendor-specific cpu_probe
>   MIPS: BMIPS: BMIPS4380 and BMIPS5000 support RIXI
>   MIPS: Expose current_cpu_data.options through debugfs
> 
>  arch/mips/Kconfig                    |  7 +++
>  arch/mips/bmips/setup.c              | 17 +++++++
>  arch/mips/include/asm/bmips.h        |  1 +
>  arch/mips/include/asm/pgtable-bits.h | 11 ++---
>  arch/mips/kernel/cpu-probe.c         | 41 ++++++++++++-----
>  arch/mips/kernel/smp-bmips.c         | 87 ++++++++++++++++++++++++++++++++++++
>  arch/mips/mm/tlbex.c                 |  2 +-
>  7 files changed, 150 insertions(+), 16 deletions(-)
> 


-- 
Florian