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]

Date:	Wed, 21 Dec 2011 04:26:48 +0800
From:	"Liu, Jinsong" <jinsong.liu@...el.com>
To:	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"

Alexey Zaytsev wrote:
> On Tue, Dec 20, 2011 at 22:19, Liu, Jinsong <jinsong.liu@...el.com>
> wrote: 
>> Alexey Zaytsev wrote:
>>> On Tue, Dec 20, 2011 at 21:21, Liu, Jinsong <jinsong.liu@...el.com>
>>> wrote:
>>>> 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. 
>>> 
>>> [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.
>> 
>>> 
>>> 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. 
> 
> 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.

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/