x86, nops settings result in kernel crash

Hi,

I am writing a file system test which I execute in qemu with kernel compiled from latest git sources and running it causes this error:

https://bugzilla.kernel.org/show_bug.cgi?id=45971

It works with v3.5, so I ran git bisect which pointed me to:

d6250a3f12edb3a86db9598ffeca3de8b4a219e9 x86, nops: Missing break resulting in incorrect selection on Intel

To be quite honest, I don't understand this stuff much but I tried to do some debugging and I figured out (I hope) that the crash is caused by setting ideal_nops to p6_nops (k8_nops was used before the break statement was added).

Quoted 12 lines

Hi,

I am writing a file system test which I execute in qemu with kernel compiled from latest git sources and running it causes this error:

https://bugzilla.kernel.org/show_bug.cgi?id=45971

It works with v3.5, so I ran git bisect which pointed me to:

d6250a3f12edb3a86db9598ffeca3de8b4a219e9 x86, nops: Missing break resulting in incorrect selection on Intel

Quoted 4 lines

To be quite honest, I don't understand this stuff much but I tried to do some debugging and I figured out (I hope) that the crash is caused by setting ideal_nops to p6_nops (k8_nops was used before the break statement was added).

Maybe I overlooked it or maybe it was implied but did you try reverting
the patch and rerunning your test? Does it work ok then?

Thanks.

--
Regards/Gruss,
Boris.

----- Original Message -----

Quoted 18 lines · Tomas Racek

Quoted 16 lines

Hi,

I am writing a file system test which I execute in qemu with kernel compiled from latest git sources and running it causes this error:

https://bugzilla.kernel.org/show_bug.cgi?id=45971

It works with v3.5, so I ran git bisect which pointed me to:

d6250a3f12edb3a86db9598ffeca3de8b4a219e9 x86, nops: Missing break resulting in incorrect selection on Intel

To be quite honest, I don't understand this stuff much but I tried to do some debugging and I figured out (I hope) that the crash is caused by setting ideal_nops to p6_nops (k8_nops was used before the break statement was added).

Quoted 4 lines

Maybe I overlooked it or maybe it was implied but did you try reverting
the patch and rerunning your test? Does it work ok then?

Yes, if I remove the break statement (introduced by this commit), it works fine.

Quoted 2 lines

Yes, if I remove the break statement (introduced by this commit), it works fine.

What version of qemu is this - do we have qemu bug here I wonder.

Quoted 6 lines

Quoted 3 lines

Yes, if I remove the break statement (introduced by this commit), it works fine.

What version of qemu is this - do we have qemu bug here I wonder.

Also, is it 32 or 64 bits?

-hpa

Quoted 24 lines · On Thu, 16 Aug 2012 14:45:15 -0400 (EDT) Tomas Racek <[email protected]> wrote:

----- Original Message -----

Quoted 22 lines · Tomas Racek

Quoted 9 lines

Hi,

I am writing a file system test which I execute in qemu with kernel compiled from latest git sources and running it causes this error:

https://bugzilla.kernel.org/show_bug.cgi?id=45971

It works with v3.5, so I ran git bisect which pointed me to:

Quoted 7 lines

d6250a3f12edb3a86db9598ffeca3de8b4a219e9 x86, nops: Missing break resulting in incorrect selection on Intel

To be quite honest, I don't understand this stuff much but I tried to do some debugging and I figured out (I hope) that the crash is caused by setting ideal_nops to p6_nops (k8_nops was used before the break statement was added).

Maybe I overlooked it or maybe it was implied but did you try reverting
the patch and rerunning your test? Does it work ok then?

Quoted 1 line

Yes, if I remove the break statement (introduced by this commit), it works fine.

What version of qemu is this - do we have qemu bug here I wonder.

From the cpuinfo, it's 0.15.1. That's old but not ancient.

From the report, using '-cpu host' solves the problem. '-cpu host'

Alan

Quoted 33 lines · Alan Cox

Quoted 30 lines · On Thu, 16 Aug 2012 14:45:15 -0400 (EDT) Tomas Racek <[email protected]> wrote:

----- Original Message -----

