[Qemu-devel] [PATCH 00/15] [uq/master] Patch queue, part V (the rest)

[Qemu-devel] [PATCH 00/15] [uq/master] Patch queue, part V (the rest)

[Jan Kiszka]

This series catches "all the rest" to prepare QEMU's KVM support for
merging with qemu-kvm. IOW, once these bits here are applied, qemu-kvm
can switch its infrastructure to upstream and is effectively only adding
own bits for in-kernel irqchip and device assignment support.

Topics of this series are:
 - support for optimized interrupt handling by hooking cpu_interrupt
 - another preparational step for in-kernel irqchip support
 - x86: Do not leave halt if interrupts are disabled
 - mark VCPU state dirty on creation (fixed deadlock on early hw_error)
 - complete KVM support for PAT MSR, some related improvements for TCG
 - further consolidation of inner kvm_cpu_exec loop
 - expose VCPU host thread ID via "info cpus" and "query-cpus"

Please review.

CC: Alexander Graf <agraf@suse.de>
CC: Riku Voipio <riku.voipio@iki.fi>

Jan Kiszka (15):
  Break up user and system cpu_interrupt implementations
  Redirect cpu_interrupt to callback handler
  kvm: Install optimized interrupt handlers
  kvm: Add in-kernel irqchip awareness to cpu_thread_is_idle
  kvm: x86: Do not leave halt if interrupts are disabled
  kvm: Mark VCPU state dirty on creation
  x86: Properly reset PAT MSR
  x86: Save/restore PAT MSR
  kvm: x86: Synchronize PAT MSR with the kernel
  kvm: Consider EXIT_DEBUG unknown without CAP_SET_GUEST_DEBUG
  kvm: Rework inner loop of kvm_cpu_exec
  kvm: Align kvm_arch_handle_exit to kvm_cpu_exec changes
  kvm: x86: Reorder functions in kvm.c
  kvm: x86: Push kvm_arch_debug to kvm_arch_handle_exit
  Expose thread_id in info cpus

 cpu-all.h             |   14 ++++-
 cpu-defs.h            |    1 +
 cpus.c                |    5 +-
 exec.c                |   21 +++++--
 kvm-all.c             |   46 ++++++++-------
 kvm.h                 |    2 -
 monitor.c             |    4 +
 os-posix.c            |   10 +++
 os-win32.c            |    5 ++
 osdep.h               |    1 +
 qmp-commands.hx       |    3 +
 target-i386/cpu.h     |    4 +-
 target-i386/cpuid.c   |    1 -
 target-i386/helper.c  |    5 ++
 target-i386/kvm.c     |  146 +++++++++++++++++++++++++++----------------------
 target-i386/machine.c |    2 +
 target-ppc/kvm.c      |    8 +-
 target-s390x/kvm.c    |    5 ++
 18 files changed, 180 insertions(+), 103 deletions(-)

[Qemu-devel] [PATCH 01/15] Break up user and system cpu_interrupt implementations

[Jan Kiszka]

Both have only two lines in common, and we will convert the system
service into a callback which is of no use for user mode operation.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
CC: Riku Voipio <riku.voipio@iki.fi>
---
 exec.c |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/exec.c b/exec.c
index 9308a97..09235bf 100644
--- a/exec.c
+++ b/exec.c
@@ -1627,6 +1627,7 @@ static void cpu_unlink_tb(CPUState *env)
     spin_unlock(&interrupt_lock);
 }
 
