Hello Sourabh,
On 25/10/18 05:34PM, Sourabh Jain wrote:
>
>
> > <...snip...>
> > The fadump boot after crash:
> >
> > [ 0.000000] rtas fadump: Firmware-assisted dump is active.
> > [ 0.000000] fadump: Updated cmdline: debug fadump=on crashkernel=1G
> > [ 0.000000] fadump: Firmware-assisted dump is active.
>
> Kernel is printing twice about fadump is active. :)
Yeah i noticed, that seems to be the case atleast with 6.14
>
> > <...snip...>
> > MachineState *ms = MACHINE(spapr);
> > MachineClass *mc = MACHINE_GET_CLASS(ms);
> > + FadumpMemStruct *fdm = &spapr->registered_fdm;
> > + uint16_t dump_status_flag;
> > + bool is_next_boot_fadump;
> > uint32_t max_possible_cpus = mc->possible_cpu_arch_ids(ms)->len;
> > uint64_t fadump_cpu_state_size = 0;
> > @@ -953,6 +956,18 @@ static void spapr_dt_rtas_fadump(SpaprMachineState
> > *spapr, void *fdt, int rtas)
> > fadump_versions, sizeof(fadump_versions))));
> > _FDT((fdt_setprop(fdt, rtas, "ibm,configure-kernel-dump-sizes",
> > fadump_rgn_sizes, sizeof(fadump_rgn_sizes))));
> > +
> > + dump_status_flag = be16_to_cpu(fdm->header.dump_status_flag);
> > + is_next_boot_fadump =
>
> Do we really need is_next_boot_fadump variable?
Agreed, not needed now, will remove it in v5.
>
> > + (dump_status_flag & FADUMP_STATUS_DUMP_TRIGGERED) != 0;
> > + if (is_next_boot_fadump) {
> > + uint64_t fdm_size =
> > + sizeof(struct FadumpSectionHeader) +
> > + (be16_to_cpu(fdm->header.dump_num_sections) *
> > + sizeof(struct FadumpSection));
> > +
> > + _FDT((fdt_setprop(fdt, rtas, "ibm,kernel-dump", fdm, fdm_size)));
>
> Is this common in QEMU to call _FDT to add prop to the FDT? Feels odd.
Yes, that's just a wrapper for error checking in QEMU, used extensively
atleast in hw/ppc
Thanks for your reviews Sourabh !
- Aditya G
>
> > + }
> > }
> > static void spapr_dt_rtas(SpaprMachineState *spapr, void *fdt)
>
