[PATCH] parisc: Ensure page-aligned addresses in cache flush and copy functions

[PATCH] parisc: Ensure page-aligned addresses in cache flush and copy functions

[Helge Deller]

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(+)

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
--
2.38.1

Re: [PATCH] parisc: Ensure page-aligned addresses in cache flush and copy functions

[Matthew Wilcox]

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.

Re: [PATCH] parisc: Ensure page-aligned addresses in cache flush and copy functions

[Helge Deller]

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

Re: [PATCH] parisc: Ensure page-aligned addresses in cache flush and copy functions

[Sam James]

[-- Attachment #1: Type: text/plain, Size: 1176 bytes --]



> 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!

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 358 bytes --]

Re: [PATCH] parisc: Ensure page-aligned addresses in cache flush and copy functions

[Helge Deller]

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