Re: [PATCH 07/18] kvm: Add MCE signal support for !CONFIG_IOTHREAD

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 3037 bytes --]

On 2011-01-24 13:36, Jan Kiszka wrote:
> On 2011-01-24 12:17, Marcelo Tosatti wrote:
>> On Mon, Jan 10, 2011 at 09:32:00AM +0100, Jan Kiszka wrote:
>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>
>>> Currently, we only configure and process MCE-related SIGBUS events if
>>> CONFIG_IOTHREAD is enabled. Fix this by factoring out the required
>>> handler registration and system configuration. Make sure that events
>>> happening over a VCPU context in non-threaded mode get dispatched as
>>> VCPU MCEs.
>>>
>>> We also need to call qemu_kvm_eat_signals in non-threaded mode now, so
>>> move it (unmodified) and add the required Windows stub.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> CC: Huang Ying <ying.huang@intel.com>
>>> ---
>>>  cpus.c |  200 +++++++++++++++++++++++++++++++++++++++------------------------
>>>  1 files changed, 124 insertions(+), 76 deletions(-)
>>>
>>> diff --git a/cpus.c b/cpus.c
>>> index 6da0f8f..b6f1cfb 100644
>>> --- a/cpus.c
>>> +++ b/cpus.c
>>> @@ -34,9 +34,6 @@
>>>  
>>>  #include "cpus.h"
>>>  #include "compatfd.h"
>>> -#ifdef CONFIG_LINUX
>>> -#include <sys/prctl.h>
>>> -#endif
>>>  
>>>  #ifdef SIGRTMIN
>>>  #define SIG_IPI (SIGRTMIN+4)
>>> @@ -44,10 +41,24 @@
>>>  #define SIG_IPI SIGUSR1
>>>  #endif
>>>  
>>
>>> @@ -912,6 +954,8 @@ static int qemu_cpu_exec(CPUState *env)
>>>  
>>>  bool cpu_exec_all(void)
>>>  {
>>> +    int r;
>>> +
>>>      if (next_cpu == NULL)
>>>          next_cpu = first_cpu;
>>>      for (; next_cpu != NULL && !exit_request; next_cpu = next_cpu->next_cpu) {
>>> @@ -923,7 +967,11 @@ bool cpu_exec_all(void)
>>>          if (qemu_alarm_pending())
>>>              break;
>>>          if (cpu_can_run(env)) {
>>> -            if (qemu_cpu_exec(env) == EXCP_DEBUG) {
>>> +            r = qemu_cpu_exec(env);
>>> +            if (kvm_enabled()) {
>>> +                qemu_kvm_eat_signals(env);
>>> +            }
>>> +            if (r == EXCP_DEBUG) {
>>>                  break;
>>>              }
>>
>> SIGBUS should be processed outside of vcpu execution context, think of a
>> non MCE SIGBUS while vm is stopped. Could use signalfd for that.
> 
> signalfd - that's the missing bit. I was thinking of how to handle
> SIGBUS events raised outside the vcpu context. We need to handle them
> synchronously, and signalfd should allow this.

This was straightforward. But now I wonder what actually makes this
pattern work. Doesn't the kernel force-inject SIGBUS, i.e. ignores any
blocking? Or does this only apply to BUS_MCEERR_AR?

> 
>>
>> But the SIGBUS handler for !IOTHREAD case should not ignore Action
>> Required, since it might have been generated in vcpu context.
>>
> 
> Yes, the sigbus handler will require some rework when we actually start
> using it for !IOTHREAD.

And this no longer makes sense to me. The current version simply uses
the same sigbus handler for both modes, an that forwards the error code
properly. What did you mean?

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]