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(>mc_delivery_cond);
> +qemu_cond_broadcast(>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 = {
> _spapr_cap_large_decr,
> _spapr_cap_ccf_assist,
> _spapr_cap_fwnmi,
> -_spapr_machine_check,
> +_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(>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(>mc_delivery_cond);
> +qemu_cond_init(>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, _abort);
> smc->irq = _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[SPAPR_CAP_FWNMI] = SPAPR_CAP_OFF;
> smc->rma_limit = 16 *
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(>mc_delivery_cond);
> +qemu_cond_broadcast(>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 = {
> _spapr_cap_large_decr,
> _spapr_cap_ccf_assist,
> _spapr_cap_fwnmi,
> -_spapr_machine_check,
> +_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(>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(>mc_delivery_cond);
> +qemu_cond_init(>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, _abort);
> smc->irq = _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[SPAPR_CAP_FWNMI] = SPAPR_CAP_OFF;
> smc->rma_limit = 16 * GiB;
>
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(>mc_delivery_cond);
> +qemu_cond_broadcast(>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 = {
> _spapr_cap_large_decr,
> _spapr_cap_ccf_assist,
> _spapr_cap_fwnmi,
> -_spapr_machine_check,
> +_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(>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(>mc_delivery_cond);
> +qemu_cond_init(>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, _abort);
> smc->irq = _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[SPAPR_CAP_FWNMI] = SPAPR_CAP_OFF;
> smc->rma_limit = 16 * GiB;
>
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.
>
