[PATCH] ppc/fadump: collect dump if the collected size is lesser than reserved

[PATCH] ppc/fadump: collect dump if the collected size is lesser than reserved

[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

Re: [PATCH] ppc/fadump: collect dump if the collected size is lesser than reserved

[Sourabh Jain]

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;
>   			}