Le 17/05/2022 à 09:31, Paul Mackerras a écrit :
> On Sun, May 15, 2022 at 07:33:52AM +0000, Christophe Leroy wrote:
>>
>>
>> Le 11/05/2022 à 09:28, Paul Mackerras a écrit :
>>> From: Daniel Axtens <d...@axtens.net>
>>>
>>> Implement a limited form of KASAN for Book3S 64-bit machines running under
>>> the Radix MMU, supporting only outline mode.
>
> [snip]
>
>>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>>> index b779603978e1..0bf76e40c9c2 100644
>>> --- a/arch/powerpc/Kconfig
>>> +++ b/arch/powerpc/Kconfig
>>> @@ -105,6 +105,7 @@ config PPC
>>> # Please keep this list sorted alphabetically.
>>> #
>>> select ARCH_32BIT_OFF_T if PPC32
>>> + select ARCH_DISABLE_KASAN_INLINE if PPC_RADIX_MMU
>>> select ARCH_ENABLE_MEMORY_HOTPLUG
>>> select ARCH_ENABLE_MEMORY_HOTREMOVE
>>> select ARCH_HAS_COPY_MC if PPC64
>>> @@ -152,6 +153,7 @@ config PPC
>>> select ARCH_WANT_IPC_PARSE_VERSION
>>> select ARCH_WANT_IRQS_OFF_ACTIVATE_MM
>>> select ARCH_WANT_LD_ORPHAN_WARN
>>> + select ARCH_WANTS_NO_INSTR
>>
>> Can you explain why we need that ?
>
> The help text for the option says:
>
> An architecture should select this if the noinstr macro is being used on
> functions to denote that the toolchain should avoid instrumenting such
> functions and is required for correctness.
>
> All it really seems to do is restrict the conditions under which the
> GCOV and KCOV options can be enabled.
>
>> Is it tied to KASAN ? Is yes, why didn't we have it for PPC32 until now ?
>
> Since we really do need to avoid instrumenting certain functions on
> ppc64 (as in things will break if we do instrument them), I think we
> need to select ARCH_WANTS_NO_INSTR.
>
> For ppc32, as far as I recall there is much less code that runs in
> real mode and it is mostly assembler (except for some boot code),
> mostly because all addresses have to be explicitly translated to
> physical addresses for 32-bit real-mode code, unlike ppc64 where we
> can use access memory in the linear mapping using virtual addresses
> because of the fact that the CPU ignores the top 4 bits of the
> effective address in real mode. That said, there is a lot less code
> that runs in real mode on ppc64 than there used to be.
>
>> Maybe that's independant of KASAN and would be worth a separate patch ?
>
> Yes, possibly, though KASAN does appear to be the only user of noinstr
> in arch/powerpc.
>
> [snip]
>
>>> diff --git a/arch/powerpc/include/asm/book3s/64/radix.h
>>> b/arch/powerpc/include/asm/book3s/64/radix.h
>>> index d090d9612348..bafc9869afcd 100644
>>> --- a/arch/powerpc/include/asm/book3s/64/radix.h
>>> +++ b/arch/powerpc/include/asm/book3s/64/radix.h
>>> @@ -35,6 +35,11 @@
>>> #define RADIX_PMD_SHIFT (PAGE_SHIFT + RADIX_PTE_INDEX_SIZE)
>>> #define RADIX_PUD_SHIFT (RADIX_PMD_SHIFT + RADIX_PMD_INDEX_SIZE)
>>> #define RADIX_PGD_SHIFT (RADIX_PUD_SHIFT + RADIX_PUD_INDEX_SIZE)
>>> +
>>> +#define R_PTRS_PER_PTE (1 << RADIX_PTE_INDEX_SIZE)
>>> +#define R_PTRS_PER_PMD (1 << RADIX_PMD_INDEX_SIZE)
>>> +#define R_PTRS_PER_PUD (1 << RADIX_PUD_INDEX_SIZE)
>>> +
>>> /*
>>> * Size of EA range mapped by our pagetables.
>>> */
>>> @@ -68,11 +73,11 @@
>>> *
>>> *
>>> * 3rd quadrant expanded:
>>> - * +------------------------------+
>>> + * +------------------------------+ Highest address (0xc010000000000000)
>>> + * +------------------------------+ KASAN shadow end (0xc00fc00000000000)
>>> * | |
>>> * | |
>>> - * | |
>>> - * +------------------------------+ Kernel vmemmap end
>>> (0xc010000000000000)
>>> + * +------------------------------+ Kernel vmemmap end/shadow start
>>> (0xc00e000000000000)
>>> * | |
>>> * | 512TB |
>>> * | |
>>> @@ -126,6 +131,8 @@
>>> #define RADIX_VMEMMAP_SIZE RADIX_KERN_MAP_SIZE
>>> #define RADIX_VMEMMAP_END (RADIX_VMEMMAP_START +
>>> RADIX_VMEMMAP_SIZE)
>>>
>>> +/* For the sizes of the shadow area, see kasan.h */
>>> +
>>
>> Why does this comment pops up here ?
>
> Do you mean, why is it there at all, or do you mean why is it in that
> particular place rather than closer to the "3rd quadrant expanded"
> diagram or something?
Why at this place mainly, I mean I can't see the relationship between
the added comment and the following lines starting with a #ifndef ASSEMBLY.
If the comment is to be usefull, it should be added to a related place I
guess.
>
>>> #ifndef __ASSEMBLY__
>>> #define RADIX_PTE_TABLE_SIZE (sizeof(pte_t) << RADIX_PTE_INDEX_SIZE)
>>> #define RADIX_PMD_TABLE_SIZE (sizeof(pmd_t) << RADIX_PMD_INDEX_SIZE)
>>> diff --git a/arch/powerpc/include/asm/interrupt.h
>>> b/arch/powerpc/include/asm/interrupt.h
>>> index fc28f46d2f9d..fb244b6ca7f0 100644
>>> --- a/arch/powerpc/include/asm/interrupt.h
>>> +++ b/arch/powerpc/include/asm/interrupt.h
>>> @@ -327,22 +327,46 @@ static inline void interrupt_nmi_enter_prepare(struct
>>> pt_regs *regs, struct inte
>>> }
>>> #endif
>>>
>>> + /* If data relocations are enabled, it's safe to use nmi_enter() */
>>> + if (mfmsr() & MSR_DR) {
>>> + nmi_enter();
>>> + return;
>>> + }
>>> +
>>> /*
>>> - * Do not use nmi_enter() for pseries hash guest taking a real-mode
>>> + * But do not use nmi_enter() for pseries hash guest taking a real-mode
>>> * NMI because not everything it touches is within the RMA limit.
>>> */
>>> + if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) &&
>>> + firmware_has_feature(FW_FEATURE_LPAR) &&
>>> + !radix_enabled())
>>> + return;
>>> +
>>> + /*
>>> + * Likewise, don't use it if we have some form of instrumentation (like
>>> + * KASAN shadow) that is not safe to access in real mode (even on radix)
>>> + */
>>> + if (IS_ENABLED(CONFIG_KASAN))
>>> + return;
>>> +
>>> + /* Otherwise, it should be safe to call it */
>>> + nmi_enter();
>>> }
>>>
>>> static inline void interrupt_nmi_exit_prepare(struct pt_regs *regs,
>>> struct interrupt_nmi_state *state)
>>> {
>>> - if (!IS_ENABLED(CONFIG_PPC_BOOK3S_64) ||
>>> - !firmware_has_feature(FW_FEATURE_LPAR) ||
>>> - radix_enabled() || (mfmsr() & MSR_DR))
>>> + if (mfmsr() & MSR_DR) {
>>> + // nmi_exit if relocations are on
>>> nmi_exit();
>>> + } else if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) &&
>>> + firmware_has_feature(FW_FEATURE_LPAR) &&
>>> + !radix_enabled()) {
>>> + // no nmi_exit for a pseries hash guest taking a real mode
>>> exception
>>> + } else if (IS_ENABLED(CONFIG_KASAN)) {
>>> + // no nmi_exit for KASAN in real mode
>>> + } else {
>>> + nmi_exit();
>>> + }
>>>
>>
>> I think all this changeset around MSR_DR versus NMI would be worth a
>> separate patch with a dedicated explanation.
>
> It's actually not as much of a change as it might appear. All that is
> really being done is to add !IS_ENABLED(CONFIG_KASAN) to the
> conditions for calling nmi_enter() and nmi_exit() when the CPU is in
> real mode. In other words, the condition is equivalent to:
>
> if (((!IS_ENABLED(CONFIG_PPC_BOOK3S_64) ||
> !firmware_has_feature(FW_FEATURE_LPAR) ||
> radix_enabled()) && !IS_ENABLED(CONFIG_KASAN)) ||
> (mfmsr() & MSR_DR))
> nmi_enter();
>
> I think the version as Daniel has transformed it is more readable
> though. It could be a separate patch, sure.
>
>>> /*
>>> * nmi does not call nap_adjust_return because nmi should not create
>>> @@ -410,7 +434,8 @@ static inline void interrupt_nmi_exit_prepare(struct
>>> pt_regs *regs, struct inter
>>> * Specific handlers may have additional restrictions.
>>> */
>>> #define DEFINE_INTERRUPT_HANDLER_RAW(func)
>>> \
>>> -static __always_inline long ____##func(struct pt_regs *regs);
>>> \
>>> +static __always_inline __no_sanitize_address __no_kcsan long
>>> \
>>
>> Why is this required for PPC64 while it wasn't for PPC32 ?
>
> I assume because we switch into virtual mode sooner on ppc32, because
> of the need to transform addresses explicitly in real mode. I haven't
> looked at the ppc32 interrupt code for a long time (and even the ppc64
> interrupt code is now quite different from what I recall).
>
>> Any side effect on PPC32 ?
>
> I suppose you won't get KASAN coverage of those interrupt handling
> functions, where you did previously. Do you want me to make the
> "__no_sanitize_address __no_kcsan" apply only on ppc64?
No, I think that should be OK for PPC32 as well now that you explained
what it really does.
>
> [snip]
>
>>> diff --git a/arch/powerpc/include/asm/kasan.h
>>> b/arch/powerpc/include/asm/kasan.h
>>> index 3c478e5ef24c..6efc822e70fd 100644
>>> --- a/arch/powerpc/include/asm/kasan.h
>>> +++ b/arch/powerpc/include/asm/kasan.h
>>> @@ -30,9 +30,31 @@
>>>
>>> #define KASAN_SHADOW_OFFSET ASM_CONST(CONFIG_KASAN_SHADOW_OFFSET)
>>>
>>> +#ifdef CONFIG_PPC32
>>> #define KASAN_SHADOW_END (-(-KASAN_SHADOW_START >>
>>> KASAN_SHADOW_SCALE_SHIFT))
>>> +#endif
>>>
>>> #ifdef CONFIG_KASAN
>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>> +/*
>>> + * The shadow ends before the highest accessible address
>>> + * because we don't need a shadow for the shadow. Instead:
>>> + * c00e000000000000 << 3 + a80e000000000000 = c00fc00000000000
>>> + */
>>> +#define KASAN_SHADOW_END 0xc00fc00000000000UL
>>
>> Would be easer to read if we could group that with PPC32 in a
>> #ifdef/#else/#endif
>
> Sure. It's a bit weird that KASAN_SHADOW_END is always defined for
> ppc32, but only if CONFIG_KASAN=y for ppc64. I'll fix it.
Thanks
>
> [snip]
>
>>> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
>>> index 4d7829399570..e46335da7f72 100644
>>> --- a/arch/powerpc/kernel/Makefile
>>> +++ b/arch/powerpc/kernel/Makefile
>>> @@ -33,6 +33,17 @@ KASAN_SANITIZE_early_32.o := n
>>> KASAN_SANITIZE_cputable.o := n
>>> KASAN_SANITIZE_prom_init.o := n
>>> KASAN_SANITIZE_btext.o := n
>>> +KASAN_SANITIZE_paca.o := n
>>> +KASAN_SANITIZE_setup_64.o := n
>>> +KASAN_SANITIZE_mce.o := n
>>> +KASAN_SANITIZE_mce_power.o := n
>>> +
>>> +# we have to be particularly careful in ppc64 to exclude code that
>>> +# runs with translations off, as we cannot access the shadow with
>>> +# translations off. However, ppc32 can sanitize this.
>>> +ifdef CONFIG_PPC64
>>> +KASAN_SANITIZE_traps.o := n
>>> +endif
>>>
>>> ifdef CONFIG_KASAN
>>> CFLAGS_early_32.o += -DDISABLE_BRANCH_PROFILING
>>> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
>>> index 984813a4d5dc..c962f9bfc7d4 100644
>>> --- a/arch/powerpc/kernel/process.c
>>> +++ b/arch/powerpc/kernel/process.c
>>> @@ -2220,8 +2220,8 @@ void __no_sanitize_address show_stack(struct
>>> task_struct *tsk,
>>> break;
>>>
>>> stack = (unsigned long *) sp;
>>> - newsp = stack[0];
>>> - ip = stack[STACK_FRAME_LR_SAVE];
>>> + newsp = READ_ONCE_NOCHECK(stack[0]);
>>> + ip = READ_ONCE_NOCHECK(stack[STACK_FRAME_LR_SAVE]);
>>> if (!firstframe || ip != lr) {
>>> printk("%s["REG"] ["REG"] %pS",
>>> loglvl, sp, ip, (void *)ip);
>>> @@ -2239,17 +2239,19 @@ void __no_sanitize_address show_stack(struct
>>> task_struct *tsk,
>>> * See if this is an exception frame.
>>> * We look for the "regshere" marker in the current frame.
>>> */
>>> - if (validate_sp(sp, tsk, STACK_FRAME_WITH_PT_REGS)
>>> - && stack[STACK_FRAME_MARKER] == STACK_FRAME_REGS_MARKER) {
>>> + if (validate_sp(sp, tsk, STACK_FRAME_WITH_PT_REGS) &&
>>> + (READ_ONCE_NOCHECK(stack[STACK_FRAME_MARKER]) ==
>>> + STACK_FRAME_REGS_MARKER)) {
>>> struct pt_regs *regs = (struct pt_regs *)
>>> (sp + STACK_FRAME_OVERHEAD);
>>>
>>> - lr = regs->link;
>>> + lr = READ_ONCE_NOCHECK(regs->link);
>>> printk("%s--- interrupt: %lx at %pS\n",
>>> - loglvl, regs->trap, (void *)regs->nip);
>>> + loglvl, READ_ONCE_NOCHECK(regs->trap),
>>> + (void *)READ_ONCE_NOCHECK(regs->nip));
>>> __show_regs(regs);
>>> printk("%s--- interrupt: %lx\n",
>>> - loglvl, regs->trap);
>>> + loglvl, READ_ONCE_NOCHECK(regs->trap));
>>>
>>> firstframe = 1;
>>> }
>>
>> This change we submitted as a standalone patch and was rejected as is,
>> see
>> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20210528074806.1311297-1-...@axtens.net/
>>
>> v2 was accepted, see
>> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20210614120907.1952321-1-...@axtens.net/
>>
>> Therefore AFAIU this change is redundant with the accepted v2 and should
>> be dropped.
>
> OK, thanks for the history. I'll drop this change.
>
>>> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
>>> index b7fd6a72aa76..5fedadc27952 100644
>>> --- a/arch/powerpc/kernel/smp.c
>>> +++ b/arch/powerpc/kernel/smp.c
>>> @@ -413,32 +413,32 @@ static struct cpumask nmi_ipi_pending_mask;
>>> static bool nmi_ipi_busy = false;
>>> static void (*nmi_ipi_function)(struct pt_regs *) = NULL;
>>>
>>> -static void nmi_ipi_lock_start(unsigned long *flags)
>>> +noinstr static void nmi_ipi_lock_start(unsigned long *flags)
>>> {
>>> raw_local_irq_save(*flags);
>>> hard_irq_disable();
>>> - while (atomic_cmpxchg(&__nmi_ipi_lock, 0, 1) == 1) {
>>> + while (arch_atomic_cmpxchg(&__nmi_ipi_lock, 0, 1) == 1) {
>>> raw_local_irq_restore(*flags);
>>> - spin_until_cond(atomic_read(&__nmi_ipi_lock) == 0);
>>> + spin_until_cond(arch_atomic_read(&__nmi_ipi_lock) == 0);
>>> raw_local_irq_save(*flags);
>>> hard_irq_disable();
>>> }
>>> }
>>>
>>> -static void nmi_ipi_lock(void)
>>> +noinstr static void nmi_ipi_lock(void)
>>> {
>>> - while (atomic_cmpxchg(&__nmi_ipi_lock, 0, 1) == 1)
>>> - spin_until_cond(atomic_read(&__nmi_ipi_lock) == 0);
>>> + while (arch_atomic_cmpxchg(&__nmi_ipi_lock, 0, 1) == 1)
>>> + spin_until_cond(arch_atomic_read(&__nmi_ipi_lock) == 0);
>>> }
>>>
>>> -static void nmi_ipi_unlock(void)
>>> +noinstr static void nmi_ipi_unlock(void)
>>> {
>>> smp_mb();
>>> - WARN_ON(atomic_read(&__nmi_ipi_lock) != 1);
>>> - atomic_set(&__nmi_ipi_lock, 0);
>>> + WARN_ON(arch_atomic_read(&__nmi_ipi_lock) != 1);
>>> + arch_atomic_set(&__nmi_ipi_lock, 0);
>>> }
>>>
>>> -static void nmi_ipi_unlock_end(unsigned long *flags)
>>> +noinstr static void nmi_ipi_unlock_end(unsigned long *flags)
>>> {
>>> nmi_ipi_unlock();
>>> raw_local_irq_restore(*flags);
>>> @@ -447,7 +447,7 @@ static void nmi_ipi_unlock_end(unsigned long *flags)
>>> /*
>>> * Platform NMI handler calls this to ack
>>> */
>>> -int smp_handle_nmi_ipi(struct pt_regs *regs)
>>> +noinstr int smp_handle_nmi_ipi(struct pt_regs *regs)
>>> {
>>> void (*fn)(struct pt_regs *) = NULL;
>>> unsigned long flags;
>>
>> Can you explain those changes from atomic_xxx to arch_atomic_xxx ?
>
> With KASAN enabled, atomic_read() and similar functions become
> wrappers which explicitly call KASAN checking functions. See
> include/linux/atomic/atomic-instrumented.h and
> include/linux/instrumented.h. Since we don't want those NMI functions
> instrumented, we have to use the arch_atomic_* functions so as to
> avoid getting the explicit instrumentation calls. As to why we don't
> want instrumentation here, it's because we want to avoid crashing when
> doing kexec or more particularly kdump, I believe.
>
>> Should it be a separate patch ?
>
> Could be - matter of taste I guess.
Well, this patch being quite big it's usually appreciable to get it
split with some preparation patches, that's the way we do usually, I
remember when I submitted the series to Switch to
CONFIG_THREAD_INFO_IN_TASK, there was already half a dozen of
preparation patches and Michael split them into several more.
>
> [snip]
>
>>> diff --git a/arch/powerpc/mm/book3s64/Makefile
>>> b/arch/powerpc/mm/book3s64/Makefile
>>> index 2d50cac499c5..3150159e5121 100644
>>> --- a/arch/powerpc/mm/book3s64/Makefile
>>> +++ b/arch/powerpc/mm/book3s64/Makefile
>>> @@ -24,3 +24,12 @@ obj-$(CONFIG_PPC_PKEY) += pkeys.o
>>>
>>> # Instrumenting the SLB fault path can lead to duplicate SLB entries
>>> KCOV_INSTRUMENT_slb.o := n
>>> +
>>> +# Parts of these can run in real mode and therefore are
>>> +# not safe with the current outline KASAN implementation
>>
>> But are we not safe about KASAN being disabled when in real mode ? Isn't
>> it the purpose of kasan_arch_is_ready ?
>
> We have not added kasan_arch_is_ready() checks to all of the KASAN
> entry points, only those that are called early in boot before KASAN
> has been initialized, as I found out when I changed kasan_init() to
> just return without doing anything if radix is not enabled. On
> POWER8, the kernel crashed shortly afterwards in generic KASAN code
> that didn't have a kasan_arch_is_ready() call in it.
>
> As to whether we should add such calls to every entry point in the
> generic KASAN code -- it would probably be a good idea to do that,
> though a bit fragile in the long term because people will add stuff to
> the generic KASAN code without realizing there needs to be the guard,
> since their architecture won't need it.
>
> In the really long term, hopefully we can eventually drop HPT and get
> to the point where practically nothing runs in real mode. That's a
> fair way off though.
>
>>> +KASAN_SANITIZE_mmu_context.o := n
>>> +KASAN_SANITIZE_pgtable.o := n
>>> +KASAN_SANITIZE_radix_pgtable.o := n
>>> +KASAN_SANITIZE_radix_tlb.o := n
>>> +KASAN_SANITIZE_slb.o := n
>>> +KASAN_SANITIZE_pkeys.o := n
>>> diff --git a/arch/powerpc/mm/kasan/Makefile b/arch/powerpc/mm/kasan/Makefile
>>> index bcbfd6f2eca3..4999aadb1867 100644
>>> --- a/arch/powerpc/mm/kasan/Makefile
>>> +++ b/arch/powerpc/mm/kasan/Makefile
>>> @@ -5,3 +5,4 @@ KASAN_SANITIZE := n
>>> obj-$(CONFIG_PPC32) += init_32.o
>>> obj-$(CONFIG_PPC_8xx) += 8xx.o
>>> obj-$(CONFIG_PPC_BOOK3S_32) += book3s_32.o
>>> +obj-$(CONFIG_PPC_BOOK3S_64) += init_book3s_64.o
>>> diff --git a/arch/powerpc/mm/kasan/init_book3s_64.c
>>> b/arch/powerpc/mm/kasan/init_book3s_64.c
>>> new file mode 100644
>>> index 000000000000..cb96e16e57a1
>>> --- /dev/null
>>> +++ b/arch/powerpc/mm/kasan/init_book3s_64.c
>>> @@ -0,0 +1,103 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * KASAN for 64-bit Book3S powerpc
>>> + *
>>> + * Copyright (C) 2019-2020 IBM Corporation
>>> + * Author: Daniel Axtens <d...@axtens.net>
>>> + */
>>> +
>>> +/*
>>> +# ppc64 turns on virtual memory late in boot, after calling into generic
>>> code
>>> +# like the device-tree parser, so it uses this in conjunction with a hook
>>> in
>>> +# outline mode to avoid invalid access early in boot.
>>> +*/
>>> +
>>> +#define DISABLE_BRANCH_PROFILING
>>> +
>>> +#include <linux/kasan.h>
>>> +#include <linux/printk.h>
>>> +#include <linux/sched/task.h>
>>> +#include <linux/memblock.h>
>>> +#include <asm/pgalloc.h>
>>> +
>>> +DEFINE_STATIC_KEY_FALSE(powerpc_kasan_enabled_key);
>>> +
>>> +static void __init kasan_init_phys_region(void *start, void *end)
>>> +{
>>> + unsigned long k_start, k_end, k_cur;
>>> + void *va;
>>> +
>>> + if (start >= end)
>>> + return;
>>> +
>>> + k_start = ALIGN_DOWN((unsigned long)kasan_mem_to_shadow(start),
>>> PAGE_SIZE);
>>> + k_end = ALIGN((unsigned long)kasan_mem_to_shadow(end), PAGE_SIZE);
>>> +
>>> + va = memblock_alloc(k_end - k_start, PAGE_SIZE);
>>> + for (k_cur = k_start; k_cur < k_end; k_cur += PAGE_SIZE, va +=
>>> PAGE_SIZE)
>>> + map_kernel_page(k_cur, __pa(va), PAGE_KERNEL);
>>> +}
>>> +
>>> +void __init kasan_init(void)
>>> +{
>>> + /*
>>> + * We want to do the following things:
>>> + * 1) Map real memory into the shadow for all physical memblocks
>>> + * This takes us from c000... to c008...
>>> + * 2) Leave a hole over the shadow of vmalloc space. KASAN_VMALLOC
>>> + * will manage this for us.
>>> + * This takes us from c008... to c00a...
>>> + * 3) Map the 'early shadow'/zero page over iomap and vmemmap space.
>>> + * This takes us up to where we start at c00e...
>>> + */
>>> +
>>> + void *k_start = kasan_mem_to_shadow((void *)RADIX_VMALLOC_END);
>>> + void *k_end = kasan_mem_to_shadow((void *)RADIX_VMEMMAP_END);
>>> + phys_addr_t start, end;
>>> + u64 i;
>>> + pte_t zero_pte = pfn_pte(virt_to_pfn(kasan_early_shadow_page),
>>> PAGE_KERNEL);
>>> +
>>> + if (!early_radix_enabled()) {
>>> + pr_warn("KASAN not enabled as it requires radix!");
>>> + return;
>>> + }
>>> +
>>> + for_each_mem_range(i, &start, &end)
>>> + kasan_init_phys_region((void *)start, (void *)end);
>>> +
>>> + for (i = 0; i < PTRS_PER_PTE; i++)
>>> + __set_pte_at(&init_mm, (unsigned long)kasan_early_shadow_page,
>>> + &kasan_early_shadow_pte[i], zero_pte, 0);
>>> +
>>> + for (i = 0; i < PTRS_PER_PMD; i++)
>>> + pmd_populate_kernel(&init_mm, &kasan_early_shadow_pmd[i],
>>> + kasan_early_shadow_pte);
>>> +
>>> + for (i = 0; i < PTRS_PER_PUD; i++)
>>> + pud_populate(&init_mm, &kasan_early_shadow_pud[i],
>>> + kasan_early_shadow_pmd);
>>> +
>>> + /* map the early shadow over the iomap and vmemmap space */
>>> + kasan_populate_early_shadow(k_start, k_end);
>>> +
>>> + /* mark early shadow region as RO and wipe it */
>>> + zero_pte = pfn_pte(virt_to_pfn(kasan_early_shadow_page),
>>> PAGE_KERNEL_RO);
>>> + for (i = 0; i < PTRS_PER_PTE; i++)
>>> + __set_pte_at(&init_mm, (unsigned long)kasan_early_shadow_page,
>>> + &kasan_early_shadow_pte[i], zero_pte, 0);
>>> +
>>> + /*
>>> + * clear_page relies on some cache info that hasn't been set up yet.
>>> + * It ends up looping ~forever and blows up other data.
>>> + * Use memset instead.
>>> + */
>>> + memset(kasan_early_shadow_page, 0, PAGE_SIZE);
>>> +
>>> + static_branch_inc(&powerpc_kasan_enabled_key);
>>> +
>>> + /* Enable error messages */
>>> + init_task.kasan_depth = 0;
>>> + pr_info("KASAN init done (64-bit Book3S)\n");
>>
>> Maybe mention RADIX in the printed message ?
>
> Why? As in, who would find it useful?
Don't know. Is it usefull to mention 64-bit Book3S at all then ?
Christophe
