[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <BC00F5384FCFC9499AF06F92E8B78A9E289048786F@shsmsx502.ccr.corp.intel.com>
Date: Wed, 21 Dec 2011 03:21:39 +0800
From: "Liu, Jinsong" <jinsong.liu@...el.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>,
Alexey Zaytsev <alexey.zaytsev@...il.com>
CC: Kernel development list <linux-kernel@...r.kernel.org>,
Jan Kiszka <jan.kiszka@...mens.com>,
Marcelo Tosatti <mtosatti@...hat.com>,
Avi Kivity <avi@...hat.com>,
Garrett D'Amore <garrett@...enta.com>
Subject: RE: [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@...il.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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists