* [PATCH] fadump: fix endianess issues in firmware assisted dump handling
@ 2014-09-03 12:29 Hari Bathini
2014-09-05 5:55 ` Mahesh Jagannath Salgaonkar
2014-09-25 1:40 ` Michael Ellerman
0 siblings, 2 replies; 3+ messages in thread
From: Hari Bathini @ 2014-09-03 12:29 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Mahesh J Salgaonkar
Firmware-assisted dump (fadump) kernel code is not LE compliant. The
below patch tries to fix this issue. Tested this patch with upstream
kernel. Did some sanity testing for the LE fadump vmcore generated.
Below output shows crash tool successfully opening LE fadump vmcore.
# crash $vmlinux vmcore
crash 7.0.5
Copyright (C) 2002-2014 Red Hat, Inc.
Copyright (C) 2004, 2005, 2006, 2010 IBM Corporation
Copyright (C) 1999-2006 Hewlett-Packard Co
Copyright (C) 2005, 2006, 2011, 2012 Fujitsu Limited
Copyright (C) 2006, 2007 VA Linux Systems Japan K.K.
Copyright (C) 2005, 2011 NEC Corporation
Copyright (C) 1999, 2002, 2007 Silicon Graphics, Inc.
Copyright (C) 1999, 2000, 2001, 2002 Mission Critical Linux, Inc.
This program is free software, covered by the GNU General Public License,
and you are welcome to change it and/or distribute copies of it under
certain conditions. Enter "help copying" to see the conditions.
This program has absolutely no warranty. Enter "help warranty" for details.
crash: /boot/vmlinux-3.16.0-rc7-7-default+: no .gnu_debuglink section
GNU gdb (GDB) 7.6
Copyright (C) 2013 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law. Type "show copying"
and "show warranty" for details.
This GDB was configured as "powerpc64le-unknown-linux-gnu"...
KERNEL: /boot/vmlinux-3.16.0-rc7-7-default+
DUMPFILE: vmcore
CPUS: 16
DATE: Sun Aug 24 14:31:28 2014
UPTIME: 00:02:57
LOAD AVERAGE: 0.05, 0.08, 0.04
TASKS: 256
NODENAME: linux-dhr2
RELEASE: 3.16.0-rc7-7-default+
VERSION: #54 SMP Mon Aug 18 14:08:23 EDT 2014
MACHINE: ppc64le (4116 Mhz)
MEMORY: 40 GB
PANIC: "Oops: Kernel access of bad area, sig: 11 [#1]" (check log for details)
PID: 2234
COMMAND: "bash"
TASK: c0000009652e4a30 [THREAD_INFO: c00000096777c000]
CPU: 2
STATE: TASK_RUNNING (PANIC)
crash>
Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/fadump.h | 52 ++++++++-------
arch/powerpc/kernel/fadump.c | 112 +++++++++++++++++----------------
arch/powerpc/platforms/pseries/lpar.c | 9 ++-
3 files changed, 89 insertions(+), 84 deletions(-)
diff --git a/arch/powerpc/include/asm/fadump.h b/arch/powerpc/include/asm/fadump.h
index a677456..493e72f 100644
--- a/arch/powerpc/include/asm/fadump.h
+++ b/arch/powerpc/include/asm/fadump.h
@@ -70,39 +70,39 @@
#define CPU_UNKNOWN (~((u32)0))
/* Utility macros */
-#define SKIP_TO_NEXT_CPU(reg_entry) \
-({ \
- while (reg_entry->reg_id != REG_ID("CPUEND")) \
- reg_entry++; \
- reg_entry++; \
+#define SKIP_TO_NEXT_CPU(reg_entry) \
+({ \
+ while (be64_to_cpu(reg_entry->reg_id) != REG_ID("CPUEND")) \
+ reg_entry++; \
+ reg_entry++; \
})
/* Kernel Dump section info */
struct fadump_section {
- u32 request_flag;
- u16 source_data_type;
- u16 error_flags;
- u64 source_address;
- u64 source_len;
- u64 bytes_dumped;
- u64 destination_address;
+ __be32 request_flag;
+ __be16 source_data_type;
+ __be16 error_flags;
+ __be64 source_address;
+ __be64 source_len;
+ __be64 bytes_dumped;
+ __be64 destination_address;
};
/* ibm,configure-kernel-dump header. */
struct fadump_section_header {
- u32 dump_format_version;
- u16 dump_num_sections;
- u16 dump_status_flag;
- u32 offset_first_dump_section;
+ __be32 dump_format_version;
+ __be16 dump_num_sections;
+ __be16 dump_status_flag;
+ __be32 offset_first_dump_section;
/* Fields for disk dump option. */
- u32 dd_block_size;
- u64 dd_block_offset;
- u64 dd_num_blocks;
- u32 dd_offset_disk_path;
+ __be32 dd_block_size;
+ __be64 dd_block_offset;
+ __be64 dd_num_blocks;
+ __be32 dd_offset_disk_path;
/* Maximum time allowed to prevent an automatic dump-reboot. */
- u32 max_time_auto;
+ __be32 max_time_auto;
};
/*
@@ -174,15 +174,15 @@ static inline u64 str_to_u64(const char *str)
/* Register save area header. */
struct fadump_reg_save_area_header {
- u64 magic_number;
- u32 version;
- u32 num_cpu_offset;
+ __be64 magic_number;
+ __be32 version;
+ __be32 num_cpu_offset;
};
/* Register entry. */
struct fadump_reg_entry {
- u64 reg_id;
- u64 reg_value;
+ __be64 reg_id;
+ __be64 reg_value;
};
/* fadump crash info structure */
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index 742694c..7d73b2d 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -72,7 +72,7 @@ int __init early_init_dt_scan_fw_dump(unsigned long node,
return 1;
fw_dump.fadump_supported = 1;
- fw_dump.ibm_configure_kernel_dump = *token;
+ fw_dump.ibm_configure_kernel_dump = be32_to_cpu(*token);
/*
* The 'ibm,kernel-dump' rtas node is present only if there is
@@ -147,11 +147,11 @@ static unsigned long init_fadump_mem_struct(struct fadump_mem_struct *fdm,
memset(fdm, 0, sizeof(struct fadump_mem_struct));
addr = addr & PAGE_MASK;
- fdm->header.dump_format_version = 0x00000001;
- fdm->header.dump_num_sections = 3;
+ fdm->header.dump_format_version = cpu_to_be32(0x00000001);
+ fdm->header.dump_num_sections = cpu_to_be16(3);
fdm->header.dump_status_flag = 0;
fdm->header.offset_first_dump_section =
- (u32)offsetof(struct fadump_mem_struct, cpu_state_data);
+ cpu_to_be32((u32)offsetof(struct fadump_mem_struct, cpu_state_data));
/*
* Fields for disk dump option.
@@ -167,27 +167,27 @@ static unsigned long init_fadump_mem_struct(struct fadump_mem_struct *fdm,
/* Kernel dump sections */
/* cpu state data section. */
- fdm->cpu_state_data.request_flag = FADUMP_REQUEST_FLAG;
- fdm->cpu_state_data.source_data_type = FADUMP_CPU_STATE_DATA;
+ fdm->cpu_state_data.request_flag = cpu_to_be32(FADUMP_REQUEST_FLAG);
+ fdm->cpu_state_data.source_data_type = cpu_to_be16(FADUMP_CPU_STATE_DATA);
fdm->cpu_state_data.source_address = 0;
- fdm->cpu_state_data.source_len = fw_dump.cpu_state_data_size;
- fdm->cpu_state_data.destination_address = addr;
+ fdm->cpu_state_data.source_len = cpu_to_be64(fw_dump.cpu_state_data_size);
+ fdm->cpu_state_data.destination_address = cpu_to_be64(addr);
addr += fw_dump.cpu_state_data_size;
/* hpte region section */
- fdm->hpte_region.request_flag = FADUMP_REQUEST_FLAG;
- fdm->hpte_region.source_data_type = FADUMP_HPTE_REGION;
+ fdm->hpte_region.request_flag = cpu_to_be32(FADUMP_REQUEST_FLAG);
+ fdm->hpte_region.source_data_type = cpu_to_be16(FADUMP_HPTE_REGION);
fdm->hpte_region.source_address = 0;
- fdm->hpte_region.source_len = fw_dump.hpte_region_size;
- fdm->hpte_region.destination_address = addr;
+ fdm->hpte_region.source_len = cpu_to_be64(fw_dump.hpte_region_size);
+ fdm->hpte_region.destination_address = cpu_to_be64(addr);
addr += fw_dump.hpte_region_size;
/* RMA region section */
- fdm->rmr_region.request_flag = FADUMP_REQUEST_FLAG;
- fdm->rmr_region.source_data_type = FADUMP_REAL_MODE_REGION;
- fdm->rmr_region.source_address = RMA_START;
- fdm->rmr_region.source_len = fw_dump.boot_memory_size;
- fdm->rmr_region.destination_address = addr;
+ fdm->rmr_region.request_flag = cpu_to_be32(FADUMP_REQUEST_FLAG);
+ fdm->rmr_region.source_data_type = cpu_to_be16(FADUMP_REAL_MODE_REGION);
+ fdm->rmr_region.source_address = cpu_to_be64(RMA_START);
+ fdm->rmr_region.source_len = cpu_to_be64(fw_dump.boot_memory_size);
+ fdm->rmr_region.destination_address = cpu_to_be64(addr);
addr += fw_dump.boot_memory_size;
return addr;
@@ -272,7 +272,7 @@ int __init fadump_reserve_mem(void)
* first kernel.
*/
if (fdm_active)
- fw_dump.boot_memory_size = fdm_active->rmr_region.source_len;
+ fw_dump.boot_memory_size = be64_to_cpu(fdm_active->rmr_region.source_len);
else
fw_dump.boot_memory_size = fadump_calculate_reserve_size();
@@ -314,8 +314,8 @@ int __init fadump_reserve_mem(void)
(unsigned long)(base >> 20));
fw_dump.fadumphdr_addr =
- fdm_active->rmr_region.destination_address +
- fdm_active->rmr_region.source_len;
+ be64_to_cpu(fdm_active->rmr_region.destination_address) +
+ be64_to_cpu(fdm_active->rmr_region.source_len);
pr_debug("fadumphdr_addr = %p\n",
(void *) fw_dump.fadumphdr_addr);
} else {
@@ -472,9 +472,9 @@ fadump_read_registers(struct fadump_reg_entry *reg_entry, struct pt_regs *regs)
{
memset(regs, 0, sizeof(struct pt_regs));
- while (reg_entry->reg_id != REG_ID("CPUEND")) {
- fadump_set_regval(regs, reg_entry->reg_id,
- reg_entry->reg_value);
+ while (be64_to_cpu(reg_entry->reg_id) != REG_ID("CPUEND")) {
+ fadump_set_regval(regs, be64_to_cpu(reg_entry->reg_id),
+ be64_to_cpu(reg_entry->reg_value));
reg_entry++;
}
reg_entry++;
@@ -603,20 +603,20 @@ static int __init fadump_build_cpu_notes(const struct fadump_mem_struct *fdm)
if (!fdm->cpu_state_data.bytes_dumped)
return -EINVAL;
- addr = fdm->cpu_state_data.destination_address;
+ addr = be64_to_cpu(fdm->cpu_state_data.destination_address);
vaddr = __va(addr);
reg_header = vaddr;
- if (reg_header->magic_number != REGSAVE_AREA_MAGIC) {
+ if (be64_to_cpu(reg_header->magic_number) != REGSAVE_AREA_MAGIC) {
printk(KERN_ERR "Unable to read register save area.\n");
return -ENOENT;
}
pr_debug("--------CPU State Data------------\n");
- pr_debug("Magic Number: %llx\n", reg_header->magic_number);
- pr_debug("NumCpuOffset: %x\n", reg_header->num_cpu_offset);
+ pr_debug("Magic Number: %llx\n", be64_to_cpu(reg_header->magic_number));
+ pr_debug("NumCpuOffset: %x\n", be32_to_cpu(reg_header->num_cpu_offset));
- vaddr += reg_header->num_cpu_offset;
- num_cpus = *((u32 *)(vaddr));
+ vaddr += be32_to_cpu(reg_header->num_cpu_offset);
+ num_cpus = be32_to_cpu(*((u32 *)(vaddr)));
pr_debug("NumCpus : %u\n", num_cpus);
vaddr += sizeof(u32);
reg_entry = (struct fadump_reg_entry *)vaddr;
@@ -639,13 +639,13 @@ static int __init fadump_build_cpu_notes(const struct fadump_mem_struct *fdm)
fdh = __va(fw_dump.fadumphdr_addr);
for (i = 0; i < num_cpus; i++) {
- if (reg_entry->reg_id != REG_ID("CPUSTRT")) {
+ if (be64_to_cpu(reg_entry->reg_id) != REG_ID("CPUSTRT")) {
printk(KERN_ERR "Unable to read CPU state data\n");
rc = -ENOENT;
goto error_out;
}
/* Lower 4 bytes of reg_value contains logical cpu id */
- cpu = reg_entry->reg_value & FADUMP_CPU_ID_MASK;
+ cpu = be64_to_cpu(reg_entry->reg_value) & FADUMP_CPU_ID_MASK;
if (fdh && !cpumask_test_cpu(cpu, &fdh->cpu_online_mask)) {
SKIP_TO_NEXT_CPU(reg_entry);
continue;
@@ -692,7 +692,7 @@ static int __init process_fadump(const struct fadump_mem_struct *fdm_active)
return -EINVAL;
/* Check if the dump data is valid. */
- if ((fdm_active->header.dump_status_flag == FADUMP_ERROR_FLAG) ||
+ if ((be16_to_cpu(fdm_active->header.dump_status_flag) == FADUMP_ERROR_FLAG) ||
(fdm_active->cpu_state_data.error_flags != 0) ||
(fdm_active->rmr_region.error_flags != 0)) {
printk(KERN_ERR "Dump taken by platform is not valid\n");
@@ -828,7 +828,7 @@ static void fadump_setup_crash_memory_ranges(void)
static inline unsigned long fadump_relocate(unsigned long paddr)
{
if (paddr > RMA_START && paddr < fw_dump.boot_memory_size)
- return fdm.rmr_region.destination_address + paddr;
+ return be64_to_cpu(fdm.rmr_region.destination_address) + paddr;
else
return paddr;
}
@@ -902,7 +902,7 @@ static int fadump_create_elfcore_headers(char *bufp)
* to the specified destination_address. Hence set
* the correct offset.
*/
- phdr->p_offset = fdm.rmr_region.destination_address;
+ phdr->p_offset = be64_to_cpu(fdm.rmr_region.destination_address);
}
phdr->p_paddr = mbase;
@@ -951,7 +951,7 @@ static void register_fadump(void)
fadump_setup_crash_memory_ranges();
- addr = fdm.rmr_region.destination_address + fdm.rmr_region.source_len;
+ addr = be64_to_cpu(fdm.rmr_region.destination_address) + be64_to_cpu(fdm.rmr_region.source_len);
/* Initialize fadump crash info header. */
addr = init_fadump_header(addr);
vaddr = __va(addr);
@@ -1023,7 +1023,7 @@ void fadump_cleanup(void)
/* Invalidate the registration only if dump is active. */
if (fw_dump.dump_active) {
init_fadump_mem_struct(&fdm,
- fdm_active->cpu_state_data.destination_address);
+ be64_to_cpu(fdm_active->cpu_state_data.destination_address));
fadump_invalidate_dump(&fdm);
}
}
@@ -1063,7 +1063,7 @@ static void fadump_invalidate_release_mem(void)
return;
}
- destination_address = fdm_active->cpu_state_data.destination_address;
+ destination_address = be64_to_cpu(fdm_active->cpu_state_data.destination_address);
fadump_cleanup();
mutex_unlock(&fadump_mutex);
@@ -1183,31 +1183,31 @@ static int fadump_region_show(struct seq_file *m, void *private)
seq_printf(m,
"CPU : [%#016llx-%#016llx] %#llx bytes, "
"Dumped: %#llx\n",
- fdm_ptr->cpu_state_data.destination_address,
- fdm_ptr->cpu_state_data.destination_address +
- fdm_ptr->cpu_state_data.source_len - 1,
- fdm_ptr->cpu_state_data.source_len,
- fdm_ptr->cpu_state_data.bytes_dumped);
+ be64_to_cpu(fdm_ptr->cpu_state_data.destination_address),
+ be64_to_cpu(fdm_ptr->cpu_state_data.destination_address) +
+ be64_to_cpu(fdm_ptr->cpu_state_data.source_len) - 1,
+ be64_to_cpu(fdm_ptr->cpu_state_data.source_len),
+ be64_to_cpu(fdm_ptr->cpu_state_data.bytes_dumped));
seq_printf(m,
"HPTE: [%#016llx-%#016llx] %#llx bytes, "
"Dumped: %#llx\n",
- fdm_ptr->hpte_region.destination_address,
- fdm_ptr->hpte_region.destination_address +
- fdm_ptr->hpte_region.source_len - 1,
- fdm_ptr->hpte_region.source_len,
- fdm_ptr->hpte_region.bytes_dumped);
+ be64_to_cpu(fdm_ptr->hpte_region.destination_address),
+ be64_to_cpu(fdm_ptr->hpte_region.destination_address) +
+ be64_to_cpu(fdm_ptr->hpte_region.source_len) - 1,
+ be64_to_cpu(fdm_ptr->hpte_region.source_len),
+ be64_to_cpu(fdm_ptr->hpte_region.bytes_dumped));
seq_printf(m,
"DUMP: [%#016llx-%#016llx] %#llx bytes, "
"Dumped: %#llx\n",
- fdm_ptr->rmr_region.destination_address,
- fdm_ptr->rmr_region.destination_address +
- fdm_ptr->rmr_region.source_len - 1,
- fdm_ptr->rmr_region.source_len,
- fdm_ptr->rmr_region.bytes_dumped);
+ be64_to_cpu(fdm_ptr->rmr_region.destination_address),
+ be64_to_cpu(fdm_ptr->rmr_region.destination_address) +
+ be64_to_cpu(fdm_ptr->rmr_region.source_len) - 1,
+ be64_to_cpu(fdm_ptr->rmr_region.source_len),
+ be64_to_cpu(fdm_ptr->rmr_region.bytes_dumped));
if (!fdm_active ||
(fw_dump.reserve_dump_area_start ==
- fdm_ptr->cpu_state_data.destination_address))
+ be64_to_cpu(fdm_ptr->cpu_state_data.destination_address)))
goto out;
/* Dump is active. Show reserved memory region. */
@@ -1215,10 +1215,10 @@ static int fadump_region_show(struct seq_file *m, void *private)
" : [%#016llx-%#016llx] %#llx bytes, "
"Dumped: %#llx\n",
(unsigned long long)fw_dump.reserve_dump_area_start,
- fdm_ptr->cpu_state_data.destination_address - 1,
- fdm_ptr->cpu_state_data.destination_address -
+ be64_to_cpu(fdm_ptr->cpu_state_data.destination_address) - 1,
+ be64_to_cpu(fdm_ptr->cpu_state_data.destination_address) -
fw_dump.reserve_dump_area_start,
- fdm_ptr->cpu_state_data.destination_address -
+ be64_to_cpu(fdm_ptr->cpu_state_data.destination_address) -
fw_dump.reserve_dump_area_start);
out:
if (fdm_active)
diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
index 34e6423..587887e 100644
--- a/arch/powerpc/platforms/pseries/lpar.c
+++ b/arch/powerpc/platforms/pseries/lpar.c
@@ -43,6 +43,7 @@
#include <asm/trace.h>
#include <asm/firmware.h>
#include <asm/plpar_wrappers.h>
+#include <asm/fadump.h>
#include "pseries.h"
@@ -249,8 +250,12 @@ static void pSeries_lpar_hptab_clear(void)
}
#ifdef __LITTLE_ENDIAN__
- /* Reset exceptions to big endian */
- if (firmware_has_feature(FW_FEATURE_SET_MODE)) {
+ /*
+ * Reset exceptions to big endian
+ * During fadump kernel boot, we dont need to reset exception to big endian
+ * as we have already booted into LE kernel.
+ */
+ if (firmware_has_feature(FW_FEATURE_SET_MODE) && !is_fadump_active()) {
long rc;
rc = pseries_big_endian_exceptions();
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] fadump: fix endianess issues in firmware assisted dump handling
2014-09-03 12:29 [PATCH] fadump: fix endianess issues in firmware assisted dump handling Hari Bathini
@ 2014-09-05 5:55 ` Mahesh Jagannath Salgaonkar
2014-09-25 1:40 ` Michael Ellerman
1 sibling, 0 replies; 3+ messages in thread
From: Mahesh Jagannath Salgaonkar @ 2014-09-05 5:55 UTC (permalink / raw)
To: Hari Bathini, linuxppc-dev
On 09/03/2014 05:59 PM, Hari Bathini wrote:
> Firmware-assisted dump (fadump) kernel code is not LE compliant. The
> below patch tries to fix this issue. Tested this patch with upstream
> kernel. Did some sanity testing for the LE fadump vmcore generated.
> Below output shows crash tool successfully opening LE fadump vmcore.
>
> # crash $vmlinux vmcore
>
> crash 7.0.5
> Copyright (C) 2002-2014 Red Hat, Inc.
> Copyright (C) 2004, 2005, 2006, 2010 IBM Corporation
> Copyright (C) 1999-2006 Hewlett-Packard Co
> Copyright (C) 2005, 2006, 2011, 2012 Fujitsu Limited
> Copyright (C) 2006, 2007 VA Linux Systems Japan K.K.
> Copyright (C) 2005, 2011 NEC Corporation
> Copyright (C) 1999, 2002, 2007 Silicon Graphics, Inc.
> Copyright (C) 1999, 2000, 2001, 2002 Mission Critical Linux, Inc.
> This program is free software, covered by the GNU General Public License,
> and you are welcome to change it and/or distribute copies of it under
> certain conditions. Enter "help copying" to see the conditions.
> This program has absolutely no warranty. Enter "help warranty" for details.
>
> crash: /boot/vmlinux-3.16.0-rc7-7-default+: no .gnu_debuglink section
> GNU gdb (GDB) 7.6
> Copyright (C) 2013 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law. Type "show copying"
> and "show warranty" for details.
> This GDB was configured as "powerpc64le-unknown-linux-gnu"...
>
> KERNEL: /boot/vmlinux-3.16.0-rc7-7-default+
> DUMPFILE: vmcore
> CPUS: 16
> DATE: Sun Aug 24 14:31:28 2014
> UPTIME: 00:02:57
> LOAD AVERAGE: 0.05, 0.08, 0.04
> TASKS: 256
> NODENAME: linux-dhr2
> RELEASE: 3.16.0-rc7-7-default+
> VERSION: #54 SMP Mon Aug 18 14:08:23 EDT 2014
> MACHINE: ppc64le (4116 Mhz)
> MEMORY: 40 GB
> PANIC: "Oops: Kernel access of bad area, sig: 11 [#1]" (check log for details)
> PID: 2234
> COMMAND: "bash"
> TASK: c0000009652e4a30 [THREAD_INFO: c00000096777c000]
> CPU: 2
> STATE: TASK_RUNNING (PANIC)
>
> crash>
>
> Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
Reviewed-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/fadump.h | 52 ++++++++-------
> arch/powerpc/kernel/fadump.c | 112 +++++++++++++++++----------------
> arch/powerpc/platforms/pseries/lpar.c | 9 ++-
> 3 files changed, 89 insertions(+), 84 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/fadump.h b/arch/powerpc/include/asm/fadump.h
> index a677456..493e72f 100644
> --- a/arch/powerpc/include/asm/fadump.h
> +++ b/arch/powerpc/include/asm/fadump.h
> @@ -70,39 +70,39 @@
> #define CPU_UNKNOWN (~((u32)0))
>
> /* Utility macros */
> -#define SKIP_TO_NEXT_CPU(reg_entry) \
> -({ \
> - while (reg_entry->reg_id != REG_ID("CPUEND")) \
> - reg_entry++; \
> - reg_entry++; \
> +#define SKIP_TO_NEXT_CPU(reg_entry) \
> +({ \
> + while (be64_to_cpu(reg_entry->reg_id) != REG_ID("CPUEND")) \
> + reg_entry++; \
> + reg_entry++; \
> })
>
> /* Kernel Dump section info */
> struct fadump_section {
> - u32 request_flag;
> - u16 source_data_type;
> - u16 error_flags;
> - u64 source_address;
> - u64 source_len;
> - u64 bytes_dumped;
> - u64 destination_address;
> + __be32 request_flag;
> + __be16 source_data_type;
> + __be16 error_flags;
> + __be64 source_address;
> + __be64 source_len;
> + __be64 bytes_dumped;
> + __be64 destination_address;
> };
>
> /* ibm,configure-kernel-dump header. */
> struct fadump_section_header {
> - u32 dump_format_version;
> - u16 dump_num_sections;
> - u16 dump_status_flag;
> - u32 offset_first_dump_section;
> + __be32 dump_format_version;
> + __be16 dump_num_sections;
> + __be16 dump_status_flag;
> + __be32 offset_first_dump_section;
>
> /* Fields for disk dump option. */
> - u32 dd_block_size;
> - u64 dd_block_offset;
> - u64 dd_num_blocks;
> - u32 dd_offset_disk_path;
> + __be32 dd_block_size;
> + __be64 dd_block_offset;
> + __be64 dd_num_blocks;
> + __be32 dd_offset_disk_path;
>
> /* Maximum time allowed to prevent an automatic dump-reboot. */
> - u32 max_time_auto;
> + __be32 max_time_auto;
> };
>
> /*
> @@ -174,15 +174,15 @@ static inline u64 str_to_u64(const char *str)
>
> /* Register save area header. */
> struct fadump_reg_save_area_header {
> - u64 magic_number;
> - u32 version;
> - u32 num_cpu_offset;
> + __be64 magic_number;
> + __be32 version;
> + __be32 num_cpu_offset;
> };
>
> /* Register entry. */
> struct fadump_reg_entry {
> - u64 reg_id;
> - u64 reg_value;
> + __be64 reg_id;
> + __be64 reg_value;
> };
>
> /* fadump crash info structure */
> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index 742694c..7d73b2d 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -72,7 +72,7 @@ int __init early_init_dt_scan_fw_dump(unsigned long node,
> return 1;
>
> fw_dump.fadump_supported = 1;
> - fw_dump.ibm_configure_kernel_dump = *token;
> + fw_dump.ibm_configure_kernel_dump = be32_to_cpu(*token);
>
> /*
> * The 'ibm,kernel-dump' rtas node is present only if there is
> @@ -147,11 +147,11 @@ static unsigned long init_fadump_mem_struct(struct fadump_mem_struct *fdm,
> memset(fdm, 0, sizeof(struct fadump_mem_struct));
> addr = addr & PAGE_MASK;
>
> - fdm->header.dump_format_version = 0x00000001;
> - fdm->header.dump_num_sections = 3;
> + fdm->header.dump_format_version = cpu_to_be32(0x00000001);
> + fdm->header.dump_num_sections = cpu_to_be16(3);
> fdm->header.dump_status_flag = 0;
> fdm->header.offset_first_dump_section =
> - (u32)offsetof(struct fadump_mem_struct, cpu_state_data);
> + cpu_to_be32((u32)offsetof(struct fadump_mem_struct, cpu_state_data));
>
> /*
> * Fields for disk dump option.
> @@ -167,27 +167,27 @@ static unsigned long init_fadump_mem_struct(struct fadump_mem_struct *fdm,
>
> /* Kernel dump sections */
> /* cpu state data section. */
> - fdm->cpu_state_data.request_flag = FADUMP_REQUEST_FLAG;
> - fdm->cpu_state_data.source_data_type = FADUMP_CPU_STATE_DATA;
> + fdm->cpu_state_data.request_flag = cpu_to_be32(FADUMP_REQUEST_FLAG);
> + fdm->cpu_state_data.source_data_type = cpu_to_be16(FADUMP_CPU_STATE_DATA);
> fdm->cpu_state_data.source_address = 0;
> - fdm->cpu_state_data.source_len = fw_dump.cpu_state_data_size;
> - fdm->cpu_state_data.destination_address = addr;
> + fdm->cpu_state_data.source_len = cpu_to_be64(fw_dump.cpu_state_data_size);
> + fdm->cpu_state_data.destination_address = cpu_to_be64(addr);
> addr += fw_dump.cpu_state_data_size;
>
> /* hpte region section */
> - fdm->hpte_region.request_flag = FADUMP_REQUEST_FLAG;
> - fdm->hpte_region.source_data_type = FADUMP_HPTE_REGION;
> + fdm->hpte_region.request_flag = cpu_to_be32(FADUMP_REQUEST_FLAG);
> + fdm->hpte_region.source_data_type = cpu_to_be16(FADUMP_HPTE_REGION);
> fdm->hpte_region.source_address = 0;
> - fdm->hpte_region.source_len = fw_dump.hpte_region_size;
> - fdm->hpte_region.destination_address = addr;
> + fdm->hpte_region.source_len = cpu_to_be64(fw_dump.hpte_region_size);
> + fdm->hpte_region.destination_address = cpu_to_be64(addr);
> addr += fw_dump.hpte_region_size;
>
> /* RMA region section */
> - fdm->rmr_region.request_flag = FADUMP_REQUEST_FLAG;
> - fdm->rmr_region.source_data_type = FADUMP_REAL_MODE_REGION;
> - fdm->rmr_region.source_address = RMA_START;
> - fdm->rmr_region.source_len = fw_dump.boot_memory_size;
> - fdm->rmr_region.destination_address = addr;
> + fdm->rmr_region.request_flag = cpu_to_be32(FADUMP_REQUEST_FLAG);
> + fdm->rmr_region.source_data_type = cpu_to_be16(FADUMP_REAL_MODE_REGION);
> + fdm->rmr_region.source_address = cpu_to_be64(RMA_START);
> + fdm->rmr_region.source_len = cpu_to_be64(fw_dump.boot_memory_size);
> + fdm->rmr_region.destination_address = cpu_to_be64(addr);
> addr += fw_dump.boot_memory_size;
>
> return addr;
> @@ -272,7 +272,7 @@ int __init fadump_reserve_mem(void)
> * first kernel.
> */
> if (fdm_active)
> - fw_dump.boot_memory_size = fdm_active->rmr_region.source_len;
> + fw_dump.boot_memory_size = be64_to_cpu(fdm_active->rmr_region.source_len);
> else
> fw_dump.boot_memory_size = fadump_calculate_reserve_size();
>
> @@ -314,8 +314,8 @@ int __init fadump_reserve_mem(void)
> (unsigned long)(base >> 20));
>
> fw_dump.fadumphdr_addr =
> - fdm_active->rmr_region.destination_address +
> - fdm_active->rmr_region.source_len;
> + be64_to_cpu(fdm_active->rmr_region.destination_address) +
> + be64_to_cpu(fdm_active->rmr_region.source_len);
> pr_debug("fadumphdr_addr = %p\n",
> (void *) fw_dump.fadumphdr_addr);
> } else {
> @@ -472,9 +472,9 @@ fadump_read_registers(struct fadump_reg_entry *reg_entry, struct pt_regs *regs)
> {
> memset(regs, 0, sizeof(struct pt_regs));
>
> - while (reg_entry->reg_id != REG_ID("CPUEND")) {
> - fadump_set_regval(regs, reg_entry->reg_id,
> - reg_entry->reg_value);
> + while (be64_to_cpu(reg_entry->reg_id) != REG_ID("CPUEND")) {
> + fadump_set_regval(regs, be64_to_cpu(reg_entry->reg_id),
> + be64_to_cpu(reg_entry->reg_value));
> reg_entry++;
> }
> reg_entry++;
> @@ -603,20 +603,20 @@ static int __init fadump_build_cpu_notes(const struct fadump_mem_struct *fdm)
> if (!fdm->cpu_state_data.bytes_dumped)
> return -EINVAL;
>
> - addr = fdm->cpu_state_data.destination_address;
> + addr = be64_to_cpu(fdm->cpu_state_data.destination_address);
> vaddr = __va(addr);
>
> reg_header = vaddr;
> - if (reg_header->magic_number != REGSAVE_AREA_MAGIC) {
> + if (be64_to_cpu(reg_header->magic_number) != REGSAVE_AREA_MAGIC) {
> printk(KERN_ERR "Unable to read register save area.\n");
> return -ENOENT;
> }
> pr_debug("--------CPU State Data------------\n");
> - pr_debug("Magic Number: %llx\n", reg_header->magic_number);
> - pr_debug("NumCpuOffset: %x\n", reg_header->num_cpu_offset);
> + pr_debug("Magic Number: %llx\n", be64_to_cpu(reg_header->magic_number));
> + pr_debug("NumCpuOffset: %x\n", be32_to_cpu(reg_header->num_cpu_offset));
>
> - vaddr += reg_header->num_cpu_offset;
> - num_cpus = *((u32 *)(vaddr));
> + vaddr += be32_to_cpu(reg_header->num_cpu_offset);
> + num_cpus = be32_to_cpu(*((u32 *)(vaddr)));
> pr_debug("NumCpus : %u\n", num_cpus);
> vaddr += sizeof(u32);
> reg_entry = (struct fadump_reg_entry *)vaddr;
> @@ -639,13 +639,13 @@ static int __init fadump_build_cpu_notes(const struct fadump_mem_struct *fdm)
> fdh = __va(fw_dump.fadumphdr_addr);
>
> for (i = 0; i < num_cpus; i++) {
> - if (reg_entry->reg_id != REG_ID("CPUSTRT")) {
> + if (be64_to_cpu(reg_entry->reg_id) != REG_ID("CPUSTRT")) {
> printk(KERN_ERR "Unable to read CPU state data\n");
> rc = -ENOENT;
> goto error_out;
> }
> /* Lower 4 bytes of reg_value contains logical cpu id */
> - cpu = reg_entry->reg_value & FADUMP_CPU_ID_MASK;
> + cpu = be64_to_cpu(reg_entry->reg_value) & FADUMP_CPU_ID_MASK;
> if (fdh && !cpumask_test_cpu(cpu, &fdh->cpu_online_mask)) {
> SKIP_TO_NEXT_CPU(reg_entry);
> continue;
> @@ -692,7 +692,7 @@ static int __init process_fadump(const struct fadump_mem_struct *fdm_active)
> return -EINVAL;
>
> /* Check if the dump data is valid. */
> - if ((fdm_active->header.dump_status_flag == FADUMP_ERROR_FLAG) ||
> + if ((be16_to_cpu(fdm_active->header.dump_status_flag) == FADUMP_ERROR_FLAG) ||
> (fdm_active->cpu_state_data.error_flags != 0) ||
> (fdm_active->rmr_region.error_flags != 0)) {
> printk(KERN_ERR "Dump taken by platform is not valid\n");
> @@ -828,7 +828,7 @@ static void fadump_setup_crash_memory_ranges(void)
> static inline unsigned long fadump_relocate(unsigned long paddr)
> {
> if (paddr > RMA_START && paddr < fw_dump.boot_memory_size)
> - return fdm.rmr_region.destination_address + paddr;
> + return be64_to_cpu(fdm.rmr_region.destination_address) + paddr;
> else
> return paddr;
> }
> @@ -902,7 +902,7 @@ static int fadump_create_elfcore_headers(char *bufp)
> * to the specified destination_address. Hence set
> * the correct offset.
> */
> - phdr->p_offset = fdm.rmr_region.destination_address;
> + phdr->p_offset = be64_to_cpu(fdm.rmr_region.destination_address);
> }
>
> phdr->p_paddr = mbase;
> @@ -951,7 +951,7 @@ static void register_fadump(void)
>
> fadump_setup_crash_memory_ranges();
>
> - addr = fdm.rmr_region.destination_address + fdm.rmr_region.source_len;
> + addr = be64_to_cpu(fdm.rmr_region.destination_address) + be64_to_cpu(fdm.rmr_region.source_len);
> /* Initialize fadump crash info header. */
> addr = init_fadump_header(addr);
> vaddr = __va(addr);
> @@ -1023,7 +1023,7 @@ void fadump_cleanup(void)
> /* Invalidate the registration only if dump is active. */
> if (fw_dump.dump_active) {
> init_fadump_mem_struct(&fdm,
> - fdm_active->cpu_state_data.destination_address);
> + be64_to_cpu(fdm_active->cpu_state_data.destination_address));
> fadump_invalidate_dump(&fdm);
> }
> }
> @@ -1063,7 +1063,7 @@ static void fadump_invalidate_release_mem(void)
> return;
> }
>
> - destination_address = fdm_active->cpu_state_data.destination_address;
> + destination_address = be64_to_cpu(fdm_active->cpu_state_data.destination_address);
> fadump_cleanup();
> mutex_unlock(&fadump_mutex);
>
> @@ -1183,31 +1183,31 @@ static int fadump_region_show(struct seq_file *m, void *private)
> seq_printf(m,
> "CPU : [%#016llx-%#016llx] %#llx bytes, "
> "Dumped: %#llx\n",
> - fdm_ptr->cpu_state_data.destination_address,
> - fdm_ptr->cpu_state_data.destination_address +
> - fdm_ptr->cpu_state_data.source_len - 1,
> - fdm_ptr->cpu_state_data.source_len,
> - fdm_ptr->cpu_state_data.bytes_dumped);
> + be64_to_cpu(fdm_ptr->cpu_state_data.destination_address),
> + be64_to_cpu(fdm_ptr->cpu_state_data.destination_address) +
> + be64_to_cpu(fdm_ptr->cpu_state_data.source_len) - 1,
> + be64_to_cpu(fdm_ptr->cpu_state_data.source_len),
> + be64_to_cpu(fdm_ptr->cpu_state_data.bytes_dumped));
> seq_printf(m,
> "HPTE: [%#016llx-%#016llx] %#llx bytes, "
> "Dumped: %#llx\n",
> - fdm_ptr->hpte_region.destination_address,
> - fdm_ptr->hpte_region.destination_address +
> - fdm_ptr->hpte_region.source_len - 1,
> - fdm_ptr->hpte_region.source_len,
> - fdm_ptr->hpte_region.bytes_dumped);
> + be64_to_cpu(fdm_ptr->hpte_region.destination_address),
> + be64_to_cpu(fdm_ptr->hpte_region.destination_address) +
> + be64_to_cpu(fdm_ptr->hpte_region.source_len) - 1,
> + be64_to_cpu(fdm_ptr->hpte_region.source_len),
> + be64_to_cpu(fdm_ptr->hpte_region.bytes_dumped));
> seq_printf(m,
> "DUMP: [%#016llx-%#016llx] %#llx bytes, "
> "Dumped: %#llx\n",
> - fdm_ptr->rmr_region.destination_address,
> - fdm_ptr->rmr_region.destination_address +
> - fdm_ptr->rmr_region.source_len - 1,
> - fdm_ptr->rmr_region.source_len,
> - fdm_ptr->rmr_region.bytes_dumped);
> + be64_to_cpu(fdm_ptr->rmr_region.destination_address),
> + be64_to_cpu(fdm_ptr->rmr_region.destination_address) +
> + be64_to_cpu(fdm_ptr->rmr_region.source_len) - 1,
> + be64_to_cpu(fdm_ptr->rmr_region.source_len),
> + be64_to_cpu(fdm_ptr->rmr_region.bytes_dumped));
>
> if (!fdm_active ||
> (fw_dump.reserve_dump_area_start ==
> - fdm_ptr->cpu_state_data.destination_address))
> + be64_to_cpu(fdm_ptr->cpu_state_data.destination_address)))
> goto out;
>
> /* Dump is active. Show reserved memory region. */
> @@ -1215,10 +1215,10 @@ static int fadump_region_show(struct seq_file *m, void *private)
> " : [%#016llx-%#016llx] %#llx bytes, "
> "Dumped: %#llx\n",
> (unsigned long long)fw_dump.reserve_dump_area_start,
> - fdm_ptr->cpu_state_data.destination_address - 1,
> - fdm_ptr->cpu_state_data.destination_address -
> + be64_to_cpu(fdm_ptr->cpu_state_data.destination_address) - 1,
> + be64_to_cpu(fdm_ptr->cpu_state_data.destination_address) -
> fw_dump.reserve_dump_area_start,
> - fdm_ptr->cpu_state_data.destination_address -
> + be64_to_cpu(fdm_ptr->cpu_state_data.destination_address) -
> fw_dump.reserve_dump_area_start);
> out:
> if (fdm_active)
> diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
> index 34e6423..587887e 100644
> --- a/arch/powerpc/platforms/pseries/lpar.c
> +++ b/arch/powerpc/platforms/pseries/lpar.c
> @@ -43,6 +43,7 @@
> #include <asm/trace.h>
> #include <asm/firmware.h>
> #include <asm/plpar_wrappers.h>
> +#include <asm/fadump.h>
>
> #include "pseries.h"
>
> @@ -249,8 +250,12 @@ static void pSeries_lpar_hptab_clear(void)
> }
>
> #ifdef __LITTLE_ENDIAN__
> - /* Reset exceptions to big endian */
> - if (firmware_has_feature(FW_FEATURE_SET_MODE)) {
> + /*
> + * Reset exceptions to big endian
> + * During fadump kernel boot, we dont need to reset exception to big endian
> + * as we have already booted into LE kernel.
> + */
> + if (firmware_has_feature(FW_FEATURE_SET_MODE) && !is_fadump_active()) {
> long rc;
>
> rc = pseries_big_endian_exceptions();
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: fadump: fix endianess issues in firmware assisted dump handling
2014-09-03 12:29 [PATCH] fadump: fix endianess issues in firmware assisted dump handling Hari Bathini
2014-09-05 5:55 ` Mahesh Jagannath Salgaonkar
@ 2014-09-25 1:40 ` Michael Ellerman
1 sibling, 0 replies; 3+ messages in thread
From: Michael Ellerman @ 2014-09-25 1:40 UTC (permalink / raw)
To: Hari Bathini, linuxppc-dev; +Cc: Mahesh J Salgaonkar
On Wed, 2014-03-09 at 12:29:48 UTC, Hari Bathini wrote:
> Firmware-assisted dump (fadump) kernel code is not LE compliant. The
> below patch tries to fix this issue. Tested this patch with upstream
> kernel. Did some sanity testing for the LE fadump vmcore generated.
> Below output shows crash tool successfully opening LE fadump vmcore.
>
> Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
> Reviewed-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
In general I really dislike this kind of endian conversion, ie. where we just
litter the code with endian conversions at every usage site.
But in this case it's probably OK, because we can't really do much better.
> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index 742694c..7d73b2d 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -72,7 +72,7 @@ int __init early_init_dt_scan_fw_dump(unsigned long node,
> return 1;
>
> fw_dump.fadump_supported = 1;
> - fw_dump.ibm_configure_kernel_dump = *token;
> + fw_dump.ibm_configure_kernel_dump = be32_to_cpu(*token);
I'm getting a sparse warning here:
arch/powerpc/kernel/fadump.c:75:45: warning: cast to restricted __be32
I think token should be a __be32 *.
> pr_debug("--------CPU State Data------------\n");
> - pr_debug("Magic Number: %llx\n", reg_header->magic_number);
> - pr_debug("NumCpuOffset: %x\n", reg_header->num_cpu_offset);
> + pr_debug("Magic Number: %llx\n", be64_to_cpu(reg_header->magic_number));
> + pr_debug("NumCpuOffset: %x\n", be32_to_cpu(reg_header->num_cpu_offset));
>
> - vaddr += reg_header->num_cpu_offset;
> - num_cpus = *((u32 *)(vaddr));
> + vaddr += be32_to_cpu(reg_header->num_cpu_offset);
> + num_cpus = be32_to_cpu(*((u32 *)(vaddr)));
And here too:
arch/powerpc/kernel/fadump.c:619:20: warning: cast to restricted __be32
I guess the cast is OK there because you are calculating the offset, but the
cast should be to __be32.
> diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
> index 34e6423..587887e 100644
> --- a/arch/powerpc/platforms/pseries/lpar.c
> +++ b/arch/powerpc/platforms/pseries/lpar.c
> @@ -43,6 +43,7 @@
> #include <asm/trace.h>
> #include <asm/firmware.h>
> #include <asm/plpar_wrappers.h>
> +#include <asm/fadump.h>
>
> #include "pseries.h"
>
> @@ -249,8 +250,12 @@ static void pSeries_lpar_hptab_clear(void)
> }
>
> #ifdef __LITTLE_ENDIAN__
> - /* Reset exceptions to big endian */
> - if (firmware_has_feature(FW_FEATURE_SET_MODE)) {
> + /*
> + * Reset exceptions to big endian
> + * During fadump kernel boot, we dont need to reset exception to big endian
> + * as we have already booted into LE kernel.
> + */
> + if (firmware_has_feature(FW_FEATURE_SET_MODE) && !is_fadump_active()) {
> long rc;
>
> rc = pseries_big_endian_exceptions();
It's really unfortunate that we need to inject this knowledge of fadump into
the setup code.
It sounds like we have to do it though, Mahesh said on irc that without it the
system won't boot. Please elaborate on *why* the system won't boot.
And please make the comment much clearer, ie. it's not that we don't need to
reset exceptions to big endian, it's that we *must not* reset them to big
endian.
cheers
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-09-25 1:40 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-03 12:29 [PATCH] fadump: fix endianess issues in firmware assisted dump handling Hari Bathini
2014-09-05 5:55 ` Mahesh Jagannath Salgaonkar
2014-09-25 1:40 ` Michael Ellerman
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).