Re: [PATCH for-8.2 v5 09/11] target/riscv: add 'max' CPU type

2023-07-27 Thread Conor Dooley
On Thu, Jul 27, 2023 at 11:12:34AM -0300, Daniel Henrique Barboza wrote:
> 
> 
> On 7/27/23 10:59, Conor Dooley wrote:
> > Hey Daniel,
> > 
> > On Thu, Jul 20, 2023 at 02:19:31PM -0300, Daniel Henrique Barboza wrote:
> > > 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.
> > > 
> > > MISA bits RVG, RVJ and RVV are also being set manually since they're
> > > default disabled.
> > > 
> > > This is the resulting 'riscv,isa' DT for this new CPU:
> > > 
> > > rv64imafdcvh_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
> > > 
> > > Signed-off-by: Daniel Henrique Barboza 
> > > Reviewed-by: Weiwei Li 
> > 
> > I was giving this another go today, like so
> > $(qemu) -smp 4 -M virt,aia=aplic,dumpdtb=qemu.dtb -cpu max -m 1G
> > which lead to a few
> > vector version is not specified, use the default value v1.0
> > printed. Should the max cpu set a vector version w/o user input
> > being required?
> 
> 
> This isn't exclusive to the 'max' cpu code per se. It's the common RVV 
> handling
> code that is expecting users to inform which vector version they're going to
> use every time we activate V.

Yah, I figured it was not exclusive to it, but it seemed "thematic" for
the max cpu to silently pick a reasonable default rather than complain.

> I believe it's too late to change the command line handling to force users to 
> pick
> a vector version, so the second better approach is to silently default to v1.0
> (or perhaps to the latest RVV version available) if the user didn't set 
> anything.

Honestly, I'm not sure why it warns at the moment. Seems like the
default is what any reasonable person would expect, no?



signature.asc
Description: PGP signature


Re: [PATCH for-8.2 v5 09/11] target/riscv: add 'max' CPU type

2023-07-27 Thread Daniel Henrique Barboza




On 7/27/23 10:59, Conor Dooley wrote:

Hey Daniel,

On Thu, Jul 20, 2023 at 02:19:31PM -0300, Daniel Henrique Barboza wrote:

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.

MISA bits RVG, RVJ and RVV are also being set manually since they're
default disabled.

This is the resulting 'riscv,isa' DT for this new CPU:

rv64imafdcvh_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

Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Weiwei Li 


I was giving this another go today, like so
$(qemu) -smp 4 -M virt,aia=aplic,dumpdtb=qemu.dtb -cpu max -m 1G
which lead to a few
vector version is not specified, use the default value v1.0
printed. Should the max cpu set a vector version w/o user input
being required?



This isn't exclusive to the 'max' cpu code per se. It's the common RVV handling
code that is expecting users to inform which vector version they're going to
use every time we activate V.

I believe it's too late to change the command line handling to force users to 
pick
a vector version, so the second better approach is to silently default to v1.0
(or perhaps to the latest RVV version available) if the user didn't set 
anything.


Thanks,

Daniel






Cheers,
Conor.




Re: [PATCH for-8.2 v5 09/11] target/riscv: add 'max' CPU type

2023-07-27 Thread Daniel Henrique Barboza




On 7/27/23 11:16, Conor Dooley wrote:

On Thu, Jul 27, 2023 at 11:12:34AM -0300, Daniel Henrique Barboza wrote:



On 7/27/23 10:59, Conor Dooley wrote:

Hey Daniel,

On Thu, Jul 20, 2023 at 02:19:31PM -0300, Daniel Henrique Barboza wrote:

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.

MISA bits RVG, RVJ and RVV are also being set manually since they're
default disabled.

This is the resulting 'riscv,isa' DT for this new CPU:

rv64imafdcvh_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

Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Weiwei Li 


I was giving this another go today, like so
$(qemu) -smp 4 -M virt,aia=aplic,dumpdtb=qemu.dtb -cpu max -m 1G
which lead to a few
vector version is not specified, use the default value v1.0
printed. Should the max cpu set a vector version w/o user input
being required?



This isn't exclusive to the 'max' cpu code per se. It's the common RVV handling
code that is expecting users to inform which vector version they're going to
use every time we activate V.


Yah, I figured it was not exclusive to it, but it seemed "thematic" for
the max cpu to silently pick a reasonable default rather than complain.


Fair point. For the 'max' CPU it definitely makes sense to pick a good default
instead of nagging about it.

I'll send a new version. Thanks,


Daniel




I believe it's too late to change the command line handling to force users to 
pick
a vector version, so the second better approach is to silently default to v1.0
(or perhaps to the latest RVV version available) if the user didn't set 
anything.


Honestly, I'm not sure why it warns at the moment. Seems like the
default is what any reasonable person would expect, no?





Re: [PATCH for-8.2 v5 09/11] target/riscv: add 'max' CPU type

2023-07-27 Thread Conor Dooley
Hey Daniel,

