a3e06bbe8445f57eb949e6474c5a9b30f24d2057: KVM: emulate lapic tsc deadline timer for guest"

Hi.

After a recent change, qemu --enable-kvm (both 0.15.92 and 1.0) fails to boot illumos. The OS gets stuck pretty late in the boot process, without any hints from the guest. MDB (the illumos kenrnel debugger) failed to clarify the situation at once, the kernel seems to be stuck idling. I've bisected the problem to commit a3e06bbe8445f57eb949e6474c5a9b30f24d2057. Please let me know if you need me to collect any debug information or test any patches.

Quoted 9 lines · Alexey Zaytsev

Hi.

After a recent change, qemu --enable-kvm (both 0.15.92 and 1.0) fails to boot illumos. The OS gets stuck pretty late in the boot process, without any hints from the guest. MDB (the illumos kenrnel debugger) failed to clarify the situation at once, the kernel seems to be stuck idling. I've bisected the problem to commit a3e06bbe8445f57eb949e6474c5a9b30f24d2057. Please let me know if you need me to collect any debug information or test any patches.

Alexey,

Does illumos use tsc deadline timer? and do you run it at Intel platform?

If yes, would you please help me to collect debug information by add some printk points at kvm tsc deadline timer logic? currently I totally have no clue to figure out what the issue root from :) and, would you please check illumos tsc deadline timer logic by confirm whether illumos
1). Enumerate tsc deadline timer capability by CPUID;
2). Enable tsc deadline timer mode by lapic MMIO;
3). Start tsc deadline timer by WRMSR;

Thanks,
Jinsong

Quoted 13 lines · Jinsong

Quoted 9 lines · Alexey Zaytsev

Hi.

After a recent change, qemu --enable-kvm (both 0.15.92 and 1.0) fails to boot illumos. The OS gets stuck pretty late in the boot process, without any hints from the guest. MDB (the illumos kenrnel debugger) failed to clarify the situation at once, the kernel seems to be stuck idling. I've bisected the problem to commit a3e06bbe8445f57eb949e6474c5a9b30f24d2057. Please let me know if you need me to collect any debug information or test any patches.

Alexey,

Quoted 7 lines

Does illumos use tsc deadline timer? and do you run it at Intel platform?

If yes, would you please help me to collect debug information by add some printk points at kvm tsc deadline timer logic? currently I totally have no clue to figure out what the issue root from :) and, would you please check illumos tsc deadline timer logic by confirm whether illumos
1). Enumerate tsc deadline timer capability by CPUID;
2). Enable tsc deadline timer mode by lapic MMIO;
3). Start tsc deadline timer by WRMSR;

I think, here's what's going on. We get into kvm_get_lapic_tscdeadline_msr/kvm_set_lapic_tscdeadline_msr, but qemu does not use the KVM_CREATE_IRQCHIP ioctl, so we are running without vcpu->arch.apic and the functions do nothing. And the Linux guest just handles broken tsc a lot  better, so it survives. Both guests seem to work fine with qemu-kvm, which uses the ioctl.

Quoted 27 lines · Alexey Zaytsev

Quoted 17 lines · Jinsong

Quoted 10 lines · Alexey Zaytsev

Hi.

After a recent change, qemu --enable-kvm (both 0.15.92 and 1.0) fails to boot illumos. The OS gets stuck pretty late in the boot process, without any hints from the guest. MDB (the illumos kenrnel debugger) failed to clarify the situation at once, the kernel seems to be stuck idling. I've bisected the problem to commit a3e06bbe8445f57eb949e6474c5a9b30f24d2057. Please let me know if you need me to collect any debug information or test any patches.

Alexey,

Does illumos use tsc deadline timer? and do you run it at Intel platform?

Quoted 7 lines

If yes, would you please help me to collect debug information by add some printk points at kvm tsc deadline timer logic? currently I totally have no clue to figure out what the issue root from :) and, would you please check illumos tsc deadline timer logic by confirm whether illumos 1). Enumerate tsc deadline timer capability by CPUID; 2). Enable tsc deadline timer mode by lapic MMIO; 3). Start tsc deadline timer by WRMSR;

Quoted 7 lines

I think, here's what's going on. We get into kvm_get_lapic_tscdeadline_msr/kvm_set_lapic_tscdeadline_msr, but qemu does not use the KVM_CREATE_IRQCHIP ioctl, so we are running without vcpu->arch.apic and the functions do nothing. And the Linux guest just handles broken tsc a lot  better, so it survives. Both guests seem to work fine with qemu-kvm, which uses the ioctl.

Hi. Do you need any more info on thins?

Quoted 37 lines · Alexey Zaytsev

Quoted 34 lines · Alexey Zaytsev

Quoted 17 lines · Jinsong

Quoted 10 lines · Alexey Zaytsev

Hi.

After a recent change, qemu --enable-kvm (both 0.15.92 and 1.0) fails to boot illumos. The OS gets stuck pretty late in the boot process, without any hints from the guest. MDB (the illumos kenrnel debugger) failed to clarify the situation at once, the kernel seems to be stuck idling. I've bisected the problem to commit a3e06bbe8445f57eb949e6474c5a9b30f24d2057. Please let me know if you need me to collect any debug information or test any patches.

Alexey,

Does illumos use tsc deadline timer? and do you run it at Intel platform?

Quoted 7 lines

If yes, would you please help me to collect debug information by add some printk points at kvm tsc deadline timer logic? currently I totally have no clue to figure out what the issue root from :) and, would you please check illumos tsc deadline timer logic by confirm whether illumos 1). Enumerate tsc deadline timer capability by CPUID; 2). Enable tsc deadline timer mode by lapic MMIO; 3). Start tsc deadline timer by WRMSR;

I think, here's what's going on. We get into kvm_get_lapic_tscdeadline_msr/kvm_set_lapic_tscdeadline_msr, but qemu does not use the KVM_CREATE_IRQCHIP ioctl, so we are running without vcpu->arch.apic and the functions do nothing. And the Linux guest just handles broken tsc a lot  better, so it survives. Both guests seem to work fine with qemu-kvm, which uses the ioctl.

Quoted 1 line

Hi. Do you need any more info on thins?

Seems illumos works fine w/ qemu-kvm, that's enough. Maybe you need to wait qemu-kvm sync into qemu?

Quoted 38 lines · Jinsong

Quoted 37 lines · Alexey Zaytsev

Quoted 34 lines · Alexey Zaytsev

Quoted 17 lines · Jinsong

Quoted 10 lines · Alexey Zaytsev

Hi.

