diff mbox series

[1/2] MIPS: Loongson, add sync before target of branch between llsc

Message ID 20190105150037.30261-1-syq@debian.org (mailing list archive)
State Superseded
Headers show
Series [1/2] MIPS: Loongson, add sync before target of branch between llsc | expand

Commit Message

YunQiang Su Jan. 5, 2019, 3 p.m. UTC 
From: YunQiang Su <ysu@wavecomp.com>

Loongson 2G/2H/3A/3B is quite weak sync'ed. If there is a branch,
and the target is not in the scope of ll/sc or lld/scd, a sync is
needed at the postion of target.

Loongson doesn't plan to fix this problem in future, so we add the
sync here for any condition.

This is based on the patch from Chen Huacai.

Signed-off-by: YunQiang Su <ysu@wavecomp.com>
---
 arch/mips/mm/tlbex.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Paul Burton Jan. 9, 2019, 10:08 p.m. UTC | #1 
Hi YunQiang,

On Sat, Jan 05, 2019 at 11:00:36PM +0800, YunQiang Su wrote:
> Loongson 2G/2H/3A/3B is quite weak sync'ed. If there is a branch,
> and the target is not in the scope of ll/sc or lld/scd, a sync is
> needed at the postion of target.

OK, so is this the same issue that the second patch in the series is
working around or a different one?

I'm pretty confused at this point about what the actual bugs are in
these various Loongson CPUs. Could someone provide an actual errata
writeup describing the bugs in detail?

What does "in the scope of ll/sc" mean?

What happens if a branch target is not "in the scope of ll/sc"?

How does the sync help?

Are jumps affected, or just branches?

Does this affect userland as well as the kernel?

...and probably more questions depending upon the answers to these ones.

> Loongson doesn't plan to fix this problem in future, so we add the
> sync here for any condition.

So are you saying that future Loongson CPUs will all be buggy too, and
someone there has said that they consider this to be OK..? I really
really hope that is not true.

If hardware people say they're not going to fix their bugs then working
around them is definitely not going to be a priority. It's one thing if
a CPU designer says "oops, my bad, work around this & I'll fix it next
time". It's quite another for them to say they're not interested in
fixing their bugs at all.

Thanks,
    Paul
Yunqiang Su Jan. 10, 2019, 1:59 a.m. UTC | #2 
> 在 2019年1月10日，上午6:08，Paul Burton <pburton@wavecomp.com> 写道：
> 
> Hi YunQiang,
> 
> On Sat, Jan 05, 2019 at 11:00:36PM +0800, YunQiang Su wrote:
>> Loongson 2G/2H/3A/3B is quite weak sync'ed. If there is a branch,
>> and the target is not in the scope of ll/sc or lld/scd, a sync is
>> needed at the postion of target.
> 
> OK, so is this the same issue that the second patch in the series is
> working around or a different one?
> 
> I'm pretty confused at this point about what the actual bugs are in
> these various Loongson CPUs. Could someone provide an actual errata
> writeup describing the bugs in detail?
> 
> What does "in the scope of ll/sc" mean?
> 

Loongson 3 series has some version, called, 1000, 2000, and 3000.

There are 2 bugs all about LL/SC. Let’s call them bug-1 and bug-2.

BUG-1:  a `sync’ is needed before LL or LLD instruction.
              This bug appears on 1000 only, and I am sure that it has been fixed in 3000.

BUG-2: if there is an branch instruction inside LL/SC, and the branch target is outside
             of the scope of LL/SC, a `sync’ is needed at the branch target.
             Aka, the first insn of the target branch should be `sync’.
             Loongson said that, we don’t plan fix this problem in short time before they
             Designe a totally new core.
              

> What happens if a branch target is not "in the scope of ll/sc”?

At least they said that there won’t be a problem

> How does the sync help?
> 
> Are jumps affected, or just branches?

I am not sure, so CC a Loongson people.
@Paul Hua

> 
> Does this affect userland as well as the kernel?
> 

There is few place can trigger these 2 bugs in kernel.
In user land we have to workaround in binutils:  
   https://www.sourceware.org/ml/binutils/2019-01/msg00025.html

In fact the kernel is the easiest since we can have a flavor build for Loongson.

> ...and probably more questions depending upon the answers to these ones.
> 
>> Loongson doesn't plan to fix this problem in future, so we add the
>> sync here for any condition.
> 
> So are you saying that future Loongson CPUs will all be buggy too, and
> someone there has said that they consider this to be OK..? I really
> really hope that is not true.
> 

Bug is bug. It is not OK.
I blame these Loongson guys here.
Some Loongson guys is not so normal people.
Anyway they are a little more normal now, and anyway again, still abnormal.

> If hardware people say they're not going to fix their bugs then working
> around them is definitely not going to be a priority. It's one thing if
> a CPU designer says "oops, my bad, work around this & I'll fix it next
> time". It's quite another for them to say they're not interested in
> fixing their bugs at all.

They have interests, while I guess the true reason is that they have no enough
people and money to desgin a core, while this bug is quilt hard to fix.

> 
> Thanks,
>    Paul
Paul Burton Jan. 10, 2019, 5:35 p.m. UTC | #3 
Hi Yunqiang,

On Wed, Jan 09, 2019 at 05:59:07PM -0800, Yunqiang Su wrote:
> > 在 2019年1月10日，上午6:08，Paul Burton <pburton@wavecomp.com> 写道：
> > On Sat, Jan 05, 2019 at 11:00:36PM +0800, YunQiang Su wrote:
> >> Loongson 2G/2H/3A/3B is quite weak sync'ed. If there is a branch,
> >> and the target is not in the scope of ll/sc or lld/scd, a sync is
> >> needed at the postion of target.
> > 
> > OK, so is this the same issue that the second patch in the series is
> > working around or a different one?
> > 
> > I'm pretty confused at this point about what the actual bugs are in
> > these various Loongson CPUs. Could someone provide an actual errata
> > writeup describing the bugs in detail?
> > 
> > What does "in the scope of ll/sc" mean?
> 
> Loongson 3 series has some version, called, 1000, 2000, and 3000.
> 
> There are 2 bugs all about LL/SC. Let’s call them bug-1 and bug-2.
> 
> BUG-1:  a `sync’ is needed before LL or LLD instruction.
>               This bug appears on 1000 only, and I am sure that it has been fixed in 3000.
> 
> BUG-2: if there is an branch instruction inside LL/SC, and the branch target is outside
>              of the scope of LL/SC, a `sync’ is needed at the branch target.
>              Aka, the first insn of the target branch should be `sync’.
>              Loongson said that, we don’t plan fix this problem in short time before they
>              Designe a totally new core.
>               
> 
> > What happens if a branch target is not "in the scope of ll/sc”?
> 
> At least they said that there won’t be a problem

You still didn't define what "in the scope of ll/sc" means - I'm
guessing that you're referring to a branch target as "in scope" if it is
in between the ll & sc instructions (inclusive?). But this is just a
guess & clarity from people who actually know would be helpful.

And there must be a problem. The whole point of this is that there's a
bug, right? If there's no problem then we don't need to do anything :)

From a look at the GCC patch it talks about placing a sync at a branch
target if it *is* in between an ll & sc [1], which I just can't
reconcile with the phrase "outside of the scope of LL/SC". Is the
problem when a branch target *is* in between an ll & sc, or when it *is
not* between an ll & sc?

Reading this kernel patch doesn't make it any clearer - for example the
sync it emits in build_loongson3_tlb_refill_handler() is nowhere near an
ll or sc instruction. Something doesn't add up here.

> > How does the sync help?
> > 
> > Are jumps affected, or just branches?
> 
> I am not sure, so CC a Loongson people.
> @Paul Hua

Hi Paul - any help obtaining a detailed description of these bugs would
be much appreciated. Even if you only have something in Chinese I can
probably get someone to help translate.

> > Does this affect userland as well as the kernel?
> 
> There is few place can trigger these 2 bugs in kernel.
> In user land we have to workaround in binutils:  
>    https://www.sourceware.org/ml/binutils/2019-01/msg00025.html
> 
> In fact the kernel is the easiest since we can have a flavor build for Loongson.

My concern with regards to userland is that there's talk of a "deadlock"
- if userland can hit this & the CPU actually stalls then the system is
hopelessly vulnerable to denial of service from a malicious or buggy
userland program, or simply an innocent program unaware of the errata.

> > ...and probably more questions depending upon the answers to these ones.
> > 
> >> Loongson doesn't plan to fix this problem in future, so we add the
> >> sync here for any condition.
> > 
> > So are you saying that future Loongson CPUs will all be buggy too, and
> > someone there has said that they consider this to be OK..? I really
> > really hope that is not true.
> 
> Bug is bug. It is not OK.
> I blame these Loongson guys here.
> Some Loongson guys is not so normal people.
> Anyway they are a little more normal now, and anyway again, still abnormal.
> 
> > If hardware people say they're not going to fix their bugs then working
> > around them is definitely not going to be a priority. It's one thing if
> > a CPU designer says "oops, my bad, work around this & I'll fix it next
> > time". It's quite another for them to say they're not interested in
> > fixing their bugs at all.
> 
> They have interests, while I guess the true reason is that they have no enough
> people and money to desgin a core, while this bug is quilt hard to fix.

I'm not sure I fully understand what you're saying above, but
essentially I want to know that Loongson care about fixing their CPU
bugs. If they don't, and the bugs are as bad as they sound, then in my
view working around them will only reinforce that producing CPUs with
such serious bugs is a good idea.

