| Message ID | 20240404103254.1752834-1-cleger@rivosinc.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add parsing for Zimop ISA extension | expand |
On Thu, Apr 04, 2024 at 12:32:46PM +0200, Clément Léger wrote: > The Zimop ISA extension was ratified recently. This series adds support > for parsing it from riscv,isa, hwprobe export and kvm support for > Guest/VM. I'm not sure we need this. Zimop by itself isn't useful, so I don't know if we need to advertise it at all. When an extension comes along that redefines some MOPs, then we'll advertise that extension, but the fact Zimop is used for that extension is really just an implementation detail. Thanks, drew > > Clément Léger (5): > dt-bindings: riscv: add Zimop ISA extension description > riscv: add ISA extension parsing for Zimop > riscv: hwprobe: export Zimop ISA extension > RISC-V: KVM: Allow Zimop extension for Guest/VM > KVM: riscv: selftests: Add Zimop extension to get-reg-list test > > Documentation/arch/riscv/hwprobe.rst | 4 ++++ > Documentation/devicetree/bindings/riscv/extensions.yaml | 5 +++++ > arch/riscv/include/asm/hwcap.h | 1 + > arch/riscv/include/uapi/asm/hwprobe.h | 1 + > arch/riscv/include/uapi/asm/kvm.h | 1 + > arch/riscv/kernel/cpufeature.c | 1 + > arch/riscv/kernel/sys_hwprobe.c | 1 + > arch/riscv/kvm/vcpu_onereg.c | 2 ++ > tools/testing/selftests/kvm/riscv/get-reg-list.c | 4 ++++ > 9 files changed, 20 insertions(+) > > -- > 2.43.0 > > > -- > kvm-riscv mailing list > kvm-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/kvm-riscv
On Fri, Apr 5, 2024 at 8:26 AM Andrew Jones <ajones@ventanamicro.com> wrote: > > On Thu, Apr 04, 2024 at 12:32:46PM +0200, Clément Léger wrote: > > The Zimop ISA extension was ratified recently. This series adds support > > for parsing it from riscv,isa, hwprobe export and kvm support for > > Guest/VM. > > I'm not sure we need this. Zimop by itself isn't useful, so I don't know > if we need to advertise it at all. When an extension comes along that > redefines some MOPs, then we'll advertise that extension, but the fact > Zimop is used for that extension is really just an implementation detail. Only situation I see this can be useful is this:-- An implementer, implemented Zimops in CPU solely for the purpose that they can run mainline distro & packages on their hardware and don't want to leverage any feature which are built on top of Zimop. As an example zicfilp and zicfiss are dependent on zimops. glibc can do following 1) check elf header if binary was compiled with zicfiss and zicfilp, if yes goto step 2, else goto step 6. 2) check if zicfiss/zicfilp is available in hw via hwprobe, if yes goto step 5. else goto step 3 3) check if zimop is available via hwprobe, if yes goto step 6, else goto step 4 4) This binary won't be able to run successfully on this platform, issue exit syscall. <-- termination 5) issue prctl to enable shadow stack and landing pad for current task <-- enable feature 6) let the binary run <-- let the binary run because no harm can be done
On 05/04/2024 19:33, Deepak Gupta wrote: > On Fri, Apr 5, 2024 at 8:26 AM Andrew Jones <ajones@ventanamicro.com> wrote: >> >> On Thu, Apr 04, 2024 at 12:32:46PM +0200, Clément Léger wrote: >>> The Zimop ISA extension was ratified recently. This series adds support >>> for parsing it from riscv,isa, hwprobe export and kvm support for >>> Guest/VM. >> >> I'm not sure we need this. Zimop by itself isn't useful, so I don't know >> if we need to advertise it at all. When an extension comes along that >> redefines some MOPs, then we'll advertise that extension, but the fact >> Zimop is used for that extension is really just an implementation detail. > > Only situation I see this can be useful is this:-- > > An implementer, implemented Zimops in CPU solely for the purpose that they can > run mainline distro & packages on their hardware and don't want to leverage any > feature which are built on top of Zimop. Yes, the rationale was that some binaries using extensions that overload MOPs could still be run. With Zimop exposed, the loader could determine if the binary can be executed without potentially crashing. We could also let the program run anyway but the execution could potentially crash unexpectedly, which IMHO is not really good for the user experience nor for debugging. I already think that the segfaults which happens when executing binaries that need some missing extension are not so easy to debug, so better add more guards. > > As an example zicfilp and zicfiss are dependent on zimops. glibc can > do following > > 1) check elf header if binary was compiled with zicfiss and zicfilp, > if yes goto step 2, else goto step 6. > 2) check if zicfiss/zicfilp is available in hw via hwprobe, if yes > goto step 5. else goto step 3 > 3) check if zimop is available via hwprobe, if yes goto step 6, else goto step 4 I think you meant step 5 rather than step 6. Clément > 4) This binary won't be able to run successfully on this platform, > issue exit syscall. <-- termination > 5) issue prctl to enable shadow stack and landing pad for current task > <-- enable feature > 6) let the binary run <-- let the binary run because no harm can be done
On Mon, Apr 08, 2024 at 10:01:12AM +0200, Clément Léger wrote: > > > On 05/04/2024 19:33, Deepak Gupta wrote: > > On Fri, Apr 5, 2024 at 8:26 AM Andrew Jones <ajones@ventanamicro.com> wrote: > >> > >> On Thu, Apr 04, 2024 at 12:32:46PM +0200, Clément Léger wrote: > >>> The Zimop ISA extension was ratified recently. This series adds support > >>> for parsing it from riscv,isa, hwprobe export and kvm support for > >>> Guest/VM. > >> > >> I'm not sure we need this. Zimop by itself isn't useful, so I don't know > >> if we need to advertise it at all. When an extension comes along that > >> redefines some MOPs, then we'll advertise that extension, but the fact > >> Zimop is used for that extension is really just an implementation detail. > > > > Only situation I see this can be useful is this:-- > > > > An implementer, implemented Zimops in CPU solely for the purpose that they can > > run mainline distro & packages on their hardware and don't want to leverage any > > feature which are built on top of Zimop. > > Yes, the rationale was that some binaries using extensions that overload > MOPs could still be run. With Zimop exposed, the loader could determine > if the binary can be executed without potentially crashing. We could > also let the program run anyway but the execution could potentially > crash unexpectedly, which IMHO is not really good for the user > experience nor for debugging. I already think that the segfaults which > happens when executing binaries that need some missing extension are not > so easy to debug, so better add more guards. OK. It's only one more extension out of dozens, so I won't complain more, but I was thinking that binaries that use particular extensions would check for those particular extensions (step 2), rather than Zimop. Thanks, drew > > > > > As an example zicfilp and zicfiss are dependent on zimops. glibc can > > do following > > > > 1) check elf header if binary was compiled with zicfiss and zicfilp, > > if yes goto step 2, else goto step 6. > > 2) check if zicfiss/zicfilp is available in hw via hwprobe, if yes > > goto step 5. else goto step 3 > > 3) check if zimop is available via hwprobe, if yes goto step 6, else goto step 4 > > I think you meant step 5 rather than step 6. > > Clément > > > 4) This binary won't be able to run successfully on this platform, > > issue exit syscall. <-- termination > > 5) issue prctl to enable shadow stack and landing pad for current task > > <-- enable feature > > 6) let the binary run <-- let the binary run because no harm can be done
On Thu, Apr 04, 2024 at 12:32:46PM +0200, Clément Léger wrote: > The Zimop ISA extension was ratified recently. This series adds support > for parsing it from riscv,isa, hwprobe export and kvm support for > Guest/VM. > > Clément Léger (5): > dt-bindings: riscv: add Zimop ISA extension description > riscv: add ISA extension parsing for Zimop > riscv: hwprobe: export Zimop ISA extension > RISC-V: KVM: Allow Zimop extension for Guest/VM > KVM: riscv: selftests: Add Zimop extension to get-reg-list test > > Documentation/arch/riscv/hwprobe.rst | 4 ++++ > Documentation/devicetree/bindings/riscv/extensions.yaml | 5 +++++ > arch/riscv/include/asm/hwcap.h | 1 + > arch/riscv/include/uapi/asm/hwprobe.h | 1 + > arch/riscv/include/uapi/asm/kvm.h | 1 + > arch/riscv/kernel/cpufeature.c | 1 + > arch/riscv/kernel/sys_hwprobe.c | 1 + > arch/riscv/kvm/vcpu_onereg.c | 2 ++ > tools/testing/selftests/kvm/riscv/get-reg-list.c | 4 ++++ > 9 files changed, 20 insertions(+) > > -- > 2.43.0 For the series, Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
On 08/04/2024 13:03, Andrew Jones wrote: > On Mon, Apr 08, 2024 at 10:01:12AM +0200, Clément Léger wrote: >> >> >> On 05/04/2024 19:33, Deepak Gupta wrote: >>> On Fri, Apr 5, 2024 at 8:26 AM Andrew Jones <ajones@ventanamicro.com> wrote: >>>> >>>> On Thu, Apr 04, 2024 at 12:32:46PM +0200, Clément Léger wrote: >>>>> The Zimop ISA extension was ratified recently. This series adds support >>>>> for parsing it from riscv,isa, hwprobe export and kvm support for >>>>> Guest/VM. >>>> >>>> I'm not sure we need this. Zimop by itself isn't useful, so I don't know >>>> if we need to advertise it at all. When an extension comes along that >>>> redefines some MOPs, then we'll advertise that extension, but the fact >>>> Zimop is used for that extension is really just an implementation detail. >>> >>> Only situation I see this can be useful is this:-- >>> >>> An implementer, implemented Zimops in CPU solely for the purpose that they can >>> run mainline distro & packages on their hardware and don't want to leverage any >>> feature which are built on top of Zimop. >> >> Yes, the rationale was that some binaries using extensions that overload >> MOPs could still be run. With Zimop exposed, the loader could determine >> if the binary can be executed without potentially crashing. We could >> also let the program run anyway but the execution could potentially >> crash unexpectedly, which IMHO is not really good for the user >> experience nor for debugging. I already think that the segfaults which >> happens when executing binaries that need some missing extension are not >> so easy to debug, so better add more guards. > > OK. It's only one more extension out of dozens, so I won't complain more, No worries, your point *is* valid since I'm not sure yet that the loader will actually do that one day. BTW, are you aware of any effort to make the elf dynamic loader "smarter" and actually check for needed extensions to be present rather than blindly running the elf and potentially catching SIGILL ? Thanks, Clément > but I was thinking that binaries that use particular extensions would > check for those particular extensions (step 2), rather than Zimop. > > Thanks, > drew > >> >>> >>> As an example zicfilp and zicfiss are dependent on zimops. glibc can >>> do following >>> >>> 1) check elf header if binary was compiled with zicfiss and zicfilp, >>> if yes goto step 2, else goto step 6. >>> 2) check if zicfiss/zicfilp is available in hw via hwprobe, if yes >>> goto step 5. else goto step 3 >>> 3) check if zimop is available via hwprobe, if yes goto step 6, else goto step 4 >> >> I think you meant step 5 rather than step 6. >> >> Clément >> >>> 4) This binary won't be able to run successfully on this platform, >>> issue exit syscall. <-- termination >>> 5) issue prctl to enable shadow stack and landing pad for current task >>> <-- enable feature >>> 6) let the binary run <-- let the binary run because no harm can be done
On Mon, Apr 08, 2024 at 01:19:39PM +0200, Clément Léger wrote: > > > On 08/04/2024 13:03, Andrew Jones wrote: > > On Mon, Apr 08, 2024 at 10:01:12AM +0200, Clément Léger wrote: > >> > >> > >> On 05/04/2024 19:33, Deepak Gupta wrote: > >>> On Fri, Apr 5, 2024 at 8:26 AM Andrew Jones <ajones@ventanamicro.com> wrote: > >>>> > >>>> On Thu, Apr 04, 2024 at 12:32:46PM +0200, Clément Léger wrote: > >>>>> The Zimop ISA extension was ratified recently. This series adds support > >>>>> for parsing it from riscv,isa, hwprobe export and kvm support for > >>>>> Guest/VM. > >>>> > >>>> I'm not sure we need this. Zimop by itself isn't useful, so I don't know > >>>> if we need to advertise it at all. When an extension comes along that > >>>> redefines some MOPs, then we'll advertise that extension, but the fact > >>>> Zimop is used for that extension is really just an implementation detail. > >>> > >>> Only situation I see this can be useful is this:-- > >>> > >>> An implementer, implemented Zimops in CPU solely for the purpose that they can > >>> run mainline distro & packages on their hardware and don't want to leverage any > >>> feature which are built on top of Zimop. > >> > >> Yes, the rationale was that some binaries using extensions that overload > >> MOPs could still be run. With Zimop exposed, the loader could determine > >> if the binary can be executed without potentially crashing. We could > >> also let the program run anyway but the execution could potentially > >> crash unexpectedly, which IMHO is not really good for the user > >> experience nor for debugging. I already think that the segfaults which > >> happens when executing binaries that need some missing extension are not > >> so easy to debug, so better add more guards. > > > > OK. It's only one more extension out of dozens, so I won't complain more, > > No worries, your point *is* valid since I'm not sure yet that the loader > will actually do that one day. > > BTW, are you aware of any effort to make the elf dynamic loader > "smarter" and actually check for needed extensions to be present rather > than blindly running the elf and potentially catching SIGILL ? Jeff Law told me a bit about FMV (function multiversioning). I don't know much about this, but, from what he's told me, it sounds like there will be an ifunc resolver which invokes hwprobe to determine which variants are possible/best to use, so it should be possible to avoid SIGILL by always having a basic variant. Thanks, drew > > Thanks, > > Clément > > > but I was thinking that binaries that use particular extensions would > > check for those particular extensions (step 2), rather than Zimop. > > > > Thanks, > > drew > > > >> > >>> > >>> As an example zicfilp and zicfiss are dependent on zimops. glibc can > >>> do following > >>> > >>> 1) check elf header if binary was compiled with zicfiss and zicfilp, > >>> if yes goto step 2, else goto step 6. > >>> 2) check if zicfiss/zicfilp is available in hw via hwprobe, if yes > >>> goto step 5. else goto step 3 > >>> 3) check if zimop is available via hwprobe, if yes goto step 6, else goto step 4 > >> > >> I think you meant step 5 rather than step 6. > >> > >> Clément > >> > >>> 4) This binary won't be able to run successfully on this platform, > >>> issue exit syscall. <-- termination > >>> 5) issue prctl to enable shadow stack and landing pad for current task > >>> <-- enable feature > >>> 6) let the binary run <-- let the binary run because no harm can be done
On Mon, Apr 08, 2024 at 10:01:12AM +0200, Clément Léger wrote: > > >On 05/04/2024 19:33, Deepak Gupta wrote: >> On Fri, Apr 5, 2024 at 8:26 AM Andrew Jones <ajones@ventanamicro.com> wrote: >>> >>> On Thu, Apr 04, 2024 at 12:32:46PM +0200, Clément Léger wrote: >>>> The Zimop ISA extension was ratified recently. This series adds support >>>> for parsing it from riscv,isa, hwprobe export and kvm support for >>>> Guest/VM. >>> >>> I'm not sure we need this. Zimop by itself isn't useful, so I don't know >>> if we need to advertise it at all. When an extension comes along that >>> redefines some MOPs, then we'll advertise that extension, but the fact >>> Zimop is used for that extension is really just an implementation detail. >> >> Only situation I see this can be useful is this:-- >> >> An implementer, implemented Zimops in CPU solely for the purpose that they can >> run mainline distro & packages on their hardware and don't want to leverage any >> feature which are built on top of Zimop. > >Yes, the rationale was that some binaries using extensions that overload >MOPs could still be run. With Zimop exposed, the loader could determine >if the binary can be executed without potentially crashing. We could >also let the program run anyway but the execution could potentially >crash unexpectedly, which IMHO is not really good for the user >experience nor for debugging. I already think that the segfaults which >happens when executing binaries that need some missing extension are not >so easy to debug, so better add more guards. > >> >> As an example zicfilp and zicfiss are dependent on zimops. glibc can >> do following >> >> 1) check elf header if binary was compiled with zicfiss and zicfilp, >> if yes goto step 2, else goto step 6. >> 2) check if zicfiss/zicfilp is available in hw via hwprobe, if yes >> goto step 5. else goto step 3 >> 3) check if zimop is available via hwprobe, if yes goto step 6, else goto step 4 > >I think you meant step 5 rather than step 6. No I did mean step 6 which is let the binary run. Step 5 is "issue the prctl" to light up the feature, this should have been reached via step 2 if feature was available. Going to step 6 from step 3 basically means that underlying hardware only supports zimops and thus this binary is safe to run on this hardware. But no need to issue any prctl to kernel to enable this feature. > >Clément > >> 4) This binary won't be able to run successfully on this platform, >> issue exit syscall. <-- termination >> 5) issue prctl to enable shadow stack and landing pad for current task >> <-- enable feature >> 6) let the binary run <-- let the binary run because no harm can be done