After a recent change, qemu --enable-kvm (both 0.15.92 and 1.0) fails to boot illumos. The OS gets stuck pretty late in the boot process, without any hints from the guest. MDB (the illumos kenrnel debugger) failed to clarify the situation at once, the kernel seems to be stuck idling. I've bisected the problem to commit a3e06bbe8445f57eb949e6474c5a9b30f24d2057. Please let me know if you need me to collect any debug information or test any patches.

Alexey,

Does illumos use tsc deadline timer? and do you run it at Intel platform?

Quoted 7 lines

If yes, would you please help me to collect debug information by add some printk points at kvm tsc deadline timer logic? currently I totally have no clue to figure out what the issue root from :) and, would you please check illumos tsc deadline timer logic by confirm whether illumos 1). Enumerate tsc deadline timer capability by CPUID; 2). Enable tsc deadline timer mode by lapic MMIO; 3). Start tsc deadline timer by WRMSR;

I think, here's what's going on. We get into kvm_get_lapic_tscdeadline_msr/kvm_set_lapic_tscdeadline_msr, but qemu does not use the KVM_CREATE_IRQCHIP ioctl, so we are running without vcpu->arch.apic and the functions do nothing. And the Linux guest just handles broken tsc a lot  better, so it survives. Both guests seem to work fine with qemu-kvm, which uses the ioctl.

Quoted 5 lines

Quoted 1 line

Hi. Do you need any more info on thins?

Seems illumos works fine w/ qemu-kvm, that's enough. Maybe you need to wait qemu-kvm sync into qemu?

Qemu-kvm is fine for me, but this is a regression and should be fixed.

Quoted 2 lines

Qemu-kvm is fine for me, but this is a regression and should be fixed.

Sorry, I haven't your environment to debug illumos regression.
I don't think it directly related to kvm tsc deadline timer, and I think it's no value to do it now, since for linux guest it works fine w/ both qemu & qemu-kvm,
for illumos guest you just need wait qemu-kvm sync into qemu.

Quoted 46 lines · Alexey Zaytsev

Quoted 43 lines · Jinsong

Quoted 37 lines · Alexey Zaytsev

Quoted 27 lines · Alexey Zaytsev

Quoted 17 lines · Jinsong

Quoted 10 lines · Alexey Zaytsev

Hi.

After a recent change, qemu --enable-kvm (both 0.15.92 and 1.0) fails to boot illumos. The OS gets stuck pretty late in the boot process, without any hints from the guest. MDB (the illumos kenrnel debugger) failed to clarify the situation at once, the kernel seems to be stuck idling. I've bisected the problem to commit a3e06bbe8445f57eb949e6474c5a9b30f24d2057. Please let me know if you need me to collect any debug information or test any patches.

Alexey,

Does illumos use tsc deadline timer? and do you run it at Intel platform?

Quoted 7 lines

If yes, would you please help me to collect debug information by add some printk points at kvm tsc deadline timer logic? currently I totally have no clue to figure out what the issue root from :) and, would you please check illumos tsc deadline timer logic by confirm whether illumos 1). Enumerate tsc deadline timer capability by CPUID; 2). Enable tsc deadline timer mode by lapic MMIO; 3). Start tsc deadline timer by WRMSR;

Quoted 7 lines

I think, here's what's going on. We get into kvm_get_lapic_tscdeadline_msr/kvm_set_lapic_tscdeadline_msr, but qemu does not use the KVM_CREATE_IRQCHIP ioctl, so we are running without vcpu->arch.apic and the functions do nothing. And the Linux guest just handles broken tsc a lot  better, so it survives. Both guests seem to work fine with qemu-kvm, which uses the ioctl.

Quoted 1 line

Hi. Do you need any more info on thins?

Seems illumos works fine w/ qemu-kvm, that's enough. Maybe you need to wait qemu-kvm sync into qemu?

Quoted 1 line

Qemu-kvm is fine for me, but this is a regression and should be fixed.

Sorry, I haven't your environment to debug illumos regression.
I don't think it directly related to kvm tsc deadline timer, and I think it's no value to do it now, since for linux guest it works fine w/ both qemu & qemu-kvm,
for illumos guest you just need wait qemu-kvm sync into qemu.

Let me clarify the situation.

Before this commit, the tsc was advertised in cpuid, and it was handled, if I understand things correctly, by qemu.
After this commit, the tsc is advertised in cpuid, and is handled in the kernel, but only after qemu issues KVM_CREATE_IRQCHIP. If it does not issue the ioctl, the kernel just discards any wrmsrs done to the tsc. This does not look like an Illumos problem to me. Linux guests kind of work here, because they are  prepared to work on utterly broken hardware. Good for you, but please don't break less-prepared guests.

Quoted 12 lines · Alexey Zaytsev

Let me clarify the situation.

Before this commit, the tsc was advertised in cpuid, and it was handled, if I understand things correctly, by qemu.
After this commit, the tsc is advertised in cpuid, and is handled in the kernel, but only after qemu issues KVM_CREATE_IRQCHIP. If it does not issue the ioctl, the kernel just discards any wrmsrs done to the tsc. This does not look like an Illumos problem to me. Linux guests kind of work here, because they are  prepared to work on utterly broken hardware. Good for you, but please don't break less-prepared guests.

Yes. This is a regression, and needs to be fixed.

Liu, if you don't have time to debug it, we'll just revert the commit. It's that easy. Regressions are not allowed. There are no excuses.

In particular, saying "just wait for qemu-kvm" is not an acceptable answer, because the point is that things *used* to work, and they broke. No "change it to work with the new kernel" allowed, except for some *very* rare critical circumstances (usually "major security-bug that we had to fix, and people who relied on it are thus out of luck").

Commit a3e06bbe8445 still seems to revert cleanly, so that is the easy option.

That said, it sounds like maybe another solution is to start with the TSC_DEADLINE timer bit in cpuid cleared, and only setting it after the KVM_CREATE_IRQCHIP ioctl.

In fact, the patch is clearly buggy, in that it apparently doesn't emulate TSC_DEADLINE correctly and natively on its own.

Jan, Marcelo, Avi - is there a quick fix, or should I just revert?

And please don't *EVER* tell people that they should just work around regressions. Regressions are absolutely unacceptable. Kernel people need to understand that.

Linus

Quoted 37 lines · Linus Torvalds

Quoted 12 lines · Alexey Zaytsev

Let me clarify the situation.

Before this commit, the tsc was advertised in cpuid, and it was handled, if I understand things correctly, by qemu.
After this commit, the tsc is advertised in cpuid, and is handled in the kernel, but only after qemu issues KVM_CREATE_IRQCHIP. If it does not issue the ioctl, the kernel just discards any wrmsrs done to the tsc. This does not look like an Illumos problem to me. Linux guests kind of work here, because they are  prepared to work on utterly broken hardware. Good for you, but please don't break less-prepared guests.

Yes. This is a regression, and needs to be fixed.

Liu, if you don't have time to debug it, we'll just revert the commit. It's that easy. Regressions are not allowed. There are no excuses.

In particular, saying "just wait for qemu-kvm" is not an acceptable answer, because the point is that things *used* to work, and they broke. No "change it to work with the new kernel" allowed, except for some *very* rare critical circumstances (usually "major security-bug that we had to fix, and people who relied on it are thus out of luck").

Commit a3e06bbe8445 still seems to revert cleanly, so that is the easy option.

That said, it sounds like maybe another solution is to start with the TSC_DEADLINE timer bit in cpuid cleared, and only setting it after the KVM_CREATE_IRQCHIP ioctl.

In fact, the patch is clearly buggy, in that it apparently doesn't emulate TSC_DEADLINE correctly and natively on its own.

Quoted 7 lines

Jan, Marcelo, Avi - is there a quick fix, or should I just revert?

And please don't *EVER* tell people that they should just work around regressions. Regressions are absolutely unacceptable. Kernel people need to understand that.

                   Linus

Yes, my fault to say 'walk around' before knowing Alex's issue clearly.

After Alex send his last email to clarify the situation, I have checked the bug.
Basically it caused from
1. qemu didn't issue KVM_CREATE_IRQCHIP, hence irqchip_in_kernel(kvm) fail when setup vcpu lapic logic at kvm_arch_vcpu_init();
2. tsc deadline work based on vcpu lapic, hence break illumos;

A fix is to update cpuid, as you said, setting it after KVM_CREATE_IRQCHIP. I just wonder is there any better solution? so I ask Alex his environment to setup at my side to do more test. If you think kvm tsc deadline timer patch itself not clean, please tell me.

Quoted 58 lines · Jinsong

Quoted 38 lines · Linus Torvalds

Quoted 12 lines · Alexey Zaytsev

Let me clarify the situation.

Before this commit, the tsc was advertised in cpuid, and it was handled, if I understand things correctly, by qemu. After this commit, the tsc is advertised in cpuid, and is handled in the kernel, but only after qemu issues KVM_CREATE_IRQCHIP. If it does not issue the ioctl, the kernel just discards any wrmsrs done to the tsc. This does not look like an Illumos problem to me. Linux guests kind of work here, because they are  prepared to work on utterly broken hardware. Good for you, but please don't break less-prepared guests.

Yes. This is a regression, and needs to be fixed.

Liu, if you don't have time to debug it, we'll just revert the commit. It's that easy. Regressions are not allowed. There are no excuses.

In particular, saying "just wait for qemu-kvm" is not an acceptable answer, because the point is that things *used* to work, and they broke. No "change it to work with the new kernel" allowed, except for some *very* rare critical circumstances (usually "major security-bug that we had to fix, and people who relied on it are thus out of luck").

Commit a3e06bbe8445 still seems to revert cleanly, so that is the easy option.

That said, it sounds like maybe another solution is to start with the TSC_DEADLINE timer bit in cpuid cleared, and only setting it after the KVM_CREATE_IRQCHIP ioctl.

In fact, the patch is clearly buggy, in that it apparently doesn't emulate TSC_DEADLINE correctly and natively on its own.

Quoted 7 lines

Jan, Marcelo, Avi - is there a quick fix, or should I just revert?

And please don't *EVER* tell people that they should just work around regressions. Regressions are absolutely unacceptable. Kernel people need to understand that.

                   Linus

Yes, my fault to say 'walk around' before knowing Alex's issue clearly.

After Alex send his last email to clarify the situation, I have checked the bug.
Basically it caused from
1. qemu didn't issue KVM_CREATE_IRQCHIP, hence irqchip_in_kernel(kvm) fail when setup vcpu lapic logic at kvm_arch_vcpu_init();
2. tsc deadline work based on vcpu lapic, hence break illumos;

Quoted 4 lines

A fix is to update cpuid, as you said, setting it after KVM_CREATE_IRQCHIP. I just wonder is there any better solution? so I ask Alex his environment to setup at my side to do more test. If you think kvm tsc deadline timer patch itself not clean, please tell me.

[Removed Linus from CC]

If your internet connection is good enough, I could just pass you the disk image. It should be around 5g when compressed. Otherwise, you can download an openindiana disk image from http://openindiana.org. You probably want the text-only "server" one. The TSC code has changed since that release, but the old one does not work as well.

Do I get it right that Linux does not use the tsc deadline timer? I've added printks to the kvm_get_lapic_tscdeadline_msr/kvm_set_lapic_tscdeadline_msr functions, and I swear I've seen them trigger a few times with the Linux guest. But I can't find the code that is issuing the msr reads/writes in the Linux kernel.

Quoted 65 lines · Alexey Zaytsev

Quoted 58 lines · Jinsong

Quoted 38 lines · Linus Torvalds

Quoted 12 lines · Alexey Zaytsev

Let me clarify the situation.

Before this commit, the tsc was advertised in cpuid, and it was handled, if I understand things correctly, by qemu. After this commit, the tsc is advertised in cpuid, and is handled in the kernel, but only after qemu issues KVM_CREATE_IRQCHIP. If it does not issue the ioctl, the kernel just discards any wrmsrs done to the tsc. This does not look like an Illumos problem to me. Linux guests kind of work here, because they are  prepared to work on utterly broken hardware. Good for you, but please don't break less-prepared guests.

Yes. This is a regression, and needs to be fixed.

Liu, if you don't have time to debug it, we'll just revert the commit. It's that easy. Regressions are not allowed. There are no excuses.

In particular, saying "just wait for qemu-kvm" is not an acceptable answer, because the point is that things *used* to work, and they broke. No "change it to work with the new kernel" allowed, except for some *very* rare critical circumstances (usually "major security-bug that we had to fix, and people who relied on it are thus out of luck").

Commit a3e06bbe8445 still seems to revert cleanly, so that is the easy option.

That said, it sounds like maybe another solution is to start with the TSC_DEADLINE timer bit in cpuid cleared, and only setting it after the KVM_CREATE_IRQCHIP ioctl.

In fact, the patch is clearly buggy, in that it apparently doesn't emulate TSC_DEADLINE correctly and natively on its own.

Quoted 7 lines

Jan, Marcelo, Avi - is there a quick fix, or should I just revert?

And please don't *EVER* tell people that they should just work around regressions. Regressions are absolutely unacceptable. Kernel people need to understand that.

                   Linus

Yes, my fault to say 'walk around' before knowing Alex's issue clearly.

After Alex send his last email to clarify the situation, I have checked the bug.
Basically it caused from
1. qemu didn't issue KVM_CREATE_IRQCHIP, hence irqchip_in_kernel(kvm) fail when setup vcpu lapic logic at kvm_arch_vcpu_init();
2. tsc deadline work based on vcpu lapic, hence break illumos;

Quoted 4 lines

A fix is to update cpuid, as you said, setting it after KVM_CREATE_IRQCHIP. I just wonder is there any better solution? so I ask Alex his environment to setup at my side to do more test. If you think kvm tsc deadline timer patch itself not clean, please tell me.

Quoted 7 lines

[Removed Linus from CC]

If your internet connection is good enough, I could just pass you the disk image. It should be around 5g when compressed. Otherwise, you can download an openindiana disk image from http://openindiana.org. You probably want the text-only "server" one. The TSC code has changed since that release, but the old one does not work as well.

I mean I will build environment at my side with your qemu version (where do you pull from? commit number?) --> just to verify whether we can solve it at qemu side, or, solve it by cpuid at kvm side. I will present a patch to fix it and you can test at your environment.

Quoted 7 lines

Do I get it right that Linux does not use the tsc deadline timer? I've added printks to the kvm_get_lapic_tscdeadline_msr/kvm_set_lapic_tscdeadline_msr functions, and I swear I've seen them trigger a few times with the Linux guest. But I can't find the code that is issuing the msr reads/writes in the Linux kernel.

I will check it tomorrow, too late now.
and would you please tell me, does illumos use tsc deadline timer? I lack this basic information.

Thanks,
Jinsong

Quoted 88 lines · Jinsong

Quoted 65 lines · Alexey Zaytsev

Quoted 57 lines · Jinsong

Quoted 38 lines · Linus Torvalds

Quoted 12 lines · Alexey Zaytsev

Let me clarify the situation.

Before this commit, the tsc was advertised in cpuid, and it was handled, if I understand things correctly, by qemu. After this commit, the tsc is advertised in cpuid, and is handled in the kernel, but only after qemu issues KVM_CREATE_IRQCHIP. If it does not issue the ioctl, the kernel just discards any wrmsrs done to the tsc. This does not look like an Illumos problem to me. Linux guests kind of work here, because they are  prepared to work on utterly broken hardware. Good for you, but please don't break less-prepared guests.

Yes. This is a regression, and needs to be fixed.

Liu, if you don't have time to debug it, we'll just revert the commit. It's that easy. Regressions are not allowed. There are no excuses.

In particular, saying "just wait for qemu-kvm" is not an acceptable answer, because the point is that things *used* to work, and they broke. No "change it to work with the new kernel" allowed, except for some *very* rare critical circumstances (usually "major security-bug that we had to fix, and people who relied on it are thus out of luck").

Commit a3e06bbe8445 still seems to revert cleanly, so that is the easy option.

That said, it sounds like maybe another solution is to start with the TSC_DEADLINE timer bit in cpuid cleared, and only setting it after the KVM_CREATE_IRQCHIP ioctl.

In fact, the patch is clearly buggy, in that it apparently doesn't emulate TSC_DEADLINE correctly and natively on its own.

Quoted 7 lines

Jan, Marcelo, Avi - is there a quick fix, or should I just revert?

And please don't *EVER* tell people that they should just work around regressions. Regressions are absolutely unacceptable. Kernel people need to understand that.

                   Linus

Yes, my fault to say 'walk around' before knowing Alex's issue clearly.

After Alex send his last email to clarify the situation, I have checked the bug. Basically it caused from
1. qemu didn't issue KVM_CREATE_IRQCHIP, hence irqchip_in_kernel(kvm) fail when setup vcpu lapic logic at kvm_arch_vcpu_init();
2. tsc deadline work based on vcpu lapic, hence break illumos;

Quoted 5 lines

A fix is to update cpuid, as you said, setting it after KVM_CREATE_IRQCHIP. I just wonder is there any better solution? so I ask Alex his environment to setup at my side to do more test. If you think kvm tsc deadline timer patch itself not clean, please tell me.

Quoted 8 lines

[Removed Linus from CC]

If your internet connection is good enough, I could just pass you the disk image. It should be around 5g when compressed. Otherwise, you can download an openindiana disk image from
http://openindiana.org. You probably want the text-only "server" one. The TSC code has changed since that release, but the old one does not work as well.

I mean I will build environment at my side with your qemu version (where do you pull from? commit number?) --> just to verify whether we can solve it at qemu side, or, solve it by cpuid at kvm side. I will present a patch to fix it and you can test at your environment.

Quoted 7 lines

Do I get it right that Linux does not use the tsc deadline timer? I've added printks to the kvm_get_lapic_tscdeadline_msr/kvm_set_lapic_tscdeadline_msr functions, and I swear I've seen them trigger a few times with the Linux guest. But I can't find the code that is issuing the msr reads/writes in the Linux kernel.

Quoted 3 lines

I will check it tomorrow, too late now.
and would you please tell me, does illumos use tsc deadline timer? I lack this basic information.

It does, if available. Please check
https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/i86pc/io/pcplusmp/apic_timer.c#L290

Quoted 94 lines · Alexey Zaytsev

Quoted 91 lines · Jinsong

Quoted 65 lines · Alexey Zaytsev

Quoted 57 lines · Jinsong

Quoted 38 lines · Linus Torvalds

Quoted 12 lines · Alexey Zaytsev

Let me clarify the situation.

Before this commit, the tsc was advertised in cpuid, and it was handled, if I understand things correctly, by qemu. After this commit, the tsc is advertised in cpuid, and is handled in the kernel, but only after qemu issues KVM_CREATE_IRQCHIP. If it does not issue the ioctl, the kernel just discards any wrmsrs done to the tsc. This does not look like an Illumos problem to me. Linux guests kind of work here, because they are  prepared to work on utterly broken hardware. Good for you, but please don't break less-prepared guests.

Yes. This is a regression, and needs to be fixed.

Liu, if you don't have time to debug it, we'll just revert the commit. It's that easy. Regressions are not allowed. There are no excuses.

In particular, saying "just wait for qemu-kvm" is not an acceptable answer, because the point is that things *used* to work, and they broke. No "change it to work with the new kernel" allowed, except for some *very* rare critical circumstances (usually "major security-bug that we had to fix, and people who relied on it are thus out of luck").

Commit a3e06bbe8445 still seems to revert cleanly, so that is the easy option.

That said, it sounds like maybe another solution is to start with the TSC_DEADLINE timer bit in cpuid cleared, and only setting it after the KVM_CREATE_IRQCHIP ioctl.

In fact, the patch is clearly buggy, in that it apparently doesn't emulate TSC_DEADLINE correctly and natively on its own.

Quoted 7 lines

Jan, Marcelo, Avi - is there a quick fix, or should I just revert?

And please don't *EVER* tell people that they should just work around regressions. Regressions are absolutely unacceptable. Kernel people need to understand that.

                   Linus

Yes, my fault to say 'walk around' before knowing Alex's issue clearly.

After Alex send his last email to clarify the situation, I have checked the bug. Basically it caused from
1. qemu didn't issue KVM_CREATE_IRQCHIP, hence irqchip_in_kernel(kvm) fail when setup vcpu lapic logic at kvm_arch_vcpu_init();
2. tsc deadline work based on vcpu lapic, hence break illumos;

Quoted 5 lines

A fix is to update cpuid, as you said, setting it after KVM_CREATE_IRQCHIP. I just wonder is there any better solution? so I ask Alex his environment to setup at my side to do more test. If you think kvm tsc deadline timer patch itself not clean, please tell me.

Quoted 8 lines

[Removed Linus from CC]

If your internet connection is good enough, I could just pass you the disk image. It should be around 5g when compressed. Otherwise, you can download an openindiana disk image from
http://openindiana.org. You probably want the text-only "server" one. The TSC code has changed since that release, but the old one does not work as well.

I mean I will build environment at my side with your qemu version (where do you pull from? commit number?) --> just to verify whether we can solve it at qemu side, or, solve it by cpuid at kvm side. I will present a patch to fix it and you can test at your environment.

Quoted 7 lines

Do I get it right that Linux does not use the tsc deadline timer? I've added printks to the kvm_get_lapic_tscdeadline_msr/kvm_set_lapic_tscdeadline_msr functions, and I swear I've seen them trigger a few times with the Linux guest. But I can't find the code that is issuing the msr reads/writes in the Linux kernel.

I will check it tomorrow, too late now.
and would you please tell me, does illumos use tsc deadline timer? I lack this basic information.

Quoted 3 lines

It does, if available. Please check
https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/i86pc/io/pcplusmp/apic_timer.c#L290

and where do you pull qemu? and commit number? I usually use Avi's qemu-kvm.

From git://git.qemu.org/qemu.git, commit

Thanks,
Jinsong

Quoted 101 lines · Jinsong

Quoted 99 lines · Alexey Zaytsev

Quoted 90 lines · Jinsong

Quoted 66 lines · Alexey Zaytsev

Quoted 58 lines · Jinsong

Quoted 38 lines · Linus Torvalds

Quoted 12 lines · Alexey Zaytsev

Let me clarify the situation.

Before this commit, the tsc was advertised in cpuid, and it was handled, if I understand things correctly, by qemu. After this commit, the tsc is advertised in cpuid, and is handled in the kernel, but only after qemu issues KVM_CREATE_IRQCHIP. If it does not issue the ioctl, the kernel just discards any wrmsrs done to the tsc. This does not look like an Illumos problem to me. Linux guests kind of work here, because they are  prepared to work on utterly broken hardware. Good for you, but please don't break less-prepared guests.

Yes. This is a regression, and needs to be fixed.

Liu, if you don't have time to debug it, we'll just revert the commit. It's that easy. Regressions are not allowed. There are no excuses.

In particular, saying "just wait for qemu-kvm" is not an acceptable answer, because the point is that things *used* to work, and they broke. No "change it to work with the new kernel" allowed, except for some *very* rare critical circumstances (usually "major security-bug that we had to fix, and people who relied on it are thus out of luck").

Commit a3e06bbe8445 still seems to revert cleanly, so that is the easy option.

That said, it sounds like maybe another solution is to start with the TSC_DEADLINE timer bit in cpuid cleared, and only setting it after the KVM_CREATE_IRQCHIP ioctl.

In fact, the patch is clearly buggy, in that it apparently doesn't emulate TSC_DEADLINE correctly and natively on its own.

Quoted 8 lines

Jan, Marcelo, Avi - is there a quick fix, or should I just revert?

And please don't *EVER* tell people that they should just work around regressions. Regressions are absolutely unacceptable. Kernel people need to understand that.

                   Linus

Yes, my fault to say 'walk around' before knowing Alex's issue clearly.

After Alex send his last email to clarify the situation, I have checked the bug. Basically it caused from
1. qemu didn't issue KVM_CREATE_IRQCHIP, hence irqchip_in_kernel(kvm) fail when setup vcpu lapic logic at kvm_arch_vcpu_init();
2. tsc deadline work based on vcpu lapic, hence break illumos;

Quoted 5 lines

A fix is to update cpuid, as you said, setting it after KVM_CREATE_IRQCHIP. I just wonder is there any better solution? so I ask Alex his environment to setup at my side to do more test. If you think kvm tsc deadline timer patch itself not clean, please tell me.

Quoted 8 lines

[Removed Linus from CC]

If your internet connection is good enough, I could just pass you the disk image. It should be around 5g when compressed. Otherwise, you can download an openindiana disk image from
http://openindiana.org. You probably want the text-only "server" one. The TSC code has changed since that release, but the old one does not work as well.

I mean I will build environment at my side with your qemu version (where do you pull from? commit number?) --> just to verify whether we can solve it at qemu side, or, solve it by cpuid at kvm side. I will present a patch to fix it and you can test at your environment.

Quoted 7 lines

Do I get it right that Linux does not use the tsc deadline timer? I've added printks to the kvm_get_lapic_tscdeadline_msr/kvm_set_lapic_tscdeadline_msr functions, and I swear I've seen them trigger a few times with the Linux guest. But I can't find the code that is issuing the msr reads/writes in the Linux kernel.

Quoted 3 lines

I will check it tomorrow, too late now.
and would you please tell me, does illumos use tsc deadline timer? I lack this basic information.

It does, if available. Please check
https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/i86pc/io/pcplusmp/apic_timer.c#L290

Quoted 2 lines

and where do you pull qemu? and commit number? I usually use Avi's qemu-kvm.

From git://git.qemu.org/qemu.git, commit
62ba9f3662c5481f31160daf25474c8f22d6b3d2
Qemu 0.15.92 exhibited the same behavior.

Yes, my fault to say 'walk around' before knowing Alex's issue clearly.