Quoted 28 lines · Tomas Racek

Quoted 15 lines

Hi,

I am writing a file system test which I execute in qemu with kernel
compiled from latest git sources and running it causes this error:

https://bugzilla.kernel.org/show_bug.cgi?id=45971

It works with v3.5, so I ran git bisect which pointed me to:

d6250a3f12edb3a86db9598ffeca3de8b4a219e9 x86, nops: Missing break
resulting in incorrect selection on Intel

Quoted 7 lines

To be quite honest, I don't understand this stuff much but I tried
to do some debugging and I figured out (I hope) that the crash is
caused by setting ideal_nops to p6_nops (k8_nops was used before
the break statement was added).

Maybe I overlooked it or maybe it was implied but did you try reverting
the patch and rerunning your test? Does it work ok then?

Quoted 4 lines

Quoted 2 lines

Yes, if I remove the break statement (introduced by this commit), it works fine.

What version of qemu is this - do we have qemu bug here I wonder.

From the cpuinfo, it's 0.15.1. That's old but not ancient.

I took a brief look at the kernel code here. The default invocation of
qemu presents an idealistic CPU with a very minimum feature bit set exposed. No processor has ever existed with this feature set.

We do this in order to maintain compatibility when migration from Intel
to AMD but also for legacy reasons.

From the report, using '-cpu host' solves the problem. '-cpu host' exposes most of the host CPUID to the guest.

That said, QEMU really doesn't do anything differently depending on what
feature bits are exposed to the guest. So my guess is that the odd combination of CPUID bits that are exposed to the guest is confusing the
kernel.

Can you post dmesg from the host kernel? Perhaps there's instruction emulation failing in the host KVM? That would manifest in strange behavior in the guest.

Regards,

Anthony Liguori

Quoted 2 lines

Alan

Well, I've added some debug statements to the code:

void __init arch_init_ideal_nops(void)

{
        switch (boot_cpu_data.x86_vendor) {
        case X86_VENDOR_INTEL:
                /*
                 * Due to a decoder implementation quirk, some
                 * specific Intel CPUs actually perform better with
                 * the "k8_nops" than with the SDM-recommended NOPs.
                 */
                if (boot_cpu_data.x86 == 6 &&
                    boot_cpu_data.x86_model >= 0x0f &&
                    boot_cpu_data.x86_model != 0x1c &&
                    boot_cpu_data.x86_model != 0x26 &&
                    boot_cpu_data.x86_model != 0x27 &&
                    boot_cpu_data.x86_model < 0x30) {
                        printk("NOPS: Option 1\n");
                        ideal_nops = k8_nops;
                } else if (boot_cpu_has(X86_FEATURE_NOPL)) {
                        printk("NOPS: Option 2\n");
                           ideal_nops = p6_nops;
                } else {
                        printk("NOPS: Option 3\n");
#ifdef CONFIG_X86_64
                        ideal_nops = k8_nops;
#else
                        ideal_nops = intel_nops;
#endif
                }
                break;
        default:
#ifdef CONFIG_X86_64
                ideal_nops = k8_nops;
#else
                if (boot_cpu_has(X86_FEATURE_K8))
                        ideal_nops = k8_nops;
                else if (boot_cpu_has(X86_FEATURE_K7))
                        ideal_nops = k7_nops;
                else
                        ideal_nops = intel_nops;
#endif
        }
}

This gives me Option 1 with "-cpu host" and Option 2 without.

Quoted 46 lines

Well, I've added some debug statements to the code:

void __init arch_init_ideal_nops(void)

