LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ppc/fadump: collect dump if the collected size is lesser than reserved
@ 2026-07-14 17:30 Shivang Upadhyay
  2026-07-19  7:09 ` Sourabh Jain
  0 siblings, 1 reply; 3+ messages in thread
From: Shivang Upadhyay @ 2026-07-14 17:30 UTC (permalink / raw)
  To: linuxppc-dev, linux-kernel
  Cc: maddy, mpe, npiggin, chleroy, sourabhjain, adri.vero.dev, adityag,
	anushree.mathur, Shivang Upadhyay

When a machine is subjected to CPUs add/remove, using dlpar
operations, the number of collected CPU_NOTES can change.
As per PAPR, collected dump size should not be more than
allocated size. Reflecting the same in source.

Signed-off-by: Shivang Upadhyay <shivangu@linux.ibm.com>
---
 arch/powerpc/platforms/pseries/rtas-fadump.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/pseries/rtas-fadump.c b/arch/powerpc/platforms/pseries/rtas-fadump.c
index 3bb4ac2ab6cc..19a5adaf326b 100644
--- a/arch/powerpc/platforms/pseries/rtas-fadump.c
+++ b/arch/powerpc/platforms/pseries/rtas-fadump.c
@@ -469,7 +469,8 @@ static int __init rtas_fadump_process(struct fw_dump *fadump_conf)
 				pr_err("Dump taken by platform is not valid (%d)\n", i);
 				rc = -EINVAL;
 			}
-			if (fdm_active->rgn[i].bytes_dumped != fdm_active->rgn[i].source_len) {
+			if (be64_to_cpu(fdm_active->rgn[i].bytes_dumped)
+			    > be64_to_cpu(fdm_active->rgn[i].source_len)) {
 				pr_err("Dump taken by platform is incomplete (%d)\n", i);
 				rc = -EINVAL;
 			}
-- 
2.54.0



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] ppc/fadump: collect dump if the collected size is lesser than reserved
  2026-07-14 17:30 [PATCH] ppc/fadump: collect dump if the collected size is lesser than reserved Shivang Upadhyay
@ 2026-07-19  7:09 ` Sourabh Jain
  2026-07-22  9:11   ` Shivang Upadhyay
  0 siblings, 1 reply; 3+ messages in thread
From: Sourabh Jain @ 2026-07-19  7:09 UTC (permalink / raw)
  To: Shivang Upadhyay, linuxppc-dev, linux-kernel
  Cc: maddy, mpe, npiggin, chleroy, adri.vero.dev, adityag,
	anushree.mathur

Could you please reword the commit title to make it a bit clearer?


On 14/07/26 23:00, Shivang Upadhyay wrote:
> When a machine is subjected to CPUs add/remove, using dlpar
> operations, the number of collected CPU_NOTES can change.
> As per PAPR, collected dump size should not be more than
> allocated size. Reflecting the same in source.

Can you add more details about the problem you are trying to solve
with this patch and how.

Adding the error message and scenario would be really helpful in
understanding the problem.

Can you add Closes tag if it is reported upstream and if possible fixes 
tag too.

> Signed-off-by: Shivang Upadhyay <shivangu@linux.ibm.com>
> ---
>   arch/powerpc/platforms/pseries/rtas-fadump.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/pseries/rtas-fadump.c b/arch/powerpc/platforms/pseries/rtas-fadump.c
> index 3bb4ac2ab6cc..19a5adaf326b 100644
> --- a/arch/powerpc/platforms/pseries/rtas-fadump.c
> +++ b/arch/powerpc/platforms/pseries/rtas-fadump.c
> @@ -469,7 +469,8 @@ static int __init rtas_fadump_process(struct fw_dump *fadump_conf)
>   				pr_err("Dump taken by platform is not valid (%d)\n", i);
>   				rc = -EINVAL;
>   			}
> -			if (fdm_active->rgn[i].bytes_dumped != fdm_active->rgn[i].source_len) {
> +			if (be64_to_cpu(fdm_active->rgn[i].bytes_dumped)
> +			    > be64_to_cpu(fdm_active->rgn[i].source_len)) {

Can you please share your observations about `bytes_dump` for both QEMU 
and a
real system (LPAR) where the number of online CPUs is not equal to the 
maximum
number of CPUs?

I agree that you're making these changes to comply with PAPR, but since 
this has been
working for so long, it would be good to have all the data before making 
this change.

- Sourabh Jain

>   				pr_err("Dump taken by platform is incomplete (%d)\n", i);
>   				rc = -EINVAL;
>   			}



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ppc/fadump: collect dump if the collected size is lesser than reserved
  2026-07-19  7:09 ` Sourabh Jain
