[PATCH] lib: sbi: fwft: Use only the provided PMLEN value

[PATCH] lib: sbi: fwft: Use only the provided PMLEN value

[Samuel Holland]

As of riscv-sbi-doc commit c7d3d1f7dcaa ("ext-fwft: use the provided
value in fwft_set(POINTER_MASKING_PMLEN)"), the SBI implementation must
use only the provided PMLEN value or else fail. It may not fall back to
a larger PMLEN value.

Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
---

 lib/sbi/sbi_fwft.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/lib/sbi/sbi_fwft.c b/lib/sbi/sbi_fwft.c
index c46006d4..a2aefb9a 100644
--- a/lib/sbi/sbi_fwft.c
+++ b/lib/sbi/sbi_fwft.c
@@ -223,32 +223,32 @@ static int fwft_pmlen_supported(struct fwft_config *conf)
 	return SBI_OK;
 }
 
-static bool fwft_try_to_set_pmm(unsigned long pmm)
-{
-	csr_set(CSR_MENVCFG, pmm);
-	return (csr_read(CSR_MENVCFG) & ENVCFG_PMM) == pmm;
-}
-
 static int fwft_set_pmlen(struct fwft_config *conf, unsigned long value)
 {
-	unsigned long prev;
+	unsigned long pmm, prev;
 
-	if (value > 16)
+	switch (value) {
+	case 0:
+		pmm = ENVCFG_PMM_PMLEN_0;
+		break;
+	case 7:
+		pmm = ENVCFG_PMM_PMLEN_7;
+		break;
+	case 16:
+		pmm = ENVCFG_PMM_PMLEN_16;
+		break;
+	default:
 		return SBI_EINVAL;
+	}
 
 	prev = csr_read_clear(CSR_MENVCFG, ENVCFG_PMM);
-	if (value == 0)
-		return SBI_OK;
-	if (value <= 7) {
-		if (fwft_try_to_set_pmm(ENVCFG_PMM_PMLEN_7))
-			return SBI_OK;
-		csr_clear(CSR_MENVCFG, ENVCFG_PMM);
+	csr_set(CSR_MENVCFG, pmm);
+	if ((csr_read(CSR_MENVCFG) & ENVCFG_PMM) != pmm) {
+		csr_write(CSR_MENVCFG, prev);
+		return SBI_EINVAL;
 	}
-	if (fwft_try_to_set_pmm(ENVCFG_PMM_PMLEN_16))
-		return SBI_OK;
-	csr_write(CSR_MENVCFG, prev);
 
-	return SBI_EINVAL;
+	return SBI_OK;
 }
 
 static int fwft_get_pmlen(struct fwft_config *conf, unsigned long *value)
-- 
2.47.2

base-commit: 69a0f0245f39ea3af4685cab4cb2dda90acd17cd
branch: up/fwft-pmlen-update

-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Re: [PATCH] lib: sbi: fwft: Use only the provided PMLEN value

[Radim Krčmář]

2025-05-21T18:34:33-07:00, Samuel Holland <samuel.holland@sifive.com>:
> As of riscv-sbi-doc commit c7d3d1f7dcaa ("ext-fwft: use the provided
> value in fwft_set(POINTER_MASKING_PMLEN)"), the SBI implementation must
> use only the provided PMLEN value or else fail. It may not fall back to
> a larger PMLEN value.
>
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
> ---

I didn't realize we already implemented the non-ratified spec, thanks,

Reviewed-by: Radim Krčmář <rkrcmar@ventanamicro.com>

-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Re: [PATCH] lib: sbi: fwft: Use only the provided PMLEN value

[Anup Patel]

On Thu, May 22, 2025 at 7:05 AM Samuel Holland
<samuel.holland@sifive.com> wrote:
>
> As of riscv-sbi-doc commit c7d3d1f7dcaa ("ext-fwft: use the provided
> value in fwft_set(POINTER_MASKING_PMLEN)"), the SBI implementation must
> use only the provided PMLEN value or else fail. It may not fall back to
> a larger PMLEN value.
>
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>

Applied this patch to the riscv/opensbi repo.

Thanks,
Anup

> ---
>
>  lib/sbi/sbi_fwft.c | 36 ++++++++++++++++++------------------
>  1 file changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/lib/sbi/sbi_fwft.c b/lib/sbi/sbi_fwft.c
> index c46006d4..a2aefb9a 100644
> --- a/lib/sbi/sbi_fwft.c
> +++ b/lib/sbi/sbi_fwft.c
> @@ -223,32 +223,32 @@ static int fwft_pmlen_supported(struct fwft_config *conf)
>         return SBI_OK;
>  }
>
> -static bool fwft_try_to_set_pmm(unsigned long pmm)
> -{
> -       csr_set(CSR_MENVCFG, pmm);
> -       return (csr_read(CSR_MENVCFG) & ENVCFG_PMM) == pmm;
> -}
> -
>  static int fwft_set_pmlen(struct fwft_config *conf, unsigned long value)
>  {
> -       unsigned long prev;
> +       unsigned long pmm, prev;
>
> -       if (value > 16)
> +       switch (value) {
> +       case 0:
> +               pmm = ENVCFG_PMM_PMLEN_0;
> +               break;
> +       case 7:
> +               pmm = ENVCFG_PMM_PMLEN_7;
> +               break;
> +       case 16:
> +               pmm = ENVCFG_PMM_PMLEN_16;
> +               break;
> +       default:
>                 return SBI_EINVAL;
> +       }
>
>         prev = csr_read_clear(CSR_MENVCFG, ENVCFG_PMM);
> -       if (value == 0)
> -               return SBI_OK;
> -       if (value <= 7) {
> -               if (fwft_try_to_set_pmm(ENVCFG_PMM_PMLEN_7))
> -                       return SBI_OK;
> -               csr_clear(CSR_MENVCFG, ENVCFG_PMM);
> +       csr_set(CSR_MENVCFG, pmm);
> +       if ((csr_read(CSR_MENVCFG) & ENVCFG_PMM) != pmm) {
> +               csr_write(CSR_MENVCFG, prev);
> +               return SBI_EINVAL;
>         }
> -       if (fwft_try_to_set_pmm(ENVCFG_PMM_PMLEN_16))
> -               return SBI_OK;
> -       csr_write(CSR_MENVCFG, prev);
>
> -       return SBI_EINVAL;
> +       return SBI_OK;
>  }
>
>  static int fwft_get_pmlen(struct fwft_config *conf, unsigned long *value)
> --
> 2.47.2
>
> base-commit: 69a0f0245f39ea3af4685cab4cb2dda90acd17cd
> branch: up/fwft-pmlen-update
>
> --
> opensbi mailing list
> opensbi@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi

-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi