| Message ID | 20200731042401.22871-1-f.fainelli@gmail.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | MIPS: BMIPS: Disable pref 30 for buggy CPUs | expand |
On Thu, Jul 30, 2020 at 09:24:01PM -0700, Florian Fainelli wrote: > Disable pref 30 by utilizing the standard quirk method and matching the > affected SoCs: 7344, 7346, 7425. > > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> > --- > arch/mips/bmips/setup.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/arch/mips/bmips/setup.c b/arch/mips/bmips/setup.c > index 19308df5f577..df0efea12611 100644 > --- a/arch/mips/bmips/setup.c > +++ b/arch/mips/bmips/setup.c > @@ -110,6 +110,20 @@ static void bcm6368_quirks(void) > bcm63xx_fixup_cpu1(); > } > > +static void bmips5000_pref30_quirk(void) > +{ > + __asm__ __volatile__( > + " li $8, 0x5a455048\n" > + " .word 0x4088b00f\n" /* mtc0 $8, $22, 15 */ > + " nop; nop; nop\n" > + " .word 0x4008b008\n" /* mfc0 $8, $22, 8 */ > + /* disable "pref 30" on buggy CPUs */ > + " lui $9, 0x0800\n" > + " or $8, $9\n" > + " .word 0x4088b008\n" /* mtc0 $8, $22, 8 */ > + : : : "$8", "$9"); what's the reason for not using mfc/mtc here ? Thomas.
在 2020/7/31 下午12:24, Florian Fainelli 写道: > Disable pref 30 by utilizing the standard quirk method and matching the > affected SoCs: 7344, 7346, 7425. > > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> > --- > arch/mips/bmips/setup.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/arch/mips/bmips/setup.c b/arch/mips/bmips/setup.c > index 19308df5f577..df0efea12611 100644 > --- a/arch/mips/bmips/setup.c > +++ b/arch/mips/bmips/setup.c > @@ -110,6 +110,20 @@ static void bcm6368_quirks(void) > bcm63xx_fixup_cpu1(); > } > > +static void bmips5000_pref30_quirk(void) > +{ > + __asm__ __volatile__( > + " li $8, 0x5a455048\n" > + " .word 0x4088b00f\n" /* mtc0 $8, $22, 15 */ > + " nop; nop; nop\n" > + " .word 0x4008b008\n" /* mfc0 $8, $22, 8 */ > + /* disable "pref 30" on buggy CPUs */ > + " lui $9, 0x0800\n" > + " or $8, $9\n" > + " .word 0x4088b008\n" /* mtc0 $8, $22, 8 */ > + : : : "$8", "$9"); > +} Hi, Is there any toolchain issue blocking read_c0_**** family helpers being used? Use .word looks unreasonable. Thanks - Jiaxun > + > static const struct bmips_quirk bmips_quirk_list[] = { > { "brcm,bcm3368", &bcm6358_quirks }, > { "brcm,bcm3384-viper", &bcm3384_viper_quirks }, > @@ -120,6 +134,9 @@ static const struct bmips_quirk bmips_quirk_list[] = { > { "brcm,bcm6368", &bcm6368_quirks }, > { "brcm,bcm63168", &bcm6368_quirks }, > { "brcm,bcm63268", &bcm6368_quirks }, > + { "brcm,bcm7344", &bmips5000_pref30_quirk }, > + { "brcm,bcm7346", &bmips5000_pref30_quirk }, > + { "brcm,bcm7425", &bmips5000_pref30_quirk }, > { }, > }; >
On 7/31/20 3:34 AM, Jiaxun Yang wrote: > > > 在 2020/7/31 下午12:24, Florian Fainelli 写道: >> Disable pref 30 by utilizing the standard quirk method and matching the >> affected SoCs: 7344, 7346, 7425. >> >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> >> --- >> arch/mips/bmips/setup.c | 17 +++++++++++++++++ >> 1 file changed, 17 insertions(+) >> >> diff --git a/arch/mips/bmips/setup.c b/arch/mips/bmips/setup.c >> index 19308df5f577..df0efea12611 100644 >> --- a/arch/mips/bmips/setup.c >> +++ b/arch/mips/bmips/setup.c >> @@ -110,6 +110,20 @@ static void bcm6368_quirks(void) >> bcm63xx_fixup_cpu1(); >> } >> +static void bmips5000_pref30_quirk(void) >> +{ >> + __asm__ __volatile__( >> + " li $8, 0x5a455048\n" >> + " .word 0x4088b00f\n" /* mtc0 $8, $22, 15 */ >> + " nop; nop; nop\n" >> + " .word 0x4008b008\n" /* mfc0 $8, $22, 8 */ >> + /* disable "pref 30" on buggy CPUs */ >> + " lui $9, 0x0800\n" >> + " or $8, $9\n" >> + " .word 0x4088b008\n" /* mtc0 $8, $22, 8 */ >> + : : : "$8", "$9"); >> +} > Hi, > > Is there any toolchain issue blocking read_c0_**** family helpers being > used? > > Use .word looks unreasonable. Yes, the assembler would be choking on the custom $22 selector, however this patch should not be necessary given that the boot loader (CFE) should have long been updated by now to disable pref 30. Thanks
On 7/31/20 2:05 AM, Thomas Bogendoerfer wrote: > On Thu, Jul 30, 2020 at 09:24:01PM -0700, Florian Fainelli wrote: >> Disable pref 30 by utilizing the standard quirk method and matching the >> affected SoCs: 7344, 7346, 7425. >> >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> >> --- >> arch/mips/bmips/setup.c | 17 +++++++++++++++++ >> 1 file changed, 17 insertions(+) >> >> diff --git a/arch/mips/bmips/setup.c b/arch/mips/bmips/setup.c >> index 19308df5f577..df0efea12611 100644 >> --- a/arch/mips/bmips/setup.c >> +++ b/arch/mips/bmips/setup.c >> @@ -110,6 +110,20 @@ static void bcm6368_quirks(void) >> bcm63xx_fixup_cpu1(); >> } >> >> +static void bmips5000_pref30_quirk(void) >> +{ >> + __asm__ __volatile__( >> + " li $8, 0x5a455048\n" >> + " .word 0x4088b00f\n" /* mtc0 $8, $22, 15 */ >> + " nop; nop; nop\n" >> + " .word 0x4008b008\n" /* mfc0 $8, $22, 8 */ >> + /* disable "pref 30" on buggy CPUs */ >> + " lui $9, 0x0800\n" >> + " or $8, $9\n" >> + " .word 0x4088b008\n" /* mtc0 $8, $22, 8 */ >> + : : : "$8", "$9"); > > what's the reason for not using mfc/mtc here ? See my response to Jiaxun, thanks!
On Fri, Jul 31, 2020 at 03:49:28PM -0700, Florian Fainelli wrote: > On 7/31/20 3:34 AM, Jiaxun Yang wrote: > > > > > > 在 2020/7/31 下午12:24, Florian Fainelli 写道: > >> Disable pref 30 by utilizing the standard quirk method and matching the > >> affected SoCs: 7344, 7346, 7425. > >> > >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> > >> --- > >> arch/mips/bmips/setup.c | 17 +++++++++++++++++ > >> 1 file changed, 17 insertions(+) > >> > >> diff --git a/arch/mips/bmips/setup.c b/arch/mips/bmips/setup.c > >> index 19308df5f577..df0efea12611 100644 > >> --- a/arch/mips/bmips/setup.c > >> +++ b/arch/mips/bmips/setup.c > >> @@ -110,6 +110,20 @@ static void bcm6368_quirks(void) > >> bcm63xx_fixup_cpu1(); > >> } > >> +static void bmips5000_pref30_quirk(void) > >> +{ > >> + __asm__ __volatile__( > >> + " li $8, 0x5a455048\n" > >> + " .word 0x4088b00f\n" /* mtc0 $8, $22, 15 */ > >> + " nop; nop; nop\n" > >> + " .word 0x4008b008\n" /* mfc0 $8, $22, 8 */ > >> + /* disable "pref 30" on buggy CPUs */ > >> + " lui $9, 0x0800\n" > >> + " or $8, $9\n" > >> + " .word 0x4088b008\n" /* mtc0 $8, $22, 8 */ > >> + : : : "$8", "$9"); > >> +} > > Hi, > > > > Is there any toolchain issue blocking read_c0_**** family helpers being > > used? > > > > Use .word looks unreasonable. > > Yes, the assembler would be choking on the custom $22 selector, however I guess you meant selector 8 and 15. If BMIPS has a 4 bit selector field it might be good to do a binutils patch supporting it. > this patch should not be necessary given that the boot loader (CFE) > should have long been updated by now to disable pref 30. so, should I add it or drop it ? Thomas.
On 8/3/2020 4:30 AM, Thomas Bogendoerfer wrote: > On Fri, Jul 31, 2020 at 03:49:28PM -0700, Florian Fainelli wrote: >> On 7/31/20 3:34 AM, Jiaxun Yang wrote: >>> >>> >>> 在 2020/7/31 下午12:24, Florian Fainelli 写道: >>>> Disable pref 30 by utilizing the standard quirk method and matching the >>>> affected SoCs: 7344, 7346, 7425. >>>> >>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> >>>> --- >>>> arch/mips/bmips/setup.c | 17 +++++++++++++++++ >>>> 1 file changed, 17 insertions(+) >>>> >>>> diff --git a/arch/mips/bmips/setup.c b/arch/mips/bmips/setup.c >>>> index 19308df5f577..df0efea12611 100644 >>>> --- a/arch/mips/bmips/setup.c >>>> +++ b/arch/mips/bmips/setup.c >>>> @@ -110,6 +110,20 @@ static void bcm6368_quirks(void) >>>> bcm63xx_fixup_cpu1(); >>>> } >>>> +static void bmips5000_pref30_quirk(void) >>>> +{ >>>> + __asm__ __volatile__( >>>> + " li $8, 0x5a455048\n" >>>> + " .word 0x4088b00f\n" /* mtc0 $8, $22, 15 */ >>>> + " nop; nop; nop\n" >>>> + " .word 0x4008b008\n" /* mfc0 $8, $22, 8 */ >>>> + /* disable "pref 30" on buggy CPUs */ >>>> + " lui $9, 0x0800\n" >>>> + " or $8, $9\n" >>>> + " .word 0x4088b008\n" /* mtc0 $8, $22, 8 */ >>>> + : : : "$8", "$9"); >>>> +} >>> Hi, >>> >>> Is there any toolchain issue blocking read_c0_**** family helpers being >>> used? >>> >>> Use .word looks unreasonable. >> >> Yes, the assembler would be choking on the custom $22 selector, however > > I guess you meant selector 8 and 15. If BMIPS has a 4 bit selector field > it might be good to do a binutils patch supporting it. Yes, sorry that is what I meant. I don't think an assembler patch makes sense at this point given this is an isolated use, and there is not just binutils these days, the Clang/LLVM integrated assembler would also need to be supported, and then we would need to have the kernel say: I require this minimum version to support the customer selectors, not worth the trouble if you ask me. > >> this patch should not be necessary given that the boot loader (CFE) >> should have long been updated by now to disable pref 30. > > so, should I add it or drop it ? You can drop it and I would resubmit it with feedback addressed if this later comes back.
On Mon, 3 Aug 2020, Florian Fainelli wrote: > >>>> diff --git a/arch/mips/bmips/setup.c b/arch/mips/bmips/setup.c > >>>> index 19308df5f577..df0efea12611 100644 > >>>> --- a/arch/mips/bmips/setup.c > >>>> +++ b/arch/mips/bmips/setup.c > >>>> @@ -110,6 +110,20 @@ static void bcm6368_quirks(void) > >>>> bcm63xx_fixup_cpu1(); > >>>> } > >>>> +static void bmips5000_pref30_quirk(void) > >>>> +{ > >>>> + __asm__ __volatile__( > >>>> + " li $8, 0x5a455048\n" > >>>> + " .word 0x4088b00f\n" /* mtc0 $8, $22, 15 */ > >>>> + " nop; nop; nop\n" > >>>> + " .word 0x4008b008\n" /* mfc0 $8, $22, 8 */ > >>>> + /* disable "pref 30" on buggy CPUs */ > >>>> + " lui $9, 0x0800\n" > >>>> + " or $8, $9\n" > >>>> + " .word 0x4088b008\n" /* mtc0 $8, $22, 8 */ > >>>> + : : : "$8", "$9"); > >>>> +} > >>> Hi, > >>> > >>> Is there any toolchain issue blocking read_c0_**** family helpers being > >>> used? > >>> > >>> Use .word looks unreasonable. > >> > >> Yes, the assembler would be choking on the custom $22 selector, however > > > > I guess you meant selector 8 and 15. If BMIPS has a 4 bit selector field > > it might be good to do a binutils patch supporting it. > > Yes, sorry that is what I meant. I don't think an assembler patch makes > sense at this point given this is an isolated use, and there is not just > binutils these days, the Clang/LLVM integrated assembler would also need > to be supported, and then we would need to have the kernel say: I > require this minimum version to support the customer selectors, not > worth the trouble if you ask me. Well, I asked for a GAS patch to add support 4.5 years ago, so by now and 9 binutils releases later it would have become fairly common. And then I also suggested how to handle it in a robust way. Cf. <https://www.linux-mips.org/cgi-bin/mesg.cgi?a=linux-mips&i=alpine.DEB.2.00.1602092245180.15885%40tp.orcam.me.uk>. Maciej
diff --git a/arch/mips/bmips/setup.c b/arch/mips/bmips/setup.c index 19308df5f577..df0efea12611 100644 --- a/arch/mips/bmips/setup.c +++ b/arch/mips/bmips/setup.c @@ -110,6 +110,20 @@ static void bcm6368_quirks(void) bcm63xx_fixup_cpu1(); } +static void bmips5000_pref30_quirk(void) +{ + __asm__ __volatile__( + " li $8, 0x5a455048\n" + " .word 0x4088b00f\n" /* mtc0 $8, $22, 15 */ + " nop; nop; nop\n" + " .word 0x4008b008\n" /* mfc0 $8, $22, 8 */ + /* 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,bcm3368", &bcm6358_quirks }, { "brcm,bcm3384-viper", &bcm3384_viper_quirks }, @@ -120,6 +134,9 @@ static const struct bmips_quirk bmips_quirk_list[] = { { "brcm,bcm6368", &bcm6368_quirks }, { "brcm,bcm63168", &bcm6368_quirks }, { "brcm,bcm63268", &bcm6368_quirks }, + { "brcm,bcm7344", &bmips5000_pref30_quirk }, + { "brcm,bcm7346", &bmips5000_pref30_quirk }, + { "brcm,bcm7425", &bmips5000_pref30_quirk }, { }, };
Disable pref 30 by utilizing the standard quirk method and matching the affected SoCs: 7344, 7346, 7425. Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> --- arch/mips/bmips/setup.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)