[PATCH] armv8: cache_v8: fix mmu_set_region_dcache_behaviour

[PATCH] armv8: cache_v8: fix mmu_set_region_dcache_behaviour

[Peng Fan]

enum dcache_option already shift left 2 bits,
PMD_ATTRINDX(option), will wrongly shift left the attr 4bits, which
is wrong. And make the region user set not has expected attribute
and might affect the splitted block region.

Reviewed-by: Ye Li <ye.li@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 arch/arm/cpu/armv8/cache_v8.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c
index 6a5518f9de..35ee5572e9 100644
--- a/arch/arm/cpu/armv8/cache_v8.c
+++ b/arch/arm/cpu/armv8/cache_v8.c
@@ -555,7 +555,7 @@ static u64 set_one_region(u64 start, u64 size, u64 attrs, bool flag, int level)
 void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
 				     enum dcache_option option)
 {
-	u64 attrs = PMD_ATTRINDX(option);
+	u64 attrs = PMD_ATTRINDX(option >> 2);
 	u64 real_start = start;
 	u64 real_size = size;
 
-- 
2.16.4

[PATCH] armv8: cache_v8: fix mmu_set_region_dcache_behaviour

[Peng Fan]

> Subject: [PATCH] armv8: cache_v8: fix mmu_set_region_dcache_behaviour

Any comments?

Thanks,
Peng.

> 
> enum dcache_option already shift left 2 bits, PMD_ATTRINDX(option), will
> wrongly shift left the attr 4bits, which is wrong. And make the region user set
> not has expected attribute and might affect the splitted block region.
> 
> Reviewed-by: Ye Li <ye.li@nxp.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  arch/arm/cpu/armv8/cache_v8.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/cpu/armv8/cache_v8.c
> b/arch/arm/cpu/armv8/cache_v8.c index 6a5518f9de..35ee5572e9 100644
> --- a/arch/arm/cpu/armv8/cache_v8.c
> +++ b/arch/arm/cpu/armv8/cache_v8.c
> @@ -555,7 +555,7 @@ static u64 set_one_region(u64 start, u64 size, u64
> attrs, bool flag, int level)  void
> mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
>  				     enum dcache_option option)
>  {
> -	u64 attrs = PMD_ATTRINDX(option);
> +	u64 attrs = PMD_ATTRINDX(option >> 2);
>  	u64 real_start = start;
>  	u64 real_size = size;
> 
> --
> 2.16.4

[PATCH] armv8: cache_v8: fix mmu_set_region_dcache_behaviour

[Tom Rini]

On Wed, May 20, 2020 at 12:10:23PM +0000, Peng Fan wrote:
> > Subject: [PATCH] armv8: cache_v8: fix mmu_set_region_dcache_behaviour
> 
> Any comments?

I'll put it on my list, thanks.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200520/f0969c74/attachment.sig>

[PATCH] armv8: cache_v8: fix mmu_set_region_dcache_behaviour

[Tom Rini]

On Mon, May 11, 2020 at 04:41:07PM +0800, Peng Fan wrote:

> enum dcache_option already shift left 2 bits,
> PMD_ATTRINDX(option), will wrongly shift left the attr 4bits, which
> is wrong. And make the region user set not has expected attribute
> and might affect the splitted block region.
> 
> Reviewed-by: Ye Li <ye.li@nxp.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>

Please note that I reworded the commit message a bit.  In the interest
of fixing the bug now:

Applied to u-boot/master.

But on reading the code and macros to understand things better for the
commit message, I wonder why we don't just use options directly now in
the code?  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200525/00be01cb/attachment.sig>

[PATCH] armv8: cache_v8: fix mmu_set_region_dcache_behaviour

[Peng Fan]

> Subject: Re: [PATCH] armv8: cache_v8: fix
> mmu_set_region_dcache_behaviour
> 
> On Mon, May 11, 2020 at 04:41:07PM +0800, Peng Fan wrote:
> 
> > enum dcache_option already shift left 2 bits, PMD_ATTRINDX(option),
> > will wrongly shift left the attr 4bits, which is wrong. And make the
> > region user set not has expected attribute and might affect the
> > splitted block region.
> >
> > Reviewed-by: Ye Li <ye.li@nxp.com>
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> 
> Please note that I reworded the commit message a bit.  In the interest of
> fixing the bug now:
> 
> Applied to u-boot/master.
> 
> But on reading the code and macros to understand things better for the
> commit message, I wonder why we don't just use options directly now in the
> code?  

Seems directly using options would make it a bit simplier! I agree.

Thanks,
Peng.

Thanks!
> 
> --
> Tom