Patchwork [v2,3/8] target-i386: Add infrastructure for reporting TPR MMIO accesses

login
register
mail settings
Submitter Jan Kiszka
Date Feb. 10, 2012, 6:31 p.m.
Message ID <55354f49d412ae1b68f77ad6b81f90a12e913579.1328898681.git.jan.kiszka@siemens.com>
Download mbox | patch
Permalink /patch/140700/
State New
Headers show

Comments

Jan Kiszka - Feb. 10, 2012, 6:31 p.m.
This will allow the APIC core to file a TPR access report. Depending on
the accelerator and kernel irqchip mode, it will either be delivered
right away or queued for later reporting.

In TCG mode, we can restart the triggering instruction and can therefore
forward the event directly. KVM does not allows us to restart, so we
postpone the delivery of events recording in the user space APIC until
the current instruction is completed.

Note that KVM without in-kernel irqchip will report the address after
the instruction that triggered a write access. In contrast, read
accesses will return the precise information.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpu-all.h            |    3 ++-
 hw/apic.h            |    2 ++
 hw/apic_common.c     |    4 ++++
 target-i386/cpu.h    |    9 +++++++++
 target-i386/helper.c |   19 +++++++++++++++++++
 target-i386/kvm.c    |   24 ++++++++++++++++++++++--
 6 files changed, 58 insertions(+), 3 deletions(-)
