| Message ID | 20181230182746.23448-3-lukas.auer@aisec.fraunhofer.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Small fixes for RISC-V | expand |
On Mon, Dec 31, 2018 at 2:28 AM Lukas Auer <lukas.auer@aisec.fraunhofer.de> wrote: > > The fence instruction is used to enforce device I/O and memory ordering > constraints in RISC-V. It does not directly affect the data cache and > particular cannot be used to flush or invalidate it. RISC-V does not > have instructions for explicit cache control. Remove the > flush_dcache_all implementation and its use in all dcache-specific > functions in lib/cache.c. > > This also adds a missing new line between flush_dcache_all and > flush_dcache_range in lib/cache.c. > > Signed-off-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de> > --- > This patch only removes the implementation itself and its use in > dcache-specific functions in lib/cache.c. There are more uses of it in > arch/riscv/, which this patch does not remove. > > arch/riscv/lib/cache.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Hi Lukas > > From: Lukas Auer [mailto:lukas.auer@aisec.fraunhofer.de] > > Sent: Monday, December 31, 2018 2:28 AM > > To: u-boot@lists.denx.de > > Cc: Anup Patel; Lukas Auer; Rick Jian-Zhi Chen(陳建志); Bin Meng; Greentime Hu > > Subject: [PATCH 2/6] riscv: remove invalid dcache flush implementation > > > > The fence instruction is used to enforce device I/O and memory ordering > > constraints in RISC-V. It does not directly affect the data cache and particular > > cannot be used to flush or invalidate it. RISC-V does not have instructions for > > explicit cache control. Remove the flush_dcache_all implementation and its use > > in all dcache-specific functions in lib/cache.c. > > The privileged mention that fence is for the purposes of ordering, but does not say it can not be used to flush or invalidate. Andes's ax25 have no coherence agent. So it need fence instruction to manipulate cache flush or invalidate. Or there will be some data synchronization error occurs on ae350 platform. Thanks Rick > > This also adds a missing new line between flush_dcache_all and > > flush_dcache_range in lib/cache.c. > > > > Signed-off-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de> > > --- > > This patch only removes the implementation itself and its use in dcache-specific > > functions in lib/cache.c. There are more uses of it in arch/riscv/, which this patch > > does not remove. > > > > arch/riscv/lib/cache.c | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > diff --git a/arch/riscv/lib/cache.c b/arch/riscv/lib/cache.c index > > ae5c60716f..203e287612 100644 > > --- a/arch/riscv/lib/cache.c > > +++ b/arch/riscv/lib/cache.c > > @@ -13,11 +13,10 @@ void invalidate_icache_all(void) > > > > void flush_dcache_all(void) > > { > > - asm volatile ("fence" :::"memory"); > > } > > + > > void flush_dcache_range(unsigned long start, unsigned long end) { > > - flush_dcache_all(); > > } > > > > void invalidate_icache_range(unsigned long start, unsigned long end) @@ -31,7 > > +30,6 @@ void invalidate_icache_range(unsigned long start, unsigned long end) > > > > void invalidate_dcache_range(unsigned long start, unsigned long end) { > > - flush_dcache_all(); > > } > > > > void cache_flush(void) > > -- > > 2.20.1
Hi Rick, On Wed, 2019-01-02 at 10:54 +0800, Rick Chen wrote: > Hi Lukas > > > > From: Lukas Auer [mailto:lukas.auer@aisec.fraunhofer.de] > > > Sent: Monday, December 31, 2018 2:28 AM > > > To: u-boot@lists.denx.de > > > Cc: Anup Patel; Lukas Auer; Rick Jian-Zhi Chen(陳建志); Bin Meng; > > > Greentime Hu > > > Subject: [PATCH 2/6] riscv: remove invalid dcache flush > > > implementation > > > > > > The fence instruction is used to enforce device I/O and memory > > > ordering > > > constraints in RISC-V. It does not directly affect the data cache > > > and particular > > > cannot be used to flush or invalidate it. RISC-V does not have > > > instructions for > > > explicit cache control. Remove the flush_dcache_all > > > implementation and its use > > > in all dcache-specific functions in lib/cache.c. > > > > > The privileged mention that fence is for the purposes of ordering, > but > does not say > it can not be used to flush or invalidate. > > Andes's ax25 have no coherence agent. So it need fence instruction to > manipulate > cache flush or invalidate. Or there will be some data synchronization > error > occurs on ae350 platform. > I was not aware of this. Does the ax25 flush the whole dcache on any fence instruction? You are right, we can't remove it in this case. Since it is specific to the ax25, should I move it into cpu/ax25/cache.c instead? Thanks, Lukas > Thanks > Rick > > > > > This also adds a missing new line between flush_dcache_all and > > > flush_dcache_range in lib/cache.c. > > > > > > Signed-off-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de> > > > --- > > > This patch only removes the implementation itself and its use in > > > dcache-specific > > > functions in lib/cache.c. There are more uses of it in > > > arch/riscv/, which this patch > > > does not remove. > > > > > > arch/riscv/lib/cache.c | 4 +--- > > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > > > diff --git a/arch/riscv/lib/cache.c b/arch/riscv/lib/cache.c > > > index > > > ae5c60716f..203e287612 100644 > > > --- a/arch/riscv/lib/cache.c > > > +++ b/arch/riscv/lib/cache.c > > > @@ -13,11 +13,10 @@ void invalidate_icache_all(void) > > > > > > void flush_dcache_all(void) > > > { > > > - asm volatile ("fence" :::"memory"); > > > } > > > + > > > void flush_dcache_range(unsigned long start, unsigned long > > > end) { > > > - flush_dcache_all(); > > > } > > > > > > void invalidate_icache_range(unsigned long start, unsigned long > > > end) @@ -31,7 > > > +30,6 @@ void invalidate_icache_range(unsigned long start, > > > unsigned long end) > > > > > > void invalidate_dcache_range(unsigned long start, unsigned long > > > end) { > > > - flush_dcache_all(); > > > } > > > > > > void cache_flush(void) > > > -- > > > 2.20.1
Hi Lukas Auer, Lukas <lukas.auer@aisec.fraunhofer.de> 於 2019年1月2日 週三 下午8:22寫道： > > Hi Rick, > > On Wed, 2019-01-02 at 10:54 +0800, Rick Chen wrote: > > Hi Lukas > > > > > > From: Lukas Auer [mailto:lukas.auer@aisec.fraunhofer.de] > > > > Sent: Monday, December 31, 2018 2:28 AM > > > > To: u-boot@lists.denx.de > > > > Cc: Anup Patel; Lukas Auer; Rick Jian-Zhi Chen(陳建志); Bin Meng; > > > > Greentime Hu > > > > Subject: [PATCH 2/6] riscv: remove invalid dcache flush > > > > implementation > > > > > > > > The fence instruction is used to enforce device I/O and memory > > > > ordering > > > > constraints in RISC-V. It does not directly affect the data cache > > > > and particular > > > > cannot be used to flush or invalidate it. RISC-V does not have > > > > instructions for > > > > explicit cache control. Remove the flush_dcache_all > > > > implementation and its use > > > > in all dcache-specific functions in lib/cache.c. > > > > > > > > The privileged mention that fence is for the purposes of ordering, > > but > > does not say > > it can not be used to flush or invalidate. > > > > Andes's ax25 have no coherence agent. So it need fence instruction to > > manipulate > > cache flush or invalidate. Or there will be some data synchronization > > error > > occurs on ae350 platform. > > > > I was not aware of this. Does the ax25 flush the whole dcache on any > fence instruction? Yes. It will flush the whole dcache when execute fence. > > You are right, we can't remove it in this case. Since it is specific to > the ax25, should I move it into cpu/ax25/cache.c instead? OK It is good to me. And thanks for your understanding and improvements. Regards Rick > > Thanks, > Lukas > > > Thanks > > Rick > > > > > > > > This also adds a missing new line between flush_dcache_all and > > > > flush_dcache_range in lib/cache.c. > > > > > > > > Signed-off-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de> > > > > --- > > > > This patch only removes the implementation itself and its use in > > > > dcache-specific > > > > functions in lib/cache.c. There are more uses of it in > > > > arch/riscv/, which this patch > > > > does not remove. > > > > > > > > arch/riscv/lib/cache.c | 4 +--- > > > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > > > > > diff --git a/arch/riscv/lib/cache.c b/arch/riscv/lib/cache.c > > > > index > > > > ae5c60716f..203e287612 100644 > > > > --- a/arch/riscv/lib/cache.c > > > > +++ b/arch/riscv/lib/cache.c > > > > @@ -13,11 +13,10 @@ void invalidate_icache_all(void) > > > > > > > > void flush_dcache_all(void) > > > > { > > > > - asm volatile ("fence" :::"memory"); > > > > } > > > > + > > > > void flush_dcache_range(unsigned long start, unsigned long > > > > end) { > > > > - flush_dcache_all(); > > > > } > > > > > > > > void invalidate_icache_range(unsigned long start, unsigned long > > > > end) @@ -31,7 > > > > +30,6 @@ void invalidate_icache_range(unsigned long start, > > > > unsigned long end) > > > > > > > > void invalidate_dcache_range(unsigned long start, unsigned long > > > > end) { > > > > - flush_dcache_all(); > > > > } > > > > > > > > void cache_flush(void) > > > > -- > > > > 2.20.1
Hi Rick, On Thu, 2019-01-03 at 08:48 +0800, Rick Chen wrote: > Hi Lukas > > Auer, Lukas <lukas.auer@aisec.fraunhofer.de> 於 2019年1月2日 週三 下午8:22寫道： > > Hi Rick, > > > > On Wed, 2019-01-02 at 10:54 +0800, Rick Chen wrote: > > > Hi Lukas > > > > > > > > From: Lukas Auer [mailto:lukas.auer@aisec.fraunhofer.de] > > > > > Sent: Monday, December 31, 2018 2:28 AM > > > > > To: u-boot@lists.denx.de > > > > > Cc: Anup Patel; Lukas Auer; Rick Jian-Zhi Chen(陳建志); Bin > > > > > Meng; > > > > > Greentime Hu > > > > > Subject: [PATCH 2/6] riscv: remove invalid dcache flush > > > > > implementation > > > > > > > > > > The fence instruction is used to enforce device I/O and > > > > > memory > > > > > ordering > > > > > constraints in RISC-V. It does not directly affect the data > > > > > cache > > > > > and particular > > > > > cannot be used to flush or invalidate it. RISC-V does not > > > > > have > > > > > instructions for > > > > > explicit cache control. Remove the flush_dcache_all > > > > > implementation and its use > > > > > in all dcache-specific functions in lib/cache.c. > > > > > > > > > > > The privileged mention that fence is for the purposes of > > > ordering, > > > but > > > does not say > > > it can not be used to flush or invalidate. > > > > > > Andes's ax25 have no coherence agent. So it need fence > > > instruction to > > > manipulate > > > cache flush or invalidate. Or there will be some data > > > synchronization > > > error > > > occurs on ae350 platform. > > > > > > > I was not aware of this. Does the ax25 flush the whole dcache on > > any > > fence instruction? > > Yes. > It will flush the whole dcache when execute fence. > > > You are right, we can't remove it in this case. Since it is > > specific to > > the ax25, should I move it into cpu/ax25/cache.c instead? > > OK > It is good to me. > And thanks for your understanding and improvements. > You are welcome! I just submitted version 2 of the patch series. Thanks, Lukas > Regards > Rick > > > Thanks, > > Lukas > > > > > Thanks > > > Rick > > > > > > > > > > > This also adds a missing new line between flush_dcache_all > > > > > and > > > > > flush_dcache_range in lib/cache.c. > > > > > > > > > > Signed-off-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de> > > > > > --- > > > > > This patch only removes the implementation itself and its use > > > > > in > > > > > dcache-specific > > > > > functions in lib/cache.c. There are more uses of it in > > > > > arch/riscv/, which this patch > > > > > does not remove. > > > > > > > > > > arch/riscv/lib/cache.c | 4 +--- > > > > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > > > > > > > diff --git a/arch/riscv/lib/cache.c b/arch/riscv/lib/cache.c > > > > > index > > > > > ae5c60716f..203e287612 100644 > > > > > --- a/arch/riscv/lib/cache.c > > > > > +++ b/arch/riscv/lib/cache.c > > > > > @@ -13,11 +13,10 @@ void invalidate_icache_all(void) > > > > > > > > > > void flush_dcache_all(void) > > > > > { > > > > > - asm volatile ("fence" :::"memory"); > > > > > } > > > > > + > > > > > void flush_dcache_range(unsigned long start, unsigned long > > > > > end) { > > > > > - flush_dcache_all(); > > > > > } > > > > > > > > > > void invalidate_icache_range(unsigned long start, unsigned > > > > > long > > > > > end) @@ -31,7 > > > > > +30,6 @@ void invalidate_icache_range(unsigned long start, > > > > > unsigned long end) > > > > > > > > > > void invalidate_dcache_range(unsigned long start, unsigned > > > > > long > > > > > end) { > > > > > - flush_dcache_all(); > > > > > } > > > > > > > > > > void cache_flush(void) > > > > > -- > > > > > 2.20.1
Hi Lukas Auer, Lukas <lukas.auer@aisec.fraunhofer.de> 於 2019年1月4日 週五 上午8:40寫道： > > Hi Rick, > > On Thu, 2019-01-03 at 08:48 +0800, Rick Chen wrote: > > Hi Lukas > > > > Auer, Lukas <lukas.auer@aisec.fraunhofer.de> 於 2019年1月2日 週三 下午8:22寫道： > > > Hi Rick, > > > > > > On Wed, 2019-01-02 at 10:54 +0800, Rick Chen wrote: > > > > Hi Lukas > > > > > > > > > > From: Lukas Auer [mailto:lukas.auer@aisec.fraunhofer.de] > > > > > > Sent: Monday, December 31, 2018 2:28 AM > > > > > > To: u-boot@lists.denx.de > > > > > > Cc: Anup Patel; Lukas Auer; Rick Jian-Zhi Chen(陳建志); Bin > > > > > > Meng; > > > > > > Greentime Hu > > > > > > Subject: [PATCH 2/6] riscv: remove invalid dcache flush > > > > > > implementation > > > > > > > > > > > > The fence instruction is used to enforce device I/O and > > > > > > memory > > > > > > ordering > > > > > > constraints in RISC-V. It does not directly affect the data > > > > > > cache > > > > > > and particular > > > > > > cannot be used to flush or invalidate it. RISC-V does not > > > > > > have > > > > > > instructions for > > > > > > explicit cache control. Remove the flush_dcache_all > > > > > > implementation and its use > > > > > > in all dcache-specific functions in lib/cache.c. > > > > > > > > > > > > > > The privileged mention that fence is for the purposes of > > > > ordering, > > > > but > > > > does not say > > > > it can not be used to flush or invalidate. > > > > > > > > Andes's ax25 have no coherence agent. So it need fence > > > > instruction to > > > > manipulate > > > > cache flush or invalidate. Or there will be some data > > > > synchronization > > > > error > > > > occurs on ae350 platform. > > > > > > > > > > I was not aware of this. Does the ax25 flush the whole dcache on > > > any > > > fence instruction? > > > > Yes. > > It will flush the whole dcache when execute fence. > > > > > You are right, we can't remove it in this case. Since it is > > > specific to > > > the ax25, should I move it into cpu/ax25/cache.c instead? > > > > OK > > It is good to me. > > And thanks for your understanding and improvements. > > > > You are welcome! I just submitted version 2 of the patch series. > OK Thanks for your efforts. B.R Rick > Thanks, > Lukas > > > Regards > > Rick > > > > > Thanks, > > > Lukas > > > > > > > Thanks > > > > Rick > > > > > > > > > > > > > > This also adds a missing new line between flush_dcache_all > > > > > > and > > > > > > flush_dcache_range in lib/cache.c. > > > > > > > > > > > > Signed-off-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de> > > > > > > --- > > > > > > This patch only removes the implementation itself and its use > > > > > > in > > > > > > dcache-specific > > > > > > functions in lib/cache.c. There are more uses of it in > > > > > > arch/riscv/, which this patch > > > > > > does not remove. > > > > > > > > > > > > arch/riscv/lib/cache.c | 4 +--- > > > > > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > > > > > > > > > diff --git a/arch/riscv/lib/cache.c b/arch/riscv/lib/cache.c > > > > > > index > > > > > > ae5c60716f..203e287612 100644 > > > > > > --- a/arch/riscv/lib/cache.c > > > > > > +++ b/arch/riscv/lib/cache.c > > > > > > @@ -13,11 +13,10 @@ void invalidate_icache_all(void) > > > > > > > > > > > > void flush_dcache_all(void) > > > > > > { > > > > > > - asm volatile ("fence" :::"memory"); > > > > > > } > > > > > > + > > > > > > void flush_dcache_range(unsigned long start, unsigned long > > > > > > end) { > > > > > > - flush_dcache_all(); > > > > > > } > > > > > > > > > > > > void invalidate_icache_range(unsigned long start, unsigned > > > > > > long > > > > > > end) @@ -31,7 > > > > > > +30,6 @@ void invalidate_icache_range(unsigned long start, > > > > > > unsigned long end) > > > > > > > > > > > > void invalidate_dcache_range(unsigned long start, unsigned > > > > > > long > > > > > > end) { > > > > > > - flush_dcache_all(); > > > > > > } > > > > > > > > > > > > void cache_flush(void) > > > > > > -- > > > > > > 2.20.1
diff --git a/arch/riscv/lib/cache.c b/arch/riscv/lib/cache.c index ae5c60716f..203e287612 100644 --- a/arch/riscv/lib/cache.c +++ b/arch/riscv/lib/cache.c @@ -13,11 +13,10 @@ void invalidate_icache_all(void) void flush_dcache_all(void) { - asm volatile ("fence" :::"memory"); } + void flush_dcache_range(unsigned long start, unsigned long end) { - flush_dcache_all(); } void invalidate_icache_range(unsigned long start, unsigned long end) @@ -31,7 +30,6 @@ void invalidate_icache_range(unsigned long start, unsigned long end) void invalidate_dcache_range(unsigned long start, unsigned long end) { - flush_dcache_all(); } void cache_flush(void)
The fence instruction is used to enforce device I/O and memory ordering constraints in RISC-V. It does not directly affect the data cache and particular cannot be used to flush or invalidate it. RISC-V does not have instructions for explicit cache control. Remove the flush_dcache_all implementation and its use in all dcache-specific functions in lib/cache.c. This also adds a missing new line between flush_dcache_all and flush_dcache_range in lib/cache.c. Signed-off-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de> --- This patch only removes the implementation itself and its use in dcache-specific functions in lib/cache.c. There are more uses of it in arch/riscv/, which this patch does not remove. arch/riscv/lib/cache.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)