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

Marcelo Tosatti
2010-12-02T16:46:31Z

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.