+#ifndef CONFIG_USER_ONLY
 /* mask must never be zero, except for A20 change call */
 void cpu_interrupt(CPUState *env, int mask)
 {
@@ -1635,7 +1636,6 @@ void cpu_interrupt(CPUState *env, int mask)
     old_mask = env->interrupt_request;
     env->interrupt_request |= mask;
 
-#ifndef CONFIG_USER_ONLY
     /*
      * If called from iothread context, wake the target cpu in
      * case its halted.
@@ -1644,21 +1644,27 @@ void cpu_interrupt(CPUState *env, int mask)
         qemu_cpu_kick(env);
         return;
     }
-#endif
 
     if (use_icount) {
         env->icount_decr.u16.high = 0xffff;
-#ifndef CONFIG_USER_ONLY
         if (!can_do_io(env)
             && (mask & ~old_mask) != 0) {
             cpu_abort(env, "Raised interrupt while not in I/O function");
         }
-#endif
     } else {
         cpu_unlink_tb(env);
     }
 }
 
+#else /* CONFIG_USER_ONLY */
+
+void cpu_interrupt(CPUState *env, int mask)
+{
+    env->interrupt_request |= mask;
+    cpu_unlink_tb(env);
+}
+#endif /* CONFIG_USER_ONLY */
+
 void cpu_reset_interrupt(CPUState *env, int mask)
 {
     env->interrupt_request &= ~mask;
-- 
1.7.1

[Qemu-devel] [PATCH 02/15] Redirect cpu_interrupt to callback handler

[Jan Kiszka]

This allows to override the interrupt handling of QEMU in system mode.
KVM will make use of it to set optimized handlers.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpu-all.h |   14 +++++++++++++-
 exec.c    |    4 +++-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/cpu-all.h b/cpu-all.h
index 4f4631d..5835cfa 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -790,7 +790,19 @@ extern CPUState *cpu_single_env;
 #define CPU_INTERRUPT_SIPI   0x800 /* SIPI pending. */
 #define CPU_INTERRUPT_MCE    0x1000 /* (x86 only) MCE pending. */
 
-void cpu_interrupt(CPUState *s, int mask);
+#ifndef CONFIG_USER_ONLY
+typedef void (*CPUInterruptHandler)(CPUState *, int);
+
+extern CPUInterruptHandler cpu_interrupt_handler;
+
+static inline void cpu_interrupt(CPUState *s, int mask)
+{
+    cpu_interrupt_handler(s, mask);
+}
+#else /* USER_ONLY */
+void cpu_interrupt(CPUState *env, int mask);
+#endif /* USER_ONLY */
+
 void cpu_reset_interrupt(CPUState *env, int mask);
 
 void cpu_exit(CPUState *s);
diff --git a/exec.c b/exec.c
index 09235bf..a733acd 100644
--- a/exec.c
+++ b/exec.c
@@ -1629,7 +1629,7 @@ static void cpu_unlink_tb(CPUState *env)
 
 #ifndef CONFIG_USER_ONLY
 /* mask must never be zero, except for A20 change call */
-void cpu_interrupt(CPUState *env, int mask)
+static void tcg_handle_interrupt(CPUState *env, int mask)
 {
     int old_mask;
 
@@ -1656,6 +1656,8 @@ void cpu_interrupt(CPUState *env, int mask)
     }
 }
 
+CPUInterruptHandler cpu_interrupt_handler = tcg_handle_interrupt;
+
 #else /* CONFIG_USER_ONLY */
 
 void cpu_interrupt(CPUState *env, int mask)
-- 
1.7.1

[Qemu-devel] [PATCH 03/15] kvm: Install optimized interrupt handlers

[Jan Kiszka]

KVM only requires to set the raised IRQ in CPUState and, if the user
space irqchip is used, to kick the receiving vcpu if it is remote.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 kvm-all.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 226843c..c460d45 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -650,6 +650,20 @@ static CPUPhysMemoryClient kvm_cpu_phys_memory_client = {
     .log_stop = kvm_log_stop,
 };
 
+static void kvm_handle_interrupt(CPUState *env, int mask)
+{
+    env->interrupt_request |= mask;
+
+    if (!qemu_cpu_self(env)) {
+        qemu_cpu_kick(env);
+    }
+}
+
+static void kvm_handle_interrupt_kernel_irqchip(CPUState *env, int mask)
+{
+    env->interrupt_request |= mask;
+}
+
 int kvm_init(void)
 {
     static const char upgrade_note[] =
@@ -758,6 +772,9 @@ int kvm_init(void)
 
     s->many_ioeventfds = kvm_check_many_ioeventfds();
 
+    cpu_interrupt_handler = kvm_irqchip_in_kernel() ?
+        kvm_handle_interrupt_kernel_irqchip : kvm_handle_interrupt;
+
     return 0;
 
 err:
-- 
1.7.1

[Qemu-devel] Re: [PATCH 03/15] kvm: Install optimized interrupt handlers

[Marcelo Tosatti]

On Fri, Mar 04, 2011 at 11:20:00AM +0100, Jan Kiszka wrote:
> KVM only requires to set the raised IRQ in CPUState and, if the user
> space irqchip is used, to kick the receiving vcpu if it is remote.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  kvm-all.c |   17 +++++++++++++++++
>  1 files changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index 226843c..c460d45 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -650,6 +650,20 @@ static CPUPhysMemoryClient kvm_cpu_phys_memory_client = {
>      .log_stop = kvm_log_stop,
>  };
>  
> +static void kvm_handle_interrupt(CPUState *env, int mask)
> +{
> +    env->interrupt_request |= mask;
> +

If the env->interrupt_request request is processed in userspace, such as
MCE, the kick is still necessary for irqchip case. CPU_INTERRUPT_DEBUG
is another example, no?

> +    if (!qemu_cpu_self(env)) {
> +        qemu_cpu_kick(env);
> +    }
> +}
> +
> +static void kvm_handle_interrupt_kernel_irqchip(CPUState *env, int mask)
> +{
> +    env->interrupt_request |= mask;
> +}
> +

[Qemu-devel] Re: [PATCH 03/15] kvm: Install optimized interrupt handlers

[Jan Kiszka]

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

On 2011-03-05 16:37, Marcelo Tosatti wrote:
> On Fri, Mar 04, 2011 at 11:20:00AM +0100, Jan Kiszka wrote:
>> KVM only requires to set the raised IRQ in CPUState and, if the user
>> space irqchip is used, to kick the receiving vcpu if it is remote.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  kvm-all.c |   17 +++++++++++++++++
>>  1 files changed, 17 insertions(+), 0 deletions(-)
>>
>> diff --git a/kvm-all.c b/kvm-all.c
>> index 226843c..c460d45 100644
>> --- a/kvm-all.c
>> +++ b/kvm-all.c
>> @@ -650,6 +650,20 @@ static CPUPhysMemoryClient kvm_cpu_phys_memory_client = {
>>      .log_stop = kvm_log_stop,
>>  };
>>  
>> +static void kvm_handle_interrupt(CPUState *env, int mask)
>> +{
>> +    env->interrupt_request |= mask;
>> +
> 
> If the env->interrupt_request request is processed in userspace, such as
> MCE, the kick is still necessary for irqchip case. CPU_INTERRUPT_DEBUG
> is another example, no?

[this probably targeted kvm_handle_interrupt_kernel_irqchip]

In principle, you are right. But MCE must be injected synchronously over
the target VCPU, see do_inject_x86_mce, and CPU_INTERRUPT_DEBUG is also
synchronous and not even used in KVM mode.

> 
>> +    if (!qemu_cpu_self(env)) {
>> +        qemu_cpu_kick(env);
>> +    }
>> +}
>> +
>> +static void kvm_handle_interrupt_kernel_irqchip(CPUState *env, int mask)
>> +{
>> +    env->interrupt_request |= mask;
>> +}
>> +

Jan


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

[Qemu-devel] Re: [PATCH 03/15] kvm: Install optimized interrupt handlers

[Marcelo Tosatti]

On Sat, Mar 05, 2011 at 07:11:53PM +0100, Jan Kiszka wrote:
> On 2011-03-05 16:37, Marcelo Tosatti wrote:
> > On Fri, Mar 04, 2011 at 11:20:00AM +0100, Jan Kiszka wrote:
> >> KVM only requires to set the raised IRQ in CPUState and, if the user
> >> space irqchip is used, to kick the receiving vcpu if it is remote.
> >>
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >> ---
> >>  kvm-all.c |   17 +++++++++++++++++
> >>  1 files changed, 17 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/kvm-all.c b/kvm-all.c
> >> index 226843c..c460d45 100644
> >> --- a/kvm-all.c
> >> +++ b/kvm-all.c
> >> @@ -650,6 +650,20 @@ static CPUPhysMemoryClient kvm_cpu_phys_memory_client = {
> >>      .log_stop = kvm_log_stop,
> >>  };
> >>  
> >> +static void kvm_handle_interrupt(CPUState *env, int mask)
> >> +{
> >> +    env->interrupt_request |= mask;
> >> +
> > 
> > If the env->interrupt_request request is processed in userspace, such as
> > MCE, the kick is still necessary for irqchip case. CPU_INTERRUPT_DEBUG
> > is another example, no?
> 
> [this probably targeted kvm_handle_interrupt_kernel_irqchip]
> 
> In principle, you are right. But MCE must be injected synchronously over
> the target VCPU, see do_inject_x86_mce, and CPU_INTERRUPT_DEBUG is also
> synchronous and not even used in KVM mode.

CPU_INTERRUPT_NMI from monitor?

Don't see what gain you expect from avoiding the signal in this case.

[Qemu-devel] Re: [PATCH 03/15] kvm: Install optimized interrupt handlers

[Jan Kiszka]

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

On 2011-03-06 03:13, Marcelo Tosatti wrote:
> On Sat, Mar 05, 2011 at 07:11:53PM +0100, Jan Kiszka wrote:
>> On 2011-03-05 16:37, Marcelo Tosatti wrote:
>>> On Fri, Mar 04, 2011 at 11:20:00AM +0100, Jan Kiszka wrote:
>>>> KVM only requires to set the raised IRQ in CPUState and, if the user
>>>> space irqchip is used, to kick the receiving vcpu if it is remote.
>>>>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>> ---
>>>>  kvm-all.c |   17 +++++++++++++++++
>>>>  1 files changed, 17 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/kvm-all.c b/kvm-all.c
>>>> index 226843c..c460d45 100644
>>>> --- a/kvm-all.c
>>>> +++ b/kvm-all.c
>>>> @@ -650,6 +650,20 @@ static CPUPhysMemoryClient kvm_cpu_phys_memory_client = {
>>>>      .log_stop = kvm_log_stop,
>>>>  };
>>>>  
>>>> +static void kvm_handle_interrupt(CPUState *env, int mask)
>>>> +{
>>>> +    env->interrupt_request |= mask;
>>>> +
>>>
>>> If the env->interrupt_request request is processed in userspace, such as
>>> MCE, the kick is still necessary for irqchip case. CPU_INTERRUPT_DEBUG
>>> is another example, no?
>>
>> [this probably targeted kvm_handle_interrupt_kernel_irqchip]
>>
>> In principle, you are right. But MCE must be injected synchronously over
>> the target VCPU, see do_inject_x86_mce, and CPU_INTERRUPT_DEBUG is also
>> synchronous and not even used in KVM mode.
> 
> CPU_INTERRUPT_NMI from monitor?
> 
> Don't see what gain you expect from avoiding the signal in this case.

Well, looking at this from a different angle again, I cannot identify my
original optimization anymore. I guess I was under the wrong impression
that cpu_interrupt is still a frequently used service even with
in-kernel irqchip. But that's by far not the case.

Will drop this.

Jan


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

[Qemu-devel] [PATCH 04/15] kvm: Add in-kernel irqchip awareness to cpu_thread_is_idle

[Jan Kiszka]

With in-kernel irqchip support enabled, the vcpu threads sleep in kernel
space while halted. Account for this difference in cpu_thread_is_idle.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpus.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/cpus.c b/cpus.c
index 0f33945..66f6b5a 100644
--- a/cpus.c
+++ b/cpus.c
@@ -147,7 +147,8 @@ static bool cpu_thread_is_idle(CPUState *env)
     if (env->stopped || !vm_running) {
         return true;
     }
-    if (!env->halted || qemu_cpu_has_work(env)) {
+    if (!env->halted || qemu_cpu_has_work(env) ||
+        (kvm_enabled() && kvm_irqchip_in_kernel())) {
         return false;
     }
     return true;
-- 
1.7.1

[Qemu-devel] [PATCH 05/15] kvm: x86: Do not leave halt if interrupts are disabled

[Jan Kiszka]

When an external interrupt is pending but IF is cleared, we must not
leave the halt state prematurely.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 target-i386/kvm.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 7b7105d..6efa491 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1590,7 +1590,9 @@ int kvm_arch_process_async_events(CPUState *env)
         return 0;
     }
 
-    if (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI)) {
+    if (((env->interrupt_request & CPU_INTERRUPT_HARD) &&
+         (env->eflags & IF_MASK)) ||
+        (env->interrupt_request & CPU_INTERRUPT_NMI)) {
         env->halted = 0;
     }
     if (env->interrupt_request & CPU_INTERRUPT_INIT) {
-- 
1.7.1

[Qemu-devel] [PATCH 06/15] kvm: Mark VCPU state dirty on creation

[Jan Kiszka]

This avoids that early cpu_synchronize_state calls try to retrieve an
uninitialized state from the kernel. That even causes a deadlock if
io-thread is enabled.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 kvm-all.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index c460d45..07a1a9c 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -211,6 +211,7 @@ int kvm_init_vcpu(CPUState *env)
 
     env->kvm_fd = ret;
     env->kvm_state = s;
+    env->kvm_vcpu_dirty = 1;
 
     mmap_size = kvm_ioctl(s, KVM_GET_VCPU_MMAP_SIZE, 0);
     if (mmap_size < 0) {
-- 
1.7.1

[Qemu-devel] [PATCH 07/15] x86: Properly reset PAT MSR

[Jan Kiszka]

Conforming to the Intel spec, set the power-on value of PAT also on
reset, but save it across INIT.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 target-i386/cpu.h    |    4 ++--
 target-i386/cpuid.c  |    1 -
 target-i386/helper.c |    5 +++++
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index d0eae75..c7047d5 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -685,8 +685,6 @@ typedef struct CPUX86State {
 
     uint64_t tsc;
 
-    uint64_t pat;
-
     uint64_t mcg_status;
 
     /* exception/interrupt handling */
@@ -707,6 +705,8 @@ typedef struct CPUX86State {
 
     CPU_COMMON
 
+    uint64_t pat;
+
     /* processor features (e.g. for CPUID insn) */
     uint32_t cpuid_level;
     uint32_t cpuid_vendor1;
diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 5382a28..814d13e 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -847,7 +847,6 @@ int cpu_x86_register (CPUX86State *env, const char *cpu_model)
     env->cpuid_version |= ((def->model & 0xf) << 4) | ((def->model >> 4) << 16);
     env->cpuid_version |= def->stepping;
     env->cpuid_features = def->features;
-    env->pat = 0x0007040600070406ULL;
     env->cpuid_ext_features = def->ext_features;
     env->cpuid_ext2_features = def->ext2_features;
     env->cpuid_ext3_features = def->ext3_features;
diff --git a/target-i386/helper.c b/target-i386/helper.c
index a08309f..d15fca5 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -99,6 +99,8 @@ void cpu_reset(CPUX86State *env)
 
     env->mxcsr = 0x1f80;
 
+    env->pat = 0x0007040600070406ULL;
+
     memset(env->dr, 0, sizeof(env->dr));
     env->dr[6] = DR6_FIXED_1;
     env->dr[7] = DR7_FIXED_1;
@@ -1280,8 +1282,11 @@ CPUX86State *cpu_x86_init(const char *cpu_model)
 void do_cpu_init(CPUState *env)
 {
     int sipi = env->interrupt_request & CPU_INTERRUPT_SIPI;
+    uint64_t pat = env->pat;
+
     cpu_reset(env);
     env->interrupt_request = sipi;
+    env->pat = pat;
     apic_init_reset(env->apic_state);
     env->halted = !cpu_is_bsp(env);
 }
-- 
1.7.1

[Qemu-devel] [PATCH 08/15] x86: Save/restore PAT MSR

[Jan Kiszka]

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 target-i386/machine.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/target-i386/machine.c b/target-i386/machine.c
index d78eceb..6384f54 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -491,6 +491,8 @@ static const VMStateDescription vmstate_cpu = {
         VMSTATE_UINT64_V(xcr0, CPUState, 12),
         VMSTATE_UINT64_V(xstate_bv, CPUState, 12),
         VMSTATE_YMMH_REGS_VARS(ymmh_regs, CPUState, CPU_NB_REGS, 12),
+
+        VMSTATE_UINT64_V(pat, CPUState, 13),
         VMSTATE_END_OF_LIST()
         /* The above list is not sorted /wrt version numbers, watch out! */
     },
-- 
1.7.1

[Qemu-devel] [PATCH 09/15] kvm: x86: Synchronize PAT MSR with the kernel

[Jan Kiszka]

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 target-i386/kvm.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 6efa491..bfc8d66 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -861,6 +861,7 @@ static int kvm_put_msrs(CPUState *env, int level)
     kvm_msr_entry_set(&msrs[n++], MSR_IA32_SYSENTER_CS, env->sysenter_cs);
     kvm_msr_entry_set(&msrs[n++], MSR_IA32_SYSENTER_ESP, env->sysenter_esp);
     kvm_msr_entry_set(&msrs[n++], MSR_IA32_SYSENTER_EIP, env->sysenter_eip);
+    kvm_msr_entry_set(&msrs[n++], MSR_PAT, env->pat);
     if (has_msr_star) {
         kvm_msr_entry_set(&msrs[n++], MSR_STAR, env->star);
     }
@@ -1113,6 +1114,7 @@ static int kvm_get_msrs(CPUState *env)
     msrs[n++].index = MSR_IA32_SYSENTER_CS;
     msrs[n++].index = MSR_IA32_SYSENTER_ESP;
     msrs[n++].index = MSR_IA32_SYSENTER_EIP;
+    msrs[n++].index = MSR_PAT;
     if (has_msr_star) {
         msrs[n++].index = MSR_STAR;
     }
@@ -1168,6 +1170,9 @@ static int kvm_get_msrs(CPUState *env)
         case MSR_IA32_SYSENTER_EIP:
             env->sysenter_eip = msrs[i].data;
             break;
+        case MSR_PAT:
+            env->pat = msrs[i].data;
+            break;
         case MSR_STAR:
             env->star = msrs[i].data;
             break;
-- 
1.7.1

[Qemu-devel] [PATCH 10/15] kvm: Consider EXIT_DEBUG unknown without CAP_SET_GUEST_DEBUG

[Jan Kiszka]

Without KVM_CAP_SET_GUEST_DEBUG, we neither motivate the kernel to
report KVM_EXIT_DEBUG nor do we expect such exits. So fall through to
the arch code which will simply report an unknown exit reason.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 kvm-all.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 07a1a9c..2952499 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -992,17 +992,17 @@ int kvm_cpu_exec(CPUState *env)
             ret = kvm_handle_internal_error(env, run);
             break;
 #endif
+#ifdef KVM_CAP_SET_GUEST_DEBUG
         case KVM_EXIT_DEBUG:
             DPRINTF("kvm_exit_debug\n");
-#ifdef KVM_CAP_SET_GUEST_DEBUG
             if (kvm_arch_debug(&run->debug.arch)) {
                 ret = EXCP_DEBUG;
                 goto out;
             }
             /* re-enter, this exception was guest-internal */
             ret = 1;
-#endif /* KVM_CAP_SET_GUEST_DEBUG */
             break;
+#endif /* KVM_CAP_SET_GUEST_DEBUG */
         default:
             DPRINTF("kvm_arch_handle_exit\n");
             ret = kvm_arch_handle_exit(env, run);
-- 
1.7.1

[Qemu-devel] [PATCH 11/15] kvm: Rework inner loop of kvm_cpu_exec

[Jan Kiszka]

Let kvm_cpu_exec return EXCP_* values consistently and generate those
codes already inside its inner loop. This means we will now re-enter the
kernel while ret == 0.

Update kvm_handle_internal_error accordingly, but keep
kvm_arch_handle_exit untouched, it will be converted in a separate step.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 kvm-all.c |   26 ++++++++++++++------------
 1 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 2952499..cc652cf 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -848,7 +848,7 @@ static int kvm_handle_internal_error(CPUState *env, struct kvm_run *run)
         fprintf(stderr, "emulation failure\n");
         if (!kvm_arch_stop_on_emulation_error(env)) {
             cpu_dump_state(env, stderr, fprintf, CPU_DUMP_CODE);
-            return 0;
+            return EXCP_INTERRUPT;
         }
     }
     /* FIXME: Should trigger a qmp message to let management know
@@ -947,7 +947,7 @@ int kvm_cpu_exec(CPUState *env)
 
         if (ret == -EINTR || ret == -EAGAIN) {
             DPRINTF("io window exit\n");
-            ret = 0;
+            ret = EXCP_INTERRUPT;
             break;
         }
 
@@ -956,7 +956,6 @@ int kvm_cpu_exec(CPUState *env)
             abort();
         }
 
-        ret = 0; /* exit loop */
         switch (run->exit_reason) {
         case KVM_EXIT_IO:
             DPRINTF("handle_io\n");
@@ -965,7 +964,7 @@ int kvm_cpu_exec(CPUState *env)
                           run->io.direction,
                           run->io.size,
                           run->io.count);
-            ret = 1;
+            ret = 0;
             break;
         case KVM_EXIT_MMIO:
             DPRINTF("handle_mmio\n");
@@ -973,14 +972,16 @@ int kvm_cpu_exec(CPUState *env)
                                    run->mmio.data,
                                    run->mmio.len,
                                    run->mmio.is_write);
-            ret = 1;
+            ret = 0;
             break;
         case KVM_EXIT_IRQ_WINDOW_OPEN:
             DPRINTF("irq_window_open\n");
+            ret = EXCP_INTERRUPT;
             break;
         case KVM_EXIT_SHUTDOWN:
             DPRINTF("shutdown\n");
             qemu_system_reset_request();
+            ret = EXCP_INTERRUPT;
             break;
         case KVM_EXIT_UNKNOWN:
             fprintf(stderr, "KVM: unknown exit, hardware reason %" PRIx64 "\n",
@@ -997,28 +998,29 @@ int kvm_cpu_exec(CPUState *env)
             DPRINTF("kvm_exit_debug\n");
             if (kvm_arch_debug(&run->debug.arch)) {
                 ret = EXCP_DEBUG;
-                goto out;
+                break;
             }
             /* re-enter, this exception was guest-internal */
-            ret = 1;
+            ret = 0;
             break;
 #endif /* KVM_CAP_SET_GUEST_DEBUG */
         default:
             DPRINTF("kvm_arch_handle_exit\n");
             ret = kvm_arch_handle_exit(env, run);
+            if (ret == 0) {
+                ret = EXCP_INTERRUPT;
+            } else if (ret > 0) {
+                ret = 0;
+            }
             break;
         }
-    } while (ret > 0);
+    } while (ret == 0);
 
     if (ret < 0) {
         cpu_dump_state(env, stderr, fprintf, CPU_DUMP_CODE);
         vm_stop(VMSTOP_PANIC);
     }
-    ret = EXCP_INTERRUPT;
 
-#ifdef KVM_CAP_SET_GUEST_DEBUG
-out:
-#endif
     env->exit_request = 0;
     cpu_single_env = NULL;
     return ret;
-- 
1.7.1

[Qemu-devel] Re: [PATCH 11/15] kvm: Rework inner loop of kvm_cpu_exec

[Marcelo Tosatti]

On Fri, Mar 04, 2011 at 11:20:08AM +0100, Jan Kiszka wrote:
> Let kvm_cpu_exec return EXCP_* values consistently and generate those
> codes already inside its inner loop. This means we will now re-enter the
> kernel while ret == 0.
> 
> Update kvm_handle_internal_error accordingly, but keep
> kvm_arch_handle_exit untouched, it will be converted in a separate step.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  kvm-all.c |   26 ++++++++++++++------------
>  1 files changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index 2952499..cc652cf 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -848,7 +848,7 @@ static int kvm_handle_internal_error(CPUState *env, struct kvm_run *run)
>          fprintf(stderr, "emulation failure\n");
>          if (!kvm_arch_stop_on_emulation_error(env)) {
>              cpu_dump_state(env, stderr, fprintf, CPU_DUMP_CODE);
> -            return 0;
> +            return EXCP_INTERRUPT;
>          }
>      }
>      /* FIXME: Should trigger a qmp message to let management know
> @@ -947,7 +947,7 @@ int kvm_cpu_exec(CPUState *env)
>  
>          if (ret == -EINTR || ret == -EAGAIN) {
>              DPRINTF("io window exit\n");
> -            ret = 0;
> +            ret = EXCP_INTERRUPT;
>              break;
>          }
>  
> @@ -956,7 +956,6 @@ int kvm_cpu_exec(CPUState *env)
>              abort();
>          }
>  
> -        ret = 0; /* exit loop */
>          switch (run->exit_reason) {

Better keep ret assignment here so default behaviour is to 
exit loop? EXCP_INTERRUPT.

[Qemu-devel] Re: [PATCH 11/15] kvm: Rework inner loop of kvm_cpu_exec

[Jan Kiszka]

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

On 2011-03-05 17:05, Marcelo Tosatti wrote:
> On Fri, Mar 04, 2011 at 11:20:08AM +0100, Jan Kiszka wrote:
>> Let kvm_cpu_exec return EXCP_* values consistently and generate those
>> codes already inside its inner loop. This means we will now re-enter the
>> kernel while ret == 0.
>>
>> Update kvm_handle_internal_error accordingly, but keep
>> kvm_arch_handle_exit untouched, it will be converted in a separate step.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  kvm-all.c |   26 ++++++++++++++------------
>>  1 files changed, 14 insertions(+), 12 deletions(-)
>>
>> diff --git a/kvm-all.c b/kvm-all.c
>> index 2952499..cc652cf 100644
>> --- a/kvm-all.c
>> +++ b/kvm-all.c
>> @@ -848,7 +848,7 @@ static int kvm_handle_internal_error(CPUState *env, struct kvm_run *run)
>>          fprintf(stderr, "emulation failure\n");
>>          if (!kvm_arch_stop_on_emulation_error(env)) {
>>              cpu_dump_state(env, stderr, fprintf, CPU_DUMP_CODE);
>> -            return 0;
>> +            return EXCP_INTERRUPT;
>>          }
>>      }
>>      /* FIXME: Should trigger a qmp message to let management know
>> @@ -947,7 +947,7 @@ int kvm_cpu_exec(CPUState *env)
>>  
>>          if (ret == -EINTR || ret == -EAGAIN) {
>>              DPRINTF("io window exit\n");
>> -            ret = 0;
>> +            ret = EXCP_INTERRUPT;
>>              break;
>>          }
>>  
>> @@ -956,7 +956,6 @@ int kvm_cpu_exec(CPUState *env)
>>              abort();
>>          }
>>  
>> -        ret = 0; /* exit loop */
>>          switch (run->exit_reason) {
> 
> Better keep ret assignment here so default behaviour is to 
> exit loop? EXCP_INTERRUPT.

There is no real default behavior: in two cases we stay in the loop, in
two others we leave, and the rest obtains ret from a return value.
Moreover, if a new case misses to set ret, the compiler will complain.

Jan


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

[Qemu-devel] Re: [PATCH 11/15] kvm: Rework inner loop of kvm_cpu_exec

[Marcelo Tosatti]

On Sat, Mar 05, 2011 at 07:12:50PM +0100, Jan Kiszka wrote:
> >> @@ -956,7 +956,6 @@ int kvm_cpu_exec(CPUState *env)
> >>              abort();
> >>          }
> >>  
> >> -        ret = 0; /* exit loop */
> >>          switch (run->exit_reason) {
> > 
> > Better keep ret assignment here so default behaviour is to 
> > exit loop? EXCP_INTERRUPT.
> 
> There is no real default behavior: in two cases we stay in the loop, in
> two others we leave, and the rest obtains ret from a return value.
> Moreover, if a new case misses to set ret, the compiler will complain.
> 
> Jan

It will not complain because "ret" is used to store return value
of KVM_RUN.

[Qemu-devel] Re: [PATCH 11/15] kvm: Rework inner loop of kvm_cpu_exec

[Jan Kiszka]

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

On 2011-03-11 22:34, Marcelo Tosatti wrote:
> On Sat, Mar 05, 2011 at 07:12:50PM +0100, Jan Kiszka wrote:
>>>> @@ -956,7 +956,6 @@ int kvm_cpu_exec(CPUState *env)
>>>>              abort();
>>>>          }
>>>>  
>>>> -        ret = 0; /* exit loop */
>>>>          switch (run->exit_reason) {
>>>
>>> Better keep ret assignment here so default behaviour is to 
>>> exit loop? EXCP_INTERRUPT.
>>
>> There is no real default behavior: in two cases we stay in the loop, in
>> two others we leave, and the rest obtains ret from a return value.
>> Moreover, if a new case misses to set ret, the compiler will complain.
>>
>> Jan
> 
> It will not complain because "ret" is used to store return value
> of KVM_RUN.
> 

Right, I'll disentangle this duplicate use of 'ret' (writing patches is
likely much better than watching more news this morning...).

Jan


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

[Qemu-devel] [PATCH 12/15] kvm: Align kvm_arch_handle_exit to kvm_cpu_exec changes

[Jan Kiszka]

Make the return code of kvm_arch_handle_exit directly usable for
kvm_cpu_exec. This is straightforward for x86 and ppc, just s390
would require more work. Avoid this for now by pushing the return code
translation logic into s390's kvm_arch_handle_exit.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
CC: Alexander Graf <agraf@suse.de>
---
 kvm-all.c          |    5 -----
 target-i386/kvm.c  |    8 ++++----
 target-ppc/kvm.c   |    8 ++++----
 target-s390x/kvm.c |    5 +++++
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index cc652cf..d643dc7 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1007,11 +1007,6 @@ int kvm_cpu_exec(CPUState *env)
         default:
             DPRINTF("kvm_arch_handle_exit\n");
             ret = kvm_arch_handle_exit(env, run);
-            if (ret == 0) {
-                ret = EXCP_INTERRUPT;
-            } else if (ret > 0) {
-                ret = 0;
-            }
             break;
         }
     } while (ret == 0);
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index bfc8d66..b43a85c 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1618,10 +1618,10 @@ static int kvm_handle_halt(CPUState *env)
           (env->eflags & IF_MASK)) &&
         !(env->interrupt_request & CPU_INTERRUPT_NMI)) {
         env->halted = 1;
-        return 0;
+        return EXCP_HLT;
     }
 
