* [PATCH] MIPS: tlbex: fix a missing statement for HUGETLB
@ 2014-07-29 6:54 ` Huacai Chen
0 siblings, 0 replies; 16+ messages in thread
From: Huacai Chen @ 2014-07-29 6:54 UTC (permalink / raw)
To: Ralf Baechle
Cc: John Crispin, Steven J. Hill, linux-mips, Fuxin Zhang,
Zhangjin Wu, Huacai Chen, Binbin Zhou, stable
In commit 2c8c53e28f1 (MIPS: Optimize TLB handlers for Octeon CPUs)
build_r4000_tlb_refill_handler() is modified. But it doesn't compatible
with the original code in HUGETLB case. Because there is a copy & paste
error and one line of code is missing. It is very easy to produce a bug
with LTP's hugemmap05 test.
Signed-off-by: Huacai Chen <chenhc@lemote.com>
Signed-off-by: Binbin Zhou <zhoubb@lemote.com>
Cc: <stable@vger.kernel.org>
---
arch/mips/mm/tlbex.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
index e80e10b..343fe0f 100644
--- a/arch/mips/mm/tlbex.c
+++ b/arch/mips/mm/tlbex.c
@@ -1299,6 +1299,7 @@ static void build_r4000_tlb_refill_handler(void)
}
#ifdef CONFIG_MIPS_HUGE_TLB_SUPPORT
uasm_l_tlb_huge_update(&l, p);
+ UASM_i_LW(&p, K0, 0, K1);
build_huge_update_entries(&p, htlb_info.huge_pte, K1);
build_huge_tlb_write_entry(&p, &l, &r, K0, tlb_random,
htlb_info.restore_scratch);
--
1.9.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH] MIPS: tlbex: fix a missing statement for HUGETLB
@ 2014-07-29 6:54 ` Huacai Chen
0 siblings, 0 replies; 16+ messages in thread
From: Huacai Chen @ 2014-07-29 6:54 UTC (permalink / raw)
To: Ralf Baechle
Cc: John Crispin, Steven J. Hill, linux-mips, Fuxin Zhang,
Zhangjin Wu, Huacai Chen, Binbin Zhou, stable
In commit 2c8c53e28f1 (MIPS: Optimize TLB handlers for Octeon CPUs)
build_r4000_tlb_refill_handler() is modified. But it doesn't compatible
with the original code in HUGETLB case. Because there is a copy & paste
error and one line of code is missing. It is very easy to produce a bug
with LTP's hugemmap05 test.
Signed-off-by: Huacai Chen <chenhc@lemote.com>
Signed-off-by: Binbin Zhou <zhoubb@lemote.com>
Cc: <stable@vger.kernel.org>
---
arch/mips/mm/tlbex.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
index e80e10b..343fe0f 100644
--- a/arch/mips/mm/tlbex.c
+++ b/arch/mips/mm/tlbex.c
@@ -1299,6 +1299,7 @@ static void build_r4000_tlb_refill_handler(void)
}
#ifdef CONFIG_MIPS_HUGE_TLB_SUPPORT
uasm_l_tlb_huge_update(&l, p);
+ UASM_i_LW(&p, K0, 0, K1);
build_huge_update_entries(&p, htlb_info.huge_pte, K1);
build_huge_tlb_write_entry(&p, &l, &r, K0, tlb_random,
htlb_info.restore_scratch);
--
1.9.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] MIPS: tlbex: fix a missing statement for HUGETLB
2014-07-29 6:54 ` Huacai Chen
(?)
@ 2014-07-30 16:37 ` Aurelien Jarno
-1 siblings, 0 replies; 16+ messages in thread
From: Aurelien Jarno @ 2014-07-30 16:37 UTC (permalink / raw)
To: Huacai Chen
Cc: Ralf Baechle, John Crispin, Steven J. Hill, linux-mips,
Fuxin Zhang, Zhangjin Wu, Binbin Zhou, stable
On Tue, Jul 29, 2014 at 02:54:40PM +0800, Huacai Chen wrote:
> In commit 2c8c53e28f1 (MIPS: Optimize TLB handlers for Octeon CPUs)
> build_r4000_tlb_refill_handler() is modified. But it doesn't compatible
> with the original code in HUGETLB case. Because there is a copy & paste
> error and one line of code is missing. It is very easy to produce a bug
> with LTP's hugemmap05 test.
>
> Signed-off-by: Huacai Chen <chenhc@lemote.com>
> Signed-off-by: Binbin Zhou <zhoubb@lemote.com>
> Cc: <stable@vger.kernel.org>
> ---
> arch/mips/mm/tlbex.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
> index e80e10b..343fe0f 100644
> --- a/arch/mips/mm/tlbex.c
> +++ b/arch/mips/mm/tlbex.c
> @@ -1299,6 +1299,7 @@ static void build_r4000_tlb_refill_handler(void)
> }
> #ifdef CONFIG_MIPS_HUGE_TLB_SUPPORT
> uasm_l_tlb_huge_update(&l, p);
> + UASM_i_LW(&p, K0, 0, K1);
> build_huge_update_entries(&p, htlb_info.huge_pte, K1);
> build_huge_tlb_write_entry(&p, &l, &r, K0, tlb_random,
> htlb_info.restore_scratch);
Thanks for this patch. It also fixes TRANSPARENT_HUGEPAGE=y, which we
had to disable on Loongson 3 as it was causing kernel crashes. This
option is quite interesting for most MIPS systems, which are using
a software driven TLB.
Tested-by: Aurelien Jarno <aurelien@aurel32.net>
Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] MIPS: tlbex: fix a missing statement for HUGETLB
2014-07-29 6:54 ` Huacai Chen
(?)
(?)
@ 2014-07-30 21:41 ` James Hogan
2014-07-30 21:44 ` David Daney
-1 siblings, 1 reply; 16+ messages in thread
From: James Hogan @ 2014-07-30 21:41 UTC (permalink / raw)
To: linux-mips
Cc: Huacai Chen, Ralf Baechle, John Crispin, Steven J. Hill,
Fuxin Zhang, Zhangjin Wu, Binbin Zhou, stable, David Daney
Hi Huacai,
On Tuesday 29 July 2014 14:54:40 Huacai Chen wrote:
> In commit 2c8c53e28f1 (MIPS: Optimize TLB handlers for Octeon CPUs)
> build_r4000_tlb_refill_handler() is modified. But it doesn't compatible
> with the original code in HUGETLB case. Because there is a copy & paste
> error and one line of code is missing. It is very easy to produce a bug
> with LTP's hugemmap05 test.
>
> Signed-off-by: Huacai Chen <chenhc@lemote.com>
> Signed-off-by: Binbin Zhou <zhoubb@lemote.com>
> Cc: <stable@vger.kernel.org>
> ---
> arch/mips/mm/tlbex.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
> index e80e10b..343fe0f 100644
> --- a/arch/mips/mm/tlbex.c
> +++ b/arch/mips/mm/tlbex.c
> @@ -1299,6 +1299,7 @@ static void build_r4000_tlb_refill_handler(void)
> }
> #ifdef CONFIG_MIPS_HUGE_TLB_SUPPORT
> uasm_l_tlb_huge_update(&l, p);
> + UASM_i_LW(&p, K0, 0, K1);
> build_huge_update_entries(&p, htlb_info.huge_pte, K1);
> build_huge_tlb_write_entry(&p, &l, &r, K0, tlb_random,
> htlb_info.restore_scratch);
build_huge_tlb_write_entry only uses K0 as a temp and clobbers without using
the value, so the K0 must be being used by the code generated by
build_huge_update_entires, but the patch you mentioned changed the second
argument from K0 to htlb_info.huge_pte.
So should the K0 in the new UASM_i_LW call be changed to htlb_info.huge_pte
too?
(David Daney on Cc)
Thanks
James
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] MIPS: tlbex: fix a missing statement for HUGETLB
@ 2014-07-30 21:44 ` David Daney
0 siblings, 0 replies; 16+ messages in thread
From: David Daney @ 2014-07-30 21:44 UTC (permalink / raw)
To: James Hogan
Cc: linux-mips, Huacai Chen, Ralf Baechle, John Crispin,
Steven J. Hill, Fuxin Zhang, Zhangjin Wu, Binbin Zhou, stable
On 07/30/2014 02:41 PM, James Hogan wrote:
> Hi Huacai,
>
> On Tuesday 29 July 2014 14:54:40 Huacai Chen wrote:
>> In commit 2c8c53e28f1 (MIPS: Optimize TLB handlers for Octeon CPUs)
>> build_r4000_tlb_refill_handler() is modified. But it doesn't compatible
>> with the original code in HUGETLB case. Because there is a copy & paste
>> error and one line of code is missing. It is very easy to produce a bug
>> with LTP's hugemmap05 test.
>>
>> Signed-off-by: Huacai Chen <chenhc@lemote.com>
>> Signed-off-by: Binbin Zhou <zhoubb@lemote.com>
>> Cc: <stable@vger.kernel.org>
>> ---
>> arch/mips/mm/tlbex.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
>> index e80e10b..343fe0f 100644
>> --- a/arch/mips/mm/tlbex.c
>> +++ b/arch/mips/mm/tlbex.c
>> @@ -1299,6 +1299,7 @@ static void build_r4000_tlb_refill_handler(void)
>> }
>> #ifdef CONFIG_MIPS_HUGE_TLB_SUPPORT
>> uasm_l_tlb_huge_update(&l, p);
>> + UASM_i_LW(&p, K0, 0, K1);
>> build_huge_update_entries(&p, htlb_info.huge_pte, K1);
>> build_huge_tlb_write_entry(&p, &l, &r, K0, tlb_random,
>> htlb_info.restore_scratch);
>
> build_huge_tlb_write_entry only uses K0 as a temp and clobbers without using
> the value, so the K0 must be being used by the code generated by
> build_huge_update_entires, but the patch you mentioned changed the second
> argument from K0 to htlb_info.huge_pte.
>
> So should the K0 in the new UASM_i_LW call be changed to htlb_info.huge_pte
> too?
>
I don't know. You have to dump out the generated handler (by #define
DEBUG at the top of the file), then assemble/disassemble it. Looking at
the disassembly, we could make sensible statements about what is happening.
> (David Daney on Cc)
>
> Thanks
> James
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] MIPS: tlbex: fix a missing statement for HUGETLB
@ 2014-07-30 21:44 ` David Daney
0 siblings, 0 replies; 16+ messages in thread
From: David Daney @ 2014-07-30 21:44 UTC (permalink / raw)
To: James Hogan
Cc: linux-mips, Huacai Chen, Ralf Baechle, John Crispin,
Steven J. Hill, Fuxin Zhang, Zhangjin Wu, Binbin Zhou, stable
On 07/30/2014 02:41 PM, James Hogan wrote:
> Hi Huacai,
>
> On Tuesday 29 July 2014 14:54:40 Huacai Chen wrote:
>> In commit 2c8c53e28f1 (MIPS: Optimize TLB handlers for Octeon CPUs)
>> build_r4000_tlb_refill_handler() is modified. But it doesn't compatible
>> with the original code in HUGETLB case. Because there is a copy & paste
>> error and one line of code is missing. It is very easy to produce a bug
>> with LTP's hugemmap05 test.
>>
>> Signed-off-by: Huacai Chen <chenhc@lemote.com>
>> Signed-off-by: Binbin Zhou <zhoubb@lemote.com>
>> Cc: <stable@vger.kernel.org>
>> ---
>> arch/mips/mm/tlbex.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
>> index e80e10b..343fe0f 100644
>> --- a/arch/mips/mm/tlbex.c
>> +++ b/arch/mips/mm/tlbex.c
>> @@ -1299,6 +1299,7 @@ static void build_r4000_tlb_refill_handler(void)
>> }
>> #ifdef CONFIG_MIPS_HUGE_TLB_SUPPORT
>> uasm_l_tlb_huge_update(&l, p);
>> + UASM_i_LW(&p, K0, 0, K1);
>> build_huge_update_entries(&p, htlb_info.huge_pte, K1);
>> build_huge_tlb_write_entry(&p, &l, &r, K0, tlb_random,
>> htlb_info.restore_scratch);
>
> build_huge_tlb_write_entry only uses K0 as a temp and clobbers without using
> the value, so the K0 must be being used by the code generated by
> build_huge_update_entires, but the patch you mentioned changed the second
> argument from K0 to htlb_info.huge_pte.
>
> So should the K0 in the new UASM_i_LW call be changed to htlb_info.huge_pte
> too?
>
I don't know. You have to dump out the generated handler (by #define
DEBUG at the top of the file), then assemble/disassemble it. Looking at
the disassembly, we could make sensible statements about what is happening.
> (David Daney on Cc)
>
> Thanks
> James
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] MIPS: tlbex: fix a missing statement for HUGETLB
2014-07-30 21:44 ` David Daney
(?)
@ 2014-07-31 0:48 ` Huacai Chen
2014-07-31 1:13 ` David Daney
-1 siblings, 1 reply; 16+ messages in thread
From: Huacai Chen @ 2014-07-31 0:48 UTC (permalink / raw)
To: David Daney
Cc: James Hogan, Linux MIPS Mailing List, Ralf Baechle, John Crispin,
Steven J. Hill, Fuxin Zhang, Zhangjin Wu, Binbin Zhou, stable
Hi, David,
For non-Octeon CPU, htlb_info.huge_pte is equal to K0, but I don't
know much about Octeon. So I think you know whether we should use K0
or htlb_info.huge_pte here, since you are the original author.
On Thu, Jul 31, 2014 at 5:44 AM, David Daney <ddaney@caviumnetworks.com> wrote:
> On 07/30/2014 02:41 PM, James Hogan wrote:
>>
>> Hi Huacai,
>>
>> On Tuesday 29 July 2014 14:54:40 Huacai Chen wrote:
>>>
>>> In commit 2c8c53e28f1 (MIPS: Optimize TLB handlers for Octeon CPUs)
>>> build_r4000_tlb_refill_handler() is modified. But it doesn't compatible
>>> with the original code in HUGETLB case. Because there is a copy & paste
>>> error and one line of code is missing. It is very easy to produce a bug
>>> with LTP's hugemmap05 test.
>>>
>>> Signed-off-by: Huacai Chen <chenhc@lemote.com>
>>> Signed-off-by: Binbin Zhou <zhoubb@lemote.com>
>>> Cc: <stable@vger.kernel.org>
>>> ---
>>> arch/mips/mm/tlbex.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
>>> index e80e10b..343fe0f 100644
>>> --- a/arch/mips/mm/tlbex.c
>>> +++ b/arch/mips/mm/tlbex.c
>>> @@ -1299,6 +1299,7 @@ static void build_r4000_tlb_refill_handler(void)
>>> }
>>> #ifdef CONFIG_MIPS_HUGE_TLB_SUPPORT
>>> uasm_l_tlb_huge_update(&l, p);
>>> + UASM_i_LW(&p, K0, 0, K1);
>>> build_huge_update_entries(&p, htlb_info.huge_pte, K1);
>>> build_huge_tlb_write_entry(&p, &l, &r, K0, tlb_random,
>>> htlb_info.restore_scratch);
>>
>>
>> build_huge_tlb_write_entry only uses K0 as a temp and clobbers without
>> using
>> the value, so the K0 must be being used by the code generated by
>> build_huge_update_entires, but the patch you mentioned changed the second
>> argument from K0 to htlb_info.huge_pte.
>>
>> So should the K0 in the new UASM_i_LW call be changed to
>> htlb_info.huge_pte
>> too?
>>
>
> I don't know. You have to dump out the generated handler (by #define DEBUG
> at the top of the file), then assemble/disassemble it. Looking at the
> disassembly, we could make sensible statements about what is happening.
>
>
>
>> (David Daney on Cc)
>>
>> Thanks
>> James
>>
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] MIPS: tlbex: fix a missing statement for HUGETLB
2014-07-31 0:48 ` Huacai Chen
@ 2014-07-31 1:13 ` David Daney
2014-07-31 11:54 ` James Hogan
0 siblings, 1 reply; 16+ messages in thread
From: David Daney @ 2014-07-31 1:13 UTC (permalink / raw)
To: Huacai Chen
Cc: James Hogan, Linux MIPS Mailing List, Ralf Baechle, John Crispin,
Steven J. Hill, Fuxin Zhang, Zhangjin Wu, Binbin Zhou, stable
On 07/30/2014 05:48 PM, Huacai Chen wrote:
> Hi, David,
>
> For non-Octeon CPU, htlb_info.huge_pte is equal to K0, but I don't
> know much about Octeon. So I think you know whether we should use K0
> or htlb_info.huge_pte here, since you are the original author.
>
This is why I requested that somebody show me a disassembly of the
faulty handler. I cannot tell where the problem is unless I see that.
Really I think the problem is in build_is_huge_pte(), where we are
clobbering 'tmp' which is K0.
So you could reload tmp/K0 in build_is_huge_pte().
> On Thu, Jul 31, 2014 at 5:44 AM, David Daney
> <ddaney@caviumnetworks.com> wrote:
>> On 07/30/2014 02:41 PM, James Hogan wrote:
>>>
>>> Hi Huacai,
>>>
>>> On Tuesday 29 July 2014 14:54:40 Huacai Chen wrote:
>>>>
>>>> In commit 2c8c53e28f1 (MIPS: Optimize TLB handlers for Octeon
>>>> CPUs) build_r4000_tlb_refill_handler() is modified. But it
>>>> doesn't compatible with the original code in HUGETLB case.
>>>> Because there is a copy & paste error and one line of code is
>>>> missing. It is very easy to produce a bug with LTP's
>>>> hugemmap05 test.
>>>>
>>>> Signed-off-by: Huacai Chen <chenhc@lemote.com> Signed-off-by:
>>>> Binbin Zhou <zhoubb@lemote.com> Cc: <stable@vger.kernel.org>
>>>> --- arch/mips/mm/tlbex.c | 1 + 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c index
>>>> e80e10b..343fe0f 100644 --- a/arch/mips/mm/tlbex.c +++
>>>> b/arch/mips/mm/tlbex.c @@ -1299,6 +1299,7 @@ static void
>>>> build_r4000_tlb_refill_handler(void) } #ifdef
>>>> CONFIG_MIPS_HUGE_TLB_SUPPORT uasm_l_tlb_huge_update(&l, p); +
>>>> UASM_i_LW(&p, K0, 0, K1); build_huge_update_entries(&p,
>>>> htlb_info.huge_pte, K1); build_huge_tlb_write_entry(&p, &l,
>>>> &r, K0, tlb_random, htlb_info.restore_scratch);
>>>
>>>
>>> build_huge_tlb_write_entry only uses K0 as a temp and clobbers
>>> without using the value, so the K0 must be being used by the
>>> code generated by build_huge_update_entires, but the patch you
>>> mentioned changed the second argument from K0 to
>>> htlb_info.huge_pte.
>>>
>>> So should the K0 in the new UASM_i_LW call be changed to
>>> htlb_info.huge_pte too?
>>>
>>
>> I don't know. You have to dump out the generated handler (by
>> #define DEBUG at the top of the file), then assemble/disassemble
>> it. Looking at the disassembly, we could make sensible statements
>> about what is happening.
>>
>>
>>
>>> (David Daney on Cc)
>>>
>>> Thanks James
>>>
>>
>>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] MIPS: tlbex: fix a missing statement for HUGETLB
2014-07-31 1:13 ` David Daney
@ 2014-07-31 11:54 ` James Hogan
2014-07-31 17:33 ` David Daney
0 siblings, 1 reply; 16+ messages in thread
From: James Hogan @ 2014-07-31 11:54 UTC (permalink / raw)
To: David Daney, Huacai Chen
Cc: James Hogan, Linux MIPS Mailing List, Ralf Baechle, John Crispin,
Steven J. Hill, Fuxin Zhang, Zhangjin Wu, Binbin Zhou, stable
Hi,
On 31/07/14 02:13, David Daney wrote:
> On 07/30/2014 05:48 PM, Huacai Chen wrote:
>> For non-Octeon CPU, htlb_info.huge_pte is equal to K0, but I don't
>> know much about Octeon. So I think you know whether we should use K0
>> or htlb_info.huge_pte here, since you are the original author.
>>
>
> This is why I requested that somebody show me a disassembly of the
> faulty handler. I cannot tell where the problem is unless I see that.
>
> Really I think the problem is in build_is_huge_pte(), where we are
> clobbering 'tmp' which is K0.
>
> So you could reload tmp/K0 in build_is_huge_pte().
Here's the difference with this patch (using k0) on an Octeon I have to
hand, with some slightly munged offsets for nicer diffing:
#define _PAGE_PRESENT_SHIFT 0
#define _PAGE_READ_SHIFT 0
#define _PAGE_WRITE_SHIFT 1
#define _PAGE_ACCESSED_SHIFT 2
#define _PAGE_MODIFIED_SHIFT 3
#define _PAGE_HUGE_SHIFT 4
#define _PAGE_SPLITTING_SHIFT 5
#define _PAGE_NO_EXEC_SHIFT 6
#define _PAGE_NO_READ_SHIFT 7
#define _PAGE_GLOBAL_SHIFT 8
#define _PAGE_VALID_SHIFT 9
#define _PAGE_DIRTY_SHIFT 10
#define _PFN_SHIFT 14
00000000 <r4000_tlb_refill>:
+ 0: df7a0000 ld k0,0(k1)
4: 00210a3a dror at,at,0x8
8: 40a11000 dmtc0 at,c0_entrylo0
c: 64214000 daddiu at,at,16384
10: 40a11800 dmtc0 at,c0_entrylo1
14: 3c1a001f lui k0,0x1f
18: 375ae000 ori k0,k0,0xe000
1c: 409a2800 mtc0 k0,c0_pagemask
20: 000000c0 ehb
24: 42000006 tlbwr
28: 40802800 mtc0 zero,c0_pagemask
- 28: 1000002e b e4 <r4000_tlb_refill+0xe4>
+ 2c: 1000002d b e4 <r4000_tlb_refill+0xe4>
30: 4021f802 dmfc0 at,$31,2
- 30: 07400019 bltz k0,98 <r4000_tlb_refill+0x98>
+ 34: 07400018 bltz k0,98 <r4000_tlb_refill+0x98>
38: 3c1b81da lui k1,0x81da
3c: 3c1b8113 lui k1,0x8113
- 3c: 277b7ef0 addiu k1,k1,32496
+ 40: 277b7f00 addiu k1,k1,32512
44: 03600008 jr k1
48: 4021f802 dmfc0 at,$31,2
...
80: 403a4000 dmfc0 k0,c0_badvaddr
84: 403bf803 dmfc0 k1,$31,3
88: 40a1f802 dmtc0 at,$31,2
8c: 001a0a3e dsrl32 at,k0,0x8
- 90: 1420ffe7 bnez at,30 <r4000_tlb_refill+0x30>
+ 90: 1420ffe8 bnez at,34 <r4000_tlb_refill+0x34>
94: 001a0efa dsrl at,k0,0x1b
98: 30211ff8 andi at,at,0x1ff8
9c: 7c3bda0a ldx k1,k1(at)
a0: 001a0cba dsrl at,k0,0x12
a4: 30210ff8 andi at,at,0xff8
a8: 403aa000 dmfc0 k0,c0_xcontext
ac: 7c3b0a0a ldx at,k1(at)
b0: 335a0ff0 andi k0,k0,0xff0
b4: e824ffd2 bbit1 at,0x4,0 <r4000_tlb_refill>
b8: 00000000 nop
bc: 7c3ada0a ldx k1,k0(at)
c0: 675a0008 daddiu k0,k0,8
c4: 7c3ad20a ldx k0,k0(at)
c8: 003bda3a dror k1,k1,0x8
cc: 40bb1000 dmtc0 k1,c0_entrylo0
d0: 003ad23a dror k0,k0,0x8
d4: 40ba1800 dmtc0 k0,c0_entrylo1
d8: 4021f802 dmfc0 at,$31,2
dc: 000000c0 ehb
e0: 42000006 tlbwr
e4: 42000018 eret
b4 is apparently where it branches back to the huge page case at the
beginning. In that case the at register (htlb_info.huge_pte) is set to
*(k1+at) instead of *(k1), so loading to htlb_info.huge_pte instead of
k0 would I think be bad and change the behaviour. So forget my suggestion!
On the other hand loading the pte to k0 is redundant when
build_fast_tlb_refill_handler is used (which depends on bbit1), and also
in the other case if bbit1 is available since it won't get clobbered by
build_is_huge_pte().
Maybe the reload should simply be conditional on !use_bbit_insns()?
Cheers
James
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] MIPS: tlbex: fix a missing statement for HUGETLB
2014-07-31 11:54 ` James Hogan
@ 2014-07-31 17:33 ` David Daney
2014-08-02 21:35 ` Aurelien Jarno
0 siblings, 1 reply; 16+ messages in thread
From: David Daney @ 2014-07-31 17:33 UTC (permalink / raw)
To: James Hogan
Cc: Huacai Chen, James Hogan, Linux MIPS Mailing List, Ralf Baechle,
John Crispin, Steven J. Hill, Fuxin Zhang, Zhangjin Wu,
Binbin Zhou, stable
On 07/31/2014 04:54 AM, James Hogan wrote:
> Hi,
>
> On 31/07/14 02:13, David Daney wrote:
>> On 07/30/2014 05:48 PM, Huacai Chen wrote:
>>> For non-Octeon CPU, htlb_info.huge_pte is equal to K0, but I don't
>>> know much about Octeon. So I think you know whether we should use K0
>>> or htlb_info.huge_pte here, since you are the original author.
>>>
>>
>> This is why I requested that somebody show me a disassembly of the
>> faulty handler. I cannot tell where the problem is unless I see that.
>>
>> Really I think the problem is in build_is_huge_pte(), where we are
>> clobbering 'tmp' which is K0.
>>
>> So you could reload tmp/K0 in build_is_huge_pte().
>
> Here's the difference with this patch (using k0) on an Octeon I have to
> hand, with some slightly munged offsets for nicer diffing:
>
> #define _PAGE_PRESENT_SHIFT 0
> #define _PAGE_READ_SHIFT 0
> #define _PAGE_WRITE_SHIFT 1
> #define _PAGE_ACCESSED_SHIFT 2
> #define _PAGE_MODIFIED_SHIFT 3
> #define _PAGE_HUGE_SHIFT 4
> #define _PAGE_SPLITTING_SHIFT 5
> #define _PAGE_NO_EXEC_SHIFT 6
> #define _PAGE_NO_READ_SHIFT 7
> #define _PAGE_GLOBAL_SHIFT 8
> #define _PAGE_VALID_SHIFT 9
> #define _PAGE_DIRTY_SHIFT 10
> #define _PFN_SHIFT 14
>
> 00000000 <r4000_tlb_refill>:
> + 0: df7a0000 ld k0,0(k1)
Completely redundant, it is not used and then clobbered...
> 4: 00210a3a dror at,at,0x8
> 8: 40a11000 dmtc0 at,c0_entrylo0
> c: 64214000 daddiu at,at,16384
> 10: 40a11800 dmtc0 at,c0_entrylo1
> 14: 3c1a001f lui k0,0x1f
... here.
> 18: 375ae000 ori k0,k0,0xe000
> 1c: 409a2800 mtc0 k0,c0_pagemask
> 20: 000000c0 ehb
> 24: 42000006 tlbwr
> 28: 40802800 mtc0 zero,c0_pagemask
> - 28: 1000002e b e4 <r4000_tlb_refill+0xe4>
> + 2c: 1000002d b e4 <r4000_tlb_refill+0xe4>
> 30: 4021f802 dmfc0 at,$31,2
> - 30: 07400019 bltz k0,98 <r4000_tlb_refill+0x98>
> + 34: 07400018 bltz k0,98 <r4000_tlb_refill+0x98>
> 38: 3c1b81da lui k1,0x81da
> 3c: 3c1b8113 lui k1,0x8113
> - 3c: 277b7ef0 addiu k1,k1,32496
> + 40: 277b7f00 addiu k1,k1,32512
> 44: 03600008 jr k1
> 48: 4021f802 dmfc0 at,$31,2
> ...
> 80: 403a4000 dmfc0 k0,c0_badvaddr
> 84: 403bf803 dmfc0 k1,$31,3
> 88: 40a1f802 dmtc0 at,$31,2
> 8c: 001a0a3e dsrl32 at,k0,0x8
> - 90: 1420ffe7 bnez at,30 <r4000_tlb_refill+0x30>
> + 90: 1420ffe8 bnez at,34 <r4000_tlb_refill+0x34>
> 94: 001a0efa dsrl at,k0,0x1b
> 98: 30211ff8 andi at,at,0x1ff8
> 9c: 7c3bda0a ldx k1,k1(at)
> a0: 001a0cba dsrl at,k0,0x12
> a4: 30210ff8 andi at,at,0xff8
> a8: 403aa000 dmfc0 k0,c0_xcontext
> ac: 7c3b0a0a ldx at,k1(at)
> b0: 335a0ff0 andi k0,k0,0xff0
> b4: e824ffd2 bbit1 at,0x4,0 <r4000_tlb_refill>
> b8: 00000000 nop
> bc: 7c3ada0a ldx k1,k0(at)
> c0: 675a0008 daddiu k0,k0,8
> c4: 7c3ad20a ldx k0,k0(at)
> c8: 003bda3a dror k1,k1,0x8
> cc: 40bb1000 dmtc0 k1,c0_entrylo0
> d0: 003ad23a dror k0,k0,0x8
> d4: 40ba1800 dmtc0 k0,c0_entrylo1
> d8: 4021f802 dmfc0 at,$31,2
> dc: 000000c0 ehb
> e0: 42000006 tlbwr
> e4: 42000018 eret
>
>
> b4 is apparently where it branches back to the huge page case at the
> beginning. In that case the at register (htlb_info.huge_pte) is set to
> *(k1+at) instead of *(k1), so loading to htlb_info.huge_pte instead of
> k0 would I think be bad and change the behaviour. So forget my suggestion!
>
> On the other hand loading the pte to k0 is redundant when
> build_fast_tlb_refill_handler is used (which depends on bbit1), and also
> in the other case if bbit1 is available since it won't get clobbered by
> build_is_huge_pte().
>
> Maybe the reload should simply be conditional on !use_bbit_insns()?
>
That was kind of my suggestion. What happens if you do something like
(untested):
--- a/arch/mips/mm/tlbex.c
+++ b/arch/mips/mm/tlbex.c
@@ -716,6 +716,7 @@ build_is_huge_pte(u32 **p, struct uasm_reloc **r,
unsigned int tmp,
} else {
uasm_i_andi(p, tmp, tmp, _PAGE_HUGE);
uasm_il_bnez(p, r, tmp, lid);
+ UASM_i_LW(p, tmp, 0, pmd);
}
}
or
diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
index f99ec587..341add1 100644
--- a/arch/mips/mm/tlbex.c
+++ b/arch/mips/mm/tlbex.c
@@ -1299,6 +1299,8 @@ static void build_r4000_tlb_refill_handler(void)
}
#ifdef CONFIG_MIPS_HUGE_TLB_SUPPORT
uasm_l_tlb_huge_update(&l, p);
+ if (!use_bbit_insns())
+ UASM_i_LW(&p, K0, 0, K1);
build_huge_update_entries(&p, htlb_info.huge_pte, K1);
build_huge_tlb_write_entry(&p, &l, &r, K0, tlb_random,
htlb_info.restore_scratch);
> Cheers
> James
>
>
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] MIPS: tlbex: fix a missing statement for HUGETLB
2014-07-31 17:33 ` David Daney
@ 2014-08-02 21:35 ` Aurelien Jarno
2014-08-04 10:08 ` James Hogan
0 siblings, 1 reply; 16+ messages in thread
From: Aurelien Jarno @ 2014-08-02 21:35 UTC (permalink / raw)
To: David Daney
Cc: James Hogan, Huacai Chen, James Hogan, Linux MIPS Mailing List,
Ralf Baechle, John Crispin, Steven J. Hill, Fuxin Zhang,
Zhangjin Wu, Binbin Zhou, stable
On Thu, Jul 31, 2014 at 10:33:55AM -0700, David Daney wrote:
> On 07/31/2014 04:54 AM, James Hogan wrote:
> >Hi,
> >
> >On 31/07/14 02:13, David Daney wrote:
> >>On 07/30/2014 05:48 PM, Huacai Chen wrote:
> >>>For non-Octeon CPU, htlb_info.huge_pte is equal to K0, but I don't
> >>>know much about Octeon. So I think you know whether we should use K0
> >>> or htlb_info.huge_pte here, since you are the original author.
> >>>
> >>
> >>This is why I requested that somebody show me a disassembly of the
> >>faulty handler. I cannot tell where the problem is unless I see that.
Here is the faulty handler, that is a dump on a machine affected by the
bug:
| #define _PAGE_PRESENT_SHIFT 0
| #define _PAGE_READ_SHIFT 1
| #define _PAGE_WRITE_SHIFT 2
| #define _PAGE_ACCESSED_SHIFT 3
| #define _PAGE_MODIFIED_SHIFT 4
| #define _PAGE_HUGE_SHIFT 5
| #define _PAGE_SPLITTING_SHIFT 6
| #define _PAGE_GLOBAL_SHIFT 7
| #define _PAGE_VALID_SHIFT 8
| #define _PAGE_DIRTY_SHIFT 9
| #define _PFN_SHIFT 13
|
| 0000000000000000 <r4000_tlb_refill>:
| 0: 001ad1fa dsrl k0,k0,0x7
| 4: 40ba1000 dmtc0 k0,c0_entrylo0
| 8: 675a4000 daddiu k0,k0,16384
| c: 40ba1800 dmtc0 k0,c0_entrylo1
| 10: 3c1a001f lui k0,0x1f
| 14: 375ae000 ori k0,k0,0xe000
| 18: 409a2800 mtc0 k0,c0_pagemask
| 1c: 42000006 tlbwr
| 20: 10000031 b e8 <r4000_tlb_refill+0xe8>
| 24: 40802800 mtc0 zero,c0_pagemask
| 28: 10000019 b 90 <r4000_tlb_refill+0x90>
| 2c: 3c1b8095 lui k1,0x8095
| ...
| 80: 403a4000 dmfc0 k0,c0_badvaddr
| 84: 0740ffe8 bltz k0,28 <r4000_tlb_refill+0x28>
| 88: 3c1b8095 lui k1,0x8095
| 8c: df7b4fb0 ld k1,20400(k1)
| 90: 001ad6fa dsrl k0,k0,0x1b
| 94: 335a1ff8 andi k0,k0,0x1ff8
| 98: 037ad82d daddu k1,k1,k0
| 9c: 403a4000 dmfc0 k0,c0_badvaddr
| a0: df7b0000 ld k1,0(k1)
| a4: 001ad4ba dsrl k0,k0,0x12
| a8: 335a0ff8 andi k0,k0,0xff8
| ac: 037ad82d daddu k1,k1,k0
| b0: df7a0000 ld k0,0(k1)
| b4: 335a0020 andi k0,k0,0x20
| b8: 1740ffd1 bnez k0,0 <r4000_tlb_refill>
| bc: 403aa000 dmfc0 k0,c0_xcontext
| c0: df7b0000 ld k1,0(k1)
| c4: 335a0ff0 andi k0,k0,0xff0
| c8: 037ad82d daddu k1,k1,k0
| cc: df7a0000 ld k0,0(k1)
| d0: df7b0008 ld k1,8(k1)
| d4: 001ad1fa dsrl k0,k0,0x7
| d8: 40ba1000 dmtc0 k0,c0_entrylo0
| dc: 001bd9fa dsrl k1,k1,0x7
| e0: 40bb1800 dmtc0 k1,c0_entrylo1
| e4: 42000006 tlbwr
| e8: 42000018 eret
| ...
> >>Really I think the problem is in build_is_huge_pte(), where we are
> >>clobbering 'tmp' which is K0.
Indeed, that is the problem. In the above code build_is_huge_pte()
corresponds to addresses b4 and b8. It needs huge_pte loaded in K0, but
at the same time it clobbers it. That's the reason why prior to commit
2c8c53e28f1, k0 was reloaded before calling build_huge_update_entries.
> >>So you could reload tmp/K0 in build_is_huge_pte().
> >
> >b4 is apparently where it branches back to the huge page case at the
> >beginning. In that case the at register (htlb_info.huge_pte) is set to
> >*(k1+at) instead of *(k1), so loading to htlb_info.huge_pte instead of
> >k0 would I think be bad and change the behaviour. So forget my suggestion!
> >
> >On the other hand loading the pte to k0 is redundant when
> >build_fast_tlb_refill_handler is used (which depends on bbit1), and also
> >in the other case if bbit1 is available since it won't get clobbered by
> >build_is_huge_pte().
> >
> >Maybe the reload should simply be conditional on !use_bbit_insns()?
> >
>
> That was kind of my suggestion. What happens if you do something
> like (untested):
>
> --- a/arch/mips/mm/tlbex.c
> +++ b/arch/mips/mm/tlbex.c
> @@ -716,6 +716,7 @@ build_is_huge_pte(u32 **p, struct uasm_reloc
> **r, unsigned int tmp,
> } else {
> uasm_i_andi(p, tmp, tmp, _PAGE_HUGE);
> uasm_il_bnez(p, r, tmp, lid);
> + UASM_i_LW(p, tmp, 0, pmd);
> }
> }
I haven't tested it, but it my opinion this patch won't work or at least
is suboptimal given build_is_huge_pte is also used to build
r4000_tlb_load, r4000_tlb_store and r4000_tlb_modify handlers.
> or
>
> diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
> index f99ec587..341add1 100644
> --- a/arch/mips/mm/tlbex.c
> +++ b/arch/mips/mm/tlbex.c
> @@ -1299,6 +1299,8 @@ static void build_r4000_tlb_refill_handler(void)
> }
> #ifdef CONFIG_MIPS_HUGE_TLB_SUPPORT
> uasm_l_tlb_huge_update(&l, p);
> + if (!use_bbit_insns())
> + UASM_i_LW(&p, K0, 0, K1);
> build_huge_update_entries(&p, htlb_info.huge_pte, K1);
> build_huge_tlb_write_entry(&p, &l, &r, K0, tlb_random,
> htlb_info.restore_scratch);
This patch fixes the issue, thanks. That said it doesn't look fully
correct. The test should be done the same way as for
build_fast_tlb_refill_handler. For example the fast handler is not
called on a 32-bit machine with bbit instructions, so it would need
to reload K0.
Now I do wonder if we should add yet another test there, or simply move
and duplicate these 3 or 4 lines in the fast and non-fast branches. At
least it will improve readability.
Aurelien
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] MIPS: tlbex: fix a missing statement for HUGETLB
2014-08-02 21:35 ` Aurelien Jarno
@ 2014-08-04 10:08 ` James Hogan
2014-08-04 13:05 ` Aurelien Jarno
0 siblings, 1 reply; 16+ messages in thread
From: James Hogan @ 2014-08-04 10:08 UTC (permalink / raw)
To: Aurelien Jarno, David Daney
Cc: Huacai Chen, James Hogan, Linux MIPS Mailing List, Ralf Baechle,
John Crispin, Steven J. Hill, Fuxin Zhang, Zhangjin Wu,
Binbin Zhou, stable
Hi Aurelien,
On 02/08/14 22:35, Aurelien Jarno wrote:
> On Thu, Jul 31, 2014 at 10:33:55AM -0700, David Daney wrote:
>> diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
>> index f99ec587..341add1 100644
>> --- a/arch/mips/mm/tlbex.c
>> +++ b/arch/mips/mm/tlbex.c
>> @@ -1299,6 +1299,8 @@ static void build_r4000_tlb_refill_handler(void)
>> }
>> #ifdef CONFIG_MIPS_HUGE_TLB_SUPPORT
>> uasm_l_tlb_huge_update(&l, p);
>> + if (!use_bbit_insns())
>> + UASM_i_LW(&p, K0, 0, K1);
>> build_huge_update_entries(&p, htlb_info.huge_pte, K1);
>> build_huge_tlb_write_entry(&p, &l, &r, K0, tlb_random,
>> htlb_info.restore_scratch);
>
> This patch fixes the issue, thanks. That said it doesn't look fully
> correct. The test should be done the same way as for
> build_fast_tlb_refill_handler. For example the fast handler is not
> called on a 32-bit machine with bbit instructions, so it would need
> to reload K0.
In the non fast case build_is_huge_pte() will still use bbit1 if
available after restoring K0, and I don't think the bbit1 would clobber
K0 when the branch is taken, so I think the test for !use_bbit_insns()
is correct.
Cheers
James
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] MIPS: tlbex: fix a missing statement for HUGETLB
2014-08-04 10:08 ` James Hogan
@ 2014-08-04 13:05 ` Aurelien Jarno
2014-08-04 13:10 ` James Hogan
0 siblings, 1 reply; 16+ messages in thread
From: Aurelien Jarno @ 2014-08-04 13:05 UTC (permalink / raw)
To: James Hogan
Cc: David Daney, Huacai Chen, James Hogan, Linux MIPS Mailing List,
Ralf Baechle, John Crispin, Steven J. Hill, Fuxin Zhang,
Zhangjin Wu, Binbin Zhou, stable
On Mon, Aug 04, 2014 at 11:08:50AM +0100, James Hogan wrote:
> Hi Aurelien,
>
> On 02/08/14 22:35, Aurelien Jarno wrote:
> > On Thu, Jul 31, 2014 at 10:33:55AM -0700, David Daney wrote:
> >> diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
> >> index f99ec587..341add1 100644
> >> --- a/arch/mips/mm/tlbex.c
> >> +++ b/arch/mips/mm/tlbex.c
> >> @@ -1299,6 +1299,8 @@ static void build_r4000_tlb_refill_handler(void)
> >> }
> >> #ifdef CONFIG_MIPS_HUGE_TLB_SUPPORT
> >> uasm_l_tlb_huge_update(&l, p);
> >> + if (!use_bbit_insns())
> >> + UASM_i_LW(&p, K0, 0, K1);
> >> build_huge_update_entries(&p, htlb_info.huge_pte, K1);
> >> build_huge_tlb_write_entry(&p, &l, &r, K0, tlb_random,
> >> htlb_info.restore_scratch);
> >
> > This patch fixes the issue, thanks. That said it doesn't look fully
> > correct. The test should be done the same way as for
> > build_fast_tlb_refill_handler. For example the fast handler is not
> > called on a 32-bit machine with bbit instructions, so it would need
> > to reload K0.
>
> In the non fast case build_is_huge_pte() will still use bbit1 if
> available after restoring K0, and I don't think the bbit1 would clobber
> K0 when the branch is taken, so I think the test for !use_bbit_insns()
> is correct.
>
Oh you are right! Therefore this second patch is:
Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
Tested-by: Aurelien Jarno <aurelien@aurel32.net>
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] MIPS: tlbex: fix a missing statement for HUGETLB
2014-08-04 13:05 ` Aurelien Jarno
@ 2014-08-04 13:10 ` James Hogan
2014-08-05 7:09 ` Huacai Chen
0 siblings, 1 reply; 16+ messages in thread
From: James Hogan @ 2014-08-04 13:10 UTC (permalink / raw)
To: Aurelien Jarno
Cc: David Daney, Huacai Chen, James Hogan, Linux MIPS Mailing List,
Ralf Baechle, John Crispin, Steven J. Hill, Fuxin Zhang,
Zhangjin Wu, Binbin Zhou, stable
On 04/08/14 14:05, Aurelien Jarno wrote:
> On Mon, Aug 04, 2014 at 11:08:50AM +0100, James Hogan wrote:
>> Hi Aurelien,
>>
>> On 02/08/14 22:35, Aurelien Jarno wrote:
>>> On Thu, Jul 31, 2014 at 10:33:55AM -0700, David Daney wrote:
>>>> diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
>>>> index f99ec587..341add1 100644
>>>> --- a/arch/mips/mm/tlbex.c
>>>> +++ b/arch/mips/mm/tlbex.c
>>>> @@ -1299,6 +1299,8 @@ static void build_r4000_tlb_refill_handler(void)
>>>> }
>>>> #ifdef CONFIG_MIPS_HUGE_TLB_SUPPORT
>>>> uasm_l_tlb_huge_update(&l, p);
>>>> + if (!use_bbit_insns())
>>>> + UASM_i_LW(&p, K0, 0, K1);
>>>> build_huge_update_entries(&p, htlb_info.huge_pte, K1);
>>>> build_huge_tlb_write_entry(&p, &l, &r, K0, tlb_random,
>>>> htlb_info.restore_scratch);
>>>
>>> This patch fixes the issue, thanks. That said it doesn't look fully
>>> correct. The test should be done the same way as for
>>> build_fast_tlb_refill_handler. For example the fast handler is not
>>> called on a 32-bit machine with bbit instructions, so it would need
>>> to reload K0.
>>
>> In the non fast case build_is_huge_pte() will still use bbit1 if
>> available after restoring K0, and I don't think the bbit1 would clobber
>> K0 when the branch is taken, so I think the test for !use_bbit_insns()
>> is correct.
>>
> Oh you are right! Therefore this second patch is:
>
> Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
Likewise:
Reviewed-by: James Hogan <james.hogan@imgtec.com>
Cheers
James
> Tested-by: Aurelien Jarno <aurelien@aurel32.net>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] MIPS: tlbex: fix a missing statement for HUGETLB
2014-08-04 13:10 ` James Hogan
@ 2014-08-05 7:09 ` Huacai Chen
2014-08-06 16:27 ` Ralf Baechle
0 siblings, 1 reply; 16+ messages in thread
From: Huacai Chen @ 2014-08-05 7:09 UTC (permalink / raw)
To: James Hogan
Cc: Aurelien Jarno, David Daney, James Hogan,
Linux MIPS Mailing List, Ralf Baechle, John Crispin,
Steven J. Hill, Fuxin Zhang, Zhangjin Wu, Binbin Zhou, stable
Now what should we do? My original patch is not perfect, but it has
already merged into Ralf's tree (but hasn't merged in Linus's tree).
Let me send Ralf a new version of this patch? Or let David send
another patch on top of my original one?
Huacai
On Mon, Aug 4, 2014 at 9:10 PM, James Hogan <james.hogan@imgtec.com> wrote:
> On 04/08/14 14:05, Aurelien Jarno wrote:
>> On Mon, Aug 04, 2014 at 11:08:50AM +0100, James Hogan wrote:
>>> Hi Aurelien,
>>>
>>> On 02/08/14 22:35, Aurelien Jarno wrote:
>>>> On Thu, Jul 31, 2014 at 10:33:55AM -0700, David Daney wrote:
>>>>> diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
>>>>> index f99ec587..341add1 100644
>>>>> --- a/arch/mips/mm/tlbex.c
>>>>> +++ b/arch/mips/mm/tlbex.c
>>>>> @@ -1299,6 +1299,8 @@ static void build_r4000_tlb_refill_handler(void)
>>>>> }
>>>>> #ifdef CONFIG_MIPS_HUGE_TLB_SUPPORT
>>>>> uasm_l_tlb_huge_update(&l, p);
>>>>> + if (!use_bbit_insns())
>>>>> + UASM_i_LW(&p, K0, 0, K1);
>>>>> build_huge_update_entries(&p, htlb_info.huge_pte, K1);
>>>>> build_huge_tlb_write_entry(&p, &l, &r, K0, tlb_random,
>>>>> htlb_info.restore_scratch);
>>>>
>>>> This patch fixes the issue, thanks. That said it doesn't look fully
>>>> correct. The test should be done the same way as for
>>>> build_fast_tlb_refill_handler. For example the fast handler is not
>>>> called on a 32-bit machine with bbit instructions, so it would need
>>>> to reload K0.
>>>
>>> In the non fast case build_is_huge_pte() will still use bbit1 if
>>> available after restoring K0, and I don't think the bbit1 would clobber
>>> K0 when the branch is taken, so I think the test for !use_bbit_insns()
>>> is correct.
>>>
>> Oh you are right! Therefore this second patch is:
>>
>> Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
>
> Likewise:
>
> Reviewed-by: James Hogan <james.hogan@imgtec.com>
>
> Cheers
> James
>
>> Tested-by: Aurelien Jarno <aurelien@aurel32.net>
>>
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] MIPS: tlbex: fix a missing statement for HUGETLB
2014-08-05 7:09 ` Huacai Chen
@ 2014-08-06 16:27 ` Ralf Baechle
0 siblings, 0 replies; 16+ messages in thread
From: Ralf Baechle @ 2014-08-06 16:27 UTC (permalink / raw)
To: Huacai Chen
Cc: James Hogan, Aurelien Jarno, David Daney, James Hogan,
Linux MIPS Mailing List, John Crispin, Steven J. Hill,
Fuxin Zhang, Zhangjin Wu, Binbin Zhou, stable
On Tue, Aug 05, 2014 at 03:09:04PM +0800, Huacai Chen wrote:
> Now what should we do? My original patch is not perfect, but it has
> already merged into Ralf's tree (but hasn't merged in Linus's tree).
> Let me send Ralf a new version of this patch? Or let David send
> another patch on top of my original one?
I'll create an incremental patch so you don't need to do anything.
Ralf
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2014-08-06 17:10 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-29 6:54 [PATCH] MIPS: tlbex: fix a missing statement for HUGETLB Huacai Chen
2014-07-29 6:54 ` Huacai Chen
2014-07-30 16:37 ` Aurelien Jarno
2014-07-30 21:41 ` James Hogan
2014-07-30 21:44 ` David Daney
2014-07-30 21:44 ` David Daney
2014-07-31 0:48 ` Huacai Chen
2014-07-31 1:13 ` David Daney
2014-07-31 11:54 ` James Hogan
2014-07-31 17:33 ` David Daney
2014-08-02 21:35 ` Aurelien Jarno
2014-08-04 10:08 ` James Hogan
2014-08-04 13:05 ` Aurelien Jarno
2014-08-04 13:10 ` James Hogan
2014-08-05 7:09 ` Huacai Chen
2014-08-06 16:27 ` Ralf Baechle
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.