[PATCH] lib: sbi: Fix PMP granularity handling in sbi_hart_map_saddr()

[PATCH] lib: sbi: Fix PMP granularity handling in sbi_hart_map_saddr()

[Anup Patel]

The sbi_hart_map_saddr() must create PMP mapping of size greater
than or equal to PMP granularity otherwise PMP mapping does not
work when size parameter less than sbi_hart_pmp_granularity(scratch).

Fixes: 6e44ef686a9b ("lib: sbi: Add functions to map/unmap shared
memory")
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 lib/sbi/sbi_hart.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index 77eef49..bf8792a 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -508,7 +508,8 @@ int sbi_hart_map_saddr(unsigned long addr, unsigned long size)
 	if (is_pmp_entry_mapped(SBI_SMEPMP_RESV_ENTRY))
 		return SBI_ENOSPC;
 
-	for (order = log2roundup(size) ; order <= __riscv_xlen; order++) {
+	for (order = log2roundup(MAX(sbi_hart_pmp_granularity(scratch), size));
+	     order <= __riscv_xlen; order++) {
 		if (order < __riscv_xlen) {
 			base = addr & ~((1UL << order) - 1UL);
 			if ((base <= addr) &&
-- 
2.34.1

[PATCH] lib: sbi: Fix PMP granularity handling in sbi_hart_map_saddr()

[Anup Patel]

The sbi_hart_map_saddr() must create PMP mapping of size greater
than or equal to PMP granularity otherwise PMP mapping does not
work when size parameter less than sbi_hart_pmp_granularity(scratch).

Fixes: 6e44ef686a9b ("lib: sbi: Add functions to map/unmap shared
memory")
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 lib/sbi/sbi_hart.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index 77eef49..bf8792a 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -508,7 +508,8 @@ int sbi_hart_map_saddr(unsigned long addr, unsigned long size)
 	if (is_pmp_entry_mapped(SBI_SMEPMP_RESV_ENTRY))
 		return SBI_ENOSPC;
 
-	for (order = log2roundup(size) ; order <= __riscv_xlen; order++) {
+	for (order = log2roundup(MAX(sbi_hart_pmp_granularity(scratch), size));
+	     order <= __riscv_xlen; order++) {
 		if (order < __riscv_xlen) {
 			base = addr & ~((1UL << order) - 1UL);
 			if ((base <= addr) &&
-- 
2.34.1

[PATCH] lib: sbi: Fix PMP granularity handling in sbi_hart_map_saddr()

[Samuel Holland]

Hi Anup,

On 2023-11-20 11:49 AM, Anup Patel wrote:
> The sbi_hart_map_saddr() must create PMP mapping of size greater
> than or equal to PMP granularity otherwise PMP mapping does not
> work when size parameter less than sbi_hart_pmp_granularity(scratch).
> 
> Fixes: 6e44ef686a9b ("lib: sbi: Add functions to map/unmap shared
> memory")

This line should not be wrapped. Otherwise,

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

> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  lib/sbi/sbi_hart.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index 77eef49..bf8792a 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -508,7 +508,8 @@ int sbi_hart_map_saddr(unsigned long addr, unsigned long size)
>  	if (is_pmp_entry_mapped(SBI_SMEPMP_RESV_ENTRY))
>  		return SBI_ENOSPC;
>  
> -	for (order = log2roundup(size) ; order <= __riscv_xlen; order++) {
> +	for (order = log2roundup(MAX(sbi_hart_pmp_granularity(scratch), size));

I noticed that sbi_hart_pmp_granularity() is always passed to log2roundup(), and
hfeatures->pmp_gran is originally generated by shifting, so maybe the value
should be stored as the log base 2 to avoid the conversions?

Regards,
Samuel

> +	     order <= __riscv_xlen; order++) {
>  		if (order < __riscv_xlen) {
>  			base = addr & ~((1UL << order) - 1UL);
>  			if ((base <= addr) &&

[PATCH] lib: sbi: Fix PMP granularity handling in sbi_hart_map_saddr()

[Anup Patel]

On Wed, Nov 22, 2023 at 6:00?AM Samuel Holland
<samuel.holland@sifive.com> wrote:
>
> Hi Anup,
>
> On 2023-11-20 11:49 AM, Anup Patel wrote:
> > The sbi_hart_map_saddr() must create PMP mapping of size greater
> > than or equal to PMP granularity otherwise PMP mapping does not
> > work when size parameter less than sbi_hart_pmp_granularity(scratch).
> >
> > Fixes: 6e44ef686a9b ("lib: sbi: Add functions to map/unmap shared
> > memory")
>
> This line should not be wrapped. Otherwise,
>
> Reviewed-by: Samuel Holland <samuel.holland@sifive.com>

Thanks, updated the commit description at the time of merging this patch.

Applied this patch to the riscv/opensbi repo.

>
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> >  lib/sbi/sbi_hart.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> > index 77eef49..bf8792a 100644
> > --- a/lib/sbi/sbi_hart.c
> > +++ b/lib/sbi/sbi_hart.c
> > @@ -508,7 +508,8 @@ int sbi_hart_map_saddr(unsigned long addr, unsigned long size)
> >       if (is_pmp_entry_mapped(SBI_SMEPMP_RESV_ENTRY))
> >               return SBI_ENOSPC;
> >
> > -     for (order = log2roundup(size) ; order <= __riscv_xlen; order++) {
> > +     for (order = log2roundup(MAX(sbi_hart_pmp_granularity(scratch), size));
>
> I noticed that sbi_hart_pmp_granularity() is always passed to log2roundup(), and
> hfeatures->pmp_gran is originally generated by shifting, so maybe the value
> should be stored as the log base 2 to avoid the conversions?

I agree with your suggestion. Please go ahead and submit a patch for this.

>
> Regards,
> Samuel
>
> > +          order <= __riscv_xlen; order++) {
> >               if (order < __riscv_xlen) {
> >                       base = addr & ~((1UL << order) - 1UL);
> >                       if ((base <= addr) &&
>

Regards,
Anup