[PATCH] powerpc/powermac: Use early_* IO variants in via_calibrate_decr

[PATCH] powerpc/powermac: Use early_* IO variants in via_calibrate_decr

[Benjamin Gray]

On a powermac platform, under the call path

  start_kernel
   time_init
    ppc_md.calibrate_decr (pmac_calibrate_decr)
     via_calibrate_decr

we run ioremap and iounmap. The unmap can enable interrupts
unexpectedly (cond_resched in vunmap_pmd_range), which is warned about
later in the boot sequence in start_kernel.

Use the early_* variants of these IO functions to prevent this.

The issue is pre-existing, but is surfaced by commit 721255b9826b
("genirq: Use a maple tree for interrupt descriptor management").
It's not clear to me why this causes it to surface.

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
---
 arch/powerpc/platforms/powermac/time.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/powermac/time.c b/arch/powerpc/platforms/powermac/time.c
index 4c5790aff1b5..8633891b7aa5 100644
--- a/arch/powerpc/platforms/powermac/time.c
+++ b/arch/powerpc/platforms/powermac/time.c
@@ -26,8 +26,8 @@
 #include <linux/rtc.h>
 #include <linux/of_address.h>
 
+#include <asm/early_ioremap.h>
 #include <asm/sections.h>
-#include <asm/io.h>
 #include <asm/machdep.h>
 #include <asm/time.h>
 #include <asm/nvram.h>
@@ -182,7 +182,7 @@ static int __init via_calibrate_decr(void)
 		return 0;
 	}
 	of_node_put(vias);
-	via = ioremap(rsrc.start, resource_size(&rsrc));
+	via = early_ioremap(rsrc.start, resource_size(&rsrc));
 	if (via == NULL) {
 		printk(KERN_ERR "Failed to map VIA for timer calibration !\n");
 		return 0;
@@ -207,7 +207,7 @@ static int __init via_calibrate_decr(void)
 
 	ppc_tb_freq = (dstart - dend) * 100 / 6;
 
-	iounmap(via);
+	early_iounmap((void *)via, resource_size(&rsrc));
 
 	return 1;
 }
-- 
2.41.0

Re: [PATCH] powerpc/powermac: Use early_* IO variants in via_calibrate_decr

[Christophe Leroy]

Le 06/07/2023 à 03:08, Benjamin Gray a écrit :
> On a powermac platform, under the call path
> 
>    start_kernel
>     time_init
>      ppc_md.calibrate_decr (pmac_calibrate_decr)
>       via_calibrate_decr
> 
> we run ioremap and iounmap. The unmap can enable interrupts
> unexpectedly (cond_resched in vunmap_pmd_range), which is warned about
> later in the boot sequence in start_kernel.
> 
> Use the early_* variants of these IO functions to prevent this.
> 
> The issue is pre-existing, but is surfaced by commit 721255b9826b
> ("genirq: Use a maple tree for interrupt descriptor management").
> It's not clear to me why this causes it to surface.
> 
> Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>

> ---
>   arch/powerpc/platforms/powermac/time.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powermac/time.c b/arch/powerpc/platforms/powermac/time.c
> index 4c5790aff1b5..8633891b7aa5 100644
> --- a/arch/powerpc/platforms/powermac/time.c
> +++ b/arch/powerpc/platforms/powermac/time.c
> @@ -26,8 +26,8 @@
>   #include <linux/rtc.h>
>   #include <linux/of_address.h>
>   
> +#include <asm/early_ioremap.h>
>   #include <asm/sections.h>
> -#include <asm/io.h>
>   #include <asm/machdep.h>
>   #include <asm/time.h>
>   #include <asm/nvram.h>
> @@ -182,7 +182,7 @@ static int __init via_calibrate_decr(void)
>   		return 0;
>   	}
>   	of_node_put(vias);
> -	via = ioremap(rsrc.start, resource_size(&rsrc));
> +	via = early_ioremap(rsrc.start, resource_size(&rsrc));
>   	if (via == NULL) {
>   		printk(KERN_ERR "Failed to map VIA for timer calibration !\n");
>   		return 0;
> @@ -207,7 +207,7 @@ static int __init via_calibrate_decr(void)
>   
>   	ppc_tb_freq = (dstart - dend) * 100 / 6;
>   
> -	iounmap(via);
> +	early_iounmap((void *)via, resource_size(&rsrc));
>   
>   	return 1;
>   }

Re: [PATCH] powerpc/powermac: Use early_* IO variants in via_calibrate_decr

[Benjamin Gray]

On Thu, 2023-07-06 at 11:08 +1000, Benjamin Gray wrote:
> The issue is pre-existing, but is surfaced by commit 721255b9826b
> ("genirq: Use a maple tree for interrupt descriptor management").
> It's not clear to me why this causes it to surface.

From the thread chain in [1], it looks like the maple tree
implementation just has different allocation behaviour, which follows a
pre-existing code path to kmem_cache_alloc_bulk(), which
unconditionally enables interrupts.

(thanks Jordan Niethe for finding this thread)

[1]: https://lore.kernel.org/all/87o7qdzfay.ffs@tglx/

Re: [PATCH] powerpc/powermac: Use early_* IO variants in via_calibrate_decr

[Michael Ellerman]

Benjamin Gray <bgray@linux.ibm.com> writes:
> On Thu, 2023-07-06 at 11:08 +1000, Benjamin Gray wrote:
>> The issue is pre-existing, but is surfaced by commit 721255b9826b
>> ("genirq: Use a maple tree for interrupt descriptor management").
>> It's not clear to me why this causes it to surface.
>
> From the thread chain in [1], it looks like the maple tree
> implementation just has different allocation behaviour, which follows a
> pre-existing code path to kmem_cache_alloc_bulk(), which
> unconditionally enables interrupts.

That was a bug that was fixed before the series was merged.

See:
  f5451547b831 ("mm, slab/slub: Ensure kmem_cache_alloc_bulk() is available early")


It looks like the trigger here is that the maple tree code uses
call_rcu() in ma_free_rcu(), and call_rcu() can cause TIF_NEED_RESCHED
to be set, which causes cond_resched() to actually reschedule, enabling
interrupts.

cheers


> (thanks Jordan Niethe for finding this thread)
>
> [1]: https://lore.kernel.org/all/87o7qdzfay.ffs@tglx/