Linux MIPS Architecture development
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox