| Message ID | 20190405160531.GF33393@sx9 (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | [RFC] MIPS: Install final length of TLB refill handler rather than 256 bytes | expand |
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) > >
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:
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
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
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)
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 ---