Blue Swirl - Feb. 11, 2012, 2:32 p.m.
On Fri, Feb 10, 2012 at 18:31, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> This will allow the APIC core to file a TPR access report. Depending on
> the accelerator and kernel irqchip mode, it will either be delivered
> right away or queued for later reporting.
>
> In TCG mode, we can restart the triggering instruction and can therefore
> forward the event directly. KVM does not allows us to restart, so we
> postpone the delivery of events recording in the user space APIC until
> the current instruction is completed.
>
> Note that KVM without in-kernel irqchip will report the address after
> the instruction that triggered a write access. In contrast, read
> accesses will return the precise information.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  cpu-all.h            |    3 ++-
>  hw/apic.h            |    2 ++
>  hw/apic_common.c     |    4 ++++
>  target-i386/cpu.h    |    9 +++++++++
>  target-i386/helper.c |   19 +++++++++++++++++++
>  target-i386/kvm.c    |   24 ++++++++++++++++++++++--
>  6 files changed, 58 insertions(+), 3 deletions(-)
>
> diff --git a/cpu-all.h b/cpu-all.h
> index e2c3c49..80e6d42 100644
> --- a/cpu-all.h
> +++ b/cpu-all.h
> @@ -375,8 +375,9 @@ DECLARE_TLS(CPUState *,cpu_single_env);
>  #define CPU_INTERRUPT_TGT_INT_0   0x0100
>  #define CPU_INTERRUPT_TGT_INT_1   0x0400
>  #define CPU_INTERRUPT_TGT_INT_2   0x0800
> +#define CPU_INTERRUPT_TGT_INT_3   0x2000
>
> -/* First unused bit: 0x2000.  */
> +/* First unused bit: 0x4000.  */
>
>  /* The set of all bits that should be masked when single-stepping.  */
>  #define CPU_INTERRUPT_SSTEP_MASK \
> diff --git a/hw/apic.h b/hw/apic.h
> index a62d83b..45598bd 100644
> --- a/hw/apic.h
> +++ b/hw/apic.h
> @@ -18,6 +18,8 @@ void cpu_set_apic_tpr(DeviceState *s, uint8_t val);
>  uint8_t cpu_get_apic_tpr(DeviceState *s);
>  void apic_init_reset(DeviceState *s);
>  void apic_sipi(DeviceState *s);
> +void apic_handle_tpr_access_report(DeviceState *d, target_ulong ip,
> +                                   int access);
>
>  /* pc.c */
>  int cpu_is_bsp(CPUState *env);
> diff --git a/hw/apic_common.c b/hw/apic_common.c
> index 8373d79..588531b 100644
> --- a/hw/apic_common.c
> +++ b/hw/apic_common.c
> @@ -68,6 +68,10 @@ uint8_t cpu_get_apic_tpr(DeviceState *d)
>     return s ? s->tpr >> 4 : 0;
>  }
>
> +void apic_handle_tpr_access_report(DeviceState *d, target_ulong ip, int access)
> +{
> +}
> +
>  void apic_report_irq_delivered(int delivered)
>  {
>     apic_irq_delivered += delivered;
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 37dde79..92e9c87 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -482,6 +482,7 @@
>  #define CPU_INTERRUPT_VIRQ      CPU_INTERRUPT_TGT_INT_0
>  #define CPU_INTERRUPT_INIT      CPU_INTERRUPT_TGT_INT_1
>  #define CPU_INTERRUPT_SIPI      CPU_INTERRUPT_TGT_INT_2
> +#define CPU_INTERRUPT_TPR       CPU_INTERRUPT_TGT_INT_3
>
>
>  enum {
> @@ -772,6 +773,9 @@ typedef struct CPUX86State {
>     XMMReg ymmh_regs[CPU_NB_REGS];
>
>     uint64_t xcr0;
> +
> +    target_ulong tpr_access_ip;
> +    int tpr_access_type;
>  } CPUX86State;
>
>  CPUX86State *cpu_x86_init(const char *cpu_model);
> @@ -1064,4 +1068,9 @@ void svm_check_intercept(CPUState *env1, uint32_t type);
>
>  uint32_t cpu_cc_compute_all(CPUState *env1, int op);
>
> +#define TPR_ACCESS_READ     0
> +#define TPR_ACCESS_WRITE    1

enum would be nicer.

> +
> +void cpu_report_tpr_access(CPUState *env, int access);
> +
>  #endif /* CPU_I386_H */
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index 2586aff..eca20cd 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -1189,6 +1189,25 @@ void cpu_x86_inject_mce(Monitor *mon, CPUState *cenv, int bank,
>         }
>     }
>  }
> +
> +void cpu_report_tpr_access(CPUState *env, int access)
> +{
> +    TranslationBlock *tb;
> +
> +    if (kvm_enabled()) {
> +        cpu_synchronize_state(env);
> +
> +        env->tpr_access_ip = env->eip;
> +        env->tpr_access_type = access;
> +
> +        cpu_interrupt(env, CPU_INTERRUPT_TPR);
> +    } else {
> +        tb = tb_find_pc(env->mem_io_pc);
> +        cpu_restore_state(tb, env, env->mem_io_pc);
> +
> +        apic_handle_tpr_access_report(env->apic_state, env->eip, access);
> +    }
> +}
>  #endif /* !CONFIG_USER_ONLY */
>
>  static void mce_init(CPUX86State *cenv)
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 981192d..fa77f9d 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -1635,8 +1635,10 @@ void kvm_arch_pre_run(CPUState *env, struct kvm_run *run)
>     }
>
>     if (!kvm_irqchip_in_kernel()) {
> -        /* Force the VCPU out of its inner loop to process the INIT request */
> -        if (env->interrupt_request & CPU_INTERRUPT_INIT) {
> +        /* Force the VCPU out of its inner loop to process any INIT requests
> +         * or pending TPR access reports. */
> +        if (env->interrupt_request &
> +            (CPU_INTERRUPT_INIT | CPU_INTERRUPT_TPR)) {
>             env->exit_request = 1;
>         }
>
> @@ -1730,6 +1732,11 @@ int kvm_arch_process_async_events(CPUState *env)
>         kvm_cpu_synchronize_state(env);
>         do_cpu_sipi(env);
>     }
> +    if (env->interrupt_request & CPU_INTERRUPT_TPR) {
> +        env->interrupt_request &= ~CPU_INTERRUPT_TPR;
> +        apic_handle_tpr_access_report(env->apic_state, env->tpr_access_ip,
> +                                      env->tpr_access_type);
> +    }
>
>     return env->halted;
>  }
> @@ -1746,6 +1753,16 @@ static int kvm_handle_halt(CPUState *env)
>     return 0;
>  }
>
> +static int kvm_handle_tpr_access(CPUState *env)
> +{
> +    struct kvm_run *run = env->kvm_run;
> +
> +    apic_handle_tpr_access_report(env->apic_state, run->tpr_access.rip,
> +                                  run->tpr_access.is_write ? TPR_ACCESS_WRITE
> +                                                           : TPR_ACCESS_READ);
> +    return 1;
> +}
> +
>  int kvm_arch_insert_sw_breakpoint(CPUState *env, struct kvm_sw_breakpoint *bp)
>  {
>     static const uint8_t int3 = 0xcc;
> @@ -1950,6 +1967,9 @@ int kvm_arch_handle_exit(CPUState *env, struct kvm_run *run)
>     case KVM_EXIT_SET_TPR:
>         ret = 0;
>         break;
> +    case KVM_EXIT_TPR_ACCESS:
> +        ret = kvm_handle_tpr_access(env);
> +        break;
>     case KVM_EXIT_FAIL_ENTRY:
>         code = run->fail_entry.hardware_entry_failure_reason;
>         fprintf(stderr, "KVM: entry failed, hardware error 0x%" PRIx64 "\n",
> --
> 1.7.3.4
>
>

Patch

diff --git a/cpu-all.h b/cpu-all.h
index e2c3c49..80e6d42 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -375,8 +375,9 @@  DECLARE_TLS(CPUState *,cpu_single_env);
 #define CPU_INTERRUPT_TGT_INT_0   0x0100
 #define CPU_INTERRUPT_TGT_INT_1   0x0400
 #define CPU_INTERRUPT_TGT_INT_2   0x0800
+#define CPU_INTERRUPT_TGT_INT_3   0x2000
 
-/* First unused bit: 0x2000.  */
+/* First unused bit: 0x4000.  */
 
 /* The set of all bits that should be masked when single-stepping.  */
 #define CPU_INTERRUPT_SSTEP_MASK \
diff --git a/hw/apic.h b/hw/apic.h
index a62d83b..45598bd 100644
--- a/hw/apic.h
+++ b/hw/apic.h
@@ -18,6 +18,8 @@  void cpu_set_apic_tpr(DeviceState *s, uint8_t val);
 uint8_t cpu_get_apic_tpr(DeviceState *s);
 void apic_init_reset(DeviceState *s);
 void apic_sipi(DeviceState *s);
+void apic_handle_tpr_access_report(DeviceState *d, target_ulong ip,
+                                   int access);
 
 /* pc.c */
 int cpu_is_bsp(CPUState *env);
diff --git a/hw/apic_common.c b/hw/apic_common.c
index 8373d79..588531b 100644
--- a/hw/apic_common.c
+++ b/hw/apic_common.c
@@ -68,6 +68,10 @@  uint8_t cpu_get_apic_tpr(DeviceState *d)
     return s ? s->tpr >> 4 : 0;
 }
 
+void apic_handle_tpr_access_report(DeviceState *d, target_ulong ip, int access)
+{
+}
+
 void apic_report_irq_delivered(int delivered)
 {
     apic_irq_delivered += delivered;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 37dde79..92e9c87 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -482,6 +482,7 @@ 
 #define CPU_INTERRUPT_VIRQ      CPU_INTERRUPT_TGT_INT_0
 #define CPU_INTERRUPT_INIT      CPU_INTERRUPT_TGT_INT_1
 #define CPU_INTERRUPT_SIPI      CPU_INTERRUPT_TGT_INT_2
+#define CPU_INTERRUPT_TPR       CPU_INTERRUPT_TGT_INT_3
 
 
 enum {
@@ -772,6 +773,9 @@  typedef struct CPUX86State {
     XMMReg ymmh_regs[CPU_NB_REGS];
 
     uint64_t xcr0;
+
+    target_ulong tpr_access_ip;
+    int tpr_access_type;
 } CPUX86State;
 
 CPUX86State *cpu_x86_init(const char *cpu_model);
@@ -1064,4 +1068,9 @@  void svm_check_intercept(CPUState *env1, uint32_t type);
 
 uint32_t cpu_cc_compute_all(CPUState *env1, int op);
 
+#define TPR_ACCESS_READ     0
+#define TPR_ACCESS_WRITE    1
+
+void cpu_report_tpr_access(CPUState *env, int access);
+
 #endif /* CPU_I386_H */
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 2586aff..eca20cd 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1189,6 +1189,25 @@  void cpu_x86_inject_mce(Monitor *mon, CPUState *cenv, int bank,
         }
     }
 }
+
+void cpu_report_tpr_access(CPUState *env, int access)
+{
+    TranslationBlock *tb;
+
+    if (kvm_enabled()) {
+        cpu_synchronize_state(env);
+
+        env->tpr_access_ip = env->eip;
+        env->tpr_access_type = access;
+
+        cpu_interrupt(env, CPU_INTERRUPT_TPR);
+    } else {
+        tb = tb_find_pc(env->mem_io_pc);
+        cpu_restore_state(tb, env, env->mem_io_pc);
+
+        apic_handle_tpr_access_report(env->apic_state, env->eip, access);
+    }
+}
 #endif /* !CONFIG_USER_ONLY */
 
 static void mce_init(CPUX86State *cenv)
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 981192d..fa77f9d 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1635,8 +1635,10 @@  void kvm_arch_pre_run(CPUState *env, struct kvm_run *run)
     }
 
     if (!kvm_irqchip_in_kernel()) {
-        /* Force the VCPU out of its inner loop to process the INIT request */
-        if (env->interrupt_request & CPU_INTERRUPT_INIT) {
+        /* Force the VCPU out of its inner loop to process any INIT requests
+         * or pending TPR access reports. */
+        if (env->interrupt_request &
+            (CPU_INTERRUPT_INIT | CPU_INTERRUPT_TPR)) {
             env->exit_request = 1;
         }
 
@@ -1730,6 +1732,11 @@  int kvm_arch_process_async_events(CPUState *env)
         kvm_cpu_synchronize_state(env);
         do_cpu_sipi(env);
     }
+    if (env->interrupt_request & CPU_INTERRUPT_TPR) {
+        env->interrupt_request &= ~CPU_INTERRUPT_TPR;
+        apic_handle_tpr_access_report(env->apic_state, env->tpr_access_ip,
+                                      env->tpr_access_type);
+    }
 
     return env->halted;
 }
@@ -1746,6 +1753,16 @@  static int kvm_handle_halt(CPUState *env)
     return 0;
 }
 
+static int kvm_handle_tpr_access(CPUState *env)
+{
+    struct kvm_run *run = env->kvm_run;
+
+    apic_handle_tpr_access_report(env->apic_state, run->tpr_access.rip,
+                                  run->tpr_access.is_write ? TPR_ACCESS_WRITE
+                                                           : TPR_ACCESS_READ);
+    return 1;
+}
+
 int kvm_arch_insert_sw_breakpoint(CPUState *env, struct kvm_sw_breakpoint *bp)
 {
     static const uint8_t int3 = 0xcc;
@@ -1950,6 +1967,9 @@  int kvm_arch_handle_exit(CPUState *env, struct kvm_run *run)
     case KVM_EXIT_SET_TPR:
         ret = 0;
         break;
+    case KVM_EXIT_TPR_ACCESS:
+        ret = kvm_handle_tpr_access(env);
+        break;
     case KVM_EXIT_FAIL_ENTRY:
         code = run->fail_entry.hardware_entry_failure_reason;
         fprintf(stderr, "KVM: entry failed, hardware error 0x%" PRIx64 "\n",