Re: [PATCH v3 1/7] powerpc/fadump: Move the metadata region to start of the reserved area.

2018-04-04 Thread Mahesh Jagannath Salgaonkar
On 04/04/2018 12:56 AM, Hari Bathini wrote:
> 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..?

Yeah we could do that. In ideal case production and capture kernel will
be same. But yes, there can be cases where we may end up in older/newer
kernel scenario. My earlier versions had backward compatibility which I
broke in v3. I can fix that in v4. Thanks for catching it. Will also
update documentation mentioning working kernel combinations.

Thanks,
-Mahesh.

> 
> Thanks
> Hari
> 
> 
> On Monday 02 April 2018 11:59 AM, Mahesh J Salgaonkar wrote:
>> From: Mahesh Salgaonkar 
>>
>> 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 
>> ---
>>   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(0x0001);
>> -    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 += 

Re: [PATCH v3 1/7] powerpc/fadump: Move the metadata region to start of the reserved area.

2018-04-03 Thread Hari Bathini
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 

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 
---
  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_SECTIONS4
+
  /* 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(0x0001);
-   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 

[PATCH v3 1/7] powerpc/fadump: Move the metadata region to start of the reserved area.

2018-04-02 Thread Mahesh J Salgaonkar
From: Mahesh Salgaonkar 

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 
---
 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_SECTIONS4
+
 /* 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(0x0001);
-   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);
+