On Thu, Jul 20, 2023 at 02:19:31PM -0300, Daniel Henrique Barboza wrote:
> 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.
> 
> MISA bits RVG, RVJ and RVV are also being set manually since they're
> default disabled.
> 
> This is the resulting 'riscv,isa' DT for this new CPU:
> 
> rv64imafdcvh_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
> 
> Signed-off-by: Daniel Henrique Barboza 
> Reviewed-by: Weiwei Li 

I was giving this another go today, like so
$(qemu) -smp 4 -M virt,aia=aplic,dumpdtb=qemu.dtb -cpu max -m 1G
which lead to a few
vector version is not specified, use the default value v1.0
printed. Should the max cpu set a vector version w/o user input
being required?

Cheers,
Conor.


signature.asc
Description: PGP signature


[PATCH for-8.2 v5 09/11] target/riscv: add 'max' CPU type

2023-07-20 Thread 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.

MISA bits RVG, RVJ and RVV are also being set manually since they're
default disabled.

This is the resulting 'riscv,isa' DT for this new CPU:

rv64imafdcvh_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

Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Weiwei Li 
---
 target/riscv/cpu-qom.h |  1 +
 target/riscv/cpu.c | 53 ++
 2 files changed, 54 insertions(+)

diff --git a/target/riscv/cpu-qom.h b/target/riscv/cpu-qom.h
index 04af50983e..f3fbe37a2c 100644
--- a/target/riscv/cpu-qom.h
+++ b/target/riscv/cpu-qom.h
@@ -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")
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 8675839cb4..0221bfcbef 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -248,6 +248,7 @@ static const char * const riscv_intr_names[] = {
 };
 
 static void riscv_cpu_add_user_properties(Object *obj);
+static void riscv_init_max_cpu_extensions(Object *obj);
 
 const char *riscv_cpu_get_trap_name(target_ulong cause, bool async)
 {
@@ -374,6 +375,25 @@ static void riscv_any_cpu_init(Object *obj)
 cpu->cfg.pmp = true;
 }
 
+static void riscv_max_cpu_init(Object *obj)
+{
+RISCVCPU *cpu = RISCV_CPU(obj);
+CPURISCVState *env = >env;
+RISCVMXL mlx = MXL_RV64;
+
+#ifdef TARGET_RISCV32
+mlx = MXL_RV32;
+#endif
+set_misa(env, mlx, 0);
+riscv_cpu_add_user_properties(obj);
+riscv_init_max_cpu_extensions(obj);
+env->priv_ver = PRIV_VERSION_LATEST;
+#ifndef CONFIG_USER_ONLY
+set_satp_mode_max_supported(RISCV_CPU(obj), mlx == MXL_RV32 ?
+VM_1_10_SV32 : VM_1_10_SV57);
+#endif
+}
+
 #if defined(TARGET_RISCV64)
 static void rv64_base_cpu_init(Object *obj)
 {
@@ -1955,6 +1975,38 @@ static void riscv_cpu_add_user_properties(Object *obj)
 ADD_CPU_QDEV_PROPERTIES_ARRAY(dev, riscv_cpu_experimental_exts);
 }
 
+/*
+ * The 'max' type CPU will have all possible ratified
+ * non-vendor extensions enabled.
+ */
+static void riscv_init_max_cpu_extensions(Object *obj)
+{
+RISCVCPU *cpu = RISCV_CPU(obj);
+CPURISCVState *env = >env;
+
+/* Enable RVG, RVJ and RVV that are disabled by default */
+set_misa(env, env->misa_mxl, env->misa_ext | RVG | RVJ | RVV);
+
+for (int i = 0; i < ARRAY_SIZE(riscv_cpu_extensions); i++) {
+object_property_set_bool(obj, riscv_cpu_extensions[i].name,
+ true, NULL);
+}
+
+/* Zfinx is not compatible with F. Disable it */
+object_property_set_bool(obj, "zfinx", false, NULL);
+object_property_set_bool(obj, "zdinx", false, NULL);
+object_property_set_bool(obj, "zhinx", false, NULL);
+object_property_set_bool(obj, "zhinxmin", false, NULL);
+
+object_property_set_bool(obj, "zce", false, NULL);
+object_property_set_bool(obj, "zcmp", false, NULL);
+object_property_set_bool(obj, "zcmt", false, NULL);
+
+if (env->misa_mxl != MXL_RV32) {
+object_property_set_bool(obj, "zcf", false, NULL);
+}
+}
+
 static Property riscv_cpu_properties[] = {
 DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, true),
 
@@ -2293,6 +2345,7 @@ static const TypeInfo riscv_cpu_type_infos[] = {
 .abstract = true,
 },
 DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_ANY,  riscv_any_cpu_init),
+DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_MAX,  riscv_max_cpu_init),
 #if defined(CONFIG_KVM)
 DEFINE_CPU(TYPE_RISCV_CPU_HOST, riscv_host_cpu_init),
 #endif
-- 
2.41.0