Re: [PATCH v2 2/8] ppc/spapr: Change FWNMI names
On Tue, Mar 17, 2020 at 12:26:07AM +1000, Nicholas Piggin wrote: > The option is called "FWNMI", and it involves more than just machine > checks, also machine checks can be delivered without the FWNMI option, > so re-name various things to reflect that. > > Signed-off-by: Nicholas Piggin Applied to ppc-for-5.0, thanks. > --- > hw/ppc/spapr.c| 28 ++-- > hw/ppc/spapr_caps.c | 14 +++--- > hw/ppc/spapr_events.c | 14 +++--- > hw/ppc/spapr_rtas.c | 17 + > include/hw/ppc/spapr.h| 27 +-- > tests/qtest/libqos/libqos-spapr.h | 2 +- > 6 files changed, 55 insertions(+), 47 deletions(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index d3db3ec56e..b03b26370d 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1704,11 +1704,11 @@ static void spapr_machine_reset(MachineState *machine) > > spapr->cas_reboot = false; > > -spapr->mc_status = -1; > -spapr->guest_machine_check_addr = -1; > +spapr->fwnmi_machine_check_addr = -1; > +spapr->fwnmi_machine_check_interlock = -1; > > /* Signal all vCPUs waiting on this condition */ > -qemu_cond_broadcast(&spapr->mc_delivery_cond); > +qemu_cond_broadcast(&spapr->fwnmi_machine_check_interlock_cond); > > migrate_del_blocker(spapr->fwnmi_migration_blocker); > } > @@ -1997,7 +1997,7 @@ static bool spapr_fwnmi_needed(void *opaque) > { > SpaprMachineState *spapr = (SpaprMachineState *)opaque; > > -return spapr->guest_machine_check_addr != -1; > +return spapr->fwnmi_machine_check_addr != -1; > } > > static int spapr_fwnmi_pre_save(void *opaque) > @@ -2008,7 +2008,7 @@ static int spapr_fwnmi_pre_save(void *opaque) > * Check if machine check handling is in progress and print a > * warning message. > */ > -if (spapr->mc_status != -1) { > +if (spapr->fwnmi_machine_check_interlock != -1) { > warn_report("A machine check is being handled during migration. The" > "handler may run and log hardware error on the destination"); > } > @@ -2016,15 +2016,15 @@ static int spapr_fwnmi_pre_save(void *opaque) > return 0; > } > > -static const VMStateDescription vmstate_spapr_machine_check = { > -.name = "spapr_machine_check", > +static const VMStateDescription vmstate_spapr_fwnmi = { > +.name = "spapr_fwnmi", > .version_id = 1, > .minimum_version_id = 1, > .needed = spapr_fwnmi_needed, > .pre_save = spapr_fwnmi_pre_save, > .fields = (VMStateField[]) { > -VMSTATE_UINT64(guest_machine_check_addr, SpaprMachineState), > -VMSTATE_INT32(mc_status, SpaprMachineState), > +VMSTATE_UINT64(fwnmi_machine_check_addr, SpaprMachineState), > +VMSTATE_INT32(fwnmi_machine_check_interlock, SpaprMachineState), > VMSTATE_END_OF_LIST() > }, > }; > @@ -2063,7 +2063,7 @@ static const VMStateDescription vmstate_spapr = { > &vmstate_spapr_cap_large_decr, > &vmstate_spapr_cap_ccf_assist, > &vmstate_spapr_cap_fwnmi, > -&vmstate_spapr_machine_check, > +&vmstate_spapr_fwnmi, > NULL > } > }; > @@ -2884,7 +2884,7 @@ static void spapr_machine_init(MachineState *machine) > spapr_create_lmb_dr_connectors(spapr); > } > > -if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == SPAPR_CAP_ON) { > +if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI) == SPAPR_CAP_ON) { > /* Create the error string for live migration blocker */ > error_setg(&spapr->fwnmi_migration_blocker, > "A machine check is being handled during migration. The handler" > @@ -3053,7 +3053,7 @@ static void spapr_machine_init(MachineState *machine) > kvmppc_spapr_enable_inkernel_multitce(); > } > > -qemu_cond_init(&spapr->mc_delivery_cond); > +qemu_cond_init(&spapr->fwnmi_machine_check_interlock_cond); > } > > static int spapr_kvm_type(MachineState *machine, const char *vm_type) > @@ -4534,7 +4534,7 @@ static void spapr_machine_class_init(ObjectClass *oc, > void *data) > smc->default_caps.caps[SPAPR_CAP_NESTED_KVM_HV] = SPAPR_CAP_OFF; > smc->default_caps.caps[SPAPR_CAP_LARGE_DECREMENTER] = SPAPR_CAP_ON; > smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_ON; > -smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_ON; > +smc->default_caps.caps[SPAPR_CAP_FWNMI] = SPAPR_CAP_ON; > spapr_caps_add_properties(smc, &error_abort); > smc->irq = &spapr_irq_dual; > smc->dr_phb_enabled = true; > @@ -4612,7 +4612,7 @@ static void > spapr_machine_4_2_class_options(MachineClass *mc) > spapr_machine_5_0_class_options(mc); > compat_props_add(mc->compat_props, hw_compat_4_2, hw_compat_4_2_len); > smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_OFF; > -smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_OFF; > +
Re: [PATCH v2 2/8] ppc/spapr: Change FWNMI names
On 2020-03-17 00:26:07 Tue, Nicholas Piggin wrote: > The option is called "FWNMI", and it involves more than just machine > checks, also machine checks can be delivered without the FWNMI option, > so re-name various things to reflect that. > > Signed-off-by: Nicholas Piggin > --- > hw/ppc/spapr.c| 28 ++-- > hw/ppc/spapr_caps.c | 14 +++--- > hw/ppc/spapr_events.c | 14 +++--- > hw/ppc/spapr_rtas.c | 17 + > include/hw/ppc/spapr.h| 27 +-- > tests/qtest/libqos/libqos-spapr.h | 2 +- > 6 files changed, 55 insertions(+), 47 deletions(-) > [...] > @@ -626,14 +626,14 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = { > .type = "bool", > .apply = cap_ccf_assist_apply, > }, > -[SPAPR_CAP_FWNMI_MCE] = { > -.name = "fwnmi-mce", > -.description = "Handle fwnmi machine check exceptions", > -.index = SPAPR_CAP_FWNMI_MCE, > +[SPAPR_CAP_FWNMI] = { > +.name = "fwnmi", I guess this should be fine and should hit QEMU 5.0 release so that we don't end up with two different CAP names for 5.0 and future releases. Thanks, -Mahesh.
Re: [PATCH v2 2/8] ppc/spapr: Change FWNMI names
On 3/16/20 3:26 PM, Nicholas Piggin wrote: > The option is called "FWNMI", and it involves more than just machine > checks, also machine checks can be delivered without the FWNMI option, > so re-name various things to reflect that. Reviewed-by: Cédric Le Goater > Signed-off-by: Nicholas Piggin > --- > hw/ppc/spapr.c| 28 ++-- > hw/ppc/spapr_caps.c | 14 +++--- > hw/ppc/spapr_events.c | 14 +++--- > hw/ppc/spapr_rtas.c | 17 + > include/hw/ppc/spapr.h| 27 +-- > tests/qtest/libqos/libqos-spapr.h | 2 +- > 6 files changed, 55 insertions(+), 47 deletions(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index d3db3ec56e..b03b26370d 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1704,11 +1704,11 @@ static void spapr_machine_reset(MachineState *machine) > > spapr->cas_reboot = false; > > -spapr->mc_status = -1; > -spapr->guest_machine_check_addr = -1; > +spapr->fwnmi_machine_check_addr = -1; > +spapr->fwnmi_machine_check_interlock = -1; > > /* Signal all vCPUs waiting on this condition */ > -qemu_cond_broadcast(&spapr->mc_delivery_cond); > +qemu_cond_broadcast(&spapr->fwnmi_machine_check_interlock_cond); > > migrate_del_blocker(spapr->fwnmi_migration_blocker); > } > @@ -1997,7 +1997,7 @@ static bool spapr_fwnmi_needed(void *opaque) > { > SpaprMachineState *spapr = (SpaprMachineState *)opaque; > > -return spapr->guest_machine_check_addr != -1; > +return spapr->fwnmi_machine_check_addr != -1; > } > > static int spapr_fwnmi_pre_save(void *opaque) > @@ -2008,7 +2008,7 @@ static int spapr_fwnmi_pre_save(void *opaque) > * Check if machine check handling is in progress and print a > * warning message. > */ > -if (spapr->mc_status != -1) { > +if (spapr->fwnmi_machine_check_interlock != -1) { > warn_report("A machine check is being handled during migration. The" > "handler may run and log hardware error on the destination"); > } > @@ -2016,15 +2016,15 @@ static int spapr_fwnmi_pre_save(void *opaque) > return 0; > } > > -static const VMStateDescription vmstate_spapr_machine_check = { > -.name = "spapr_machine_check", > +static const VMStateDescription vmstate_spapr_fwnmi = { > +.name = "spapr_fwnmi", > .version_id = 1, > .minimum_version_id = 1, > .needed = spapr_fwnmi_needed, > .pre_save = spapr_fwnmi_pre_save, > .fields = (VMStateField[]) { > -VMSTATE_UINT64(guest_machine_check_addr, SpaprMachineState), > -VMSTATE_INT32(mc_status, SpaprMachineState), > +VMSTATE_UINT64(fwnmi_machine_check_addr, SpaprMachineState), > +VMSTATE_INT32(fwnmi_machine_check_interlock, SpaprMachineState), > VMSTATE_END_OF_LIST() > }, > }; > @@ -2063,7 +2063,7 @@ static const VMStateDescription vmstate_spapr = { > &vmstate_spapr_cap_large_decr, > &vmstate_spapr_cap_ccf_assist, > &vmstate_spapr_cap_fwnmi, > -&vmstate_spapr_machine_check, > +&vmstate_spapr_fwnmi, > NULL > } > }; > @@ -2884,7 +2884,7 @@ static void spapr_machine_init(MachineState *machine) > spapr_create_lmb_dr_connectors(spapr); > } > > -if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == SPAPR_CAP_ON) { > +if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI) == SPAPR_CAP_ON) { > /* Create the error string for live migration blocker */ > error_setg(&spapr->fwnmi_migration_blocker, > "A machine check is being handled during migration. The handler" > @@ -3053,7 +3053,7 @@ static void spapr_machine_init(MachineState *machine) > kvmppc_spapr_enable_inkernel_multitce(); > } > > -qemu_cond_init(&spapr->mc_delivery_cond); > +qemu_cond_init(&spapr->fwnmi_machine_check_interlock_cond); > } > > static int spapr_kvm_type(MachineState *machine, const char *vm_type) > @@ -4534,7 +4534,7 @@ static void spapr_machine_class_init(ObjectClass *oc, > void *data) > smc->default_caps.caps[SPAPR_CAP_NESTED_KVM_HV] = SPAPR_CAP_OFF; > smc->default_caps.caps[SPAPR_CAP_LARGE_DECREMENTER] = SPAPR_CAP_ON; > smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_ON; > -smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_ON; > +smc->default_caps.caps[SPAPR_CAP_FWNMI] = SPAPR_CAP_ON; > spapr_caps_add_properties(smc, &error_abort); > smc->irq = &spapr_irq_dual; > smc->dr_phb_enabled = true; > @@ -4612,7 +4612,7 @@ static void > spapr_machine_4_2_class_options(MachineClass *mc) > spapr_machine_5_0_class_options(mc); > compat_props_add(mc->compat_props, hw_compat_4_2, hw_compat_4_2_len); > smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_OFF; > -smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_OFF; > +smc->default_caps.caps[
Re: [PATCH v2 2/8] ppc/spapr: Change FWNMI names
On Tue, 17 Mar 2020 00:26:07 +1000 Nicholas Piggin wrote: > The option is called "FWNMI", and it involves more than just machine > checks, also machine checks can be delivered without the FWNMI option, > so re-name various things to reflect that. > > Signed-off-by: Nicholas Piggin > --- Reviewed-by: Greg Kurz > hw/ppc/spapr.c| 28 ++-- > hw/ppc/spapr_caps.c | 14 +++--- > hw/ppc/spapr_events.c | 14 +++--- > hw/ppc/spapr_rtas.c | 17 + > include/hw/ppc/spapr.h| 27 +-- > tests/qtest/libqos/libqos-spapr.h | 2 +- > 6 files changed, 55 insertions(+), 47 deletions(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index d3db3ec56e..b03b26370d 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1704,11 +1704,11 @@ static void spapr_machine_reset(MachineState *machine) > > spapr->cas_reboot = false; > > -spapr->mc_status = -1; > -spapr->guest_machine_check_addr = -1; > +spapr->fwnmi_machine_check_addr = -1; > +spapr->fwnmi_machine_check_interlock = -1; > > /* Signal all vCPUs waiting on this condition */ > -qemu_cond_broadcast(&spapr->mc_delivery_cond); > +qemu_cond_broadcast(&spapr->fwnmi_machine_check_interlock_cond); > > migrate_del_blocker(spapr->fwnmi_migration_blocker); > } > @@ -1997,7 +1997,7 @@ static bool spapr_fwnmi_needed(void *opaque) > { > SpaprMachineState *spapr = (SpaprMachineState *)opaque; > > -return spapr->guest_machine_check_addr != -1; > +return spapr->fwnmi_machine_check_addr != -1; > } > > static int spapr_fwnmi_pre_save(void *opaque) > @@ -2008,7 +2008,7 @@ static int spapr_fwnmi_pre_save(void *opaque) > * Check if machine check handling is in progress and print a > * warning message. > */ > -if (spapr->mc_status != -1) { > +if (spapr->fwnmi_machine_check_interlock != -1) { > warn_report("A machine check is being handled during migration. The" > "handler may run and log hardware error on the destination"); > } > @@ -2016,15 +2016,15 @@ static int spapr_fwnmi_pre_save(void *opaque) > return 0; > } > > -static const VMStateDescription vmstate_spapr_machine_check = { > -.name = "spapr_machine_check", > +static const VMStateDescription vmstate_spapr_fwnmi = { > +.name = "spapr_fwnmi", > .version_id = 1, > .minimum_version_id = 1, > .needed = spapr_fwnmi_needed, > .pre_save = spapr_fwnmi_pre_save, > .fields = (VMStateField[]) { > -VMSTATE_UINT64(guest_machine_check_addr, SpaprMachineState), > -VMSTATE_INT32(mc_status, SpaprMachineState), > +VMSTATE_UINT64(fwnmi_machine_check_addr, SpaprMachineState), > +VMSTATE_INT32(fwnmi_machine_check_interlock, SpaprMachineState), > VMSTATE_END_OF_LIST() > }, > }; > @@ -2063,7 +2063,7 @@ static const VMStateDescription vmstate_spapr = { > &vmstate_spapr_cap_large_decr, > &vmstate_spapr_cap_ccf_assist, > &vmstate_spapr_cap_fwnmi, > -&vmstate_spapr_machine_check, > +&vmstate_spapr_fwnmi, > NULL > } > }; > @@ -2884,7 +2884,7 @@ static void spapr_machine_init(MachineState *machine) > spapr_create_lmb_dr_connectors(spapr); > } > > -if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == SPAPR_CAP_ON) { > +if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI) == SPAPR_CAP_ON) { > /* Create the error string for live migration blocker */ > error_setg(&spapr->fwnmi_migration_blocker, > "A machine check is being handled during migration. The handler" > @@ -3053,7 +3053,7 @@ static void spapr_machine_init(MachineState *machine) > kvmppc_spapr_enable_inkernel_multitce(); > } > > -qemu_cond_init(&spapr->mc_delivery_cond); > +qemu_cond_init(&spapr->fwnmi_machine_check_interlock_cond); > } > > static int spapr_kvm_type(MachineState *machine, const char *vm_type) > @@ -4534,7 +4534,7 @@ static void spapr_machine_class_init(ObjectClass *oc, > void *data) > smc->default_caps.caps[SPAPR_CAP_NESTED_KVM_HV] = SPAPR_CAP_OFF; > smc->default_caps.caps[SPAPR_CAP_LARGE_DECREMENTER] = SPAPR_CAP_ON; > smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_ON; > -smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_ON; > +smc->default_caps.caps[SPAPR_CAP_FWNMI] = SPAPR_CAP_ON; > spapr_caps_add_properties(smc, &error_abort); > smc->irq = &spapr_irq_dual; > smc->dr_phb_enabled = true; > @@ -4612,7 +4612,7 @@ static void > spapr_machine_4_2_class_options(MachineClass *mc) > spapr_machine_5_0_class_options(mc); > compat_props_add(mc->compat_props, hw_compat_4_2, hw_compat_4_2_len); > smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_OFF; > -smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_OFF; > +smc->default
Re: [PATCH v2 2/8] ppc/spapr: Change FWNMI names
On Mon, 16 Mar 2020 22:48:45 +0530 Mahesh J Salgaonkar wrote: > On 2020-03-17 00:26:07 Tue, Nicholas Piggin wrote: > > The option is called "FWNMI", and it involves more than just machine > > checks, also machine checks can be delivered without the FWNMI option, > > so re-name various things to reflect that. > > > > Signed-off-by: Nicholas Piggin > > --- > > hw/ppc/spapr.c| 28 ++-- > > hw/ppc/spapr_caps.c | 14 +++--- > > hw/ppc/spapr_events.c | 14 +++--- > > hw/ppc/spapr_rtas.c | 17 + > > include/hw/ppc/spapr.h| 27 +-- > > tests/qtest/libqos/libqos-spapr.h | 2 +- > > 6 files changed, 55 insertions(+), 47 deletions(-) > > > [...] > > @@ -626,14 +626,14 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = > > { > > .type = "bool", > > .apply = cap_ccf_assist_apply, > > }, > > -[SPAPR_CAP_FWNMI_MCE] = { > > -.name = "fwnmi-mce", > > -.description = "Handle fwnmi machine check exceptions", > > -.index = SPAPR_CAP_FWNMI_MCE, > > +[SPAPR_CAP_FWNMI] = { > > +.name = "fwnmi", > > I guess this should be fine and should hit QEMU 5.0 release so that we > don't end up with two different CAP names for 5.0 and future releases. > Yeah we really want this patch and the next one (which affects migration) to go to 5.0. > Thanks, > -Mahesh. >