| Message ID | 20240619104919.20772-1-hbathini@linux.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2] radix/kfence: map __kfence_pool at page granularity | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 6 jobs. |
| snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 23 jobs. |
Hi Hari, Just a few little things ... Hari Bathini <hbathini@linux.ibm.com> writes: > When KFENCE is enabled, total system memory is mapped at page level > granularity. But in radix MMU mode, ~3GB additional memory is needed > to map 100GB of system memory at page level granularity when compared > to using 2MB direct mapping. This is not desired considering KFENCE is > designed to be enabled in production kernels [1]. > Also, mapping memory > allocated for KFENCE pool at page granularity seems sufficient enough > to enable KFENCE support. This should be firmer, eg: Mapping only the memory allocated for KFENCE pool at page granularity is sufficient to enable KFENCE support. > So, allocate __kfence_pool during bootup and > map it at page granularity instead of mapping all system memory at > page granularity. > > Without patch: > # cat /proc/meminfo > MemTotal: 101201920 kB > > With patch: > # cat /proc/meminfo > MemTotal: 104483904 kB > > Note that enabling KFENCE at runtime is disabled for radix MMU for now, > as it depends on the ability to split page table mappings and such APIs > are not currently implemented for radix MMU. > > All kfence_test.c testcases passed with this patch. > > [1] https://lore.kernel.org/all/20201103175841.3495947-2-elver@google.com/ > > Signed-off-by: Hari Bathini <hbathini@linux.ibm.com> > --- > > Changes in v2: > * Dropped the patch that adds support to enable KFENCE after startup. > * Added changes to avoid KFENCE enablement after system startup. > * Also, added a TODO explaining why KFENCE enablement after startup > is not supported for now. > * Functions to alloc/map __kfence_pool as suggested by Ritesh. > * Moved changes that apply to ppc32 as well to common file as suggested > by Christophe. > > > arch/powerpc/include/asm/kfence.h | 12 +++- > arch/powerpc/mm/book3s64/radix_pgtable.c | 74 ++++++++++++++++++++++-- > arch/powerpc/mm/init-common.c | 14 +++++ > 3 files changed, 95 insertions(+), 5 deletions(-) > > diff --git a/arch/powerpc/include/asm/kfence.h b/arch/powerpc/include/asm/kfence.h > index 424ceef82ae6..78590288ee80 100644 > --- a/arch/powerpc/include/asm/kfence.h > +++ b/arch/powerpc/include/asm/kfence.h > @@ -15,10 +15,20 @@ > #define ARCH_FUNC_PREFIX "." > #endif > > +#ifdef CONFIG_KFENCE > +extern bool kfence_early_init; This is only read in radix_pgtable.c, so it needed be global. > +extern bool kfence_disabled; > + > +static inline void disable_kfence(void) > +{ > + kfence_disabled = true; > +} > + > static inline bool arch_kfence_init_pool(void) > { > - return true; > + return !kfence_disabled; > } > +#endif > > #ifdef CONFIG_PPC64 > static inline bool kfence_protect_page(unsigned long addr, bool protect) > diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c > index 15e88f1439ec..a74912e0fd99 100644 > --- a/arch/powerpc/mm/book3s64/radix_pgtable.c > +++ b/arch/powerpc/mm/book3s64/radix_pgtable.c > @@ -301,7 +304,10 @@ static int __meminit create_physical_mapping(unsigned long start, > int psize; > unsigned long max_mapping_size = memory_block_size; > > - if (debug_pagealloc_enabled_or_kfence()) > + if (mapping_sz_limit < max_mapping_size) > + max_mapping_size = mapping_sz_limit; > + > + if (debug_pagealloc_enabled()) > max_mapping_size = PAGE_SIZE; > > start = ALIGN(start, PAGE_SIZE); > @@ -356,8 +362,64 @@ static int __meminit create_physical_mapping(unsigned long start, > return 0; > } > > +#ifdef CONFIG_KFENCE > +static inline phys_addr_t radix_alloc_kfence_pool_early(void) This is internal to radix_pgtable.c, so I think we can drop the radix prefix. There's also no alloc_kfence_pool(), so early is not really required IMHO. So it could just be alloc_kfence_pool(). Similar for map. ... > diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.c > index d3a7726ecf51..f881ab5107aa 100644 > --- a/arch/powerpc/mm/init-common.c > +++ b/arch/powerpc/mm/init-common.c > @@ -31,6 +31,20 @@ EXPORT_SYMBOL_GPL(kernstart_virt_addr); > > bool disable_kuep = !IS_ENABLED(CONFIG_PPC_KUEP); > bool disable_kuap = !IS_ENABLED(CONFIG_PPC_KUAP); > +#ifdef CONFIG_KFENCE > +bool __ro_after_init kfence_early_init = !!CONFIG_KFENCE_SAMPLE_INTERVAL; > +bool __ro_after_init kfence_disabled; > + > +static int __init parse_kfence_early_init(char *arg) > +{ > + int val; > + > + if (get_option(&arg, &val)) > + kfence_early_init = !!val; > + return 0; > +} > +early_param("kfence.sample_interval", parse_kfence_early_init); > +#endif AFAICS except for kfence_disabled, this can all go in radix_pgtable.c. That would make it unambiguous that kfence_early_init is only used by the radix kfence code, and avoid any bad interactions with the other subarches - which was my original concern about adding the code here. cheers
diff --git a/arch/powerpc/include/asm/kfence.h b/arch/powerpc/include/asm/kfence.h index 424ceef82ae6..78590288ee80 100644 --- a/arch/powerpc/include/asm/kfence.h +++ b/arch/powerpc/include/asm/kfence.h @@ -15,10 +15,20 @@ #define ARCH_FUNC_PREFIX "." #endif +#ifdef CONFIG_KFENCE +extern bool kfence_early_init; +extern bool kfence_disabled; + +static inline void disable_kfence(void) +{ + kfence_disabled = true; +} + static inline bool arch_kfence_init_pool(void) { - return true; + return !kfence_disabled; } +#endif #ifdef CONFIG_PPC64 static inline bool kfence_protect_page(unsigned long addr, bool protect) diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c index 15e88f1439ec..a74912e0fd99 100644 --- a/arch/powerpc/mm/book3s64/radix_pgtable.c +++ b/arch/powerpc/mm/book3s64/radix_pgtable.c @@ -17,6 +17,7 @@ #include <linux/hugetlb.h> #include <linux/string_helpers.h> #include <linux/memory.h> +#include <linux/kfence.h> #include <asm/pgalloc.h> #include <asm/mmu_context.h> @@ -31,6 +32,7 @@ #include <asm/uaccess.h> #include <asm/ultravisor.h> #include <asm/set_memory.h> +#include <asm/kfence.h> #include <trace/events/thp.h> @@ -293,7 +295,8 @@ static unsigned long next_boundary(unsigned long addr, unsigned long end) static int __meminit create_physical_mapping(unsigned long start, unsigned long end, - int nid, pgprot_t _prot) + int nid, pgprot_t _prot, + unsigned long mapping_sz_limit) { unsigned long vaddr, addr, mapping_size = 0; bool prev_exec, exec = false; @@ -301,7 +304,10 @@ static int __meminit create_physical_mapping(unsigned long start, int psize; unsigned long max_mapping_size = memory_block_size; - if (debug_pagealloc_enabled_or_kfence()) + if (mapping_sz_limit < max_mapping_size) + max_mapping_size = mapping_sz_limit; + + if (debug_pagealloc_enabled()) max_mapping_size = PAGE_SIZE; start = ALIGN(start, PAGE_SIZE); @@ -356,8 +362,64 @@ static int __meminit create_physical_mapping(unsigned long start, return 0; } +#ifdef CONFIG_KFENCE +static inline phys_addr_t radix_alloc_kfence_pool_early(void) +{ + phys_addr_t kfence_pool; + + /* + * TODO: Support to enable KFENCE after bootup depends on the ability to + * split page table mappings. As such support is not currently + * implemented for radix pagetables, support enabling KFENCE + * only at system startup for now. + * + * After support for splitting mappings is available on radix, + * radix_alloc_kfence_pool_early() & radix_map_kfence_pool_early() + * can be dropped and mapping for __kfence_pool memory can be + * split during arch_kfence_init_pool(). + */ + if (!kfence_early_init) + goto no_kfence; + + kfence_pool = memblock_phys_alloc(KFENCE_POOL_SIZE, PAGE_SIZE); + if (!kfence_pool) + goto no_kfence; + + memblock_mark_nomap(kfence_pool, KFENCE_POOL_SIZE); + return kfence_pool; + +no_kfence: + disable_kfence(); + return 0; +} + +static inline void radix_map_kfence_pool_early(phys_addr_t kfence_pool) +{ + int ret; + + if (!kfence_pool) + return; + + if (create_physical_mapping(kfence_pool, kfence_pool + KFENCE_POOL_SIZE, + -1, PAGE_KERNEL, PAGE_SIZE)) + goto err; + + memblock_clear_nomap(kfence_pool, KFENCE_POOL_SIZE); + __kfence_pool = __va(kfence_pool); + return; + +err: + memblock_phys_free(kfence_pool, KFENCE_POOL_SIZE); + disable_kfence(); +} +#else +static inline phys_addr_t radix_alloc_kfence_pool_early(void) { return 0; } +static inline void radix_map_kfence_pool_early(phys_addr_t kfence_pool) { } +#endif + static void __init radix_init_pgtable(void) { + phys_addr_t kfence_pool; unsigned long rts_field; phys_addr_t start, end; u64 i; @@ -365,6 +427,8 @@ static void __init radix_init_pgtable(void) /* We don't support slb for radix */ slb_set_size(0); + kfence_pool = radix_alloc_kfence_pool_early(); + /* * Create the linear mapping */ @@ -381,9 +445,11 @@ static void __init radix_init_pgtable(void) } WARN_ON(create_physical_mapping(start, end, - -1, PAGE_KERNEL)); + -1, PAGE_KERNEL, ~0UL)); } + radix_map_kfence_pool_early(kfence_pool); + if (!cpu_has_feature(CPU_FTR_HVMODE) && cpu_has_feature(CPU_FTR_P9_RADIX_PREFETCH_BUG)) { /* @@ -875,7 +941,7 @@ int __meminit radix__create_section_mapping(unsigned long start, } return create_physical_mapping(__pa(start), __pa(end), - nid, prot); + nid, prot, ~0UL); } int __meminit radix__remove_section_mapping(unsigned long start, unsigned long end) diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.c index d3a7726ecf51..f881ab5107aa 100644 --- a/arch/powerpc/mm/init-common.c +++ b/arch/powerpc/mm/init-common.c @@ -31,6 +31,20 @@ EXPORT_SYMBOL_GPL(kernstart_virt_addr); bool disable_kuep = !IS_ENABLED(CONFIG_PPC_KUEP); bool disable_kuap = !IS_ENABLED(CONFIG_PPC_KUAP); +#ifdef CONFIG_KFENCE +bool __ro_after_init kfence_early_init = !!CONFIG_KFENCE_SAMPLE_INTERVAL; +bool __ro_after_init kfence_disabled; + +static int __init parse_kfence_early_init(char *arg) +{ + int val; + + if (get_option(&arg, &val)) + kfence_early_init = !!val; + return 0; +} +early_param("kfence.sample_interval", parse_kfence_early_init); +#endif static int __init parse_nosmep(char *p) {
When KFENCE is enabled, total system memory is mapped at page level granularity. But in radix MMU mode, ~3GB additional memory is needed to map 100GB of system memory at page level granularity when compared to using 2MB direct mapping. This is not desired considering KFENCE is designed to be enabled in production kernels [1]. Also, mapping memory allocated for KFENCE pool at page granularity seems sufficient enough to enable KFENCE support. So, allocate __kfence_pool during bootup and map it at page granularity instead of mapping all system memory at page granularity. Without patch: # cat /proc/meminfo MemTotal: 101201920 kB With patch: # cat /proc/meminfo MemTotal: 104483904 kB Note that enabling KFENCE at runtime is disabled for radix MMU for now, as it depends on the ability to split page table mappings and such APIs are not currently implemented for radix MMU. All kfence_test.c testcases passed with this patch. [1] https://lore.kernel.org/all/20201103175841.3495947-2-elver@google.com/ Signed-off-by: Hari Bathini <hbathini@linux.ibm.com> --- Changes in v2: * Dropped the patch that adds support to enable KFENCE after startup. * Added changes to avoid KFENCE enablement after system startup. * Also, added a TODO explaining why KFENCE enablement after startup is not supported for now. * Functions to alloc/map __kfence_pool as suggested by Ritesh. * Moved changes that apply to ppc32 as well to common file as suggested by Christophe. arch/powerpc/include/asm/kfence.h | 12 +++- arch/powerpc/mm/book3s64/radix_pgtable.c | 74 ++++++++++++++++++++++-- arch/powerpc/mm/init-common.c | 14 +++++ 3 files changed, 95 insertions(+), 5 deletions(-)