linux-kernel - RE: [Regression, bisected] a3e06bbe8445f57eb949e6474c5a9b30f24d2057: KVM: emulate lapic tsc deadline timer for guest"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<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/