Re: [PATCH v6 01/12] target/riscv: add zicbop extension flag

[Andrew Jones]

On Sat, Oct 28, 2023 at 05:54:16AM -0300, Daniel Henrique Barboza wrote:
> QEMU already implements zicbom (Cache Block Management Operations) and
> zicboz (Cache Block Zero Operations). Commit 59cb29d6a5 ("target/riscv:
> add Zicbop cbo.prefetch{i, r, m} placeholder") added placeholders for
> what would be the instructions for zicbop (Cache Block Prefetch
> Operations), which are now no-ops.
> 
> The RVA22U64 profile mandates zicbop, which means that applications that
> run with this profile might expect zicbop to be present in the riscv,isa
> DT and might behave badly if it's absent.
> 
> Adding zicbop as an extension will make our future RVA22U64
> implementation more in line with what userspace expects and, if/when
> cache block prefetch operations became relevant to QEMU, we already have
> the extension flag to turn then on/off as needed.
> 
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>  hw/riscv/virt.c        | 5 +++++
>  target/riscv/cpu.c     | 3 +++
>  target/riscv/cpu_cfg.h | 2 ++
>  3 files changed, 10 insertions(+)
> 
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 1732c42915..99c087240f 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -273,6 +273,11 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, 
> int socket,
>                                    cpu_ptr->cfg.cboz_blocksize);
>          }
>  
> +        if (cpu_ptr->cfg.ext_zicbop) {
> +            qemu_fdt_setprop_cell(ms->fdt, cpu_name, "riscv,cbop-block-size",

I think we need to get this node approved by devicetree@vger.kernel.org
and merged into [1] first.

[1] Linux repo: Documentation/devicetree/bindings/riscv/cpus.yaml

> +                                  cpu_ptr->cfg.cbop_blocksize);
> +        }
> +
>          qemu_fdt_setprop_string(ms->fdt, cpu_name, "compatible", "riscv");
>          qemu_fdt_setprop_string(ms->fdt, cpu_name, "status", "okay");
>          qemu_fdt_setprop_cell(ms->fdt, cpu_name, "reg",
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index f40da4c661..6c0050988f 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -78,6 +78,7 @@ const uint32_t misa_bits[] = {RVI, RVE, RVM, RVA, RVF, RVD, 
> RVV,
>   */
>  const RISCVIsaExtData isa_edata_arr[] = {
>      ISA_EXT_DATA_ENTRY(zicbom, PRIV_VERSION_1_12_0, ext_zicbom),
> +    ISA_EXT_DATA_ENTRY(zicbop, PRIV_VERSION_1_12_0, ext_zicbop),
>      ISA_EXT_DATA_ENTRY(zicboz, PRIV_VERSION_1_12_0, ext_zicboz),
>      ISA_EXT_DATA_ENTRY(zicond, PRIV_VERSION_1_12_0, ext_zicond),
>      ISA_EXT_DATA_ENTRY(zicntr, PRIV_VERSION_1_12_0, ext_zicntr),
> @@ -1336,6 +1337,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
>      MULTI_EXT_CFG_BOOL("zhinxmin", ext_zhinxmin, false),
>  
>      MULTI_EXT_CFG_BOOL("zicbom", ext_zicbom, true),
> +    MULTI_EXT_CFG_BOOL("zicbop", ext_zicbop, true),
>      MULTI_EXT_CFG_BOOL("zicboz", ext_zicboz, true),
>  
>      MULTI_EXT_CFG_BOOL("zmmul", ext_zmmul, false),
> @@ -1424,6 +1426,7 @@ Property riscv_cpu_options[] = {
>      DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
>  
>      DEFINE_PROP_UINT16("cbom_blocksize", RISCVCPU, cfg.cbom_blocksize, 64),
> +    DEFINE_PROP_UINT16("cbop_blocksize", RISCVCPU, cfg.cbop_blocksize, 64),
>      DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU, cfg.cboz_blocksize, 64),
>  
>      DEFINE_PROP_END_OF_LIST(),
> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> index 6eef4a51ea..2203b4c45b 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -65,6 +65,7 @@ struct RISCVCPUConfig {
>      bool ext_zicntr;
>      bool ext_zicsr;
>      bool ext_zicbom;
> +    bool ext_zicbop;
>      bool ext_zicboz;
>      bool ext_zicond;
>      bool ext_zihintntl;
> @@ -134,6 +135,7 @@ struct RISCVCPUConfig {
>      uint16_t vlen;
>      uint16_t elen;
>      uint16_t cbom_blocksize;
> +    uint16_t cbop_blocksize;
>      uint16_t cboz_blocksize;
>      bool mmu;
>      bool pmp;
> -- 
> 2.41.0
>

Otherwise,

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Thanks,
drew