From: Michael Ellerman <mpe@ellerman.id.au>
To: Hari Bathini <hbathini@linux.ibm.com>,
Andrew Morton <akpm@linux-foundation.org>
Cc: Pingfan Liu <piliu@redhat.com>,
Kexec-ml <kexec@lists.infradead.org>,
Mimi Zohar <zohar@linux.ibm.com>,
Nayna Jain <nayna@linux.ibm.com>, Petr Tesarik <ptesarik@suse.cz>,
Mahesh J Salgaonkar <mahesh@linux.ibm.com>,
Sourabh Jain <sourabhjain@linux.ibm.com>,
lkml <linux-kernel@vger.kernel.org>,
linuxppc-dev <linuxppc-dev@ozlabs.org>,
Eric Biederman <ebiederm@xmission.com>,
Thiago Jung Bauermann <bauerman@linux.ibm.com>,
Dave Young <dyoung@redhat.com>, Vivek Goyal <vgoyal@redhat.com>
Subject: Re: [RESEND PATCH v5 08/11] ppc64/kexec_file: setup backup region for kdump kernel
Date: Wed, 29 Jul 2020 00:11:43 +1000 [thread overview]
Message-ID: <87y2n36868.fsf@mpe.ellerman.id.au> (raw)
In-Reply-To: <159579235754.5790.5203600072984600891.stgit@hbathini>
Hari Bathini <hbathini@linux.ibm.com> writes:
> diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
> index a5c1442590b2..88408b17a7f6 100644
> --- a/arch/powerpc/kexec/file_load_64.c
> +++ b/arch/powerpc/kexec/file_load_64.c
> @@ -697,6 +699,69 @@ static int update_usable_mem_fdt(void *fdt, struct crash_mem *usable_mem)
> return ret;
> }
>
> +/**
> + * load_backup_segment - Locate a memory hole to place the backup region.
> + * @image: Kexec image.
> + * @kbuf: Buffer contents and memory parameters.
> + *
> + * Returns 0 on success, negative errno on error.
> + */
> +static int load_backup_segment(struct kimage *image, struct kexec_buf *kbuf)
> +{
> + void *buf;
> + int ret;
> +
> + /* Setup a segment for backup region */
> + buf = vzalloc(BACKUP_SRC_SIZE);
This worried me initially, because we can't copy from physically
discontiguous pages in real mode.
But as you explained this buffer is not used for copying.
I think if you move the large comment below up here, it would be
clearer.
> diff --git a/arch/powerpc/purgatory/trampoline_64.S b/arch/powerpc/purgatory/trampoline_64.S
> index 464af8e8a4cb..d4b52961f592 100644
> --- a/arch/powerpc/purgatory/trampoline_64.S
> +++ b/arch/powerpc/purgatory/trampoline_64.S
> @@ -43,14 +44,38 @@ master:
> mr %r17,%r3 /* save cpu id to r17 */
> mr %r15,%r4 /* save physical address in reg15 */
>
> + bl 0f /* Work out where we're running */
> +0: mflr %r18
I know you just moved it, but this should use:
bcl 20, 31, $+4
mflr %r18
Which is a special form of branch and link that doesn't unbalance the
link stack in the chip.
> + /*
> + * Copy BACKUP_SRC_SIZE bytes from BACKUP_SRC_START to
> + * backup_start 8 bytes at a time.
> + *
> + * Use r3 = dest, r4 = src, r5 = size, r6 = count
> + */
> + ld %r3,(backup_start - 0b)(%r18)
> + cmpdi %cr0,%r3,0
I prefer spaces or tabs between arguments, eg:
cmpdi %cr0, %r3, 0
> + beq 80f /* skip if there is no backup region */
Local labels will make this clearer I think. eg:
beq .Lskip_copy
> + lis %r5,BACKUP_SRC_SIZE@h
> + ori %r5,%r5,BACKUP_SRC_SIZE@l
> + cmpdi %cr0,%r5,0
> + beq 80f /* skip if copy size is zero */
> + lis %r4,BACKUP_SRC_START@h
> + ori %r4,%r4,BACKUP_SRC_START@l
> + li %r6,0
> +70:
.Lcopy_loop:
> + ldx %r0,%r6,%r4
> + stdx %r0,%r6,%r3
> + addi %r6,%r6,8
> + cmpld %cr0,%r6,%r5
> + blt 70b
blt .Lcopy_loop
> +
.Lskip_copy:
> +80:
> or %r3,%r3,%r3 /* ok now to high priority, lets boot */
> lis %r6,0x1
> mtctr %r6 /* delay a bit for slaves to catch up */
> bdnz . /* before we overwrite 0-100 again */
cheers
next prev parent reply other threads:[~2020-07-28 14:18 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-26 19:36 [RESEND PATCH v5 00/11] ppc64: enable kdump support for kexec_file_load syscall Hari Bathini
2020-07-26 19:36 ` [RESEND PATCH v5 01/11] kexec_file: allow archs to handle special regions while locating memory hole Hari Bathini
2020-07-26 19:36 ` [RESEND PATCH v5 02/11] powerpc/kexec_file: mark PPC64 specific code Hari Bathini
2020-07-26 19:37 ` [RESEND PATCH v5 03/11] powerpc/kexec_file: add helper functions for getting memory ranges Hari Bathini
2020-07-28 12:58 ` Michael Ellerman
2020-07-26 19:38 ` [RESEND PATCH v5 04/11] ppc64/kexec_file: avoid stomping memory used by special regions Hari Bathini
2020-07-26 19:38 ` [RESEND PATCH v5 05/11] powerpc/drmem: make lmb walk a bit more flexible Hari Bathini
2020-07-26 19:38 ` [RESEND PATCH v5 06/11] ppc64/kexec_file: restrict memory usage of kdump kernel Hari Bathini
2020-07-28 2:10 ` Thiago Jung Bauermann
2020-07-28 13:44 ` Michael Ellerman
2020-07-28 19:34 ` Hari Bathini
2020-07-26 19:39 ` [RESEND PATCH v5 07/11] ppc64/kexec_file: enable early kernel's OPAL calls Hari Bathini
2020-07-28 2:17 ` Thiago Jung Bauermann
2020-07-28 13:46 ` Michael Ellerman
2020-07-28 19:24 ` Hari Bathini
2020-07-29 1:15 ` Michael Ellerman
2020-07-26 19:39 ` [RESEND PATCH v5 08/11] ppc64/kexec_file: setup backup region for kdump kernel Hari Bathini
2020-07-28 2:37 ` Thiago Jung Bauermann
2020-07-28 14:11 ` Michael Ellerman [this message]
2020-07-26 19:39 ` [RESEND PATCH v5 09/11] ppc64/kexec_file: prepare elfcore header for crashing kernel Hari Bathini
2020-07-26 19:40 ` [RESEND PATCH v5 10/11] ppc64/kexec_file: add appropriate regions for memory reserve map Hari Bathini
2020-07-26 19:40 ` [RESEND PATCH v5 11/11] ppc64/kexec_file: fix kexec load failure with lack of memory hole Hari Bathini
2020-07-28 2:32 ` [RESEND PATCH v5 00/11] ppc64: enable kdump support for kexec_file_load syscall piliu
2020-07-30 6:05 ` Hari Bathini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87y2n36868.fsf@mpe.ellerman.id.au \
--to=mpe@ellerman.id.au \
--cc=akpm@linux-foundation.org \
--cc=bauerman@linux.ibm.com \
--cc=dyoung@redhat.com \
--cc=ebiederm@xmission.com \
--cc=hbathini@linux.ibm.com \
--cc=kexec@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@ozlabs.org \
--cc=mahesh@linux.ibm.com \
--cc=nayna@linux.ibm.com \
--cc=piliu@redhat.com \
--cc=ptesarik@suse.cz \
--cc=sourabhjain@linux.ibm.com \
--cc=vgoyal@redhat.com \
--cc=zohar@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).