All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] MIPS: Install final length of TLB refill handler rather than 256 bytes
@ 2019-04-05 16:05 Fredrik Noring
  2019-04-14 21:20 ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 5+ messages in thread
From: Fredrik Noring @ 2019-04-05 16:05 UTC (permalink / raw)
  To: Ralf Baechle, Paul Burton, James Hogan, linux-mips; +Cc: Maciej W. Rozycki

The R5900 TLB refill handler is limited to 128 bytes, corresponding
to 32 instructions.

Installing a 256 byte TLB refill handler for the R5900 at address
0x80000000 overwrites the performance counter handler at address
0x80000080, according to the TX79 manual[1]:

        Table 5-2. Exception Vectors for Level 1 exceptions

             Exceptions      |      Vector Address
                             |   BEV = 0  |   BEV = 1
        ---------------------+------------+-----------
        TLB Refill (EXL = 0) | 0x80000000 | 0xBFC00200
        TLB Refill (EXL = 1) | 0x80000180 | 0xBFC00380
        Interrupt            | 0x80000200 | 0xBFC00400
        Others               | 0x80000180 | 0xBFC00380
        ---------------------+------------+-----------

        Table 5-3. Exception Vectors for Level 2 exceptions

             Exceptions      |      Vector Address
                             |   DEV = 0  |   DEV = 1
        ---------------------+------------+-----------
        Reset, NMI           | 0xBFC00000 | 0xBFC00000
        Performance Counter  | 0x80000080 | 0xBFC00280
        Debug, SIO           | 0x80000100 | 0xBFC00300
        ---------------------+------------+-----------

Reference:

[1] "TX System RISC TX79 Core Architecture" manual, revision 2.0,
    Toshiba Corporation, p. 5-7, https://wiki.qemu.org/File:C790.pdf

Signed-off-by: Fredrik Noring <noring@nocrew.org>
---
Hi MIPS maintainers,

Reading through the TX79 manual I noticed that the installation of
the TLB refill handler seems to overwrite the performance counter
handler. Is there any particular reason to not install the actual
lengths of the handlers, such as memory boundaries or alignments?

I have a separate patch that checks the R5900 handler length limit,
but it depends on R5900 support, which isn't merged (yet).

Fredrik
---
diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
--- a/arch/mips/mm/tlbex.c
+++ b/arch/mips/mm/tlbex.c
@@ -1462,9 +1462,9 @@ static void build_r4000_tlb_refill_handler(void)
 	pr_debug("Wrote TLB refill handler (%u instructions).\n",
 		 final_len);
 
-	memcpy((void *)ebase, final_handler, 0x100);
-	local_flush_icache_range(ebase, ebase + 0x100);
-	dump_handler("r4000_tlb_refill", (u32 *)ebase, (u32 *)(ebase + 0x100));
+	memcpy((void *)ebase, final_handler, 4 * final_len);
+	local_flush_icache_range(ebase, ebase + 4 * final_len);
+	dump_handler("r4000_tlb_refill", (u32 *)ebase, (u32 *)(ebase + 4 * final_len));
 }
 
 static void setup_pw(void)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC] MIPS: Install final length of TLB refill handler rather than 256 bytes
  2019-04-05 16:05 [RFC] MIPS: Install final length of TLB refill handler rather than 256 bytes Fredrik Noring
@ 2019-04-14 21:20 ` Philippe Mathieu-Daudé
  2019-04-15 15:22   ` Fredrik Noring
  0 siblings, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-04-14 21:20 UTC (permalink / raw)
  To: Fredrik Noring, Ralf Baechle, Paul Burton, James Hogan, linux-mips
  Cc: Maciej W. Rozycki

Hi Fredrik,

On 4/5/19 6:05 PM, Fredrik Noring wrote:
> The R5900 TLB refill handler is limited to 128 bytes, corresponding
> to 32 instructions.

There is a comment about the R4000 worst case:

 /* The worst case length of the handler is around 18 instructions for
  * R3000-style TLBs and up to 63 instructions for R4000-style TLBs.
  * Maximum space available is 32 instructions for R3000 and 64
  * instructions for R4000.

So you need to check the handler generated for your cpu doesn't exceed
your 32 instructions.

> Installing a 256 byte TLB refill handler for the R5900 at address
> 0x80000000 overwrites the performance counter handler at address
> 0x80000080, according to the TX79 manual[1]:
> 
>         Table 5-2. Exception Vectors for Level 1 exceptions
> 
>              Exceptions      |      Vector Address
>                              |   BEV = 0  |   BEV = 1
>         ---------------------+------------+-----------
>         TLB Refill (EXL = 0) | 0x80000000 | 0xBFC00200
>         TLB Refill (EXL = 1) | 0x80000180 | 0xBFC00380
>         Interrupt            | 0x80000200 | 0xBFC00400
>         Others               | 0x80000180 | 0xBFC00380
>         ---------------------+------------+-----------
> 
>         Table 5-3. Exception Vectors for Level 2 exceptions
> 
>              Exceptions      |      Vector Address
>                              |   DEV = 0  |   DEV = 1
>         ---------------------+------------+-----------
>         Reset, NMI           | 0xBFC00000 | 0xBFC00000
>         Performance Counter  | 0x80000080 | 0xBFC00280
>         Debug, SIO           | 0x80000100 | 0xBFC00300
>         ---------------------+------------+-----------
> 
> Reference:
> 
> [1] "TX System RISC TX79 Core Architecture" manual, revision 2.0,
>     Toshiba Corporation, p. 5-7, https://wiki.qemu.org/File:C790.pdf
> 
> Signed-off-by: Fredrik Noring <noring@nocrew.org>
> ---
> Hi MIPS maintainers,
> 
> Reading through the TX79 manual I noticed that the installation of
> the TLB refill handler seems to overwrite the performance counter
> handler. Is there any particular reason to not install the actual
> lengths of the handlers, such as memory boundaries or alignments?
> 
> I have a separate patch that checks the R5900 handler length limit,
> but it depends on R5900 support, which isn't merged (yet).
> 
> Fredrik
> ---
> 
> diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
> --- a/arch/mips/mm/tlbex.c
> +++ b/arch/mips/mm/tlbex.c
> @@ -1462,9 +1462,9 @@ static void build_r4000_tlb_refill_handler(void)
>  	pr_debug("Wrote TLB refill handler (%u instructions).\n",
>  		 final_len);
>  

> -	memcpy((void *)ebase, final_handler, 0x100);
> -	local_flush_icache_range(ebase, ebase + 0x100);
> -	dump_handler("r4000_tlb_refill", (u32 *)ebase, (u32 *)(ebase + 0x100));
> +	memcpy((void *)ebase, final_handler, 4 * final_len);
> +	local_flush_icache_range(ebase, ebase + 4 * final_len);
> +	dump_handler("r4000_tlb_refill", (u32 *)ebase, (u32 *)(ebase + 4 * final_len));

Maybe you could modify the logic few lines earlier that check and
panic("TLB refill handler space exceeded") and add a case for your cpu
type. There you could set a handler_max_size = 0x80, 0x100 else.

Take my comment as RFC too, I'm just wondering :)

Regards,

Phil.

>  }
>  
>  static void setup_pw(void)
> 
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC] MIPS: Install final length of TLB refill handler rather than 256 bytes
  2019-04-14 21:20 ` Philippe Mathieu-Daudé
@ 2019-04-15 15:22   ` Fredrik Noring
  2019-04-15 17:17     ` Paul Burton
  0 siblings, 1 reply; 5+ messages in thread
From: Fredrik Noring @ 2019-04-15 15:22 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Ralf Baechle, Paul Burton, James Hogan, linux-mips, Maciej W. Rozycki

Hi Phil,

> There is a comment about the R4000 worst case:
> 
>  /* The worst case length of the handler is around 18 instructions for
>   * R3000-style TLBs and up to 63 instructions for R4000-style TLBs.
>   * Maximum space available is 32 instructions for R3000 and 64
>   * instructions for R4000.
> 
> So you need to check the handler generated for your cpu doesn't exceed
> your 32 instructions.

Do you mean like this:

> On 4/5/19 6:05 PM, Fredrik Noring wrote:
> > I have a separate patch that checks the R5900 handler length limit,
> > but it depends on R5900 support, which isn't merged (yet).

I have now attached this separate patch for reference, see below.

> Maybe you could modify the logic few lines earlier that check and
> panic("TLB refill handler space exceeded") and add a case for your cpu
> type. There you could set a handler_max_size = 0x80, 0x100 else.
> 
> Take my comment as RFC too, I'm just wondering :)

To check the length we would need to define CPU_R5900 first. :)

Are any MIPS kernel maintainers happy to review an initial R5900 patch
submission?

[ The patch in the original RFC is generic and it does not depend on
the availability of CPU_R5900, although it avoids clobbering the R5900
performance counter handler. ]

Fredrik

--- a/arch/mips/mm/tlbex.c
+++ b/arch/mips/mm/tlbex.c
@@ -1399,6 +1399,10 @@ static void build_r4000_tlb_refill_handler(void)
 	 * unused.
 	 */
 	switch (boot_cpu_type()) {
+	case CPU_R5900:
+		if ((p - tlb_handler) > 32)
+			panic("TLB refill handler space exceeded");
+		/* Fallthrough */
 	default:
 		if (sizeof(long) == 4) {
 	case CPU_LOONGSON2:

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC] MIPS: Install final length of TLB refill handler rather than 256 bytes
  2019-04-15 15:22   ` Fredrik Noring
@ 2019-04-15 17:17     ` Paul Burton
  2019-04-22 17:34       ` Fredrik Noring
  0 siblings, 1 reply; 5+ messages in thread
From: Paul Burton @ 2019-04-15 17:17 UTC (permalink / raw)
  To: Fredrik Noring
  Cc: Philippe Mathieu-Daudé,
	Ralf Baechle, James Hogan, linux-mips, Maciej W. Rozycki

Hi Fredrik,

On Mon, Apr 15, 2019 at 05:22:52PM +0200, Fredrik Noring wrote:
> > Maybe you could modify the logic few lines earlier that check and
> > panic("TLB refill handler space exceeded") and add a case for your cpu
> > type. There you could set a handler_max_size = 0x80, 0x100 else.
> > 
> > Take my comment as RFC too, I'm just wondering :)
> 
> To check the length we would need to define CPU_R5900 first. :)
> 
> Are any MIPS kernel maintainers happy to review an initial R5900 patch
> submission?

Yes, please submit it if you think it's ready :)

> [ The patch in the original RFC is generic and it does not depend on
> the availability of CPU_R5900, although it avoids clobbering the R5900
> performance counter handler. ]

That's true, and I don't have any real problem with it if you want to
submit it without the RFC tag. The change does only really benefit the
R5900 though so my preference would be that you include the patch as
part of your series adding R5900 support.

Thanks,
    Paul

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC] MIPS: Install final length of TLB refill handler rather than 256 bytes
  2019-04-15 17:17     ` Paul Burton
@ 2019-04-22 17:34       ` Fredrik Noring
  0 siblings, 0 replies; 5+ messages in thread
From: Fredrik Noring @ 2019-04-22 17:34 UTC (permalink / raw)
  To: Paul Burton
  Cc: Philippe Mathieu-Daudé,
	Ralf Baechle, James Hogan, linux-mips, Maciej W. Rozycki,
	Jürgen Urban

Hi Paul,

> > Are any MIPS kernel maintainers happy to review an initial R5900 patch
> > submission?
> 
> Yes, please submit it if you think it's ready :)

Great! I need a few weeks to prepare the patches.

> That's true, and I don't have any real problem with it if you want to
> submit it without the RFC tag. The change does only really benefit the
> R5900 though so my preference would be that you include the patch as
> part of your series adding R5900 support.

Agreed!

Fredrik

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-04-22 17:34 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-05 16:05 [RFC] MIPS: Install final length of TLB refill handler rather than 256 bytes Fredrik Noring
2019-04-14 21:20 ` Philippe Mathieu-Daudé
2019-04-15 15:22   ` Fredrik Noring
2019-04-15 17:17     ` Paul Burton
2019-04-22 17:34       ` Fredrik Noring

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.