Mahesh, I think we should explicitly document that production and capture kernel versions should be same. For changes like below, older/newer production kernel vs capture kernel is bound to fail. Of course, production and capture kernel versions would be the same case usually but for the uninitiated, mentioning that explicitly
in documentation would help..?

Thanks
Hari


On Monday 02 April 2018 11:59 AM, Mahesh J Salgaonkar wrote:
From: Mahesh Salgaonkar <mah...@linux.vnet.ibm.com>

Currently the metadata region that holds crash info structure and ELF core
header is placed towards the end of reserved memory area. This patch places
it at the beginning of the reserved memory area.

Signed-off-by: Mahesh Salgaonkar <mah...@linux.vnet.ibm.com>
---
  arch/powerpc/include/asm/fadump.h |    4 ++
  arch/powerpc/kernel/fadump.c      |   92 ++++++++++++++++++++++++++++++++-----
  2 files changed, 83 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/include/asm/fadump.h 
b/arch/powerpc/include/asm/fadump.h
index 5a23010af600..44b6ebfe9be6 100644
--- a/arch/powerpc/include/asm/fadump.h
+++ b/arch/powerpc/include/asm/fadump.h
@@ -61,6 +61,9 @@
  #define FADUMP_UNREGISTER     2
  #define FADUMP_INVALIDATE     3

+/* Number of dump sections requested by kernel */
+#define FADUMP_NUM_SECTIONS    4
+
  /* Dump status flag */
  #define FADUMP_ERROR_FLAG     0x2000

@@ -119,6 +122,7 @@ struct fadump_mem_struct {
        struct fadump_section           cpu_state_data;
        struct fadump_section           hpte_region;
        struct fadump_section           rmr_region;
+       struct fadump_section           metadata_region;
  };

  /* Firmware-assisted dump configuration details. */
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index 3c2c2688918f..80552fd7d5f8 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -188,17 +188,48 @@ static void fadump_show_config(void)
        pr_debug("Boot memory size  : %lx\n", fw_dump.boot_memory_size);
  }

+static unsigned long get_fadump_metadata_size(void)
+{
+       unsigned long size = 0;
+
+       /*
+        * If fadump is active then look into fdm_active to get metadata
+        * size set by previous kernel.
+        */
+       if (fw_dump.dump_active) {
+               size = fdm_active->cpu_state_data.destination_address -
+                               fdm_active->metadata_region.source_address;
+               goto out;
+       }
+       size += sizeof(struct fadump_crash_info_header);
+       size += sizeof(struct elfhdr); /* ELF core header.*/
+       size += sizeof(struct elf_phdr); /* place holder for cpu notes */
+       /* Program headers for crash memory regions. */
+       size += sizeof(struct elf_phdr) * (memblock_num_regions(memory) + 2);
+
+       size = PAGE_ALIGN(size);
+out:
+       pr_debug("fadump Metadata size is %ld\n", size);
+       return size;
+}
+
  static unsigned long init_fadump_mem_struct(struct fadump_mem_struct *fdm,
                                unsigned long addr)
  {
+       uint16_t num_sections = 0;
+       unsigned long metadata_base = 0;
+
        if (!fdm)
                return 0;

+       /* Skip the fadump metadata area. */
+       metadata_base = addr;
+       addr += get_fadump_metadata_size();
+
        memset(fdm, 0, sizeof(struct fadump_mem_struct));
        addr = addr & PAGE_MASK;

        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 =
                cpu_to_be32((u32)offsetof(struct fadump_mem_struct, 
cpu_state_data));
@@ -222,6 +253,7 @@ static unsigned long init_fadump_mem_struct(struct 
fadump_mem_struct *fdm,
        fdm->cpu_state_data.source_address = 0;
        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);
+       num_sections++;
        addr += fw_dump.cpu_state_data_size;

        /* hpte region section */
@@ -230,6 +262,7 @@ static unsigned long init_fadump_mem_struct(struct 
fadump_mem_struct *fdm,
        fdm->hpte_region.source_address = 0;
        fdm->hpte_region.source_len = cpu_to_be64(fw_dump.hpte_region_size);
        fdm->hpte_region.destination_address = cpu_to_be64(addr);
+       num_sections++;
        addr += fw_dump.hpte_region_size;

        /* RMA region section */
@@ -238,8 +271,25 @@ static unsigned long init_fadump_mem_struct(struct 
fadump_mem_struct *fdm,
        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);
+       num_sections++;
        addr += fw_dump.boot_memory_size;

+       /*
+        * fadump metadata section.
+        * Add an entry with source len a zero. We are only intereseted in
+        * source address which will help us to detect the location of
+        * metadata area where faump header and elf core header is placed.
+        */
+       fdm->metadata_region.request_flag = cpu_to_be32(FADUMP_REQUEST_FLAG);
+       fdm->metadata_region.source_data_type =
+                                       cpu_to_be16(FADUMP_REAL_MODE_REGION);
+       fdm->metadata_region.source_address = cpu_to_be64(metadata_base);
+       fdm->metadata_region.source_len = 0;
+       fdm->metadata_region.destination_address = cpu_to_be64(addr);
+       num_sections++;
+
+       fdm->header.dump_num_sections = cpu_to_be16(num_sections);
+       BUILD_BUG_ON(num_sections != FADUMP_NUM_SECTIONS);
        return addr;
  }

@@ -325,16 +375,17 @@ static unsigned long get_fadump_area_size(void)
        size += fw_dump.cpu_state_data_size;
        size += fw_dump.hpte_region_size;
        size += fw_dump.boot_memory_size;
-       size += sizeof(struct fadump_crash_info_header);
-       size += sizeof(struct elfhdr); /* ELF core header.*/
-       size += sizeof(struct elf_phdr); /* place holder for cpu notes */
-       /* Program headers for crash memory regions. */
-       size += sizeof(struct elf_phdr) * (memblock_num_regions(memory) + 2);
-
+       size += get_fadump_metadata_size();
        size = PAGE_ALIGN(size);
        return size;
  }

+static inline unsigned long get_fadump_metadata_base(
+                       const struct fadump_mem_struct *fdm)
+{
+       return be64_to_cpu(fdm->metadata_region.source_address);
+}
+
  int __init fadump_reserve_mem(void)
  {
        unsigned long base, size, memory_boundary;
@@ -395,9 +446,9 @@ int __init fadump_reserve_mem(void)
                                (unsigned long)(size >> 20),
                                (unsigned long)(base >> 20));

-               fw_dump.fadumphdr_addr =
-                               
be64_to_cpu(fdm_active->rmr_region.destination_address) +
-                               be64_to_cpu(fdm_active->rmr_region.source_len);
+               pr_info("Number of kernel Dump sections: %d\n",
+                       be16_to_cpu(fdm_active->header.dump_num_sections));
+               fw_dump.fadumphdr_addr = get_fadump_metadata_base(fdm_active);
                pr_debug("fadumphdr_addr = %p\n",
                                (void *) fw_dump.fadumphdr_addr);
        } else {
@@ -803,14 +854,24 @@ static int __init fadump_build_cpu_notes(const struct 
fadump_mem_struct *fdm)
  static int __init process_fadump(const struct fadump_mem_struct *fdm_active)
  {
        struct fadump_crash_info_header *fdh;
+       uint16_t num_sections = 0;
        int rc = 0;

        if (!fdm_active || !fw_dump.fadumphdr_addr)
                return -EINVAL;

+       /* Check if platform has honored all 4 dump sections requested. */
+       num_sections = be16_to_cpu(fdm_active->header.dump_num_sections);
+       if (num_sections < FADUMP_NUM_SECTIONS) {
+               printk(KERN_ERR "Dump taken by platform does not "
+                               "contain all requested sections\n");
+               return -EINVAL;
+       }
+
        /* Check if the dump data is valid. */
        if ((be16_to_cpu(fdm_active->header.dump_status_flag) == 
FADUMP_ERROR_FLAG) ||
                        (fdm_active->cpu_state_data.error_flags != 0) ||
+                       (fdm_active->metadata_region.error_flags != 0) ||
                        (fdm_active->rmr_region.error_flags != 0)) {
                printk(KERN_ERR "Dump taken by platform is not valid\n");
                return -EINVAL;
@@ -821,6 +882,11 @@ static int __init process_fadump(const struct 
fadump_mem_struct *fdm_active)
                printk(KERN_ERR "Dump taken by platform is incomplete\n");
                return -EINVAL;
        }
+       if ((fdm_active->metadata_region.bytes_dumped !=
+                       fdm_active->metadata_region.source_len)) {
+               printk(KERN_ERR "Dump taken by platform is incomplete\n");
+               return -EINVAL;
+       }

        /* Validate the fadump crash info header */
        fdh = __va(fw_dump.fadumphdr_addr);
@@ -1082,7 +1148,7 @@ static int register_fadump(void)

        fadump_setup_crash_memory_ranges();

-       addr = be64_to_cpu(fdm.rmr_region.destination_address) + 
be64_to_cpu(fdm.rmr_region.source_len);
+       addr = get_fadump_metadata_base(&fdm);
        /* Initialize fadump crash info header. */
        addr = init_fadump_header(addr);
        vaddr = __va(addr);
@@ -1153,7 +1219,7 @@ void fadump_cleanup(void)
        /* Invalidate the registration only if dump is active. */
        if (fw_dump.dump_active) {
                init_fadump_mem_struct(&fdm,
-                       
be64_to_cpu(fdm_active->cpu_state_data.destination_address));
+                               get_fadump_metadata_base(fdm_active));
                fadump_invalidate_dump(&fdm);
        }
  }
@@ -1236,7 +1302,7 @@ static void fadump_invalidate_release_mem(void)
                return;
        }

-       destination_address = 
be64_to_cpu(fdm_active->cpu_state_data.destination_address);
+       destination_address = get_fadump_metadata_base(fdm_active);
        fadump_cleanup();
        mutex_unlock(&fadump_mutex);



Reply via email to