@ 2026-07-22  9:11   ` Shivang Upadhyay
  0 siblings, 0 replies; 3+ messages in thread
From: Shivang Upadhyay @ 2026-07-22  9:11 UTC (permalink / raw)
  To: Sourabh Jain, linuxppc-dev, linux-kernel
  Cc: maddy, mpe, npiggin, chleroy, adri.vero.dev, adityag,
	anushree.mathur

On Sun, 2026-07-19 at 12:39 +0530, Sourabh Jain wrote:
> Could you please reword the commit title to make it a bit clearer?
> 
> 
> On 14/07/26 23:00, Shivang Upadhyay wrote:
> > When a machine is subjected to CPUs add/remove, using dlpar
> > operations, the number of collected CPU_NOTES can change.
> > As per PAPR, collected dump size should not be more than
> > allocated size. Reflecting the same in source.
> 
> Can you add more details about the problem you are trying to solve
> with this patch and how.
> 
> Adding the error message and scenario would be really helpful in
> understanding the problem.
> 

Hi Sourabh,

 When a qemu ppc machine is booted with fadump=on and `-
smp=x,maxcpus=y`, on the panic kernel, /proc/vmcore is not generated
because dump_bytes and source_len does'nt match for CPU_STATE_DATA
region in fadump, then we just give up on parsing rest of the data.


> Can you add Closes tag if it is reported upstream and if possible
> fixes 
> tag too.
> 
> > Signed-off-by: Shivang Upadhyay <shivangu@linux.ibm.com>
> > ---
> >   arch/powerpc/platforms/pseries/rtas-fadump.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/powerpc/platforms/pseries/rtas-fadump.c
> > b/arch/powerpc/platforms/pseries/rtas-fadump.c
> > index 3bb4ac2ab6cc..19a5adaf326b 100644
> > --- a/arch/powerpc/platforms/pseries/rtas-fadump.c
> > +++ b/arch/powerpc/platforms/pseries/rtas-fadump.c
> > @@ -469,7 +469,8 @@ static int __init rtas_fadump_process(struct
> > fw_dump *fadump_conf)
> >   				pr_err("Dump taken by platform is
> > not valid (%d)\n", i);
> >   				rc = -EINVAL;
> >   			}
> > -			if (fdm_active->rgn[i].bytes_dumped !=
> > fdm_active->rgn[i].source_len) {
> > +			if (be64_to_cpu(fdm_active-
> > >rgn[i].bytes_dumped)
> > +			    > be64_to_cpu(fdm_active-
> > >rgn[i].source_len)) {
> 
> Can you please share your observations about `bytes_dump` for both
> QEMU 
> and a
> real system (LPAR) where the number of online CPUs is not equal to
> the 
> maximum

I have the following observation.

I booted LPAR with 8 cpus. After crashing it I saw that fadump
CPU_STATE_DATA has notes for total 16 cpus, and only top 8 notes have
valid entries. when trying with 16, I see all NOTES have entried
filled.

I previously proposed this fake entries fix in qemu [1]. But that is
not compliant with PAPR, which states that cpu notes should be
collected for current_cpus only. 

As per current states of things, reservation is always done for
maxcpus, (LPAR and QEMU both), NOTES collections is done for current
cpus on QEMU, and maxcpus on LPARs.

My proposal here is that we make the check for bytes_dumped and
source_len, less restrictive to let QEMU also generate /proc/vmcore,
when current_cpus != maxcpus.


Regards 
~Shivang.

[1]
https://lore.kernel.org/qemu-devel/20260520092857.1079193-1-shivangu@linux.ibm.com/



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-22  9:11 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-14 17:30 [PATCH] ppc/fadump: collect dump if the collected size is lesser than reserved Shivang Upadhyay
2026-07-19  7:09 ` Sourabh Jain
2026-07-22  9:11   ` Shivang Upadhyay

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox