All of lore.kernel.org
 help / color / mirror / Atom feed
From: James Hogan <jhogan@kernel.org>
To: Huacai Chen <chenhc@lemote.com>
Cc: Ralf Baechle <ralf@linux-mips.org>,
	"Steven J . Hill" <Steven.Hill@cavium.com>,
	linux-mips@linux-mips.org, Fuxin Zhang <zhangfx@lemote.com>,
	Zhangjin Wu <wuzhangjin@gmail.com>
Subject: Re: [PATCH V2 09/12] MIPS: Loongson: Add kexec/kdump support
Date: Mon, 19 Feb 2018 23:54:44 +0000	[thread overview]
Message-ID: <20180219235444.GD6245@saruman> (raw)
In-Reply-To: <1517023336-17575-3-git-send-email-chenhc@lemote.com>

[-- Attachment #1: Type: text/plain, Size: 7845 bytes --]

On Sat, Jan 27, 2018 at 11:22:16AM +0800, Huacai Chen wrote:
> Signed-off-by: Huacai Chen <chenhc@lemote.com>

Please add a commit message.

Worth Cc'ing kexec maintainers?

> ---
>  arch/mips/kernel/relocate_kernel.S    |  30 +++++++++
>  arch/mips/loongson64/common/env.c     |   8 +++
>  arch/mips/loongson64/common/reset.c   | 119 ++++++++++++++++++++++++++++++++++
>  arch/mips/loongson64/loongson-3/smp.c |   4 ++
>  4 files changed, 161 insertions(+)
> 
> diff --git a/arch/mips/kernel/relocate_kernel.S b/arch/mips/kernel/relocate_kernel.S
> index c6bbf21..e73edc7 100644
> --- a/arch/mips/kernel/relocate_kernel.S
> +++ b/arch/mips/kernel/relocate_kernel.S
> @@ -135,6 +135,36 @@ LEAF(kexec_smp_wait)
>  #else
>  	sync
>  #endif
> +
> +#ifdef CONFIG_CPU_LOONGSON3
> +	/* s0:prid s1:initfn */
> +	/* t0:base t1:cpuid t2:node t3:core t9:count */
> +	mfc0  t1, $15, 1

Can you use CP0_EBASE from mipsregs.h?

> +	andi  t1, 0x3ff

MIPS_EBASE_CPUNUM

> +	dli   t0, 0x900000003ff01000

Whats that?

> +	andi  t3, t1, 0x3
> +	sll   t3, 8               /* get core id */
> +	or    t0, t0, t3

Do you have the INS instruction on loongson? Does it make sense to do
this, even if only for readability?
	dins	t0, t1, 8, 2

> +	andi  t2, t1, 0xc
> +	dsll  t2, 42              /* get node id */
> +	or    t0, t0, t2

Does it make sense to do this, especially for readability (44 vs 42)?
	dext	t2, t1, 2, 2
	dins	t0, t2, 44, 2

> +	mfc0  s0, $15, 0

CP0_PRID

> +	andi  s0, s0, 0xf
> +	blt   s0, 0x6, 1f         /* Loongson-3A1000 */
> +	bgt   s0, 0x7, 1f         /* Loongson-3A2000/3A3000 */

> +	dsrl  t2, 30              /* Loongson-3B1000/3B1500 need bit15:14 */
> +	or    t0, t0, t2

maybe:
	dins	t0, t2, 14, 2

> +1:	li    t9, 0x100           /* wait for init loop */
> +2:	addiu t9, -1              /* limit mailbox access */
> +	bnez  t9, 2b
> +	ld    s1, 0x20(t0)        /* get PC via mailbox */
> +	beqz  s1, 1b
> +	ld    sp, 0x28(t0)        /* get SP via mailbox */
> +	ld    gp, 0x30(t0)        /* get GP via mailbox */
> +	ld    a1, 0x38(t0)
> +	jr    s1                  /* jump to initial PC */
> +#endif
> +
>  	j		s1
>  	END(kexec_smp_wait)
>  #endif
> diff --git a/arch/mips/loongson64/common/env.c b/arch/mips/loongson64/common/env.c
> index 2928ac5..990a2d69 100644
> --- a/arch/mips/loongson64/common/env.c
> +++ b/arch/mips/loongson64/common/env.c
> @@ -72,6 +72,7 @@ void __init prom_init_env(void)
>  
>  	pr_info("memsize=%u, highmemsize=%u\n", memsize, highmemsize);
>  #else
> +	int i;
>  	struct boot_params *boot_p;
>  	struct loongson_params *loongson_p;
>  	struct system_loongson *esys;
> @@ -149,6 +150,13 @@ void __init prom_init_env(void)
>  	loongson_sysconf.nr_cpus = ecpu->nr_cpus;
>  	loongson_sysconf.boot_cpu_id = ecpu->cpu_startup_core_id;
>  	loongson_sysconf.reserved_cpus_mask = ecpu->reserved_cores_mask;
> +#ifdef CONFIG_KEXEC
> +	loongson_sysconf.boot_cpu_id = get_ebase_cpunum();
> +	for (i = 0; i < loongson_sysconf.boot_cpu_id; i++)
> +		loongson_sysconf.reserved_cpus_mask |= (1<<i);

maybe this?
	loongson_sysconf.reserved_cpus_mask |=
		(1 << loongson_sysconf.boot_cpu_id) - 1;

> +	pr_info("Boot CPU ID is being fixed from %d to %d\n",
> +		ecpu->cpu_startup_core_id, loongson_sysconf.boot_cpu_id);
> +#endif
>  	if (ecpu->nr_cpus > NR_CPUS || ecpu->nr_cpus == 0)
>  		loongson_sysconf.nr_cpus = NR_CPUS;
>  	loongson_sysconf.nr_nodes = (loongson_sysconf.nr_cpus +
> diff --git a/arch/mips/loongson64/common/reset.c b/arch/mips/loongson64/common/reset.c
> index a60715e..5f65a4e 100644
> --- a/arch/mips/loongson64/common/reset.c
> +++ b/arch/mips/loongson64/common/reset.c
> @@ -11,9 +11,14 @@
>   */
>  #include <linux/init.h>
>  #include <linux/pm.h>
> +#include <linux/cpu.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/kexec.h>
>  
>  #include <asm/idle.h>
>  #include <asm/reboot.h>
> +#include <asm/bootinfo.h>

alphabetical ordering wherever possible please.

>  
>  #include <loongson.h>
>  #include <boot_param.h>
> @@ -80,12 +85,126 @@ static void loongson_halt(void)
>  	}
>  }
>  
> +#ifdef CONFIG_KEXEC
> +
> +/* 0X80000000~0X80200000 is safe */
> +#define MAX_ARGS	64
> +#define KEXEC_CTRL_CODE	0XFFFFFFFF80100000UL
> +#define KEXEC_ARGV_ADDR	0XFFFFFFFF80108000UL