{
        switch (boot_cpu_data.x86_vendor) {
        case X86_VENDOR_INTEL:
                /*
                 * Due to a decoder implementation quirk, some
                 * specific Intel CPUs actually perform better with
                 * the "k8_nops" than with the SDM-recommended
                 NOPs.
                 */
                if (boot_cpu_data.x86 == 6 &&
                    boot_cpu_data.x86_model >= 0x0f &&
                    boot_cpu_data.x86_model != 0x1c &&
                    boot_cpu_data.x86_model != 0x26 &&
                    boot_cpu_data.x86_model != 0x27 &&
                    boot_cpu_data.x86_model < 0x30) {
                        printk("NOPS: Option 1\n");
                        ideal_nops = k8_nops;
                } else if (boot_cpu_has(X86_FEATURE_NOPL)) {
                        printk("NOPS: Option 2\n");
                           ideal_nops = p6_nops;
                } else {
                        printk("NOPS: Option 3\n");
#ifdef CONFIG_X86_64
                        ideal_nops = k8_nops;
#else
                        ideal_nops = intel_nops;
#endif
                }
                break;
        default:
#ifdef CONFIG_X86_64
                ideal_nops = k8_nops;
#else
                if (boot_cpu_has(X86_FEATURE_K8))
                        ideal_nops = k8_nops;
                else if (boot_cpu_has(X86_FEATURE_K7))
                        ideal_nops = k7_nops;
                else
                        ideal_nops = intel_nops;
#endif
        }
}

Quoted 1 line

This gives me Option 1 with "-cpu host" and Option 2 without.

This looks like an emulation bug. The interesting thing is that your both traces from the bugzilla point to generic_make_request_checks but
it could also be due to timing.

Decoding the instruction stream in the second trace in the bugzilla gives:

[ 278.595106] Code: 03 48 89 03 48 8b 57 70 48 89 53 10 48 2b 01 8b

3f 48 89 45 98 48 8b 82 90 00 00 00 89 7d 94 48 8b 80 60 02 00 00 48 89 45 88 ac <17> 00 00 c8 45 85 e4 74 30 48 8b 43 10 48 8b 40 08 48 8b 40 48
All code
========

   0:   03 48 89                add    -0x77(%rax),%ecx
   3:   03 48 8b                add    -0x75(%rax),%ecx
   6:   57                      push   %rdi
   7:   70 48                   jo     0x51
   9:   89 53 10                mov    %edx,0x10(%rbx)
   c:   48 2b 01                sub    (%rcx),%rax
   f:   8b 3f                   mov    (%rdi),%edi
  11:   48 89 45 98             mov    %rax,-0x68(%rbp)
  15:   48 8b 82 90 00 00 00    mov    0x90(%rdx),%rax
  1c:   89 7d 94                mov    %edi,-0x6c(%rbp)
  1f:   48 8b 80 60 02 00 00    mov    0x260(%rax),%rax
  26:   48 89 45 88             mov    %rax,-0x78(%rbp)
  2a:   ac                      lods   %ds:(%rsi),%al
  2b:*  17                      (bad)       <-- trapping instruction
  2c:   00 00                   add    %al,(%rax)
  2e:   c8 45 85 e4             enterq $0x8545,$0xe4
  32:   74 30                   je     0x64
  34:   48 8b 43 10             mov    0x10(%rbx),%rax
  38:   48 8b 40 08             mov    0x8(%rax),%rax
  3c:   48 8b 40 48             mov    0x48(%rax),%rax
        ...

Code starting with the faulting instruction
===========================================

   0:   17                      (bad)
   1:   00 00                   add    %al,(%rax)
   3:   c8 45 85 e4             enterq $0x8545,$0xe4
   7:   74 30                   je     0x39
   9:   48 8b 43 10             mov    0x10(%rbx),%rax
   d:   48 8b 40 08             mov    0x8(%rax),%rax
  11:   48 8b 40 48             mov    0x48(%rax),%rax

and an instruction with opcode 0x17 in 64-bit mode is, AFAICT, invalid (on 32-bit it is "pop %ss" according to this thing:
http://www.onlinedisassembler.com).

--
Regards/Gruss,
Boris.

I can provide you with more different traces if it can help. But I thought that maybe it will be more useful for you to try it on your own. So I've prepared some minimal debian installation which you could download here (apx 163M bzipped):

http://fi.muni.cz/~xracek/debian.img.bz2

Password:
root/asdfgh

Here is my config for guest kernel:

http://fi.muni.cz/~xracek/config

I use

qemu-kvm -m 1500 -hda debian.img -kernel linux/arch/x86/boot/bzImage -append "root=/dev/sda1"

After logging in just run "sh runtest.sh". This leads to crash in my case (host: Intel Core i5-2540M, kernel 3.5.2-1.fc17.x86_64, qemu 1.0.1).

[]
Can we trim the old, large and now not-so-relevant discussion please? ;)

Quoted 16 lines

I can provide you with more different traces if it can help. But I thought that maybe it will be more useful for you to try it on your own. So I've prepared some minimal debian installation which you could download here (apx 163M bzipped):

http://fi.muni.cz/~xracek/debian.img.bz2

Password:
root/asdfgh

Here is my config for guest kernel:

http://fi.muni.cz/~xracek/config

I use

Quoted 2 lines

qemu-kvm -m 1500 -hda debian.img -kernel linux/arch/x86/boot/bzImage -append "root=/dev/sda1"

Um. I'd expect the image to be self-contained, no external kernel. I wanted to do a quick test to see if it fails on my machine too, d/loaded debian.img.bz2 but there's no kernel. So.. no quick test for you ;)

Quoted 4 lines

After logging in just run "sh runtest.sh". This leads to crash in my case (host: Intel Core i5-2540M, kernel 3.5.2-1.fc17.x86_64, qemu 1.0.1).

With all the above, this "runtest.sh" is informationally equal to your disk image.

/mjt

http://fi.muni.cz/~xracek/debian2.img.bz2

Other things are the same.

The runtest.sh sets environment for xfstests and runs test 285 which I wrote and and which should test if FS sends discard requests only on free sectors:
285:
1. Create loop device and FS on it.
2. Populate it with some garbage.
3. Get free sectors from FS.
4. Run fstrim and look for discard requests via blk tracer.
5. Compare free sectors to discard requests.

The test itself can have some issues but I'm pretty sure it shouldn't crash the system. ;-)

Quoted 13 lines · Tomas Racek

http://fi.muni.cz/~xracek/debian2.img.bz2

Other things are the same.

The runtest.sh sets environment for xfstests and runs test 285 which I wrote and and which should test if FS sends discard requests only on free sectors:
285:
1. Create loop device and FS on it.
2. Populate it with some garbage.
3. Get free sectors from FS.
4. Run fstrim and look for discard requests via blk tracer.
5. Compare free sectors to discard requests.

Quoted 1 line

The test itself can have some issues but I'm pretty sure it shouldn't crash the system. ;-)

Does the following patch help?

Quoted 19 lines · Tomas Racek

Quoted 17 lines

http://fi.muni.cz/~xracek/debian2.img.bz2

Other things are the same.

The runtest.sh sets environment for xfstests and runs test 285 which I wrote and and which should test if FS sends discard requests only on free sectors:
285:
1. Create loop device and FS on it.
2. Populate it with some garbage.
3. Get free sectors from FS.
4. Run fstrim and look for discard requests via blk tracer.
5. Compare free sectors to discard requests.

The test itself can have some issues but I'm pretty sure it shouldn't crash the system. ;-)

Quoted 2 lines

Does the following patch help?

It's obvious that it should. You're running a non-modular kernel, and those nops are discarded (probably a leftover from the days patching was a boot-only activity), so the kernel patched garbage over its own code.

-------8<----cut-here-----8<-----------------------------------

From: Avi Kivity <[email protected]>
Date: Wed, 22 Aug 2012 12:58:18 +0300
Subject: [PATCH] x86, alternative: fix p6 nops on non-modular kernels

Probably a leftover from the early days of self-patching, p6nops are marked __initconst_or_module, which causes them to be discarded in a non-modular kernel. If something later triggers patching, it will overwrite kernel code with garbage.

Reported-by Tomas Racek <[email protected]>

Signed-off-by Avi Kivity <[email protected]>

arch/x86/kernel/alternative.c +1 -1

index afb7ff7..ced4534 100644
@@ -165,7 +165,7 @@ static int __init setup_noreplace_paravirt(char
 *str)
 #endif
 
 #ifdef P6_NOP1
-static const unsigned char  __initconst_or_module p6nops[] =
+static const unsigned char p6nops[] =
 {
 	P6_NOP1,

P6_NOP2,

--
error compiling committee.c: too many arguments to function