After Alex send his last email to clarify the situation, I have checked the bug.
Basically it caused from
1. qemu didn't issue KVM_CREATE_IRQCHIP, hence irqchip_in_kernel(kvm) fail when setup vcpu lapic logic at kvm_arch_vcpu_init();
2. tsc deadline work based on vcpu lapic, hence break illumos;

A fix is to update cpuid, as you said, setting it after KVM_CREATE_IRQCHIP.

I just wonder is there any better solution? so I ask Alex his environment to setup at my side to do more test. If you think kvm tsc deadline timer patch itself not clean, please tell me.

Quoted 12 lines · Alexey Zaytsev

Let me clarify the situation.

Before this commit, the tsc was advertised in cpuid, and it was handled, if I understand things correctly, by qemu.
After this commit, the tsc is advertised in cpuid, and is handled in the kernel, but only after qemu issues KVM_CREATE_IRQCHIP. If it does not issue the ioctl, the kernel just discards any wrmsrs done to the tsc. This does not look like an Illumos problem to me. Linux guests kind of work here, because they are prepared to work on utterly broken hardware. Good for you, but please don't break less-prepared guests.

Yes. This is a regression, and needs to be fixed.

Liu, if you don't have time to debug it, we'll just revert the commit. It's that easy. Regressions are not allowed. There are no excuses.

In particular, saying "just wait for qemu-kvm" is not an acceptable answer, because the point is that things *used* to work, and they broke. No "change it to work with the new kernel" allowed, except for some *very* rare critical circumstances (usually "major security-bug that we had to fix, and people who relied on it are thus out of luck").

Commit a3e06bbe8445 still seems to revert cleanly, so that is the easy option.

That said, it sounds like maybe another solution is to start with the TSC_DEADLINE timer bit in cpuid cleared, and only setting it after the KVM_CREATE_IRQCHIP ioctl.

In fact, the patch is clearly buggy, in that it apparently doesn't emulate TSC_DEADLINE correctly and natively on its own.

Jan, Marcelo, Avi - is there a quick fix, or should I just revert?

Quoted 27 lines · Linus Torvalds

Quoted 12 lines · Alexey Zaytsev

Let me clarify the situation.

Before this commit, the tsc was advertised in cpuid, and it was handled, if I understand things correctly, by qemu.
After this commit, the tsc is advertised in cpuid, and is handled in the kernel, but only after qemu issues KVM_CREATE_IRQCHIP. If it does not issue the ioctl, the kernel just discards any wrmsrs done to the tsc. This does not look like an Illumos problem to me. Linux guests kind of work here, because they are prepared to work on utterly broken hardware. Good for you, but please don't break less-prepared guests.

Yes. This is a regression, and needs to be fixed.

Liu, if you don't have time to debug it, we'll just revert the commit. It's that easy. Regressions are not allowed. There are no excuses.

In particular, saying "just wait for qemu-kvm" is not an acceptable answer, because the point is that things *used* to work, and they broke. No "change it to work with the new kernel" allowed, except for some *very* rare critical circumstances (usually "major security-bug that we had to fix, and people who relied on it are thus out of luck").

Quoted 5 lines

Commit a3e06bbe8445 still seems to revert cleanly, so that is the easy option.

That said, it sounds like maybe another solution is to start with the TSC_DEADLINE timer bit in cpuid cleared, and only setting it after the KVM_CREATE_IRQCHIP ioctl.

KVM cpuid discovery can (and is) unfortunately be done before user space decides about creating an in-kernel irqchip or not. But the patch in question suggests to user space that it's optimal and perfectly fine to expose TSC_DEADLINE_TIMER to the guest - obviously a wrong suggestion in case user space does not use the in-kernel irqchip and does not implement the deadline timer in its own APIC model.

So the only proper solution I see is to drop that feature flag exposure from kvm_update_cpuid. Instead, KVM should indicate to *well informed* (i.e. updated) user space differently that there is now deadline timer support in the in-kernel APIC model. That different channel should be a KVM_CAP flag that user space can easily check and then merge the necessary flag on its own.

Quoted 1 line

Quoted 4 lines

In fact, the patch is clearly buggy, in that it apparently doesn't emulate TSC_DEADLINE correctly and natively on its own.

Jan, Marcelo, Avi - is there a quick fix, or should I just revert?

Maybe fixing can be done quickly. In any case, the current ABI should not be exposed to user space, even at the price of postponing it's introduction. Just my 2 cents, though.

We must not report the TSC deadline timer feature on our own when user space provides the APIC as we have no clue about its features.

Quoted 2 lines · Jan Kiszka

We must not report the TSC deadline timer feature on our own when user space provides the APIC as we have no clue about its features.

We must not report the TSC deadline timer feature on our own, period. We should just update the timer mode mask there. Don't know how this slipped through review.

I think your original idea was correct. Add a new KVM_CAP for the tsc deadline timer. Userspace can add the bit to cpuid if either it implements the feature in a userspace apic, or if it finds the new capability and uses the kernel apic.

Quoted 4 lines · Avi Kivity

Quoted 2 lines · Jan Kiszka

We must not report the TSC deadline timer feature on our own when user space provides the APIC as we have no clue about its features.

Quoted 8 lines

We must not report the TSC deadline timer feature on our own, period. We should just update the timer mode mask there. Don't know how this slipped through review.

I think your original idea was correct. Add a new KVM_CAP for the tsc deadline timer. Userspace can add the bit to cpuid if either it implements the feature in a userspace apic, or if it finds the new capability and uses the kernel apic.

Right, we do need some control for user space to keep the feature disabled when migrating from an older host.

However, there is also the timer_mode_mask which requires tuning in addition to the cpuid flag.

Quoted 17 lines · Jan Kiszka

Quoted 4 lines · Avi Kivity

Quoted 2 lines · Jan Kiszka

We must not report the TSC deadline timer feature on our own when user space provides the APIC as we have no clue about its features.

Quoted 8 lines

We must not report the TSC deadline timer feature on our own, period. We should just update the timer mode mask there. Don't know how this slipped through review.

I think your original idea was correct. Add a new KVM_CAP for the tsc deadline timer. Userspace can add the bit to cpuid if either it implements the feature in a userspace apic, or if it finds the new capability and uses the kernel apic.

Right, we do need some control for user space to keep the feature disabled when migrating from an older host.

Quoted 3 lines

However, there is also the timer_mode_mask which requires tuning in addition to the cpuid flag.

timer_mode_mask can just be slaved to the bit (as received by KVM_SET_CPUID); that's exactly kvm_update_cpuid()'s role.

Quoted 3 lines

timer_mode_mask can just be slaved to the bit (as received by KVM_SET_CPUID); that's exactly kvm_update_cpuid()'s role.

Like

arch/x86/kvm/cpuid.c +2 -10

index 230f713..89b02bf 100644
@@ -27,7 +27,6 @@ void kvm_update_cpuid(struct kvm_vcpu *vcpu)
 {
 	struct kvm_cpuid_entry2 *best;
 	struct kvm_lapic *apic = vcpu->arch.apic;
-	u32 timer_mode_mask;
 
 	best = kvm_find_cpuid_entry(vcpu, 1, 0);
 	if (!best)

@@ -40,15 +39,12 @@ void kvm_update_cpuid(struct kvm_vcpu *vcpu)
 			best->ecx |= bit(X86_FEATURE_OSXSAVE);
 	}
 
-	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
-		best->function == 0x1) {
-		best->ecx |= bit(X86_FEATURE_TSC_DEADLINE_TIMER);
-		timer_mode_mask = 3 << 17;
-	} else
-		timer_mode_mask = 1 << 17;
-
-	if (apic)
-		apic->lapic_timer.timer_mode_mask = timer_mode_mask;
+	if (apic) {
+		if (best->ecx & bit(X86_FEATURE_TSC_DEADLINE_TIMER))

Quoted 5 lines · Jan Kiszka

Quoted 3 lines

timer_mode_mask can just be slaved to the bit (as received by KVM_SET_CPUID); that's exactly kvm_update_cpuid()'s role.

Like

Ugh, the facebook generation invades lkml.

Quoted 33 lines

arch/x86/kvm/cpuid.c +6 -10

index 230f713..89b02bf 100644
@@ -27,7 +27,6 @@ void kvm_update_cpuid(struct kvm_vcpu *vcpu)
 {
 	struct kvm_cpuid_entry2 *best;
 	struct kvm_lapic *apic = vcpu->arch.apic;
-	u32 timer_mode_mask;
 
 	best = kvm_find_cpuid_entry(vcpu, 1, 0);
 	if (!best)

@@ -40,15 +39,12 @@ void kvm_update_cpuid(struct kvm_vcpu *vcpu)
 			best->ecx |= bit(X86_FEATURE_OSXSAVE);
 	}
 
-	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
-		best->function == 0x1) {
-		best->ecx |= bit(X86_FEATURE_TSC_DEADLINE_TIMER);
-		timer_mode_mask = 3 << 17;
-	} else
-		timer_mode_mask = 1 << 17;
-
-	if (apic)
-		apic->lapic_timer.timer_mode_mask = timer_mode_mask;
+	if (apic) {
+		if (best->ecx & bit(X86_FEATURE_TSC_DEADLINE_TIMER))
+			apic->lapic_timer.timer_mode_mask = 3 << 17;
+		else
+			apic->lapic_timer.timer_mode_mask = 1 << 17;
+	}
 

arch/x86/kvm/cpuid.c void kvm_update_cpuid(struct kvm_vcpu *vcpu)

Quoted 6 lines

arch/x86/kvm/cpuid.c +1 -0

 	kvm_pmu_cpuid_update(vcpu);
 }
 
 
+ the KVM_CAP thing?

+1

Quoted 1 line

+1

Ah, the adult crowd that trusts the other side.

OK, will see that I can cook up a proper patch later.

Quoted 2 lines · Jan Kiszka

We must not report the TSC deadline timer feature on our own when user space provides the APIC as we have no clue about its features.

We must not report the TSC deadline timer feature on our own, period. We should just update the timer mode mask there. Don't know how this slipped through review.

I think your original idea was correct. Add a new KVM_CAP for the tsc deadline timer. Userspace can add the bit to cpuid if either it implements the feature in a userspace apic, or if it finds the new capability and uses the kernel apic.

Quoted 4 lines · Avi Kivity

Quoted 2 lines · Jan Kiszka

We must not report the TSC deadline timer feature on our own when user space provides the APIC as we have no clue about its features.

Quoted 8 lines

We must not report the TSC deadline timer feature on our own, period. We should just update the timer mode mask there. Don't know how this slipped through review.

I think your original idea was correct. Add a new KVM_CAP for the tsc deadline timer. Userspace can add the bit to cpuid if either it implements the feature in a userspace apic, or if it finds the new capability and uses the kernel apic.

Is it necessary to use KVM_CAP? If I didn't misunderstand, the KVM_CAP sulotion would be:
1. qemu get kvm tsc deadline timer capability by KVM_CAP_...;
2. qemu add cpuid bit

        if ((guest use qemu apic && qemu emualte tsc deadline timer) || 
           (guest use kvm apic && kvm emulate tsc deadline timer (KVM_CAP)))

3. qemu ioctl KVM_SET_CPUID2
4. kvm expose the feature to guest by saving it at vcpu->arch.cpuid_entries,

seems it's logically redundant.

Jan's patch v2 is a straight forward and simple fix. in the patch

    if (apic) { ... }

means apic (and then its sub-logic tsc deadline timer) emulated by kvm, that's enough:
if quest use kvm apic, it's OK to add cpuid bit and expose to guest;
if guest don't use kvm apic, it will not touch cpuid bit;

Quoted 18 lines · Jinsong

Quoted 4 lines · Avi Kivity

Quoted 2 lines · Jan Kiszka

We must not report the TSC deadline timer feature on our own when user space provides the APIC as we have no clue about its features.

Quoted 8 lines

We must not report the TSC deadline timer feature on our own, period. We should just update the timer mode mask there. Don't know how this slipped through review.

I think your original idea was correct. Add a new KVM_CAP for the tsc deadline timer. Userspace can add the bit to cpuid if either it implements the feature in a userspace apic, or if it finds the new capability and uses the kernel apic.

Is it necessary to use KVM_CAP? If I didn't misunderstand, the KVM_CAP sulotion would be:
1. qemu get kvm tsc deadline timer capability by KVM_CAP_...;
2. qemu add cpuid bit

Quoted 6 lines

        if ((guest use qemu apic && qemu emualte tsc deadline timer)
           || (guest use kvm apic && kvm emulate tsc deadline timer

(KVM_CAP)))
3. qemu ioctl KVM_SET_CPUID2
4. kvm expose the feature to guest by saving it at vcpu->arch.cpuid_entries,

Correct.

Quoted 1 line

seems it's logically redundant.

What's logically redundant?

Quoted 6 lines

Jan's patch v2 is a straight forward and simple fix. in the patch if (apic) { ... } means apic (and then its sub-logic tsc deadline timer) emulated by kvm, that's enough: if quest use kvm apic, it's OK to add cpuid bit and expose to guest;
if guest don't use kvm apic, it will not touch cpuid bit;

