Message ID | 159319828304.16351.6990340111766605842.stgit@hbathini.in.ibm.com (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
Series | ppc64: enable kdump support for kexec_file_load syscall | expand |
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 <hbathini@linux.ibm.com> > --- > 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
Hi Hari, If in [4/11], get_exclude_memory_ranges() turns out to be unnecessary ,then this patch is abundant either. As my understanding, memblock has already helped to achieved the purpose that get_exclude_memory_ranges() wants. Thanks, Pingfan On 06/27/2020 03:04 AM, Hari Bathini wrote: > 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 <hbathini@linux.ibm.com> > --- > 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); > + > +/* arch_kexec_add_buffer calls this when it is ready */ > +extern int __kexec_add_buffer(struct kexec_buf *kbuf); > + > 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) > { > > > _______________________________________________ > kexec mailing list > kexec@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/kexec >
On 28/06/20 7:58 am, piliu wrote: > Hi Hari, > > If in [4/11], get_exclude_memory_ranges() turns out to be unnecessary > ,then this patch is abundant either. As my understanding, memblock has > already helped to achieved the purpose that get_exclude_memory_ranges() > wants. As mentioned in the other patch, there is a need for @exclude_ranges as crashkernel region is likely to have an overlap with regions like opal, rtas.. But yeah.. the weak function should have been kexec_locate_mem_hole() instead of kexec_add_buffer(). Will take care of that in v2. > On 06/27/2020 03:04 AM, Hari Bathini wrote: >> 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 <hbathini@linux.ibm.com> Thanks Hari
Hi Hari, is there any good reason to add two more functions with a very similar name to an existing function? AFAICS all you need is a way to call a PPC64-specific function from within kexec_add_buffer (PATCH 4/11), so you could add something like this: int __weak arch_kexec_locate_mem_hole(struct kexec_buf *kbuf) { return 0; } Call this function from kexec_add_buffer where appropriate and then override it for PPC64 (it roughly corresponds to your kexec_locate_mem_hole_ppc64() from PATCH 4/11). FWIW it would make it easier for me to follow the resulting code. Petr T On Sat, 27 Jun 2020 00:34:43 +0530 Hari Bathini <hbathini@linux.ibm.com> wrote: > 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 <hbathini@linux.ibm.com> > --- > 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); > + > +/* arch_kexec_add_buffer calls this when it is ready */ > +extern int __kexec_add_buffer(struct kexec_buf *kbuf); > + > 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) > { >
Hi Petr, On 29/06/20 5:09 pm, Petr Tesarik wrote: > Hi Hari, > > is there any good reason to add two more functions with a very similar > name to an existing function? AFAICS all you need is a way to call a > PPC64-specific function from within kexec_add_buffer (PATCH 4/11), so > you could add something like this: > > int __weak arch_kexec_locate_mem_hole(struct kexec_buf *kbuf) > { > return 0; > } > > Call this function from kexec_add_buffer where appropriate and then > override it for PPC64 (it roughly corresponds to your > kexec_locate_mem_hole_ppc64() from PATCH 4/11). > > FWIW it would make it easier for me to follow the resulting code. Right, Petr. I was trying out a few things before I ended up with what I sent here. Bu yeah.. I did realize arch_kexec_locate_mem_hole() would have been better after sending out v1. Will take care of that in v2. Thanks Hari
On 06/29/20 at 05:26pm, Hari Bathini wrote: > Hi Petr, > > On 29/06/20 5:09 pm, Petr Tesarik wrote: > > Hi Hari, > > > > is there any good reason to add two more functions with a very similar > > name to an existing function? AFAICS all you need is a way to call a > > PPC64-specific function from within kexec_add_buffer (PATCH 4/11), so > > you could add something like this: > > > > int __weak arch_kexec_locate_mem_hole(struct kexec_buf *kbuf) > > { > > return 0; > > } > > > > Call this function from kexec_add_buffer where appropriate and then > > override it for PPC64 (it roughly corresponds to your > > kexec_locate_mem_hole_ppc64() from PATCH 4/11). > > > > FWIW it would make it easier for me to follow the resulting code. > > Right, Petr. > > I was trying out a few things before I ended up with what I sent here. > Bu yeah.. I did realize arch_kexec_locate_mem_hole() would have been better > after sending out v1. Will take care of that in v2. Another way is use arch private function to locate mem hole, then set kbuf->mem, and then call kexec_add_buf, it will skip the common locate hole function. But other than that I have some confusion about those excluded ranges. Replied a question to patch 4. Thanks Dave
On 01/07/20 1:16 pm, Dave Young wrote: > On 06/29/20 at 05:26pm, Hari Bathini wrote: >> Hi Petr, >> >> On 29/06/20 5:09 pm, Petr Tesarik wrote: >>> Hi Hari, >>> >>> is there any good reason to add two more functions with a very similar >>> name to an existing function? AFAICS all you need is a way to call a >>> PPC64-specific function from within kexec_add_buffer (PATCH 4/11), so >>> you could add something like this: >>> >>> int __weak arch_kexec_locate_mem_hole(struct kexec_buf *kbuf) >>> { >>> return 0; >>> } >>> >>> Call this function from kexec_add_buffer where appropriate and then >>> override it for PPC64 (it roughly corresponds to your >>> kexec_locate_mem_hole_ppc64() from PATCH 4/11). >>> >>> FWIW it would make it easier for me to follow the resulting code. >> >> Right, Petr. >> >> I was trying out a few things before I ended up with what I sent here. >> Bu yeah.. I did realize arch_kexec_locate_mem_hole() would have been better >> after sending out v1. Will take care of that in v2. > > Another way is use arch private function to locate mem hole, then set > kbuf->mem, and then call kexec_add_buf, it will skip the common locate > hole function. Dave, I did think about it. But there are a couple of places this can get tricky. One is ima_add_kexec_buffer() and the other is kexec_elf_load(). These call sites could be updated to set kbuf->mem before kexec_add_buffer(). But the current approach seemed like the better option for it creates a single point of control in setting up segment buffers and also, makes adding any new segments simpler, arch-specific segments or otherwise. Thanks Hari
On 07/02/20 at 12:01am, Hari Bathini wrote: > > > On 01/07/20 1:16 pm, Dave Young wrote: > > On 06/29/20 at 05:26pm, Hari Bathini wrote: > >> Hi Petr, > >> > >> On 29/06/20 5:09 pm, Petr Tesarik wrote: > >>> Hi Hari, > >>> > >>> is there any good reason to add two more functions with a very similar > >>> name to an existing function? AFAICS all you need is a way to call a > >>> PPC64-specific function from within kexec_add_buffer (PATCH 4/11), so > >>> you could add something like this: > >>> > >>> int __weak arch_kexec_locate_mem_hole(struct kexec_buf *kbuf) > >>> { > >>> return 0; > >>> } > >>> > >>> Call this function from kexec_add_buffer where appropriate and then > >>> override it for PPC64 (it roughly corresponds to your > >>> kexec_locate_mem_hole_ppc64() from PATCH 4/11). > >>> > >>> FWIW it would make it easier for me to follow the resulting code. > >> > >> Right, Petr. > >> > >> I was trying out a few things before I ended up with what I sent here. > >> Bu yeah.. I did realize arch_kexec_locate_mem_hole() would have been better > >> after sending out v1. Will take care of that in v2. > > > > Another way is use arch private function to locate mem hole, then set > > kbuf->mem, and then call kexec_add_buf, it will skip the common locate > > hole function. > > Dave, I did think about it. But there are a couple of places this can get > tricky. One is ima_add_kexec_buffer() and the other is kexec_elf_load(). > These call sites could be updated to set kbuf->mem before kexec_add_buffer(). > But the current approach seemed like the better option for it creates a > single point of control in setting up segment buffers and also, makes adding > any new segments simpler, arch-specific segments or otherwise. > Ok, thanks for the explanation.
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); + +/* arch_kexec_add_buffer calls this when it is ready */ +extern int __kexec_add_buffer(struct kexec_buf *kbuf); + 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) {
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 <hbathini@linux.ibm.com> --- include/linux/kexec.h | 5 +++++ kernel/kexec_file.c | 37 +++++++++++++++++++++++++++++++++---- 2 files changed, 38 insertions(+), 4 deletions(-)