On Fri, Oct 22, 2010 at 03:13:06PM -0400, Dave Anderson wrote:
>
> ----- "Hu Tao" <hutao@xxxxxxxxxxxxxx> wrote:
>
> > > > On Tue, Oct 19, 2010 at 09:06:33AM -0400, Dave Anderson wrote:
> > > > >
> > > > > ----- "Hu Tao" <hutao cn fujitsu com> wrote:
> > > > >
> > > > > > Hi Dave,
> > > > > >
> > > > > > These are updated patches tested with SMP system and panic task.
> > > > > >
> > > > > > When testing a x86 guest, I found another bug about reading cpu
> > > > > > registers from dumpfile. Qemu simulated system is x86_64
> > > > > > (qemu-system-x86_64), guest OS is x86. When crash reads cpu registers
> > > > > > from dumpfile, it uses cpu_load_32(), this will read gp registers by
> > > > > > get_be_long(fp, 32), that is, treate them as 32bits. But in fact,
> > > > > > qemu-system-x86_64 saves 64bits for each of them(although guest OS
> > > > > > uses only lower 32 bits). As a result, crash gets wrong cpu gp
> > > > > > register values.
> > > > >
> > > > > As I understand it, you're running a 32-bit guest on a 64-bit host.
> > > >
> > > > Yes.
> > > >
> > > > > If you were to read 64-bit register values instead of 32-bit register
> > > > > values, wouldn't that cause the file offsets of the subsequent get_xxx()
> > > > > calls in cpu_load() to read from the wrong file offsets? And then
> > > > > that would leave the ending file offset incorrect, such that the
> > > > > qemu_load() loop would fail to find the next device?
> > > > >
> > > > > In other words, the cpu_load() function, which is used for both
> > > > > 32-bit and 64-bit guests, must be reading the correct amount of
> > > > > data from the "cpu" device, or else qemu_load() would fail to
> > > > > find the next device in the next location in the dumpfile.
> > > >
> > > > True. In fact, in my case if read 32-bit registers, following devices
> > > > are found:
> > > > block, ram, kvm-tpr-opt, kvmclock, timer, cpu_common, cpu.
> > > > If read 64-bit registers, following devices are found:
> > > > block, ram, kvm-tpr-opt, kvmclock, timer, cpu_common, cpu, apic, fw_cfg
> > >
> > > Right -- so it got "lost" after incorrectly gathering the data for the
> > > first "cpu" device instance.
> > >
> > > > > > Is there any way we can know from dumpfile that these gp
> > > > > > registers(and those similar registers) are 32bits or 64bits?
> > > > >
> > > > > I don't know. If what you say is true, when would those registers
> > > > > ever be 32-bit values?
> > > >
> > > > I did tests on a 64-bit machine. Result is:
> > > >
> > > > machine OS guest machine guest OS saved gp regs
> > > > ------------------------------------------------------------------------
> > > > 64-bit x86 qemu-kvm(kvm enabled) x86 64 bits
> > > > 64-bit x86 qemu(kvm disabled) x86 32 bits
> > >
> > > I don't understand what you mean when you say that the guest machine
> > > is "kvm enabled" or "kvm disabled"?
> >
> > Sorry for being vague. "kvm enabled" means using qemu-kvm to bring up
> > guest machine and this enables KVM hardware virtualization on host.
> > "kvm disabled" means using qemu to bring up guest machine and this
> > disables KVM hardware virtualization on host.
> >
> > >
> > > And if your host machine is running a 32-bit x86 OS (on 64-bit hardware),
> > > that's something I've never seen given that Red Hat only allows 64-bit
> > > kernels as KVM hosts.
> >
> > I did the test on Fedora 13 i686. Just tried rhel6 i386, as you said,
> > there is no kvm support.
>
> Hello Hu,
>
> Your supposition that the "cpu" device layout is dependent upon the
> host kernel type is correct, but unfortunately there's no readily-evident
> way to determine what type of kernel the host was running. This is Paolo's
> response to the question:
>
> > So the question is:
> >
> > Can it be determined from something in the dumpfile header that
> > the *host* machine was running a 32-bit kernel?
>
> It's not an exact science, but you can do some trial-and-error. I
> suggest measuring the distance from between the cpu and apic blocks
> (which you can do using code from your "workaround" explained below, I
> guess) and deciding based on the size of the CPU block.
>
> A 64-bit image I have lying around takes 987 bytes, I'd guess that
> anything above 850 is 64-bit. Maybe you can start searching after the
> first 250 bytes, since the registers are at the beginning and if you're
> going to get a false match you're going to get it there.
>
> The "workaround" he's referring to is this, which will be in the next
> release:
>
> Re: [patch] crash on a KVM-generated dump
> https://www.redhat.com/archives/crash-utility/2010-October/msg00034.html
>
> But it's not a particularly graceful solution in this case, because it
> would require walking through all of the "block" and "ram" devices
> to find the first "cpu" device -- but at that point the 32-vs-64 bit
> device has already been selected. I suppose another alternative would
> be to always start reading the "cpu" data in cpu_load() as if it were
> created by a 64-bit host, and making a determination somewhere along the
> way that the data being read is bogus and that it should be using the
> 32-bit device mechanism, seeking back, and calling the other function?
The point is how we can make such a determination. What about still try
first reading as 32-bit, if we read a zeroed sp then we seek back and
read as 64-bit. We can do this because:
1. Given that qemu/kvm saves gp registers in the order of EAX, ECX,
EDX, EBX, ESP, EBP, ESI and EDI and in big endian, if we read 64-bit
of these registers as if they are 32-bit, then we will read high
32bits of rdx into esp.
2. As specified in Intel's Software Developer's Manual, 32-bit operands
generate a 32-bit result, zero-extended to a 64-bit result in the
destination general-purpose register. So in the case of x86 guest OS
running on a 64bit machine, we can expect all zero high 32 bits of
rdx, and read a zeroed sp.
3. A zeroed sp never happens.
4. qemu/kvm don't fault on sp.
The attached patch does this. Of course, it's still an ugly way.
>
> I don't know -- either option would be be really ugly...
>
> Anyway, given that the use of 32-bit KVM hosts should be fairly rare,
> what would you think of handling it this way:
>
> (1) use the 64-bit functions by default
> (2) adding a crash command line option like "--kvmhost 32" to force the
> use of the 32-bit functions
I prefer this way as the reason you stated. And to be more user-friendly,
it's better to make user aware of this option that he/she can try with
besides giving messages of invaid registers contents or can't backtrace
stack, etc.
>
> And of course, even if the new option were *not* used on a 32-bit dumpfile,
> it would still behave as it does now -- crash still comes up OK -- but it
> just wouldn't be able to use the registers from the header.
>
> What do you think?
>
> Dave
>
--
Thanks,
Hu Tao
>From 35967080206ecfe9970314b9072d9f3369a25411 Mon Sep 17 00:00:00 2001
From: Hu Tao <hutao@xxxxxxxxxxxxxx>
Date: Mon, 25 Oct 2010 14:19:02 +0800
Subject: [PATCH] Guess we're reading 32- or 64-bit GPRs by a zero sp
---
qemu-load.c | 12 +++++++++++-
1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/qemu-load.c b/qemu-load.c
index 303ed94..148ebc7 100644
--- a/qemu-load.c
+++ b/qemu-load.c
@@ -439,14 +439,18 @@ cpu_load (struct qemu_device *d, FILE *fp, int size)
{
struct qemu_device_x86 *dx86 = (struct qemu_device_x86 *)d;
uint32_t qemu_hflags = 0, qemu_hflags2 = 0;
- int nregs = size == 32 ? 8 : 16;
+ int nregs;
uint32_t version_id = dx86->dev_base.version_id;
uint32_t rhel5_version_id;
int i;
+ long start;
struct qemu_device *drhel5;
struct qemu_device_cpu_common *dcpu;
+ start = ftell(fp);
+retry:
+ nregs = size == 32 ? 8 : 16;
drhel5 = device_find_instance (d->list, "__rhel5", 0);
if (drhel5 || (version_id >= 7 && version_id <= 9)) {
rhel5_version_id = version_id;
@@ -470,6 +474,12 @@ cpu_load (struct qemu_device *d, FILE *fp, int size)
for (i = 0; i < nregs; i++)
dx86->regs[i] = get_be_long (fp, size);
+ if (dx86->regs[R_ESP] == 0 && size == 32) {
+ size = 64;
+ fseek(fp, start, SEEK_SET);
+ goto retry;
+ }
+
dx86->eip = get_be_long (fp, size);
dx86->eflags = get_be_long (fp, size);
qemu_hflags = get_be32 (fp);
--
1.7.3
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility
