All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tsukasa OI <research_trasio@irq.a4lg.com>
To: Tsukasa OI <research_trasio@irq.a4lg.com>,
	Alistair Francis <alistair23@gmail.com>,
	Frank Chang <frank.chang@sifive.com>
Cc: qemu-riscv@nongnu.org, qemu-devel@nongnu.org
Subject: [PATCH 0/2] target/riscv: ISA string conversion fix and enhancement
Date: Sun, 24 Apr 2022 14:22:34 +0900	[thread overview]
Message-ID: <cover.1650777360.git.research_trasio@irq.a4lg.com> (raw)

Hello,

There is two issues related to RISC-V ISA extension string
I want to be fixed before QEMU 7.1 release.



Issue 1 (workaround in PATCH 1):

Related: <https://lists.gnu.org/archive/html/qemu-devel/2022-03/msg03726.html>

Generating long ISA extension string is definately a good thing to merge.
However, it includes two extensions with possibly invalid order:

-   Zhinx (IEEE754 binary16 arithmetic in GPR)
-   Zhinxmin (subset of Zhinx, conversion only)

This is because:

1.  Z* extensions are ordered with the second character by closely
    related extension category list
    (RISC-V ISA Manual draft: IMAFDQLCBKJTPV)
2.  ... but it doesn't have the character "H" yet

I raised this issue on RISC-V ISA Manual GitHub and being discussed:
<https://github.com/riscv/riscv-isa-manual/issues/837>

Considering software compatibility, "H" is likely placed after "V" (and
"N").  I kept single-letter "H" based on this assumption.

However, Zhinx and Zhinxmin extensions are not that important because
it's incompatible with F and D.  That's why I proposing to remove those
from ISA extension string generation for now.  If "H"-extension ordering
is determined, we can safely add Zhinx* extensions again.

Note that this patch does not remove extensions.  It just disables
putting Zhinx* extensions in a DeviceTree entry ("riscv,isa").

Of course, we can alternatively move Zhinx and Zhinxmin
before "Svinval" but after "Zve64f", assuming "H" comes after "V".
Let me know which might be better.



Issue 2 (fixed in PATCH 2):

Some operating systems does not correctly parse ISA extension string with
version numbers and multi-letter extensions.

On Linux, 5.18 is the first version to implement safe parser.
However, old Linux kernels are still confused by ISA extension strings
(generated by QEMU >= 7.1) containing multi-letter extensions.
Much worse, those multi-letter extensions are enabled by default:

1.  Zba
2.  Zbb
3.  Zbc
4.  Zbs

For instance, existence of "Zbc" can cause problems if we disable
compressed instructions ("C" extension).

As I searched through, I found this kind of issue on following OSes:

-   Linux (kernel version 5.17 or earlier)
-   FreeBSD (at least 14.0-CURRENT)
-   OpenBSD (at least current development version)

I propose a new CPU option "short-isa-string" (default: false), which
disables generating ISA extension string with multi-letter extensions.

Example:
    qemu-system-riscv64 ... \
        -cpu rv64,h=on,svnapot=on,svinval=on,short-isa-string=on \
        ...
    Without "short-isa-string=on", QEMU generates DeviceTree with
    following ISA extension string:
        rv64imafdch_zba_zbb_zbc_zbs_svinval_svnapot
    With it, QEMU generates following ISA extension string:
        rv64imafdch




Tsukasa OI (2):
  target/riscv: Tentatively remove Zhinx* from ISA extension string
  target/riscv: Add short-isa-string option

 target/riscv/cpu.c | 7 ++++---
 target/riscv/cpu.h | 2 ++
 2 files changed, 6 insertions(+), 3 deletions(-)


base-commit: 754f756cc4c6d9d14b7230c62b5bb20f9d655888
-- 
2.32.0



WARNING: multiple messages have this Message-ID (diff)
From: Tsukasa OI <research_trasio@irq.a4lg.com>
To: Tsukasa OI <research_trasio@irq.a4lg.com>,
	Alistair Francis <alistair23@gmail.com>,
	Frank Chang <frank.chang@sifive.com>
Cc: qemu-devel@nongnu.org, qemu-riscv@nongnu.org
Subject: [PATCH 0/2] target/riscv: ISA string conversion fix and enhancement
Date: Sun, 24 Apr 2022 14:22:34 +0900	[thread overview]
Message-ID: <cover.1650777360.git.research_trasio@irq.a4lg.com> (raw)

