Re: [PATCH] target: ppc: Correctly initialize HILE in HID-0 for book3s processors

[Narayana Murty N]

On 4/28/23 20:00, Fabiano Rosas wrote:

> Vaibhav Jain <vaibhav@linux.ibm.com> writes:
>
> > Hi Fabiano,
> >
> > Thanks for looking into this patch and apologies for the delayed reponse.
> > Fabiano Rosas <farosas@suse.de> writes:
> >
> > > Narayana Murty N <nnmlinux@linux.ibm.com> writes:
> > >
> > > > On PPC64 the HILE(Hypervisor Interrupt Little Endian) bit in HID-0
> > > > register needs to be initialized as per isa 3.0b[1] section
> > > > 2.10. This bit gets copied to the MSR_LE when handling interrupts that
> > > > are handled in HV mode to establish the Endianess mode of the interrupt
> > > > handler.
> > > >
> > > > Qemu's ppc_interrupts_little_endian() depends on HILE to determine Host
> > > > endianness which is then used to determine the endianess of the guest dump.
> > > >
> > > >           
> > > Not quite. We use the interrupt endianness as a proxy to guest
> > > endianness to avoid reading MSR_LE at an inopportune moment when the
> > > guest is switching endianness.
> > >         
> > Agreed
> >       
>     
Agreed
> > > This is not dependent on host
> > > endianness. The HILE check is used when taking a memory dump of a
> > > HV-capable machine such as the emulated powernv.
> > >         
> > I think one concern which the patch tries to address is the guest memorydump file
> > generated of a BigEndian(BE) guest on a LittleEndian(LE) host is not readable on
> > the same LE host since 'crash' doesnt support cross endianess
> > dumps. Also even for a LE guest on LE host the memory dumps are marked as BE
> > making it not possible to analyze any guest memory dumps on the host.
> >
> >       
> >From QEMU's perspective there's no "host" in this equation. We'll
> generate a BE dump for a BE guest and a LE dump for a LE guest. Anything
> different is a bug in QEMU (as the one this patch addresses).
>
> > However setting the HILE based on host endianess of qemu might not be
> > the right way to fix this problem. Based on an off mailing list discussion
> > with Narayana, he is working on another patch which doesnt set HILE
> > based on host endianess. However the problem seems to be stemming from
> > fact that qemu on KVM is using the HILE to set up the endianess of
> > memory-dump elf and since its not setup correctly the memory dumps are
> > in wrong endianess.
> >
> >       
>     
> > > I think the actual issue might be that we're calling
> > > ppc_interrupts_little_endian with hv=true for the dump.
> > >
> > >         
> > Yes, that is currently the case with cpu_get_dump_info(). Excerpt from
> > that function below that sets the endianess of the dump:
> >
> >     if (ppc_interrupts_little_endian(cpu, cpu->env.has_hv_mode)) {
> >       
> This should probably be looking at cpu->vhyp or MSR_HVB since
> has_hv_mode will not change after we init the cpu.
>     
yes, I agree. New version patch is under testing once done,
will post the patch.
> >         info->d_endian = ELFDATA2LSB;
> >     } else {
> >         info->d_endian = ELFDATA2MSB;
> >     }
> >
> > for pseries kvm guest cpu->env.has_hv_mode is already set hence
> > ppc_interrupts_little_endian() assumes its running in 'hv' mode. The new
> > patch from Narayana will be addressing this.
> >
> > > > Currently the HILE bit is never set in the HID0 register even if the
> > > > qemu is running in Little-Endian mode. This causes the guest dumps to be
> > > > always taken in Big-Endian byte ordering. A guest memory dump of a
> > > > Little-Endian guest running on Little-Endian qemu guest fails with the
> > > > crash tool as illustrated below:
> > > >
> > > >           
> > > Could you describe in more detail what is your setup? Specifically
> > > whether both guests are running TCG or KVM (info kvm) and the state of
> > > the nested-hv capability in QEMU command line.
> > >         
> > Currently the issue is seen with any pseries KVM guest running on a PowerNV host.
> >       
>