Re: [Crash-utility] [PATCH v1] diskdump: add hook for additional checks on prstatus notes validity

Hi Lianbo, I have sent the V2 now. Got delayed due to me earlier not being able to test the V2.

On Tue, Sep 26, 2023 at 11:55:06AM +0530, Aditya Gupta wrote: > On Tue, Sep 26, 2023 at 01:55:32PM +0800, lijiang wrote: > > > > > > ... > > > > > > diff --git a/ppc64.c b/ppc64.c > > > index fc34006f4863..1159b8c3a8e7 100644 > > > --- a/ppc64.c > > > +++ b/ppc64.c > > > @@ -298,6 +298,15 @@ struct machine_specific book3e_machine_specific = { > > > .is_vmaddr = book3e_is_vmaddr, > > > }; > > > > > > +/** > > > + * No additional checks are required on PPC64, for checking if PRSTATUS > > > notes > > > + * is valid > > > + */ > > > +int ppc64_is_cpu_prstatus_valid(int cpu) > > > +{ > > > + return TRUE; > > > +} > > > + > > > #define SKIBOOT_BASE 0x30000000 > > > > > > /* > > > @@ -418,6 +427,7 @@ ppc64_init(int when) > > > break; > > > > > > case POST_GDB: > > > + machdep->is_cpu_prstatus_valid = > > > ppc64_is_cpu_prstatus_valid; > > > > > > > The hook is set in the stage of POST_GDB, I'm wondering if the current > > warning is still shown in the crash minimal mode(with option --minimal). > > Can you help to confirm this one? > > Sure, will check this. Just looked at it, seems the warning might still be there, > if it is minimal mode. > Basically what I wanted is, this machdep->is_cpu_prstatus_valid to be > overwritten, after diskdump_init has run (which sets machdep->is_cpu_prstatus_valid > to a default), and before 'map_cpus_to_prstatus_kdump_cmprs' (where it is used), > will see if the warning comes, will try to understand the flow and move the > code accordingly. > I tested this also. The warning is not shown in minimal mode. Still, your suggestion was right logically, moved it to PRE_GDB stage in V2, since then we don't have this ambiguity of the 'is_cpu_prstatus_valid' hook being set or not. Now, it will be correctly initialised in all cases.

Thanks, - Aditya Gupta