It breaks live migration: if you start a guest on a TSC-deadline capable host kernel, and migrate it to a TSC-deadline incapable host kernel, you end up with a broken guest.

More broadly, kvm never exposes features transparently to the guest, it always passes them to userspace first, so userspace controls the ABI exposed to the guest. This prevents the following scenario:

- a guest is started on some hardware, which doesn't support some cpuid feature (say AVX for example)
- the guest or one of its applications are broken wrt AVX, but because the feature is not exposed, it works correctly
- the host hardware is upgraded to one which supports AVX
- the guest is now broken

Quoted 8 lines

It breaks live migration: if you start a guest on a TSC-deadline capable host kernel, and migrate it to a TSC-deadline incapable host kernel, you end up with a broken guest.

More broadly, kvm never exposes features transparently to the guest, it always passes them to userspace first, so userspace controls the ABI exposed to the guest. This prevents the following scenario:

Do you mean, by the method qemu control cpuid exposing, it can avoid live migration broken issue by
1. user probe the lowest ability host of whole pool where vm may live migrate;
2. only if the lowest ablility host support the feature can user enable the feature when boot a vm;
3. if the lowest ability host didn't support the feature (say tsc deadline timer as example), user disable the feature when boot a vm; In this way, live migration wouldn't be broken. Right?

or, do you mean qemu-kvm solve live migration broken issue by some other method?

Quoted 8 lines

- a guest is started on some hardware, which doesn't support some cpuid feature (say AVX for example)
- the guest or one of its applications are broken wrt AVX, but because the feature is not exposed, it works correctly
- the host hardware is upgraded to one which supports AVX
- the guest is now broken

You mean, live migrate from 'old' (which doesn't support the feature) platform to 'new' platform would broken?

Quoted 9 lines · Jinsong

Quoted 8 lines

It breaks live migration: if you start a guest on a TSC-deadline capable host kernel, and migrate it to a TSC-deadline incapable host kernel, you end up with a broken guest.

More broadly, kvm never exposes features transparently to the guest, it always passes them to userspace first, so userspace controls the ABI exposed to the guest. This prevents the following scenario:

Quoted 9 lines

Do you mean, by the method qemu control cpuid exposing, it can avoid live migration broken issue by
1. user probe the lowest ability host of whole pool where vm may live migrate;
2. only if the lowest ablility host support the feature can user enable the feature when boot a vm;
3. if the lowest ability host didn't support the feature (say tsc deadline timer as example), user disable the feature when boot a vm; In this way, live migration wouldn't be broken. Right?

Right.

Quoted 3 lines

or, do you mean qemu-kvm solve live migration broken issue by some other method?

The method you outlined, or any other method, such as partitioning the cluster according to hardware capabilities.

Quoted 11 lines

Quoted 8 lines

- a guest is started on some hardware, which doesn't support some cpuid feature (say AVX for example)
- the guest or one of its applications are broken wrt AVX, but because the feature is not exposed, it works correctly
- the host hardware is upgraded to one which supports AVX
- the guest is now broken

You mean, live migrate from 'old' (which doesn't support the feature) platform to 'new' platform would broken?

Live migration, or even just a guest restart on updated hardware.

We must not report the TSC deadline timer feature on our own when user space provides the APIC as we have no clue about its features.

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v2:
 - reorganized code to make the flow clearer

Quoted 48 lines · Jinsong

Quoted 47 lines · Alexey Zaytsev

Quoted 44 lines · Jinsong

Quoted 38 lines · Alexey Zaytsev

Quoted 27 lines · Alexey Zaytsev

Quoted 17 lines · Jinsong

Quoted 10 lines · Alexey Zaytsev

Hi.

After a recent change, qemu --enable-kvm (both 0.15.92 and 1.0) fails to boot illumos. The OS gets stuck pretty late in the boot process, without any hints from the guest. MDB (the illumos kenrnel debugger) failed to clarify the situation at once, the kernel seems to be stuck idling. I've bisected the problem to commit a3e06bbe8445f57eb949e6474c5a9b30f24d2057. Please let me know if you need me to collect any debug information or test any patches.

Alexey,

Does illumos use tsc deadline timer? and do you run it at Intel platform?

Quoted 7 lines

If yes, would you please help me to collect debug information by add some printk points at kvm tsc deadline timer logic? currently I totally have no clue to figure out what the issue root from :) and, would you please check illumos tsc deadline timer logic by confirm whether illumos 1). Enumerate tsc deadline timer capability by CPUID; 2). Enable tsc deadline timer mode by lapic MMIO; 3). Start tsc deadline timer by WRMSR;

Quoted 8 lines

I think, here's what's going on. We get into kvm_get_lapic_tscdeadline_msr/kvm_set_lapic_tscdeadline_msr, but qemu does not use the KVM_CREATE_IRQCHIP ioctl, so we are running without vcpu->arch.apic and the functions do nothing. And the Linux guest just handles broken tsc a lot  better, so it survives. Both guests seem to work fine with qemu-kvm, which uses the ioctl.

Quoted 1 line

Hi. Do you need any more info on thins?

Seems illumos works fine w/ qemu-kvm, that's enough. Maybe you need to wait qemu-kvm sync into qemu?

Quoted 9 lines

Quoted 2 lines

Qemu-kvm is fine for me, but this is a regression and should be fixed.

Sorry, I haven't your environment to debug illumos regression. I don't think it directly related to kvm tsc deadline timer, and I think it's no value to do it now, since for linux guest it works fine w/ both qemu & qemu-kvm,
for illumos guest you just need wait qemu-kvm sync into qemu.

Let me clarify the situation.

Before this commit, the tsc was advertised in cpuid, and it was handled, if I understand things correctly, by qemu.
After this commit, the tsc is advertised in cpuid, and is handled in the kernel, but only after qemu issues KVM_CREATE_IRQCHIP. If it does not issue the ioctl, the kernel just discards any wrmsrs done to the tsc. This does not look like an Illumos problem to me. Linux guests kind of work here, because they are prepared to work on utterly broken hardware. Good for you, but please don't break less-prepared guests.

You can just run a Linux guest in qemu, and see what happens in kvm_get_lapic_tscdeadline_msr and kvm_set_lapic_tscdeadline_msr.

I double checked kvm tsc deadline timer patch, make sure your issue not *directly* related to the patch. I guess it maybe a qemu bug (which not exist at Avi's qemu-kvm). Can you tell me where do you pull qemu? I used Avi's qemu-kvm tree, but can try your environment.