RE: [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 
> > Signed-off-by: Peng Fan 
> 
> 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

Re: [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 
> Signed-off-by: Peng Fan 

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


signature.asc
Description: PGP signature

Re: [PATCH] armv8: cache_v8: fix mmu_set_region_dcache_behaviour

[Tom Rini]

On Wed, May 20, 2020 at 12:10:23PM +, 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


signature.asc
Description: PGP signature

RE: [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 
> Signed-off-by: Peng Fan 
> ---
>  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]

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 
Signed-off-by: Peng Fan 
---
 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