-    return 1;
+    return 0;
 }
 
 static bool host_supports_vmx(void)
@@ -1637,7 +1637,7 @@ static bool host_supports_vmx(void)
 int kvm_arch_handle_exit(CPUState *env, struct kvm_run *run)
 {
     uint64_t code;
-    int ret = 0;
+    int ret;
 
     switch (run->exit_reason) {
     case KVM_EXIT_HLT:
@@ -1645,7 +1645,7 @@ int kvm_arch_handle_exit(CPUState *env, struct kvm_run *run)
         ret = kvm_handle_halt(env);
         break;
     case KVM_EXIT_SET_TPR:
-        ret = 1;
+        ret = 0;
         break;
     case KVM_EXIT_FAIL_ENTRY:
         code = run->fail_entry.hardware_entry_failure_reason;
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 6c99a16..593eb98 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -271,7 +271,7 @@ static int kvmppc_handle_halt(CPUState *env)
         env->exception_index = EXCP_HLT;
     }
 
-    return 1;
+    return 0;
 }
 
 /* map dcr access to existing qemu dcr emulation */
@@ -280,7 +280,7 @@ static int kvmppc_handle_dcr_read(CPUState *env, uint32_t dcrn, uint32_t *data)
     if (ppc_dcr_read(env->dcr_env, dcrn, data) < 0)
         fprintf(stderr, "Read to unhandled DCR (0x%x)\n", dcrn);
 