So if anyone from Loongson is reading, I'd really like to hear that the
above is a miscommunication & that you're not intending to knowingly
design any further CPUs with these bugs.

Thanks,
    Paul

[1] https://gcc.gnu.org/ml/gcc-patches/2018-12/msg01064.html
    ("Loongson3 need a sync before branch target that between ll and sc.")
YunQiang Su Jan. 10, 2019, 6:42 p.m. UTC | #4 
Paul Burton <paul.burton@mips.com> 于2019年1月11日周五 上午1:35写道：
>
> Hi Yunqiang,
>
> On Wed, Jan 09, 2019 at 05:59:07PM -0800, Yunqiang Su wrote:
> > > 在 2019年1月10日，上午6:08，Paul Burton <pburton@wavecomp.com> 写道：
> > > On Sat, Jan 05, 2019 at 11:00:36PM +0800, YunQiang Su wrote:
> > >> Loongson 2G/2H/3A/3B is quite weak sync'ed. If there is a branch,
> > >> and the target is not in the scope of ll/sc or lld/scd, a sync is
> > >> needed at the postion of target.
> > >
> > > OK, so is this the same issue that the second patch in the series is
> > > working around or a different one?
> > >
> > > I'm pretty confused at this point about what the actual bugs are in
> > > these various Loongson CPUs. Could someone provide an actual errata
> > > writeup describing the bugs in detail?
> > >
> > > What does "in the scope of ll/sc" mean?
> >
> > Loongson 3 series has some version, called, 1000, 2000, and 3000.
> >
> > There are 2 bugs all about LL/SC. Let’s call them bug-1 and bug-2.
> >
> > BUG-1:  a `sync’ is needed before LL or LLD instruction.
> >               This bug appears on 1000 only, and I am sure that it has been fixed in 3000.
> >
> > BUG-2: if there is an branch instruction inside LL/SC, and the branch target is outside
> >              of the scope of LL/SC, a `sync’ is needed at the branch target.
> >              Aka, the first insn of the target branch should be `sync’.
> >              Loongson said that, we don’t plan fix this problem in short time before they
> >              Designe a totally new core.
> >
> >
> > > What happens if a branch target is not "in the scope of ll/sc”?
> >
> > At least they said that there won’t be a problem
>
> You still didn't define what "in the scope of ll/sc" means - I'm
> guessing that you're referring to a branch target as "in scope" if it is
> in between the ll & sc instructions (inclusive?). But this is just a
> guess & clarity from people who actually know would be helpful.
>

Yes. your guess is correct. It is between.

> And there must be a problem. The whole point of this is that there's a
> bug, right? If there's no problem then we don't need to do anything :)
>

Sure. It is a problem.
Some Loongson guys seem no dare to say out their CPU is buggy.

> From a look at the GCC patch it talks about placing a sync at a branch
> target if it *is* in between an ll & sc [1], which I just can't
> reconcile with the phrase "outside of the scope of LL/SC". Is the
> problem when a branch target *is* in between an ll & sc, or when it *is
> not* between an ll & sc?

This problem happens when:
    the branch insn like `beq' is between ll and sc
      AND
    the target of the branch insn is not between ll/sc

>
> Reading this kernel patch doesn't make it any clearer - for example the
> sync it emits in build_loongson3_tlb_refill_handler() is nowhere near an
> ll or sc instruction. Something doesn't add up here.
>

Loongson guys told me that, there is a branch insn between ll and sc
may jump here.
In fact I don't know where is the insn.

> > > How does the sync help?
> > >
> > > Are jumps affected, or just branches?
> >
> > I am not sure, so CC a Loongson people.
> > @Paul Hua
>
> Hi Paul - any help obtaining a detailed description of these bugs would
> be much appreciated. Even if you only have something in Chinese I can
> probably get someone to help translate.
>
> > > Does this affect userland as well as the kernel?
> >
> > There is few place can trigger these 2 bugs in kernel.
> > In user land we have to workaround in binutils:
> >    https://www.sourceware.org/ml/binutils/2019-01/msg00025.html
> >
> > In fact the kernel is the easiest since we can have a flavor build for Loongson.
>
> My concern with regards to userland is that there's talk of a "deadlock"
> - if userland can hit this & the CPU actually stalls then the system is
> hopelessly vulnerable to denial of service from a malicious or buggy
> userland program, or simply an innocent program unaware of the errata.
>

I have an Loongson 3A 3000 laptop.
If without any workaround, the whole system hangs very frequently.
With this patch, the whole system hangs rarely.

