Re: [PATCH v6 (proposal)] powerpc/cpu: enable nr_cpus for crash kernel
Hi, Le 22/05/2018 à 10:23, Pingfan Liu a écrit : > For kexec -p, the boot cpu can be not the cpu0, this causes the problem > to alloc paca[]. In theory, there is no requirement to assign cpu's logical > id as its present seq by device tree. But we have something like > cpu_first_thread_sibling(), which makes assumption on the mapping inside > a core. Hence partially changing the mapping, i.e. unbind the mapping of > core while keep the mapping inside a core. After this patch, the core with > boot-cpu will always be mapped into core 0. > > And at present, the code to discovery cpu spreads over two functions: > early_init_dt_scan_cpus() and smp_setup_cpu_maps(). > This patch tries to fold smp_setup_cpu_maps() into the "previous" one This patch is pretty old and doesn't apply anymore. If still relevant can you please rebase and resubmit. Thanks Christophe > > Signed-off-by: Pingfan Liu > --- > v5 -> v6: >simplify the loop logic (Hope it can answer Benjamin's concern) >concentrate the cpu recovery code to early stage (Hope it can answer > Michael's concern) > Todo: (if this method is accepted) >fold the whole smp_setup_cpu_maps() > > arch/powerpc/include/asm/smp.h | 1 + > arch/powerpc/kernel/prom.c | 123 > - > arch/powerpc/kernel/setup-common.c | 58 ++--- > drivers/of/fdt.c | 2 +- > include/linux/of_fdt.h | 2 + > 5 files changed, 103 insertions(+), 83 deletions(-) > > diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h > index fac963e..80c7693 100644 > --- a/arch/powerpc/include/asm/smp.h > +++ b/arch/powerpc/include/asm/smp.h > @@ -30,6 +30,7 @@ > #include > > extern int boot_cpuid; > +extern int threads_in_core; > extern int spinning_secondaries; > > extern void cpu_die(void); > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c > index 4922162..2ae0b4a 100644 > --- a/arch/powerpc/kernel/prom.c > +++ b/arch/powerpc/kernel/prom.c > @@ -77,7 +77,6 @@ unsigned long tce_alloc_start, tce_alloc_end; > u64 ppc64_rma_size; > #endif > static phys_addr_t first_memblock_size; > -static int __initdata boot_cpu_count; > > static int __init early_parse_mem(char *p) > { > @@ -305,6 +304,14 @@ static void __init check_cpu_feature_properties(unsigned > long node) > } > } > > +struct bootinfo { > + int boot_thread_id; > + unsigned int cpu_cnt; > + int cpu_hwids[NR_CPUS]; > + bool avail[NR_CPUS]; > +}; > +static struct bootinfo *bt_info; > + > static int __init early_init_dt_scan_cpus(unsigned long node, > const char *uname, int depth, > void *data) > @@ -312,10 +319,12 @@ static int __init early_init_dt_scan_cpus(unsigned long > node, > const char *type = of_get_flat_dt_prop(node, "device_type", NULL); > const __be32 *prop; > const __be32 *intserv; > - int i, nthreads; > + int i, nthreads, maxidx; > int len; > - int found = -1; > - int found_thread = 0; > + int found_thread = -1; > + struct bootinfo *info = data; > + bool avail; > + int rotate_cnt, id; > > /* We are scanning "cpu" nodes only */ > if (type == NULL || strcmp(type, "cpu") != 0) > @@ -325,8 +334,15 @@ static int __init early_init_dt_scan_cpus(unsigned long > node, > intserv = of_get_flat_dt_prop(node, "ibm,ppc-interrupt-server#s", &len); > if (!intserv) > intserv = of_get_flat_dt_prop(node, "reg", &len); > + avail = of_fdt_device_is_available(initial_boot_params, node); > +#if 0 > + //todo > + if (!avail) > + avail = !of_fdt_property_match_string(node, > + "enable-method", "spin-table"); > +#endif > > - nthreads = len / sizeof(int); > + threads_in_core = nthreads = len / sizeof(int); > > /* >* Now see if any of these threads match our boot cpu. > @@ -338,9 +354,10 @@ static int __init early_init_dt_scan_cpus(unsigned long > node, >* booted proc. >*/ > if (fdt_version(initial_boot_params) >= 2) { > + info->cpu_hwids[info->cpu_cnt] = > + be32_to_cpu(intserv[i]); > if (be32_to_cpu(intserv[i]) == > fdt_boot_cpuid_phys(initial_boot_params)) { > - found = boot_cpu_count; > found_thread = i; > } > } else { > @@ -351,22 +368,37 @@ static int __init early_init_dt_scan_cpus(unsigned long > node, >*/ > if (of_get_flat_dt_prop(node, > "linux,boot-cpu", NULL) != NULL) > - found = boot_cpu_count; > + found_
Re: [PATCH v11 1/4] powerpc/kexec: turn some static helper functions public
Le 19/06/2023 à 04:49, Sourabh Jain a écrit : > Move update_cpus_node and get_crash_memory_ranges functions from > kexec/file_load_64.c to kexec/core_64.c to make these functions usable > by other kexec components. > > Later in the series, these functions are utilized to do in-kernel update > to kexec segments on CPU/Memory hot plug/unplug or online/offline events > for both kexec_load and kexec_file_load syscalls. > > No functional change intended. > > Signed-off-by: Sourabh Jain > Reviewed-by: Laurent Dufour This patch doesn't apply, if still applicable can you rebase and resubmit ? Thanks Christophe > --- > arch/powerpc/include/asm/kexec.h | 6 ++ > arch/powerpc/kexec/core_64.c | 166 ++ > arch/powerpc/kexec/file_load_64.c | 162 - > 3 files changed, 172 insertions(+), 162 deletions(-) > > diff --git a/arch/powerpc/include/asm/kexec.h > b/arch/powerpc/include/asm/kexec.h > index a1ddba01e7d1..8090ad7d97d9 100644 > --- a/arch/powerpc/include/asm/kexec.h > +++ b/arch/powerpc/include/asm/kexec.h > @@ -99,6 +99,12 @@ void relocate_new_kernel(unsigned long indirection_page, > unsigned long reboot_co > > void kexec_copy_flush(struct kimage *image); > > +#ifdef CONFIG_PPC64 > +struct crash_mem; > +int update_cpus_node(void *fdt); > +int get_crash_memory_ranges(struct crash_mem **mem_ranges); > +#endif > + > #if defined(CONFIG_CRASH_DUMP) && defined(CONFIG_PPC_RTAS) > void crash_free_reserved_phys_range(unsigned long begin, unsigned long end); > #define crash_free_reserved_phys_range crash_free_reserved_phys_range > diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c > index a79e28c91e2b..0b292f93a74c 100644 > --- a/arch/powerpc/kexec/core_64.c > +++ b/arch/powerpc/kexec/core_64.c > @@ -17,6 +17,8 @@ > #include > #include > #include > +#include > +#include > > #include > #include > @@ -30,6 +32,8 @@ > #include > #include > #include > +#include > +#include > > int machine_kexec_prepare(struct kimage *image) > { > @@ -377,6 +381,168 @@ void default_machine_kexec(struct kimage *image) > /* NOTREACHED */ > } > > +/** > + * get_crash_memory_ranges - Get crash memory ranges. This list includes > + * first/crashing kernel's memory regions that > + * would be exported via an elfcore. > + * @mem_ranges: Range list to add the memory ranges to. > + * > + * Returns 0 on success, negative errno on error. > + */ > +int get_crash_memory_ranges(struct crash_mem **mem_ranges) > +{ > + phys_addr_t base, end; > + struct crash_mem *tmem; > + u64 i; > + int ret; > + > + for_each_mem_range(i, &base, &end) { > + u64 size = end - base; > + > + /* Skip backup memory region, which needs a separate entry */ > + if (base == BACKUP_SRC_START) { > + if (size > BACKUP_SRC_SIZE) { > + base = BACKUP_SRC_END + 1; > + size -= BACKUP_SRC_SIZE; > + } else > + continue; > + } > + > + ret = add_mem_range(mem_ranges, base, size); > + if (ret) > + goto out; > + > + /* Try merging adjacent ranges before reallocation attempt */ > + if ((*mem_ranges)->nr_ranges == (*mem_ranges)->max_nr_ranges) > + sort_memory_ranges(*mem_ranges, true); > + } > + > + /* Reallocate memory ranges if there is no space to split ranges */ > + tmem = *mem_ranges; > + if (tmem && (tmem->nr_ranges == tmem->max_nr_ranges)) { > + tmem = realloc_mem_ranges(mem_ranges); > + if (!tmem) > + goto out; > + } > + > + /* Exclude crashkernel region */ > + ret = crash_exclude_mem_range(tmem, crashk_res.start, crashk_res.end); > + if (ret) > + goto out; > + > + /* > + * FIXME: For now, stay in parity with kexec-tools but if RTAS/OPAL > + *regions are exported to save their context at the time of > + *crash, they should actually be backed up just like the > + *first 64K bytes of memory. > + */ > + ret = add_rtas_mem_range(mem_ranges); > + if (ret) > + goto out; > + > + ret = add_opal_mem_range(mem_ranges); > + if (ret) > + goto out; > + > + /* create a separate program header for the backup region */ > + ret = add_mem_range(mem_ranges, BACKUP_SRC_START, BACKUP_SRC_SIZE); > + if (ret) > + goto out; > + > + sort_memory_ranges(*mem_ranges, false); > +out: > + if (ret) > + pr_err("Failed to setup crash memory ranges\n"); > + return ret; > +} > + > +/** > + * add_node_props - Reads node properties from device node structure and add > + * them to fdt. > + *
Re: [PATCHv7 2/4] powerpc/setup: Loosen the mapping between cpu logical id and its seq in dt
Le 28/09/2023 à 22:36, Wen Xiong a écrit : > Hi Pingfan, > > + avail = intserv_node->avail; > + nthreads = intserv_node->len / sizeof(int); > + for (j = 0; j < nthreads && cpu < nr_cpu_ids; j++) { > set_cpu_present(cpu, avail); > set_cpu_possible(cpu, true); > - cpu_to_phys_id[cpu] = be32_to_cpu(intserv[j]); > + cpu_to_phys_id[cpu] = > be32_to_cpu(intserv_node->intserv[j]); > + DBG("thread %d -> cpu %d (hard id %d)\n", > + j, cpu, be32_to_cpu(intserv[j])); > > Intserv is not defined. Should "be32_to_cpu(intserv_node->intserv[j])? > cpu++; > } > + } Please don't top-post , see https://docs.kernel.org/process/submitting-patches.html#use-trimmed-interleaved-replies-in-email-discussions Make comments inside the patch directly, making sure that your mail client is properly configured to add the standard > in front of all lines of the quoted mail. Christophe > > -Original Message- > From: Pingfan Liu > Sent: Monday, September 25, 2023 2:54 AM > To: linuxppc-...@lists.ozlabs.org > Cc: Pingfan Liu ; Michael Ellerman ; > Nicholas Piggin ; Christophe Leroy > ; Mahesh Salgaonkar ; Wen > Xiong ; Baoquan He ; Ming Lei > ; kexec@lists.infradead.org > Subject: [EXTERNAL] [PATCHv7 2/4] powerpc/setup: Loosen the mapping between > cpu logical id and its seq in dt > > *** Idea *** > For kexec -p, the boot cpu can be not the cpu0, this causes the problem of > allocating memory for paca_ptrs[]. However, in theory, there is no > requirement to assign cpu's logical id as its present sequence in the device > tree. But there is something like cpu_first_thread_sibling(), which makes > assumption on the mapping inside a core. Hence partially loosening the > mapping, i.e. unbind the mapping of core while keep the mapping inside a core. > > *** Implement *** > At this early stage, there are plenty of memory to utilize. Hence, this patch > allocates interim memory to link the cpu info on a list, then reorder cpus by > changing the list head. As a result, there is a rotate shift between the > sequence number in dt and the cpu logical number. > > *** Result *** > After this patch, a boot-cpu's logical id will always be mapped into the > range [0,threads_per_core). > > Besides this, at this phase, all threads in the boot core are forced to be > onlined. This restriction will be lifted in a later patch with extra effort. > > Signed-off-by: Pingfan Liu > Cc: Michael Ellerman > Cc: Nicholas Piggin > Cc: Christophe Leroy > Cc: Mahesh Salgaonkar > Cc: Wen Xiong > Cc: Baoquan He > Cc: Ming Lei > Cc: kexec@lists.infradead.org > To: linuxppc-...@lists.ozlabs.org > --- > arch/powerpc/kernel/prom.c | 25 + > arch/powerpc/kernel/setup-common.c | 87 +++--- > 2 files changed, 85 insertions(+), 27 deletions(-) > > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c index > ec82f5bda908..87272a2d8c10 100644 > --- a/arch/powerpc/kernel/prom.c > +++ b/arch/powerpc/kernel/prom.c > @@ -76,7 +76,9 @@ u64 ppc64_rma_size; > unsigned int boot_cpu_node_count __ro_after_init; #endif static > phys_addr_t first_memblock_size; > +#ifdef CONFIG_SMP > static int __initdata boot_cpu_count; > +#endif > > static int __init early_parse_mem(char *p) { @@ -331,8 +333,7 @@ static > int __init early_init_dt_scan_cpus(unsigned long node, > const __be32 *intserv; > int i, nthreads; > int len; > - int found = -1; > - int found_thread = 0; > + bool found = false; > > /* We are scanning "cpu" nodes only */ > if (type == NULL || strcmp(type, "cpu") != 0) @@ -355,8 +356,15 @@ > static int __init early_init_dt_scan_cpus(unsigned long node, > for (i = 0; i < nthreads; i++) { > if (be32_to_cpu(intserv[i]) == > fdt_boot_cpuid_phys(initial_boot_params)) { > - found = boot_cpu_count; > - found_thread = i; > + /* > +* always map the boot-cpu logical id into the > +* range of [0, thread_per_core) > +*/ > + boot_cpuid = i; > + found = true; > + /* This works around the hole in paca_ptrs[]. */ > + if (nr_cpu_ids &
Re: [PATCH v2] kexec_file: Drop weak attribute from arch_kexec_apply_relocations[_add]
Le 29/05/2022 à 08:56, Uwe Kleine-König a écrit : > Hello, > > on current linux-next ARCH=riscv allmodconfig breaks with: > >CC arch/riscv/kernel/elf_kexec.o > arch/riscv/kernel/elf_kexec.c:345:5: error: redefinition of > ‘arch_kexec_apply_relocations_add’ >345 | int arch_kexec_apply_relocations_add(struct purgatory_info *pi, >| ^~~~ > In file included from arch/riscv/kernel/elf_kexec.c:16: > include/linux/kexec.h:236:1: note: previous definition of > ‘arch_kexec_apply_relocations_add’ with type ‘int(struct purgatory_info *, > Elf64_Shdr *, const Elf64_Shdr *, const Elf64_Shdr *)’ {aka ‘int(struct > purgatory_info *, struct elf64_shdr *, const struct elf64_shdr *, const > struct elf64_shdr *)’} >236 | arch_kexec_apply_relocations_add(struct purgatory_info *pi, Elf_Shdr > *section, >| ^~~~ > > (I think) because there is a conflict between the two commits: > > 233c1e6c319c kexec_file: drop weak attribute from > arch_kexec_apply_relocations[_add] > 838b3e28488f RISC-V: Load purgatory in kexec_file > > And so next is broken starting from > 164a9037b1d33f28ba27671c16ec1c23d4a11acf which merges the riscv tree. > In arch/riscv/include/asm/kexec.h, do the same as s390 did in commit 233c1e6c319c: diff --git a/arch/s390/include/asm/kexec.h b/arch/s390/include/asm/kexec.h index 7f3c9ac34bd8..540dd469b088 100644 --- a/arch/s390/include/asm/kexec.h +++ b/arch/s390/include/asm/kexec.h @@ -83,4 +83,12 @@ struct kimage_arch { extern const struct kexec_file_ops s390_kexec_image_ops; extern const struct kexec_file_ops s390_kexec_elf_ops; +#ifdef CONFIG_KEXEC_FILE +struct purgatory_info; +int arch_kexec_apply_relocations_add(struct purgatory_info *pi, +Elf_Shdr *section, +const Elf_Shdr *relsec, +const Elf_Shdr *symtab); +#define arch_kexec_apply_relocations_add arch_kexec_apply_relocations_add +#endif #endif /*_S390_KEXEC_H */ ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCHv5 0/3] enable nr_cpus for powerpc
Le 23/03/2018 à 06:28, Pingfan Liu a écrit : Maintainers, ping? Any comment? This series doesn't apply anymore, and it has build failure report from the robot. If it's still relevant, please rebase and fix. Christophe Thanks On Thu, Mar 15, 2018 at 12:41 PM, Pingfan Liu wrote: This topic has a very long history. It comes from Mahesh Salgaonkar For v3: https://patchwork.ozlabs.org/patch/834860/ I hope we can acquire it for "kexec -p" soon. V4->V5: improve the [1/3] implementation based on Benjamin's suggestion. Mahesh Salgaonkar (1): ppc64 boot: Wait for boot cpu to show up if nr_cpus limit is about to hit. Pingfan Liu (2): powerpc, cpu: partially unbind the mapping between cpu logical id and its seq in dt powerpc, cpu: handling the special case when boot_cpuid greater than nr_cpus arch/powerpc/include/asm/paca.h| 3 +++ arch/powerpc/include/asm/smp.h | 2 ++ arch/powerpc/kernel/paca.c | 19 ++- arch/powerpc/kernel/prom.c | 27 --- arch/powerpc/kernel/setup-common.c | 35 --- 5 files changed, 67 insertions(+), 19 deletions(-) -- 2.7.4 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH] kexec/powerpc: fix exporting memory limit
Le 07/03/2014 à 05:38, Nikita Yushchenko a écrit : On Thu, 2014-03-06 at 18:24 +0400, Nikita Yushchenko wrote: When preparing dump-capturing kernel, kexec userspace tool needs to know actual amount of memory used by the running kernel. This may differ from extire available DRAM for a couple of reasons. To address this issue, kdump kernel support code injects several attributes into device tree that are later captured by userspace kexec tool via /proc interface. One such attrubute is 'chosen/linux,memory_limit' that is used to pass memory limit of the running kernel. This was initialized using kernel's 'memory_limit' variable, that is set by early init code based on mem= kernel parameter and other reasons. But there are cases when memory_limit variable does not contain proper information. One such case is when !CONFIG_HIGHMEM kernel runs on system with memory large enough not to fit into lowmem. Why doesn't the !CONFIG_HIGHMEM code update memory_limit to reflect reality. I guess because memory_limit is used for ... well, memory limit, set by mem=. And for the rest memblock is used (and it *is* updated). And code elsewhere does use memblock, see e.g. numa_enforce_memory_limit() in arch/powerpc/mm/numa.c In MMU init (MMU_init() in arch/powerpc/mm/init_32.c -which is the point where final memory configuration is set) memblock, not memory_limit, is both used and updated. We still have this patch as "New" in patchwork. I don't know if it is relevant but directory structure has changed so if still needed this patch needs rebase. Christophe ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 0/2] kdump: simplify code
Le 11/12/2021 à 18:53, David Laight a écrit : > From: Tiezhu Yang >> Sent: 11 December 2021 03:33 >> >> v2: >>-- add copy_to_user_or_kernel() in lib/usercopy.c >>-- define userbuf as bool type > > Instead of having a flag to indicate whether the buffer is user or kernel, > would it be better to have two separate buffer pointers. > One for a user space buffer, the other for a kernel space buffer. > Exactly one of the buffers should always be NULL. > > That way the flag is never incorrectly set. > It's a very good idea. I was worried about the casts forcing the __user property away and back. With that approach we will preserve the __user tags on user buffers and enable sparse checking. The only little drawback I see is that apparently GCC doesn't consider the NULL value as a constant and therefore doesn't perform constant folding on pointers. Not sure if this is a problem here. Christophe ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 1/2] kdump: vmcore: remove copy_to() and add copy_to_user_or_kernel()
Le 11/12/2021 à 04:33, Tiezhu Yang a écrit : > In arch/*/kernel/crash_dump*.c, there exist many similar code > about copy_oldmem_page(), remove copy_to() in fs/proc/vmcore.c > and add copy_to_user_or_kernel() in lib/usercopy.c, then we can > use copy_to_user_or_kernel() to simplify the related code. It should be an inline function in uaccess.h, see below why. > > Signed-off-by: Tiezhu Yang > --- > fs/proc/vmcore.c| 28 +++- > include/linux/uaccess.h | 1 + > lib/usercopy.c | 15 +++ > 3 files changed, 23 insertions(+), 21 deletions(-) > > diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c > index 509f851..f67fd77 100644 > --- a/fs/proc/vmcore.c > +++ b/fs/proc/vmcore.c > @@ -238,22 +238,8 @@ copy_oldmem_page_encrypted(unsigned long pfn, char *buf, > size_t csize, > return copy_oldmem_page(pfn, buf, csize, offset, userbuf); > } > > -/* > - * Copy to either kernel or user space > - */ > -static int copy_to(void *target, void *src, size_t size, int userbuf) > -{ > - if (userbuf) { > - if (copy_to_user((char __user *) target, src, size)) > - return -EFAULT; > - } else { > - memcpy(target, src, size); > - } > - return 0; > -} > - > #ifdef CONFIG_PROC_VMCORE_DEVICE_DUMP > -static int vmcoredd_copy_dumps(void *dst, u64 start, size_t size, int > userbuf) > +static int vmcoredd_copy_dumps(void *dst, u64 start, size_t size, bool > userbuf) Changing int to bool in all the callers should be another patch. You can have copy_to_user_or_kernel() take a bool in the patch while still having all the callers using an int. > { > struct vmcoredd_node *dump; > u64 offset = 0; > @@ -266,7 +252,7 @@ static int vmcoredd_copy_dumps(void *dst, u64 start, > size_t size, int userbuf) > if (start < offset + dump->size) { > tsz = min(offset + (u64)dump->size - start, (u64)size); > buf = dump->buf + start - offset; > - if (copy_to(dst, buf, tsz, userbuf)) { > + if (copy_to_user_or_kernel(dst, buf, tsz, userbuf)) { > ret = -EFAULT; > goto out_unlock; > } > @@ -330,7 +316,7 @@ static int vmcoredd_mmap_dumps(struct vm_area_struct > *vma, unsigned long dst, >* returned otherwise number of bytes read are returned. >*/ > static ssize_t __read_vmcore(char *buffer, size_t buflen, loff_t *fpos, > - int userbuf) > + bool userbuf) > { > ssize_t acc = 0, tmp; > size_t tsz; > @@ -347,7 +333,7 @@ static ssize_t __read_vmcore(char *buffer, size_t buflen, > loff_t *fpos, > /* Read ELF core header */ > if (*fpos < elfcorebuf_sz) { > tsz = min(elfcorebuf_sz - (size_t)*fpos, buflen); > - if (copy_to(buffer, elfcorebuf + *fpos, tsz, userbuf)) > + if (copy_to_user_or_kernel(buffer, elfcorebuf + *fpos, tsz, > userbuf)) > return -EFAULT; > buflen -= tsz; > *fpos += tsz; > @@ -395,7 +381,7 @@ static ssize_t __read_vmcore(char *buffer, size_t buflen, > loff_t *fpos, > /* Read remaining elf notes */ > tsz = min(elfcorebuf_sz + elfnotes_sz - (size_t)*fpos, buflen); > kaddr = elfnotes_buf + *fpos - elfcorebuf_sz - vmcoredd_orig_sz; > - if (copy_to(buffer, kaddr, tsz, userbuf)) > + if (copy_to_user_or_kernel(buffer, kaddr, tsz, userbuf)) > return -EFAULT; > > buflen -= tsz; > @@ -435,7 +421,7 @@ static ssize_t __read_vmcore(char *buffer, size_t buflen, > loff_t *fpos, > static ssize_t read_vmcore(struct file *file, char __user *buffer, > size_t buflen, loff_t *fpos) > { > - return __read_vmcore((__force char *) buffer, buflen, fpos, 1); > + return __read_vmcore((__force char *) buffer, buflen, fpos, true); > } > > /* > @@ -461,7 +447,7 @@ static vm_fault_t mmap_vmcore_fault(struct vm_fault *vmf) > if (!PageUptodate(page)) { > offset = (loff_t) index << PAGE_SHIFT; > buf = __va((page_to_pfn(page) << PAGE_SHIFT)); > - rc = __read_vmcore(buf, PAGE_SIZE, &offset, 0); > + rc = __read_vmcore(buf, PAGE_SIZE, &offset, false); > if (rc < 0) { > unlock_page(page); > put_page(page); > diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h > index ac03940..a25e682e 100644 > --- a/include/linux/uaccess.h > +++ b/include/linux/uaccess.h > @@ -283,6 +283,7 @@ __copy_from_user_inatomic_nocache(void *to, const void > __user *from, > #endif /* ARCH_HAS_NOCACHE_UACCESS */ > > extern __must_check int check_zeroed_user(const void __user *from, size_t > size); > +extern __must_check int cop
Re: [PATCH 1/2] kdump: vmcore: move copy_to() from vmcore.c to uaccess.h
Le 10/12/2021 à 17:59, Andrew Morton a écrit : > On Fri, 10 Dec 2021 21:36:00 +0800 Tiezhu Yang wrote: > >> In arch/*/kernel/crash_dump*.c, there exist similar code about >> copy_oldmem_page(), move copy_to() from vmcore.c to uaccess.h, >> and then we can use copy_to() to simplify the related code. >> >> ... >> >> --- a/fs/proc/vmcore.c >> +++ b/fs/proc/vmcore.c >> @@ -238,20 +238,6 @@ copy_oldmem_page_encrypted(unsigned long pfn, char >> *buf, size_t csize, >> return copy_oldmem_page(pfn, buf, csize, offset, userbuf); >> } >> >> -/* >> - * Copy to either kernel or user space >> - */ >> -static int copy_to(void *target, void *src, size_t size, int userbuf) >> -{ >> -if (userbuf) { >> -if (copy_to_user((char __user *) target, src, size)) >> -return -EFAULT; >> -} else { >> -memcpy(target, src, size); >> -} >> -return 0; >> -} >> - >> #ifdef CONFIG_PROC_VMCORE_DEVICE_DUMP >> static int vmcoredd_copy_dumps(void *dst, u64 start, size_t size, int >> userbuf) >> { >> diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h >> index ac03940..4a6c3e4 100644 >> --- a/include/linux/uaccess.h >> +++ b/include/linux/uaccess.h >> @@ -201,6 +201,20 @@ copy_to_user(void __user *to, const void *from, >> unsigned long n) >> return n; >> } >> >> +/* >> + * Copy to either kernel or user space >> + */ >> +static inline int copy_to(void *target, void *src, size_t size, int userbuf) >> +{ >> +if (userbuf) { >> +if (copy_to_user((char __user *) target, src, size)) >> +return -EFAULT; >> +} else { >> +memcpy(target, src, size); >> +} >> +return 0; >> +} >> + > > Ordinarily I'd say "this is too large to be inlined". But the function > has only a single callsite per architecture so inlining it won't cause > bloat at present. > > But hopefully copy_to() will get additional callers in the future, in > which case it shouldn't be inlined. So I'm thinking it would be best > to start out with this as a regular non-inlined function, in > lib/usercopy.c. > > Also, copy_to() is a very poor name for a globally-visible helper > function. Better would be copy_to_user_or_kernel(), although that's > perhaps a bit long. > > And the `userbuf' arg should have type bool, yes? > I think keeping it inlined is better. copy_oldmem_page() is bigger with v2 (outlined) than with v1 (inlined), see both below: v1: : 0: 94 21 ff e0 stwur1,-32(r1) 4: 93 e1 00 1c stw r31,28(r1) 8: 7c bf 2b 79 mr. r31,r5 c: 40 82 00 14 bne 20 10: 83 e1 00 1c lwz r31,28(r1) 14: 38 60 00 00 li r3,0 18: 38 21 00 20 addir1,r1,32 1c: 4e 80 00 20 blr 20: 28 1f 10 00 cmplwi r31,4096 24: 93 61 00 0c stw r27,12(r1) 28: 7c 08 02 a6 mflrr0 2c: 93 81 00 10 stw r28,16(r1) 30: 93 a1 00 14 stw r29,20(r1) 34: 7c 9b 23 78 mr r27,r4 38: 90 01 00 24 stw r0,36(r1) 3c: 7c dd 33 78 mr r29,r6 40: 93 c1 00 18 stw r30,24(r1) 44: 7c fc 3b 78 mr r28,r7 48: 40 81 00 08 ble 50 4c: 3b e0 10 00 li r31,4096 50: 54 7e 60 26 rlwinm r30,r3,12,0,19 54: 7f c3 f3 78 mr r3,r30 58: 7f e4 fb 78 mr r4,r31 5c: 48 00 00 01 bl 5c 5c: R_PPC_REL24 memblock_is_region_memory 60: 2c 03 00 00 cmpwi r3,0 64: 41 82 00 30 beq 94 68: 2c 1c 00 00 cmpwi r28,0 6c: 3f de c0 00 addis r30,r30,-16384 70: 7f 63 db 78 mr r3,r27 74: 7f e5 fb 78 mr r5,r31 78: 7c 9e ea 14 add r4,r30,r29 7c: 41 82 00 7c beq f8 80: 48 00 00 01 bl 80 80: R_PPC_REL24 _copy_to_user 84: 2c 03 00 00 cmpwi r3,0 88: 41 a2 00 48 beq d0 8c: 3b e0 ff f2 li r31,-14 90: 48 00 00 40 b d0 94: 7f c3 f3 78 mr r3,r30 98: 38 a0 05 91 li r5,1425 9c: 38 80 10 00 li r4,4096 a0: 48 00 00 01 bl a0 a0: R_PPC_REL24 ioremap_prot a4: 2c 1c 00 00 cmpwi r28,0 a8: 7c 7e 1b 78 mr r30,r3 ac: 7c 83 ea 14 add r4,r3,r29 b0: 7f e5 fb 78 mr r5,r31 b4: 7f 63 db 78 mr r3,r27 b8: 41 82 00 48 beq 100 bc: 48 00 00 01 bl bc bc: R_PPC_REL24 _copy_to_user c0: 2c 03 00 00 cmpwi r3,0 c4: 40 82 00 44 bne 108 c8: 7f c3 f3 78 mr r3,r30 cc: 48 00 00 01 bl cc cc: R_PPC_REL24 iounmap d0: 80 01 00 24 lwz r0,36(r1) d4: 7f e3 fb 78 mr r3,r31 d8: 83 61 00 0c lwz r27,12(r1) dc: 83 81 00 10 lwz r28,16(r1) e0: 7c 08 03 a6 mtlrr0 e4:
Re: [PATCH v3 05/18] powerpc/85xx: Load all early TLB entries at once
Hi Scott, Le 23/10/2015 à 05:54, Scott Wood a écrit : Use an AS=1 trampoline TLB entry to allow all normal TLB1 entries to be loaded at once. This avoids the need to keep the translation that code is executing from in the same TLB entry in the final TLB configuration as during early boot, which in turn is helpful for relocatable kernels (e.g. kdump) where the kernel is not running from what would be the first TLB entry. On e6500, we limit map_mem_in_cams() to the primary hwthread of a core (the boot cpu is always considered primary, as a kdump kernel can be entered on any cpu). Each TLB only needs to be set up once, and when we do, we don't want another thread to be running when we create a temporary trampoline TLB1 entry. Isn't that redundant with commit 78a235efdc42 ("powerpc/fsl_booke: set the tlb entry for the kernel address in AS1") ? Should we revert commit 78a235efdc42 ? Christophe Signed-off-by: Scott Wood --- v3: Drop rename of settlbcam to preptlbcam, as the settlbcam wrapper was unused. arch/powerpc/kernel/setup_64.c | 8 + arch/powerpc/mm/fsl_booke_mmu.c | 4 +-- arch/powerpc/mm/mmu_decl.h | 1 + arch/powerpc/mm/tlb_nohash.c | 19 +++- arch/powerpc/mm/tlb_nohash_low.S | 63 5 files changed, 92 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c index bdcbb71..505ec2c 100644 --- a/arch/powerpc/kernel/setup_64.c +++ b/arch/powerpc/kernel/setup_64.c @@ -108,6 +108,14 @@ static void setup_tlb_core_data(void) for_each_possible_cpu(cpu) { int first = cpu_first_thread_sibling(cpu); + /* +* If we boot via kdump on a non-primary thread, +* make sure we point at the thread that actually +* set up this TLB. +*/ + if (cpu_first_thread_sibling(boot_cpuid) == first) + first = boot_cpuid; + paca[cpu].tcd_ptr = &paca[first].tcd; /* diff --git a/arch/powerpc/mm/fsl_booke_mmu.c b/arch/powerpc/mm/fsl_booke_mmu.c index 354ba3c..bb1f88c 100644 --- a/arch/powerpc/mm/fsl_booke_mmu.c +++ b/arch/powerpc/mm/fsl_booke_mmu.c @@ -141,8 +141,6 @@ static void settlbcam(int index, unsigned long virt, phys_addr_t phys, tlbcam_addrs[index].start = virt; tlbcam_addrs[index].limit = virt + size - 1; tlbcam_addrs[index].phys = phys; - - loadcam_entry(index); } unsigned long calc_cam_sz(unsigned long ram, unsigned long virt, @@ -188,6 +186,8 @@ static unsigned long map_mem_in_cams_addr(phys_addr_t phys, unsigned long virt, virt += cam_sz; phys += cam_sz; } + + loadcam_multi(0, i, max_cam_idx); tlbcam_index = i; #ifdef CONFIG_PPC64 diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h index 085b66b..27c3a2d 100644 --- a/arch/powerpc/mm/mmu_decl.h +++ b/arch/powerpc/mm/mmu_decl.h @@ -152,6 +152,7 @@ extern int switch_to_as1(void); extern void restore_to_as0(int esel, int offset, void *dt_ptr, int bootcpu); #endif extern void loadcam_entry(unsigned int index); +extern void loadcam_multi(int first_idx, int num, int tmp_idx); struct tlbcam { u32 MAS0; diff --git a/arch/powerpc/mm/tlb_nohash.c b/arch/powerpc/mm/tlb_nohash.c index 723a099..a7381fb 100644 --- a/arch/powerpc/mm/tlb_nohash.c +++ b/arch/powerpc/mm/tlb_nohash.c @@ -42,6 +42,7 @@ #include #include #include +#include #include #include @@ -628,10 +629,26 @@ static void early_init_this_mmu(void) #ifdef CONFIG_PPC_FSL_BOOK3E if (mmu_has_feature(MMU_FTR_TYPE_FSL_E)) { unsigned int num_cams; + int __maybe_unused cpu = smp_processor_id(); + bool map = true; /* use a quarter of the TLBCAM for bolted linear map */ num_cams = (mfspr(SPRN_TLB1CFG) & TLBnCFG_N_ENTRY) / 4; - linear_map_top = map_mem_in_cams(linear_map_top, num_cams); + + /* +* Only do the mapping once per core, or else the +* transient mapping would cause problems. +*/ +#ifdef CONFIG_SMP + if (cpu != boot_cpuid && + (cpu != cpu_first_thread_sibling(cpu) || +cpu == cpu_first_thread_sibling(boot_cpuid))) + map = false; +#endif + + if (map) + linear_map_top = map_mem_in_cams(linear_map_top, +num_cams); } #endif diff --git a/arch/powerpc/mm/tlb_nohash_low.S b/arch/powerpc/mm/tlb_nohash_low.S index 43ff3c7..68c4775 100644 --- a/arch/powerpc/mm/tlb_nohash_low.S +++ b/arch/powerpc/mm/tlb_nohash_low.S @@ -400,6 +400,7 @@ _GLOBAL(set_context) * extern void loadcam_entry(unsigned int index) * * Load TLBCAM[index] entry in to the L2 CAM MMU + *
Re: [PATCH v3 4/8] powerpc/pseries/svm: Add a powerpc version of cc_platform_has()
Le 15/09/2021 à 12:08, Borislav Petkov a écrit : On Wed, Sep 15, 2021 at 10:28:59AM +1000, Michael Ellerman wrote: I don't love it, a new C file and an out-of-line call to then call back to a static inline that for most configuration will return false ... but whatever :) Yeah, hch thinks it'll cause a big mess otherwise: https://lore.kernel.org/lkml/ysscwvpxevxw%2f...@infradead.org/ Could you please provide more explicit explanation why inlining such an helper is considered as bad practice and messy ? Because as demonstrated in my previous response some days ago, taking that outline ends up with an unneccessary ugly generated code and we don't benefit front GCC's capability to fold in and opt out unreachable code. As pointed by Michael in most cases the function will just return false so behind the performance concern, there is also the code size and code coverage topic that is to be taken into account. And even when the function doesn't return false, the only thing it does folds into a single powerpc instruction so there is really no point in making a dedicated out-of-line fonction for that and suffer the cost and the size of a function call and to justify the addition of a dedicated C file. I guess less ifdeffery is nice too. I can't see your point here. Inlining the function wouldn't add any ifdeffery as far as I can see. So, would you mind reconsidering your approach and allow architectures to provide inline implementation by just not enforcing a generic prototype ? Or otherwise provide more details and exemple of why the cons are more important versus the pros ? Thanks Christophe ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v3 4/8] powerpc/pseries/svm: Add a powerpc version of cc_platform_has()
Le 14/09/2021 à 13:58, Borislav Petkov a écrit : On Wed, Sep 08, 2021 at 05:58:35PM -0500, Tom Lendacky wrote: Introduce a powerpc version of the cc_platform_has() function. This will be used to replace the powerpc mem_encrypt_active() implementation, so the implementation will initially only support the CC_ATTR_MEM_ENCRYPT attribute. Cc: Michael Ellerman Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Signed-off-by: Tom Lendacky --- arch/powerpc/platforms/pseries/Kconfig | 1 + arch/powerpc/platforms/pseries/Makefile | 2 ++ arch/powerpc/platforms/pseries/cc_platform.c | 26 3 files changed, 29 insertions(+) create mode 100644 arch/powerpc/platforms/pseries/cc_platform.c Michael, can I get an ACK for the ppc bits to carry them through the tip tree pls? Btw, on a related note, cross-compiling this throws the following error here: $ make CROSS_COMPILE=/home/share/src/crosstool/gcc-9.4.0-nolibc/powerpc64-linux/bin/powerpc64-linux- V=1 ARCH=powerpc ... /home/share/src/crosstool/gcc-9.4.0-nolibc/powerpc64-linux/bin/powerpc64-linux-gcc -Wp,-MD,arch/powerpc/boot/.crt0.o.d -D__ASSEMBLY__ -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -O2 -msoft-float -mno-altivec -mno-vsx -pipe -fomit-frame-pointer -fno-builtin -fPIC -nostdinc -include ./include/linux/compiler_attributes.h -I./arch/powerpc/include -I./arch/powerpc/include/generated -I./include -I./arch/powerpc/include/uapi -I./arch/powerpc/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h -m32 -isystem /home/share/src/crosstool/gcc-9.4.0-nolibc/powerpc64-linux/bin/../lib/gcc/powerpc64-linux/9.4.0/include -mbig-endian -nostdinc -c -o arch/powerpc/boot/crt0.o arch/powerpc/boot/crt0.S In file included from : ././include/linux/compiler_attributes.h:62:5: warning: "__has_attribute" is not defined, evaluates to 0 [-Wundef] 62 | #if __has_attribute(__assume_aligned__) | ^~~ ././include/linux/compiler_attributes.h:62:20: error: missing binary operator before token "(" 62 | #if __has_attribute(__assume_aligned__) |^ ././include/linux/compiler_attributes.h:88:5: warning: "__has_attribute" is not defined, evaluates to 0 [-Wundef] 88 | #if __has_attribute(__copy__) | ^~~ ... Known issue? This __has_attribute() thing is supposed to be supported in gcc since 5.1 and I'm using the crosstool stuff from https://www.kernel.org/pub/tools/crosstool/ and gcc-9.4 above is pretty new so that should not happen actually. But it does... Hmmm. Yes, see https://lore.kernel.org/linuxppc-dev/20210914123919.58203...@canb.auug.org.au/T/#t ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v3 4/8] powerpc/pseries/svm: Add a powerpc version of cc_platform_has()
On 9/8/21 10:58 PM, Tom Lendacky wrote: Introduce a powerpc version of the cc_platform_has() function. This will be used to replace the powerpc mem_encrypt_active() implementation, so the implementation will initially only support the CC_ATTR_MEM_ENCRYPT attribute. Cc: Michael Ellerman Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Signed-off-by: Tom Lendacky --- arch/powerpc/platforms/pseries/Kconfig | 1 + arch/powerpc/platforms/pseries/Makefile | 2 ++ arch/powerpc/platforms/pseries/cc_platform.c | 26 3 files changed, 29 insertions(+) create mode 100644 arch/powerpc/platforms/pseries/cc_platform.c diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig index 5e037df2a3a1..2e57391e0778 100644 --- a/arch/powerpc/platforms/pseries/Kconfig +++ b/arch/powerpc/platforms/pseries/Kconfig @@ -159,6 +159,7 @@ config PPC_SVM select SWIOTLB select ARCH_HAS_MEM_ENCRYPT select ARCH_HAS_FORCE_DMA_UNENCRYPTED + select ARCH_HAS_CC_PLATFORM help There are certain POWER platforms which support secure guests using the Protected Execution Facility, with the help of an Ultravisor diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile index 4cda0ef87be0..41d8aee98da4 100644 --- a/arch/powerpc/platforms/pseries/Makefile +++ b/arch/powerpc/platforms/pseries/Makefile @@ -31,3 +31,5 @@ obj-$(CONFIG_FA_DUMP) += rtas-fadump.o obj-$(CONFIG_SUSPEND) += suspend.o obj-$(CONFIG_PPC_VAS) += vas.o + +obj-$(CONFIG_ARCH_HAS_CC_PLATFORM) += cc_platform.o diff --git a/arch/powerpc/platforms/pseries/cc_platform.c b/arch/powerpc/platforms/pseries/cc_platform.c new file mode 100644 index ..e8021af83a19 --- /dev/null +++ b/arch/powerpc/platforms/pseries/cc_platform.c @@ -0,0 +1,26 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Confidential Computing Platform Capability checks + * + * Copyright (C) 2021 Advanced Micro Devices, Inc. + * + * Author: Tom Lendacky + */ + +#include +#include + +#include +#include + +bool cc_platform_has(enum cc_attr attr) +{ Please keep this function inline as mem_encrypt_active() is + switch (attr) { + case CC_ATTR_MEM_ENCRYPT: + return is_secure_guest(); + + default: + return false; + } +} +EXPORT_SYMBOL_GPL(cc_platform_has); ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v3 2/8] mm: Introduce a function to check for confidential computing features
On 9/8/21 10:58 PM, Tom Lendacky wrote: In prep for other confidential computing technologies, introduce a generic helper function, cc_platform_has(), that can be used to check for specific I have little problem with that naming. For me CC has always meant Compiler Collection. active confidential computing attributes, like memory encryption. This is intended to eliminate having to add multiple technology-specific checks to the code (e.g. if (sev_active() || tdx_active())). Co-developed-by: Andi Kleen Signed-off-by: Andi Kleen Co-developed-by: Kuppuswamy Sathyanarayanan Signed-off-by: Kuppuswamy Sathyanarayanan Signed-off-by: Tom Lendacky --- arch/Kconfig| 3 ++ include/linux/cc_platform.h | 88 + 2 files changed, 91 insertions(+) create mode 100644 include/linux/cc_platform.h diff --git a/arch/Kconfig b/arch/Kconfig index 3743174da870..ca7c359e5da8 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -1234,6 +1234,9 @@ config RELR config ARCH_HAS_MEM_ENCRYPT bool +config ARCH_HAS_CC_PLATFORM + bool + config HAVE_SPARSE_SYSCALL_NR bool help diff --git a/include/linux/cc_platform.h b/include/linux/cc_platform.h new file mode 100644 index ..253f3ea66cd8 --- /dev/null +++ b/include/linux/cc_platform.h @@ -0,0 +1,88 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Confidential Computing Platform Capability checks + * + * Copyright (C) 2021 Advanced Micro Devices, Inc. + * + * Author: Tom Lendacky + */ + +#ifndef _CC_PLATFORM_H +#define _CC_PLATFORM_H + +#include +#include + +/** + * enum cc_attr - Confidential computing attributes + * + * These attributes represent confidential computing features that are + * currently active. + */ +enum cc_attr { + /** +* @CC_ATTR_MEM_ENCRYPT: Memory encryption is active +* +* The platform/OS is running with active memory encryption. This +* includes running either as a bare-metal system or a hypervisor +* and actively using memory encryption or as a guest/virtual machine +* and actively using memory encryption. +* +* Examples include SME, SEV and SEV-ES. +*/ + CC_ATTR_MEM_ENCRYPT, + + /** +* @CC_ATTR_HOST_MEM_ENCRYPT: Host memory encryption is active +* +* The platform/OS is running as a bare-metal system or a hypervisor +* and actively using memory encryption. +* +* Examples include SME. +*/ + CC_ATTR_HOST_MEM_ENCRYPT, + + /** +* @CC_ATTR_GUEST_MEM_ENCRYPT: Guest memory encryption is active +* +* The platform/OS is running as a guest/virtual machine and actively +* using memory encryption. +* +* Examples include SEV and SEV-ES. +*/ + CC_ATTR_GUEST_MEM_ENCRYPT, + + /** +* @CC_ATTR_GUEST_STATE_ENCRYPT: Guest state encryption is active +* +* The platform/OS is running as a guest/virtual machine and actively +* using memory encryption and register state encryption. +* +* Examples include SEV-ES. +*/ + CC_ATTR_GUEST_STATE_ENCRYPT, +}; + +#ifdef CONFIG_ARCH_HAS_CC_PLATFORM + +/** + * cc_platform_has() - Checks if the specified cc_attr attribute is active + * @attr: Confidential computing attribute to check + * + * The cc_platform_has() function will return an indicator as to whether the + * specified Confidential Computing attribute is currently active. + * + * Context: Any context + * Return: + * * TRUE - Specified Confidential Computing attribute is active + * * FALSE - Specified Confidential Computing attribute is not active + */ +bool cc_platform_has(enum cc_attr attr); This declaration make it impossible for architectures to define this function inline. For such function, having it inline would make more sense as it would allow GCC to perform constant folding and avoid the overhead of calling a sub-function. + +#else /* !CONFIG_ARCH_HAS_CC_PLATFORM */ + +static inline bool cc_platform_has(enum cc_attr attr) { return false; } + +#endif /* CONFIG_ARCH_HAS_CC_PLATFORM */ + +#endif /* _CC_PLATFORM_H */ ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v3 8/8] treewide: Replace the use of mem_encrypt_active() with cc_platform_has()
On 9/8/21 10:58 PM, Tom Lendacky wrote: diff --git a/arch/powerpc/include/asm/mem_encrypt.h b/arch/powerpc/include/asm/mem_encrypt.h index ba9dab07c1be..2f26b8fc8d29 100644 --- a/arch/powerpc/include/asm/mem_encrypt.h +++ b/arch/powerpc/include/asm/mem_encrypt.h @@ -10,11 +10,6 @@ #include -static inline bool mem_encrypt_active(void) -{ - return is_secure_guest(); -} - static inline bool force_dma_unencrypted(struct device *dev) { return is_secure_guest(); diff --git a/arch/powerpc/platforms/pseries/svm.c b/arch/powerpc/platforms/pseries/svm.c index 87f001b4c4e4..c083ecbbae4d 100644 --- a/arch/powerpc/platforms/pseries/svm.c +++ b/arch/powerpc/platforms/pseries/svm.c @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -63,7 +64,7 @@ void __init svm_swiotlb_init(void) int set_memory_encrypted(unsigned long addr, int numpages) { - if (!mem_encrypt_active()) + if (!cc_platform_has(CC_ATTR_MEM_ENCRYPT)) return 0; if (!PAGE_ALIGNED(addr)) @@ -76,7 +77,7 @@ int set_memory_encrypted(unsigned long addr, int numpages) int set_memory_decrypted(unsigned long addr, int numpages) { - if (!mem_encrypt_active()) + if (!cc_platform_has(CC_ATTR_MEM_ENCRYPT)) return 0; if (!PAGE_ALIGNED(addr)) This change unnecessarily complexifies the two functions. This is due to cc_platform_has() being out-line. It should really remain inline. Before the change we got: <.set_memory_encrypted>: 0: 7d 20 00 a6 mfmsr r9 4: 75 29 00 40 andis. r9,r9,64 8: 41 82 00 48 beq 50 <.set_memory_encrypted+0x50> c: 78 69 04 20 clrldi r9,r3,48 10: 2c 29 00 00 cmpdi r9,0 14: 40 82 00 4c bne 60 <.set_memory_encrypted+0x60> 18: 7c 08 02 a6 mflrr0 1c: 7c 85 23 78 mr r5,r4 20: 78 64 85 02 rldicl r4,r3,48,20 24: 61 23 f1 34 ori r3,r9,61748 28: f8 01 00 10 std r0,16(r1) 2c: f8 21 ff 91 stdur1,-112(r1) 30: 48 00 00 01 bl 30 <.set_memory_encrypted+0x30> 30: R_PPC64_REL24 .ucall_norets 34: 60 00 00 00 nop 38: 38 60 00 00 li r3,0 3c: 38 21 00 70 addir1,r1,112 40: e8 01 00 10 ld r0,16(r1) 44: 7c 08 03 a6 mtlrr0 48: 4e 80 00 20 blr 50: 38 60 00 00 li r3,0 54: 4e 80 00 20 blr 60: 38 60 ff ea li r3,-22 64: 4e 80 00 20 blr After the change we get: <.set_memory_encrypted>: 0: 7c 08 02 a6 mflrr0 4: fb c1 ff f0 std r30,-16(r1) 8: fb e1 ff f8 std r31,-8(r1) c: 7c 7f 1b 78 mr r31,r3 10: 38 60 00 00 li r3,0 14: 7c 9e 23 78 mr r30,r4 18: f8 01 00 10 std r0,16(r1) 1c: f8 21 ff 81 stdur1,-128(r1) 20: 48 00 00 01 bl 20 <.set_memory_encrypted+0x20> 20: R_PPC64_REL24 .cc_platform_has 24: 60 00 00 00 nop 28: 2c 23 00 00 cmpdi r3,0 2c: 41 82 00 44 beq 70 <.set_memory_encrypted+0x70> 30: 7b e9 04 20 clrldi r9,r31,48 34: 2c 29 00 00 cmpdi r9,0 38: 40 82 00 58 bne 90 <.set_memory_encrypted+0x90> 3c: 38 60 00 00 li r3,0 40: 7f c5 f3 78 mr r5,r30 44: 7b e4 85 02 rldicl r4,r31,48,20 48: 60 63 f1 34 ori r3,r3,61748 4c: 48 00 00 01 bl 4c <.set_memory_encrypted+0x4c> 4c: R_PPC64_REL24 .ucall_norets 50: 60 00 00 00 nop 54: 38 60 00 00 li r3,0 58: 38 21 00 80 addir1,r1,128 5c: e8 01 00 10 ld r0,16(r1) 60: eb c1 ff f0 ld r30,-16(r1) 64: eb e1 ff f8 ld r31,-8(r1) 68: 7c 08 03 a6 mtlrr0 6c: 4e 80 00 20 blr 70: 38 21 00 80 addir1,r1,128 74: 38 60 00 00 li r3,0 78: e8 01 00 10 ld r0,16(r1) 7c: eb c1 ff f0 ld r30,-16(r1) 80: eb e1 ff f8 ld r31,-8(r1) 84: 7c 08 03 a6 mtlrr0 88: 4e 80 00 20 blr 90: 38 60 ff ea li r3,-22 94: 4b ff ff c4 b 58 <.set_memory_encrypted+0x58> ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 07/11] treewide: Replace the use of mem_encrypt_active() with prot_guest_has()
Le 28/07/2021 à 00:26, Tom Lendacky a écrit : Replace occurrences of mem_encrypt_active() with calls to prot_guest_has() with the PATTR_MEM_ENCRYPT attribute. What about https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20210730114231.23445-1-w...@kernel.org/ ? Christophe Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Borislav Petkov Cc: Dave Hansen Cc: Andy Lutomirski Cc: Peter Zijlstra Cc: David Airlie Cc: Daniel Vetter Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Thomas Zimmermann Cc: VMware Graphics Cc: Joerg Roedel Cc: Will Deacon Cc: Dave Young Cc: Baoquan He Signed-off-by: Tom Lendacky --- arch/x86/kernel/head64.c| 4 ++-- arch/x86/mm/ioremap.c | 4 ++-- arch/x86/mm/mem_encrypt.c | 5 ++--- arch/x86/mm/pat/set_memory.c| 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 +++- drivers/gpu/drm/drm_cache.c | 4 ++-- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 4 ++-- drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 6 +++--- drivers/iommu/amd/iommu.c | 3 ++- drivers/iommu/amd/iommu_v2.c| 3 ++- drivers/iommu/iommu.c | 3 ++- fs/proc/vmcore.c| 6 +++--- kernel/dma/swiotlb.c| 4 ++-- 13 files changed, 29 insertions(+), 24 deletions(-) diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c index de01903c3735..cafed6456d45 100644 --- a/arch/x86/kernel/head64.c +++ b/arch/x86/kernel/head64.c @@ -19,7 +19,7 @@ #include #include #include -#include +#include #include #include @@ -285,7 +285,7 @@ unsigned long __head __startup_64(unsigned long physaddr, * there is no need to zero it after changing the memory encryption * attribute. */ - if (mem_encrypt_active()) { + if (prot_guest_has(PATTR_MEM_ENCRYPT)) { vaddr = (unsigned long)__start_bss_decrypted; vaddr_end = (unsigned long)__end_bss_decrypted; for (; vaddr < vaddr_end; vaddr += PMD_SIZE) { diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c index 0f2d5ace5986..5e1c1f5cbbe8 100644 --- a/arch/x86/mm/ioremap.c +++ b/arch/x86/mm/ioremap.c @@ -693,7 +693,7 @@ static bool __init early_memremap_is_setup_data(resource_size_t phys_addr, bool arch_memremap_can_ram_remap(resource_size_t phys_addr, unsigned long size, unsigned long flags) { - if (!mem_encrypt_active()) + if (!prot_guest_has(PATTR_MEM_ENCRYPT)) return true; if (flags & MEMREMAP_ENC) @@ -723,7 +723,7 @@ pgprot_t __init early_memremap_pgprot_adjust(resource_size_t phys_addr, { bool encrypted_prot; - if (!mem_encrypt_active()) + if (!prot_guest_has(PATTR_MEM_ENCRYPT)) return prot; encrypted_prot = true; diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c index 451de8e84fce..0f1533dbe81c 100644 --- a/arch/x86/mm/mem_encrypt.c +++ b/arch/x86/mm/mem_encrypt.c @@ -364,8 +364,7 @@ int __init early_set_memory_encrypted(unsigned long vaddr, unsigned long size) /* * SME and SEV are very similar but they are not the same, so there are * times that the kernel will need to distinguish between SME and SEV. The - * sme_active() and sev_active() functions are used for this. When a - * distinction isn't needed, the mem_encrypt_active() function can be used. + * sme_active() and sev_active() functions are used for this. * * The trampoline code is a good example for this requirement. Before * paging is activated, SME will access all memory as decrypted, but SEV @@ -451,7 +450,7 @@ void __init mem_encrypt_free_decrypted_mem(void) * The unused memory range was mapped decrypted, change the encryption * attribute from decrypted to encrypted before freeing it. */ - if (mem_encrypt_active()) { + if (sme_me_mask) { r = set_memory_encrypted(vaddr, npages); if (r) { pr_warn("failed to free unused decrypted pages\n"); diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c index ad8a5c586a35..6925f2bb4be1 100644 --- a/arch/x86/mm/pat/set_memory.c +++ b/arch/x86/mm/pat/set_memory.c @@ -18,6 +18,7 @@ #include #include #include +#include #include #include @@ -1986,7 +1987,7 @@ static int __set_memory_enc_dec(unsigned long addr, int numpages, bool enc) int ret; /* Nothing to do if memory encryption is not active */ - if (!mem_encrypt_active()) + if (!prot_guest_has(PATTR_MEM_ENCRYPT)) return 0; /* Should not be working on unaligned addresses */ diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index abb928894eac..8407224717df 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -38,6 +38,7 @@ #i
Re: [PATCH 02/11] powerpc/kexec_file: mark PPC64 specific code
Le 26/06/2020 à 21:04, Hari Bathini a écrit : Some of the kexec_file_load code isn't PPC64 specific. Move PPC64 specific code from kexec/file_load.c to kexec/file_load_64.c. Also, rename purgatory/trampoline.S to purgatory/trampoline_64.S in the same spirit. At the time being, CONFIG_KEXEC_FILE depends on PPC64. Are you planning to make it work on PPC32 as well ? Otherwise I don't understand the purpose of this patch. Also, what is being done in this patch seems to go far beyond what you describe above. It is propably worth splitting in several patches with proper explanation. Christophe Signed-off-by: Hari Bathini --- arch/powerpc/include/asm/kexec.h | 11 +++ arch/powerpc/kexec/Makefile|2 - arch/powerpc/kexec/elf_64.c|7 +- arch/powerpc/kexec/file_load.c | 37 ++ arch/powerpc/kexec/file_load_64.c | 108 ++ arch/powerpc/purgatory/Makefile|4 + arch/powerpc/purgatory/trampoline.S| 117 arch/powerpc/purgatory/trampoline_64.S | 117 8 files changed, 248 insertions(+), 155 deletions(-) create mode 100644 arch/powerpc/kexec/file_load_64.c delete mode 100644 arch/powerpc/purgatory/trampoline.S create mode 100644 arch/powerpc/purgatory/trampoline_64.S diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h index c684768..7008ea1 100644 --- a/arch/powerpc/include/asm/kexec.h +++ b/arch/powerpc/include/asm/kexec.h @@ -114,8 +114,17 @@ int setup_purgatory(struct kimage *image, const void *slave_code, unsigned long fdt_load_addr); int setup_new_fdt(const struct kimage *image, void *fdt, unsigned long initrd_load_addr, unsigned long initrd_len, - const char *cmdline); + const char *cmdline, int *node); int delete_fdt_mem_rsv(void *fdt, unsigned long start, unsigned long size); + +#ifdef CONFIG_PPC64 +int setup_purgatory_ppc64(struct kimage *image, const void *slave_code, + const void *fdt, unsigned long kernel_load_addr, + unsigned long fdt_load_addr); +int setup_new_fdt_ppc64(const struct kimage *image, void *fdt, + unsigned long initrd_load_addr, + unsigned long initrd_len, const char *cmdline); +#endif /* CONFIG_PPC64 */ #endif /* CONFIG_KEXEC_FILE */ #else /* !CONFIG_KEXEC_CORE */ diff --git a/arch/powerpc/kexec/Makefile b/arch/powerpc/kexec/Makefile index 86380c6..67c3553 100644 --- a/arch/powerpc/kexec/Makefile +++ b/arch/powerpc/kexec/Makefile @@ -7,7 +7,7 @@ obj-y += core.o crash.o core_$(BITS).o obj-$(CONFIG_PPC32) += relocate_32.o -obj-$(CONFIG_KEXEC_FILE) += file_load.o elf_$(BITS).o +obj-$(CONFIG_KEXEC_FILE) += file_load.o file_load_$(BITS).o elf_$(BITS).o ifdef CONFIG_HAVE_IMA_KEXEC ifdef CONFIG_IMA diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c index 3072fd6..23ad04c 100644 --- a/arch/powerpc/kexec/elf_64.c +++ b/arch/powerpc/kexec/elf_64.c @@ -88,7 +88,8 @@ static void *elf64_load(struct kimage *image, char *kernel_buf, goto out; } - ret = setup_new_fdt(image, fdt, initrd_load_addr, initrd_len, cmdline); + ret = setup_new_fdt_ppc64(image, fdt, initrd_load_addr, + initrd_len, cmdline); if (ret) goto out; @@ -107,8 +108,8 @@ static void *elf64_load(struct kimage *image, char *kernel_buf, pr_debug("Loaded device tree at 0x%lx\n", fdt_load_addr); slave_code = elf_info.buffer + elf_info.proghdrs[0].p_offset; - ret = setup_purgatory(image, slave_code, fdt, kernel_load_addr, - fdt_load_addr); + ret = setup_purgatory_ppc64(image, slave_code, fdt, kernel_load_addr, + fdt_load_addr); if (ret) pr_err("Error setting up the purgatory.\n"); diff --git a/arch/powerpc/kexec/file_load.c b/arch/powerpc/kexec/file_load.c index 143c917..99a2c4d 100644 --- a/arch/powerpc/kexec/file_load.c +++ b/arch/powerpc/kexec/file_load.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * ppc64 code to implement the kexec_file_load syscall + * powerpc code to implement the kexec_file_load syscall * * Copyright (C) 2004 Adam Litke (a...@us.ibm.com) * Copyright (C) 2004 IBM Corp. @@ -16,26 +16,10 @@ #include #include -#include #include #include -#define SLAVE_CODE_SIZE 256 - -const struct kexec_file_ops * const kexec_file_loaders[] = { - &kexec_elf64_ops, - NULL -}; - -int arch_kexec_kernel_image_probe(struct kimage *image, void *buf, - unsigned long buf_len) -{ - /* We don't support crash kernels yet. */ - if (image->type == KEXEC_TYPE_CRASH) -
Re: [PATCH 01/11] kexec_file: allow archs to handle special regions while locating memory hole
Le 26/06/2020 à 21:04, Hari Bathini a écrit : Some archs can have special memory regions, within the given memory range, which can't be used for the buffer in a kexec segment. As kexec_add_buffer() function is being called from generic code as well, add weak arch_kexec_add_buffer definition for archs to override & take care of special regions before trying to locate a memory hole. Signed-off-by: Hari Bathini --- include/linux/kexec.h |5 + kernel/kexec_file.c | 37 + 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/include/linux/kexec.h b/include/linux/kexec.h index 1776eb2..1237682 100644 --- a/include/linux/kexec.h +++ b/include/linux/kexec.h @@ -195,6 +195,11 @@ int __weak arch_kexec_apply_relocations(struct purgatory_info *pi, const Elf_Shdr *relsec, const Elf_Shdr *symtab); +extern int arch_kexec_add_buffer(struct kexec_buf *kbuf); + extern keywork is useless here, please remove (checkpatch also complains about it usually). +/* arch_kexec_add_buffer calls this when it is ready */ +extern int __kexec_add_buffer(struct kexec_buf *kbuf); + same extern int kexec_add_buffer(struct kexec_buf *kbuf); int kexec_locate_mem_hole(struct kexec_buf *kbuf); diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c index bb05fd5..a0b4f7f 100644 --- a/kernel/kexec_file.c +++ b/kernel/kexec_file.c @@ -669,10 +669,6 @@ int kexec_locate_mem_hole(struct kexec_buf *kbuf) */ int kexec_add_buffer(struct kexec_buf *kbuf) { - - struct kexec_segment *ksegment; - int ret; - /* Currently adding segment this way is allowed only in file mode */ if (!kbuf->image->file_mode) return -EINVAL; @@ -696,6 +692,25 @@ int kexec_add_buffer(struct kexec_buf *kbuf) kbuf->memsz = ALIGN(kbuf->memsz, PAGE_SIZE); kbuf->buf_align = max(kbuf->buf_align, PAGE_SIZE); + return arch_kexec_add_buffer(kbuf); +} + +/** + * __kexec_add_buffer - arch_kexec_add_buffer would call this function after + * updating kbuf, to place a buffer in a kexec segment. + * @kbuf: Buffer contents and memory parameters. + * + * This function assumes that kexec_mutex is held. + * On successful return, @kbuf->mem will have the physical address of + * the buffer in memory. + * + * Return: 0 on success, negative errno on error. + */ +int __kexec_add_buffer(struct kexec_buf *kbuf) +{ + struct kexec_segment *ksegment; + int ret; + /* Walk the RAM ranges and allocate a suitable range for the buffer */ ret = kexec_locate_mem_hole(kbuf); if (ret) @@ -711,6 +726,20 @@ int kexec_add_buffer(struct kexec_buf *kbuf) return 0; } +/** + * arch_kexec_add_buffer - Some archs have memory regions within the given + * range that can't be used to place a kexec segment. + * Such archs can override this function to take care + * of them before trying to locate the memory hole. + * @kbuf: Buffer contents and memory parameters. + * + * Return: 0 on success, negative errno on error. + */ +int __weak arch_kexec_add_buffer(struct kexec_buf *kbuf) +{ + return __kexec_add_buffer(kbuf); +} + /* Calculate and store the digest of segments */ static int kexec_calculate_store_digests(struct kimage *image) { Christophe ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 3/3] pseries/scm: buffer pmem's bound addr in dt for kexec kernel
Le 28/02/2020 à 06:53, Pingfan Liu a écrit : At present, plpar_hcall(H_SCM_BIND_MEM, ...) takes a very long time, so if dumping to fsdax, it will take a very long time. Take a closer look, during the papr_scm initialization, the only configuration is through drc_pmem_bind()-> plpar_hcall(H_SCM_BIND_MEM, ...), which helps to set up the bound address. On pseries, for kexec -l/-p kernel, there is no reset of hardware, and this step can be stepped around to save times. So the pmem bound address can be passed to the 2nd kernel through a dynamic added property "bound-addr" in dt node 'ibm,pmemory'. Signed-off-by: Pingfan Liu To: linuxppc-...@lists.ozlabs.org Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: Michael Ellerman Cc: Hari Bathini Cc: Aneesh Kumar K.V Cc: Oliver O'Halloran Cc: Dan Williams Cc: kexec@lists.infradead.org --- note: I can not find such a pseries machine, and not finish it yet. --- arch/powerpc/platforms/pseries/papr_scm.c | 32 +-- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c index c2ef320..555e746 100644 --- a/arch/powerpc/platforms/pseries/papr_scm.c +++ b/arch/powerpc/platforms/pseries/papr_scm.c @@ -382,7 +382,7 @@ static int papr_scm_probe(struct platform_device *pdev) { struct device_node *dn = pdev->dev.of_node; u32 drc_index, metadata_size; - u64 blocks, block_size; + u64 blocks, block_size, bound_addr = 0; struct papr_scm_priv *p; const char *uuid_str; u64 uuid[2]; @@ -439,17 +439,29 @@ static int papr_scm_probe(struct platform_device *pdev) p->metadata_size = metadata_size; p->pdev = pdev; - /* request the hypervisor to bind this region to somewhere in memory */ - rc = drc_pmem_bind(p); + of_property_read_u64(dn, "bound-addr", &bound_addr); + if (bound_addr) + p->bound_addr = bound_addr; + else { All legs of an if/else must have { } when one leg need them, see codying style. + struct property *property; + u64 big; - /* If phyp says drc memory still bound then force unbound and retry */ - if (rc == H_OVERLAP) - rc = drc_pmem_query_n_bind(p); + /* request the hypervisor to bind this region to somewhere in memory */ + rc = drc_pmem_bind(p); - if (rc != H_SUCCESS) { - dev_err(&p->pdev->dev, "bind err: %d\n", rc); - rc = -ENXIO; - goto err; + /* If phyp says drc memory still bound then force unbound and retry */ + if (rc == H_OVERLAP) + rc = drc_pmem_query_n_bind(p); + + if (rc != H_SUCCESS) { + dev_err(&p->pdev->dev, "bind err: %d\n", rc); + rc = -ENXIO; + goto err; + } + big = cpu_to_be64(p->bound_addr); + property = new_property("bound-addr", sizeof(u64), &big, + NULL); Why plitting this line in two parts ? You have lines far longer above. In powerpc we allow lines 90 chars long. + of_add_property(dn, property); } /* setup the resource for the newly bound range */ Christophe ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 1/3] powerpc/of: split out new_property() for reusing
Le 28/02/2020 à 06:53, Pingfan Liu a écrit : Since new_property() is used in several calling sites, splitting it out for reusing. To ease the review, although the split out part has coding style issue, keeping it untouched and fixed in next patch. The moved function fits in one screen. I don't think it is worth splitting out for coding style that applies on the new lines only. You can squash the two patches, it will still be easy to review. Signed-off-by: Pingfan Liu To: linuxppc-...@lists.ozlabs.org Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: Michael Ellerman Cc: Hari Bathini Cc: Aneesh Kumar K.V Cc: Oliver O'Halloran Cc: Dan Williams Cc: kexec@lists.infradead.org --- arch/powerpc/include/asm/prom.h | 2 ++ arch/powerpc/kernel/Makefile | 2 +- arch/powerpc/kernel/of_property.c | 32 +++ arch/powerpc/mm/drmem.c | 26 - arch/powerpc/platforms/pseries/reconfig.c | 26 - 5 files changed, 35 insertions(+), 53 deletions(-) create mode 100644 arch/powerpc/kernel/of_property.c diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h index 94e3fd5..02f7b1b 100644 --- a/arch/powerpc/include/asm/prom.h +++ b/arch/powerpc/include/asm/prom.h @@ -90,6 +90,8 @@ struct of_drc_info { extern int of_read_drc_info_cell(struct property **prop, const __be32 **curval, struct of_drc_info *data); +extern struct property *new_property(const char *name, const int length, + const unsigned char *value, struct property *last); Don't add useless 'extern' keyword. /* * There are two methods for telling firmware what our capabilities are. diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile index 157b014..23375fd 100644 --- a/arch/powerpc/kernel/Makefile +++ b/arch/powerpc/kernel/Makefile @@ -47,7 +47,7 @@ obj-y := cputable.o ptrace.o syscalls.o \ signal.o sysfs.o cacheinfo.o time.o \ prom.o traps.o setup-common.o \ udbg.o misc.o io.o misc_$(BITS).o \ - of_platform.o prom_parse.o + of_platform.o prom_parse.o of_property.o obj-$(CONFIG_PPC64) += setup_64.o sys_ppc32.o \ signal_64.o ptrace32.o \ paca.o nvram_64.o firmware.o note.o diff --git a/arch/powerpc/kernel/of_property.c b/arch/powerpc/kernel/of_property.c new file mode 100644 index 000..e56c832 --- /dev/null +++ b/arch/powerpc/kernel/of_property.c @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: GPL-2.0-only +#include +#include +#include +#include + +struct property *new_property(const char *name, const int length, +const unsigned char *value, struct property *last) +{ + struct property *new = kzalloc(sizeof(*new), GFP_KERNEL); + + if (!new) + return NULL; + + if (!(new->name = kstrdup(name, GFP_KERNEL))) + goto cleanup; + if (!(new->value = kmalloc(length + 1, GFP_KERNEL))) + goto cleanup; + + memcpy(new->value, value, length); + *(((char *)new->value) + length) = 0; + new->length = length; + new->next = last; + return new; + +cleanup: + kfree(new->name); + kfree(new->value); + kfree(new); + return NULL; +} + diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c index 85b088a..888227e 100644 --- a/arch/powerpc/mm/drmem.c +++ b/arch/powerpc/mm/drmem.c @@ -99,32 +99,6 @@ static void init_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell, extern int test_hotplug; -static struct property *new_property(const char *name, const int length, -const unsigned char *value, struct property *last) -{ - struct property *new = kzalloc(sizeof(*new), GFP_KERNEL); - - if (!new) - return NULL; - - if (!(new->name = kstrdup(name, GFP_KERNEL))) - goto cleanup; - if (!(new->value = kmalloc(length + 1, GFP_KERNEL))) - goto cleanup; - - memcpy(new->value, value, length); - *(((char *)new->value) + length) = 0; - new->length = length; - new->next = last; - return new; - -cleanup: - kfree(new->name); - kfree(new->value); - kfree(new); - return NULL; -} - static int drmem_update_dt_v2(struct device_node *memory, struct property *prop) { diff --git a/arch/powerpc/platforms/pseries/reconfig.c b/arch/powerpc/platforms/pseries/reconfig.c index 7f7369f..8e5a2ba 100644 --- a/arch/powerpc/platforms/pseries/reconfig.c +++ b/arch/powerpc/platforms/pseries/reconfig.c @@ -165,32 +165,6 @@ static char * parse_next_property(ch
Re: [PATCH v3 5/7] kexec_elf: remove elf_addr_to_cpu macro
Le 10/07/2019 à 16:29, Sven Schnelle a écrit : It had only one definition, so just use the function directly. It had only one definition because it was for ppc64 only. But as far as I understand (at least from the name of the new file), you want it to be generic, don't you ? Therefore I get on 32 bits it would be elf32_to_cpu(). Christophe Signed-off-by: Sven Schnelle --- kernel/kexec_elf.c | 20 +--- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c index 70d31b8feeae..99e6d63b5dfc 100644 --- a/kernel/kexec_elf.c +++ b/kernel/kexec_elf.c @@ -8,8 +8,6 @@ #include #include -#define elf_addr_to_cpu elf64_to_cpu - #ifndef Elf_Rel #define Elf_Rel Elf64_Rel #endif /* Elf_Rel */ @@ -143,9 +141,9 @@ static int elf_read_ehdr(const char *buf, size_t len, struct elfhdr *ehdr) ehdr->e_type = elf16_to_cpu(ehdr, buf_ehdr->e_type); ehdr->e_machine = elf16_to_cpu(ehdr, buf_ehdr->e_machine); ehdr->e_version = elf32_to_cpu(ehdr, buf_ehdr->e_version); - ehdr->e_entry = elf_addr_to_cpu(ehdr, buf_ehdr->e_entry); - ehdr->e_phoff = elf_addr_to_cpu(ehdr, buf_ehdr->e_phoff); - ehdr->e_shoff = elf_addr_to_cpu(ehdr, buf_ehdr->e_shoff); + ehdr->e_entry = elf64_to_cpu(ehdr, buf_ehdr->e_entry); + ehdr->e_phoff = elf64_to_cpu(ehdr, buf_ehdr->e_phoff); + ehdr->e_shoff = elf64_to_cpu(ehdr, buf_ehdr->e_shoff); ehdr->e_flags = elf32_to_cpu(ehdr, buf_ehdr->e_flags); ehdr->e_phentsize = elf16_to_cpu(ehdr, buf_ehdr->e_phentsize); ehdr->e_phnum = elf16_to_cpu(ehdr, buf_ehdr->e_phnum); @@ -190,18 +188,18 @@ static int elf_read_phdr(const char *buf, size_t len, buf_phdr = (struct elf_phdr *) pbuf; phdr->p_type = elf32_to_cpu(elf_info->ehdr, buf_phdr->p_type); - phdr->p_offset = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_offset); - phdr->p_paddr = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_paddr); - phdr->p_vaddr = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_vaddr); + phdr->p_offset = elf64_to_cpu(elf_info->ehdr, buf_phdr->p_offset); + phdr->p_paddr = elf64_to_cpu(elf_info->ehdr, buf_phdr->p_paddr); + phdr->p_vaddr = elf64_to_cpu(elf_info->ehdr, buf_phdr->p_vaddr); phdr->p_flags = elf32_to_cpu(elf_info->ehdr, buf_phdr->p_flags); /* * The following fields have a type equivalent to Elf_Addr * both in 32 bit and 64 bit ELF. */ - phdr->p_filesz = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_filesz); - phdr->p_memsz = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_memsz); - phdr->p_align = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_align); + phdr->p_filesz = elf64_to_cpu(elf_info->ehdr, buf_phdr->p_filesz); + phdr->p_memsz = elf64_to_cpu(elf_info->ehdr, buf_phdr->p_memsz); + phdr->p_align = elf64_to_cpu(elf_info->ehdr, buf_phdr->p_align); return elf_is_phdr_sane(phdr, len) ? 0 : -ENOEXEC; } ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v3 6/7] kexec_elf: remove Elf_Rel macro
Le 10/07/2019 à 16:29, Sven Schnelle a écrit : It wasn't used anywhere, so lets drop it. And also, it is already defined in asm-generic/module.h Signed-off-by: Sven Schnelle Reviewed-by: Christophe Leroy --- kernel/kexec_elf.c | 4 1 file changed, 4 deletions(-) diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c index 99e6d63b5dfc..b7e47ddd7cad 100644 --- a/kernel/kexec_elf.c +++ b/kernel/kexec_elf.c @@ -8,10 +8,6 @@ #include #include -#ifndef Elf_Rel -#define Elf_RelElf64_Rel -#endif /* Elf_Rel */ - static inline bool elf_is_elf_file(const struct elfhdr *ehdr) { return memcmp(ehdr->e_ident, ELFMAG, SELFMAG) == 0; ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v3 7/7] kexec_elf: remove unused variable in kexec_elf_load()
Le 10/07/2019 à 16:29, Sven Schnelle a écrit : base was never unsigned, so we can remove it. Do you mean never assigned ? Signed-off-by: Sven Schnelle Reviewed-by: Christophe Leroy --- kernel/kexec_elf.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c index b7e47ddd7cad..a56ec5481e71 100644 --- a/kernel/kexec_elf.c +++ b/kernel/kexec_elf.c @@ -348,7 +348,7 @@ int kexec_elf_load(struct kimage *image, struct elfhdr *ehdr, struct kexec_buf *kbuf, unsigned long *lowest_load_addr) { - unsigned long base = 0, lowest_addr = UINT_MAX; + unsigned long lowest_addr = UINT_MAX; int ret; size_t i; @@ -370,7 +370,7 @@ int kexec_elf_load(struct kimage *image, struct elfhdr *ehdr, kbuf->bufsz = size; kbuf->memsz = phdr->p_memsz; kbuf->buf_align = phdr->p_align; - kbuf->buf_min = phdr->p_paddr + base; + kbuf->buf_min = phdr->p_paddr; ret = kexec_add_buffer(kbuf); if (ret) goto out; @@ -380,9 +380,6 @@ int kexec_elf_load(struct kimage *image, struct elfhdr *ehdr, lowest_addr = load_addr; } - /* Update entry point to reflect new load address. */ - ehdr->e_entry += base; - *lowest_load_addr = lowest_addr; ret = 0; out: ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 1/7] kexec: add KEXEC_ELF
Hi Sven, On 07/09/2019 07:43 PM, Sven Schnelle wrote: Right now powerpc provides an implementation to read elf files with the kexec_file() syscall. Make that available as a public kexec interface so it can be re-used on other architectures. Signed-off-by: Sven Schnelle --- arch/Kconfig | 3 + arch/powerpc/Kconfig | 1 + arch/powerpc/kernel/kexec_elf_64.c | 551 + include/linux/kexec.h | 24 ++ kernel/Makefile| 1 + kernel/kexec_elf.c | 537 6 files changed, 576 insertions(+), 541 deletions(-) create mode 100644 kernel/kexec_elf.c Why are you persisting at not using -C when creating your patch ? Do you want to hide the changes you did while copying arch/powerpc/kernel/kexec_elf_64.c to kernel/kexec_elf.c ? Or you want to make life harder for the reviewers ? git format-patch -C shows: arch/Kconfig | 3 + arch/powerpc/Kconfig | 1 + arch/powerpc/kernel/kexec_elf_64.c | 551 + include/linux/kexec.h | 24 + kernel/Makefile| 1 + .../kernel/kexec_elf_64.c => kernel/kexec_elf.c| 199 ++-- 6 files changed, 75 insertions(+), 704 deletions(-) copy arch/powerpc/kernel/kexec_elf_64.c => kernel/kexec_elf.c (71%) I mentionned it a couple of times, I even resent your last patch formatted that way to show the advantage. I can ear if you find it worthless, but tell what your concern are with that, don't just ignore it please. Cheers Christophe diff --git a/arch/Kconfig b/arch/Kconfig index c47b328eada0..30694aca4316 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -18,6 +18,9 @@ config KEXEC_CORE select CRASH_CORE bool +config KEXEC_ELF + bool + config HAVE_IMA_KEXEC bool diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 12cee37f15c4..addc2dad78e0 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -510,6 +510,7 @@ config KEXEC_FILE select KEXEC_CORE select HAVE_IMA_KEXEC select BUILD_BIN2C + select KEXEC_ELF depends on PPC64 depends on CRYPTO=y depends on CRYPTO_SHA256=y diff --git a/arch/powerpc/kernel/kexec_elf_64.c b/arch/powerpc/kernel/kexec_elf_64.c index ba4f18a43ee8..30bd57a93c17 100644 --- a/arch/powerpc/kernel/kexec_elf_64.c +++ b/arch/powerpc/kernel/kexec_elf_64.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0-only /* * Load ELF vmlinux file for the kexec_file_load syscall. * @@ -10,15 +11,6 @@ * Based on kexec-tools' kexec-elf-exec.c and kexec-elf-ppc64.c. * Heavily modified for the kernel by * Thiago Jung Bauermann . - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation (version 2 of the License). - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. */ #define pr_fmt(fmt) "kexec_elf: " fmt @@ -39,532 +31,6 @@ #define Elf_Rel Elf64_Rel #endif /* Elf_Rel */ -struct elf_info { - /* -* Where the ELF binary contents are kept. -* Memory managed by the user of the struct. -*/ - const char *buffer; - - const struct elfhdr *ehdr; - const struct elf_phdr *proghdrs; - struct elf_shdr *sechdrs; -}; - -static inline bool elf_is_elf_file(const struct elfhdr *ehdr) -{ - return memcmp(ehdr->e_ident, ELFMAG, SELFMAG) == 0; -} - -static uint64_t elf64_to_cpu(const struct elfhdr *ehdr, uint64_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le64_to_cpu(value); - else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be64_to_cpu(value); - - return value; -} - -static uint16_t elf16_to_cpu(const struct elfhdr *ehdr, uint16_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le16_to_cpu(value); - else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be16_to_cpu(value); - - return value; -} - -static uint32_t elf32_to_cpu(const struct elfhdr *ehdr, uint32_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le32_to_cpu(value); - else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be32_to_cpu(value); - - return value; -} - -/** - * elf_is_ehdr_sane - check that it is safe to use the ELF header - * @buf_len: size of the buffer in which the ELF file is loaded. - */ -static bool elf_is_ehdr_sane(const struct elfhdr *ehdr, size_
Re: [PATCH using git format-patch -C] kexec: add generic support for elf kernel images
Hi Sven, Le 08/07/2019 à 12:06, Christophe Leroy a écrit : From: Sven Schnelle Please describe you patch. All the changes you are doing to the ppc64 version in order to make it generic should be described. Those changes are also maybe worth splitting the patch in several parts, either preparing the ppc64 for generic then moving to kernel/kexec_elf.c or moving to kernel/kexec_elf.c without modifications, then modifying it to make it generic. Note that your patch only applies on Linux 5.1, it doesn't apply on powerpc/next. I think it also be worth taking the opportunity to fix the sparse warnings, see https://patchwork.ozlabs.org/patch/1128720/ checkpatch comments should also be fixed, see https://openpower.xyz/job/snowpatch/job/snowpatch-linux-checkpatch/8044//artifact/linux/checkpatch.log Other comments below, Signed-off-by: Sven Schnelle --- patch generated with 'git format-patch -C' in order to see the modifications done in kexec_elf.c in addition to copying it from arch/powerpc/kernel/kexec_elf_64.c arch/Kconfig | 3 + arch/powerpc/Kconfig | 1 + arch/powerpc/kernel/kexec_elf_64.c | 547 + include/linux/kexec.h | 35 ++ kernel/Makefile| 1 + .../kernel/kexec_elf_64.c => kernel/kexec_elf.c| 264 +++--- 6 files changed, 118 insertions(+), 733 deletions(-) copy arch/powerpc/kernel/kexec_elf_64.c => kernel/kexec_elf.c (64%) diff --git a/arch/Kconfig b/arch/Kconfig index 3368786a..c7c75fbc0b79 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -18,6 +18,9 @@ config KEXEC_CORE select CRASH_CORE bool +config KEXEC_ELF + bool + config HAVE_IMA_KEXEC bool diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 2d0be82c3061..447b6e3ad999 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -501,6 +501,7 @@ config KEXEC_FILE select KEXEC_CORE select HAVE_IMA_KEXEC select BUILD_BIN2C + select KEXEC_ELF depends on PPC64 depends on CRYPTO=y depends on CRYPTO_SHA256=y diff --git a/arch/powerpc/kernel/kexec_elf_64.c b/arch/powerpc/kernel/kexec_elf_64.c index ba4f18a43ee8..d062c5991722 100644 --- a/arch/powerpc/kernel/kexec_elf_64.c +++ b/arch/powerpc/kernel/kexec_elf_64.c @@ -31,539 +31,7 @@ #include #include -#define PURGATORY_STACK_SIZE (16 * 1024) [snip] +#define PURGATORY_STACK_SIZE (16 * 1024) This line shouldn't be modified by your patch. I see a tab was replaced by space. static void *elf64_load(struct kimage *image, char *kernel_buf, unsigned long kernel_len, char *initrd, @@ -577,17 +45,17 @@ static void *elf64_load(struct kimage *image, char *kernel_buf, void *fdt; const void *slave_code; struct elfhdr ehdr; - struct elf_info elf_info; + struct kexec_elf_info elf_info; struct kexec_buf kbuf = { .image = image, .buf_min = 0, .buf_max = ppc64_rma_size }; struct kexec_buf pbuf = { .image = image, .buf_min = 0, .buf_max = ppc64_rma_size, .top_down = true }; - ret = build_elf_exec_info(kernel_buf, kernel_len, &ehdr, &elf_info); + ret = kexec_build_elf_info(kernel_buf, kernel_len, &ehdr, &elf_info); if (ret) goto out; - ret = elf_exec_load(image, &ehdr, &elf_info, &kernel_load_addr); + ret = kexec_elf_load(image, &ehdr, &elf_info, &kbuf, &kernel_load_addr); if (ret) goto out; @@ -606,6 +74,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf, kbuf.bufsz = kbuf.memsz = initrd_len; kbuf.buf_align = PAGE_SIZE; kbuf.top_down = false; + kbuf.mem = KEXEC_BUF_MEM_UNKNOWN; Is that correct ? I see: - kbuf.mem is unsigned - KEXEC_BUF_MEM_UNKNOWN is -1 on s390 ret = kexec_add_buffer(&kbuf); if (ret) goto out; @@ -638,6 +107,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf, kbuf.bufsz = kbuf.memsz = fdt_size; kbuf.buf_align = PAGE_SIZE; kbuf.top_down = true; + kbuf.mem = KEXEC_BUF_MEM_UNKNOWN; ret = kexec_add_buffer(&kbuf); if (ret) goto out; @@ -652,13 +122,12 @@ static void *elf64_load(struct kimage *image, char *kernel_buf, pr_err("Error setting up the purgatory.\n"); out: - elf_free_info(&elf_info); - + kexec_free_elf_info(&elf_info); /* Make kimage_file_post_load_cleanup free the fdt buffer for us. */ return ret ? ERR_PTR(ret) : fdt; } const struct kexec_file_ops kexec_elf64_ops
Re: [PATCH] kexec: add generic support for elf kernel images
Le 07/07/2019 à 21:21, Sven Schnelle a écrit : Signed-off-by: Sven Schnelle --- Please add here a description of the changes done since RFC. arch/Kconfig | 3 + arch/powerpc/Kconfig | 1 + arch/powerpc/kernel/kexec_elf_64.c | 547 + include/linux/kexec.h | 35 ++ kernel/Makefile| 1 + kernel/kexec_elf.c | 540 6 files changed, 588 insertions(+), 539 deletions(-) create mode 100644 kernel/kexec_elf.c kernel/kexec_elf.c is a modified copy of arch/powerpc/kernel/kexec_elf_64.c. You should generate your patch usign 'git format-patch -C' in order to let git identify the copy and the changes to the copy. That would ease the review. I have regenerated your patch with -C and resent it. Christophe diff --git a/arch/Kconfig b/arch/Kconfig index c47b328eada0..30694aca4316 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -18,6 +18,9 @@ config KEXEC_CORE select CRASH_CORE bool +config KEXEC_ELF + bool + config HAVE_IMA_KEXEC bool diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 8c1c636308c8..97aa81622452 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -502,6 +502,7 @@ config KEXEC_FILE select KEXEC_CORE select HAVE_IMA_KEXEC select BUILD_BIN2C + select KEXEC_ELF depends on PPC64 depends on CRYPTO=y depends on CRYPTO_SHA256=y diff --git a/arch/powerpc/kernel/kexec_elf_64.c b/arch/powerpc/kernel/kexec_elf_64.c index ba4f18a43ee8..d062c5991722 100644 --- a/arch/powerpc/kernel/kexec_elf_64.c +++ b/arch/powerpc/kernel/kexec_elf_64.c @@ -31,539 +31,7 @@ #include #include -#define PURGATORY_STACK_SIZE (16 * 1024) - -#define elf_addr_to_cpuelf64_to_cpu - -#ifndef Elf_Rel -#define Elf_RelElf64_Rel -#endif /* Elf_Rel */ - -struct elf_info { - /* -* Where the ELF binary contents are kept. -* Memory managed by the user of the struct. -*/ - const char *buffer; - - const struct elfhdr *ehdr; - const struct elf_phdr *proghdrs; - struct elf_shdr *sechdrs; -}; - -static inline bool elf_is_elf_file(const struct elfhdr *ehdr) -{ - return memcmp(ehdr->e_ident, ELFMAG, SELFMAG) == 0; -} - -static uint64_t elf64_to_cpu(const struct elfhdr *ehdr, uint64_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le64_to_cpu(value); - else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be64_to_cpu(value); - - return value; -} - -static uint16_t elf16_to_cpu(const struct elfhdr *ehdr, uint16_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le16_to_cpu(value); - else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be16_to_cpu(value); - - return value; -} - -static uint32_t elf32_to_cpu(const struct elfhdr *ehdr, uint32_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le32_to_cpu(value); - else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be32_to_cpu(value); - - return value; -} - -/** - * elf_is_ehdr_sane - check that it is safe to use the ELF header - * @buf_len: size of the buffer in which the ELF file is loaded. - */ -static bool elf_is_ehdr_sane(const struct elfhdr *ehdr, size_t buf_len) -{ - if (ehdr->e_phnum > 0 && ehdr->e_phentsize != sizeof(struct elf_phdr)) { - pr_debug("Bad program header size.\n"); - return false; - } else if (ehdr->e_shnum > 0 && - ehdr->e_shentsize != sizeof(struct elf_shdr)) { - pr_debug("Bad section header size.\n"); - return false; - } else if (ehdr->e_ident[EI_VERSION] != EV_CURRENT || - ehdr->e_version != EV_CURRENT) { - pr_debug("Unknown ELF version.\n"); - return false; - } - - if (ehdr->e_phoff > 0 && ehdr->e_phnum > 0) { - size_t phdr_size; - - /* -* e_phnum is at most 65535 so calculating the size of the -* program header cannot overflow. -*/ - phdr_size = sizeof(struct elf_phdr) * ehdr->e_phnum; - - /* Sanity check the program header table location. */ - if (ehdr->e_phoff + phdr_size < ehdr->e_phoff) { - pr_debug("Program headers at invalid location.\n"); - return false; - } else if (ehdr->e_phoff + phdr_size > buf_len) { - pr_debug("Program headers truncated.\n"); - return false; - } - } - - if (ehdr->e_shoff > 0 && ehdr->e_shnum > 0) { - size_t shdr_size; - - /* -* e_shnum is at most 655
[PATCH using git format-patch -C] kexec: add generic support for elf kernel images
From: Sven Schnelle Signed-off-by: Sven Schnelle --- patch generated with 'git format-patch -C' in order to see the modifications done in kexec_elf.c in addition to copying it from arch/powerpc/kernel/kexec_elf_64.c arch/Kconfig | 3 + arch/powerpc/Kconfig | 1 + arch/powerpc/kernel/kexec_elf_64.c | 547 + include/linux/kexec.h | 35 ++ kernel/Makefile| 1 + .../kernel/kexec_elf_64.c => kernel/kexec_elf.c| 264 +++--- 6 files changed, 118 insertions(+), 733 deletions(-) copy arch/powerpc/kernel/kexec_elf_64.c => kernel/kexec_elf.c (64%) diff --git a/arch/Kconfig b/arch/Kconfig index 3368786a..c7c75fbc0b79 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -18,6 +18,9 @@ config KEXEC_CORE select CRASH_CORE bool +config KEXEC_ELF + bool + config HAVE_IMA_KEXEC bool diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 2d0be82c3061..447b6e3ad999 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -501,6 +501,7 @@ config KEXEC_FILE select KEXEC_CORE select HAVE_IMA_KEXEC select BUILD_BIN2C + select KEXEC_ELF depends on PPC64 depends on CRYPTO=y depends on CRYPTO_SHA256=y diff --git a/arch/powerpc/kernel/kexec_elf_64.c b/arch/powerpc/kernel/kexec_elf_64.c index ba4f18a43ee8..d062c5991722 100644 --- a/arch/powerpc/kernel/kexec_elf_64.c +++ b/arch/powerpc/kernel/kexec_elf_64.c @@ -31,539 +31,7 @@ #include #include -#define PURGATORY_STACK_SIZE (16 * 1024) - -#define elf_addr_to_cpuelf64_to_cpu - -#ifndef Elf_Rel -#define Elf_RelElf64_Rel -#endif /* Elf_Rel */ - -struct elf_info { - /* -* Where the ELF binary contents are kept. -* Memory managed by the user of the struct. -*/ - const char *buffer; - - const struct elfhdr *ehdr; - const struct elf_phdr *proghdrs; - struct elf_shdr *sechdrs; -}; - -static inline bool elf_is_elf_file(const struct elfhdr *ehdr) -{ - return memcmp(ehdr->e_ident, ELFMAG, SELFMAG) == 0; -} - -static uint64_t elf64_to_cpu(const struct elfhdr *ehdr, uint64_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le64_to_cpu(value); - else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be64_to_cpu(value); - - return value; -} - -static uint16_t elf16_to_cpu(const struct elfhdr *ehdr, uint16_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le16_to_cpu(value); - else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be16_to_cpu(value); - - return value; -} - -static uint32_t elf32_to_cpu(const struct elfhdr *ehdr, uint32_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - value = le32_to_cpu(value); - else if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - value = be32_to_cpu(value); - - return value; -} - -/** - * elf_is_ehdr_sane - check that it is safe to use the ELF header - * @buf_len: size of the buffer in which the ELF file is loaded. - */ -static bool elf_is_ehdr_sane(const struct elfhdr *ehdr, size_t buf_len) -{ - if (ehdr->e_phnum > 0 && ehdr->e_phentsize != sizeof(struct elf_phdr)) { - pr_debug("Bad program header size.\n"); - return false; - } else if (ehdr->e_shnum > 0 && - ehdr->e_shentsize != sizeof(struct elf_shdr)) { - pr_debug("Bad section header size.\n"); - return false; - } else if (ehdr->e_ident[EI_VERSION] != EV_CURRENT || - ehdr->e_version != EV_CURRENT) { - pr_debug("Unknown ELF version.\n"); - return false; - } - - if (ehdr->e_phoff > 0 && ehdr->e_phnum > 0) { - size_t phdr_size; - - /* -* e_phnum is at most 65535 so calculating the size of the -* program header cannot overflow. -*/ - phdr_size = sizeof(struct elf_phdr) * ehdr->e_phnum; - - /* Sanity check the program header table location. */ - if (ehdr->e_phoff + phdr_size < ehdr->e_phoff) { - pr_debug("Program headers at invalid location.\n"); - return false; - } else if (ehdr->e_phoff + phdr_size > buf_len) { - pr_debug("Program headers truncated.\n"); - return false; - } - } - - if (ehdr->e_shoff > 0 && ehdr->e_shnum > 0) { - size_t shdr_size; - - /* -* e_shnum is at most 65536 so calculating -* the size of the section header cannot overflow. -*/ - shdr_size = sizeof(struc
Re: [PATCH RFC] generic ELF support for kexec
Hi Sven, On 06/25/2019 06:54 PM, Sven Schnelle wrote: Hi List, i recently started working on kexec for PA-RISC. While doing so, i figured that powerpc already has support for reading ELF images inside of the Kernel. My first attempt was to steal the source code and modify it for PA-RISC, but it turned out that i didn't had to change much. Only ARM specific stuff like fdt blob fetching had to be removed. So instead of duplicating the code, i thought about moving the ELF stuff to the core kexec code, and exposing several function to use that code from the arch specific code. I'm attaching the patch to this Mail. What do you think about that change? s390 also uses ELF files, and (maybe?) could also switch to this implementation. But i don't know anything about S/390 and don't have one in my basement. So i'll leave s390 to the IBM folks. I haven't really tested PowerPC yet. Can anyone give me a helping hand what would be a good target to test this code in QEMU? Or even better, test this code on real Hardware? Where did you start from ? Your patch doesn't apply on latest powerpc/merge branch (https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git): [root@localhost linux-powerpc]# git am -3 /root/Downloads/RFC-generic-ELF-support-for-kexec.patch Applying: generic ELF support for kexec Using index info to reconstruct a base tree... M arch/powerpc/kernel/kexec_elf_64.c M kernel/Makefile Falling back to patching base and 3-way merge... Auto-merging kernel/Makefile Auto-merging arch/powerpc/kernel/kexec_elf_64.c CONFLICT (content): Merge conflict in arch/powerpc/kernel/kexec_elf_64.c error: Failed to merge in the changes. Patch failed at 0001 generic ELF support for kexec Neither does it apply on 5.2-rc6 Looks like it cleanly applies on 5.1 Could you generate your patch using 'git format-patch -M -C ' ? It would be a lot easier to see the real changes: arch/Kconfig | 3 + arch/powerpc/Kconfig | 1 + arch/powerpc/kernel/kexec_elf_64.c | 547 + include/linux/kexec.h | 35 ++ kernel/Makefile| 1 + .../kexec_elf_64.c => kernel/kexec_file_elf.c | 192 ++-- 6 files changed, 96 insertions(+), 683 deletions(-) copy arch/powerpc/kernel/kexec_elf_64.c => kernel/kexec_file_elf.c (77%) Thanks Christophe If that change is acceptable i would finish the patch and submit it. I think best would be to push this change through Helge's parisc tree, so we don't have any dependencies to sort out. Regards, Sven [PATCH] kexec: add generic support for elf kernel images Signed-off-by: Sven Schnelle --- arch/Kconfig | 3 + arch/powerpc/Kconfig | 1 + arch/powerpc/kernel/kexec_elf_64.c | 547 +-- include/linux/kexec.h | 35 ++ kernel/Makefile| 1 + kernel/kexec_file_elf.c| 574 + 6 files changed, 619 insertions(+), 542 deletions(-) create mode 100644 kernel/kexec_file_elf.c diff --git a/arch/Kconfig b/arch/Kconfig index c47b328eada0..de7520100136 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -18,6 +18,9 @@ config KEXEC_CORE select CRASH_CORE bool +config KEXEC_FILE_ELF + bool + config HAVE_IMA_KEXEC bool diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 8c1c636308c8..48241260b6ae 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -502,6 +502,7 @@ config KEXEC_FILE select KEXEC_CORE select HAVE_IMA_KEXEC select BUILD_BIN2C + select KEXEC_FILE_ELF depends on PPC64 depends on CRYPTO=y depends on CRYPTO_SHA256=y diff --git a/arch/powerpc/kernel/kexec_elf_64.c b/arch/powerpc/kernel/kexec_elf_64.c index ba4f18a43ee8..0059e36913e9 100644 --- a/arch/powerpc/kernel/kexec_elf_64.c +++ b/arch/powerpc/kernel/kexec_elf_64.c @@ -21,8 +21,6 @@ * GNU General Public License for more details. */ -#define pr_fmt(fmt) "kexec_elf: " fmt - #include #include #include @@ -31,540 +29,6 @@ #include #include -#define PURGATORY_STACK_SIZE (16 * 1024) - -#define elf_addr_to_cpuelf64_to_cpu - -#ifndef Elf_Rel -#define Elf_RelElf64_Rel -#endif /* Elf_Rel */ - -struct elf_info { - /* -* Where the ELF binary contents are kept. -* Memory managed by the user of the struct. -*/ - const char *buffer; - - const struct elfhdr *ehdr; - const struct elf_phdr *proghdrs; - struct elf_shdr *sechdrs; -}; - -static inline bool elf_is_elf_file(const struct elfhdr *ehdr) -{ - return memcmp(ehdr->e_ident, ELFMAG, SELFMAG) == 0; -} - -static uint64_t elf64_to_cpu(const struct elfhdr *ehdr, uint64_t value) -{ - if (ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - va