Re: [Qemu-devel] [PATCH 5/5] target-arm: support QMP dump-guest-memory

[Andrew Jones <drjones@redhat.com>]

On Sat, Nov 21, 2015 at 10:05:37AM -0500, Andrew Jones wrote:
> On Fri, Nov 20, 2015 at 04:41:21PM -0500, Andrew Jones wrote:
> > On Fri, Nov 20, 2015 at 06:19:14PM +0000, Peter Maydell wrote:
> > > On 19 November 2015 at 14:53, Andrew Jones <drjones@redhat.com> wrote:
> > > > +
> > > > +    if (is_a64(env)) {
> > > 
> > > Are you really sure you want the core dump format to depend on
> > > whether the CPU happens to be in 32-bit or 64-bit format at
> > > the point in time we write it out? (Consider a 64-bit kernel
> > > which happens to be running a 32-bit userspace binary.)
> > 
> > I simply forgot to consider the case where a 64-bit kernel would
> > run a 32-bit userspace binary. I'm actually quite sure we would
> > want 64-bit in that case, as crash is the only tool we're able to
> > generate dumps for at this time (gdb requires the 'paging' option
> > of dump-guest-memory to work). Is there something in the env I can
> > look at to determine that we have a 64-bit kernel? (Sorry for being
> > lazy and just asking, rather than reading...)
> 
> Duh, I momentarily forgot about arm_el_is_aa64(env, 1). I see we unset
> ARM_FEATURE_AARCH64 in aarch64_cpu_set_aarch64, so that should work
> fine for our 32bit guests on 64bit hosts.
> 
> > > > +    if (is_a64(env)) {
> > > > +        info->d_machine = EM_AARCH64;
> > > > +        info->d_class = ELFCLASS64;
> > > > +        if (cur_el == 0) {
> > > > +            be = (env->cp15.sctlr_el[1] & SCTLR_E0E) != 0;
> > > > +        } else {
> > > > +            be = (env->cp15.sctlr_el[cur_el] & SCTLR_EE) != 0;
> > > > +        }
> > > 
> > > Again, are you sure you want the core dump format to depend on
> > > whether we currently happen to be executing a BE userspace
> > > process?
> > 
> > We'll want to match the kernel. Hopefully we can determine it.
> 
> Here's a bigger, duh. I guess I just need to drop all the cur_el
> stuff and stick to el==1.
> 
> I'll wait to hear back on the 'should we add ptrace.h to linux-headers',
> and 'should we add floating point registers, even though crash won't
> care' questions before sending a v2.

Hi Peter,

I've pulled a v2 together that I'll be testing and posting soon. Here's
what I decided to do

1) Throw the fp registers in. Why not?
2) No linux-headers update, as we'd also need 
   include/uapi/linux/elfcore.h and arch/arm/include/asm/user.h.
   However I've added comments stating where the structs come from.
3) Base the vmcore type on the guest kernel, i.e. use arm_el_is_aa64()
   and (env->cp15.sctlr_el[1] & SCTLR_EE). However,
   aarch64_write_elf64_note() will shoehorn 32-bit state into 64-bit
   elf notes when the current state is 32-bit. Those analyzing the
   dumps will need to look at the captured pstate to determine the
   endianness of the registers.

How does that sound?

Thanks,
drew