Since the bug effects the userland, applications still hangs
frequently, for example `tmux'.

In Debian, we have a list packages that can build on Cavium while
cannot on Loongson 1000.
       bcftools botch casacore ceres-solver chemps2 clippoly cpl-plugin-giraf
       cpl-plugin-xshoo dolfin freeipa git golang-1.11 graphicsmagick
igraph libminc knot-resolver nodejs
       octave-ltfat prodigal pypy redis ruby2.3 ghc yade
Most of them fail due to hangs.
I tested them on Loongson 3K, some of them can build successfully now,
and some of them cannot
build still.

I guess the reason is that we also need some workaround in userland,
like libc etc.

> > > ...and probably more questions depending upon the answers to these ones.
> > >
> > >> Loongson doesn't plan to fix this problem in future, so we add the
> > >> sync here for any condition.
> > >
> > > So are you saying that future Loongson CPUs will all be buggy too, and
> > > someone there has said that they consider this to be OK..? I really
> > > really hope that is not true.
> >
> > Bug is bug. It is not OK.
> > I blame these Loongson guys here.
> > Some Loongson guys is not so normal people.
> > Anyway they are a little more normal now, and anyway again, still abnormal.
> >
> > > If hardware people say they're not going to fix their bugs then working
> > > around them is definitely not going to be a priority. It's one thing if
> > > a CPU designer says "oops, my bad, work around this & I'll fix it next
> > > time". It's quite another for them to say they're not interested in
> > > fixing their bugs at all.
> >
> > They have interests, while I guess the true reason is that they have no enough
> > people and money to desgin a core, while this bug is quilt hard to fix.
>
> I'm not sure I fully understand what you're saying above, but
> essentially I want to know that Loongson care about fixing their CPU
> bugs. If they don't, and the bugs are as bad as they sound, then in my
> view working around them will only reinforce that producing CPUs with
> such serious bugs is a good idea.
>

Yes, you are correct. (some bad words here.

> So if anyone from Loongson is reading, I'd really like to hear that the
> above is a miscommunication & that you're not intending to knowingly
> design any further CPUs with these bugs.
>

In fact I told with them lots of  times face to face.
The only improve of them is that finally they can say out this is a
bug not features.

> Thanks,
>     Paul
>
> [1] https://gcc.gnu.org/ml/gcc-patches/2018-12/msg01064.html
>     ("Loongson3 need a sync before branch target that between ll and sc.")
diff mbox series

Patch

diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
index 37b1cb246..08a9a66ef 100644
--- a/arch/mips/mm/tlbex.c
+++ b/arch/mips/mm/tlbex.c
@@ -932,6 +932,8 @@  build_get_pgd_vmalloc64(u32 **p, struct uasm_label **l, struct uasm_reloc **r,
 		 * to mimic that here by taking a load/istream page
 		 * fault.
 		 */
+		if(current_cpu_type() == CPU_LOONGSON3)
+			uasm_i_sync(p, 0);
 		UASM_i_LA(p, ptr, (unsigned long)tlb_do_page_fault_0);
 		uasm_i_jr(p, ptr);
 
@@ -1556,6 +1558,7 @@  static void build_loongson3_tlb_refill_handler(void)
 
 	if (check_for_high_segbits) {
 		uasm_l_large_segbits_fault(&l, p);
+		uasm_i_sync(&p, 0);
 		UASM_i_LA(&p, K1, (unsigned long)tlb_do_page_fault_0);
 		uasm_i_jr(&p, K1);
 		uasm_i_nop(&p);
@@ -2259,6 +2262,8 @@  static void build_r4000_tlb_load_handler(void)
 #endif
 
 	uasm_l_nopage_tlbl(&l, p);
+	if(current_cpu_type() == CPU_LOONGSON3)
+		uasm_i_sync(&p, 0);
 	build_restore_work_registers(&p);
 #ifdef CONFIG_CPU_MICROMIPS
 	if ((unsigned long)tlb_do_page_fault_0 & 1) {
@@ -2313,6 +2318,8 @@  static void build_r4000_tlb_store_handler(void)
 #endif
 
 	uasm_l_nopage_tlbs(&l, p);
+	if(current_cpu_type() == CPU_LOONGSON3)
+		uasm_i_sync(&p, 0);
 	build_restore_work_registers(&p);
 #ifdef CONFIG_CPU_MICROMIPS
 	if ((unsigned long)tlb_do_page_fault_1 & 1) {
@@ -2368,6 +2375,8 @@  static void build_r4000_tlb_modify_handler(void)
 #endif
 
 	uasm_l_nopage_tlbm(&l, p);
+	if(current_cpu_type() == CPU_LOONGSON3)
+		uasm_i_sync(&p, 0);
 	build_restore_work_registers(&p);
 #ifdef CONFIG_CPU_MICROMIPS
 	if ((unsigned long)tlb_do_page_fault_1 & 1) {