Messages in this thread | | | From | Jeungwoo Yoo <> | Date | Tue, 6 Sep 2022 14:35:02 +0200 | Subject | Re: [PATCH] arm64: Remove d-cache clean operation at preserve_boot_args(). |
| |
2022년 9월 5일 (월) 오후 12:47, Robin Murphy <robin.murphy@arm.com>님이 작성: > > On 2022-09-04 20:30, Jeungwoo Yoo wrote: > > Kernel expects only the clean operation as a booting requirement in > > arm64 architecture [1], therefore, the kernel has to invalidate any > > cache entries after accessing a memory in the booting time (before > > enabling D-cache and MMU) not to overwrite the memory with the stale > > cache entry. > > > > Same applied in preserve_boot_args(), kernel saves boot arguments into > > 'boot_args' and invalidates the corresponding cache entry. However, > > according to the 'dcache_inval_poc()' implementation, the cache entry > > will be not only invalidated but also cleaned. That means if there is a > > stale cache entry corresponding to the address of the 'boot_args', the > > saved boot arguments in 'boot_args' will be overwritten by the stale > > cache entry. Therefore, it uses 'dv ivac' instruction directly instead > > of calling 'dcache_inval_poc()'. > > You've already said in the first paragraph that we expect these > locations to be clean. Clean lines are not written back, so your > reasoning here is spurious. If boot_args has somehow become dirtied such > that the clean operation *would* write back to memory, that can only > mean one of two things: either the kernel image was not cleaned per the > boot protocol, in which case there's every chance that other things will > also go wrong elsewhere and there's not much we can do, or the prior > stores hit in the cache (either unexpectedly or because the MMU was left > on), in which case we almost certainly *would* want writeback here anyway.
Yes, you are right. I misunderstood that the "clean operation" will propagate the content from the d-cache to the memory always. However, as the cache line is already cleaned by the bootloader and not modified in the d-cache, the "clean operation" in "dcache_inval_poc()" will be ignored. Thank you for your correction.
> > > The address of the 'boot_args' is aligned to the cache line size and the > > size of 'boot_args' is 32 byte (8 byte * 4), therefore, a single > > invalidate operation is enough to invalidate the cache line belonging to > > the 'boot_args'. > > The architecture allows the CWG to be as small as 2 words, so this is > clearly untrue. > > Thanks, > Robin. > > > Sometimes clean operation is required not to lose any contents in the > > cache entry but not the target of the invalidation. However, in this > > case, there is no valid cache entries at a very early booting stage and > > preserve_boot_args() is not called by any other (non-primary) CPUs. > > Therefore, this invalidation operation will not introduce any problems. > > > > [1] in Documentation/arm64/booting.rst: > > The address range corresponding to the loaded kernel image must be > > cleaned to the PoC. > > > > Co-developed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> > > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> > > > > Co-developed-by: Sangyun Kim <sangyun.kim@snu.ac.kr> > > Signed-off-by: Sangyun Kim <sangyun.kim@snu.ac.kr> > > > > Signed-off-by: Jeungwoo Yoo <casionwoo@gmail.com> > > --- > > arch/arm64/kernel/head.S | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S > > index cefe6a73ee54..916227666b07 100644 > > --- a/arch/arm64/kernel/head.S > > +++ b/arch/arm64/kernel/head.S > > @@ -121,9 +121,7 @@ SYM_CODE_START_LOCAL(preserve_boot_args) > > > > dmb sy // needed before dc ivac with > > // MMU off > > - > > - add x1, x0, #0x20 // 4 x 8 bytes > > - b dcache_inval_poc // tail call > > + dc ivac, x0 // Invalidate potentially stale cache line > > SYM_CODE_END(preserve_boot_args) > > > > SYM_FUNC_START_LOCAL(clear_page_tables)
-- Best Regards, Jeungwoo Yoo
| |