On Tue, 23 May 2023 12:20:17 +0530 Narayana Murty N <nnmli...@linux.vnet.ibm.com> wrote:
> > On 5/22/23 23:50, Greg Kurz wrote: > > On Mon, 22 May 2023 12:02:42 -0400 > > Narayana Murty N<nnmli...@linux.ibm.com> wrote: > > > >> Currently on PPC64 qemu always dumps the guest memory in > >> Big Endian (BE) format even though the guest running in Little Endian > >> (LE) mode. So crash tool fails to load the dump as illustrated below: > >> > >> Log : > >> $ virsh dump DOMAIN --memory-only dump.file > >> > >> Domain 'DOMAIN' dumped to dump.file > >> > >> $ crash vmlinux dump.file > >> > >> <snip> > >> crash 8.0.2-1.el9 > >> > >> WARNING: endian mismatch: > >> crash utility: little-endian > >> dump.file: big-endian > >> > >> WARNING: machine type mismatch: > >> crash utility: PPC64 > >> dump.file: (unknown) > >> > >> crash: dump.file: not a supported file format > >> <snip> > >> > >> This happens because cpu_get_dump_info() passes cpu->env->has_hv_mode > >> to function ppc_interrupts_little_endian(), the cpu->env->has_hv_mode > >> always set for powerNV even though the guest is not running in hv mode. > >> The hv mode should be taken from msr_mask MSR_HVB bit > >> (cpu->env.msr_mask & MSR_HVB). This patch fixes the issue by passing > >> MSR_HVB value to ppc_interrupts_little_endian() in order to determine > >> the guest endianness. > >> > >> The crash tool also expects guest kernel endianness should match the > >> endianness of the dump. > >> > >> The patch was tested on POWER9 box booted with Linux as host in > >> following cases: > >> > >> Host-Endianess Qemu-Target-Machine Qemu-Guest-Endianess > >> Qemu-Generated-Guest > >> > >> Memory-Dump-Format > >> BE powernv LE KVM guest LE > >> BE powernv BE KVM guest BE > >> LE powernv LE KVM guest LE > >> LE powernv BE KVM guest BE > > I don't quite understand why KVM is mentioned with the powernv machine. > > guest running mode was mentioned. > QEMU cannot use KVM on the host to run a powernv machine. The guest is thus necessarily running in TCG mode. Please describe your setup and what exactly you are testing. > > > > Also have you tried to dump at various moments, e.g. during skiboot > > and when guest is booted, as in [1] which introduced the code this > > patch is changing ? > > > > [1]https://github.com/qemu/qemu/commit/5609400a422809c89ea788e4d0e13124a617582e. > > > >> LE pseries KVM LE KVM guest LE > >> LE pseries TCG LE guest LE > >> > > Fixes: 5609400a4228 ("target/ppc: Set the correct endianness for powernv > > memory dumps") > > I agree, commit 5609400a4228 fixes endianness detection only for initial > stage (skiboot) till endianness switch happens. > However, has_hv_mode is just a capability flag which is always set based on > command-line param and doesnt really represent current hv state. > With this patch, it relies on the current state of the hv state based on the > MSR_HVB of the msr_mask. > Yes I see what your patch is doing. The 'Fixes: 5609400a4228 ...' line is intended to the changelog because it is supposedly a fix to this commit. > > > >> Signed-off-by: Narayana Murty N<nnmli...@linux.ibm.com> > >> --- > >> Changes since V2: > >> commit message modified as per feedbak from Nicholas Piggin. > >> Changes since V1: > >> https://lore.kernel.org/qemu-devel/20230420145055.10196-1-nnmli...@linux.ibm.com/ > >> The approach to solve the issue was changed based on feedback from > >> Fabiano Rosas on patch V1. > >> --- > >> target/ppc/arch_dump.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/target/ppc/arch_dump.c b/target/ppc/arch_dump.c > >> index f58e6359d5..a8315659d9 100644 > >> --- a/target/ppc/arch_dump.c > >> +++ b/target/ppc/arch_dump.c > >> @@ -237,7 +237,7 @@ int cpu_get_dump_info(ArchDumpInfo *info, > >> info->d_machine = PPC_ELF_MACHINE; > >> info->d_class = ELFCLASS; > >> > >> - if (ppc_interrupts_little_endian(cpu, cpu->env.has_hv_mode)) { > >> + if (ppc_interrupts_little_endian(cpu, !!(cpu->env.msr_mask & > >> MSR_HVB))) { > >> info->d_endian = ELFDATA2LSB; > >> } else { > >> info->d_endian = ELFDATA2MSB;