12:55 a.m.
Hi, Aditya
Thank you for the fix.
On Fri, Sep 22, 2023 at 7:03 AM <crash-utility-request(a)redhat.com> wrote:
Date: Thu, 21 Sep 2023 12:29:05 +0530
From: Aditya Gupta <adityag(a)linux.ibm.com>
To: crash-utility(a)redhat.com
Cc: Hari Bathini <hbathini(a)linux.ibm.com>, Mahesh J Salgaonkar
<mahesh(a)linux.ibm.com>, Sourabh Jain <sourabhjain(a)linux.ibm.com>,
d.hatayama(a)fujitsu.com
Subject: [Crash-utility] [PATCH v1] diskdump: add hook for additional
checks on prstatus notes validity
Message-ID: <20230921065905.1020839-1-adityag(a)linux.ibm.com>
Content-Type: text/plain; charset="US-ASCII"; x-default=true
Upstream crash reports these warnings on PowerPC64:
WARNING: cpu 0 invalid NT_PRSTATUS note (n_type != NT_PRSTATUS)
...
Apart from these warnings, register values are also invalid.
This warning was found in the commit:
commit db8c030857b4 ("diskdump/netdump: fix segmentation fault
caused by failure of stopping CPUs")
With above commit, crash checks whether 'crash_notes' is initialised,
before mapping PRSTATUS notes.
But some architectures such as PowerPC64, in fadump case
(firmware-assisted dump), don't populate 'crash_notes' since the
registers are already stored in the cpu notes in the vmcore.
Instead of checking 'crash_notes' for all architectures, introduce
a machdep hook ('is_cpu_prstatus_valid'), for architectures to
decide validity checks for PRSTATUS notes
A default hook ('diskdump_is_cpu_prstatus_valid') has also been provided
for all architectures other than PowerPC64, which checks if 'crash_notes'
for a given cpu is valid, maintaining the current behaviour
PowerPC64 doesn't utilise 'crash_notes' to get register values, so no
additional checks are required
Fixes: db8c030857b4 ("diskdump/netdump: fix segmentation fault caused by
failure of stopping CPUs")
Signed-off-by: Aditya Gupta <adityag(a)linux.ibm.com>
---
Testing
=======
NOTE: To test this on PowerPC64 with upstream kernel dump, AND on system
with Radix MMU, following patch will also be needed to be applied:
Link:
https://listman.redhat.com/archives/crash-utility/2023-September/010961.html
This is due to change in vmemmap address mapping for Radix MMU, since
following patch in the kernel:
368a0590d954: (powerpc/book3s64/vmemmap: switch radix to use a
different vmemmap handling function)
More details about the change are in the linked patch. Basically what
changed is, the address mapping for vmemmap address is now in kernel
page table, in case of Radix MMU, instead of 'vmemmap_list' which is
currently
used in crash.
Git Tree for Testing
====================
1. With this patch (diskdump: add hook for additional ...) applied:
https://github.com/adi-g15-ibm/crash/tree/bugzilla-203256-list-v1
2. With both this and the linked patch (ppc64: do page traversal ...)
applied:
https://github.com/adi-g15-ibm/crash/tree/bugzilla-203256-withupstreamradix
---
---
defs.h | 1 +
diskdump.c | 15 ++++++++++++---
ppc64.c | 10 ++++++++++
3 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/defs.h b/defs.h
index 96a7a2a31471..f7f56947e5ac 100644
--- a/defs.h
+++ b/defs.h
@@ -1073,6 +1073,7 @@ struct machdep_table {
int (*verify_line_number)(ulong, ulong, ulong);
void (*get_irq_affinity)(int);
void (*show_interrupts)(int, ulong *);
+ int (*is_cpu_prstatus_valid)(int cpu);
I would suggest putting it at the end of this table. Although it may not
break the compatibility of the extension module, just like the
offset_table/size_table, I get used to doing that if there is no special
reason.
int (*is_page_ptr)(ulong, physaddr_t *);
int (*get_cpu_reg)(int, int, const char *, int, void *);
};
diff --git a/diskdump.c b/diskdump.c
index 2c284ff3f97f..ad9a00b08ce1 100644
--- a/diskdump.c
+++ b/diskdump.c
@@ -142,13 +142,22 @@ int have_crash_notes(int cpu)
return TRUE;
}
+int diskdump_is_cpu_prstatus_valid(int cpu)
+{
+ static int crash_notes_exists = -1;
+
+ if (crash_notes_exists == -1)
+ crash_notes_exists = kernel_symbol_exists("crash_notes");
+
+ return (!crash_notes_exists || have_crash_notes(cpu));
+}
+
Got a warning as below:
cc -c -g -DX86_64 -DLZO -DGDB_10_2 diskdump.c -Wall -O2
-Wstrict-prototypes -Wmissing-prototypes -fstack-protector
-Wformat-security
diskdump.c:145:5: warning: no previous prototype for
‘diskdump_is_cpu_prstatus_valid’ [-Wmissing-prototypes]
145 | int diskdump_is_cpu_prstatus_valid(int cpu)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
void
map_cpus_to_prstatus_kdump_cmprs(void)
{
void **nt_ptr;
int online, i, j, nrcpus;
size_t size;
- int crash_notes_exists;
if (pc->flags2 & QEMU_MEM_DUMP_COMPRESSED) /* notes exist for all
cpus */
goto resize_note_pointers;
@@ -171,10 +180,9 @@ map_cpus_to_prstatus_kdump_cmprs(void)
* Re-populate the array with the notes mapping to online cpus
*/
nrcpus = (kt->kernel_NR_CPUS ? kt->kernel_NR_CPUS : NR_CPUS);
- crash_notes_exists = kernel_symbol_exists("crash_notes");
for (i = 0, j = 0; i < nrcpus; i++) {
- if (in_cpu_map(ONLINE_MAP, i) && (!crash_notes_exists ||
have_crash_notes(i))) {
+ if (in_cpu_map(ONLINE_MAP, i) &&
machdep->is_cpu_prstatus_valid(i)) {
dd->nt_prstatus_percpu[i] = nt_ptr[j++];
dd->num_prstatus_notes =
MAX(dd->num_prstatus_notes, i+1);
@@ -1076,6 +1084,7 @@ diskdump_init(char *unused, FILE *fptr)
if (!DISKDUMP_VALID() && !KDUMP_CMPRS_VALID())
return FALSE;
+ machdep->is_cpu_prstatus_valid = diskdump_is_cpu_prstatus_valid;
dd->ofp = fptr;
return TRUE;
}
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?
And other changes are fine to me.
Thanks.
Lianbo
ms = machdep->machspec;
if (!(machdep->flags & BOOK3E)) {
--
2.41.0
1:25 a.m.
New subject: [PATCH v1] diskdump: add hook for additional checks on prstatus notes validity
Hi Lianbo,
Thanks for the reviews.
On Tue, Sep 26, 2023 at 01:55:32PM +0800, lijiang wrote:
Hi, Aditya
Thank you for the fix.
On Fri, Sep 22, 2023 at 7:03 AM <crash-utility-request(a)redhat.com> wrote:
> Date: Thu, 21 Sep 2023 12:29:05 +0530
> From: Aditya Gupta <adityag(a)linux.ibm.com>
> To: crash-utility(a)redhat.com
> Cc: Hari Bathini <hbathini(a)linux.ibm.com>, Mahesh J Salgaonkar
> <mahesh(a)linux.ibm.com>, Sourabh Jain
<sourabhjain(a)linux.ibm.com>,
> d.hatayama(a)fujitsu.com
> Subject: [Crash-utility] [PATCH v1] diskdump: add hook for additional
> checks on prstatus notes validity
> Message-ID: <20230921065905.1020839-1-adityag(a)linux.ibm.com>
> Content-Type: text/plain; charset="US-ASCII"; x-default=true
>
> Upstream crash reports these warnings on PowerPC64:
>
> WARNING: cpu 0 invalid NT_PRSTATUS note (n_type != NT_PRSTATUS)
> ...
>
> Apart from these warnings, register values are also invalid.
>
> This warning was found in the commit:
>
> commit db8c030857b4 ("diskdump/netdump: fix segmentation fault
> caused by failure of stopping CPUs")
>
> With above commit, crash checks whether 'crash_notes' is initialised,
> before mapping PRSTATUS notes.
>
> But some architectures such as PowerPC64, in fadump case
> (firmware-assisted dump), don't populate 'crash_notes' since the
> registers are already stored in the cpu notes in the vmcore.
>
> Instead of checking 'crash_notes' for all architectures, introduce
> a machdep hook ('is_cpu_prstatus_valid'), for architectures to
> decide validity checks for PRSTATUS notes
>
> A default hook ('diskdump_is_cpu_prstatus_valid') has also been provided
> for all architectures other than PowerPC64, which checks if 'crash_notes'
> for a given cpu is valid, maintaining the current behaviour
>
> PowerPC64 doesn't utilise 'crash_notes' to get register values, so no
> additional checks are required
>
> ...
>
> ---
> ---
> defs.h | 1 +
> diskdump.c | 15 ++++++++++++---
> ppc64.c | 10 ++++++++++
> 3 files changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/defs.h b/defs.h
> index 96a7a2a31471..f7f56947e5ac 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -1073,6 +1073,7 @@ struct machdep_table {
> int (*verify_line_number)(ulong, ulong, ulong);
> void (*get_irq_affinity)(int);
> void (*show_interrupts)(int, ulong *);
> + int (*is_cpu_prstatus_valid)(int cpu);
>
I would suggest putting it at the end of this table. Although it may not
break the compatibility of the extension module, just like the
offset_table/size_table, I get used to doing that if there is no special
reason.
Sure, will move it to the end of the table. I did not think of the compatibility
issue, thanks.
> int (*is_page_ptr)(ulong, physaddr_t *);
> int (*get_cpu_reg)(int, int, const char *, int, void *);
> };
> diff --git a/diskdump.c b/diskdump.c
> index 2c284ff3f97f..ad9a00b08ce1 100644
> --- a/diskdump.c
> +++ b/diskdump.c
> @@ -142,13 +142,22 @@ int have_crash_notes(int cpu)
> return TRUE;
> }
>
> +int diskdump_is_cpu_prstatus_valid(int cpu)
> +{
> + static int crash_notes_exists = -1;
> +
> + if (crash_notes_exists == -1)
> + crash_notes_exists = kernel_symbol_exists("crash_notes");
> +
> + return (!crash_notes_exists || have_crash_notes(cpu));
> +}
> +
>
Got a warning as below:
cc -c -g -DX86_64 -DLZO -DGDB_10_2 diskdump.c -Wall -O2
-Wstrict-prototypes -Wmissing-prototypes -fstack-protector
-Wformat-security
diskdump.c:145:5: warning: no previous prototype for
‘diskdump_is_cpu_prstatus_valid’ [-Wmissing-prototypes]
145 | int diskdump_is_cpu_prstatus_valid(int cpu)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Will fix in V2. Will add a declaration in defs.h, just above 'have_crash_notes'
declaration, or should I add the declaration at the end of all diskdump.c
declarations in defs.h ?
> void
> map_cpus_to_prstatus_kdump_cmprs(void)
> {
> void **nt_ptr;
> int online, i, j, nrcpus;
> size_t size;
> - int crash_notes_exists;
>
> if (pc->flags2 & QEMU_MEM_DUMP_COMPRESSED) /* notes exist for all
> cpus */
> goto resize_note_pointers;
> @@ -171,10 +180,9 @@ map_cpus_to_prstatus_kdump_cmprs(void)
> * Re-populate the array with the notes mapping to online cpus
> */
> nrcpus = (kt->kernel_NR_CPUS ? kt->kernel_NR_CPUS : NR_CPUS);
> - crash_notes_exists = kernel_symbol_exists("crash_notes");
>
> for (i = 0, j = 0; i < nrcpus; i++) {
> - if (in_cpu_map(ONLINE_MAP, i) && (!crash_notes_exists ||
> have_crash_notes(i))) {
> + if (in_cpu_map(ONLINE_MAP, i) &&
> machdep->is_cpu_prstatus_valid(i)) {
> dd->nt_prstatus_percpu[i] = nt_ptr[j++];
> dd->num_prstatus_notes =
> MAX(dd->num_prstatus_notes, i+1);
> @@ -1076,6 +1084,7 @@ diskdump_init(char *unused, FILE *fptr)
> if (!DISKDUMP_VALID() && !KDUMP_CMPRS_VALID())
> return FALSE;
>
> + machdep->is_cpu_prstatus_valid = diskdump_is_cpu_prstatus_valid;
> dd->ofp = fptr;
> return TRUE;
> }
> 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.
And other changes are fine to me.
Thanks,
- Aditya Gupta
Thanks.
Lianbo
ms = machdep->machspec;
>
> if (!(machdep->flags & BOOK3E)) {
> --
> 2.41.0
>
2:51 a.m.
New subject: [PATCH v1] diskdump: add hook for additional checks on prstatus notes validity
On Tue, Sep 26, 2023 at 2:25 PM Aditya Gupta <adityag(a)linux.ibm.com> wrote:
>
> Got a warning as below:
>
> cc -c -g -DX86_64 -DLZO -DGDB_10_2 diskdump.c -Wall -O2
> -Wstrict-prototypes -Wmissing-prototypes -fstack-protector
> -Wformat-security
> diskdump.c:145:5: warning: no previous prototype for
> ‘diskdump_is_cpu_prstatus_valid’ [-Wmissing-prototypes]
> 145 | int diskdump_is_cpu_prstatus_valid(int cpu)
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
Will fix in V2. Will add a declaration in defs.h, just above
'have_crash_notes'
declaration, or should I add the declaration at the end of all diskdump.c
declarations in defs.h ?
For now they are only invoked in the local file, not used in the other
modules, so it could be good to add a 'static' keyword and declare them in
the local file. For example:
warning[1]:
cc -c -g -DPPC64 -m64 -DLZO -DGDB_10_2 ppc64.c -Wall -O2
-Wstrict-prototypes -Wmissing-prototypes -fstack-protector
-Wformat-security
ppc64.c:305:5: warning: no previous prototype for
‘ppc64_is_cpu_prstatus_valid’ [-Wmissing-prototypes]
305 | int ppc64_is_cpu_prstatus_valid(int cpu)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
static int is_opal_context(ulong sp, ulong nip);
+static int ppc64_is_cpu_prstatus_valid(int cpu);
void opalmsg(void);
static int is_opal_context(ulong sp, ulong nip)
@@ -298,6 +299,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
+ */
+static int ppc64_is_cpu_prstatus_valid(int cpu)
+{
+ return TRUE;
+}
warning[2]:
cc -c -g -DPPC64 -m64 -DLZO -DGDB_10_2 diskdump.c -Wall -O2
-Wstrict-prototypes -Wmissing-prototypes -fstack-protector
-Wformat-security
diskdump.c:145:5: warning: no previous prototype for
‘diskdump_is_cpu_prstatus_valid’ [-Wmissing-prototypes]
145 | int diskdump_is_cpu_prstatus_valid(int cpu)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
static int valid_note_address(unsigned char *);
+static int diskdump_is_cpu_prstatus_valid(int cpu);
/* For split dumpfile */
static struct diskdump_data **dd_list = NULL;
@@ -142,13 +143,22 @@ int have_crash_notes(int cpu)
return TRUE;
}
+static int diskdump_is_cpu_prstatus_valid(int cpu)
+{
+ static int crash_notes_exists = -1;
+
...
}
What do you think about it?
Thanks.
Lianbo
>
7:15 a.m.
New subject: [PATCH v1] diskdump: add hook for additional checks on prstatus notes validity
On Tue, Sep 26, 2023 at 03:51:31PM +0800, lijiang wrote:
On Tue, Sep 26, 2023 at 2:25 PM Aditya Gupta
<adityag(a)linux.ibm.com> wrote:
> >
> > Got a warning as below:
> >
> > cc -c -g -DX86_64 -DLZO -DGDB_10_2 diskdump.c -Wall -O2
> > -Wstrict-prototypes -Wmissing-prototypes -fstack-protector
> > -Wformat-security
> > diskdump.c:145:5: warning: no previous prototype for
> > ‘diskdump_is_cpu_prstatus_valid’ [-Wmissing-prototypes]
> > 145 | int diskdump_is_cpu_prstatus_valid(int cpu)
> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
>
> Will fix in V2. Will add a declaration in defs.h, just above
> 'have_crash_notes'
> declaration, or should I add the declaration at the end of all diskdump.c
> declarations in defs.h ?
>
>
For now they are only invoked in the local file, not used in the other
modules, so it could be good to add a 'static' keyword and declare them in
the local file. For example:
warning[1]:
cc -c -g -DPPC64 -m64 -DLZO -DGDB_10_2 ppc64.c -Wall -O2
-Wstrict-prototypes -Wmissing-prototypes -fstack-protector
-Wformat-security
ppc64.c:305:5: warning: no previous prototype for
‘ppc64_is_cpu_prstatus_valid’ [-Wmissing-prototypes]
305 | int ppc64_is_cpu_prstatus_valid(int cpu)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
static int is_opal_context(ulong sp, ulong nip);
+static int ppc64_is_cpu_prstatus_valid(int cpu);
void opalmsg(void);
static int is_opal_context(ulong sp, ulong nip)
@@ -298,6 +299,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
+ */
+static int ppc64_is_cpu_prstatus_valid(int cpu)
+{
+ return TRUE;
+}
warning[2]:
cc -c -g -DPPC64 -m64 -DLZO -DGDB_10_2 diskdump.c -Wall -O2
-Wstrict-prototypes -Wmissing-prototypes -fstack-protector
-Wformat-security
diskdump.c:145:5: warning: no previous prototype for
‘diskdump_is_cpu_prstatus_valid’ [-Wmissing-prototypes]
145 | int diskdump_is_cpu_prstatus_valid(int cpu)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
static int valid_note_address(unsigned char *);
+static int diskdump_is_cpu_prstatus_valid(int cpu);
/* For split dumpfile */
static struct diskdump_data **dd_list = NULL;
@@ -142,13 +143,22 @@ int have_crash_notes(int cpu)
return TRUE;
}
+static int diskdump_is_cpu_prstatus_valid(int cpu)
+{
+ static int crash_notes_exists = -1;
+
...
}
What do you think about it?
Yes, makes sense. Thanks. Will add static declarations for them.
Will send a V2 after testing the changes, also I noticed a possible bug in
netdump.c (line 104) due to same 'have_crash_notes' check, may have to rename
'diskdump_is_cpu_prstatus_valid' to something like
'default_is_cpu_prstatus_valid' or something similar, if it is going to be used
in both places.
Thanks,
- Aditya
Thanks.
Lianbo
>
> >
>
8:33 a.m.
New subject: [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
6:46 a.m.
New subject: [PATCH v1] diskdump: add hook for additional checks on prstatus notes validity
On Sat, Oct 14, 2023 at 9:34 PM Aditya Gupta <adityag(a)linux.ibm.com> wrote:
Hi Lianbo,
I have sent the V2 now. Got delayed due to me earlier not being able to
test the V2.
Thank you for updating the V2, Aditya.
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.
That's great. I will look at the V2 later.
Thanks
Lianbo
Thanks,
- Aditya Gupta
406
days inactive
426
days old
devel@lists.crash-utility.osci.io
5 comments
2 participants
participants (2)
-
Aditya Gupta -
lijiang