| Message ID | 20230127213941.83967-1-deller@gmx.de (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | parisc: Ensure page-aligned addresses in cache flush and copy functions | expand |
On Fri, Jan 27, 2023 at 10:39:41PM +0100, Helge Deller wrote: > Matthew Wilcox noticed, that if ARCH_HAS_FLUSH_ON_KUNMAP is defined > (which is the case for PA-RISC), __kunmap_local() calls > kunmap_flush_on_unmap(), which may call the parisc flush functions with > a non-page-aligned address and thus the page might not be fully flushed. > > To prevent similiar cases, page-align any given address in the > following parisc low-level calls: > - clear_page_asm(), > - copy_page_asm(), > - copy_user_page_asm(), > - clear_user_page_asm(), > - flush_kernel_dcache_page_asm(), > - purge_kernel_dcache_page_asm() and > - flush_kernel_icache_page() I don't think this is the right way to go. Imagine that we enable large folios on architectures that don't support TRANSPARENT_HUGEPAGE (ie PA-RISC). Then folio_zero_range() is going to call kmap_local() and kunmap_local() once per folio instead of once per page, and so we'll need to flush the entire folio, not just the page.
On 1/28/23 00:39, Matthew Wilcox wrote: > On Fri, Jan 27, 2023 at 10:39:41PM +0100, Helge Deller wrote: >> Matthew Wilcox noticed, that if ARCH_HAS_FLUSH_ON_KUNMAP is defined >> (which is the case for PA-RISC), __kunmap_local() calls >> kunmap_flush_on_unmap(), which may call the parisc flush functions with >> a non-page-aligned address and thus the page might not be fully flushed. >> >> To prevent similiar cases, page-align any given address in the >> following parisc low-level calls: >> - clear_page_asm(), >> - copy_page_asm(), >> - copy_user_page_asm(), >> - clear_user_page_asm(), >> - flush_kernel_dcache_page_asm(), >> - purge_kernel_dcache_page_asm() and >> - flush_kernel_icache_page() > > I don't think this is the right way to go. Imagine that we enable > large folios on architectures that don't support TRANSPARENT_HUGEPAGE > (ie PA-RISC). Then folio_zero_range() is going to call kmap_local() > and kunmap_local() once per folio instead of once per page, and so we'll > need to flush the entire folio, not just the page. Don't the functions mentioned above in my patch operate on standard 4k page size for all architectures? So, if you want to flush a whole folio you will probably not want to call those functions by iterating over the folio-memory, but call some other function, right? In that case flush_kernel_dcache_range_asm() is probably better suited then, as it doesn't page-align the given address. Helge
> On 27 Jan 2023, at 21:39, Helge Deller <deller@gmx.de> wrote: > > Matthew Wilcox noticed, that if ARCH_HAS_FLUSH_ON_KUNMAP is defined > (which is the case for PA-RISC), __kunmap_local() calls > kunmap_flush_on_unmap(), which may call the parisc flush functions with > a non-page-aligned address and thus the page might not be fully flushed. > > To prevent similiar cases, page-align any given address in the > following parisc low-level calls: > - clear_page_asm(), > - copy_page_asm(), > - copy_user_page_asm(), > - clear_user_page_asm(), > - flush_kernel_dcache_page_asm(), > - purge_kernel_dcache_page_asm() and > - flush_kernel_icache_page() > > Signed-off-by: Helge Deller <deller@gmx.de> > Cc: Matthew Wilcox <willy@infradead.org> > Cc: Al Viro <viro@zeniv.linux.org.uk> > Cc: Ira Weiny <ira.weiny@intel.com> > Cc: Fabio M. De Francesco <fmdefrancesco@gmail.com> Is this patch obsolete as of https://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux.git/commit/?h=for-next&id=76008c1008dca3cdd7709f4a468b0c3ff9787632 or still relevant? Asking because I want to know if I should carry on testing with it or not. Thanks!
On 2/28/23 01:20, Sam James wrote: > > >> On 27 Jan 2023, at 21:39, Helge Deller <deller@gmx.de> wrote: >> >> Matthew Wilcox noticed, that if ARCH_HAS_FLUSH_ON_KUNMAP is defined >> (which is the case for PA-RISC), __kunmap_local() calls >> kunmap_flush_on_unmap(), which may call the parisc flush functions with >> a non-page-aligned address and thus the page might not be fully flushed. >> >> To prevent similiar cases, page-align any given address in the >> following parisc low-level calls: >> - clear_page_asm(), >> - copy_page_asm(), >> - copy_user_page_asm(), >> - clear_user_page_asm(), >> - flush_kernel_dcache_page_asm(), >> - purge_kernel_dcache_page_asm() and >> - flush_kernel_icache_page() >> >> Signed-off-by: Helge Deller <deller@gmx.de> >> Cc: Matthew Wilcox <willy@infradead.org> >> Cc: Al Viro <viro@zeniv.linux.org.uk> >> Cc: Ira Weiny <ira.weiny@intel.com> >> Cc: Fabio M. De Francesco <fmdefrancesco@gmail.com> > > Is this patch obsolete as of https://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux.git/commit/?h=for-next&id=76008c1008dca3cdd7709f4a468b0c3ff9787632 > or still relevant? > > Asking because I want to know if I should carry on testing with it or not. You can drop it. I'm going to drop it from my for-next git tree as well. Helge
diff --git a/arch/parisc/kernel/pacache.S b/arch/parisc/kernel/pacache.S index 9a0018f1f42c..b59c55fc35e9 100644 --- a/arch/parisc/kernel/pacache.S +++ b/arch/parisc/kernel/pacache.S @@ -310,6 +310,8 @@ ENDPROC_CFI(flush_data_cache_local) /* Clear page using kernel mapping. */ ENTRY_CFI(clear_page_asm) + depi_safe 0, 31,PAGE_SHIFT, %r26 /* Clear any offset bits */ + #ifdef CONFIG_64BIT /* Unroll the loop. */ @@ -373,6 +375,9 @@ ENDPROC_CFI(clear_page_asm) /* Copy page using kernel mapping. */ ENTRY_CFI(copy_page_asm) + depi_safe 0, 31,PAGE_SHIFT, %r26 /* Clear any offset bits */ + depi_safe 0, 31,PAGE_SHIFT, %r25 /* Clear any offset bits */ + #ifdef CONFIG_64BIT /* PA8x00 CPUs can consume 2 loads or 1 store per cycle. * Unroll the loop by hand and arrange insn appropriately. @@ -525,6 +530,9 @@ ENDPROC_CFI(copy_page_asm) */ ENTRY_CFI(copy_user_page_asm) + depi_safe 0, 31,PAGE_SHIFT, %r26 /* Clear any offset bits */ + depi_safe 0, 31,PAGE_SHIFT, %r25 /* Clear any offset bits */ + /* Convert virtual `to' and `from' addresses to physical addresses. Move `from' physical address to non shadowed register. */ ldil L%(__PAGE_OFFSET), %r1 @@ -662,6 +670,7 @@ ENTRY_CFI(copy_user_page_asm) ENDPROC_CFI(copy_user_page_asm) ENTRY_CFI(clear_user_page_asm) + depi_safe 0, 31,PAGE_SHIFT, %r26 /* Clear any offset bits */ tophys_r1 %r26 ldil L%(TMPALIAS_MAP_START), %r28 @@ -889,6 +898,7 @@ ENDPROC_CFI(flush_icache_page_asm) ENTRY_CFI(flush_kernel_dcache_page_asm) 88: ldil L%dcache_stride, %r1 ldw R%dcache_stride(%r1), %r23 + depi_safe 0, 31,PAGE_SHIFT, %r26 /* Clear any offset bits */ #ifdef CONFIG_64BIT depdi,z 1, 63-PAGE_SHIFT,1, %r25 @@ -925,6 +935,7 @@ ENDPROC_CFI(flush_kernel_dcache_page_asm) ENTRY_CFI(purge_kernel_dcache_page_asm) 88: ldil L%dcache_stride, %r1 ldw R%dcache_stride(%r1), %r23 + depi_safe 0, 31,PAGE_SHIFT, %r26 /* Clear any offset bits */ #ifdef CONFIG_64BIT depdi,z 1, 63-PAGE_SHIFT,1, %r25 @@ -1125,6 +1136,7 @@ ENDPROC_CFI(flush_user_icache_range_asm) ENTRY_CFI(flush_kernel_icache_page) 88: ldil L%icache_stride, %r1 ldw R%icache_stride(%r1), %r23 + depi_safe 0, 31,PAGE_SHIFT, %r26 /* Clear any offset bits */ #ifdef CONFIG_64BIT depdi,z 1, 63-PAGE_SHIFT,1, %r25
Matthew Wilcox noticed, that if ARCH_HAS_FLUSH_ON_KUNMAP is defined (which is the case for PA-RISC), __kunmap_local() calls kunmap_flush_on_unmap(), which may call the parisc flush functions with a non-page-aligned address and thus the page might not be fully flushed. To prevent similiar cases, page-align any given address in the following parisc low-level calls: - clear_page_asm(), - copy_page_asm(), - copy_user_page_asm(), - clear_user_page_asm(), - flush_kernel_dcache_page_asm(), - purge_kernel_dcache_page_asm() and - flush_kernel_icache_page() Signed-off-by: Helge Deller <deller@gmx.de> Cc: Matthew Wilcox <willy@infradead.org> Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: Ira Weiny <ira.weiny@intel.com> Cc: Fabio M. De Francesco <fmdefrancesco@gmail.com> --- arch/parisc/kernel/pacache.S | 12 ++++++++++++ 1 file changed, 12 insertions(+) -- 2.38.1