On Thu, 2023-06-15 at 21:14 +0800, Weiwei Li wrote: > > On 2023/6/15 20:58, Rob Bradford wrote: > > On Thu, 2023-06-15 at 14:32 +0800, Weiwei Li wrote: > > > Add ext_zfbfmin/zvfbfmin/zvfbfwma properties. > > > Add require check for BF16 extensions. > > > > > > Signed-off-by: Weiwei Li <liwei...@iscas.ac.cn> > > > Signed-off-by: Junqiang Wang <wangjunqi...@iscas.ac.cn> > > > Reviewed-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com> > > > --- > > > target/riscv/cpu.c | 20 ++++++++++++++++++++ > > > target/riscv/cpu_cfg.h | 3 +++ > > > 2 files changed, 23 insertions(+) > > > > > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > > > index 881bddf393..dc6b2f72f6 100644 > > > --- a/target/riscv/cpu.c > > > +++ b/target/riscv/cpu.c > > > @@ -1059,6 +1059,11 @@ void > > > riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) > > > return; > > > } > > > > > > + if (cpu->cfg.ext_zfbfmin && !riscv_has_ext(env, RVF)) { > > > + error_setg(errp, "Zfbfmin extension depends on F > > > extension"); > > > + return; > > > + } > > > + > > > if (riscv_has_ext(env, RVD) && !riscv_has_ext(env, RVF)) { > > > error_setg(errp, "D extension requires F extension"); > > > return; > > > @@ -1109,6 +1114,21 @@ void > > > riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) > > > return; > > > } > > > > > > + if (cpu->cfg.ext_zvfbfmin && !cpu->cfg.ext_zfbfmin) { > > > + error_setg(errp, "Zvfbfmin extension depends on Zfbfmin > > > extension"); > > > + return; > > > + } > > > + > > > + if (cpu->cfg.ext_zvfbfmin && !cpu->cfg.ext_zve32f) { > > > + error_setg(errp, "Zvfbfmin extension depends on Zve32f > > > extension"); > > > + return; > > > + } > > I don't think this is correct - from the spec: > > > > "This extension [Zvfbfmin] depends on the Zfbfmin extension and > > either > > the "V" extension or the Zve32f embedded vector extension." > > > > So this should be: > > > > + if (cpu->cfg.ext_zvfbfmin && !(cpu->cfg.ext_zve32f || cpu- > > > cfg.ext_v) { > > + error_setg(errp, "Zvfbfmin extension depends on Zve32f or > > V > > extension"); > > + return; > > + } > > Zve32f will be enabled when V is enabled. So we can simply check > Zve32f > here.
Great, thank you for the clarification - I missed that this this enforced directly above. Cheers, Rob > > Regards, > > Weiwei Li > > > Cheers, > > > > Rob > > > > > + > > > + if (cpu->cfg.ext_zvfbfwma && !cpu->cfg.ext_zvfbfmin) { > > > + error_setg(errp, "Zvfbfwma extension depends on Zvfbfmin > > > extension"); > > > + return; > > > + } > > > + > > > /* Set the ISA extensions, checks should have happened > > > above */ > > > if (cpu->cfg.ext_zhinx) { > > > cpu->cfg.ext_zhinxmin = true; > > > diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h > > > index c4a627d335..7d16f32720 100644 > > > --- a/target/riscv/cpu_cfg.h > > > +++ b/target/riscv/cpu_cfg.h > > > @@ -75,6 +75,7 @@ struct RISCVCPUConfig { > > > bool ext_svpbmt; > > > bool ext_zdinx; > > > bool ext_zawrs; > > > + bool ext_zfbfmin; > > > bool ext_zfh; > > > bool ext_zfhmin; > > > bool ext_zfinx; > > > @@ -84,6 +85,8 @@ struct RISCVCPUConfig { > > > bool ext_zve64f; > > > bool ext_zve64d; > > > bool ext_zmmul; > > > + bool ext_zvfbfmin; > > > + bool ext_zvfbfwma; > > > bool ext_zvfh; > > > bool ext_zvfhmin; > > > bool ext_smaia; >