From: "Stefan O'Rear" <sorear@fastmail.com> To: "Heiko Stuebner" <heiko@sntech.de>, "Palmer Dabbelt" <palmer@dabbelt.com> Cc: linux-riscv@lists.infradead.org, samuel@sholland.org, guoren@kernel.org, christoph.muellner@vrull.eu, conor.dooley@microchip.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH RFC 2/2] RISC-V: add T-Head vector errata handling Date: Fri, 23 Jun 2023 23:23:11 -0400 [thread overview] Message-ID: <8f1ba16d-42e3-4e45-9fac-a91b3b0556a8@app.fastmail.com> (raw) In-Reply-To: <1788842.TLkxdtWsSY@diego> On Fri, Jun 23, 2023, at 7:26 PM, Heiko Stübner wrote: > Am Freitag, 23. Juni 2023, 12:22:35 CEST schrieb Heiko Stübner: >> Am Freitag, 23. Juni 2023, 05:06:44 CEST schrieb Stefan O'Rear: >> > On Thu, Jun 22, 2023, at 4:35 PM, Heiko Stübner wrote: >> > > Am Donnerstag, 22. Juni 2023, 20:58:37 CEST schrieb Stefan O'Rear: >> > >> Are you aware of "3.7. Vector Fixed-Point Fields in fcsr" in >> > >> riscv-v-spec-0.7.1.pdf? >> > > >> > > oh wow, thanks a lot for that pointer, now I understand your concern. >> > > >> > > So in vector-0.7.1 fcsr[10:9] mirrors vxrm and fcsr[8] mirrors vxsat. >> > > >> > > >> > > On a positive note, the T-Head cores seem to not implement the full >> > > vector 0.7.1 specification after all, in the documentation I have [0] >> > > fcsr[31:8] are declared as "0" and uppermost bits are [7:5] for the "frm" >> > > field. >> > >> > Given that the pdf you linked does not mention any vector CSRs, I am not >> > confident that it provides a complete and accurate description of vector >> > functionality in other registers for the C906 with vector extension. >> > >> > Assuming that you have access to such a chip, I would be much happier with >> > the proposed "just a comment" approach if our understanding of the behavior >> > were confirmed on hardware (specifically: csr_write(CSR_FCSR, 0x700) should >> > not affect csr_read(CSR_VXRM) or csr_read(CSR_VXSAT)). >> >> For one, you're right that I should definitly try to confirm this on hardware :-) . > > ok, so now I know the documentation is wrong. > > before, vxrm 0x0, vxsat 0x0 > writing the 0x700 to fcsr > after, vxrm 0x3, vxsat 0x1 > > Essentially the link between the CSRs really is there - oh fun. > So we're back at your original concern - sadly. > > I guess I need to figure out how to not have this stuff break > because relying on the fpu parts to handle feels not correct > at first glance. I don't see a conceptual problem here. There are a large number of vector instructions that access the floating point state (frm, fflags, f{0..31}). These instructions require a valid floating point context, sstatus.FS>0, trap if sstatus.FS=0, and set sstatus.FS=3 if they change anything. vfadd, vfsub, vfmul, vfdiv, vfmv, etc, etc in both 0.7.1 and 1.0. 0.7.1 extends the floating point state to include vxrm and vxsat and adds vaaddu, vnclip, vsmul, etc to the list of instructions which access both floating point and vector state. This breaks floating-point emulation (if a core provides integer vectors in hardware, it provides a fcsr register with three writeable bits, which means that accesses to fcsr won't trap and can't be emulated to provide access to the software frm and fflags), which is probably why the behavior was changed in 1.0, but is otherwise internally consistent. You can continue to treat "fpu parts" and "vector parts" as independent as long as you recognize vxrm and vxsat as _exclusively_ owned by the "fpu parts". Access to vxrm and vxsat should be exclusively controlled by sstatus.FS since they are aliases for fields in fcsr, while access to vstart/vtype/vlen should be exclusively controlled by sstatus.VS. It would be good to test this, since it's not actually in the spec (risc-v has a rather systematic underspecification problem), and T-Head's idea of obviously correct behavior may differ from mine. 1.0 puts vxrm and vxsat into the vector state, controlled by sstatus.VS; vsmul now works on the vector state only, so as far as state management is concerned it now acts like vadd and not like vfadd. This is also internally consistent, but vxcsr must be owned by whoever manages sstatus.VS. It's a little bit weird to say "vxrm and vxsat live in different parts of the kernel state depending on V extension version" but it might be less weird to say "fcsr and all its parts lives somewhere, vxcsr and all its parts lives somewhere else, vxcsr doesn't exist in 0.7.1". Is this adequate? -s > > Heiko _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
WARNING: multiple messages have this Message-ID (diff)
From: "Stefan O'Rear" <sorear@fastmail.com> To: "Heiko Stuebner" <heiko@sntech.de>, "Palmer Dabbelt" <palmer@dabbelt.com> Cc: linux-riscv@lists.infradead.org, samuel@sholland.org, guoren@kernel.org, christoph.muellner@vrull.eu, conor.dooley@microchip.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH RFC 2/2] RISC-V: add T-Head vector errata handling Date: Fri, 23 Jun 2023 23:23:11 -0400 [thread overview] Message-ID: <8f1ba16d-42e3-4e45-9fac-a91b3b0556a8@app.fastmail.com> (raw) In-Reply-To: <1788842.TLkxdtWsSY@diego> On Fri, Jun 23, 2023, at 7:26 PM, Heiko Stübner wrote: > Am Freitag, 23. Juni 2023, 12:22:35 CEST schrieb Heiko Stübner: >> Am Freitag, 23. Juni 2023, 05:06:44 CEST schrieb Stefan O'Rear: >> > On Thu, Jun 22, 2023, at 4:35 PM, Heiko Stübner wrote: >> > > Am Donnerstag, 22. Juni 2023, 20:58:37 CEST schrieb Stefan O'Rear: >> > >> Are you aware of "3.7. Vector Fixed-Point Fields in fcsr" in >> > >> riscv-v-spec-0.7.1.pdf? >> > > >> > > oh wow, thanks a lot for that pointer, now I understand your concern. >> > > >> > > So in vector-0.7.1 fcsr[10:9] mirrors vxrm and fcsr[8] mirrors vxsat. >> > > >> > > >> > > On a positive note, the T-Head cores seem to not implement the full >> > > vector 0.7.1 specification after all, in the documentation I have [0] >> > > fcsr[31:8] are declared as "0" and uppermost bits are [7:5] for the "frm" >> > > field. >> > >> > Given that the pdf you linked does not mention any vector CSRs, I am not >> > confident that it provides a complete and accurate description of vector >> > functionality in other registers for the C906 with vector extension. >> > >> > Assuming that you have access to such a chip, I would be much happier with >> > the proposed "just a comment" approach if our understanding of the behavior >> > were confirmed on hardware (specifically: csr_write(CSR_FCSR, 0x700) should >> > not affect csr_read(CSR_VXRM) or csr_read(CSR_VXSAT)). >> >> For one, you're right that I should definitly try to confirm this on hardware :-) . > > ok, so now I know the documentation is wrong. > > before, vxrm 0x0, vxsat 0x0 > writing the 0x700 to fcsr > after, vxrm 0x3, vxsat 0x1 > > Essentially the link between the CSRs really is there - oh fun. > So we're back at your original concern - sadly. > > I guess I need to figure out how to not have this stuff break > because relying on the fpu parts to handle feels not correct > at first glance. I don't see a conceptual problem here. There are a large number of vector instructions that access the floating point state (frm, fflags, f{0..31}). These instructions require a valid floating point context, sstatus.FS>0, trap if sstatus.FS=0, and set sstatus.FS=3 if they change anything. vfadd, vfsub, vfmul, vfdiv, vfmv, etc, etc in both 0.7.1 and 1.0. 0.7.1 extends the floating point state to include vxrm and vxsat and adds vaaddu, vnclip, vsmul, etc to the list of instructions which access both floating point and vector state. This breaks floating-point emulation (if a core provides integer vectors in hardware, it provides a fcsr register with three writeable bits, which means that accesses to fcsr won't trap and can't be emulated to provide access to the software frm and fflags), which is probably why the behavior was changed in 1.0, but is otherwise internally consistent. You can continue to treat "fpu parts" and "vector parts" as independent as long as you recognize vxrm and vxsat as _exclusively_ owned by the "fpu parts". Access to vxrm and vxsat should be exclusively controlled by sstatus.FS since they are aliases for fields in fcsr, while access to vstart/vtype/vlen should be exclusively controlled by sstatus.VS. It would be good to test this, since it's not actually in the spec (risc-v has a rather systematic underspecification problem), and T-Head's idea of obviously correct behavior may differ from mine. 1.0 puts vxrm and vxsat into the vector state, controlled by sstatus.VS; vsmul now works on the vector state only, so as far as state management is concerned it now acts like vadd and not like vfadd. This is also internally consistent, but vxcsr must be owned by whoever manages sstatus.VS. It's a little bit weird to say "vxrm and vxsat live in different parts of the kernel state depending on V extension version" but it might be less weird to say "fcsr and all its parts lives somewhere, vxcsr and all its parts lives somewhere else, vxcsr doesn't exist in 0.7.1". Is this adequate? -s > > Heiko
next prev parent reply other threads:[~2023-06-24 3:24 UTC|newest] Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-02-28 21:54 [PATCH RFC 0/2] RISC-V: T-Head vector handling Heiko Stuebner 2023-02-28 21:54 ` Heiko Stuebner 2023-02-28 21:54 ` [PATCH RFC 1/2] RISC-V: define the elements of the VCSR vector CSR Heiko Stuebner 2023-02-28 21:54 ` Heiko Stuebner 2023-03-01 2:22 ` Guo Ren 2023-03-01 2:22 ` Guo Ren 2023-03-15 18:31 ` Conor Dooley 2023-03-15 18:31 ` Conor Dooley 2023-02-28 21:54 ` [PATCH RFC 2/2] RISC-V: add T-Head vector errata handling Heiko Stuebner 2023-02-28 21:54 ` Heiko Stuebner 2023-03-01 2:12 ` Guo Ren 2023-03-01 2:12 ` Guo Ren 2023-03-15 18:56 ` Conor Dooley 2023-03-15 18:56 ` Conor Dooley 2023-06-13 6:35 ` Stefan O'Rear 2023-06-13 6:35 ` Stefan O'Rear 2023-06-22 17:39 ` Heiko Stübner 2023-06-22 17:39 ` Heiko Stübner 2023-06-22 18:58 ` Stefan O'Rear 2023-06-22 18:58 ` Stefan O'Rear 2023-06-22 20:35 ` Heiko Stübner 2023-06-22 20:35 ` Heiko Stübner 2023-06-23 3:06 ` Stefan O'Rear 2023-06-23 3:06 ` Stefan O'Rear 2023-06-23 10:22 ` Heiko Stübner 2023-06-23 10:22 ` Heiko Stübner 2023-06-23 23:26 ` Heiko Stübner 2023-06-23 23:26 ` Heiko Stübner 2023-06-24 3:23 ` Stefan O'Rear [this message] 2023-06-24 3:23 ` Stefan O'Rear 2023-06-23 9:12 ` Emil Renner Berthing 2023-06-23 9:12 ` Emil Renner Berthing 2023-03-01 2:21 ` [PATCH RFC 0/2] RISC-V: T-Head vector handling Guo Ren 2023-03-01 2:21 ` Guo Ren 2023-03-15 5:29 ` Palmer Dabbelt 2023-03-15 5:29 ` Palmer Dabbelt 2023-03-15 6:31 ` Heiko Stuebner 2023-03-15 6:31 ` Heiko Stuebner 2023-06-12 15:29 ` Palmer Dabbelt 2023-06-12 15:29 ` Palmer Dabbelt 2023-06-12 15:44 ` Heiko Stübner 2023-06-12 15:44 ` Heiko Stübner
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=8f1ba16d-42e3-4e45-9fac-a91b3b0556a8@app.fastmail.com \ --to=sorear@fastmail.com \ --cc=christoph.muellner@vrull.eu \ --cc=conor.dooley@microchip.com \ --cc=guoren@kernel.org \ --cc=heiko@sntech.de \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-riscv@lists.infradead.org \ --cc=palmer@dabbelt.com \ --cc=samuel@sholland.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.