Re: [PATCH v12 7/7] target/riscv: Enable updates for pointer masking variables and thus enable pointer masking extension

[Alexey Baturo]

Hi Daniel,

Indeed this series doesn't include the Supm extension, but it's mandatory for RVA23U64.

As for the Supm itself, if I get it right, I don't think it should be always-on, unless the proper profile is selected.

I think we might need to add extra flags like ext_supm and ext_sspm. Then, to avoid adding extra checks to the existing code, I think we could enable Ssnpm or Smnpm or both somewhere during the initialization if Supm is set. And do the same for Sspm.

From what I see right now in the code, there's no mention of RVA23, so maybe we could just add some fields for the missing extensions and support them later, when RVA23 support is implemented.

Personally, I'd like to do this in a separate patch after these ones are merged.

What do you think?

Thanks

чт, 12 дек. 2024 г. в 12:48, Daniel Henrique Barboza <dbarboza@ventanamicro.com>:

On 12/5/24 8:23 AM, baturo.alexey@gmail.com wrote:
> From: Alexey Baturo <baturo.alexey@gmail.com>
>
> Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>   target/riscv/cpu.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 4e80dcd2e6..fd3ea9ce76 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -186,11 +186,14 @@ const RISCVIsaExtData isa_edata_arr[] = {
>       ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia),
>       ISA_EXT_DATA_ENTRY(smcntrpmf, PRIV_VERSION_1_12_0, ext_smcntrpmf),
>       ISA_EXT_DATA_ENTRY(smepmp, PRIV_VERSION_1_12_0, ext_smepmp),
> +    ISA_EXT_DATA_ENTRY(smmpm, PRIV_VERSION_1_13_0, ext_smmpm),
> +    ISA_EXT_DATA_ENTRY(smnpm, PRIV_VERSION_1_13_0, ext_smnpm),
>       ISA_EXT_DATA_ENTRY(smstateen, PRIV_VERSION_1_12_0, ext_smstateen),
>       ISA_EXT_DATA_ENTRY(ssaia, PRIV_VERSION_1_12_0, ext_ssaia),
>       ISA_EXT_DATA_ENTRY(ssccptr, PRIV_VERSION_1_11_0, has_priv_1_11),
>       ISA_EXT_DATA_ENTRY(sscofpmf, PRIV_VERSION_1_12_0, ext_sscofpmf),
>       ISA_EXT_DATA_ENTRY(sscounterenw, PRIV_VERSION_1_12_0, has_priv_1_12),
> +    ISA_EXT_DATA_ENTRY(ssnpm, PRIV_VERSION_1_13_0, ext_ssnpm),

I just realized that we're not adding "ext_supm":

"Supm Pointer masking, with the execution environment providing a means
to select PMLEN=0 and PMLEN=7 at minimum."

IIUC this is always enabled in the code so this would be a flag that would
be always enabled, i.e. it would be a ISA_EXT_DATA_ENTRY that defaults to
"has_priv_1_13". "sscounterenw" is an example of this kind of extension.

If that's really the case I believe you can add "supm" in this patch or maybe
a new patch right after. We need to advertise support for "supm" for RVA23
anyway.


Thanks,

Daniel


>       ISA_EXT_DATA_ENTRY(sstc, PRIV_VERSION_1_12_0, ext_sstc),
>       ISA_EXT_DATA_ENTRY(sstvala, PRIV_VERSION_1_12_0, has_priv_1_12),
>       ISA_EXT_DATA_ENTRY(sstvecd, PRIV_VERSION_1_12_0, has_priv_1_12),
> @@ -1490,9 +1493,12 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
>       MULTI_EXT_CFG_BOOL("zvfh", ext_zvfh, false),
>       MULTI_EXT_CFG_BOOL("zvfhmin", ext_zvfhmin, false),
>       MULTI_EXT_CFG_BOOL("sstc", ext_sstc, true),
> +    MULTI_EXT_CFG_BOOL("ssnpm", ext_ssnpm, false),
>   
>       MULTI_EXT_CFG_BOOL("smaia", ext_smaia, false),
>       MULTI_EXT_CFG_BOOL("smepmp", ext_smepmp, false),
> +    MULTI_EXT_CFG_BOOL("smmpm", ext_smmpm, false),
> +    MULTI_EXT_CFG_BOOL("smnpm", ext_smnpm, false),
>       MULTI_EXT_CFG_BOOL("smstateen", ext_smstateen, false),
>       MULTI_EXT_CFG_BOOL("ssaia", ext_ssaia, false),
>       MULTI_EXT_CFG_BOOL("svade", ext_svade, false),