lower case 0x please.

> +#define KEXEC_ARGV_SIZE	3060
> +#define KEXEC_ENVP_SIZE	4500
> +
> +void *kexec_argv;
> +void *kexec_envp;
> +extern const size_t relocate_new_kernel_size;

A couple of other architectures have moved this declaration to
asm/kexec.h.

> +
> +static int loongson_kexec_prepare(struct kimage *image)
> +{
> +	int i, argc = 0;
> +	unsigned int *argv;
> +	char *str, *ptr, *bootloader = "kexec";
> +
> +	/* argv at offset 0, argv[] at offset KEXEC_ARGV_SIZE/2 */
> +	argv = (unsigned int *)kexec_argv;

the cast is redundant as kexec_argv is void *.

> +	argv[argc++] = (unsigned int)(KEXEC_ARGV_ADDR + KEXEC_ARGV_SIZE/2);
> +
> +	for (i = 0; i < image->nr_segments; i++) {
> +		if (!strncmp(bootloader, (char *)image->segment[i].buf,
> +				strlen(bootloader))) {
> +			/*
> +			 * convert command line string to array
> +			 * of parameters (as bootloader does).
> +			 */
> +			int offt;
> +			memcpy(kexec_argv + KEXEC_ARGV_SIZE/2, image->segment[i].buf, KEXEC_ARGV_SIZE/2);
> +			str = (char *)kexec_argv + KEXEC_ARGV_SIZE/2;
> +			ptr = strchr(str, ' ');
> +
> +			while (ptr && (argc < MAX_ARGS)) {
> +				*ptr = '\0';
> +				if (ptr[1] != ' ') {
> +					offt = (int)(ptr - str + 1);
> +					argv[argc] = KEXEC_ARGV_ADDR + KEXEC_ARGV_SIZE/2 + offt;
> +					argc++;
> +				}
> +				ptr = strchr(ptr + 1, ' ');
> +			}
> +			break;
> +		}
> +	}
> +
> +	kexec_args[0] = argc;
> +	kexec_args[1] = fw_arg1;
> +	kexec_args[2] = fw_arg2;
> +	image->control_code_page = virt_to_page((void *)KEXEC_CTRL_CODE);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_SMP
> +static void kexec_smp_down(void *ignored)
> +{
> +	int cpu = smp_processor_id();
> +
> +	local_irq_disable();
> +	set_cpu_online(cpu, false);
> +	while (!atomic_read(&kexec_ready_to_reboot))
> +		cpu_relax();
> +
> +	asm volatile (
> +	"	sync					\n"
> +	"	synci	($0)				\n");

The intermediate assembly looks nicer, and you'd get fewer checkpatch
complaints, if you do something more like this:
	asm volatile ("sync\n\t"
		      "synci ($0)");

> +
> +	relocated_kexec_smp_wait(NULL);
> +}
> +#endif
> +
> +static void loongson_kexec_shutdown(void)
> +{
> +#ifdef CONFIG_SMP
> +	int cpu;
> +
> +	for_each_possible_cpu(cpu)
> +		if (!cpu_online(cpu))
> +			cpu_up(cpu); /* Everyone should go to reboot_code_buffer */

long line 

> +
> +	smp_call_function(kexec_smp_down, NULL, 0);
> +	smp_wmb();

This probably needs a comment to explain what smp_rmb() it pairs with,
or what writes need a barrier between them and why. It certainly isn't
clear from reading the code.

> +	while (num_online_cpus() > 1) {
> +		mdelay(1);
> +		cpu_relax();
> +	}
> +#endif
> +	memcpy((void *)fw_arg1, kexec_argv, KEXEC_ARGV_SIZE);
> +	memcpy((void *)fw_arg2, kexec_envp, KEXEC_ENVP_SIZE);
> +}

> diff --git a/arch/mips/loongson64/loongson-3/smp.c b/arch/mips/loongson64/loongson-3/smp.c
> index 470e9c1..1e21ac4 100644
> --- a/arch/mips/loongson64/loongson-3/smp.c
> +++ b/arch/mips/loongson64/loongson-3/smp.c
> @@ -387,6 +387,10 @@ static void __init loongson3_smp_setup(void)
>  	ipi_status0_regs_init();
>  	ipi_en0_regs_init();
>  	ipi_mailbox_buf_init();
> +
> +	for (i = 0; i < loongson_sysconf.nr_cpus; i++)
> +		loongson3_ipi_write64(0, (void *)(ipi_mailbox_buf[i]+0x0));

what does that do? Please consider adding a comment.

Cheers
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2018-02-19 23:55 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-27  3:12 [PATCH V2 00/12] MIPS: Loongson: new features and improvements Huacai Chen
2018-01-27  3:12 ` [PATCH V2 01/12] MIPS: Loongson: Add Loongson-3A R3.1 basic support Huacai Chen
2018-01-27  3:12 ` [PATCH V2 02/12] MIPS: Loongson64: Define and use some CP0 registers Huacai Chen
2018-02-15 11:36   ` James Hogan
2018-01-27  3:12 ` [PATCH V2 03/12] MIPS: Loongson-3: Enable Store Fill Buffer at runtime Huacai Chen
2018-02-15 12:43   ` James Hogan
2018-01-27  3:19 ` [PATCH V2 04/12] MIPS: c-r4k: Add r4k_blast_scache_node for Loongson-3 Huacai Chen
2018-02-19 22:19   ` James Hogan
2018-01-27  3:20 ` [PATCH V2 05/12] MIPS: Loongson fix name confict - MEM_RESERVED Huacai Chen
2018-01-27  3:21 ` [PATCH V2 06/12] MIPS: Ensure pmd_present() returns false after pmd_mknotpresent() Huacai Chen
2018-01-27  3:22 ` [PATCH V2 07/12] MIPS: Add __cpu_full_name[] to make CPU names more human-readable Huacai Chen
2018-01-27  3:22   ` [PATCH V2 08/12] MIPS: Align kernel load address to 64KB Huacai Chen
2018-02-19 23:07     ` James Hogan
2018-02-20 22:14       ` Maciej W. Rozycki
2018-02-20 22:14         ` Maciej W. Rozycki
2018-02-20 22:25         ` James Hogan
2018-02-20 22:25           ` James Hogan
2018-02-20 22:53           ` Maciej W. Rozycki
2018-02-20 22:53             ` Maciej W. Rozycki
2018-02-20 22:58             ` James Hogan
2018-02-20 22:58               ` James Hogan
2018-02-20 23:38               ` Maciej W. Rozycki
2018-02-20 23:38                 ` Maciej W. Rozycki
2018-02-21 11:13                 ` James Hogan
2018-02-21 11:13                   ` James Hogan
2018-02-26 12:41                   ` Maciej W. Rozycki
2018-02-26 12:41                     ` Maciej W. Rozycki
2018-01-27  3:22   ` [PATCH V2 09/12] MIPS: Loongson: Add kexec/kdump support Huacai Chen
2018-02-19 23:54     ` James Hogan [this message]
2018-01-27  3:22 ` [PATCH V2 10/12] MIPS: Loongson: Make CPUFreq usable for Loongson-3 Huacai Chen
2018-01-27  3:23   ` [PATCH V2 11/12] MIPS: Loongson-3: Fix CPU UART irq delivery problem Huacai Chen
2018-02-20 21:49     ` James Hogan
2018-01-27  3:23   ` [PATCH V2 12/12] MIPS: Loongson: Introduce and use WAR_LLSC_MB Huacai Chen
2018-02-20 22:21     ` James Hogan
2018-02-21 10:09       ` Maciej W. Rozycki
2018-02-21 10:09         ` Maciej W. Rozycki
2018-02-21 11:43         ` James Hogan
2018-02-20 21:42   ` [PATCH V2 10/12] MIPS: Loongson: Make CPUFreq usable for Loongson-3 James Hogan
2018-02-15 11:05 ` [PATCH V2 00/12] MIPS: Loongson: new features and improvements James Hogan
2018-02-28  2:23   ` Huacai Chen
2018-02-28 10:03     ` James Hogan
2018-03-01  2:35       ` Huacai Chen

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=20180219235444.GD6245@saruman \
    --to=jhogan@kernel.org \
    --cc=Steven.Hill@cavium.com \
    --cc=chenhc@lemote.com \
    --cc=linux-mips@linux-mips.org \
    --cc=ralf@linux-mips.org \
    --cc=wuzhangjin@gmail.com \
    --cc=zhangfx@lemote.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.