Hi Jan,
On Fri, Oct 11, 2013 at 12:12 AM, Jan Kiszka <jan.kiszka@xxxxxxxxxxx> wrote:
> On 2013-10-02 20:47, Jan Kiszka wrote:
>> On 2013-09-30 11:08, Jan Kiszka wrote:
>>> On 2013-09-26 17:04, Paolo Bonzini wrote:
>>>> Il 16/09/2013 10:11, Arthur Chunqi Li ha scritto:
>>>>> This patch contains the following two changes:
>>>>> 1. Fix the bug in nested preemption timer support. If vmexit L2->L0
>>>>> with some reasons not emulated by L1, preemption timer value should
>>>>> be save in such exits.
>>>>> 2. Add support of "Save VMX-preemption timer value" VM-Exit controls
>>>>> to nVMX.
>>>>>
>>>>> With this patch, nested VMX preemption timer features are fully
>>>>> supported.
>>>>>
>>>>> Signed-off-by: Arthur Chunqi Li <yzt356@xxxxxxxxx>
>>>>> ---
>>>>> ChangeLog to v4:
>>>>> Format changes and remove a flag in nested_vmx.
>>>>> arch/x86/include/uapi/asm/msr-index.h | 1 +
>>>>> arch/x86/kvm/vmx.c | 44 +++++++++++++++++++++++++++++++--
>>>>> 2 files changed, 43 insertions(+), 2 deletions(-)
>>>>
>>>> Hi all,
>>>>
>>>> the test fails for me if the preemption timer value is set to a value
>>>> that is above ~2000 (which means ~65000 TSC cycles on this machine).
>>>> The preemption timer seems to count faster than what is expected, for
>>>> example only up to 4 million cycles if you set it to one million.
>>>> So, I am leaving the patch out of kvm/queue for now, until I can
>>>> test it on more processors.
>>>
>>> I've done some measurements with the help of ftrace on the time it takes
>>> to let the preemption timer trigger (no adjustments via Arthur's patch
>>> were involved): On my Core i7-620M, the preemption timer seems to tick
>>> almost 10 times faster than spec and scale value (5) suggests. I've
>>> loaded a value of 100000, and it took about 130 盜 until I got a vmexit
>>> with reason PREEMPTION_TIMER (no other exists in between).
>>>
>>> qemu-system-x86-13765 [003] 298562.966079: bprint: prepare_vmcs02: preempt val 100000
>>> qemu-system-x86-13765 [003] 298562.966083: kvm_entry: vcpu 0
>>> qemu-system-x86-13765 [003] 298562.966212: kvm_exit: reason PREEMPTION_TIMER rip 0x401fea info 0 0
>>>
>>> That's a frequency of ~769 MHz. The TSC ticks at 2.66 GHz. But 769 MHz *
>>> 2^5 is 24.6 GHz. I've read the spec several times, but it seems pretty
>>> clear on this. It just doesn't match reality. Very strange.
>>
>> ...but documented: I found an related errata for my processor (AAT59)
>> and also for Xeon 5500 (AAK139). At least current Haswell generation is
>> no affected. I can test the patch on a Haswell board I have at work
>> later this week.
>
> To complete this story: Arthur's patch works fine on a non-broken CPU
> (here: i7-4770S).
>
> Arthur, find some fix-ups for your test case below. It avoids printing
> from within L2 as this could deadlock when the timer fires and L1 then
> tries to print something. Also, it disables the preemption timer on
> leave so that it cannot fire later on again. If you want to fold this
> into your patch, feel free. Otherwise I can post a separate patch on
> top.
I think this can be treated as a separate patch to our test suite. You
can post it on top.
I have tested it and it works fine.
Arthur
>
> Jan
>
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index 4372878..66a4201 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -141,6 +141,9 @@ void preemption_timer_init()
> preempt_val = 10000000;
> vmcs_write(PREEMPT_TIMER_VALUE, preempt_val);
> preempt_scale = rdmsr(MSR_IA32_VMX_MISC) & 0x1F;
> +
> + if (!(ctrl_exit_rev.clr & EXI_SAVE_PREEMPT))
> + printf("\tSave preemption value is not supported\n");
> }
>
> void preemption_timer_main()
> @@ -150,9 +153,7 @@ void preemption_timer_main()
> printf("\tPreemption timer is not supported\n");
> return;
> }
> - if (!(ctrl_exit_rev.clr & EXI_SAVE_PREEMPT))
> - printf("\tSave preemption value is not supported\n");
> - else {
> + if (ctrl_exit_rev.clr & EXI_SAVE_PREEMPT) {
> set_stage(0);
> vmcall();
> if (get_stage() == 1)
> @@ -161,8 +162,8 @@ void preemption_timer_main()
> while (1) {
> if (((rdtsc() - tsc_val) >> preempt_scale)
> > 10 * preempt_val) {
> - report("Preemption timer", 0);
> - break;
> + set_stage(2);
> + vmcall();
> }
> }
> }
> @@ -183,7 +184,7 @@ int preemption_timer_exit_handler()
> report("Preemption timer", 0);
> else
> report("Preemption timer", 1);
> - return VMX_TEST_VMEXIT;
> + break;
> case VMX_VMCALL:
> switch (get_stage()) {
> case 0:
> @@ -195,24 +196,29 @@ int preemption_timer_exit_handler()
> EXI_SAVE_PREEMPT) & ctrl_exit_rev.clr;
> vmcs_write(EXI_CONTROLS, ctrl_exit);
> }
> - break;
> + vmcs_write(GUEST_RIP, guest_rip + insn_len);
> + return VMX_TEST_RESUME;
> case 1:
> if (vmcs_read(PREEMPT_TIMER_VALUE) >= preempt_val)
> report("Save preemption value", 0);
> else
> report("Save preemption value", 1);
> + vmcs_write(GUEST_RIP, guest_rip + insn_len);
> + return VMX_TEST_RESUME;
> + case 2:
> + report("Preemption timer", 0);
> break;
> default:
> printf("Invalid stage.\n");
> print_vmexit_info();
> - return VMX_TEST_VMEXIT;
> + break;
> }
> - vmcs_write(GUEST_RIP, guest_rip + insn_len);
> - return VMX_TEST_RESUME;
> + break;
> default:
> printf("Unknown exit reason, %d\n", reason);
> print_vmexit_info();
> }
> + vmcs_write(PIN_CONTROLS, vmcs_read(PIN_CONTROLS) & ~PIN_PREEMPT);
> return VMX_TEST_VMEXIT;
> }
>
> --
> Siemens AG, Corporate Technology, CT RTC ITP SES-DE
> Corporate Competence Center Embedded Linux
--
Arthur Chunqi Li
Department of Computer Science
School of EECS
Peking University
Beijing, China
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
