Re: [PATCH v3] target: ppc: Use MSR_HVB bit to get the target endianness for memory dump

[Nicholas Piggin]

On Tue May 23, 2023 at 2:02 AM AEST, Narayana Murty N 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.

The guest? Are you talking about the immediate target, or a
KVM guest running under that? Or both?

> 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
> LE             pseries KVM         LE KVM guest                 LE
> LE             pseries TCG         LE guest                     LE

I'm still having a bit of trouble with this like the others.

First thing first, take "guest" out of it entirely. If I read it right,
then the dump will be done in whichever endian the OS running on the
machine has set the interrupt endian mode to, and crash expects that to
match the vmlinux. Is that correct?

For the powernv machine, when the OS runs a KVM guest, the crash dump
continues to be generated in the same endianness even if the guest OS is
the opposite.

If my understanding is correct, I don't see why that is desirable to
change it to the arbitrary guest setting. The OS running on the target
is the one you want to debug if you dump from the first QEMU. If you
want to debug the guest you would dump from the QEMU running in the
target that is running the guest, no?

For the pseries machine, the TCG nested HV thing probably takes over
LPCR when running the nested HV guest. I would argue that's wrong and it
should actually match powernv behaviour.

Thanks,
Nick

>
> Signed-off-by: Narayana Murty N <nnmlinux@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-nnmlinux@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;
> -- 
> 2.39.2