[Qemu-devel] Re: [PATCH 04/35] Add "broadcast" option for mce command

[Jan Kiszka]

Am 06.01.2011 18:56, Marcelo Tosatti wrote:
> From: Jin Dongming <address@hidden>
> 
> When the following test case is injected with mce command, maybe user could 
> not
> get the expected result.
>     DATA
>                command cpu bank status             mcg_status  addr   misc
>         (qemu) mce     1   1    0xbd00000000000000 0x05        0x1234 0x8c
> 
>     Expected Result
>            panic type: "Fatal Machine check"
> 
> That is because each mce command can only inject the given cpu and could not
> inject mce interrupt to other cpus. So user will get the following result:
>     panic type: "Fatal machine check on current CPU"
> 
> "broadcast" option is used for injecting dummy data into other cpus. Injecting
> mce with this option the expected result could be gotten.
> 
> Usage:
>     Broadcast[on]
>            command broadcast cpu bank status             mcg_status  addr   
> misc
>     (qemu) mce     -b        1   1    0xbd00000000000000 0x05        0x1234 
> 0x8c
> 
>     Broadcast[off]
>            command cpu bank status             mcg_status  addr   misc
>     (qemu) mce     1   1    0xbd00000000000000 0x05        0x1234 0x8c
> 
> Signed-off-by: Jin Dongming <address@hidden>
> Signed-off-by: Marcelo Tosatti <address@hidden>
> ---
>  cpu-all.h             |    3 ++-
>  hmp-commands.hx       |    6 +++---
>  monitor.c             |    7 +++++--
>  target-i386/helper.c  |   20 ++++++++++++++++++--
>  target-i386/kvm.c     |   16 ++++++++++++----
>  target-i386/kvm_x86.h |    5 ++++-
>  6 files changed, 44 insertions(+), 13 deletions(-)
> 
> diff --git a/cpu-all.h b/cpu-all.h
> index 30ae17d..4ce4e83 100644
> --- a/cpu-all.h
> +++ b/cpu-all.h
> @@ -964,6 +964,7 @@ int cpu_memory_rw_debug(CPUState *env, target_ulong addr,
>                          uint8_t *buf, int len, int is_write);
>  
>  void cpu_inject_x86_mce(CPUState *cenv, int bank, uint64_t status,
> -                        uint64_t mcg_status, uint64_t addr, uint64_t misc);
> +                        uint64_t mcg_status, uint64_t addr, uint64_t misc,
> +                        int broadcast);
>  
>  #endif /* CPU_ALL_H */
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index df134f8..c82fb10 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1091,9 +1091,9 @@ ETEXI
>  
>      {
>          .name       = "mce",
> -        .args_type  = 
> "cpu_index:i,bank:i,status:l,mcg_status:l,addr:l,misc:l",
> -        .params     = "cpu bank status mcgstatus addr misc",
> -        .help       = "inject a MCE on the given CPU",
> +        .args_type  = 
> "broadcast:-b,cpu_index:i,bank:i,status:l,mcg_status:l,addr:l,misc:l",
> +        .params     = "[-b] cpu bank status mcgstatus addr misc",
> +        .help       = "inject a MCE on the given CPU [and broadcast to other 
> CPUs with -b option]",
>          .mhandler.cmd = do_inject_mce,
>      },
>  
> diff --git a/monitor.c b/monitor.c
> index f258000..f4f624b 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2671,12 +2671,15 @@ static void do_inject_mce(Monitor *mon, const QDict 
> *qdict)
>      uint64_t mcg_status = qdict_get_int(qdict, "mcg_status");
>      uint64_t addr = qdict_get_int(qdict, "addr");
>      uint64_t misc = qdict_get_int(qdict, "misc");
> +    int broadcast = qdict_get_try_bool(qdict, "broadcast", 0);
>  
> -    for (cenv = first_cpu; cenv != NULL; cenv = cenv->next_cpu)
> +    for (cenv = first_cpu; cenv != NULL; cenv = cenv->next_cpu) {
>          if (cenv->cpu_index == cpu_index && cenv->mcg_cap) {
> -            cpu_inject_x86_mce(cenv, bank, status, mcg_status, addr, misc);
> +            cpu_inject_x86_mce(cenv, bank, status, mcg_status, addr, misc,
> +                               broadcast);
>              break;
>          }
> +    }
>  }
>  #endif
>  
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index 2c94130..2cfb4a4 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -1069,18 +1069,34 @@ static void qemu_inject_x86_mce(CPUState *cenv, int 
> bank, uint64_t status,
>  }
>  
>  void cpu_inject_x86_mce(CPUState *cenv, int bank, uint64_t status,
> -                        uint64_t mcg_status, uint64_t addr, uint64_t misc)
> +                        uint64_t mcg_status, uint64_t addr, uint64_t misc,
> +                        int broadcast)
>  {
>      unsigned bank_num = cenv->mcg_cap & 0xff;
> +    CPUState *env;
> +    int flag = 0;
>  
>      if (bank >= bank_num || !(status & MCI_STATUS_VAL)) {
>          return;
>      }
>  
>      if (kvm_enabled()) {
> -        kvm_inject_x86_mce(cenv, bank, status, mcg_status, addr, misc, 0);
> +        if (broadcast) {
> +            flag |= MCE_BROADCAST;
> +        }
> +
> +        kvm_inject_x86_mce(cenv, bank, status, mcg_status, addr, misc, flag);
>      } else {
>          qemu_inject_x86_mce(cenv, bank, status, mcg_status, addr, misc);
> +        if (broadcast) {
> +            for (env = first_cpu; env != NULL; env = env->next_cpu) {
> +                if (cenv == env) {
> +                    continue;
> +                }
> +
> +                qemu_inject_x86_mce(env, 1, 0xa000000000000000, 0, 0, 0);

Constant lacks "ULL". Can probably be fixed up on commit.

Jan

> +            }
> +        }
>      }
>  }
>  #endif /* !CONFIG_USER_ONLY */
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 4004de7..8b868ad 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -264,11 +264,13 @@ static void kvm_do_inject_x86_mce(void *_data)
>          }
>      }
>  }
> +
> +static void kvm_mce_broadcast_rest(CPUState *env);
>  #endif
>  
>  void kvm_inject_x86_mce(CPUState *cenv, int bank, uint64_t status,
>                          uint64_t mcg_status, uint64_t addr, uint64_t misc,
> -                        int abort_on_error)
> +                        int flag)
>  {
>  #ifdef KVM_CAP_MCE
>      struct kvm_x86_mce mce = {
> @@ -288,10 +290,15 @@ void kvm_inject_x86_mce(CPUState *cenv, int bank, 
> uint64_t status,
>          return;
>      }
>  
> +    if (flag & MCE_BROADCAST) {
> +        kvm_mce_broadcast_rest(cenv);
> +    }
> +
>      run_on_cpu(cenv, kvm_do_inject_x86_mce, &data);
>  #else
> -    if (abort_on_error)
> +    if (flag & ABORT_ON_ERROR) {
>          abort();
> +    }
>  #endif
>  }
>  
> @@ -1716,7 +1723,8 @@ static void kvm_mce_broadcast_rest(CPUState *env)
>                  continue;
>              }
>              kvm_inject_x86_mce(cenv, 1, MCI_STATUS_VAL | MCI_STATUS_UC,
> -                               MCG_STATUS_MCIP | MCG_STATUS_RIPV, 0, 0, 1);
> +                               MCG_STATUS_MCIP | MCG_STATUS_RIPV, 0, 0,
> +                               ABORT_ON_ERROR);
>          }
>      }
>  }
> @@ -1816,7 +1824,7 @@ int kvm_on_sigbus(int code, void *addr)
>              | 0xc0;
>          kvm_inject_x86_mce(first_cpu, 9, status,
>                             MCG_STATUS_MCIP | MCG_STATUS_RIPV, paddr,
> -                           (MCM_ADDR_PHYS << 6) | 0xc, 1);
> +                           (MCM_ADDR_PHYS << 6) | 0xc, ABORT_ON_ERROR);
>          kvm_mce_broadcast_rest(first_cpu);
>      } else
>  #endif
> diff --git a/target-i386/kvm_x86.h b/target-i386/kvm_x86.h
> index 04932cf..9d7b584 100644
> --- a/target-i386/kvm_x86.h
> +++ b/target-i386/kvm_x86.h
> @@ -15,8 +15,11 @@
>  #ifndef __KVM_X86_H__
>  #define __KVM_X86_H__
>  
> +#define ABORT_ON_ERROR  0x01
> +#define MCE_BROADCAST   0x02
> +
>  void kvm_inject_x86_mce(CPUState *cenv, int bank, uint64_t status,
>                          uint64_t mcg_status, uint64_t addr, uint64_t misc,
> -                        int abort_on_error);
> +                        int flag);
>  
>  #endif

signature.asc
Description: OpenPGP digital signature