-    return 1;
+    return 0;
 }
 
 static int kvmppc_handle_dcr_write(CPUState *env, uint32_t dcrn, uint32_t data)
@@ -288,12 +288,12 @@ static int kvmppc_handle_dcr_write(CPUState *env, uint32_t dcrn, uint32_t data)
     if (ppc_dcr_write(env->dcr_env, dcrn, data) < 0)
         fprintf(stderr, "Write to unhandled DCR (0x%x)\n", dcrn);
 
-    return 1;
+    return 0;
 }
 
 int kvm_arch_handle_exit(CPUState *env, struct kvm_run *run)
 {
-    int ret = 0;
+    int ret;
 
     switch (run->exit_reason) {
     case KVM_EXIT_DCR:
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 5673a95..4761d5d 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -496,6 +496,11 @@ int kvm_arch_handle_exit(CPUState *env, struct kvm_run *run)
             break;
     }
 
+    if (ret == 0) {
+        ret = EXCP_INTERRUPT;
+    } else if (ret > 0) {
+        ret = 0;
+    }
     return ret;
 }
 
-- 
1.7.1

[Qemu-devel] Re: [PATCH 12/15] kvm: Align kvm_arch_handle_exit to kvm_cpu_exec changes

[Alexander Graf]

On 04.03.2011, at 11:20, Jan Kiszka wrote:

> Make the return code of kvm_arch_handle_exit directly usable for
> kvm_cpu_exec. This is straightforward for x86 and ppc, just s390
> would require more work. Avoid this for now by pushing the return code
> translation logic into s390's kvm_arch_handle_exit.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> CC: Alexander Graf <agraf@suse.de>

Looks good, haven't tested it though. Do you have a git tree for all this?


Alex

[Qemu-devel] Re: [PATCH 12/15] kvm: Align kvm_arch_handle_exit to kvm_cpu_exec changes

[Jan Kiszka]

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

On 2011-03-11 07:50, Alexander Graf wrote:
> 
> On 04.03.2011, at 11:20, Jan Kiszka wrote:
> 
>> Make the return code of kvm_arch_handle_exit directly usable for
>> kvm_cpu_exec. This is straightforward for x86 and ppc, just s390
>> would require more work. Avoid this for now by pushing the return code
>> translation logic into s390's kvm_arch_handle_exit.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> CC: Alexander Graf <agraf@suse.de>
> 
> Looks good, haven't tested it though. Do you have a git tree for all this?
> 

See

git://git.kiszka.org/qemu-kvm.git queues/kvm-upstream

for the latest version.

Jan


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

[Qemu-devel] Re: [PATCH 12/15] kvm: Align kvm_arch_handle_exit to kvm_cpu_exec changes

