[PATCH 3/6] hw/ppc: Preserve memory regions registered for fadump

Aditya Gupta posted 6 patches 11 months, 3 weeks ago
Maintainers: Nicholas Piggin <npiggin@gmail.com>, Daniel Henrique Barboza <danielhb413@gmail.com>, Harsh Prateek Bora <harshpb@linux.ibm.com>
There is a newer version of this series
[PATCH 3/6] hw/ppc: Preserve memory regions registered for fadump
Posted by Aditya Gupta 11 months, 3 weeks ago
While the first kernel boots, it registers memory regions for fadump
such as:
    * CPU state data  (has to be populated by the platform)
    * HPTE state data (has to be populated by the platform)
    * Real Mode Regions (platform should copy it to requested
      destination addresses)
    * OS defined regions (such as parameter save area)

Platform is also expected to modify the 'bytes_dumped' to the length of
data preserved/copied by platform (ideally same as the source length
passed by kernel).

The kernel passes source address and length for the memory regions, and
a destination address to where the memory is to be copied.

Implement the preserving/copying of the Real Mode Regions and the
Parameter Save Area in QEMU Pseries

Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
---
 hw/ppc/spapr_rtas.c    | 117 ++++++++++++++++++++++++++++++++++++++++-
 include/hw/ppc/spapr.h |  27 +++++++++-
 2 files changed, 142 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 01c82375f03d..9b29cadab2c9 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -344,6 +344,120 @@ static void rtas_ibm_set_system_parameter(PowerPCCPU *cpu,
 struct fadump_metadata fadump_metadata;
 bool is_next_boot_fadump;
 
+/* Preserve the memory locations registered for fadump */
+static bool fadump_preserve_mem(void)
+{
+    struct rtas_fadump_mem_struct *fdm = &fadump_metadata.registered_fdm;
+    uint64_t next_section_addr;
+    int dump_num_sections, data_type;
+    uint64_t src_addr, src_len, dest_addr;
+    void *copy_buffer;
+
+    assert(fadump_metadata.fadump_registered);
+    assert(fadump_metadata.fdm_addr != -1);
+
+    /* Read the fadump header passed during fadump registration */
+    cpu_physical_memory_read(fadump_metadata.fdm_addr,
+            &fdm->header, sizeof(fdm->header));
+
+    /* Verify that we understand the fadump header version */
+    if (fdm->header.dump_format_version != cpu_to_be32(FADUMP_VERSION)) {
+        /*
+         * Dump format version is unknown and likely changed from the time
+         * of fadump registration. Back out now.
+         */
+        return false;
+    }
+
+    dump_num_sections = be16_to_cpu(fdm->header.dump_num_sections);
+
+    if (dump_num_sections > FADUMP_MAX_SECTIONS) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+            "FADUMP: Too many sections: %d\n", fdm->header.dump_num_sections);
+        return false;
+    }
+
+    next_section_addr =
+        fadump_metadata.fdm_addr +
+        be32_to_cpu(fdm->header.offset_first_dump_section);
+
+    /*
+     * Handle all sections
+     *
+     * CPU State Data and HPTE regions are handled in their own cases
+     *
+     * RMR regions and any custom OS reserved regions such as parameter
+     * save area, are handled by simply copying the source region to
+     * destination address
+     */
+    for (int i = 0; i < dump_num_sections; ++i) {
+        /* Read the fadump section from memory */
+        cpu_physical_memory_read(next_section_addr,
+                &fdm->rgn[i], sizeof(fdm->rgn[i]));
+
+        next_section_addr += sizeof(fdm->rgn[i]);
+
+        data_type = be16_to_cpu(fdm->rgn[i].source_data_type);
+        src_addr  = be64_to_cpu(fdm->rgn[i].source_address);
+        src_len   = be64_to_cpu(fdm->rgn[i].source_len);
+        dest_addr = be64_to_cpu(fdm->rgn[i].destination_address);
+
+        /* Reset error_flags & bytes_dumped for now */
+        fdm->rgn[i].error_flags = 0;
+        fdm->rgn[i].bytes_dumped = 0;
+
+        if (be32_to_cpu(fdm->rgn[i].request_flag) != FADUMP_REQUEST_FLAG) {
+            qemu_log_mask(LOG_UNIMP,
+                "FADUMP: Skipping copying region as not requested\n");
+            continue;
+        }
+
+        switch (data_type) {
+        case FADUMP_CPU_STATE_DATA:
+            /* TODO: Add CPU state data */
+            break;
+        case FADUMP_HPTE_REGION:
+            /* TODO: Add hpte state data */
+            break;
+        case FADUMP_REAL_MODE_REGION:
+        case FADUMP_PARAM_AREA:
+            /* Skip copy if source and destination are same (eg. param area) */
+            if (src_addr != dest_addr) {
+                copy_buffer = g_malloc(src_len + 1);
+                if (copy_buffer == NULL) {
+                    qemu_log_mask(LOG_GUEST_ERROR,
+                        "FADUMP: Failed allocating memory for copying reserved memory regions\n");
+                    fdm->rgn[i].error_flags =
+                        cpu_to_be16(FADUMP_ERROR_LENGTH_EXCEEDS_SOURCE);
+
+                    continue;
+                }
+
+                /* Copy the source region to destination */
+                cpu_physical_memory_read(src_addr, copy_buffer, src_len);
+                cpu_physical_memory_write(dest_addr, copy_buffer, src_len);
+                g_free(copy_buffer);
+            }
+
+            /*
+             * Considering cpu_physical_memory_write would have copied the
+             * complete region
+             */
+            fdm->rgn[i].bytes_dumped = cpu_to_be64(src_len);
+
+            break;
+        default:
+            qemu_log_mask(LOG_GUEST_ERROR,
+                "FADUMP: Skipping unknown source data type: %d\n", data_type);
+
+            fdm->rgn[i].error_flags =
+                cpu_to_be16(FADUMP_ERROR_INVALID_DATA_TYPE);
+        }
+    }
+
+    return true;
+}
+
 static void trigger_fadump_boot(target_ulong spapr_retcode)
 {
     /*
@@ -353,7 +467,8 @@ static void trigger_fadump_boot(target_ulong spapr_retcode)
      */
     pause_all_vcpus();
 
-    if (true /* TODO: Preserve memory registered for fadump */) {
+    /* Preserve the memory locations registered for fadump */
+    if (!fadump_preserve_mem()) {
         /* Failed to preserve the registered memory regions */
         rtas_st(spapr_retcode, 0, RTAS_OUT_HW_ERROR);
 
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index efa2f891a8a7..a80704187583 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -776,7 +776,32 @@ void push_sregs_to_kvm_pr(SpaprMachineState *spapr);
 #define FADUMP_CMD_UNREGISTER          2
 #define FADUMP_CMD_INVALIDATE          3
 
-#define FADUMP_VERSION    1
+#define FADUMP_VERSION                 1
+
+/*
+ * The Firmware Assisted Dump Memory structure supports a maximum of 10 sections
+ * in the dump memory structure. Presently, three sections are used for
+ * CPU state data, HPTE & Parameters area, while the remaining seven sections
+ * can be used for boot memory regions.
+ */
+#define FADUMP_MAX_SECTIONS            10
+#define RTAS_FADUMP_MAX_BOOT_MEM_REGS  7
+
+/* Firmware provided dump sections */
+#define FADUMP_CPU_STATE_DATA   0x0001
+#define FADUMP_HPTE_REGION      0x0002
+#define FADUMP_REAL_MODE_REGION 0x0011
+
+/* OS defined sections */
+#define FADUMP_PARAM_AREA       0x0100
+
+/* Dump request flag */
+#define FADUMP_REQUEST_FLAG     0x00000001
+
+/* Dump status flag */
+#define FADUMP_ERROR_INVALID_DATA_TYPE          0x8000
+#define FADUMP_ERROR_INVALID_SOURCE_ADDR        0x4000
+#define FADUMP_ERROR_LENGTH_EXCEEDS_SOURCE      0x2000
 
 /*
  * The Firmware Assisted Dump Memory structure supports a maximum of 10 sections
-- 
2.48.1
Re: [PATCH 3/6] hw/ppc: Preserve memory regions registered for fadump
Posted by Harsh Prateek Bora 11 months, 1 week ago

On 2/17/25 12:47, Aditya Gupta wrote:
> While the first kernel boots, it registers memory regions for fadump
> such as:
>      * CPU state data  (has to be populated by the platform)
>      * HPTE state data (has to be populated by the platform)
>      * Real Mode Regions (platform should copy it to requested
>        destination addresses)
>      * OS defined regions (such as parameter save area)
> 
> Platform is also expected to modify the 'bytes_dumped' to the length of
> data preserved/copied by platform (ideally same as the source length
> passed by kernel).
> 
> The kernel passes source address and length for the memory regions, and
> a destination address to where the memory is to be copied.
> 
> Implement the preserving/copying of the Real Mode Regions and the
> Parameter Save Area in QEMU Pseries
> 
> Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
> ---
>   hw/ppc/spapr_rtas.c    | 117 ++++++++++++++++++++++++++++++++++++++++-
>   include/hw/ppc/spapr.h |  27 +++++++++-
>   2 files changed, 142 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 01c82375f03d..9b29cadab2c9 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -344,6 +344,120 @@ static void rtas_ibm_set_system_parameter(PowerPCCPU *cpu,
>   struct fadump_metadata fadump_metadata;
>   bool is_next_boot_fadump;
>   
> +/* Preserve the memory locations registered for fadump */
> +static bool fadump_preserve_mem(void)
> +{
> +    struct rtas_fadump_mem_struct *fdm = &fadump_metadata.registered_fdm;
> +    uint64_t next_section_addr;
> +    int dump_num_sections, data_type;
> +    uint64_t src_addr, src_len, dest_addr;
> +    void *copy_buffer;
> +
> +    assert(fadump_metadata.fadump_registered);
> +    assert(fadump_metadata.fdm_addr != -1);
> +
> +    /* Read the fadump header passed during fadump registration */
> +    cpu_physical_memory_read(fadump_metadata.fdm_addr,
> +            &fdm->header, sizeof(fdm->header));
> +
> +    /* Verify that we understand the fadump header version */
> +    if (fdm->header.dump_format_version != cpu_to_be32(FADUMP_VERSION)) {
> +        /*
> +         * Dump format version is unknown and likely changed from the time
> +         * of fadump registration. Back out now.
> +         */
> +        return false;
> +    }
> +
> +    dump_num_sections = be16_to_cpu(fdm->header.dump_num_sections);
> +
> +    if (dump_num_sections > FADUMP_MAX_SECTIONS) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +            "FADUMP: Too many sections: %d\n", fdm->header.dump_num_sections);
> +        return false;
> +    }
> +
> +    next_section_addr =
> +        fadump_metadata.fdm_addr +
> +        be32_to_cpu(fdm->header.offset_first_dump_section);
> +
> +    /*
> +     * Handle all sections
> +     *
> +     * CPU State Data and HPTE regions are handled in their own cases
> +     *
> +     * RMR regions and any custom OS reserved regions such as parameter
> +     * save area, are handled by simply copying the source region to
> +     * destination address
> +     */
> +    for (int i = 0; i < dump_num_sections; ++i) {
> +        /* Read the fadump section from memory */
> +        cpu_physical_memory_read(next_section_addr,
> +                &fdm->rgn[i], sizeof(fdm->rgn[i]));
> +
> +        next_section_addr += sizeof(fdm->rgn[i]);
> +
> +        data_type = be16_to_cpu(fdm->rgn[i].source_data_type);
> +        src_addr  = be64_to_cpu(fdm->rgn[i].source_address);
> +        src_len   = be64_to_cpu(fdm->rgn[i].source_len);
> +        dest_addr = be64_to_cpu(fdm->rgn[i].destination_address);
> +
> +        /* Reset error_flags & bytes_dumped for now */
> +        fdm->rgn[i].error_flags = 0;
> +        fdm->rgn[i].bytes_dumped = 0;
> +
> +        if (be32_to_cpu(fdm->rgn[i].request_flag) != FADUMP_REQUEST_FLAG) {
> +            qemu_log_mask(LOG_UNIMP,
> +                "FADUMP: Skipping copying region as not requested\n");
> +            continue;
> +        }
> +
> +        switch (data_type) {
> +        case FADUMP_CPU_STATE_DATA:
> +            /* TODO: Add CPU state data */
> +            break;
> +        case FADUMP_HPTE_REGION:
> +            /* TODO: Add hpte state data */
> +            break;
> +        case FADUMP_REAL_MODE_REGION:
> +        case FADUMP_PARAM_AREA:
> +            /* Skip copy if source and destination are same (eg. param area) */
> +            if (src_addr != dest_addr) {
> +                copy_buffer = g_malloc(src_len + 1);
> +                if (copy_buffer == NULL) {
> +                    qemu_log_mask(LOG_GUEST_ERROR,
> +                        "FADUMP: Failed allocating memory for copying reserved memory regions\n");
> +                    fdm->rgn[i].error_flags =
> +                        cpu_to_be16(FADUMP_ERROR_LENGTH_EXCEEDS_SOURCE);
> +
> +                    continue;
> +                }
> +
> +                /* Copy the source region to destination */
> +                cpu_physical_memory_read(src_addr, copy_buffer, src_len);
> +                cpu_physical_memory_write(dest_addr, copy_buffer, src_len);
> +                g_free(copy_buffer);
> +            }
> +
> +            /*
> +             * Considering cpu_physical_memory_write would have copied the
> +             * complete region
> +             */
> +            fdm->rgn[i].bytes_dumped = cpu_to_be64(src_len);

Is this really valid for FADUMP_PARAM_AREA where we intend to skip copy?

> +
> +            break;
> +        default:
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                "FADUMP: Skipping unknown source data type: %d\n", data_type);
> +
> +            fdm->rgn[i].error_flags =
> +                cpu_to_be16(FADUMP_ERROR_INVALID_DATA_TYPE);
> +        }
> +    }
> +
> +    return true;
> +}
> +
>   static void trigger_fadump_boot(target_ulong spapr_retcode)
>   {
>       /*
> @@ -353,7 +467,8 @@ static void trigger_fadump_boot(target_ulong spapr_retcode)
>        */
>       pause_all_vcpus();
>   
> -    if (true /* TODO: Preserve memory registered for fadump */) {
> +    /* Preserve the memory locations registered for fadump */
> +    if (!fadump_preserve_mem()) {

This change can be avoided as suggested in previous patch.

>           /* Failed to preserve the registered memory regions */
>           rtas_st(spapr_retcode, 0, RTAS_OUT_HW_ERROR);
>   
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index efa2f891a8a7..a80704187583 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -776,7 +776,32 @@ void push_sregs_to_kvm_pr(SpaprMachineState *spapr);
>   #define FADUMP_CMD_UNREGISTER          2
>   #define FADUMP_CMD_INVALIDATE          3
>   
> -#define FADUMP_VERSION    1
> +#define FADUMP_VERSION                 1

This change can be avoided if taken care initially.

Thanks
Harsh
> +
> +/*
> + * The Firmware Assisted Dump Memory structure supports a maximum of 10 sections
> + * in the dump memory structure. Presently, three sections are used for
> + * CPU state data, HPTE & Parameters area, while the remaining seven sections
> + * can be used for boot memory regions.
> + */
> +#define FADUMP_MAX_SECTIONS            10
> +#define RTAS_FADUMP_MAX_BOOT_MEM_REGS  7
> +
> +/* Firmware provided dump sections */
> +#define FADUMP_CPU_STATE_DATA   0x0001
> +#define FADUMP_HPTE_REGION      0x0002
> +#define FADUMP_REAL_MODE_REGION 0x0011
> +
> +/* OS defined sections */
> +#define FADUMP_PARAM_AREA       0x0100
> +
> +/* Dump request flag */
> +#define FADUMP_REQUEST_FLAG     0x00000001
> +
> +/* Dump status flag */
> +#define FADUMP_ERROR_INVALID_DATA_TYPE          0x8000
> +#define FADUMP_ERROR_INVALID_SOURCE_ADDR        0x4000
> +#define FADUMP_ERROR_LENGTH_EXCEEDS_SOURCE      0x2000
>   
>   /*
>    * The Firmware Assisted Dump Memory structure supports a maximum of 10 sections
Re: [PATCH 3/6] hw/ppc: Preserve memory regions registered for fadump
Posted by Aditya Gupta 11 months, 1 week ago
On 05/03/25 12:10, Harsh Prateek Bora wrote:

>
>
> On 2/17/25 12:47, Aditya Gupta wrote:
>> <...snip...>
>>
>> +        /* Reset error_flags & bytes_dumped for now */
>> +        fdm->rgn[i].error_flags = 0;
>> +        fdm->rgn[i].bytes_dumped = 0;
>> +
>> +        if (be32_to_cpu(fdm->rgn[i].request_flag) != 
>> FADUMP_REQUEST_FLAG) {
>> +            qemu_log_mask(LOG_UNIMP,
>> +                "FADUMP: Skipping copying region as not requested\n");
>> +            continue;
>> +        }
>> +
>> +        switch (data_type) {
>> +        case FADUMP_CPU_STATE_DATA:
>> +            /* TODO: Add CPU state data */
>> +            break;
>> +        case FADUMP_HPTE_REGION:
>> +            /* TODO: Add hpte state data */
>> +            break;
>> +        case FADUMP_REAL_MODE_REGION:
>> +        case FADUMP_PARAM_AREA:
>> +            /* Skip copy if source and destination are same (eg. 
>> param area) */
>> +            if (src_addr != dest_addr) {
>> +                copy_buffer = g_malloc(src_len + 1);
>> +                if (copy_buffer == NULL) {
>> +                    qemu_log_mask(LOG_GUEST_ERROR,
>> +                        "FADUMP: Failed allocating memory for 
>> copying reserved memory regions\n");
>> +                    fdm->rgn[i].error_flags =
>> + cpu_to_be16(FADUMP_ERROR_LENGTH_EXCEEDS_SOURCE);
>> +
>> +                    continue;
>> +                }
>> +
>> +                /* Copy the source region to destination */
>> +                cpu_physical_memory_read(src_addr, copy_buffer, 
>> src_len);
>> +                cpu_physical_memory_write(dest_addr, copy_buffer, 
>> src_len);
>> +                g_free(copy_buffer);
>> +            }
>> +
>> +            /*
>> +             * Considering cpu_physical_memory_write would have 
>> copied the
>> +             * complete region
>> +             */
>> +            fdm->rgn[i].bytes_dumped = cpu_to_be64(src_len);
>
> Is this really valid for FADUMP_PARAM_AREA where we intend to skip copy?
>
Yes I think it's good to keep it. Because that's an optimisation i did 
to skip the copy if src and dest are same.

But the actual copy depends on the OS passing us the 
"FADUMP_REQUEST_FLAG" in the region's request flag.

If the flag is set, i am expecting the kernel asked us to copy, and 
hence the .bytes_dumped should be same as the number of bytes asked by 
kernel to copy ideally.

If the flag is not set, we return early, so we let the .bytes_dumped be 0.

>> +
>> +            break;
>> +        default:
>> +            qemu_log_mask(LOG_GUEST_ERROR,
>> +                "FADUMP: Skipping unknown source data type: %d\n", 
>> data_type);
>> +
>> +            fdm->rgn[i].error_flags =
>> +                cpu_to_be16(FADUMP_ERROR_INVALID_DATA_TYPE);
>> +        }
>> +    }
>> +
>> +    return true;
>> +}
>> +
>>   static void trigger_fadump_boot(target_ulong spapr_retcode)
>>   {
>>       /*
>> @@ -353,7 +467,8 @@ static void trigger_fadump_boot(target_ulong 
>> spapr_retcode)
>>        */
>>       pause_all_vcpus();
>>   -    if (true /* TODO: Preserve memory registered for fadump */) {
>> +    /* Preserve the memory locations registered for fadump */
>> +    if (!fadump_preserve_mem()) {
>
> This change can be avoided as suggested in previous patch.
Agreed, will do it in previous patch.
>
>>           /* Failed to preserve the registered memory regions */
>>           rtas_st(spapr_retcode, 0, RTAS_OUT_HW_ERROR);
>>   diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>> index efa2f891a8a7..a80704187583 100644
>> --- a/include/hw/ppc/spapr.h
>> +++ b/include/hw/ppc/spapr.h
>> @@ -776,7 +776,32 @@ void push_sregs_to_kvm_pr(SpaprMachineState 
>> *spapr);
>>   #define FADUMP_CMD_UNREGISTER          2
>>   #define FADUMP_CMD_INVALIDATE          3
>>   -#define FADUMP_VERSION    1
>> +#define FADUMP_VERSION                 1
>
> This change can be avoided if taken care initially.

Nice, I missed this diff. Will fix it in the patch which introduced this.


Thanks,

- Aditya Gupta

>
> Thanks
> Harsh
>> +
>> +/*
>> + * The Firmware Assisted Dump Memory structure supports a maximum of 
>> 10 sections
>> + * in the dump memory structure. Presently, three sections are used for
>> + * CPU state data, HPTE & Parameters area, while the remaining seven 
>> sections
>> + * can be used for boot memory regions.
>> + */
>> +#define FADUMP_MAX_SECTIONS            10
>> +#define RTAS_FADUMP_MAX_BOOT_MEM_REGS  7
>> +
>> +/* Firmware provided dump sections */
>> +#define FADUMP_CPU_STATE_DATA   0x0001
>> +#define FADUMP_HPTE_REGION      0x0002
>> +#define FADUMP_REAL_MODE_REGION 0x0011
>> +
>> +/* OS defined sections */
>> +#define FADUMP_PARAM_AREA       0x0100
>> +
>> +/* Dump request flag */
>> +#define FADUMP_REQUEST_FLAG     0x00000001
>> +
>> +/* Dump status flag */
>> +#define FADUMP_ERROR_INVALID_DATA_TYPE          0x8000
>> +#define FADUMP_ERROR_INVALID_SOURCE_ADDR        0x4000
>> +#define FADUMP_ERROR_LENGTH_EXCEEDS_SOURCE      0x2000
>>     /*
>>    * The Firmware Assisted Dump Memory structure supports a maximum 
>> of 10 sections