From: "Maciej W. Rozycki" <macro@imgtec.com> To: Fredrik Noring <noring@nocrew.org> Cc: <linux-mips@linux-mips.org> Subject: Re: [PATCH v2] MIPS: Add basic R5900 support Date: Thu, 14 Sep 2017 14:50:07 +0100 [thread overview] Message-ID: <alpine.DEB.2.00.1709141423180.16752@tp.orcam.me.uk> (raw) In-Reply-To: <20170911151737.GA2265@localhost.localdomain> Hi Fredrik, > This sounds like a good plan. I did have a few comments and questions on the > rest of the code in > > https://www.linux-mips.org/archives/linux-mips/2017-08/msg00570.html I'll go through and try to address those questions one by one separately. > Can this be improved? CONFIG_R5900_128BIT_SUPPORT is configurable but the 32 > bit programs I've tested become unstable unless it's set, so something isn't > quite working without it (which may or may not be related to the registers, > since CONFIG_R5900_128BIT_SUPPORT activates some other changes as well). For the initial R5900 support I think there are two options here, depending on what hardware supports: 1. If (for binary compatibility reasons) 128-bit GPR support can somehow be disabled in hardware, by flipping a CP0 register bit or suchlike, then I suggest doing that in the first stage. 2. Otherwise I think that the context initialisation/switch code has to be adjusted such that the upper GPR halves are set to a known state, either zeroed or sign-extended from bit #63 (or #31 really, given the initial 32-bit port only) according to hardware requirements, so as to make execution stable and prevent data from leaking between contexts. Later on proper 128-bit support can be added, though for that to make sense you need to have compiler support too, which AFAICT is currently missing. Myself I'd rather defer commenting on that further support until we get to it, although of course someone else might be willing to sketch an idea. > At what stage in the patch series would it be appropriate to introduce > support for quadword registers, and in what form? Well, I think we need to stabilise base 32-bit and then 64-bit support first. So that'll be a separate patch or patch set to consider at that point. > The rest of the notes comment on the new SYNC.P instruction, MFC0/MTC0, > short loop crashes in the memcpy/strlen family of functions, etc. Several of > these changes and workarounds are required for a stable system and would > need to be introduced in some form. The exception handler workarounds should be easy to implement as we generate machine code for them at run-time, so inserting a pair of NOPs should be straightforward while not affecting any other target. As a solution addressing a grave hardware erratum this obviously has to be included with your initial patch set, as a separate change because it's self-contained. Any other workarounds are handled via <asm/war.h>. Those which are needed for correct initial 32-bit operation need to go with the initial patch set as well, one change per issue. If SYNC acts as a hazard barrier for MFC0/MTC0, etc., then it can be substituted for EHB/JR.HB where applicable. We have infrastructure for that in <asm/hazards.h> already, so you just need to hook in. You may have to expand that header of course if in the R5900 there are hazards not already covered by EHB/JR.HB for other processors. These will need to go with the initial patch set too. And last but not least I suggest to structure your initial patch set such that the commit containing Kconfig changes to enable R5900/PS2 comes last, so that once that last patch goes in you can build a kernel that boots and correctly works on actual hardware. Maciej
WARNING: multiple messages have this Message-ID (diff)
From: "Maciej W. Rozycki" <macro@imgtec.com> To: Fredrik Noring <noring@nocrew.org> Cc: linux-mips@linux-mips.org Subject: Re: [PATCH v2] MIPS: Add basic R5900 support Date: Thu, 14 Sep 2017 14:50:07 +0100 [thread overview] Message-ID: <alpine.DEB.2.00.1709141423180.16752@tp.orcam.me.uk> (raw) Message-ID: <20170914135007.vMO4vF3sxDDX09ZlJgJeWgrmcNVPy0iTMvTQ9mcZcjM@z> (raw) In-Reply-To: <20170911151737.GA2265@localhost.localdomain> Hi Fredrik, > This sounds like a good plan. I did have a few comments and questions on the > rest of the code in > > https://www.linux-mips.org/archives/linux-mips/2017-08/msg00570.html I'll go through and try to address those questions one by one separately. > Can this be improved? CONFIG_R5900_128BIT_SUPPORT is configurable but the 32 > bit programs I've tested become unstable unless it's set, so something isn't > quite working without it (which may or may not be related to the registers, > since CONFIG_R5900_128BIT_SUPPORT activates some other changes as well). For the initial R5900 support I think there are two options here, depending on what hardware supports: 1. If (for binary compatibility reasons) 128-bit GPR support can somehow be disabled in hardware, by flipping a CP0 register bit or suchlike, then I suggest doing that in the first stage. 2. Otherwise I think that the context initialisation/switch code has to be adjusted such that the upper GPR halves are set to a known state, either zeroed or sign-extended from bit #63 (or #31 really, given the initial 32-bit port only) according to hardware requirements, so as to make execution stable and prevent data from leaking between contexts. Later on proper 128-bit support can be added, though for that to make sense you need to have compiler support too, which AFAICT is currently missing. Myself I'd rather defer commenting on that further support until we get to it, although of course someone else might be willing to sketch an idea. > At what stage in the patch series would it be appropriate to introduce > support for quadword registers, and in what form? Well, I think we need to stabilise base 32-bit and then 64-bit support first. So that'll be a separate patch or patch set to consider at that point. > The rest of the notes comment on the new SYNC.P instruction, MFC0/MTC0, > short loop crashes in the memcpy/strlen family of functions, etc. Several of > these changes and workarounds are required for a stable system and would > need to be introduced in some form. The exception handler workarounds should be easy to implement as we generate machine code for them at run-time, so inserting a pair of NOPs should be straightforward while not affecting any other target. As a solution addressing a grave hardware erratum this obviously has to be included with your initial patch set, as a separate change because it's self-contained. Any other workarounds are handled via <asm/war.h>. Those which are needed for correct initial 32-bit operation need to go with the initial patch set as well, one change per issue. If SYNC acts as a hazard barrier for MFC0/MTC0, etc., then it can be substituted for EHB/JR.HB where applicable. We have infrastructure for that in <asm/hazards.h> already, so you just need to hook in. You may have to expand that header of course if in the R5900 there are hazards not already covered by EHB/JR.HB for other processors. These will need to go with the initial patch set too. And last but not least I suggest to structure your initial patch set such that the commit containing Kconfig changes to enable R5900/PS2 comes last, so that once that last patch goes in you can build a kernel that boots and correctly works on actual hardware. Maciej
next prev parent reply other threads:[~2017-09-14 13:50 UTC|newest] Thread overview: 117+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-08-27 13:23 [PATCH] MIPS: Add basic R5900 support Fredrik Noring 2017-08-28 13:53 ` Ralf Baechle 2017-08-28 17:11 ` Maciej W. Rozycki 2017-08-29 17:33 ` Fredrik Noring 2017-08-29 17:24 ` Maciej W. Rozycki 2017-08-29 17:24 ` Maciej W. Rozycki 2017-08-30 13:23 ` Fredrik Noring 2017-08-31 15:11 ` Maciej W. Rozycki 2017-08-31 15:11 ` Maciej W. Rozycki 2017-09-02 10:28 ` Fredrik Noring 2017-09-09 10:13 ` Maciej W. Rozycki 2017-09-09 10:13 ` Maciej W. Rozycki 2017-09-11 5:21 ` Maciej W. Rozycki 2017-09-11 5:21 ` Maciej W. Rozycki 2017-09-12 17:59 ` Fredrik Noring 2017-09-15 11:12 ` Maciej W. Rozycki 2017-09-15 11:12 ` Maciej W. Rozycki 2017-09-15 13:19 ` Fredrik Noring 2017-09-15 18:28 ` Maciej W. Rozycki 2017-09-15 18:28 ` Maciej W. Rozycki 2017-09-02 14:10 ` [PATCH v2] " Fredrik Noring 2017-09-11 5:18 ` Maciej W. Rozycki 2017-09-11 5:18 ` Maciej W. Rozycki 2017-09-11 15:17 ` Fredrik Noring 2017-09-14 13:50 ` Maciej W. Rozycki [this message] 2017-09-14 13:50 ` Maciej W. Rozycki 2017-09-16 13:34 ` Fredrik Noring 2017-09-18 17:05 ` Maciej W. Rozycki 2017-09-18 17:05 ` Maciej W. Rozycki 2017-09-18 19:24 ` Fredrik Noring 2017-09-19 12:44 ` Maciej W. Rozycki 2017-09-19 12:44 ` Maciej W. Rozycki 2017-09-20 14:54 ` Fredrik Noring 2017-09-26 11:50 ` Maciej W. Rozycki 2017-09-26 11:50 ` Maciej W. Rozycki 2017-09-27 17:21 ` Fredrik Noring 2017-09-28 12:13 ` Maciej W. Rozycki 2017-09-28 12:13 ` Maciej W. Rozycki 2017-09-30 6:56 ` Fredrik Noring 2017-10-02 9:05 ` Maciej W. Rozycki 2017-10-02 9:05 ` Maciej W. Rozycki 2017-10-02 16:33 ` Fredrik Noring 2017-10-29 17:20 ` Fredrik Noring 2017-11-10 23:34 ` Maciej W. Rozycki 2017-11-10 23:34 ` Maciej W. Rozycki 2017-11-11 16:04 ` Fredrik Noring 2018-01-29 20:27 ` Fredrik Noring 2018-01-31 23:01 ` Maciej W. Rozycki 2018-02-11 7:29 ` [RFC] MIPS: R5900: Workaround for the short loop bug Fredrik Noring 2018-02-12 9:25 ` Maciej W. Rozycki 2018-02-12 15:22 ` Fredrik Noring 2018-02-11 7:46 ` [RFC] MIPS: R5900: Use SYNC.L for data cache and SYNC.P for instruction cache Fredrik Noring 2018-02-11 7:56 ` [RFC] MIPS: R5900: Workaround exception NOP execution bug (FLX05) Fredrik Noring 2018-02-12 9:28 ` Maciej W. Rozycki 2018-02-15 19:15 ` [RFC v2] " Fredrik Noring 2018-02-15 20:49 ` Maciej W. Rozycki 2018-02-17 11:16 ` Fredrik Noring 2018-02-17 11:57 ` Maciej W. Rozycki 2018-02-17 13:38 ` Fredrik Noring 2018-02-17 15:03 ` Maciej W. Rozycki 2018-02-17 20:04 ` Fredrik Noring 2018-02-20 14:09 ` Maciej W. Rozycki 2018-02-22 17:04 ` Fredrik Noring 2018-02-18 8:47 ` Fredrik Noring 2018-02-20 14:41 ` Maciej W. Rozycki 2018-02-22 17:27 ` Fredrik Noring 2018-02-11 8:01 ` [RFC] MIPS: R5900: Workaround for CACHE instruction near branch delay slot Fredrik Noring 2018-02-11 11:16 ` Aw: " "Jürgen Urban" 2018-02-11 8:09 ` [RFC] MIPS: R5900: The ERET instruction has issues with delay slot and CACHE Fredrik Noring 2018-02-11 11:07 ` Aw: " "Jürgen Urban" 2018-02-11 8:29 ` [RFC] MIPS: R5900: Use mandatory SYNC.L in exception handlers Fredrik Noring 2018-02-11 10:33 ` Aw: " "Jürgen Urban" 2018-02-12 9:22 ` Maciej W. Rozycki 2018-02-12 9:22 ` Maciej W. Rozycki 2018-02-18 10:30 ` Fredrik Noring 2018-02-17 14:43 ` [RFC] MIPS: R5900: Workaround for saving and restoring FPU registers Fredrik Noring 2018-02-17 15:18 ` Maciej W. Rozycki 2018-02-17 17:47 ` Fredrik Noring 2018-02-17 19:33 ` Maciej W. Rozycki 2018-02-18 9:26 ` [RFC] MIPS: R5900: Workaround where MSB must be 0 for the instruction cache Fredrik Noring 2018-02-18 11:08 ` [RFC] MIPS: R5900: Add mandatory SYNC.P to all M[FT]C0 instructions Fredrik Noring 2018-03-03 12:26 ` [RFC] MIPS: PS2: Interrupt request (IRQ) support Fredrik Noring 2018-03-03 13:09 ` Maciej W. Rozycki 2018-03-03 14:14 ` Fredrik Noring 2018-04-09 15:51 ` Fredrik Noring 2018-03-18 10:45 ` Fredrik Noring 2018-03-19 19:15 ` Thomas Gleixner 2018-06-18 18:52 ` [RFC v2] " Fredrik Noring 2017-10-30 17:55 ` [PATCH v2] MIPS: Add basic R5900 support Fredrik Noring 2017-11-24 10:26 ` Maciej W. Rozycki 2017-11-24 10:26 ` Maciej W. Rozycki 2017-11-24 10:39 ` Maciej W. Rozycki 2017-11-24 10:39 ` Maciej W. Rozycki 2017-09-20 14:07 ` Fredrik Noring 2017-09-21 21:07 ` Maciej W. Rozycki 2017-09-21 21:07 ` Maciej W. Rozycki 2017-09-22 16:37 ` Fredrik Noring 2017-09-22 16:37 ` Fredrik Noring 2017-09-29 23:55 ` Maciej W. Rozycki 2017-09-29 23:55 ` Maciej W. Rozycki 2017-09-30 18:26 ` Fredrik Noring 2017-10-02 9:11 ` Maciej W. Rozycki 2017-10-02 9:11 ` Maciej W. Rozycki 2017-10-03 19:49 ` Fredrik Noring 2017-10-05 19:04 ` Fredrik Noring 2017-10-06 20:28 ` Fredrik Noring 2017-10-15 16:39 ` Fredrik Noring 2017-10-17 12:23 ` Maciej W. Rozycki 2017-10-17 12:23 ` Maciej W. Rozycki 2017-10-21 18:00 ` Fredrik Noring 2017-10-23 16:10 ` Maciej W. Rozycki 2017-10-23 16:10 ` Maciej W. Rozycki 2017-09-21 18:11 ` Paul Burton 2017-09-21 18:11 ` Paul Burton 2017-09-21 19:48 ` Maciej W. Rozycki 2017-09-21 19:48 ` Maciej W. Rozycki 2017-10-29 18:42 ` Fredrik Noring
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=alpine.DEB.2.00.1709141423180.16752@tp.orcam.me.uk \ --to=macro@imgtec.com \ --cc=linux-mips@linux-mips.org \ --cc=noring@nocrew.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.