[Alexander Graf]

On 11.03.2011, at 08:13, Jan Kiszka wrote:

> On 2011-03-11 07:50, Alexander Graf wrote:
>> 
>> On 04.03.2011, at 11:20, Jan Kiszka wrote:
>> 
>>> Make the return code of kvm_arch_handle_exit directly usable for
>>> kvm_cpu_exec. This is straightforward for x86 and ppc, just s390
>>> would require more work. Avoid this for now by pushing the return code
>>> translation logic into s390's kvm_arch_handle_exit.
>>> 
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> CC: Alexander Graf <agraf@suse.de>
>> 
>> Looks good, haven't tested it though. Do you have a git tree for all this?
>> 
> 
> See
> 
> git://git.kiszka.org/qemu-kvm.git queues/kvm-upstream

With the following patch s390x-softmmu compiles and runs the bootloader code just fine, breaks in early Linux boot code though. I haven't quite figured out why yet.


diff --git a/Makefile.target b/Makefile.target
index 220589e..21106c6 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -209,7 +209,7 @@ QEMU_CFLAGS += $(VNC_PNG_CFLAGS)
 obj-$(CONFIG_XEN) += xen_machine_pv.o xen_domainbuild.o
 
 # Inter-VM PCI shared memory
-obj-$(CONFIG_KVM) += ivshmem.o
+obj-i386-$(CONFIG_KVM) += ivshmem.o
 
 # Hardware support
 obj-i386-y += vga.o
diff --git a/exec.c b/exec.c
index 0b7a7b2..10e6528 100644
--- a/exec.c
+++ b/exec.c
@@ -2963,7 +2963,7 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
     RAMBlock *block;
     ram_addr_t offset;
     int flags;
-    void *area, *vaddr;
+    void *area = NULL, *vaddr;
 
     QLIST_FOREACH(block, &ram_list.blocks, next) {
         offset = addr - block->offset;

[Qemu-devel] Re: [PATCH 12/15] kvm: Align kvm_arch_handle_exit to kvm_cpu_exec changes

[Jan Kiszka]

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

On 2011-03-11 08:26, Alexander Graf wrote:
> 
> On 11.03.2011, at 08:13, Jan Kiszka wrote:
> 
>> On 2011-03-11 07:50, Alexander Graf wrote:
>>>
>>> On 04.03.2011, at 11:20, Jan Kiszka wrote:
>>>
>>>> Make the return code of kvm_arch_handle_exit directly usable for
>>>> kvm_cpu_exec. This is straightforward for x86 and ppc, just s390
>>>> would require more work. Avoid this for now by pushing the return code
>>>> translation logic into s390's kvm_arch_handle_exit.
>>>>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>> CC: Alexander Graf <agraf@suse.de>
>>>
>>> Looks good, haven't tested it though. Do you have a git tree for all this?
>>>
>>
>> See
>>
>> git://git.kiszka.org/qemu-kvm.git queues/kvm-upstream
> 
> With the following patch s390x-softmmu compiles and runs the bootloader code just fine, breaks in early Linux boot code though. I haven't quite figured out why yet.
> 
> 
> diff --git a/Makefile.target b/Makefile.target
> index 220589e..21106c6 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -209,7 +209,7 @@ QEMU_CFLAGS += $(VNC_PNG_CFLAGS)
>  obj-$(CONFIG_XEN) += xen_machine_pv.o xen_domainbuild.o
>  
>  # Inter-VM PCI shared memory
> -obj-$(CONFIG_KVM) += ivshmem.o
> +obj-i386-$(CONFIG_KVM) += ivshmem.o

Looks like s390 hasn't been built for a while - or what makes this
workaround necessary?

>  
>  # Hardware support
>  obj-i386-y += vga.o
> diff --git a/exec.c b/exec.c
> index 0b7a7b2..10e6528 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2963,7 +2963,7 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
>      RAMBlock *block;
>      ram_addr_t offset;
>      int flags;
> -    void *area, *vaddr;
> +    void *area = NULL, *vaddr;
>  
>      QLIST_FOREACH(block, &ram_list.blocks, next) {
>          offset = addr - block->offset;
> 
> 

Yeah, we should abort() on mem_path != 0 for unsupported targets.

Jan


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

[Qemu-devel] Re: [PATCH 12/15] kvm: Align kvm_arch_handle_exit to kvm_cpu_exec changes

[Alexander Graf]

On 11.03.2011, at 08:33, Jan Kiszka wrote:

> On 2011-03-11 08:26, Alexander Graf wrote:
>> 
>> On 11.03.2011, at 08:13, Jan Kiszka wrote:
>> 
>>> On 2011-03-11 07:50, Alexander Graf wrote:
>>>> 
>>>> On 04.03.2011, at 11:20, Jan Kiszka wrote:
>>>> 
>>>>> Make the return code of kvm_arch_handle_exit directly usable for
>>>>> kvm_cpu_exec. This is straightforward for x86 and ppc, just s390
>>>>> would require more work. Avoid this for now by pushing the return code
>>>>> translation logic into s390's kvm_arch_handle_exit.
>>>>> 
>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>> CC: Alexander Graf <agraf@suse.de>
>>>> 
>>>> Looks good, haven't tested it though. Do you have a git tree for all this?
>>>> 
>>> 
>>> See
>>> 
>>> git://git.kiszka.org/qemu-kvm.git queues/kvm-upstream
>> 
>> With the following patch s390x-softmmu compiles and runs the bootloader code just fine, breaks in early Linux boot code though. I haven't quite figured out why yet.
>> 
>> 
>> diff --git a/Makefile.target b/Makefile.target
>> index 220589e..21106c6 100644
>> --- a/Makefile.target
>> +++ b/Makefile.target
>> @@ -209,7 +209,7 @@ QEMU_CFLAGS += $(VNC_PNG_CFLAGS)
>> obj-$(CONFIG_XEN) += xen_machine_pv.o xen_domainbuild.o
>> 
>> # Inter-VM PCI shared memory
>> -obj-$(CONFIG_KVM) += ivshmem.o
>> +obj-i386-$(CONFIG_KVM) += ivshmem.o
> 
> Looks like s390 hasn't been built for a while - or what makes this
> workaround necessary?

It's been broken for quite a while, yes. I always fixed it locally in my trees, thinking "I'll get around to submitting a _proper_ patch upstream some day". Well, some day is a very long time span :).

> 
>> 
>> # Hardware support
>> obj-i386-y += vga.o
>> diff --git a/exec.c b/exec.c
>> index 0b7a7b2..10e6528 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -2963,7 +2963,7 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
>>     RAMBlock *block;
>>     ram_addr_t offset;
>>     int flags;
>> -    void *area, *vaddr;
>> +    void *area = NULL, *vaddr;
>> 
>>     QLIST_FOREACH(block, &ram_list.blocks, next) {
>>         offset = addr - block->offset;
>> 
>> 
> 
> Yeah, we should abort() on mem_path != 0 for unsupported targets.

Yes, that would work too :).


Alex

[Qemu-devel] Re: [PATCH 12/15] kvm: Align kvm_arch_handle_exit to kvm_cpu_exec changes

[Alexander Graf]

On 11.03.2011, at 08:13, Jan Kiszka wrote:

> On 2011-03-11 07:50, Alexander Graf wrote:
>> 
>> On 04.03.2011, at 11:20, Jan Kiszka wrote:
>> 
>>> Make the return code of kvm_arch_handle_exit directly usable for
>>> kvm_cpu_exec. This is straightforward for x86 and ppc, just s390
>>> would require more work. Avoid this for now by pushing the return code
>>> translation logic into s390's kvm_arch_handle_exit.
>>> 
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> CC: Alexander Graf <agraf@suse.de>
>> 
>> Looks good, haven't tested it though. Do you have a git tree for all this?
>> 
> 
> See
> 
> git://git.kiszka.org/qemu-kvm.git queues/kvm-upstream

ppc64 book3s works just fine.


Alex

[Qemu-devel] [PATCH 13/15] kvm: x86: Reorder functions in kvm.c

[Jan Kiszka]

Required for next patch which will access guest debug services from
kvm_arch_handle_exit. No functional changes.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 target-i386/kvm.c |  108 ++++++++++++++++++++++++++--------------------------
 1 files changed, 54 insertions(+), 54 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index b43a85c..103c86d 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1624,60 +1624,6 @@ static int kvm_handle_halt(CPUState *env)
     return 0;
 }
 
-static bool host_supports_vmx(void)
-{
-    uint32_t ecx, unused;
-
-    host_cpuid(1, 0, &unused, &unused, &ecx, &unused);
-    return ecx & CPUID_EXT_VMX;
-}
-
-#define VMX_INVALID_GUEST_STATE 0x80000021
-
-int kvm_arch_handle_exit(CPUState *env, struct kvm_run *run)
-{
-    uint64_t code;
-    int ret;
-
-    switch (run->exit_reason) {
-    case KVM_EXIT_HLT:
-        DPRINTF("handle_hlt\n");
-        ret = kvm_handle_halt(env);
-        break;
-    case KVM_EXIT_SET_TPR:
-        ret = 0;
-        break;
-    case KVM_EXIT_FAIL_ENTRY:
-        code = run->fail_entry.hardware_entry_failure_reason;
-        fprintf(stderr, "KVM: entry failed, hardware error 0x%" PRIx64 "\n",
-                code);
-        if (host_supports_vmx() && code == VMX_INVALID_GUEST_STATE) {
-            fprintf(stderr,
-                    "\nIf you're runnning a guest on an Intel machine without "
-                        "unrestricted mode\n"
-                    "support, the failure can be most likely due to the guest "
-                        "entering an invalid\n"
-                    "state for Intel VT. For example, the guest maybe running "
-                        "in big real mode\n"
-                    "which is not supported on less recent Intel processors."
-                        "\n\n");
-        }
-        ret = -1;
-        break;
-    case KVM_EXIT_EXCEPTION:
-        fprintf(stderr, "KVM: exception %d exit (error code 0x%x)\n",
-                run->ex.exception, run->ex.error_code);
-        ret = -1;
-        break;
-    default:
-        fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason);
-        ret = -1;
-        break;
-    }
-
-    return ret;
-}
-
 #ifdef KVM_CAP_SET_GUEST_DEBUG
 int kvm_arch_insert_sw_breakpoint(CPUState *env, struct kvm_sw_breakpoint *bp)
 {
@@ -1860,6 +1806,60 @@ void kvm_arch_update_guest_debug(CPUState *env, struct kvm_guest_debug *dbg)
 }
 #endif /* KVM_CAP_SET_GUEST_DEBUG */
 
