Re: [PATCH 2/3] Add "broadcast" option for mce command.

[Marcelo Tosatti <mtosatti@redhat.com>]

On Tue, Nov 30, 2010 at 05:14:13PM +0900, Jin Dongming wrote:
> 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.
> 
> Signed-off-by: Jin Dongming <jin.dongming@np.css.fujitsu.com>
> ---
>  cpu-all.h             |    3 ++-
>  hmp-commands.hx       |    6 +++---
>  monitor.c             |   18 ++++++++++++++++--
>  target-i386/helper.c  |   17 +++++++++++++++--
>  target-i386/kvm.c     |   12 ++++++++++--
>  target-i386/kvm_x86.h |    2 +-
>  6 files changed, 47 insertions(+), 11 deletions(-)
> 
> diff --git a/cpu-all.h b/cpu-all.h
> index 11edddc..a3b3cd8 100644
> --- a/cpu-all.h
> +++ b/cpu-all.h
> @@ -967,6 +967,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 ba6de28..44ad571 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1053,9 +1053,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  = "cpu_index:i,bank:i,status:l,mcg_status:l,addr:l,misc:l,broadcast:s?",
> +        .params     = "cpu bank status mcgstatus addr misc [broadcast|b]",
> +        .help       = "inject a MCE on the given CPU [and broadcast to other CPUs]",
>          .mhandler.cmd = do_inject_mce,
>      },
>  
> diff --git a/monitor.c b/monitor.c
> index 66d6acd..e316eb6 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2263,12 +2263,26 @@ 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");
> +    const char *b = qdict_get_try_str(qdict, "broadcast");

qdict_get_try_bool is easier.

> +    int broadcast = 0;
>  
> -    for (cenv = first_cpu; cenv != NULL; cenv = cenv->next_cpu)
> +    if (b) {
> +        if (!strncmp(b, "broadcast", sizeof("broadcast")) ||
> +            !strncmp(b, "b", sizeof("b"))) {
> +            broadcast = 1;
> +        } else {
> +            monitor_printf(mon, "unexpected option: %s\n", b);
> +            return;
> +        }
> +    }
> +
> +    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 e384fdc..7e07ebd 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -1071,17 +1071,30 @@ 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 |= 0x02; /* bit 1: 1(broadcast); 0(not 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);
> +            }
> +        }
>      }
>  }
>  #endif /* !CONFIG_USER_ONLY */
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index b7b2430..7eaa388 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -261,11 +261,15 @@ static void kvm_do_inject_x86_mce(void *_data)
>          }
>      }
>  }
> +
> +static void kvm_mce_broadcast_rest(CPUState *env);
>  #endif
>  
> +#define ABORT_ON_ERROR  0x01
> +#define MCE_BROADCAST   0x02

Move to kvm_x86.h and use names everywhere.