lkml.org 
[lkml]   [2011]   [Dec]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRE: [Regression, bisected] a3e06bbe8445f57eb949e6474c5a9b30f24d2057: KVM: emulate lapic tsc deadline timer for guest"
    Linus Torvalds wrote:
    > On Tue, Dec 20, 2011 at 1:51 AM, Alexey Zaytsev
    > <alexey.zaytsev@gmail.com> wrote:
    >>
    >> 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

    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.

    Thanks,
    Jinsong--
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2011-12-20 20:25    [W:2.681 / U:0.084 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site