On 25/08/26 14:39, Shivang Upadhyay wrote:

When system is restarted with fadump, cpu registers data is copied
to CPU_STATE_DATA region. The size of data depends on the number of
cpus that are currently present in the system. But when maxcpus config
option is provided in qemu command line, this causes the linux kernel to
allocate extra space for CPU_STATE_DATA region. If there are no hotplug
operations, this will lead to part of region being empty.

As per current Linux kernel, It expects the size of collected Fadump
region to be exactly equal to allocated size. Otherwise
kernel assumes that dump is incomplete [1].

If maxcpus option is used, This causes qemu to send partially filled
region, and Kernel then rejects to collect to any dump. As a result of
this /proc/vmcore is not generated.

To fix this, Changing fadump to always report CPU_STATE_DATA's
dumped_bytes as source_len. This behaviour is also noticed in Phyp.

[1] 
https://github.com/torvalds/linux/blob/66498c75b4f8017f62d720d9b59675bdf3abce91/arch/powerpc/platforms/pseries/rtas-fadump.c#L472
Cc: Sourabh Jain <[email protected]>
Cc: Aditya Gupta <[email protected]>
Reported-by: Anushree Mathur <[email protected]>
Signed-off-by: Shivang Upadhyay <[email protected]>
---
  hw/ppc/spapr_fadump.c | 7 +++++--
  1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_fadump.c b/hw/ppc/spapr_fadump.c
index 13cab0cfe1..c0f6474ae9 100644
--- a/hw/ppc/spapr_fadump.c
+++ b/hw/ppc/spapr_fadump.c
@@ -575,8 +575,9 @@ static bool do_populate_cpu_state(FadumpSection *region)
if (region->source_len != region->bytes_dumped) {
          /*
-         * Log the error, but don't fail the dump collection here, let
-         * kernel handle the mismatch
+         * Log the error, but don't fail the dump collection here.
+         * This is probably because of maxcpus config. Linux kernel would
+         * not collect any dump if dumped_bytes does not match source_len.
           */

Understood the problem, can we mention something like "this is not specifically
suggested by spapr, matching phyp's behaviour here" or something like that ?

          qemu_log_mask(LOG_GUEST_ERROR,
                  "FADump: Mismatch in CPU State region's length exported:"
@@ -584,6 +585,8 @@ static bool do_populate_cpu_state(FadumpSection *region)
                  " QEMU exported: 0x%" PRIx64 " bytes\n",
                  be64_to_cpu(region->source_len),
                  be64_to_cpu(region->bytes_dumped));
+
+        region->bytes_dumped = region->source_len;
source_len = bytes_dumped makes more sense here, than increasing bytes_dumped to
source_len. what do you say ?

otherwise okay with me, but good to have a clear comment about deviating from spapr to match phyp here, so we can fix it if phyp or spapr were to change to
follow one thing.


thanks,

- Aditya G


Reply via email to