Re: [PATCH v5 5/8] hw/ppc: Pass dump-sizes property for fadump in device tree

[Sourabh Jain <sourabhjain@linux.ibm.com>]

On 21/10/25 19:18, Aditya Gupta wrote:
> Platform (ie. QEMU) is expected to pass few device tree properties for
> details for fadump:
>
>    * "ibm,configure-kernel-dump-sizes": Space required to store dump data
>      for firmware provided dump sections (ie. CPU & HPTE regions)
>    * "ibm,configure-kernel-dump-version": Versions of fadump supported
>
> Pass the above device tree nodes so that kernel can reserve sufficient
> space for preserving the CPU state data
>
> Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
> ---
>   hw/ppc/spapr.c        | 57 +++++++++++++++++++++++++++++++++++++++++++
>   hw/ppc/spapr_fadump.c |  6 ++---
>   2 files changed, 60 insertions(+), 3 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 97ab6bebd25c..d40d5a9dcde5 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -900,6 +900,61 @@ static int spapr_dt_rng(void *fdt)
>       return ret ? -1 : 0;
>   }
>   
> +static void spapr_dt_rtas_fadump(SpaprMachineState *spapr, void *fdt, int rtas)
> +{
> +    MachineState *ms = MACHINE(spapr);
> +    MachineClass *mc = MACHINE_GET_CLASS(ms);
> +
> +    uint32_t max_possible_cpus = mc->possible_cpu_arch_ids(ms)->len;
> +    uint64_t fadump_cpu_state_size = 0;
> +    uint16_t fadump_versions[2] = {
> +        FADUMP_VERSION /* min supported version */,
> +        FADUMP_VERSION /* max supported version */
> +    };
> +    uint32_t fadump_rgn_sizes[2][3] = {
> +        {
> +            cpu_to_be32(FADUMP_CPU_STATE_DATA),
> +            0, 0 /* Calculated later */
> +        },
> +        {
> +            cpu_to_be32(FADUMP_HPTE_REGION),
> +            0, 0 /* HPTE region not implemented */
> +        }
> +    };
> +
> +    /*
> +     * CPU State Data contains multiple fields such as header, num_cpus and
> +     * register entries
> +     *
> +     * Calculate the maximum CPU State Data size, according to maximum
> +     * possible CPUs the QEMU VM can have
> +     *
> +     * This calculation must match the 'cpu_state_len' calculation done in
> +     * 'populate_cpu_state_data' in spapr_fadump.c
> +     */
> +    fadump_cpu_state_size += sizeof(struct FadumpRegSaveAreaHeader);
> +    fadump_cpu_state_size += 0xc;                      /* padding as in PAPR */
> +    fadump_cpu_state_size += sizeof(uint32_t);         /* num_cpus */
> +    fadump_cpu_state_size += max_possible_cpus   *     /* reg entries */
> +                             FADUMP_PER_CPU_REG_ENTRIES *
> +                             sizeof(struct FadumpRegEntry);
> +
> +    /* Set maximum size for CPU state data region */
> +    assert(fadump_rgn_sizes[0][0] == cpu_to_be32(FADUMP_CPU_STATE_DATA));
> +
> +    /* Upper 32 bits of size, usually 0 */
> +    fadump_rgn_sizes[0][1] = cpu_to_be32(fadump_cpu_state_size >> 32);
> +
> +    /* Lower 32 bits of size */
> +    fadump_rgn_sizes[0][2] = cpu_to_be32(fadump_cpu_state_size & 0xffffffff);
> +
> +    /* Add device tree properties required from platform for fadump */
> +    _FDT((fdt_setprop(fdt, rtas, "ibm,configure-kernel-dump-version",
> +                    fadump_versions, sizeof(fadump_versions))));
> +    _FDT((fdt_setprop(fdt, rtas, "ibm,configure-kernel-dump-sizes",
> +                    fadump_rgn_sizes, sizeof(fadump_rgn_sizes))));
> +}
> +
>   static void spapr_dt_rtas(SpaprMachineState *spapr, void *fdt)
>   {
>       MachineState *ms = MACHINE(spapr);
> @@ -1015,6 +1070,8 @@ static void spapr_dt_rtas(SpaprMachineState *spapr, void *fdt)
>       _FDT(fdt_setprop(fdt, rtas, "ibm,lrdr-capacity",
>                        lrdr_capacity, sizeof(lrdr_capacity)));
>   
> +    spapr_dt_rtas_fadump(spapr, fdt, rtas);
> +
>       spapr_dt_rtas_tokens(fdt, rtas);
>   }
>   
> diff --git a/hw/ppc/spapr_fadump.c b/hw/ppc/spapr_fadump.c
> index fdd49291483e..fa3aeac94cb7 100644
> --- a/hw/ppc/spapr_fadump.c
> +++ b/hw/ppc/spapr_fadump.c
> @@ -492,7 +492,7 @@ static void *get_cpu_state_data(uint64_t *cpu_state_len)
>       *cpu_state_len = 0;
>       *cpu_state_len += sizeof(reg_save_hdr);     /* reg save header */
>       *cpu_state_len += 0xc;                      /* padding as in PAPR */
> -    *cpu_state_len += sizeof(__be32);           /* num_cpus */
> +    *cpu_state_len += sizeof(num_cpus);         /* num_cpus */
>       *cpu_state_len += reg_entries_size;         /* reg entries */
>   
>       cpu_state_buffer = g_malloc(*cpu_state_len);
> @@ -503,8 +503,8 @@ static void *get_cpu_state_data(uint64_t *cpu_state_len)
>   
>       /* Write num_cpus */
>       num_cpus = cpu_to_be32(num_cpus);
> -    memcpy(cpu_state_buffer + offset, &num_cpus, sizeof(__be32));
> -    offset += sizeof(__be32);
> +    memcpy(cpu_state_buffer + offset, &num_cpus, sizeof(num_cpus));
> +    offset += sizeof(num_cpus);
>   
>       /* Write the register entries */
>       memcpy(cpu_state_buffer + offset, reg_entries, reg_entries_size);

LGTM
Reviewed-by: Sourabh Jain <sourabhjain@linux.ibm.com>