+static bool host_supports_vmx(void)
+{
+    uint32_t ecx, unused;
+
+    host_cpuid(1, 0, &unused, &unused, &ecx, &unused);
+    return ecx & CPUID_EXT_VMX;
+}
+
+#define VMX_INVALID_GUEST_STATE 0x80000021
+
+int kvm_arch_handle_exit(CPUState *env, struct kvm_run *run)
+{
+    uint64_t code;
+    int ret;
+
+    switch (run->exit_reason) {
+    case KVM_EXIT_HLT:
+        DPRINTF("handle_hlt\n");
+        ret = kvm_handle_halt(env);
+        break;
+    case KVM_EXIT_SET_TPR:
+        ret = 0;
+        break;
+    case KVM_EXIT_FAIL_ENTRY:
+        code = run->fail_entry.hardware_entry_failure_reason;
+        fprintf(stderr, "KVM: entry failed, hardware error 0x%" PRIx64 "\n",
+                code);
+        if (host_supports_vmx() && code == VMX_INVALID_GUEST_STATE) {
+            fprintf(stderr,
+                    "\nIf you're runnning a guest on an Intel machine without "
+                        "unrestricted mode\n"
+                    "support, the failure can be most likely due to the guest "
+                        "entering an invalid\n"
+                    "state for Intel VT. For example, the guest maybe running "
+                        "in big real mode\n"
+                    "which is not supported on less recent Intel processors."
+                        "\n\n");
+        }
+        ret = -1;
+        break;
+    case KVM_EXIT_EXCEPTION:
+        fprintf(stderr, "KVM: exception %d exit (error code 0x%x)\n",
+                run->ex.exception, run->ex.error_code);
+        ret = -1;
+        break;
+    default:
+        fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason);
+        ret = -1;
+        break;
+    }
+
+    return ret;
+}
+
 bool kvm_arch_stop_on_emulation_error(CPUState *env)
 {
     return !(env->cr[0] & CR0_PE_MASK) ||
-- 
1.7.1

[Qemu-devel] [PATCH 14/15] kvm: x86: Push kvm_arch_debug to kvm_arch_handle_exit

[Jan Kiszka]

There are no generic bits remaining in the handling of KVM_EXIT_DEBUG.
So push its logic completely into arch hands, i.e. only x86 so far.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 kvm-all.c         |   11 -----------
 kvm.h             |    2 --
 target-i386/kvm.c |   25 ++++++++++++++++---------
 3 files changed, 16 insertions(+), 22 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index d643dc7..a534c06 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -993,17 +993,6 @@ int kvm_cpu_exec(CPUState *env)
             ret = kvm_handle_internal_error(env, run);
             break;
 #endif
-#ifdef KVM_CAP_SET_GUEST_DEBUG
-        case KVM_EXIT_DEBUG:
-            DPRINTF("kvm_exit_debug\n");
-            if (kvm_arch_debug(&run->debug.arch)) {
-                ret = EXCP_DEBUG;
-                break;
-            }
-            /* re-enter, this exception was guest-internal */
-            ret = 0;
-            break;
-#endif /* KVM_CAP_SET_GUEST_DEBUG */
         default:
             DPRINTF("kvm_arch_handle_exit\n");
             ret = kvm_arch_handle_exit(env, run);
diff --git a/kvm.h b/kvm.h
index 7bc04e0..d565dba 100644
--- a/kvm.h
+++ b/kvm.h
@@ -136,8 +136,6 @@ struct kvm_sw_breakpoint {
 
 QTAILQ_HEAD(kvm_sw_breakpoint_head, kvm_sw_breakpoint);
 
-int kvm_arch_debug(struct kvm_debug_exit_arch *arch_info);
-
 struct kvm_sw_breakpoint *kvm_find_sw_breakpoint(CPUState *env,
                                                  target_ulong pc);
 
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 103c86d..4258a5d 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1731,31 +1731,31 @@ void kvm_arch_remove_all_hw_breakpoints(void)
 
 static CPUWatchpoint hw_watchpoint;
 
-int kvm_arch_debug(struct kvm_debug_exit_arch *arch_info)
+static int kvm_handle_debug(struct kvm_debug_exit_arch *arch_info)
 {
-    int handle = 0;
+    int ret = 0;
     int n;
 
     if (arch_info->exception == 1) {
         if (arch_info->dr6 & (1 << 14)) {
             if (cpu_single_env->singlestep_enabled) {
-                handle = 1;
+                ret = EXCP_DEBUG;
             }
         } else {
             for (n = 0; n < 4; n++) {
                 if (arch_info->dr6 & (1 << n)) {
                     switch ((arch_info->dr7 >> (16 + n*4)) & 0x3) {
                     case 0x0:
-                        handle = 1;
+                        ret = EXCP_DEBUG;
                         break;
                     case 0x1:
-                        handle = 1;
+                        ret = EXCP_DEBUG;
                         cpu_single_env->watchpoint_hit = &hw_watchpoint;
                         hw_watchpoint.vaddr = hw_breakpoint[n].addr;
                         hw_watchpoint.flags = BP_MEM_WRITE;
                         break;
                     case 0x3:
-                        handle = 1;
+                        ret = EXCP_DEBUG;
                         cpu_single_env->watchpoint_hit = &hw_watchpoint;
                         hw_watchpoint.vaddr = hw_breakpoint[n].addr;
                         hw_watchpoint.flags = BP_MEM_ACCESS;
@@ -1765,17 +1765,18 @@ int kvm_arch_debug(struct kvm_debug_exit_arch *arch_info)
             }
         }
     } else if (kvm_find_sw_breakpoint(cpu_single_env, arch_info->pc)) {
-        handle = 1;
+        ret = EXCP_DEBUG;
     }
-    if (!handle) {
+    if (ret == 0) {
         cpu_synchronize_state(cpu_single_env);
         assert(cpu_single_env->exception_injected == -1);
 
+        /* pass to guest */
         cpu_single_env->exception_injected = arch_info->exception;
         cpu_single_env->has_error_code = 0;
     }
 
-    return handle;
+    return ret;
 }
 
 void kvm_arch_update_guest_debug(CPUState *env, struct kvm_guest_debug *dbg)
@@ -1851,6 +1852,12 @@ int kvm_arch_handle_exit(CPUState *env, struct kvm_run *run)
                 run->ex.exception, run->ex.error_code);
         ret = -1;
         break;
+#ifdef KVM_CAP_SET_GUEST_DEBUG
+    case KVM_EXIT_DEBUG:
+        DPRINTF("kvm_exit_debug\n");
+        ret = kvm_handle_debug(&run->debug.arch);
+        break;
+#endif /* KVM_CAP_SET_GUEST_DEBUG */
     default:
         fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason);
         ret = -1;
-- 
1.7.1

[Qemu-devel] [PATCH 15/15] Expose thread_id in info cpus

[Jan Kiszka]

Based on patch by Glauber Costa:

To allow management applications like libvirt to apply CPU affinities to
the VCPU threads, expose their ID via info cpus. This patch provides the
pre-existing and used interface from qemu-kvm.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpu-defs.h      |    1 +
 cpus.c          |    2 ++
 exec.c          |    3 +++
 monitor.c       |    4 ++++
 os-posix.c      |   10 ++++++++++
 os-win32.c      |    5 +++++
 osdep.h         |    1 +
 qmp-commands.hx |    3 +++
 8 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/cpu-defs.h b/cpu-defs.h
index 2b59fa6..db48a7a 100644
--- a/cpu-defs.h
+++ b/cpu-defs.h
@@ -203,6 +203,7 @@ typedef struct CPUWatchpoint {
     int nr_cores;  /* number of cores within this CPU package */        \
     int nr_threads;/* number of threads within this CPU */              \
     int running; /* Nonzero if cpu is currently running(usermode).  */  \
+    int thread_id;                                                      \
     /* user data */                                                     \
     void *opaque;                                                       \
                                                                         \
diff --git a/cpus.c b/cpus.c
index 66f6b5a..30fe568 100644
--- a/cpus.c
+++ b/cpus.c
@@ -810,6 +810,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
 
     qemu_mutex_lock(&qemu_global_mutex);
     qemu_thread_self(env->thread);
+    env->thread_id = qemu_get_thread_id();
 
     r = kvm_init_vcpu(env);
     if (r < 0) {
@@ -851,6 +852,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
     /* signal CPU creation */
     qemu_mutex_lock(&qemu_global_mutex);
     for (env = first_cpu; env != NULL; env = env->next_cpu) {
+        env->thread_id = qemu_get_thread_id();
         env->created = 1;
     }
     qemu_cond_signal(&qemu_cpu_cond);
diff --git a/exec.c b/exec.c
index a733acd..0b7a7b2 100644
--- a/exec.c
+++ b/exec.c
@@ -638,6 +638,9 @@ void cpu_exec_init(CPUState *env)
     env->numa_node = 0;
     QTAILQ_INIT(&env->breakpoints);
     QTAILQ_INIT(&env->watchpoints);
+#ifndef CONFIG_USER_ONLY
+    env->thread_id = qemu_get_thread_id();
+#endif
     *penv = env;
 #if defined(CONFIG_USER_ONLY)
     cpu_list_unlock();
diff --git a/monitor.c b/monitor.c
index ae20927..481572d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -897,6 +897,9 @@ static void print_cpu_iter(QObject *obj, void *opaque)
         monitor_printf(mon, " (halted)");
     }
 
+    monitor_printf(mon, " thread_id=%" PRId64 " ",
+                   qdict_get_int(cpu, "thread_id"));
+
     monitor_printf(mon, "\n");
 }
 
@@ -941,6 +944,7 @@ static void do_info_cpus(Monitor *mon, QObject **ret_data)
 #elif defined(TARGET_MIPS)
         qdict_put(cpu, "PC", qint_from_int(env->active_tc.PC));
 #endif
+        qdict_put(cpu, "thread_id", qint_from_int(env->thread_id));
 
         qlist_append(cpu_list, cpu);
     }
diff --git a/os-posix.c b/os-posix.c
index 38c29d1..7971f86 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -41,6 +41,7 @@
 
 #ifdef CONFIG_LINUX
 #include <sys/prctl.h>
+#include <sys/syscall.h>
 #endif
 
 #ifdef CONFIG_EVENTFD
@@ -382,3 +383,12 @@ int qemu_create_pidfile(const char *filename)
 
     return 0;
 }
+
+int qemu_get_thread_id(void)
+{
+#if defined (__linux__)
+    return syscall(SYS_gettid);
+#else
+    return getpid();
+#endif
+}
diff --git a/os-win32.c b/os-win32.c
index b214e6a..dd5517e 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -264,3 +264,8 @@ int qemu_create_pidfile(const char *filename)
     }
     return 0;
 }
+
+int qemu_get_thread_id(void)
+{
+    return GetCurrentThreadId();
+}
diff --git a/osdep.h b/osdep.h
index 27eedcf..748df54 100644
--- a/osdep.h
+++ b/osdep.h
@@ -130,5 +130,6 @@ void qemu_vfree(void *ptr);
 int qemu_madvise(void *addr, size_t len, int advice);
 
 int qemu_create_pidfile(const char *filename);
+int qemu_get_thread_id(void);
 
 #endif
diff --git a/qmp-commands.hx b/qmp-commands.hx
index df40a3d..1f72a8d 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1194,6 +1194,7 @@ Return a json-array. Each CPU is represented by a json-object, which contains:
      "nip": PPC (json-int)
      "pc" and "npc": sparc (json-int)
      "PC": mips (json-int)
+- "thread_id": ID of the underlying host thread (json-int)
 
 Example:
 
@@ -1205,12 +1206,14 @@ Example:
             "current":true,
             "halted":false,
             "pc":3227107138
+            "thread_id":3134
          },
          {
             "CPU":1,
             "current":false,
             "halted":true,
             "pc":7108165
+            "thread_id":3135
          }
       ]
    }
-- 
1.7.1