Re: [PATCH v6 (proposal)] powerpc/cpu: enable nr_cpus for crash kernel

2024-01-25 Thread Christophe Leroy
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

2023-10-13 Thread Christophe Leroy


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

2023-09-28 Thread Christophe Leroy


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]

2022-05-29 Thread Christophe Leroy


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

2022-03-11 Thread Christophe Leroy



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

2022-03-11 Thread Christophe Leroy



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

2021-12-11 Thread Christophe Leroy


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

2021-12-11 Thread Christophe Leroy


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

2021-12-10 Thread Christophe Leroy


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

2021-10-19 Thread Christophe Leroy

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

2021-09-15 Thread Christophe Leroy



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

2021-09-14 Thread Christophe Leroy



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

2021-09-09 Thread Christophe Leroy




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

2021-09-09 Thread Christophe Leroy




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

2021-09-09 Thread Christophe Leroy




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

2021-08-02 Thread Christophe Leroy



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

2020-06-26 Thread Christophe Leroy



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

2020-06-26 Thread Christophe Leroy



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

2020-02-27 Thread Christophe Leroy



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

2020-02-27 Thread Christophe Leroy



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

2019-07-10 Thread Christophe Leroy



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

2019-07-10 Thread Christophe Leroy



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

2019-07-10 Thread Christophe Leroy



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

2019-07-10 Thread Christophe Leroy

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

2019-07-08 Thread Christophe Leroy

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

2019-07-08 Thread Christophe Leroy



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

2019-07-08 Thread Christophe Leroy
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

2019-07-02 Thread Christophe Leroy

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