linux-kernel - Re: [RFC PATCH v5 2/9] fadump: Reserve the memory for firmware assisted dump.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 25 Nov 2011 10:02:13 +1100
From:	Paul Mackerras <paulus@...ba.org>
To:	Mahesh J Salgaonkar <mahesh@...ux.vnet.ibm.com>
Cc:	linuxppc-dev <linuxppc-dev@...abs.org>,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Amerigo Wang <amwang@...hat.com>,
	Kexec-ml <kexec@...ts.infradead.org>,
	Milton Miller <miltonm@....com>,
	Haren Myneni <hbabu@...ibm.com>,
	Randy Dunlap <rdunlap@...otime.net>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Ananth Narayan <ananth@...ibm.com>,
	Vivek Goyal <vgoyal@...hat.com>,
	Anton Blanchard <anton@...ba.org>
Subject: Re: [RFC PATCH v5 2/9] fadump: Reserve the memory for firmware
 assisted dump.

On Tue, Nov 15, 2011 at 08:43:43PM +0530, Mahesh J Salgaonkar wrote:
> From: Mahesh Salgaonkar <mahesh@...ux.vnet.ibm.com>
> 
> Reserve the memory during early boot to preserve CPU state data, HPTE region
> and RMR region data in case of kernel crash. At the time of crash, powerpc
> firmware will store CPU state data, HPTE region data and move RMR region
> data to the reserved memory area.

What is "RMR"?  I don't see anywhere that you explain this acronym.
Is it the same as the RMA (real mode area)?

> +config FA_DUMP
> +	bool "Firmware-assisted dump"

Is this new fadump infrastructure intended to supersede the existing
phyp dump code?  Does it use the same phyp interfaces as phyp dump?
If so, you should probably remove the phyp dump code and config option
as the final patch in your series.

> +/*
> + * The RMR region will be saved for later dumping when kernel crashes.
> + * Set this to RMO size.
> + */
> +#define RMR_START	0x0
> +#define RMR_END		(ppc64_rma_size)

An explanation of "RMR" here, and what the distinction (if any)
between RMR and RMA/RMO is, would help future readers.

> +	sections = of_get_flat_dt_prop(node, "ibm,configure-kernel-dump-sizes",
> +					&size);
> +
> +	if (!sections)
> +		return 0;
> +
> +	num_sections = size / sizeof(struct dump_section);
> +
> +	for (i = 0; i < num_sections; i++) {
> +		switch (sections[i].dump_section) {
> +		case FADUMP_CPU_STATE_DATA:
> +			fw_dump.cpu_state_data_size = sections[i].section_size;
> +			break;
> +		case FADUMP_HPTE_REGION:
> +			fw_dump.hpte_region_size = sections[i].section_size;
> +			break;

It's generally better to use of_read_number() or of_read_ulong() to
parse OF properties, rather than using a structure like this.

> +	/* divide by 20 to get 5% of value */
> +	size = memblock_end_of_DRAM();
> +	do_div(size, 20);

You could just say size = memblock_end_of_DRAM() / 20 here; no need to
use do_div, since we won't be using this code on 32-bit platforms.

> +	if (!fw_dump.fadump_supported) {
> +		printk(KERN_ERR "Firmware-assisted dump is not supported on"
> +				" this hardware\n");

This shouldn't be KERN_ERR; it's not an error to boot a kernel with
fadump configured in on a machine that doesn't have firmware fadump
support.  I don't think we really need any message, but if we have one
it should be KERN_INFO at most.

> +/* Look for fadump= cmdline option. */
> +static int __init early_fadump_param(char *p)
> +{
> +	if (!p)
> +		return 1;
> +
> +	if (p[0] == '1')
> +		fw_dump.fadump_enabled = 1;
> +	else if (p[0] == '0')
> +		fw_dump.fadump_enabled = 0;

I think it's usual to allow "on" and "off" as values for this kind of
option.  There might be a handy little helper function to parse this
sort of thing (but if there is I don't know what it is called).

Paul.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/