target/riscv: add 'max' CPU type

PATCH Superseded Daniel Henrique Barboza 9 participants 36 messages Jul 12, 2023 – Jul 19, 2023 RISC-V

lore

Share & tools

lore

Daniel Henrique Barboza

author

2023-07-12 19:02 UTC #0 lore raw

cover target/riscv: add 'max' CPU type

Hi,

Following the discussions made in [1] I decided to go ahead and implement the 'max' CPU type.

It's a CPU that has (almost) all ratified non-vendor extensions enabled by default. Tooling such as libvirt uses this kind of CPU to do capabilities discovery. It's also used for testing purposes.

To implement this CPU I did some cleanups in the riscv_cpu_extensions[] array. After this series this array contains only ratified extensions. This is a preliminary step for future changes we're planning to do in the CPU modelling in QEMU, including 'profile' support.

Daniel Henrique Barboza (7):
target/riscv/cpu.c: split CPU options from riscv_cpu_extensions[] target/riscv/cpu.c: skip 'bool' check when filtering KVM props target/riscv/cpu.c: split vendor exts from riscv_cpu_extensions[] target/riscv/cpu.c: split non-ratified exts from

    riscv_cpu_extensions[]

target/riscv/cpu.c: add a ADD_CPU_PROPERTIES_ARRAY() macro target/riscv: add 'max' CPU type
avocado, risc-v: add opensbi tests for 'max' CPU

 target/riscv/cpu-qom.h         |   1 +
 target/riscv/cpu.c             | 106 ++++++++++++++++++++++++++-------
 tests/avocado/riscv_opensbi.py |  16 +++++
 3 files changed, 103 insertions(+), 20 deletions(-)

Signature · 1 line

2.41.0

Daniel Henrique Barboza

author

2023-07-12 19:03 UTC #1 lore raw

PATCH 1/7 target/riscv/cpu.c: split CPU options from riscv_cpu_extensions[]

We'll add a new CPU type that will enable a considerable amount of extensions. To make it easier for us we'll do a few cleanups in our existing riscv_cpu_extensions[] array.

Start by splitting all CPU non-boolean options from it. Create a new riscv_cpu_options[] array for them. Add all these properties in riscv_cpu_add_user_properties() as it is already being done today.

No functional changes made.

Signed-off-by Daniel Henrique Barboza <[email protected]>

---
 target/riscv/cpu.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

target/riscv/cpu.c +19 -8

index 9339c0241d..cdf9eeeb6b 100644
@@ -1751,7 +1751,6 @@ static void riscv_cpu_add_misa_properties(Object *cpu_obj)
 
 static Property riscv_cpu_extensions[] = {
     /* Defaults for standard extensions */
-    DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16),
     DEFINE_PROP_BOOL("sscofpmf", RISCVCPU, cfg.ext_sscofpmf, false),
     DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
     DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),

@@ -1767,11 +1766,6 @@ static Property riscv_cpu_extensions[] = {
     DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
     DEFINE_PROP_BOOL("sstc", RISCVCPU, cfg.ext_sstc, true),
 
-    DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
-    DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),
-    DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
-    DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
-
     DEFINE_PROP_BOOL("smstateen", RISCVCPU, cfg.ext_smstateen, false),
     DEFINE_PROP_BOOL("svadu", RISCVCPU, cfg.ext_svadu, true),
     DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),

@@ -1802,9 +1796,7 @@ static Property riscv_cpu_extensions[] = {
     DEFINE_PROP_BOOL("zhinxmin", RISCVCPU, cfg.ext_zhinxmin, false),
 
     DEFINE_PROP_BOOL("zicbom", RISCVCPU, cfg.ext_icbom, true),
-    DEFINE_PROP_UINT16("cbom_blocksize", RISCVCPU, cfg.cbom_blocksize, 64),
     DEFINE_PROP_BOOL("zicboz", RISCVCPU, cfg.ext_icboz, true),
-    DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU, cfg.cboz_blocksize, 64),
 
     DEFINE_PROP_BOOL("zmmul", RISCVCPU, cfg.ext_zmmul, false),

@@ -1848,6 +1840,20 @@ static Property riscv_cpu_extensions[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
+static Property riscv_cpu_options[] = {
+    DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16),
+
+    DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
+    DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),
+
+    DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
+    DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
+
+    DEFINE_PROP_UINT16("cbom_blocksize", RISCVCPU, cfg.cbom_blocksize, 64),
+    DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU, cfg.cboz_blocksize, 64),
+
+    DEFINE_PROP_END_OF_LIST(),
+};
 
 #ifndef CONFIG_USER_ONLY
 static void cpu_set_cfg_unavailable(Object *obj, Visitor *v,

@@ -1916,6 +1922,11 @@ static void riscv_cpu_add_user_properties(Object *obj)
 #endif
         qdev_property_add_static(dev, prop);
     }
+
+    for (prop = riscv_cpu_options; prop && prop->name; prop++) {
+        qdev_property_add_static(dev, prop);
+    }
+
 }
 
 static Property riscv_cpu_properties[] = {

Signature · 1 line

2.41.0

Daniel Henrique Barboza

author

2023-07-12 19:03 UTC #2 lore raw

PATCH 2/7 target/riscv/cpu.c: skip 'bool' check when filtering KVM props

In reply to Daniel Henrique Barboza

After the introduction of riscv_cpu_options[] all properties in riscv_cpu_extensions[] are booleans. This check is now obsolete.

Signed-off-by Daniel Henrique Barboza <[email protected]>

---
 target/riscv/cpu.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

target/riscv/cpu.c +4 ~~-10~~

index cdf9eeeb6b..735e0ed793 100644
@@ -1907,17 +1907,11 @@ static void riscv_cpu_add_user_properties(Object *obj)
              * Set the default to disabled for every extension
              * unknown to KVM and error out if the user attempts
              * to enable any of them.
-             *
-             * We're giving a pass for non-bool properties since they're
-             * not related to the availability of extensions and can be
-             * safely ignored as is.
              */
-            if (prop->info == &qdev_prop_bool) {
-                object_property_add(obj, prop->name, "bool",
-                                    NULL, cpu_set_cfg_unavailable,
-                                    NULL, (void *)prop->name);
-                continue;
-            }
+            object_property_add(obj, prop->name, "bool",
+                                NULL, cpu_set_cfg_unavailable,
+                                NULL, (void *)prop->name);
+            continue;
         }
 #endif
         qdev_property_add_static(dev, prop);

Signature · 1 line

2.41.0

Daniel Henrique Barboza

author

2023-07-12 19:02 UTC #3 lore raw

PATCH 3/7 target/riscv/cpu.c: split vendor exts from riscv_cpu_extensions[]

In reply to Daniel Henrique Barboza

Our goal is to make riscv_cpu_extensions[] hold only ratified, non-vendor extensions.

Create a new riscv_cpu_vendor_exts[] array for them, changing riscv_cpu_add_user_properties() accordingly.

Signed-off-by Daniel Henrique Barboza <[email protected]>

---
 target/riscv/cpu.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

target/riscv/cpu.c +20 ~~-14~~

index 735e0ed793..9bbdc46126 100644
@@ -1808,20 +1808,6 @@ static Property riscv_cpu_extensions[] = {
     DEFINE_PROP_BOOL("zcmp", RISCVCPU, cfg.ext_zcmp, false),
     DEFINE_PROP_BOOL("zcmt", RISCVCPU, cfg.ext_zcmt, false),
 
-    /* Vendor-specific custom extensions */
-    DEFINE_PROP_BOOL("xtheadba", RISCVCPU, cfg.ext_xtheadba, false),
-    DEFINE_PROP_BOOL("xtheadbb", RISCVCPU, cfg.ext_xtheadbb, false),
-    DEFINE_PROP_BOOL("xtheadbs", RISCVCPU, cfg.ext_xtheadbs, false),
-    DEFINE_PROP_BOOL("xtheadcmo", RISCVCPU, cfg.ext_xtheadcmo, false),
-    DEFINE_PROP_BOOL("xtheadcondmov", RISCVCPU, cfg.ext_xtheadcondmov, false),
-    DEFINE_PROP_BOOL("xtheadfmemidx", RISCVCPU, cfg.ext_xtheadfmemidx, false),
-    DEFINE_PROP_BOOL("xtheadfmv", RISCVCPU, cfg.ext_xtheadfmv, false),
-    DEFINE_PROP_BOOL("xtheadmac", RISCVCPU, cfg.ext_xtheadmac, false),
-    DEFINE_PROP_BOOL("xtheadmemidx", RISCVCPU, cfg.ext_xtheadmemidx, false),
-    DEFINE_PROP_BOOL("xtheadmempair", RISCVCPU, cfg.ext_xtheadmempair, false),
-    DEFINE_PROP_BOOL("xtheadsync", RISCVCPU, cfg.ext_xtheadsync, false),
-    DEFINE_PROP_BOOL("xventanacondops", RISCVCPU, cfg.ext_XVentanaCondOps, false),
-
     /* These are experimental so mark with 'x-' */
     DEFINE_PROP_BOOL("x-zicond", RISCVCPU, cfg.ext_zicond, false),

@@ -1840,6 +1826,23 @@ static Property riscv_cpu_extensions[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
+static Property riscv_cpu_vendor_exts[] = {
+    DEFINE_PROP_BOOL("xtheadba", RISCVCPU, cfg.ext_xtheadba, false),
+    DEFINE_PROP_BOOL("xtheadbb", RISCVCPU, cfg.ext_xtheadbb, false),
+    DEFINE_PROP_BOOL("xtheadbs", RISCVCPU, cfg.ext_xtheadbs, false),
+    DEFINE_PROP_BOOL("xtheadcmo", RISCVCPU, cfg.ext_xtheadcmo, false),
+    DEFINE_PROP_BOOL("xtheadcondmov", RISCVCPU, cfg.ext_xtheadcondmov, false),
+    DEFINE_PROP_BOOL("xtheadfmemidx", RISCVCPU, cfg.ext_xtheadfmemidx, false),
+    DEFINE_PROP_BOOL("xtheadfmv", RISCVCPU, cfg.ext_xtheadfmv, false),
+    DEFINE_PROP_BOOL("xtheadmac", RISCVCPU, cfg.ext_xtheadmac, false),
+    DEFINE_PROP_BOOL("xtheadmemidx", RISCVCPU, cfg.ext_xtheadmemidx, false),
+    DEFINE_PROP_BOOL("xtheadmempair", RISCVCPU, cfg.ext_xtheadmempair, false),
+    DEFINE_PROP_BOOL("xtheadsync", RISCVCPU, cfg.ext_xtheadsync, false),
+    DEFINE_PROP_BOOL("xventanacondops", RISCVCPU, cfg.ext_XVentanaCondOps, false),
+
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static Property riscv_cpu_options[] = {
     DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16),

@@ -1921,6 +1924,9 @@ static void riscv_cpu_add_user_properties(Object *obj)
         qdev_property_add_static(dev, prop);
     }
 
+    for (prop = riscv_cpu_vendor_exts; prop && prop->name; prop++) {
+        qdev_property_add_static(dev, prop);
+    }
 }
 
 static Property riscv_cpu_properties[] = {

Signature · 1 line

2.41.0

Daniel Henrique Barboza

author

2023-07-12 19:03 UTC #4 lore raw

PATCH 4/7 target/riscv/cpu.c: split non-ratified exts from riscv_cpu_extensions[]

In reply to Daniel Henrique Barboza

Create a new riscv_cpu_experimental_exts[] to store the non-ratified extensions properties. Once they are ratified we'll move them back to riscv_cpu_extensions[].

Change riscv_cpu_add_user_properties to keep adding them to users.

Signed-off-by Daniel Henrique Barboza <[email protected]>

---
 target/riscv/cpu.c | 38 +++++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 15 deletions(-)

target/riscv/cpu.c +23 ~~-15~~

index 9bbdc46126..c0826b449d 100644
@@ -1808,21 +1808,6 @@ static Property riscv_cpu_extensions[] = {
     DEFINE_PROP_BOOL("zcmp", RISCVCPU, cfg.ext_zcmp, false),
     DEFINE_PROP_BOOL("zcmt", RISCVCPU, cfg.ext_zcmt, false),
 
-    /* These are experimental so mark with 'x-' */
-    DEFINE_PROP_BOOL("x-zicond", RISCVCPU, cfg.ext_zicond, false),
-
-    /* ePMP 0.9.3 */
-    DEFINE_PROP_BOOL("x-epmp", RISCVCPU, cfg.epmp, false),
-    DEFINE_PROP_BOOL("x-smaia", RISCVCPU, cfg.ext_smaia, false),
-    DEFINE_PROP_BOOL("x-ssaia", RISCVCPU, cfg.ext_ssaia, false),
-
-    DEFINE_PROP_BOOL("x-zvfh", RISCVCPU, cfg.ext_zvfh, false),
-    DEFINE_PROP_BOOL("x-zvfhmin", RISCVCPU, cfg.ext_zvfhmin, false),
-
-    DEFINE_PROP_BOOL("x-zfbfmin", RISCVCPU, cfg.ext_zfbfmin, false),
-    DEFINE_PROP_BOOL("x-zvfbfmin", RISCVCPU, cfg.ext_zvfbfmin, false),
-    DEFINE_PROP_BOOL("x-zvfbfwma", RISCVCPU, cfg.ext_zvfbfwma, false),
-
     DEFINE_PROP_END_OF_LIST(),
 };

@@ -1843,6 +1828,25 @@ static Property riscv_cpu_vendor_exts[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
+/* These are experimental so mark with 'x-' */
+static Property riscv_cpu_experimental_exts[] = {
+    DEFINE_PROP_BOOL("x-zicond", RISCVCPU, cfg.ext_zicond, false),
+
+    /* ePMP 0.9.3 */
+    DEFINE_PROP_BOOL("x-epmp", RISCVCPU, cfg.epmp, false),
+    DEFINE_PROP_BOOL("x-smaia", RISCVCPU, cfg.ext_smaia, false),
+    DEFINE_PROP_BOOL("x-ssaia", RISCVCPU, cfg.ext_ssaia, false),
+
+    DEFINE_PROP_BOOL("x-zvfh", RISCVCPU, cfg.ext_zvfh, false),
+    DEFINE_PROP_BOOL("x-zvfhmin", RISCVCPU, cfg.ext_zvfhmin, false),
+
+    DEFINE_PROP_BOOL("x-zfbfmin", RISCVCPU, cfg.ext_zfbfmin, false),
+    DEFINE_PROP_BOOL("x-zvfbfmin", RISCVCPU, cfg.ext_zvfbfmin, false),
+    DEFINE_PROP_BOOL("x-zvfbfwma", RISCVCPU, cfg.ext_zvfbfwma, false),
+
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static Property riscv_cpu_options[] = {
     DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16),

@@ -1927,6 +1931,10 @@ static void riscv_cpu_add_user_properties(Object *obj)
     for (prop = riscv_cpu_vendor_exts; prop && prop->name; prop++) {
         qdev_property_add_static(dev, prop);
     }
+
+    for (prop = riscv_cpu_experimental_exts; prop && prop->name; prop++) {
+        qdev_property_add_static(dev, prop);
+    }
 }
 
 static Property riscv_cpu_properties[] = {

Signature · 1 line

2.41.0

Daniel Henrique Barboza

author

2023-07-12 19:03 UTC #5 lore raw

PATCH 5/7 target/riscv/cpu.c: add a ADD_CPU_PROPERTIES_ARRAY() macro

In reply to Daniel Henrique Barboza

The code inside riscv_cpu_add_user_properties() became quite repetitive after recent changes. Add a macro to hide the repetition away.

Signed-off-by Daniel Henrique Barboza <[email protected]>

---
 target/riscv/cpu.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

target/riscv/cpu.c +8 ~~-11~~

index c0826b449d..b61465c8c4 100644
@@ -1881,6 +1881,11 @@ static void cpu_set_cfg_unavailable(Object *obj, Visitor *v,
 }
 #endif
 
+#define ADD_CPU_PROPERTIES_ARRAY(_dev, _array) \
+    for (prop = _array; prop && prop->name; prop++) { \
+        qdev_property_add_static(_dev, prop); \
+    } \
+
 /*
  * Add CPU properties with user-facing flags.
  *

@@ -1924,17 +1929,9 @@ static void riscv_cpu_add_user_properties(Object *obj)
         qdev_property_add_static(dev, prop);
     }
 
-    for (prop = riscv_cpu_options; prop && prop->name; prop++) {
-        qdev_property_add_static(dev, prop);
-    }
-
-    for (prop = riscv_cpu_vendor_exts; prop && prop->name; prop++) {
-        qdev_property_add_static(dev, prop);
-    }
-
-    for (prop = riscv_cpu_experimental_exts; prop && prop->name; prop++) {
-        qdev_property_add_static(dev, prop);
-    }
+    ADD_CPU_PROPERTIES_ARRAY(dev, riscv_cpu_options);
+    ADD_CPU_PROPERTIES_ARRAY(dev, riscv_cpu_vendor_exts);
+    ADD_CPU_PROPERTIES_ARRAY(dev, riscv_cpu_experimental_exts);
 }
 
 static Property riscv_cpu_properties[] = {

Signature · 1 line

2.41.0

Daniel Henrique Barboza

author

2023-07-12 19:03 UTC #6 lore raw

PATCH 6/7 target/riscv: add 'max' CPU type

In reply to Daniel Henrique Barboza

The 'max' CPU type is used by tooling to determine what's the most capable CPU a current QEMU version implements. Other archs such as ARM implements this type. Let's add it to RISC-V.

What we consider "most capable CPU" in this context are related to ratified, non-vendor extensions. This means that we want the 'max' CPU to enable all (possible) ratified extensions by default. The reasoning behind this design is (1) vendor extensions can conflict with each other and we won't play favorities deciding which one is default or not and
(2) non-ratified extensions are always prone to changes, not being stable enough to be enabled by default.

All this said, we're still not able to enable all ratified extensions due to conflicts between them. Zfinx and all its dependencies aren't enabled because of a conflict with RVF. zce, zcmp and zcmt are also disabled due to RVD conflicts. When running with 64 bits we're also disabling zcf.

Signed-off-by Daniel Henrique Barboza <[email protected]>

---
 target/riscv/cpu-qom.h |  1 +
 target/riscv/cpu.c     | 50 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

target/riscv/cpu-qom.h +1 -0

index 04af50983e..f3fbe37a2c 100644
@@ -30,6 +30,7 @@
 #define CPU_RESOLVING_TYPE TYPE_RISCV_CPU
 
 #define TYPE_RISCV_CPU_ANY              RISCV_CPU_TYPE_NAME("any")
+#define TYPE_RISCV_CPU_MAX              RISCV_CPU_TYPE_NAME("max")
 #define TYPE_RISCV_CPU_BASE32           RISCV_CPU_TYPE_NAME("rv32")
 #define TYPE_RISCV_CPU_BASE64           RISCV_CPU_TYPE_NAME("rv64")
 #define TYPE_RISCV_CPU_BASE128          RISCV_CPU_TYPE_NAME("x-rv128")

target/riscv/cpu.c +50 -0

74 lines not on this page · raw message

Signature · 1 line

2.41.0

Conor Dooley

2023-07-12 19:23 UTC #7 lore raw

Quoted 12 lines · Daniel Henrique Barboza

The 'max' CPU type is used by tooling to determine what's the most capable CPU a current QEMU version implements. Other archs such as ARM implements this type. Let's add it to RISC-V.
What we consider "most capable CPU" in this context are related to ratified, non-vendor extensions. This means that we want the 'max' CPU to enable all (possible) ratified extensions by default. The reasoning behind this design is (1) vendor extensions can conflict with each other and we won't play favorities deciding which one is default or not and
(2) non-ratified extensions are always prone to changes, not being stable enough to be enabled by default.

Quoted 7 lines

All this said, we're still not able to enable all ratified extensions due to conflicts between them. Zfinx and all its dependencies aren't enabled because of a conflict with RVF. zce, zcmp and zcmt are also disabled due to RVD conflicts. When running with 64 bits we're also disabling zcf.
Signed-off-by Daniel Henrique Barboza <[email protected]>

This seems like it will be super helpful for CI stuff etc, thanks for doing it.

Daniel Henrique Barboza

author

2023-07-12 20:31 UTC #8 lore raw

Quoted 19 lines · Conor Dooley

Quoted 18 lines · Daniel Henrique Barboza
The 'max' CPU type is used by tooling to determine what's the most capable CPU a current QEMU version implements. Other archs such as ARM implements this type. Let's add it to RISC-V.
What we consider "most capable CPU" in this context are related to ratified, non-vendor extensions. This means that we want the 'max' CPU to enable all (possible) ratified extensions by default. The reasoning behind this design is (1) vendor extensions can conflict with each other and we won't play favorities deciding which one is default or not and
(2) non-ratified extensions are always prone to changes, not being stable enough to be enabled by default.
All this said, we're still not able to enable all ratified extensions due to conflicts between them. Zfinx and all its dependencies aren't enabled because of a conflict with RVF. zce, zcmp and zcmt are also disabled due to RVD conflicts. When running with 64 bits we're also disabling zcf.

Quoted 4 lines

Quoted 1 line
Signed-off-by Daniel Henrique Barboza <[email protected]>
This seems like it will be super helpful for CI stuff etc, thanks for doing it.

And Linux actually boots on it, which was remarkable to see. I was expecting something to blow up I guess.

This is the riscv,isa DT generated:

# cat /proc/device-tree/cpus/cpu@0/riscv,isa
rv64imafdch_zicbom_zicboz_zicsr_zifencei_zihintpause_zawrs_zfa_zfh_zfhmin_zca_zcb_zcd_
zba_zbb_zbc_zbkb_zbkc_zbkx_zbs_zk_zkn_zknd_zkne_zknh_zkr_zks_zksed_zksh_zkt_
zve32f_zve64f_zve64d_smstateen_sscofpmf_sstc_svadu_svinval_svnapot_svpbmt#

I'll put this in the commit message for the next version.

Oh, and I just realized that I forgot to light up all the MISA bits (we're missing RVV). Guess I'll have to send the v2 right away.

Thanks,

Daniel

Conor Dooley

2023-07-12 21:00 UTC #9 lore raw

Quoted 28 lines · Daniel Henrique Barboza

Quoted 19 lines · Conor Dooley
Quoted 18 lines · Daniel Henrique Barboza
The 'max' CPU type is used by tooling to determine what's the most capable CPU a current QEMU version implements. Other archs such as ARM implements this type. Let's add it to RISC-V.
What we consider "most capable CPU" in this context are related to ratified, non-vendor extensions. This means that we want the 'max' CPU to enable all (possible) ratified extensions by default. The reasoning behind this design is (1) vendor extensions can conflict with each other and we won't play favorities deciding which one is default or not and
(2) non-ratified extensions are always prone to changes, not being stable enough to be enabled by default.
All this said, we're still not able to enable all ratified extensions due to conflicts between them. Zfinx and all its dependencies aren't enabled because of a conflict with RVF. zce, zcmp and zcmt are also disabled due to RVD conflicts. When running with 64 bits we're also disabling zcf.
Quoted 4 lines
Quoted 1 line
Signed-off-by Daniel Henrique Barboza <[email protected]>
This seems like it will be super helpful for CI stuff etc, thanks for doing it.
And Linux actually boots on it, which was remarkable to see. I was expecting something to blow up I guess.

Quoted 6 lines

This is the riscv,isa DT generated:

# cat /proc/device-tree/cpus/cpu@0/riscv,isa
rv64imafdch_zicbom_zicboz_zicsr_zifencei_zihintpause_zawrs_zfa_zfh_zfhmin_zca_zcb_zcd_
zba_zbb_zbc_zbkb_zbkc_zbkx_zbs_zk_zkn_zknd_zkne_zknh_zkr_zks_zksed_zksh_zkt_
zve32f_zve64f_zve64d_smstateen_sscofpmf_sstc_svadu_svinval_svnapot_svpbmt#

Of which an upstream Linux kernel, building using something close to
defconfig, accepts only rv64imafdch_zicbom_zicboz_zicntr_zicsr_zifencei_zihintpause_zihpm_zba_zbb_zbs_sscofpmf_sstc_svinval_svnapot_svpbmt so the set of possible things that break could break it has been reduced
somewhat.

btw, I noticed that the default marchid/mimpid have changed. Previously I used to see something like:
processor : 15
hart : 15
isa : rv64imafdcvh_zicbom_zicboz_zicntr_zicsr_zifencei_zihintpause_zihpm_zba_zbb_zbs_sscofpmf_sstc
mmu : sv57
mvendorid : 0x0
marchid : 0x80032
mimpid : 0x80032
in /proc/cpuinfo, but "now" I see 0x0 for marchid & mimpid. Is this change to the default behaviour intentional? I saw "now" in "s because I applied your patches on top of Alistair's next branch, which contains the changes to m*id stuff.

Cheers,
Conor.

Daniel Henrique Barboza

author

2023-07-12 21:09 UTC #10 lore raw

Quoted 42 lines · Conor Dooley

Quoted 28 lines · Daniel Henrique Barboza
Quoted 19 lines · Conor Dooley
Quoted 18 lines · Daniel Henrique Barboza
The 'max' CPU type is used by tooling to determine what's the most capable CPU a current QEMU version implements. Other archs such as ARM implements this type. Let's add it to RISC-V.
What we consider "most capable CPU" in this context are related to ratified, non-vendor extensions. This means that we want the 'max' CPU to enable all (possible) ratified extensions by default. The reasoning behind this design is (1) vendor extensions can conflict with each other and we won't play favorities deciding which one is default or not and
(2) non-ratified extensions are always prone to changes, not being stable enough to be enabled by default.
All this said, we're still not able to enable all ratified extensions due to conflicts between them. Zfinx and all its dependencies aren't enabled because of a conflict with RVF. zce, zcmp and zcmt are also disabled due to RVD conflicts. When running with 64 bits we're also disabling zcf.
Quoted 4 lines
Quoted 1 line
Signed-off-by Daniel Henrique Barboza <[email protected]>
This seems like it will be super helpful for CI stuff etc, thanks for doing it.
And Linux actually boots on it, which was remarkable to see. I was expecting something to blow up I guess.
Quoted 6 lines
This is the riscv,isa DT generated:
# cat /proc/device-tree/cpus/cpu@0/riscv,isa
rv64imafdch_zicbom_zicboz_zicsr_zifencei_zihintpause_zawrs_zfa_zfh_zfhmin_zca_zcb_zcd_
zba_zbb_zbc_zbkb_zbkc_zbkx_zbs_zk_zkn_zknd_zkne_zknh_zkr_zks_zksed_zksh_zkt_
zve32f_zve64f_zve64d_smstateen_sscofpmf_sstc_svadu_svinval_svnapot_svpbmt#
Of which an upstream Linux kernel, building using something close to
defconfig, accepts only rv64imafdch_zicbom_zicboz_zicntr_zicsr_zifencei_zihintpause_zihpm_zba_zbb_zbs_sscofpmf_sstc_svinval_svnapot_svpbmt so the set of possible things that break could break it has been reduced
somewhat.

Quoted 13 lines

btw, I noticed that the default marchid/mimpid have changed. Previously I used to see something like:
processor : 15
hart : 15
isa : rv64imafdcvh_zicbom_zicboz_zicntr_zicsr_zifencei_zihintpause_zihpm_zba_zbb_zbs_sscofpmf_sstc
mmu : sv57
mvendorid : 0x0
marchid : 0x80032
mimpid : 0x80032
in /proc/cpuinfo, but "now" I see 0x0 for marchid & mimpid. Is this change to the default behaviour intentional? I saw "now" in "s because I applied your patches on top of Alistair's next branch, which contains the changes to m*id stuff.

It is intentional. Those default marchid/mimpid vals were derived from the current QEMU version ID/build and didn't mean much.

It is still possible to set them via "-cpu rv64,marchid=N,mimpid=N" if needed when using the generic (rv64,rv32) CPUs. Vendor CPUs can't have their machine IDs changed via command line.

Thanks,

Daniel

Quoted 3 lines

Cheers,
Conor.

Conor Dooley

2023-07-12 21:35 UTC #11 lore raw

Quoted 6 lines · Daniel Henrique Barboza

It is intentional. Those default marchid/mimpid vals were derived from the current QEMU version ID/build and didn't mean much.
It is still possible to set them via "-cpu rv64,marchid=N,mimpid=N" if needed when using the generic (rv64,rv32) CPUs. Vendor CPUs can't have their machine IDs changed via command line.

Sounds good, thanks. I did just now go and check icicle to see what it would report & it does not boot. I'll go bisect...

Daniel Henrique Barboza

author

2023-07-12 21:39 UTC #12 lore raw

Quoted 11 lines · Conor Dooley

Quoted 6 lines · Daniel Henrique Barboza
It is intentional. Those default marchid/mimpid vals were derived from the current QEMU version ID/build and didn't mean much.
It is still possible to set them via "-cpu rv64,marchid=N,mimpid=N" if needed when using the generic (rv64,rv32) CPUs. Vendor CPUs can't have their machine IDs changed via command line.
Sounds good, thanks. I did just now go and check icicle to see what it would report & it does not boot. I'll go bisect...

BTW how are you booting the icicle board nowadays? I remember you mentioning about some changes in the FDT being required to boot and whatnot.

If it's not too hard I'll add it in my test scripts to keep it under check. Perhaps we can even add it to QEMU testsuite.

Daniel

Conor Dooley

2023-07-12 22:15 UTC #13 lore raw

Quoted 10 lines · Daniel Henrique Barboza

Quoted 9 lines · Conor Dooley
Quoted 6 lines · Daniel Henrique Barboza
It is intentional. Those default marchid/mimpid vals were derived from the current QEMU version ID/build and didn't mean much.
It is still possible to set them via "-cpu rv64,marchid=N,mimpid=N" if needed when using the generic (rv64,rv32) CPUs. Vendor CPUs can't have their machine IDs changed via command line.

Quoted 5 lines

Quoted 2 lines
Sounds good, thanks. I did just now go and check icicle to see what it would report & it does not boot. I'll go bisect...
BTW how are you booting the icicle board nowadays? I remember you mentioning about some changes in the FDT being required to boot and whatnot.

I do direct kernel boots, as the HSS doesn't work anymore, and just lie a bit to QEMU about how much DDR we have.
.PHONY: qemu-icicle
qemu-icicle:

	$(qemu) -M microchip-icicle-kit \
		-m 3G -smp 5 \
		-kernel $(vmlinux_bin) \
		-dtb $(icicle_dtb) \
		-initrd $(initramfs) \
		-display none -serial null \
		-serial stdio \
		-D qemu.log -d unimp

The platform only supports 2 GiB of DDR, not 3, but if I pass 2 to QEMU it thinks there's 1 GiB at 0x8000_0000 and 1 GiB at 0x10_0000_0000. The upstream devicetree (and current FPGA reference design) expects there to be 1 GiB at 0x8000_0000 and 1 GiB at 0x10_4000_0000. If I lie to QEMU, it thinks there is 1 GiB at 0x8000_0000 and 2 GiB at 0x10_0000_0000, and things just work. I prefer doing it this way than having to modify the DT, it is a lot easier to explain to people this way.

I've been meaning to work the support for the icicle & mpfs in QEMU, but it just gets shunted down the priority list. I'd really like if a proper boot flow would run in QEMU, which means fixing whatever broke the HSS, but I've recently picked up maintainership of dt-binding stuff in Linux, so I've unfortunately got even less time to try and work on it. Maybe we'll get some new graduate in and I can make them suffer in my stead...

Quoted 2 lines

If it's not too hard I'll add it in my test scripts to keep it under check. Perhaps we can even add it to QEMU testsuite.

I don't think it really should be that bad, at least for the direct kernel boot, which is what I mainly care about, since I use it fairly often for debugging boot stuff in Linux.

Anyways, aa903cf31391dd505b399627158f1292a6d19896 is the first bad commit:
commit aa903cf31391dd505b399627158f1292a6d19896

Author: Bin Meng <[email protected]>
Date:   Fri Jun 30 23:36:04 2023 +0800

roms/opensbi: Upgrade from v1.2 to v1.3

Upgrade OpenSBI from v1.2 to v1.3 and the pre-built bios images.

And I see something like:

qemu//build/qemu-system-riscv64 -M microchip-icicle-kit \
        -m 3G -smp 5 \
        -kernel vmlinux.bin \
        -dtb icicle.dtb \
        -initrd initramfs.cpio.gz \
        -display none -serial null \
        -serial stdio \
        -D qemu.log -d unimp

qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000000 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000001 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000001 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000002 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000002 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000003 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000003 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000004 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000004 because privilege spec version does not match

OpenSBI v1.3
____ _____ ____ _____

  / __ \                  / ____|  _ \_   _|
 | |  | |_ __   ___ _ __ | (___ | |_) || |
 | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
 | |__| | |_) |  __/ | | |____) | |_) || |_
  \____/| .__/ \___|_| |_|_____/|___/_____|
        | |
        |_|

init_coldboot: ipi init failed (error -1009)

Just to note, because we use our own firmware that vendors in OpenSBI and compiles only a significantly cut down number of files from it, we do not use the fw_dynamic etc flow on our hardware. As a result, we have not tested v1.3, nor do we have any immediate plans to change our platform firmware to vendor v1.3 either.

I unless there's something obvious to you, it sounds like I will need to go and bisect OpenSBI. That's a job for another day though, given the time.

Cheers,
Conor.

Conor Dooley

2023-07-13 22:12 UTC #14 lore raw

Boot failure after QEMU's upgrade to OpenSBI v1.3 (was Re: [PATCH for-8.2 6/7] target/riscv: add 'max' CPU type)

+CC OpenSBI Mailing list

I've not yet had the chance to bisect this, so adding the OpenSBI folks to CC in case they might have an idea for what to try.

And a question for you below Daniel.

Quoted 71 lines · Conor Dooley

Quoted 10 lines · Daniel Henrique Barboza
Quoted 9 lines · Conor Dooley
Quoted 6 lines · Daniel Henrique Barboza
It is intentional. Those default marchid/mimpid vals were derived from the current QEMU version ID/build and didn't mean much.
It is still possible to set them via "-cpu rv64,marchid=N,mimpid=N" if needed when using the generic (rv64,rv32) CPUs. Vendor CPUs can't have their machine IDs changed via command line.
Quoted 5 lines
Quoted 2 lines
Sounds good, thanks. I did just now go and check icicle to see what it would report & it does not boot. I'll go bisect...
BTW how are you booting the icicle board nowadays? I remember you mentioning about some changes in the FDT being required to boot and whatnot.
I do direct kernel boots, as the HSS doesn't work anymore, and just lie a bit to QEMU about how much DDR we have.
.PHONY: qemu-icicle
qemu-icicle:
	$(qemu) -M microchip-icicle-kit \
		-m 3G -smp 5 \
		-kernel $(vmlinux_bin) \
		-dtb $(icicle_dtb) \
		-initrd $(initramfs) \
		-display none -serial null \
		-serial stdio \
		-D qemu.log -d unimp
The platform only supports 2 GiB of DDR, not 3, but if I pass 2 to QEMU it thinks there's 1 GiB at 0x8000_0000 and 1 GiB at 0x10_0000_0000. The upstream devicetree (and current FPGA reference design) expects there to be 1 GiB at 0x8000_0000 and 1 GiB at 0x10_4000_0000. If I lie to QEMU, it thinks there is 1 GiB at 0x8000_0000 and 2 GiB at 0x10_0000_0000, and things just work. I prefer doing it this way than having to modify the DT, it is a lot easier to explain to people this way.
I've been meaning to work the support for the icicle & mpfs in QEMU, but it just gets shunted down the priority list. I'd really like if a proper boot flow would run in QEMU, which means fixing whatever broke the HSS, but I've recently picked up maintainership of dt-binding stuff in Linux, so I've unfortunately got even less time to try and work on it. Maybe we'll get some new graduate in and I can make them suffer in my stead...
Quoted 2 lines
If it's not too hard I'll add it in my test scripts to keep it under check. Perhaps we can even add it to QEMU testsuite.
I don't think it really should be that bad, at least for the direct kernel boot, which is what I mainly care about, since I use it fairly often for debugging boot stuff in Linux.
Anyways, aa903cf31391dd505b399627158f1292a6d19896 is the first bad commit:
commit aa903cf31391dd505b399627158f1292a6d19896
Author: Bin Meng <[email protected]>
Date:   Fri Jun 30 23:36:04 2023 +0800
roms/opensbi: Upgrade from v1.2 to v1.3
Upgrade OpenSBI from v1.2 to v1.3 and the pre-built bios images.
And I see something like:
qemu//build/qemu-system-riscv64 -M microchip-icicle-kit \
        -m 3G -smp 5 \
        -kernel vmlinux.bin \
        -dtb icicle.dtb \
        -initrd initramfs.cpio.gz \
        -display none -serial null \
        -serial stdio \
        -D qemu.log -d unimp

Quoted 9 lines

qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000000 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000001 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000001 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000002 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000002 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000003 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000003 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000004 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000004 because privilege spec version does not match

Why am I seeing these warnings? Does the mpfs machine type need to disable some things? It only supports rv64imafdc per the DT, and predates things like Zca existing, so emitting warnings does not seem fair at all to me!

Quoted 19 lines

OpenSBI v1.3
____ _____ ____ _____
  / __ \                  / ____|  _ \_   _|
 | |  | |_ __   ___ _ __ | (___ | |_) || |
 | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
 | |__| | |_) |  __/ | | |____) | |_) || |_
  \____/| .__/ \___|_| |_|_____/|___/_____|
        | |
        |_|
init_coldboot: ipi init failed (error -1009)
Just to note, because we use our own firmware that vendors in OpenSBI and compiles only a significantly cut down number of files from it, we do not use the fw_dynamic etc flow on our hardware. As a result, we have not tested v1.3, nor do we have any immediate plans to change our platform firmware to vendor v1.3 either.

Quoted 6 lines

I unless there's something obvious to you, it sounds like I will need to go and bisect OpenSBI. That's a job for another day though, given the time.
Cheers,
Conor.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/opensbi/attachments/20230713/04de663d/attachment-0001.sig>

Daniel Henrique Barboza

author

2023-07-13 22:35 UTC #15 lore raw

Boot failure after QEMU's upgrade to OpenSBI v1.3 (was Re: [PATCH for-8.2 6/7] target/riscv: add 'max' CPU type)

Quoted 89 lines · Conor Dooley

+CC OpenSBI Mailing list
I've not yet had the chance to bisect this, so adding the OpenSBI folks to CC in case they might have an idea for what to try.
And a question for you below Daniel.
Quoted 71 lines · Conor Dooley
Quoted 10 lines · Daniel Henrique Barboza
Quoted 9 lines · Conor Dooley
Quoted 6 lines · Daniel Henrique Barboza
It is intentional. Those default marchid/mimpid vals were derived from the current QEMU version ID/build and didn't mean much.
It is still possible to set them via "-cpu rv64,marchid=N,mimpid=N" if needed when using the generic (rv64,rv32) CPUs. Vendor CPUs can't have their machine IDs changed via command line.
Quoted 5 lines
Quoted 2 lines
Sounds good, thanks. I did just now go and check icicle to see what it would report & it does not boot. I'll go bisect...
BTW how are you booting the icicle board nowadays? I remember you mentioning about some changes in the FDT being required to boot and whatnot.
I do direct kernel boots, as the HSS doesn't work anymore, and just lie a bit to QEMU about how much DDR we have.
.PHONY: qemu-icicle
qemu-icicle:
	$(qemu) -M microchip-icicle-kit \
		-m 3G -smp 5 \
		-kernel $(vmlinux_bin) \
		-dtb $(icicle_dtb) \
		-initrd $(initramfs) \
		-display none -serial null \
		-serial stdio \
		-D qemu.log -d unimp
The platform only supports 2 GiB of DDR, not 3, but if I pass 2 to QEMU it thinks there's 1 GiB at 0x8000_0000 and 1 GiB at 0x10_0000_0000. The upstream devicetree (and current FPGA reference design) expects there to be 1 GiB at 0x8000_0000 and 1 GiB at 0x10_4000_0000. If I lie to QEMU, it thinks there is 1 GiB at 0x8000_0000 and 2 GiB at 0x10_0000_0000, and things just work. I prefer doing it this way than having to modify the DT, it is a lot easier to explain to people this way.
I've been meaning to work the support for the icicle & mpfs in QEMU, but it just gets shunted down the priority list. I'd really like if a proper boot flow would run in QEMU, which means fixing whatever broke the HSS, but I've recently picked up maintainership of dt-binding stuff in Linux, so I've unfortunately got even less time to try and work on it. Maybe we'll get some new graduate in and I can make them suffer in my stead...
Quoted 2 lines
If it's not too hard I'll add it in my test scripts to keep it under check. Perhaps we can even add it to QEMU testsuite.
I don't think it really should be that bad, at least for the direct kernel boot, which is what I mainly care about, since I use it fairly often for debugging boot stuff in Linux.
Anyways, aa903cf31391dd505b399627158f1292a6d19896 is the first bad commit:
commit aa903cf31391dd505b399627158f1292a6d19896
Author: Bin Meng <[email protected]>
Date:   Fri Jun 30 23:36:04 2023 +0800
roms/opensbi: Upgrade from v1.2 to v1.3
Upgrade OpenSBI from v1.2 to v1.3 and the pre-built bios images.
And I see something like:
qemu//build/qemu-system-riscv64 -M microchip-icicle-kit \
         -m 3G -smp 5 \
         -kernel vmlinux.bin \
         -dtb icicle.dtb \
         -initrd initramfs.cpio.gz \
         -display none -serial null \
         -serial stdio \
         -D qemu.log -d unimp
Quoted 9 lines
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000000 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000001 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000001 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000002 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000002 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000003 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000003 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000004 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000004 because privilege spec version does not match

Quoted 4 lines

Why am I seeing these warnings? Does the mpfs machine type need to disable some things? It only supports rv64imafdc per the DT, and predates things like Zca existing, so emitting warnings does not seem fair at all to me!

QEMU will disable extensions that are newer than a priv spec version that is set by the CPU. IIUC the icicle board is running a sifive_u54 CPU by default. That CPU has a priv spec version 1_10_0. The CPU is also enabling C.

We will enable zca if C is enabled. C and D enabled will also enable zcd. But then the priv check will disabled both because zca and zcd have priv spec 1_12_0.

This is a side effect for a change that I did a few months ago. Back then we weren't disabling stuff correctly. The warnings are annoying but are benign. And apparently the sifive_u54 CPU is being inconsistent for some time and we noticed just now.

Now, if the icicle board is supposed to have zca and zcd then we have a problem. We'll need to discuss whether we move sifive_u54 CPU priv spec to 1_12_0 (I'm not sure how this will affect other boards that uses this CPU) or remove this priv spec disable code altogether from QEMU.

Thanks,

Daniel

Quoted 28 lines

Quoted 26 lines
OpenSBI v1.3
    ____                    _____ ____ _____
   / __ \                  / ____|  _ \_   _|
  | |  | |_ __   ___ _ __ | (___ | |_) || |
  | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
  | |__| | |_) |  __/ | | |____) | |_) || |_
   \____/| .__/ \___|_| |_|_____/|___/_____|
         | |
         |_|
init_coldboot: ipi init failed (error -1009)
Just to note, because we use our own firmware that vendors in OpenSBI and compiles only a significantly cut down number of files from it, we do not use the fw_dynamic etc flow on our hardware. As a result, we have not tested v1.3, nor do we have any immediate plans to change our platform firmware to vendor v1.3 either.
I unless there's something obvious to you, it sounds like I will need to go and bisect OpenSBI. That's a job for another day though, given the time.
Cheers,
Conor.

Conor Dooley

2023-07-13 22:48 UTC #16 lore raw

Re: Boot failure after QEMU's upgrade to OpenSBI v1.3 (was Re: [PATCH for-8.2 6/7] target/riscv: add 'max' CPU type)

Quoted 27 lines · Daniel Henrique Barboza

Quoted 16 lines · Conor Dooley
And a question for you below Daniel.
Quoted 9 lines · Conor Dooley
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000000 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000001 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000001 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000002 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000002 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000003 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000003 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000004 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000004 because privilege spec version does not match
Quoted 4 lines
Why am I seeing these warnings? Does the mpfs machine type need to disable some things? It only supports rv64imafdc per the DT, and predates things like Zca existing, so emitting warnings does not seem fair at all to me!
QEMU will disable extensions that are newer than a priv spec version that is set by the CPU. IIUC the icicle board is running a sifive_u54 CPU by default. That CPU has a priv spec version 1_10_0. The CPU is also enabling C.

Quoted 5 lines

We will enable zca if C is enabled. C and D enabled will also enable zcd. But then the priv check will disabled both because zca and zcd have priv spec 1_12_0.
This is a side effect for a change that I did a few months ago. Back then we weren't disabling stuff correctly.

Yah, I did check out the blame, hence directing it at you. Thanks for the explanation.

Quoted 1 line

The warnings are annoying but are benign.

To be honest, benign or not, this is kind of thing is only going to lead to grief. Even though only the direct kernel boot works, we do actually have some customers that are using the icicle target in QEMU.

Quoted 3 lines

And apparently the sifive_u54 CPU is being inconsistent for some time and we noticed just now.
Now, if the icicle board is supposed to have zca and zcd then we have a problem.

I don't know, this depends on how you see things in QEMU. I would say that it supports c, and not Zca/Zcf/Zcd, given it predates the extensions. I have no interest in retrofitting my devicetree stuff with them, for example.

Quoted 3 lines

We'll need to discuss whether we move sifive_u54 CPU priv spec to 1_12_0 (I'm not sure how this will affect other boards that uses this CPU) or remove this priv spec disable code altogether from QEMU.

I think you should stop warning for this? From my dumb-user perspective, the warning only "scares" me into thinking something is wrong, when there isn't. I can see a use case for the warning where someone tries to enable Zca & Co. in their QEMU incantation for a CPU that does not have the correct privilege level to support it, but I didn't try to set any options at all in that way, so the warnings seem unfair?

Cheers,
Conor.

Daniel Henrique Barboza

author

2023-07-14 01:13 UTC #17 lore raw

Re: Boot failure after QEMU's upgrade to OpenSBI v1.3 (was Re: [PATCH for-8.2 6/7] target/riscv: add 'max' CPU type)

Quoted 56 lines · Conor Dooley

Quoted 27 lines · Daniel Henrique Barboza
Quoted 16 lines · Conor Dooley
And a question for you below Daniel.
Quoted 9 lines · Conor Dooley
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000000 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000001 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000001 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000002 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000002 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000003 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000003 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000004 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000004 because privilege spec version does not match
Quoted 4 lines
Why am I seeing these warnings? Does the mpfs machine type need to disable some things? It only supports rv64imafdc per the DT, and predates things like Zca existing, so emitting warnings does not seem fair at all to me!
QEMU will disable extensions that are newer than a priv spec version that is set by the CPU. IIUC the icicle board is running a sifive_u54 CPU by default. That CPU has a priv spec version 1_10_0. The CPU is also enabling C.
Quoted 5 lines
We will enable zca if C is enabled. C and D enabled will also enable zcd. But then the priv check will disabled both because zca and zcd have priv spec 1_12_0.
This is a side effect for a change that I did a few months ago. Back then we weren't disabling stuff correctly.
Yah, I did check out the blame, hence directing it at you. Thanks for the explanation.
Quoted 1 line
The warnings are annoying but are benign.
To be honest, benign or not, this is kind of thing is only going to lead to grief. Even though only the direct kernel boot works, we do actually have some customers that are using the icicle target in QEMU.
Quoted 3 lines
And apparently the sifive_u54 CPU is being inconsistent for some time and we noticed just now.
Now, if the icicle board is supposed to have zca and zcd then we have a problem.
I don't know, this depends on how you see things in QEMU. I would say that it supports c, and not Zca/Zcf/Zcd, given it predates the extensions. I have no interest in retrofitting my devicetree stuff with them, for example.
Quoted 3 lines
We'll need to discuss whether we move sifive_u54 CPU priv spec to 1_12_0 (I'm not sure how this will affect other boards that uses this CPU) or remove this priv spec disable code altogether from QEMU.

Quoted 6 lines

I think you should stop warning for this? From my dumb-user perspective, the warning only "scares" me into thinking something is wrong, when there isn't. I can see a use case for the warning where someone tries to enable Zca & Co. in their QEMU incantation for a CPU that does not have the correct privilege level to support it, but I didn't try to set any options at all in that way, so the warnings seem unfair?

That's a fair criticism. We had similar discussions a few months back. It's weird to send warnings when the user didn't set the extensions manually, but ATM we can't tell whether an extension was user enabled or not.

So we can either show unfair warning messages or not show warnings and take the risk of silently disabling extensions that users enabled in the command line. It seems that the former is more annoying to deal with than the latter.

I guess I can propose a patch to remove the warnings. We can send warning again when we have a better solution.

Daniel

Quoted 3 lines

Cheers,
Conor.

Alistair Francis

2023-07-14 03:13 UTC #18 lore raw

Re: Boot failure after QEMU's upgrade to OpenSBI v1.3 (was Re: [PATCH for-8.2 6/7] target/riscv: add 'max' CPU type)

Quoted 72 lines · Daniel Henrique Barboza

Quoted 56 lines · Conor Dooley
Quoted 27 lines · Daniel Henrique Barboza
Quoted 16 lines · Conor Dooley
And a question for you below Daniel.
Quoted 9 lines · Conor Dooley
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000000 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000001 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000001 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000002 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000002 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000003 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000003 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000004 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000004 because privilege spec version does not match
Quoted 4 lines
Why am I seeing these warnings? Does the mpfs machine type need to disable some things? It only supports rv64imafdc per the DT, and predates things like Zca existing, so emitting warnings does not seem fair at all to me!
QEMU will disable extensions that are newer than a priv spec version that is set by the CPU. IIUC the icicle board is running a sifive_u54 CPU by default. That CPU has a priv spec version 1_10_0. The CPU is also enabling C.
Quoted 5 lines
We will enable zca if C is enabled. C and D enabled will also enable zcd. But then the priv check will disabled both because zca and zcd have priv spec 1_12_0.
This is a side effect for a change that I did a few months ago. Back then we weren't disabling stuff correctly.
Yah, I did check out the blame, hence directing it at you. Thanks for the explanation.
Quoted 1 line
The warnings are annoying but are benign.
To be honest, benign or not, this is kind of thing is only going to lead to grief. Even though only the direct kernel boot works, we do actually have some customers that are using the icicle target in QEMU.
Quoted 3 lines
And apparently the sifive_u54 CPU is being inconsistent for some time and we noticed just now.
Now, if the icicle board is supposed to have zca and zcd then we have a problem.
I don't know, this depends on how you see things in QEMU. I would say that it supports c, and not Zca/Zcf/Zcd, given it predates the extensions. I have no interest in retrofitting my devicetree stuff with them, for example.
Quoted 3 lines
We'll need to discuss whether we move sifive_u54 CPU priv spec to 1_12_0 (I'm not sure how this will affect other boards that uses this CPU) or remove this priv spec disable code altogether from QEMU.
Quoted 6 lines
I think you should stop warning for this? From my dumb-user perspective, the warning only "scares" me into thinking something is wrong, when there isn't. I can see a use case for the warning where someone tries to enable Zca & Co. in their QEMU incantation for a CPU that does not have the correct privilege level to support it, but I didn't try to set any options at all in that way, so the warnings seem unfair?
That's a fair criticism. We had similar discussions a few months back. It's weird to send warnings when the user didn't set the extensions manually, but ATM we can't tell whether an extension was user enabled or not.

Quoted 6 lines

So we can either show unfair warning messages or not show warnings and take the risk of silently disabling extensions that users enabled in the command line. It seems that the former is more annoying to deal with than the latter.
I guess I can propose a patch to remove the warnings. We can send warning again when we have a better solution.

A better solution is to just not enable Zca and friends automatically, or at least look at the priv spec before we do

Alistair

Quoted 9 lines

Daniel
Quoted 3 lines
Cheers,
Conor.

Daniel Henrique Barboza

author

2023-07-14 09:26 UTC #19 lore raw

Re: Boot failure after QEMU's upgrade to OpenSBI v1.3 (was Re: [PATCH for-8.2 6/7] target/riscv: add 'max' CPU type)

Quoted 78 lines · Alistair Francis

Quoted 76 lines · Daniel Henrique Barboza
Quoted 56 lines · Conor Dooley
Quoted 27 lines · Daniel Henrique Barboza
Quoted 16 lines · Conor Dooley
And a question for you below Daniel.
Quoted 9 lines · Conor Dooley
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000000 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000001 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000001 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000002 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000002 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000003 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000003 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000004 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000004 because privilege spec version does not match
Quoted 4 lines
Why am I seeing these warnings? Does the mpfs machine type need to disable some things? It only supports rv64imafdc per the DT, and predates things like Zca existing, so emitting warnings does not seem fair at all to me!
QEMU will disable extensions that are newer than a priv spec version that is set by the CPU. IIUC the icicle board is running a sifive_u54 CPU by default. That CPU has a priv spec version 1_10_0. The CPU is also enabling C.
Quoted 5 lines
We will enable zca if C is enabled. C and D enabled will also enable zcd. But then the priv check will disabled both because zca and zcd have priv spec 1_12_0.
This is a side effect for a change that I did a few months ago. Back then we weren't disabling stuff correctly.
Yah, I did check out the blame, hence directing it at you. Thanks for the explanation.
Quoted 1 line
The warnings are annoying but are benign.
To be honest, benign or not, this is kind of thing is only going to lead to grief. Even though only the direct kernel boot works, we do actually have some customers that are using the icicle target in QEMU.
Quoted 3 lines
And apparently the sifive_u54 CPU is being inconsistent for some time and we noticed just now.
Now, if the icicle board is supposed to have zca and zcd then we have a problem.
I don't know, this depends on how you see things in QEMU. I would say that it supports c, and not Zca/Zcf/Zcd, given it predates the extensions. I have no interest in retrofitting my devicetree stuff with them, for example.
Quoted 3 lines
We'll need to discuss whether we move sifive_u54 CPU priv spec to 1_12_0 (I'm not sure how this will affect other boards that uses this CPU) or remove this priv spec disable code altogether from QEMU.
Quoted 6 lines
I think you should stop warning for this? From my dumb-user perspective, the warning only "scares" me into thinking something is wrong, when there isn't. I can see a use case for the warning where someone tries to enable Zca & Co. in their QEMU incantation for a CPU that does not have the correct privilege level to support it, but I didn't try to set any options at all in that way, so the warnings seem unfair?
That's a fair criticism. We had similar discussions a few months back. It's weird to send warnings when the user didn't set the extensions manually, but ATM we can't tell whether an extension was user enabled or not.
So we can either show unfair warning messages or not show warnings and take the risk of silently disabling extensions that users enabled in the command line. It seems that the former is more annoying to deal with than the latter.

Quoted 5 lines

Quoted 2 lines
I guess I can propose a patch to remove the warnings. We can send warning again when we have a better solution.
A better solution is to just not enable Zca and friends automatically, or at least look at the priv spec before we do

Good idea. In fact we should do that for all extensions that we're enabling automatically.

I'll work something out. Thanks,

Daniel

Quoted 12 lines

Alistair
Quoted 9 lines
Daniel
Quoted 3 lines
Cheers,
Conor.

Conor Dooley

Reviewed-by Andrew Jones

2023-07-13 23:04 UTC #20 lore raw

Boot failure after QEMU's upgrade to OpenSBI v1.3 (was Re: [PATCH for-8.2 6/7] target/riscv: add 'max' CPU type)

In reply to Conor Dooley

Quoted 4 lines · Conor Dooley

+CC OpenSBI Mailing list
I've not yet had the chance to bisect this, so adding the OpenSBI folks to CC in case they might have an idea for what to try.

NVM this, I bisected it. Logs below.

Quoted 84 lines

And a question for you below Daniel.
Quoted 71 lines · Conor Dooley
Quoted 10 lines · Daniel Henrique Barboza
Quoted 9 lines · Conor Dooley
Quoted 6 lines · Daniel Henrique Barboza
It is intentional. Those default marchid/mimpid vals were derived from the current QEMU version ID/build and didn't mean much.
It is still possible to set them via "-cpu rv64,marchid=N,mimpid=N" if needed when using the generic (rv64,rv32) CPUs. Vendor CPUs can't have their machine IDs changed via command line.
Quoted 5 lines
Quoted 2 lines
Sounds good, thanks. I did just now go and check icicle to see what it would report & it does not boot. I'll go bisect...
BTW how are you booting the icicle board nowadays? I remember you mentioning about some changes in the FDT being required to boot and whatnot.
I do direct kernel boots, as the HSS doesn't work anymore, and just lie a bit to QEMU about how much DDR we have.
.PHONY: qemu-icicle
qemu-icicle:
	$(qemu) -M microchip-icicle-kit \
		-m 3G -smp 5 \
		-kernel $(vmlinux_bin) \
		-dtb $(icicle_dtb) \
		-initrd $(initramfs) \
		-display none -serial null \
		-serial stdio \
		-D qemu.log -d unimp
The platform only supports 2 GiB of DDR, not 3, but if I pass 2 to QEMU it thinks there's 1 GiB at 0x8000_0000 and 1 GiB at 0x10_0000_0000. The upstream devicetree (and current FPGA reference design) expects there to be 1 GiB at 0x8000_0000 and 1 GiB at 0x10_4000_0000. If I lie to QEMU, it thinks there is 1 GiB at 0x8000_0000 and 2 GiB at 0x10_0000_0000, and things just work. I prefer doing it this way than having to modify the DT, it is a lot easier to explain to people this way.
I've been meaning to work the support for the icicle & mpfs in QEMU, but it just gets shunted down the priority list. I'd really like if a proper boot flow would run in QEMU, which means fixing whatever broke the HSS, but I've recently picked up maintainership of dt-binding stuff in Linux, so I've unfortunately got even less time to try and work on it. Maybe we'll get some new graduate in and I can make them suffer in my stead...
Quoted 2 lines
If it's not too hard I'll add it in my test scripts to keep it under check. Perhaps we can even add it to QEMU testsuite.
I don't think it really should be that bad, at least for the direct kernel boot, which is what I mainly care about, since I use it fairly often for debugging boot stuff in Linux.
Anyways, aa903cf31391dd505b399627158f1292a6d19896 is the first bad commit:
commit aa903cf31391dd505b399627158f1292a6d19896
Author: Bin Meng <[email protected]>
Date:   Fri Jun 30 23:36:04 2023 +0800
roms/opensbi: Upgrade from v1.2 to v1.3
Upgrade OpenSBI from v1.2 to v1.3 and the pre-built bios images.
And I see something like:
qemu//build/qemu-system-riscv64 -M microchip-icicle-kit \
        -m 3G -smp 5 \
        -kernel vmlinux.bin \
        -dtb icicle.dtb \
        -initrd initramfs.cpio.gz \
        -display none -serial null \
        -serial stdio \
        -D qemu.log -d unimp
Quoted 9 lines
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000000 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000001 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000001 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000002 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000002 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000003 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000003 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000004 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000004 because privilege spec version does not match

Quoted 16 lines

Why am I seeing these warnings? Does the mpfs machine type need to disable some things? It only supports rv64imafdc per the DT, and predates things like Zca existing, so emitting warnings does not seem fair at all to me!
Quoted 11 lines
OpenSBI v1.3
____ _____ ____ _____
  / __ \                  / ____|  _ \_   _|
 | |  | |_ __   ___ _ __ | (___ | |_) || |
 | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
 | |__| | |_) |  __/ | | |____) | |_) || |_
  \____/| .__/ \___|_| |_|_____/|___/_____|
        | |
        |_|
init_coldboot: ipi init failed (error -1009)

This can be reproduced using OpenSBI built using make PLATFORM=generic and the QEMU incantation linked above with a -bios argument added to the incantation.

Thanks,
Conor.

acbd8fce9e5d92f07d344388a3b046f1722ce072 is the first bad commit commit acbd8fce9e5d92f07d344388a3b046f1722ce072

Author: Anup Patel <[email protected]>
Date:   Wed Apr 19 21:23:53 2023 +0530

lib: utils/ipi: Use scratch space to save per-HART MSWI pointer

Instead of using a global array indexed by hartid, we should use scratch space to save per-HART MSWI pointer.

Signed-off-by Anup Patel <[email protected]>

Reviewed-by Andrew Jones <[email protected]>

 lib/utils/ipi/aclint_mswi.c | 43 +++++++++++++++++++++++++++++++++----------
 1 file changed, 33 insertions(+), 10 deletions(-)

git bisect start
# status: waiting for both good and bad commits
# bad: [2552799a1df30a3dcd2321a8b75d61d06f5fb9fc] include: Bump-up version to 1.3
git bisect bad 2552799a1df30a3dcd2321a8b75d61d06f5fb9fc
# status: waiting for good commit(s), bad commit known
# good: [6b5188ca14e59ce7bf71afe4e7d3d557c3d31bf8] include: Bump-up version to 1.2
git bisect good 6b5188ca14e59ce7bf71afe4e7d3d557c3d31bf8
# good: [6b5188ca14e59ce7bf71afe4e7d3d557c3d31bf8] include: Bump-up version to 1.2
git bisect good 6b5188ca14e59ce7bf71afe4e7d3d557c3d31bf8
# good: [908be1b85c8ff0695ea226fbbf0ff24a779cdece] gpio/starfive: add gpio driver and support gpio reset git bisect good 908be1b85c8ff0695ea226fbbf0ff24a779cdece
# good: [6bc02dede86c47f87e65293b7099e9caf3b22c29] lib: sbi: Simplify sbi_ipi_process remove goto git bisect good 6bc02dede86c47f87e65293b7099e9caf3b22c29
# good: [bbff53fe3b6cdd3c9bc084d489640d7ee2a3f831] lib: sbi_pmu: Use heap for per-HART PMU state git bisect good bbff53fe3b6cdd3c9bc084d489640d7ee2a3f831
# bad: [f0516beae068ffce0d5a79f09a96904a661a25ba] lib: utils/timer: Use scratch space to save per-HART MTIMER pointer git bisect bad f0516beae068ffce0d5a79f09a96904a661a25ba
# good: [5a8cfcdf19d98b8dc5dd5a087a2eceb7f5b185fb] lib: utils/ipi: Use heap in ACLINT MSWI driver git bisect good 5a8cfcdf19d98b8dc5dd5a087a2eceb7f5b185fb
# good: [7e5636ac3788451991a65791c5adcc7798dcc22a] lib: utils/timer: Use heap in ACLINT MTIMER driver git bisect good 7e5636ac3788451991a65791c5adcc7798dcc22a
# bad: [acbd8fce9e5d92f07d344388a3b046f1722ce072] lib: utils/ipi: Use scratch space to save per-HART MSWI pointer git bisect bad acbd8fce9e5d92f07d344388a3b046f1722ce072
# good: [3c1c972cb69d670ddc309391c4db76f1f19fd77e] lib: utils/fdt: Use heap in FDT domain parsing git bisect good 3c1c972cb69d670ddc309391c4db76f1f19fd77e

# first bad commit: [acbd8fce9e5d92f07d344388a3b046f1722ce072] lib: utils/ipi: Use scratch space to save per-HART MSWI pointer

Anup Patel

2023-07-14 04:30 UTC #21 lore raw

Boot failure after QEMU's upgrade to OpenSBI v1.3 (was Re: [PATCH for-8.2 6/7] target/riscv: add 'max' CPU type)

In reply to Conor Dooley

Quoted 90 lines · On Fri, Jul 14, 2023 at 3:43?AM Conor Dooley <[email protected]> wrote:

+CC OpenSBI Mailing list
I've not yet had the chance to bisect this, so adding the OpenSBI folks to CC in case they might have an idea for what to try.
And a question for you below Daniel.
Quoted 71 lines · Conor Dooley
Quoted 10 lines · Daniel Henrique Barboza
Quoted 9 lines · Conor Dooley
Quoted 6 lines · Daniel Henrique Barboza
It is intentional. Those default marchid/mimpid vals were derived from the current QEMU version ID/build and didn't mean much.
It is still possible to set them via "-cpu rv64,marchid=N,mimpid=N" if needed when using the generic (rv64,rv32) CPUs. Vendor CPUs can't have their machine IDs changed via command line.
Quoted 5 lines
Quoted 2 lines
Sounds good, thanks. I did just now go and check icicle to see what it would report & it does not boot. I'll go bisect...
BTW how are you booting the icicle board nowadays? I remember you mentioning about some changes in the FDT being required to boot and whatnot.
I do direct kernel boots, as the HSS doesn't work anymore, and just lie a bit to QEMU about how much DDR we have.
.PHONY: qemu-icicle
qemu-icicle:
      $(qemu) -M microchip-icicle-kit \
              -m 3G -smp 5 \
              -kernel $(vmlinux_bin) \
              -dtb $(icicle_dtb) \
              -initrd $(initramfs) \
              -display none -serial null \
              -serial stdio \
              -D qemu.log -d unimp
The platform only supports 2 GiB of DDR, not 3, but if I pass 2 to QEMU it thinks there's 1 GiB at 0x8000_0000 and 1 GiB at 0x10_0000_0000. The upstream devicetree (and current FPGA reference design) expects there to be 1 GiB at 0x8000_0000 and 1 GiB at 0x10_4000_0000. If I lie to QEMU, it thinks there is 1 GiB at 0x8000_0000 and 2 GiB at 0x10_0000_0000, and things just work. I prefer doing it this way than having to modify the DT, it is a lot easier to explain to people this way.
I've been meaning to work the support for the icicle & mpfs in QEMU, but it just gets shunted down the priority list. I'd really like if a proper boot flow would run in QEMU, which means fixing whatever broke the HSS, but I've recently picked up maintainership of dt-binding stuff in Linux, so I've unfortunately got even less time to try and work on it. Maybe we'll get some new graduate in and I can make them suffer in my stead...
Quoted 2 lines
If it's not too hard I'll add it in my test scripts to keep it under check. Perhaps we can even add it to QEMU testsuite.
I don't think it really should be that bad, at least for the direct kernel boot, which is what I mainly care about, since I use it fairly often for debugging boot stuff in Linux.
Anyways, aa903cf31391dd505b399627158f1292a6d19896 is the first bad commit:
commit aa903cf31391dd505b399627158f1292a6d19896
Author: Bin Meng <[email protected]>
Date:   Fri Jun 30 23:36:04 2023 +0800
roms/opensbi: Upgrade from v1.2 to v1.3
Upgrade OpenSBI from v1.2 to v1.3 and the pre-built bios images.
And I see something like:
qemu//build/qemu-system-riscv64 -M microchip-icicle-kit \
        -m 3G -smp 5 \
        -kernel vmlinux.bin \
        -dtb icicle.dtb \
        -initrd initramfs.cpio.gz \
        -display none -serial null \
        -serial stdio \
        -D qemu.log -d unimp
Quoted 9 lines
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000000 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000001 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000001 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000002 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000002 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000003 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000003 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000004 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000004 because privilege spec version does not match

Quoted 28 lines

Why am I seeing these warnings? Does the mpfs machine type need to disable some things? It only supports rv64imafdc per the DT, and predates things like Zca existing, so emitting warnings does not seem fair at all to me!
Quoted 23 lines
OpenSBI v1.3
____ _____ ____ _____
  / __ \                  / ____|  _ \_   _|
 | |  | |_ __   ___ _ __ | (___ | |_) || |
 | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
 | |__| | |_) |  __/ | | |____) | |_) || |_
  \____/| .__/ \___|_| |_|_____/|___/_____|
        | |
        |_|
init_coldboot: ipi init failed (error -1009)
Just to note, because we use our own firmware that vendors in OpenSBI and compiles only a significantly cut down number of files from it, we do not use the fw_dynamic etc flow on our hardware. As a result, we have not tested v1.3, nor do we have any immediate plans to change our platform firmware to vendor v1.3 either.
I unless there's something obvious to you, it sounds like I will need to go and bisect OpenSBI. That's a job for another day though, given the time.

The real issue is some CPU/HART DT nodes marked as disabled in the DT passed to OpenSBI 1.3.

This issue does not exist in any of the DTs generated by QEMU but some of the DTs in the kernel (such as microchip and SiFive board DTs) have the E-core disabled.

I had discovered this issue in a totally different context after the OpenSBI 1.3 release happened. This issue is already fixed in the latest OpenSBI by the following commit c6a35733b74aeff612398f274ed19a74f81d1f37 ("lib: utils:
Fix sbi_hartid_to_scratch() usage in ACLINT drivers").

I always assumed that Microchip hss.bin is the preferred BIOS for the QEMU microchip-icicle-kit machine but I guess that's not true.

At this point, you can either:
1) Use latest OpenSBI on QEMU microchip-icicle-kit machine
2) Ensure CPU0 DT node is enabled in DT when booting on QEMU microchip-icicle-kit machine with OpenSBI 1.3

Regards,
Anup

Conor Dooley

2023-07-14 10:19 UTC #22 lore raw

Boot failure after QEMU's upgrade to OpenSBI v1.3 (was Re: [PATCH for-8.2 6/7] target/riscv: add 'max' CPU type)

Quoted 26 lines · Anup Patel

Quoted 18 lines
Quoted 18 lines
OpenSBI v1.3
____ _____ ____ _____
  / __ \                  / ____|  _ \_   _|
 | |  | |_ __   ___ _ __ | (___ | |_) || |
 | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
 | |__| | |_) |  __/ | | |____) | |_) || |_
  \____/| .__/ \___|_| |_|_____/|___/_____|
        | |
        |_|
init_coldboot: ipi init failed (error -1009)
Just to note, because we use our own firmware that vendors in OpenSBI and compiles only a significantly cut down number of files from it, we do not use the fw_dynamic etc flow on our hardware. As a result, we have not tested v1.3, nor do we have any immediate plans to change our platform firmware to vendor v1.3 either.
Quoted 4 lines
Quoted 4 lines
I unless there's something obvious to you, it sounds like I will need to go and bisect OpenSBI. That's a job for another day though, given the time.
The real issue is some CPU/HART DT nodes marked as disabled in the DT passed to OpenSBI 1.3.

Quoted 8 lines

This issue does not exist in any of the DTs generated by QEMU but some of the DTs in the kernel (such as microchip and SiFive board DTs) have the E-core disabled.
I had discovered this issue in a totally different context after the OpenSBI 1.3 release happened. This issue is already fixed in the latest OpenSBI by the following commit c6a35733b74aeff612398f274ed19a74f81d1f37 ("lib: utils:
Fix sbi_hartid_to_scratch() usage in ACLINT drivers").

Great, thanks Anup! I thought I had tested tip-of-tree too, but obviously not.

Quoted 2 lines

I always assumed that Microchip hss.bin is the preferred BIOS for the QEMU microchip-icicle-kit machine but I guess that's not true.

Unfortunately the HSS has not worked in QEMU for a long time, and while I would love to fix it, but am pretty stretched for spare time to begin with.
I usually just do direct kernel boots, which use the OpenSBI that comes with QEMU, as I am sure you already know :)

Quoted 5 lines

At this point, you can either:
1) Use latest OpenSBI on QEMU microchip-icicle-kit machine
2) Ensure CPU0 DT node is enabled in DT when booting on QEMU microchip-icicle-kit machine with OpenSBI 1.3

Will OpenSBI disable it? If not, I think option 2) needs to be remove
the DT node. I'll just use tip-of-tree myself & up to the
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/opensbi/attachments/20230714/3cdabf44/attachment-0001.sig>

Conor Dooley

2023-07-14 12:28 UTC #23 lore raw

Boot failure after QEMU's upgrade to OpenSBI v1.3 (was Re: [PATCH for-8.2 6/7] target/riscv: add 'max' CPU type)

Quoted 40 lines · Conor Dooley

Quoted 26 lines · Anup Patel
Quoted 18 lines
Quoted 18 lines
OpenSBI v1.3
____ _____ ____ _____
  / __ \                  / ____|  _ \_   _|
 | |  | |_ __   ___ _ __ | (___ | |_) || |
 | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
 | |__| | |_) |  __/ | | |____) | |_) || |_
  \____/| .__/ \___|_| |_|_____/|___/_____|
        | |
        |_|
init_coldboot: ipi init failed (error -1009)
Just to note, because we use our own firmware that vendors in OpenSBI and compiles only a significantly cut down number of files from it, we do not use the fw_dynamic etc flow on our hardware. As a result, we have not tested v1.3, nor do we have any immediate plans to change our platform firmware to vendor v1.3 either.
Quoted 4 lines
Quoted 4 lines
I unless there's something obvious to you, it sounds like I will need to go and bisect OpenSBI. That's a job for another day though, given the time.
The real issue is some CPU/HART DT nodes marked as disabled in the DT passed to OpenSBI 1.3.
Quoted 8 lines
This issue does not exist in any of the DTs generated by QEMU but some of the DTs in the kernel (such as microchip and SiFive board DTs) have the E-core disabled.
I had discovered this issue in a totally different context after the OpenSBI 1.3 release happened. This issue is already fixed in the latest OpenSBI by the following commit c6a35733b74aeff612398f274ed19a74f81d1f37 ("lib: utils:
Fix sbi_hartid_to_scratch() usage in ACLINT drivers").
Great, thanks Anup! I thought I had tested tip-of-tree too, but obviously not.

Quoted 11 lines

Quoted 2 lines
I always assumed that Microchip hss.bin is the preferred BIOS for the QEMU microchip-icicle-kit machine but I guess that's not true.
Unfortunately the HSS has not worked in QEMU for a long time, and while I would love to fix it, but am pretty stretched for spare time to begin with.
I usually just do direct kernel boots, which use the OpenSBI that comes with QEMU, as I am sure you already know :)
Quoted 2 lines
At this point, you can either:
1) Use latest OpenSBI on QEMU microchip-icicle-kit machine

I forgot to reply to this point, wondering what should be done with QEMU. Bumping to v1.3 in QEMU introduces a regression here, regardless of whether I can go and build a fixed version of OpenSBI.

Quoted 5 lines

Quoted 2 lines
2) Ensure CPU0 DT node is enabled in DT when booting on QEMU microchip-icicle-kit machine with OpenSBI 1.3
Will OpenSBI disable it? If not, I think option 2) needs to be remove the DT node. I'll just use tip-of-tree myself & up to the

Clearly didn't finish this comment. It was meant to say "up to the QEMU maintainers what they want to do on the QEMU side of things".

Thanks,
Conor.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/opensbi/attachments/20230714/7ad470a8/attachment.sig>

Atish Patra

2023-07-15 09:12 UTC #24 lore raw

Boot failure after QEMU's upgrade to OpenSBI v1.3 (was Re: [PATCH for-8.2 6/7] target/riscv: add 'max' CPU type)

Quoted 51 lines · On Fri, Jul 14, 2023 at 5:29?AM Conor Dooley <[email protected]> wrote:

Quoted 49 lines · Conor Dooley
Quoted 26 lines · Anup Patel
Quoted 18 lines
Quoted 18 lines
OpenSBI v1.3
____ _____ ____ _____
  / __ \                  / ____|  _ \_   _|
 | |  | |_ __   ___ _ __ | (___ | |_) || |
 | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
 | |__| | |_) |  __/ | | |____) | |_) || |_
  \____/| .__/ \___|_| |_|_____/|___/_____|
        | |
        |_|
init_coldboot: ipi init failed (error -1009)
Just to note, because we use our own firmware that vendors in OpenSBI and compiles only a significantly cut down number of files from it, we do not use the fw_dynamic etc flow on our hardware. As a result, we have not tested v1.3, nor do we have any immediate plans to change our platform firmware to vendor v1.3 either.
Quoted 4 lines
Quoted 4 lines
I unless there's something obvious to you, it sounds like I will need to go and bisect OpenSBI. That's a job for another day though, given the time.
The real issue is some CPU/HART DT nodes marked as disabled in the DT passed to OpenSBI 1.3.
Quoted 8 lines
This issue does not exist in any of the DTs generated by QEMU but some of the DTs in the kernel (such as microchip and SiFive board DTs) have the E-core disabled.
I had discovered this issue in a totally different context after the OpenSBI 1.3 release happened. This issue is already fixed in the latest OpenSBI by the following commit c6a35733b74aeff612398f274ed19a74f81d1f37 ("lib: utils:
Fix sbi_hartid_to_scratch() usage in ACLINT drivers").
Great, thanks Anup! I thought I had tested tip-of-tree too, but obviously not.
Quoted 2 lines
I always assumed that Microchip hss.bin is the preferred BIOS for the QEMU microchip-icicle-kit machine but I guess that's not true.
Unfortunately the HSS has not worked in QEMU for a long time, and while I would love to fix it, but am pretty stretched for spare time to begin with.
I usually just do direct kernel boots, which use the OpenSBI that comes with QEMU, as I am sure you already know :)

Quoted 7 lines

Quoted 2 lines
Quoted 2 lines
At this point, you can either:
1) Use latest OpenSBI on QEMU microchip-icicle-kit machine
I forgot to reply to this point, wondering what should be done with QEMU. Bumping to v1.3 in QEMU introduces a regression here, regardless of whether I can go and build a fixed version of OpenSBI.

FYI: The no-map fix went in OpenSBI v1.3. Without the upgrade, any user using the latest kernel (> v6.4)
may hit those random linear map related issues (in hibernation or EFI booting path).

There are three possible scenarios:

1. Upgrade to OpenSBI v1.3: Any user of microchip-icicle-kit machine or sifive fu540 machine users
may hit this issue if the device tree has the disabled hart (e core).
2. No upgrade to OpenSBI v1.2. Any user using hibernation or UEFI may have issues [1]
3. Include a non-release version OpenSBI in Qemu with the fix as an exception.

#3 probably deviates from policy and sets a bad precedent. So I am not advocating for it though ;)
For both #1 & #2, the solution would be to use the latest OpenSBI in -bios argument instead of the stock one.
I could be wrong but my guess is the number of users facing #2 would be higher than #1.

[1] https://lore.kernel.org/linux-riscv/20230625140931.1266216-1-songshuaishuai at tinylab.org/

Quoted 11 lines

Quoted 5 lines
Quoted 2 lines
2) Ensure CPU0 DT node is enabled in DT when booting on QEMU microchip-icicle-kit machine with OpenSBI 1.3
Will OpenSBI disable it? If not, I think option 2) needs to be remove the DT node. I'll just use tip-of-tree myself & up to the
Clearly didn't finish this comment. It was meant to say "up to the QEMU maintainers what they want to do on the QEMU side of things".
Thanks,
Conor.

Signature · 2 lines

Regards,
Atish

Alistair Francis

2023-07-19 01:32 UTC #25 lore raw

Boot failure after QEMU's upgrade to OpenSBI v1.3 (was Re: [PATCH for-8.2 6/7] target/riscv: add 'max' CPU type)

Quoted 74 lines · On Sat, Jul 15, 2023 at 7:14?PM Atish Patra <[email protected]> wrote:

Quoted 51 lines · On Fri, Jul 14, 2023 at 5:29?AM Conor Dooley <[email protected]> wrote:
Quoted 49 lines · Conor Dooley
Quoted 26 lines · Anup Patel
Quoted 18 lines
Quoted 18 lines
OpenSBI v1.3
____ _____ ____ _____
  / __ \                  / ____|  _ \_   _|
 | |  | |_ __   ___ _ __ | (___ | |_) || |
 | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
 | |__| | |_) |  __/ | | |____) | |_) || |_
  \____/| .__/ \___|_| |_|_____/|___/_____|
        | |
        |_|
init_coldboot: ipi init failed (error -1009)
Just to note, because we use our own firmware that vendors in OpenSBI and compiles only a significantly cut down number of files from it, we do not use the fw_dynamic etc flow on our hardware. As a result, we have not tested v1.3, nor do we have any immediate plans to change our platform firmware to vendor v1.3 either.
Quoted 4 lines
Quoted 4 lines
I unless there's something obvious to you, it sounds like I will need to go and bisect OpenSBI. That's a job for another day though, given the time.
The real issue is some CPU/HART DT nodes marked as disabled in the DT passed to OpenSBI 1.3.
Quoted 8 lines
This issue does not exist in any of the DTs generated by QEMU but some of the DTs in the kernel (such as microchip and SiFive board DTs) have the E-core disabled.
I had discovered this issue in a totally different context after the OpenSBI 1.3 release happened. This issue is already fixed in the latest OpenSBI by the following commit c6a35733b74aeff612398f274ed19a74f81d1f37 ("lib: utils:
Fix sbi_hartid_to_scratch() usage in ACLINT drivers").
Great, thanks Anup! I thought I had tested tip-of-tree too, but obviously not.
Quoted 2 lines
I always assumed that Microchip hss.bin is the preferred BIOS for the QEMU microchip-icicle-kit machine but I guess that's not true.
Unfortunately the HSS has not worked in QEMU for a long time, and while I would love to fix it, but am pretty stretched for spare time to begin with.
I usually just do direct kernel boots, which use the OpenSBI that comes with QEMU, as I am sure you already know :)
Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
At this point, you can either:
1) Use latest OpenSBI on QEMU microchip-icicle-kit machine
I forgot to reply to this point, wondering what should be done with QEMU. Bumping to v1.3 in QEMU introduces a regression here, regardless of whether I can go and build a fixed version of OpenSBI.
FYI: The no-map fix went in OpenSBI v1.3. Without the upgrade, any user using the latest kernel (> v6.4)
may hit those random linear map related issues (in hibernation or EFI booting path).
There are three possible scenarios:
1. Upgrade to OpenSBI v1.3: Any user of microchip-icicle-kit machine or sifive fu540 machine users
may hit this issue if the device tree has the disabled hart (e core).
2. No upgrade to OpenSBI v1.2. Any user using hibernation or UEFI may have issues [1]
3. Include a non-release version OpenSBI in Qemu with the fix as an exception.

Quoted 6 lines

#3 probably deviates from policy and sets a bad precedent. So I am not advocating for it though ;)
For both #1 & #2, the solution would be to use the latest OpenSBI in -bios argument instead of the stock one.
I could be wrong but my guess is the number of users facing #2 would be higher than #1.

Thanks for that info Atish!

We are stuck in a bad situation.

The best solution would be if OpenSBI can release a 1.3.1, @Anup Patel do you think you could do that?

Otherwise I think we should stick with OpenSBI 1.3. Considering that it fixes UEFI boot issues for the virt board (which would be the most used) it seems like a best call to make. People using the other boards are unfortunately stuck building their own OpenSBI release.

If there is no OpenSBI 1.3.1 release we should add something to the release notes. @Conor Dooley are you able to give a clear sentence on how the boot fails?

Alistair

Quoted 20 lines

[1] https://lore.kernel.org/linux-riscv/20230625140931.1266216-1-songshuaishuai at tinylab.org/
Quoted 11 lines
Quoted 5 lines
Quoted 2 lines
2) Ensure CPU0 DT node is enabled in DT when booting on QEMU microchip-icicle-kit machine with OpenSBI 1.3
Will OpenSBI disable it? If not, I think option 2) needs to be remove the DT node. I'll just use tip-of-tree myself & up to the
Clearly didn't finish this comment. It was meant to say "up to the QEMU maintainers what they want to do on the QEMU side of things".
Thanks,
Conor.
--
Regards,
Atish

Anup Patel

2023-07-19 05:39 UTC #26 lore raw

Boot failure after QEMU's upgrade to OpenSBI v1.3 (was Re: [PATCH for-8.2 6/7] target/riscv: add 'max' CPU type)

Quoted 83 lines · On Wed, Jul 19, 2023 at 7:03?AM Alistair Francis <[email protected]> wrote:

Quoted 74 lines · On Sat, Jul 15, 2023 at 7:14?PM Atish Patra <[email protected]> wrote:
Quoted 51 lines · On Fri, Jul 14, 2023 at 5:29?AM Conor Dooley <[email protected]> wrote:
Quoted 49 lines · Conor Dooley
Quoted 26 lines · Anup Patel
Quoted 18 lines
Quoted 18 lines
OpenSBI v1.3
____ _____ ____ _____
  / __ \                  / ____|  _ \_   _|
 | |  | |_ __   ___ _ __ | (___ | |_) || |
 | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
 | |__| | |_) |  __/ | | |____) | |_) || |_
  \____/| .__/ \___|_| |_|_____/|___/_____|
        | |
        |_|
init_coldboot: ipi init failed (error -1009)
Just to note, because we use our own firmware that vendors in OpenSBI and compiles only a significantly cut down number of files from it, we do not use the fw_dynamic etc flow on our hardware. As a result, we have not tested v1.3, nor do we have any immediate plans to change our platform firmware to vendor v1.3 either.
Quoted 4 lines
Quoted 4 lines
I unless there's something obvious to you, it sounds like I will need to go and bisect OpenSBI. That's a job for another day though, given the time.
The real issue is some CPU/HART DT nodes marked as disabled in the DT passed to OpenSBI 1.3.
Quoted 8 lines
This issue does not exist in any of the DTs generated by QEMU but some of the DTs in the kernel (such as microchip and SiFive board DTs) have the E-core disabled.
I had discovered this issue in a totally different context after the OpenSBI 1.3 release happened. This issue is already fixed in the latest OpenSBI by the following commit c6a35733b74aeff612398f274ed19a74f81d1f37 ("lib: utils:
Fix sbi_hartid_to_scratch() usage in ACLINT drivers").
Great, thanks Anup! I thought I had tested tip-of-tree too, but obviously not.
Quoted 2 lines
I always assumed that Microchip hss.bin is the preferred BIOS for the QEMU microchip-icicle-kit machine but I guess that's not true.
Unfortunately the HSS has not worked in QEMU for a long time, and while I would love to fix it, but am pretty stretched for spare time to begin with.
I usually just do direct kernel boots, which use the OpenSBI that comes with QEMU, as I am sure you already know :)
Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
At this point, you can either:
1) Use latest OpenSBI on QEMU microchip-icicle-kit machine
I forgot to reply to this point, wondering what should be done with QEMU. Bumping to v1.3 in QEMU introduces a regression here, regardless of whether I can go and build a fixed version of OpenSBI.
FYI: The no-map fix went in OpenSBI v1.3. Without the upgrade, any user using the latest kernel (> v6.4)
may hit those random linear map related issues (in hibernation or EFI booting path).
There are three possible scenarios:
1. Upgrade to OpenSBI v1.3: Any user of microchip-icicle-kit machine or sifive fu540 machine users
may hit this issue if the device tree has the disabled hart (e core).
2. No upgrade to OpenSBI v1.2. Any user using hibernation or UEFI may have issues [1]
3. Include a non-release version OpenSBI in Qemu with the fix as an exception.
Quoted 6 lines
#3 probably deviates from policy and sets a bad precedent. So I am not advocating for it though ;)
For both #1 & #2, the solution would be to use the latest OpenSBI in -bios argument instead of the stock one.
I could be wrong but my guess is the number of users facing #2 would be higher than #1.

Quoted 6 lines

Thanks for that info Atish!
We are stuck in a bad situation.
The best solution would be if OpenSBI can release a 1.3.1, @Anup Patel do you think you could do that?

OpenSBI has a major number and minor number in the version but it does not have release/patch number so best would be to treat OpenSBI vX.Y.Z as bug fixes on-top-of OpenSBI vX.Y. In other words, supervisor software won't be able to differentiate between OpenSBI vX.Y.Z and OpenSBI vX.Y using sbi_get_impl_version().

There are only three commits between the ACLINT fix and OpenSBI v1.3 so as one-of case I will go ahead create OpenSBI v1.3.1 containing only four commits on-top of OpenSBI v1.3

Does this sound okay ?

Quoted 6 lines

Otherwise I think we should stick with OpenSBI 1.3. Considering that it fixes UEFI boot issues for the virt board (which would be the most used) it seems like a best call to make. People using the other boards are unfortunately stuck building their own OpenSBI release.

Quoted 26 lines

If there is no OpenSBI 1.3.1 release we should add something to the release notes. @Conor Dooley are you able to give a clear sentence on how the boot fails?
Alistair
Quoted 20 lines
[1] https://lore.kernel.org/linux-riscv/20230625140931.1266216-1-songshuaishuai at tinylab.org/
Quoted 11 lines
Quoted 5 lines
Quoted 2 lines
2) Ensure CPU0 DT node is enabled in DT when booting on QEMU microchip-icicle-kit machine with OpenSBI 1.3
Will OpenSBI disable it? If not, I think option 2) needs to be remove the DT node. I'll just use tip-of-tree myself & up to the
Clearly didn't finish this comment. It was meant to say "up to the QEMU maintainers what they want to do on the QEMU side of things".
Thanks,
Conor.
--
Regards,
Atish

Regards,
Anup

Alistair Francis

2023-07-19 09:53 UTC #27 lore raw

Boot failure after QEMU's upgrade to OpenSBI v1.3 (was Re: [PATCH for-8.2 6/7] target/riscv: add 'max' CPU type)

Quoted 98 lines · On Wed, Jul 19, 2023 at 3:39?PM Anup Patel <[email protected]> wrote:

Quoted 83 lines · On Wed, Jul 19, 2023 at 7:03?AM Alistair Francis <[email protected]> wrote:
Quoted 74 lines · On Sat, Jul 15, 2023 at 7:14?PM Atish Patra <[email protected]> wrote:
Quoted 51 lines · On Fri, Jul 14, 2023 at 5:29?AM Conor Dooley <[email protected]> wrote:
Quoted 49 lines · Conor Dooley
Quoted 26 lines · Anup Patel
Quoted 18 lines
Quoted 18 lines
OpenSBI v1.3
____ _____ ____ _____
  / __ \                  / ____|  _ \_   _|
 | |  | |_ __   ___ _ __ | (___ | |_) || |
 | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
 | |__| | |_) |  __/ | | |____) | |_) || |_
  \____/| .__/ \___|_| |_|_____/|___/_____|
        | |
        |_|
init_coldboot: ipi init failed (error -1009)
Just to note, because we use our own firmware that vendors in OpenSBI and compiles only a significantly cut down number of files from it, we do not use the fw_dynamic etc flow on our hardware. As a result, we have not tested v1.3, nor do we have any immediate plans to change our platform firmware to vendor v1.3 either.
Quoted 4 lines
Quoted 4 lines
I unless there's something obvious to you, it sounds like I will need to go and bisect OpenSBI. That's a job for another day though, given the time.
The real issue is some CPU/HART DT nodes marked as disabled in the DT passed to OpenSBI 1.3.
Quoted 8 lines
This issue does not exist in any of the DTs generated by QEMU but some of the DTs in the kernel (such as microchip and SiFive board DTs) have the E-core disabled.
I had discovered this issue in a totally different context after the OpenSBI 1.3 release happened. This issue is already fixed in the latest OpenSBI by the following commit c6a35733b74aeff612398f274ed19a74f81d1f37 ("lib: utils:
Fix sbi_hartid_to_scratch() usage in ACLINT drivers").
Great, thanks Anup! I thought I had tested tip-of-tree too, but obviously not.
Quoted 2 lines
I always assumed that Microchip hss.bin is the preferred BIOS for the QEMU microchip-icicle-kit machine but I guess that's not true.
Unfortunately the HSS has not worked in QEMU for a long time, and while I would love to fix it, but am pretty stretched for spare time to begin with.
I usually just do direct kernel boots, which use the OpenSBI that comes with QEMU, as I am sure you already know :)
Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
At this point, you can either:
1) Use latest OpenSBI on QEMU microchip-icicle-kit machine
I forgot to reply to this point, wondering what should be done with QEMU. Bumping to v1.3 in QEMU introduces a regression here, regardless of whether I can go and build a fixed version of OpenSBI.
FYI: The no-map fix went in OpenSBI v1.3. Without the upgrade, any user using the latest kernel (> v6.4)
may hit those random linear map related issues (in hibernation or EFI booting path).
There are three possible scenarios:
1. Upgrade to OpenSBI v1.3: Any user of microchip-icicle-kit machine or sifive fu540 machine users
may hit this issue if the device tree has the disabled hart (e core).
2. No upgrade to OpenSBI v1.2. Any user using hibernation or UEFI may have issues [1]
3. Include a non-release version OpenSBI in Qemu with the fix as an exception.
Quoted 6 lines
#3 probably deviates from policy and sets a bad precedent. So I am not advocating for it though ;)
For both #1 & #2, the solution would be to use the latest OpenSBI in -bios argument instead of the stock one.
I could be wrong but my guess is the number of users facing #2 would be higher than #1.
Quoted 6 lines
Thanks for that info Atish!
We are stuck in a bad situation.
The best solution would be if OpenSBI can release a 1.3.1, @Anup Patel do you think you could do that?
OpenSBI has a major number and minor number in the version but it does not have release/patch number so best would be to treat OpenSBI vX.Y.Z as bug fixes on-top-of OpenSBI vX.Y. In other words, supervisor software won't be able to differentiate between OpenSBI vX.Y.Z and OpenSBI vX.Y using sbi_get_impl_version().

Quoted 5 lines

There are only three commits between the ACLINT fix and OpenSBI v1.3 so as one-of case I will go ahead create OpenSBI v1.3.1 containing only four commits on-top of OpenSBI v1.3
Does this sound okay ?

That sounds fine to me. It fixes the issue for the Microsemi board and it's a very small change between 1.3 and 1.3.1

Alistair

Quoted 36 lines

Quoted 7 lines
Otherwise I think we should stick with OpenSBI 1.3. Considering that it fixes UEFI boot issues for the virt board (which would be the most used) it seems like a best call to make. People using the other boards are unfortunately stuck building their own OpenSBI release.
Quoted 26 lines
If there is no OpenSBI 1.3.1 release we should add something to the release notes. @Conor Dooley are you able to give a clear sentence on how the boot fails?
Alistair
Quoted 20 lines
[1] https://lore.kernel.org/linux-riscv/20230625140931.1266216-1-songshuaishuai at tinylab.org/
Quoted 11 lines
Quoted 5 lines
Quoted 2 lines
2) Ensure CPU0 DT node is enabled in DT when booting on QEMU microchip-icicle-kit machine with OpenSBI 1.3
Will OpenSBI disable it? If not, I think option 2) needs to be remove the DT node. I'll just use tip-of-tree myself & up to the
Clearly didn't finish this comment. It was meant to say "up to the QEMU maintainers what they want to do on the QEMU side of things".
Thanks,
Conor.
--
Regards,
Atish
Regards,
Anup

Anup Patel

2023-07-19 15:21 UTC #28 lore raw

Boot failure after QEMU's upgrade to OpenSBI v1.3 (was Re: [PATCH for-8.2 6/7] target/riscv: add 'max' CPU type)

Quoted 100 lines · On Wed, Jul 19, 2023 at 3:23?PM Alistair Francis <[email protected]> wrote:

Quoted 98 lines · On Wed, Jul 19, 2023 at 3:39?PM Anup Patel <[email protected]> wrote:
Quoted 83 lines · On Wed, Jul 19, 2023 at 7:03?AM Alistair Francis <[email protected]> wrote:
Quoted 74 lines · On Sat, Jul 15, 2023 at 7:14?PM Atish Patra <[email protected]> wrote:
Quoted 51 lines · On Fri, Jul 14, 2023 at 5:29?AM Conor Dooley <[email protected]> wrote:
Quoted 49 lines · Conor Dooley
Quoted 26 lines · Anup Patel
Quoted 18 lines
Quoted 18 lines
OpenSBI v1.3
____ _____ ____ _____
  / __ \                  / ____|  _ \_   _|
 | |  | |_ __   ___ _ __ | (___ | |_) || |
 | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
 | |__| | |_) |  __/ | | |____) | |_) || |_
  \____/| .__/ \___|_| |_|_____/|___/_____|
        | |
        |_|
init_coldboot: ipi init failed (error -1009)
Just to note, because we use our own firmware that vendors in OpenSBI and compiles only a significantly cut down number of files from it, we do not use the fw_dynamic etc flow on our hardware. As a result, we have not tested v1.3, nor do we have any immediate plans to change our platform firmware to vendor v1.3 either.
Quoted 4 lines
Quoted 4 lines
I unless there's something obvious to you, it sounds like I will need to go and bisect OpenSBI. That's a job for another day though, given the time.
The real issue is some CPU/HART DT nodes marked as disabled in the DT passed to OpenSBI 1.3.
Quoted 8 lines
This issue does not exist in any of the DTs generated by QEMU but some of the DTs in the kernel (such as microchip and SiFive board DTs) have the E-core disabled.
I had discovered this issue in a totally different context after the OpenSBI 1.3 release happened. This issue is already fixed in the latest OpenSBI by the following commit c6a35733b74aeff612398f274ed19a74f81d1f37 ("lib: utils:
Fix sbi_hartid_to_scratch() usage in ACLINT drivers").
Great, thanks Anup! I thought I had tested tip-of-tree too, but obviously not.
Quoted 2 lines
I always assumed that Microchip hss.bin is the preferred BIOS for the QEMU microchip-icicle-kit machine but I guess that's not true.
Unfortunately the HSS has not worked in QEMU for a long time, and while I would love to fix it, but am pretty stretched for spare time to begin with.
I usually just do direct kernel boots, which use the OpenSBI that comes with QEMU, as I am sure you already know :)
Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
At this point, you can either:
1) Use latest OpenSBI on QEMU microchip-icicle-kit machine
I forgot to reply to this point, wondering what should be done with QEMU. Bumping to v1.3 in QEMU introduces a regression here, regardless of whether I can go and build a fixed version of OpenSBI.
FYI: The no-map fix went in OpenSBI v1.3. Without the upgrade, any user using the latest kernel (> v6.4)
may hit those random linear map related issues (in hibernation or EFI booting path).
There are three possible scenarios:
1. Upgrade to OpenSBI v1.3: Any user of microchip-icicle-kit machine or sifive fu540 machine users
may hit this issue if the device tree has the disabled hart (e core).
2. No upgrade to OpenSBI v1.2. Any user using hibernation or UEFI may have issues [1]
3. Include a non-release version OpenSBI in Qemu with the fix as an exception.
Quoted 6 lines
#3 probably deviates from policy and sets a bad precedent. So I am not advocating for it though ;)
For both #1 & #2, the solution would be to use the latest OpenSBI in -bios argument instead of the stock one.
I could be wrong but my guess is the number of users facing #2 would be higher than #1.
Quoted 6 lines
Thanks for that info Atish!
We are stuck in a bad situation.
The best solution would be if OpenSBI can release a 1.3.1, @Anup Patel do you think you could do that?
OpenSBI has a major number and minor number in the version but it does not have release/patch number so best would be to treat OpenSBI vX.Y.Z as bug fixes on-top-of OpenSBI vX.Y. In other words, supervisor software won't be able to differentiate between OpenSBI vX.Y.Z and OpenSBI vX.Y using sbi_get_impl_version().

Quoted 8 lines

Quoted 5 lines
There are only three commits between the ACLINT fix and OpenSBI v1.3 so as one-of case I will go ahead create OpenSBI v1.3.1 containing only four commits on-top of OpenSBI v1.3
Does this sound okay ?
That sounds fine to me. It fixes the issue for the Microsemi board and it's a very small change between 1.3 and 1.3.1

Please check
https://github.com/riscv-software-src/opensbi/releases/tag/v1.3.1

I hope this helps.

Regards,
Anup

Quoted 39 lines

Alistair
Quoted 36 lines
Quoted 7 lines
Otherwise I think we should stick with OpenSBI 1.3. Considering that it fixes UEFI boot issues for the virt board (which would be the most used) it seems like a best call to make. People using the other boards are unfortunately stuck building their own OpenSBI release.
Quoted 26 lines
If there is no OpenSBI 1.3.1 release we should add something to the release notes. @Conor Dooley are you able to give a clear sentence on how the boot fails?
Alistair
Quoted 20 lines
[1] https://lore.kernel.org/linux-riscv/20230625140931.1266216-1-songshuaishuai at tinylab.org/
Quoted 11 lines
Quoted 5 lines
Quoted 2 lines
2) Ensure CPU0 DT node is enabled in DT when booting on QEMU microchip-icicle-kit machine with OpenSBI 1.3
Will OpenSBI disable it? If not, I think option 2) needs to be remove the DT node. I'll just use tip-of-tree myself & up to the
Clearly didn't finish this comment. It was meant to say "up to the QEMU maintainers what they want to do on the QEMU side of things".
Thanks,
Conor.
--
Regards,
Atish
Regards,
Anup

Bin Meng

2023-07-19 15:45 UTC #29 lore raw

Boot failure after QEMU's upgrade to OpenSBI v1.3 (was Re: [PATCH for-8.2 6/7] target/riscv: add 'max' CPU type)

Quoted 108 lines · On Wed, Jul 19, 2023 at 11:22?PM Anup Patel <[email protected]> wrote:

Quoted 106 lines · On Wed, Jul 19, 2023 at 3:23?PM Alistair Francis <[email protected]> wrote:
Quoted 103 lines · On Wed, Jul 19, 2023 at 3:39?PM Anup Patel <[email protected]> wrote:
Quoted 83 lines · On Wed, Jul 19, 2023 at 7:03?AM Alistair Francis <[email protected]> wrote:
Quoted 74 lines · On Sat, Jul 15, 2023 at 7:14?PM Atish Patra <[email protected]> wrote:
Quoted 51 lines · On Fri, Jul 14, 2023 at 5:29?AM Conor Dooley <[email protected]> wrote:
Quoted 49 lines · Conor Dooley
Quoted 26 lines · Anup Patel
Quoted 18 lines
Quoted 18 lines
OpenSBI v1.3
____ _____ ____ _____
  / __ \                  / ____|  _ \_   _|
 | |  | |_ __   ___ _ __ | (___ | |_) || |
 | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
 | |__| | |_) |  __/ | | |____) | |_) || |_
  \____/| .__/ \___|_| |_|_____/|___/_____|
        | |
        |_|
init_coldboot: ipi init failed (error -1009)
Just to note, because we use our own firmware that vendors in OpenSBI and compiles only a significantly cut down number of files from it, we do not use the fw_dynamic etc flow on our hardware. As a result, we have not tested v1.3, nor do we have any immediate plans to change our platform firmware to vendor v1.3 either.
Quoted 4 lines
Quoted 4 lines
I unless there's something obvious to you, it sounds like I will need to go and bisect OpenSBI. That's a job for another day though, given the time.
The real issue is some CPU/HART DT nodes marked as disabled in the DT passed to OpenSBI 1.3.
Quoted 8 lines
This issue does not exist in any of the DTs generated by QEMU but some of the DTs in the kernel (such as microchip and SiFive board DTs) have the E-core disabled.
I had discovered this issue in a totally different context after the OpenSBI 1.3 release happened. This issue is already fixed in the latest OpenSBI by the following commit c6a35733b74aeff612398f274ed19a74f81d1f37 ("lib: utils:
Fix sbi_hartid_to_scratch() usage in ACLINT drivers").
Great, thanks Anup! I thought I had tested tip-of-tree too, but obviously not.
Quoted 2 lines
I always assumed that Microchip hss.bin is the preferred BIOS for the QEMU microchip-icicle-kit machine but I guess that's not true.
Unfortunately the HSS has not worked in QEMU for a long time, and while I would love to fix it, but am pretty stretched for spare time to begin with.
I usually just do direct kernel boots, which use the OpenSBI that comes with QEMU, as I am sure you already know :)
Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
At this point, you can either:
1) Use latest OpenSBI on QEMU microchip-icicle-kit machine
I forgot to reply to this point, wondering what should be done with QEMU. Bumping to v1.3 in QEMU introduces a regression here, regardless of whether I can go and build a fixed version of OpenSBI.
FYI: The no-map fix went in OpenSBI v1.3. Without the upgrade, any user using the latest kernel (> v6.4)
may hit those random linear map related issues (in hibernation or EFI booting path).
There are three possible scenarios:
1. Upgrade to OpenSBI v1.3: Any user of microchip-icicle-kit machine or sifive fu540 machine users
may hit this issue if the device tree has the disabled hart (e core).
2. No upgrade to OpenSBI v1.2. Any user using hibernation or UEFI may have issues [1]
3. Include a non-release version OpenSBI in Qemu with the fix as an exception.
Quoted 6 lines
#3 probably deviates from policy and sets a bad precedent. So I am not advocating for it though ;)
For both #1 & #2, the solution would be to use the latest OpenSBI in -bios argument instead of the stock one.
I could be wrong but my guess is the number of users facing #2 would be higher than #1.
Quoted 6 lines
Thanks for that info Atish!
We are stuck in a bad situation.
The best solution would be if OpenSBI can release a 1.3.1, @Anup Patel do you think you could do that?
OpenSBI has a major number and minor number in the version but it does not have release/patch number so best would be to treat OpenSBI vX.Y.Z as bug fixes on-top-of OpenSBI vX.Y. In other words, supervisor software won't be able to differentiate between OpenSBI vX.Y.Z and OpenSBI vX.Y using sbi_get_impl_version().
There are only three commits between the ACLINT fix and OpenSBI v1.3 so as one-of case I will go ahead create OpenSBI v1.3.1 containing only four commits on-top of OpenSBI v1.3
Does this sound okay ?

Quoted 7 lines

Quoted 2 lines
That sounds fine to me. It fixes the issue for the Microsemi board and it's a very small change between 1.3 and 1.3.1
Please check
https://github.com/riscv-software-src/opensbi/releases/tag/v1.3.1
I hope this helps.

Hi Alistair,

Do we need to update QEMU's opensbi binaries to v1.3.1?

Hi Anup,

Somehow I cannot see the 'tag' v1.3.1 being populated in the opensbi git repo. Am I missing anything?

Regards,
Bin

Anup Patel

2023-07-19 16:10 UTC #30 lore raw

Boot failure after QEMU's upgrade to OpenSBI v1.3 (was Re: [PATCH for-8.2 6/7] target/riscv: add 'max' CPU type)

Hi Bin,

Quoted 118 lines · On Wed, Jul 19, 2023 at 9:15?PM Bin Meng <[email protected]> wrote:

Quoted 108 lines · On Wed, Jul 19, 2023 at 11:22?PM Anup Patel <[email protected]> wrote:
Quoted 106 lines · On Wed, Jul 19, 2023 at 3:23?PM Alistair Francis <[email protected]> wrote:
Quoted 103 lines · On Wed, Jul 19, 2023 at 3:39?PM Anup Patel <[email protected]> wrote:
Quoted 83 lines · On Wed, Jul 19, 2023 at 7:03?AM Alistair Francis <[email protected]> wrote:
Quoted 74 lines · On Sat, Jul 15, 2023 at 7:14?PM Atish Patra <[email protected]> wrote:
Quoted 51 lines · On Fri, Jul 14, 2023 at 5:29?AM Conor Dooley <[email protected]> wrote:
Quoted 49 lines · Conor Dooley
Quoted 26 lines · Anup Patel
Quoted 18 lines
Quoted 18 lines
OpenSBI v1.3
____ _____ ____ _____
  / __ \                  / ____|  _ \_   _|
 | |  | |_ __   ___ _ __ | (___ | |_) || |
 | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
 | |__| | |_) |  __/ | | |____) | |_) || |_
  \____/| .__/ \___|_| |_|_____/|___/_____|
        | |
        |_|
init_coldboot: ipi init failed (error -1009)
Just to note, because we use our own firmware that vendors in OpenSBI and compiles only a significantly cut down number of files from it, we do not use the fw_dynamic etc flow on our hardware. As a result, we have not tested v1.3, nor do we have any immediate plans to change our platform firmware to vendor v1.3 either.
Quoted 4 lines
Quoted 4 lines
I unless there's something obvious to you, it sounds like I will need to go and bisect OpenSBI. That's a job for another day though, given the time.
The real issue is some CPU/HART DT nodes marked as disabled in the DT passed to OpenSBI 1.3.
Quoted 8 lines
This issue does not exist in any of the DTs generated by QEMU but some of the DTs in the kernel (such as microchip and SiFive board DTs) have the E-core disabled.
I had discovered this issue in a totally different context after the OpenSBI 1.3 release happened. This issue is already fixed in the latest OpenSBI by the following commit c6a35733b74aeff612398f274ed19a74f81d1f37 ("lib: utils:
Fix sbi_hartid_to_scratch() usage in ACLINT drivers").
Great, thanks Anup! I thought I had tested tip-of-tree too, but obviously not.
Quoted 2 lines
I always assumed that Microchip hss.bin is the preferred BIOS for the QEMU microchip-icicle-kit machine but I guess that's not true.
Unfortunately the HSS has not worked in QEMU for a long time, and while I would love to fix it, but am pretty stretched for spare time to begin with.
I usually just do direct kernel boots, which use the OpenSBI that comes with QEMU, as I am sure you already know :)
Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
At this point, you can either:
1) Use latest OpenSBI on QEMU microchip-icicle-kit machine
I forgot to reply to this point, wondering what should be done with QEMU. Bumping to v1.3 in QEMU introduces a regression here, regardless of whether I can go and build a fixed version of OpenSBI.
FYI: The no-map fix went in OpenSBI v1.3. Without the upgrade, any user using the latest kernel (> v6.4)
may hit those random linear map related issues (in hibernation or EFI booting path).
There are three possible scenarios:
1. Upgrade to OpenSBI v1.3: Any user of microchip-icicle-kit machine or sifive fu540 machine users
may hit this issue if the device tree has the disabled hart (e core).
2. No upgrade to OpenSBI v1.2. Any user using hibernation or UEFI may have issues [1]
3. Include a non-release version OpenSBI in Qemu with the fix as an exception.
Quoted 6 lines
#3 probably deviates from policy and sets a bad precedent. So I am not advocating for it though ;)
For both #1 & #2, the solution would be to use the latest OpenSBI in -bios argument instead of the stock one.
I could be wrong but my guess is the number of users facing #2 would be higher than #1.
Quoted 6 lines
Thanks for that info Atish!
We are stuck in a bad situation.
The best solution would be if OpenSBI can release a 1.3.1, @Anup Patel do you think you could do that?
OpenSBI has a major number and minor number in the version but it does not have release/patch number so best would be to treat OpenSBI vX.Y.Z as bug fixes on-top-of OpenSBI vX.Y. In other words, supervisor software won't be able to differentiate between OpenSBI vX.Y.Z and OpenSBI vX.Y using sbi_get_impl_version().
There are only three commits between the ACLINT fix and OpenSBI v1.3 so as one-of case I will go ahead create OpenSBI v1.3.1 containing only four commits on-top of OpenSBI v1.3
Does this sound okay ?
Quoted 7 lines
Quoted 2 lines
That sounds fine to me. It fixes the issue for the Microsemi board and it's a very small change between 1.3 and 1.3.1
Please check
https://github.com/riscv-software-src/opensbi/releases/tag/v1.3.1
I hope this helps.

Quoted 8 lines

Hi Alistair,
Do we need to update QEMU's opensbi binaries to v1.3.1?
Hi Anup,
Somehow I cannot see the 'tag' v1.3.1 being populated in the opensbi git repo. Am I missing anything?

There is a v1.3.1 tag in https://github.com/riscv-software-src/opensbi
(Try cloning the repo again?)

The commit history of v1.3.1 is v1.3 tag + 5 cherry picked commits which means the commit history of the master branch is not the same as the commit history of v1.3.1.

Regards,
Anup

Bin Meng

2023-07-19 16:18 UTC #31 lore raw

Boot failure after QEMU's upgrade to OpenSBI v1.3 (was Re: [PATCH for-8.2 6/7] target/riscv: add 'max' CPU type)

Hi Anup,

Quoted 131 lines · On Thu, Jul 20, 2023 at 12:10?AM Anup Patel <[email protected]> wrote:

Hi Bin,
Quoted 118 lines · On Wed, Jul 19, 2023 at 9:15?PM Bin Meng <[email protected]> wrote:
Quoted 108 lines · On Wed, Jul 19, 2023 at 11:22?PM Anup Patel <[email protected]> wrote:
Quoted 106 lines · On Wed, Jul 19, 2023 at 3:23?PM Alistair Francis <[email protected]> wrote:
Quoted 103 lines · On Wed, Jul 19, 2023 at 3:39?PM Anup Patel <[email protected]> wrote:
Quoted 83 lines · On Wed, Jul 19, 2023 at 7:03?AM Alistair Francis <[email protected]> wrote:
Quoted 74 lines · On Sat, Jul 15, 2023 at 7:14?PM Atish Patra <[email protected]> wrote:
Quoted 51 lines · On Fri, Jul 14, 2023 at 5:29?AM Conor Dooley <[email protected]> wrote:
Quoted 49 lines · Conor Dooley
Quoted 26 lines · Anup Patel
Quoted 18 lines
Quoted 18 lines
OpenSBI v1.3
____ _____ ____ _____
  / __ \                  / ____|  _ \_   _|
 | |  | |_ __   ___ _ __ | (___ | |_) || |
 | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
 | |__| | |_) |  __/ | | |____) | |_) || |_
  \____/| .__/ \___|_| |_|_____/|___/_____|
        | |
        |_|
init_coldboot: ipi init failed (error -1009)
Just to note, because we use our own firmware that vendors in OpenSBI and compiles only a significantly cut down number of files from it, we do not use the fw_dynamic etc flow on our hardware. As a result, we have not tested v1.3, nor do we have any immediate plans to change our platform firmware to vendor v1.3 either.
Quoted 4 lines
Quoted 4 lines
I unless there's something obvious to you, it sounds like I will need to go and bisect OpenSBI. That's a job for another day though, given the time.
The real issue is some CPU/HART DT nodes marked as disabled in the DT passed to OpenSBI 1.3.
Quoted 8 lines
This issue does not exist in any of the DTs generated by QEMU but some of the DTs in the kernel (such as microchip and SiFive board DTs) have the E-core disabled.
I had discovered this issue in a totally different context after the OpenSBI 1.3 release happened. This issue is already fixed in the latest OpenSBI by the following commit c6a35733b74aeff612398f274ed19a74f81d1f37 ("lib: utils:
Fix sbi_hartid_to_scratch() usage in ACLINT drivers").
Great, thanks Anup! I thought I had tested tip-of-tree too, but obviously not.
Quoted 2 lines
I always assumed that Microchip hss.bin is the preferred BIOS for the QEMU microchip-icicle-kit machine but I guess that's not true.
Unfortunately the HSS has not worked in QEMU for a long time, and while I would love to fix it, but am pretty stretched for spare time to begin with.
I usually just do direct kernel boots, which use the OpenSBI that comes with QEMU, as I am sure you already know :)
Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
At this point, you can either:
1) Use latest OpenSBI on QEMU microchip-icicle-kit machine
I forgot to reply to this point, wondering what should be done with QEMU. Bumping to v1.3 in QEMU introduces a regression here, regardless of whether I can go and build a fixed version of OpenSBI.
FYI: The no-map fix went in OpenSBI v1.3. Without the upgrade, any user using the latest kernel (> v6.4)
may hit those random linear map related issues (in hibernation or EFI booting path).
There are three possible scenarios:
1. Upgrade to OpenSBI v1.3: Any user of microchip-icicle-kit machine or sifive fu540 machine users
may hit this issue if the device tree has the disabled hart (e core).
2. No upgrade to OpenSBI v1.2. Any user using hibernation or UEFI may have issues [1]
3. Include a non-release version OpenSBI in Qemu with the fix as an exception.
Quoted 6 lines
#3 probably deviates from policy and sets a bad precedent. So I am not advocating for it though ;)
For both #1 & #2, the solution would be to use the latest OpenSBI in -bios argument instead of the stock one.
I could be wrong but my guess is the number of users facing #2 would be higher than #1.
Quoted 6 lines
Thanks for that info Atish!
We are stuck in a bad situation.
The best solution would be if OpenSBI can release a 1.3.1, @Anup Patel do you think you could do that?
OpenSBI has a major number and minor number in the version but it does not have release/patch number so best would be to treat OpenSBI vX.Y.Z as bug fixes on-top-of OpenSBI vX.Y. In other words, supervisor software won't be able to differentiate between OpenSBI vX.Y.Z and OpenSBI vX.Y using sbi_get_impl_version().
There are only three commits between the ACLINT fix and OpenSBI v1.3 so as one-of case I will go ahead create OpenSBI v1.3.1 containing only four commits on-top of OpenSBI v1.3
Does this sound okay ?
Quoted 7 lines
Quoted 2 lines
That sounds fine to me. It fixes the issue for the Microsemi board and it's a very small change between 1.3 and 1.3.1
Please check
https://github.com/riscv-software-src/opensbi/releases/tag/v1.3.1
I hope this helps.
Quoted 8 lines
Hi Alistair,
Do we need to update QEMU's opensbi binaries to v1.3.1?
Hi Anup,
Somehow I cannot see the 'tag' v1.3.1 being populated in the opensbi git repo. Am I missing anything?

Quoted 6 lines

There is a v1.3.1 tag in https://github.com/riscv-software-src/opensbi
(Try cloning the repo again?)
The commit history of v1.3.1 is v1.3 tag + 5 cherry picked commits which means the commit history of the master branch is not the same as the commit history of v1.3.1.

I see. I was seeing a github warning message when I see the last commit [1] from tag v1.3.1:

"This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository."

and was misled. Thanks for the hint. I now added "--tags" when I do a git fetch and now is seeing the new tag.

[1] https://github.com/riscv-software-src/opensbi/commit/057eb10b6d523540012e6947d5c9f63e95244e94

Regards,
Bin

Andreas Schwab

2023-07-19 16:17 UTC #32 lore raw

Boot failure after QEMU's upgrade to OpenSBI v1.3 (was Re: [PATCH for-8.2 6/7] target/riscv: add 'max' CPU type)

In reply to Bin Meng

Quoted 5 lines · Bin Meng

Quoted 4 lines
Please check
https://github.com/riscv-software-src/opensbi/releases/tag/v1.3.1
I hope this helps.

Quoted 8 lines

Hi Alistair,
Do we need to update QEMU's opensbi binaries to v1.3.1?
Hi Anup,
Somehow I cannot see the 'tag' v1.3.1 being populated in the opensbi git repo. Am I missing anything?

You need to run git fetch --tags, because the tag is not part of any branch, thus not fetched automatically.

Signature · 3 lines

Andreas Schwab, schwab at linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Conor Dooley

2023-07-19 07:07 UTC #33 lore raw

Boot failure after QEMU's upgrade to OpenSBI v1.3 (was Re: [PATCH for-8.2 6/7] target/riscv: add 'max' CPU type)

In reply to Alistair Francis

Quoted 3 lines · Alistair Francis

If there is no OpenSBI 1.3.1 release we should add something to the release notes. @Conor Dooley are you able to give a clear sentence on how the boot fails?

Uhh, I'll give it a shot, but hopefully it is not required :)

In version v1.3, OpenSBI's aclint drivers fail to initialise if they encounter a disabled CPU node in the devicetree. Attempting to boot using, for example, the Linux kernel's PolarFire SoC or Freedom U540 devicetrees, will fail with the error:
"init_coldboot: ipi init failed (error -1009)"
Please see OpenSBI commit c6a3573 ("lib: utils: Fix sbi_hartid_to_scratch() usage in ACLINT drivers")
<https://github.com/riscv-software-src/opensbi/commit/c6a35733b74aeff612398f274ed19a74f81d1f37>
for the fix.

Anup Patel

2023-07-14 12:35 UTC #34 lore raw

Boot failure after QEMU's upgrade to OpenSBI v1.3 (was Re: [PATCH for-8.2 6/7] target/riscv: add 'max' CPU type)

In reply to Conor Dooley

Quoted 50 lines · On Fri, Jul 14, 2023 at 3:50?PM Conor Dooley <[email protected]> wrote:

Quoted 26 lines · Anup Patel
Quoted 18 lines
Quoted 18 lines
OpenSBI v1.3
____ _____ ____ _____
  / __ \                  / ____|  _ \_   _|
 | |  | |_ __   ___ _ __ | (___ | |_) || |
 | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
 | |__| | |_) |  __/ | | |____) | |_) || |_
  \____/| .__/ \___|_| |_|_____/|___/_____|
        | |
        |_|
init_coldboot: ipi init failed (error -1009)
Just to note, because we use our own firmware that vendors in OpenSBI and compiles only a significantly cut down number of files from it, we do not use the fw_dynamic etc flow on our hardware. As a result, we have not tested v1.3, nor do we have any immediate plans to change our platform firmware to vendor v1.3 either.
Quoted 4 lines
Quoted 4 lines
I unless there's something obvious to you, it sounds like I will need to go and bisect OpenSBI. That's a job for another day though, given the time.
The real issue is some CPU/HART DT nodes marked as disabled in the DT passed to OpenSBI 1.3.
Quoted 8 lines
This issue does not exist in any of the DTs generated by QEMU but some of the DTs in the kernel (such as microchip and SiFive board DTs) have the E-core disabled.
I had discovered this issue in a totally different context after the OpenSBI 1.3 release happened. This issue is already fixed in the latest OpenSBI by the following commit c6a35733b74aeff612398f274ed19a74f81d1f37 ("lib: utils:
Fix sbi_hartid_to_scratch() usage in ACLINT drivers").
Great, thanks Anup! I thought I had tested tip-of-tree too, but obviously not.
Quoted 2 lines
I always assumed that Microchip hss.bin is the preferred BIOS for the QEMU microchip-icicle-kit machine but I guess that's not true.
Unfortunately the HSS has not worked in QEMU for a long time, and while I would love to fix it, but am pretty stretched for spare time to begin with.
I usually just do direct kernel boots, which use the OpenSBI that comes with QEMU, as I am sure you already know :)

Quoted 8 lines

Quoted 5 lines
At this point, you can either:
1) Use latest OpenSBI on QEMU microchip-icicle-kit machine
2) Ensure CPU0 DT node is enabled in DT when booting on QEMU microchip-icicle-kit machine with OpenSBI 1.3
Will OpenSBI disable it? If not, I think option 2) needs to be remove the DT node. I'll just use tip-of-tree myself & up to the

Current, FDT fixup code in OpenSBI will disable any CPU DT node which satisfies any of the following:
1) CPU is not assigned to the current domain
2) CPU does not have "mmu-type" DT property

Regards,
Anup

Daniel Henrique Barboza

author

2023-07-12 19:03 UTC #35 lore raw

PATCH 7/7 avocado, risc-v: add opensbi tests for 'max' CPU

In reply to Daniel Henrique Barboza

Add smoke tests to ensure that we'll not break the 'max' CPU type when adding new ratified extensions to be enabled.

Signed-off-by Daniel Henrique Barboza <[email protected]>

---
 tests/avocado/riscv_opensbi.py | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

tests/avocado/riscv_opensbi.py +16 -0

index bfff9cc3c3..15fd57fe51 100644
@@ -61,3 +61,19 @@ def test_riscv64_virt(self):
         :avocado: tags=machine:virt
         """
         self.boot_opensbi()
+
+    def test_riscv32_virt_maxcpu(self):
+        """
+        :avocado: tags=arch:riscv32
+        :avocado: tags=machine:virt
+        :avocado: tags=cpu:max
+        """
+        self.boot_opensbi()
+
+    def test_riscv64_virt_maxcpu(self):
+        """
+        :avocado: tags=arch:riscv64
+        :avocado: tags=machine:virt
+        :avocado: tags=cpu:max
+        """
+        self.boot_opensbi()

Signature · 1 line

2.41.0