Hello,

There is two issues related to RISC-V ISA extension string
I want to be fixed before QEMU 7.1 release.



Issue 1 (workaround in PATCH 1):

Related: <https://lists.gnu.org/archive/html/qemu-devel/2022-03/msg03726.html>

Generating long ISA extension string is definately a good thing to merge.
However, it includes two extensions with possibly invalid order:

-   Zhinx (IEEE754 binary16 arithmetic in GPR)
-   Zhinxmin (subset of Zhinx, conversion only)

This is because:

1.  Z* extensions are ordered with the second character by closely
    related extension category list
    (RISC-V ISA Manual draft: IMAFDQLCBKJTPV)
2.  ... but it doesn't have the character "H" yet

I raised this issue on RISC-V ISA Manual GitHub and being discussed:
<https://github.com/riscv/riscv-isa-manual/issues/837>

Considering software compatibility, "H" is likely placed after "V" (and
"N").  I kept single-letter "H" based on this assumption.

However, Zhinx and Zhinxmin extensions are not that important because
it's incompatible with F and D.  That's why I proposing to remove those
from ISA extension string generation for now.  If "H"-extension ordering
is determined, we can safely add Zhinx* extensions again.

Note that this patch does not remove extensions.  It just disables
putting Zhinx* extensions in a DeviceTree entry ("riscv,isa").

Of course, we can alternatively move Zhinx and Zhinxmin
before "Svinval" but after "Zve64f", assuming "H" comes after "V".
Let me know which might be better.



Issue 2 (fixed in PATCH 2):

Some operating systems does not correctly parse ISA extension string with
version numbers and multi-letter extensions.

On Linux, 5.18 is the first version to implement safe parser.
However, old Linux kernels are still confused by ISA extension strings
(generated by QEMU >= 7.1) containing multi-letter extensions.
Much worse, those multi-letter extensions are enabled by default:

1.  Zba
2.  Zbb
3.  Zbc
4.  Zbs

For instance, existence of "Zbc" can cause problems if we disable
compressed instructions ("C" extension).

As I searched through, I found this kind of issue on following OSes:

-   Linux (kernel version 5.17 or earlier)
-   FreeBSD (at least 14.0-CURRENT)
-   OpenBSD (at least current development version)

I propose a new CPU option "short-isa-string" (default: false), which
disables generating ISA extension string with multi-letter extensions.

Example:
    qemu-system-riscv64 ... \
        -cpu rv64,h=on,svnapot=on,svinval=on,short-isa-string=on \
        ...
    Without "short-isa-string=on", QEMU generates DeviceTree with
    following ISA extension string:
        rv64imafdch_zba_zbb_zbc_zbs_svinval_svnapot
    With it, QEMU generates following ISA extension string:
        rv64imafdch




Tsukasa OI (2):
  target/riscv: Tentatively remove Zhinx* from ISA extension string
  target/riscv: Add short-isa-string option

 target/riscv/cpu.c | 7 ++++---
 target/riscv/cpu.h | 2 ++
 2 files changed, 6 insertions(+), 3 deletions(-)


base-commit: 754f756cc4c6d9d14b7230c62b5bb20f9d655888
-- 
2.32.0



             reply	other threads:[~2022-04-24  5:24 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-24  5:22 Tsukasa OI [this message]
2022-04-24  5:22 ` [PATCH 0/2] target/riscv: ISA string conversion fix and enhancement Tsukasa OI
2022-04-24  5:22 ` [PATCH 1/2] target/riscv: Tentatively remove Zhinx* from ISA extension string Tsukasa OI
2022-04-24  5:22   ` Tsukasa OI
2022-04-27 23:58   ` Alistair Francis
2022-04-27 23:58     ` Alistair Francis
2022-04-28  2:39     ` Weiwei Li
2022-04-28  2:39       ` Weiwei Li
2022-05-10 11:25       ` Tsukasa OI
2022-04-24  5:22 ` [PATCH 2/2] target/riscv: Add short-isa-string option Tsukasa OI
2022-04-24  5:22   ` Tsukasa OI
2022-05-09  9:51   ` Alistair Francis
2022-05-10 11:20     ` Tsukasa OI

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=cover.1650777360.git.research_trasio@irq.a4lg.com \
    --to=research_trasio@irq.a4lg.com \
    --cc=alistair23@gmail.com \
    --cc=frank.chang@sifive.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-riscv@nongnu.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: link
Be 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.