diff mbox series

armv8: cache_v8: fix mmu_set_region_dcache_behaviour

Message ID 20200511084107.7196-1-peng.fan@nxp.com
State Accepted
Commit b4b26192112bd2c225b8e424c2e2d360761cd864
Headers show
Series armv8: cache_v8: fix mmu_set_region_dcache_behaviour | expand

Commit Message

Peng Fan May 11, 2020, 8:41 a.m. UTC
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 at nxp.com>
Signed-off-by: Peng Fan <peng.fan at nxp.com>
---
 arch/arm/cpu/armv8/cache_v8.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Peng Fan May 20, 2020, 12:10 p.m. UTC | #1
> 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 at nxp.com>
> Signed-off-by: Peng Fan <peng.fan at 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
Tom Rini May 20, 2020, 12:23 p.m. UTC | #2
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 Rini May 25, 2020, 5:59 p.m. UTC | #3
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 at nxp.com>
> Signed-off-by: Peng Fan <peng.fan at 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!
Peng Fan May 26, 2020, 1:24 a.m. UTC | #4
> 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 at nxp.com>
> > Signed-off-by: Peng Fan <peng.fan at 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
diff mbox series

Patch

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;