Message ID | 688de146287dc589a2e0fcce4cdec85d4f2c1127.1296594961.git.jan.kiszka@web.de |
---|---|
State | New |
Headers | show |
On 02/01/2011 11:15 PM, Jan Kiszka wrote: > From: Jan Kiszka<jan.kiszka@siemens.com> > > Block SIG_IPI, unblock it during KVM_RUN, just like in io-thread mode. > It's unused so far, but this infrastructure will be required for > self-IPIs and to process SIGBUS plus, in KVM mode, SIGIO and SIGALRM. As > Windows doesn't support signal services, we need to provide a stub for > the init function. > This patch breaks qemu-kvm after merging. The symptoms are that Windows XP x64 does not respond when netcat connects to some server in it, via -net user,hostfwd. The vcpu thread loops indefinitely on KVM_EXIT_INTR, which is consistent with signals being messed up. I verified that 981085dd465c1 merged with ff48eb5fe79ad works, while 981085dd465c1 merged with ff48eb5fe79ad^ fails. > diff --git a/cpus.c b/cpus.c > index 42717ba..a33e470 100644 > --- a/cpus.c > +++ b/cpus.c > @@ -231,11 +231,9 @@ fail: > return err; > } > > -#ifdef CONFIG_IOTHREAD > static void dummy_signal(int sig) > { > } > -#endif > > #else /* _WIN32 */ > > @@ -267,6 +265,32 @@ static void qemu_event_increment(void) > #endif /* _WIN32 */ > > #ifndef CONFIG_IOTHREAD > +static void qemu_kvm_init_cpu_signals(CPUState *env) > +{ > +#ifndef _WIN32 > + int r; > + sigset_t set; > + struct sigaction sigact; > + > + memset(&sigact, 0, sizeof(sigact)); > + sigact.sa_handler = dummy_signal; > + sigaction(SIG_IPI,&sigact, NULL); > + > + sigemptyset(&set); > + sigaddset(&set, SIG_IPI); > + pthread_sigmask(SIG_BLOCK,&set, NULL); > + > + pthread_sigmask(SIG_BLOCK, NULL,&set); > + sigdelset(&set, SIG_IPI); > + sigdelset(&set, SIGBUS); > + r = kvm_set_signal_mask(env,&set); > + if (r) { > + fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(-r)); > + exit(1); > + } > +#endif > +} > + > int qemu_init_main_loop(void) > { > cpu_set_debug_excp_handler(cpu_debug_handler); > @@ -292,6 +316,7 @@ void qemu_init_vcpu(void *_env) > fprintf(stderr, "kvm_init_vcpu failed: %s\n", strerror(-r)); > exit(1); > } > + qemu_kvm_init_cpu_signals(env); > } > } >
On 2011-02-28 16:55, Avi Kivity wrote: > On 02/01/2011 11:15 PM, Jan Kiszka wrote: >> From: Jan Kiszka<jan.kiszka@siemens.com> >> >> Block SIG_IPI, unblock it during KVM_RUN, just like in io-thread mode. >> It's unused so far, but this infrastructure will be required for >> self-IPIs and to process SIGBUS plus, in KVM mode, SIGIO and SIGALRM. As >> Windows doesn't support signal services, we need to provide a stub for >> the init function. >> > > This patch breaks qemu-kvm after merging. The symptoms are that Windows > XP x64 does not respond when netcat connects to some server in it, via > -net user,hostfwd. The vcpu thread loops indefinitely on KVM_EXIT_INTR, > which is consistent with signals being messed up. Does the same test case work with qemu, iothread on and off? Just to ensure we are not hunting an issue with the patch itself but of the merge. Will have a look as well. Jan > > I verified that 981085dd465c1 merged with ff48eb5fe79ad works, > while 981085dd465c1 merged with ff48eb5fe79ad^ fails. > > >> diff --git a/cpus.c b/cpus.c >> index 42717ba..a33e470 100644 >> --- a/cpus.c >> +++ b/cpus.c >> @@ -231,11 +231,9 @@ fail: >> return err; >> } >> >> -#ifdef CONFIG_IOTHREAD >> static void dummy_signal(int sig) >> { >> } >> -#endif >> >> #else /* _WIN32 */ >> >> @@ -267,6 +265,32 @@ static void qemu_event_increment(void) >> #endif /* _WIN32 */ >> >> #ifndef CONFIG_IOTHREAD >> +static void qemu_kvm_init_cpu_signals(CPUState *env) >> +{ >> +#ifndef _WIN32 >> + int r; >> + sigset_t set; >> + struct sigaction sigact; >> + >> + memset(&sigact, 0, sizeof(sigact)); >> + sigact.sa_handler = dummy_signal; >> + sigaction(SIG_IPI,&sigact, NULL); >> + >> + sigemptyset(&set); >> + sigaddset(&set, SIG_IPI); >> + pthread_sigmask(SIG_BLOCK,&set, NULL); >> + >> + pthread_sigmask(SIG_BLOCK, NULL,&set); >> + sigdelset(&set, SIG_IPI); >> + sigdelset(&set, SIGBUS); >> + r = kvm_set_signal_mask(env,&set); >> + if (r) { >> + fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(-r)); >> + exit(1); >> + } >> +#endif >> +} >> + >> int qemu_init_main_loop(void) >> { >> cpu_set_debug_excp_handler(cpu_debug_handler); >> @@ -292,6 +316,7 @@ void qemu_init_vcpu(void *_env) >> fprintf(stderr, "kvm_init_vcpu failed: %s\n", >> strerror(-r)); >> exit(1); >> } >> + qemu_kvm_init_cpu_signals(env); >> } >> } >> > >
On 2011-02-28 17:02, Jan Kiszka wrote: > On 2011-02-28 16:55, Avi Kivity wrote: >> On 02/01/2011 11:15 PM, Jan Kiszka wrote: >>> From: Jan Kiszka<jan.kiszka@siemens.com> >>> >>> Block SIG_IPI, unblock it during KVM_RUN, just like in io-thread mode. >>> It's unused so far, but this infrastructure will be required for >>> self-IPIs and to process SIGBUS plus, in KVM mode, SIGIO and SIGALRM. As >>> Windows doesn't support signal services, we need to provide a stub for >>> the init function. >>> >> >> This patch breaks qemu-kvm after merging. The symptoms are that Windows >> XP x64 does not respond when netcat connects to some server in it, via >> -net user,hostfwd. The vcpu thread loops indefinitely on KVM_EXIT_INTR, >> which is consistent with signals being messed up. > > Does the same test case work with qemu, iothread on and off? Just to Err, "iothread on" makes no sense here, of course. > ensure we are not hunting an issue with the patch itself but of the merge. > > Will have a look as well. > Jan
On 2011-02-28 16:55, Avi Kivity wrote: > On 02/01/2011 11:15 PM, Jan Kiszka wrote: >> From: Jan Kiszka<jan.kiszka@siemens.com> >> >> Block SIG_IPI, unblock it during KVM_RUN, just like in io-thread mode. >> It's unused so far, but this infrastructure will be required for >> self-IPIs and to process SIGBUS plus, in KVM mode, SIGIO and SIGALRM. As >> Windows doesn't support signal services, we need to provide a stub for >> the init function. >> > > This patch breaks qemu-kvm after merging. The symptoms are that Windows > XP x64 does not respond when netcat connects to some server in it, via > -net user,hostfwd. The vcpu thread loops indefinitely on KVM_EXIT_INTR, > which is consistent with signals being messed up. > > I verified that 981085dd465c1 merged with ff48eb5fe79ad works, > while 981085dd465c1 merged with ff48eb5fe79ad^ fails. > > >> diff --git a/cpus.c b/cpus.c >> index 42717ba..a33e470 100644 >> --- a/cpus.c >> +++ b/cpus.c >> @@ -231,11 +231,9 @@ fail: >> return err; >> } >> >> -#ifdef CONFIG_IOTHREAD >> static void dummy_signal(int sig) >> { >> } >> -#endif >> >> #else /* _WIN32 */ >> >> @@ -267,6 +265,32 @@ static void qemu_event_increment(void) >> #endif /* _WIN32 */ >> >> #ifndef CONFIG_IOTHREAD >> +static void qemu_kvm_init_cpu_signals(CPUState *env) >> +{ >> +#ifndef _WIN32 >> + int r; >> + sigset_t set; >> + struct sigaction sigact; >> + >> + memset(&sigact, 0, sizeof(sigact)); >> + sigact.sa_handler = dummy_signal; >> + sigaction(SIG_IPI,&sigact, NULL); >> + >> + sigemptyset(&set); >> + sigaddset(&set, SIG_IPI); >> + pthread_sigmask(SIG_BLOCK,&set, NULL); >> + >> + pthread_sigmask(SIG_BLOCK, NULL,&set); >> + sigdelset(&set, SIG_IPI); >> + sigdelset(&set, SIGBUS); >> + r = kvm_set_signal_mask(env,&set); >> + if (r) { >> + fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(-r)); >> + exit(1); >> + } >> +#endif >> +} >> + >> int qemu_init_main_loop(void) >> { >> cpu_set_debug_excp_handler(cpu_debug_handler); >> @@ -292,6 +316,7 @@ void qemu_init_vcpu(void *_env) >> fprintf(stderr, "kvm_init_vcpu failed: %s\n", >> strerror(-r)); >> exit(1); >> } >> + qemu_kvm_init_cpu_signals(env); Just comment that out as long as qemu-kvm is (mis-)using !IOTHREAD mode. I thought it would run before setup_kernel_sigmask, but it's the other way around, and then the wrong non-iothread signal setup is applied. Jan
On 02/28/2011 06:16 PM, Jan Kiszka wrote: > On 2011-02-28 16:55, Avi Kivity wrote: > > On 02/01/2011 11:15 PM, Jan Kiszka wrote: > >> From: Jan Kiszka<jan.kiszka@siemens.com> > >> > >> Block SIG_IPI, unblock it during KVM_RUN, just like in io-thread mode. > >> It's unused so far, but this infrastructure will be required for > >> self-IPIs and to process SIGBUS plus, in KVM mode, SIGIO and SIGALRM. As > >> Windows doesn't support signal services, we need to provide a stub for > >> the init function. > >> > > > > This patch breaks qemu-kvm after merging. The symptoms are that Windows > > XP x64 does not respond when netcat connects to some server in it, via > > -net user,hostfwd. The vcpu thread loops indefinitely on KVM_EXIT_INTR, > > which is consistent with signals being messed up. > > > > I verified that 981085dd465c1 merged with ff48eb5fe79ad works, > > while 981085dd465c1 merged with ff48eb5fe79ad^ fails. > > > > > >> diff --git a/cpus.c b/cpus.c > >> index 42717ba..a33e470 100644 > >> --- a/cpus.c > >> +++ b/cpus.c > >> @@ -231,11 +231,9 @@ fail: > >> return err; > >> } > >> > >> -#ifdef CONFIG_IOTHREAD > >> static void dummy_signal(int sig) > >> { > >> } > >> -#endif > >> > >> #else /* _WIN32 */ > >> > >> @@ -267,6 +265,32 @@ static void qemu_event_increment(void) > >> #endif /* _WIN32 */ > >> > >> #ifndef CONFIG_IOTHREAD > >> +static void qemu_kvm_init_cpu_signals(CPUState *env) > >> +{ > >> +#ifndef _WIN32 > >> + int r; > >> + sigset_t set; > >> + struct sigaction sigact; > >> + > >> + memset(&sigact, 0, sizeof(sigact)); > >> + sigact.sa_handler = dummy_signal; > >> + sigaction(SIG_IPI,&sigact, NULL); > >> + > >> + sigemptyset(&set); > >> + sigaddset(&set, SIG_IPI); > >> + pthread_sigmask(SIG_BLOCK,&set, NULL); > >> + > >> + pthread_sigmask(SIG_BLOCK, NULL,&set); > >> + sigdelset(&set, SIG_IPI); > >> + sigdelset(&set, SIGBUS); > >> + r = kvm_set_signal_mask(env,&set); > >> + if (r) { > >> + fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(-r)); > >> + exit(1); > >> + } > >> +#endif > >> +} > >> + > >> int qemu_init_main_loop(void) > >> { > >> cpu_set_debug_excp_handler(cpu_debug_handler); > >> @@ -292,6 +316,7 @@ void qemu_init_vcpu(void *_env) > >> fprintf(stderr, "kvm_init_vcpu failed: %s\n", > >> strerror(-r)); > >> exit(1); > >> } > >> + qemu_kvm_init_cpu_signals(env); > > Just comment that out as long as qemu-kvm is (mis-)using !IOTHREAD mode. > I thought it would run before setup_kernel_sigmask, but it's the other > way around, and then the wrong non-iothread signal setup is applied. That's what I tried, and it didn't work?! Maybe I forgot to compile or something.
On 02/28/2011 06:45 PM, Avi Kivity wrote: > > That's what I tried, and it didn't work?! Maybe I forgot to compile > or something. > I misspelled #ifdef.
On 2011-02-28 17:45, Avi Kivity wrote: > On 02/28/2011 06:16 PM, Jan Kiszka wrote: >> On 2011-02-28 16:55, Avi Kivity wrote: >>> On 02/01/2011 11:15 PM, Jan Kiszka wrote: >>>> From: Jan Kiszka<jan.kiszka@siemens.com> >>>> >>>> Block SIG_IPI, unblock it during KVM_RUN, just like in io-thread mode. >>>> It's unused so far, but this infrastructure will be required for >>>> self-IPIs and to process SIGBUS plus, in KVM mode, SIGIO and SIGALRM. As >>>> Windows doesn't support signal services, we need to provide a stub for >>>> the init function. >>>> >>> >>> This patch breaks qemu-kvm after merging. The symptoms are that Windows >>> XP x64 does not respond when netcat connects to some server in it, via >>> -net user,hostfwd. The vcpu thread loops indefinitely on KVM_EXIT_INTR, >>> which is consistent with signals being messed up. >>> >>> I verified that 981085dd465c1 merged with ff48eb5fe79ad works, >>> while 981085dd465c1 merged with ff48eb5fe79ad^ fails. >>> >>> >>>> diff --git a/cpus.c b/cpus.c >>>> index 42717ba..a33e470 100644 >>>> --- a/cpus.c >>>> +++ b/cpus.c >>>> @@ -231,11 +231,9 @@ fail: >>>> return err; >>>> } >>>> >>>> -#ifdef CONFIG_IOTHREAD >>>> static void dummy_signal(int sig) >>>> { >>>> } >>>> -#endif >>>> >>>> #else /* _WIN32 */ >>>> >>>> @@ -267,6 +265,32 @@ static void qemu_event_increment(void) >>>> #endif /* _WIN32 */ >>>> >>>> #ifndef CONFIG_IOTHREAD >>>> +static void qemu_kvm_init_cpu_signals(CPUState *env) >>>> +{ >>>> +#ifndef _WIN32 >>>> + int r; >>>> + sigset_t set; >>>> + struct sigaction sigact; >>>> + >>>> + memset(&sigact, 0, sizeof(sigact)); >>>> + sigact.sa_handler = dummy_signal; >>>> + sigaction(SIG_IPI,&sigact, NULL); >>>> + >>>> + sigemptyset(&set); >>>> + sigaddset(&set, SIG_IPI); >>>> + pthread_sigmask(SIG_BLOCK,&set, NULL); >>>> + >>>> + pthread_sigmask(SIG_BLOCK, NULL,&set); >>>> + sigdelset(&set, SIG_IPI); >>>> + sigdelset(&set, SIGBUS); >>>> + r = kvm_set_signal_mask(env,&set); >>>> + if (r) { >>>> + fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(-r)); >>>> + exit(1); >>>> + } >>>> +#endif >>>> +} >>>> + >>>> int qemu_init_main_loop(void) >>>> { >>>> cpu_set_debug_excp_handler(cpu_debug_handler); >>>> @@ -292,6 +316,7 @@ void qemu_init_vcpu(void *_env) >>>> fprintf(stderr, "kvm_init_vcpu failed: %s\n", >>>> strerror(-r)); >>>> exit(1); >>>> } >>>> + qemu_kvm_init_cpu_signals(env); >> >> Just comment that out as long as qemu-kvm is (mis-)using !IOTHREAD mode. >> I thought it would run before setup_kernel_sigmask, but it's the other >> way around, and then the wrong non-iothread signal setup is applied. > > That's what I tried, and it didn't work?! Maybe I forgot to compile or > something. Well, it maybe failed to build as qemu_kvm_init_cpu_signals became unused and the compiler should have bailed out? Probably it's better to disable it directly in the function. Jan
On 02/28/2011 06:49 PM, Jan Kiszka wrote: > > > > That's what I tried, and it didn't work?! Maybe I forgot to compile or > > something. > > Well, it maybe failed to build as qemu_kvm_init_cpu_signals became > unused and the compiler should have bailed out? Probably it's better to > disable it directly in the function. > That's what I did, with #ifdefs, but brokenly (#ifndef).
On 02/28/2011 06:54 PM, Avi Kivity wrote: > On 02/28/2011 06:49 PM, Jan Kiszka wrote: >> > >> > That's what I tried, and it didn't work?! Maybe I forgot to >> compile or >> > something. >> >> Well, it maybe failed to build as qemu_kvm_init_cpu_signals became >> unused and the compiler should have bailed out? Probably it's better to >> disable it directly in the function. >> > > That's what I did, with #ifdefs, but brokenly (#ifndef). > > Well it fails even with the correct #ifdef. Maybe some later patch adds to the breakage. This is really strange - the same test (migrate.tcp) works for Fedora and Windows XP x86. Installation and setup of Windows XP x64 work fine. It is only migration.tcp (when using netcat to connect to the guest) that fails.
On 2011-03-01 09:39, Avi Kivity wrote: > On 02/28/2011 06:54 PM, Avi Kivity wrote: >> On 02/28/2011 06:49 PM, Jan Kiszka wrote: >>>> >>>> That's what I tried, and it didn't work?! Maybe I forgot to >>> compile or >>>> something. >>> >>> Well, it maybe failed to build as qemu_kvm_init_cpu_signals became >>> unused and the compiler should have bailed out? Probably it's better to >>> disable it directly in the function. >>> >> >> That's what I did, with #ifdefs, but brokenly (#ifndef). >> >> > > Well it fails even with the correct #ifdef. Maybe some later patch adds > to the breakage. But when ifdef'ed out, this patch should be a nop for qemu-kvm. Indeed strange. > > This is really strange - the same test (migrate.tcp) works for Fedora > and Windows XP x86. Installation and setup of Windows XP x64 work > fine. It is only migration.tcp (when using netcat to connect to the > guest) that fails. > Guess this has to be classically debugged. :-/ Let me know if I can help (though not today). Jan
On 03/01/2011 10:58 AM, Jan Kiszka wrote: > On 2011-03-01 09:39, Avi Kivity wrote: > > On 02/28/2011 06:54 PM, Avi Kivity wrote: > >> On 02/28/2011 06:49 PM, Jan Kiszka wrote: > >>>> > >>>> That's what I tried, and it didn't work?! Maybe I forgot to > >>> compile or > >>>> something. > >>> > >>> Well, it maybe failed to build as qemu_kvm_init_cpu_signals became > >>> unused and the compiler should have bailed out? Probably it's better to > >>> disable it directly in the function. > >>> > >> > >> That's what I did, with #ifdefs, but brokenly (#ifndef). > >> > >> > > > > Well it fails even with the correct #ifdef. Maybe some later patch adds > > to the breakage. > > But when ifdef'ed out, this patch should be a nop for qemu-kvm. Indeed > strange. Well, there are two functions in cpus.c named qemu_kvm_init_cpu_signals() (an intriguing coincidence). I #ifdefed the wrong one. With the right #ifdef it works correctly. > > > > This is really strange - the same test (migrate.tcp) works for Fedora > > and Windows XP x86. Installation and setup of Windows XP x64 work > > fine. It is only migration.tcp (when using netcat to connect to the > > guest) that fails. > > > > Guess this has to be classically debugged. :-/ Let me know if I can help > (though not today). Still has to be debugged, but at least the tree is alive now.
diff --git a/cpus.c b/cpus.c index 42717ba..a33e470 100644 --- a/cpus.c +++ b/cpus.c @@ -231,11 +231,9 @@ fail: return err; } -#ifdef CONFIG_IOTHREAD static void dummy_signal(int sig) { } -#endif #else /* _WIN32 */ @@ -267,6 +265,32 @@ static void qemu_event_increment(void) #endif /* _WIN32 */ #ifndef CONFIG_IOTHREAD +static void qemu_kvm_init_cpu_signals(CPUState *env) +{ +#ifndef _WIN32 + int r; + sigset_t set; + struct sigaction sigact; + + memset(&sigact, 0, sizeof(sigact)); + sigact.sa_handler = dummy_signal; + sigaction(SIG_IPI, &sigact, NULL); + + sigemptyset(&set); + sigaddset(&set, SIG_IPI); + pthread_sigmask(SIG_BLOCK, &set, NULL); + + pthread_sigmask(SIG_BLOCK, NULL, &set); + sigdelset(&set, SIG_IPI); + sigdelset(&set, SIGBUS); + r = kvm_set_signal_mask(env, &set); + if (r) { + fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(-r)); + exit(1); + } +#endif +} + int qemu_init_main_loop(void) { cpu_set_debug_excp_handler(cpu_debug_handler); @@ -292,6 +316,7 @@ void qemu_init_vcpu(void *_env) fprintf(stderr, "kvm_init_vcpu failed: %s\n", strerror(-r)); exit(1); } + qemu_kvm_init_cpu_signals(env); } }