kvm-unit-test failed to complete

Commit Message

Bandan Das July 30, 2015, 4:41 a.m. UTC

Shih-Wei Li <[e-mail]> writes:

> On Wed, Jul 29, 2015 at 2:38 PM, Shih-Wei Li <[e-mail]> wrote:
>> On Wed, Jul 29, 2015 at 2:30 PM, Christoffer Dall <[e-mail]> wrote:
>>> Hi Shih-Wei,
>>>
>>> [Something weird happened when sending these e-mails, you sent two where
>>> one seems to be a slight modification of the other?]
>>
>> yes, the previous one just got rejected by the mailing list. sorry
>> about the spam.
>>
>>>
>>> On Wed, Jul 29, 2015 at 02:18:23PM -0400, Shih-Wei Li wrote:
>>>> Hi all,
>>>>
>>>> This is Shih-Wei, I'm Christoffer's colleague at Columbia University.
>>>> We have experienced some problems in running kvm-unit-tests in our
>>>> environment.
>>>> Here's what we did:
>>>> ./configure
>>>> make
>>>> ./run_test.sh
>>>>
>>>> run_test.sh halted in some specific test items and couldn't finish the
>>>> run. I managed to get it finish running after removing the following
>>>> items:
>>>> x86/apic.c:
>>>> -> removed:
>>>> test_sti_nmi();
>>>> test_multiple_nmi();
>>>
>>> I'm wondering if there's a dependency on the version of QEMU?  Have you
>>> tried with the most recent upstream QEMU?
>>
>> I haven't, but I will try. We are using qemu 2.2.50 now.
>
> Here's the update. I just used the upstream QEMU to run kvm-unit-test,
> but it still failed to finish the same test items I mentioned earlier.

(Just a cursory look) It seems this commit introduced it -

commit 402d4596789335f540a0697955f6dcf43e2bb796
Author: Paolo Bonzini <[e-mail]>
Date:   Thu Jul 23 09:16:45 2015 +0200

    x86: load 64-bit segments into the segment registers
    
    kvm-unit-tests was keeping DS/ES/FS/GS loaded with the segment descriptors
    provided by the multiboot boot loader (which are 32-bit), and instead loading
    SS with 0.  The vmx.flat test failed because KVM did not like doing writes
    into such an SS.
    
    Load again the segment registers after entering 64-bit mode, for both
    the BSP and the APs.
    
    Signed-off-by: Paolo Bonzini <[e-mail]>


Paolo, what is the purpose of initializing %gs and %fs ? It seems if I comment
out "mov %ax, %gs", the test works.

handle_irq() does seem to trigger but the handler doesn't seem
to get called if %gs is initialized. Weird.

> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to [e-mail]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Paolo Bonzini July 30, 2015, 1:32 p.m. UTC | #1

On 30/07/2015 06:41, Bandan Das wrote:
> 
> diff --git a/x86/cstart64.S b/x86/cstart64.S
> index 8d0d95d..8d5ee2d 100644
> --- a/x86/cstart64.S
> +++ b/x86/cstart64.S
> @@ -213,7 +213,11 @@ idt_descr:
>  
>  load_tss:
>         lidtq idt_descr
> -       mov $0, %eax
> +       mov $0x10, %eax
> +       mov %ax, %ds
> +       mov %ax, %es
> +       mov %ax, %fs
> +       mov %ax, %gs
>         mov %ax, %ss
>         mov $(APIC_DEFAULT_PHYS_BASE + APIC_ID), %eax
>         mov (%rax), %eax
> 
> Paolo, what is the purpose of initializing %gs and %fs ? It seems if I comment
> out "mov %ax, %gs", the test works.

Oh, you're right!  setup_percpu_area writes the GS base.  Also the 0x10
selector is the same between the 32-bit and 64-bit GDT, so it should be
okay to remove the whole sequence of moves (from mov $0, %eax to mov
%ax, %ss).

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/x86/cstart64.S b/x86/cstart64.S
index 8d0d95d..8d5ee2d 100644
--- a/x86/cstart64.S
+++ b/x86/cstart64.S
@@ -213,7 +213,11 @@  idt_descr:
 
 load_tss:
        lidtq idt_descr
-       mov $0, %eax
+       mov $0x10, %eax
+       mov %ax, %ds
+       mov %ax, %es
+       mov %ax, %fs
+       mov %ax, %gs
        mov %ax, %ss
        mov $(APIC_DEFAULT_PHYS_BASE + APIC_ID), %eax
        mov (%rax), %eax