diff mbox series

[v2,2/8] ppc/spapr: Change FWNMI names

Message ID 20200316142613.121089-3-npiggin@gmail.com (mailing list archive)
State New, archived
Headers show
Series FWNMI fixes / changes | expand

Commit Message

Nicholas Piggin March 16, 2020, 2:26 p.m. UTC
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 <npiggin@gmail.com>
---
 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(-)

Comments

Greg Kurz March 16, 2020, 5:15 p.m. UTC | #1
On Tue, 17 Mar 2020 00:26:07 +1000
Nicholas Piggin <npiggin@gmail.com> 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 <npiggin@gmail.com>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  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[SPAPR_CAP_FWNMI] = SPAPR_CAP_OFF;
>      smc->rma_limit = 16 * GiB;
>      mc->nvdimm_supported = false;
>  }
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index 8b27d3ac09..f626d769a0 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -509,7 +509,7 @@ static void cap_ccf_assist_apply(SpaprMachineState *spapr, uint8_t val,
>      }
>  }
>  
> -static void cap_fwnmi_mce_apply(SpaprMachineState *spapr, uint8_t val,
> +static void cap_fwnmi_apply(SpaprMachineState *spapr, uint8_t val,
>                                  Error **errp)
>  {
>      if (!val) {
> @@ -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",
> +        .description = "Implements PAPR FWNMI option",
> +        .index = SPAPR_CAP_FWNMI,
>          .get = spapr_cap_get_bool,
>          .set = spapr_cap_set_bool,
>          .type = "bool",
> -        .apply = cap_fwnmi_mce_apply,
> +        .apply = cap_fwnmi_apply,
>      },
>  };
>  
> @@ -774,7 +774,7 @@ SPAPR_CAP_MIG_STATE(hpt_maxpagesize, SPAPR_CAP_HPT_MAXPAGESIZE);
>  SPAPR_CAP_MIG_STATE(nested_kvm_hv, SPAPR_CAP_NESTED_KVM_HV);
>  SPAPR_CAP_MIG_STATE(large_decr, SPAPR_CAP_LARGE_DECREMENTER);
>  SPAPR_CAP_MIG_STATE(ccf_assist, SPAPR_CAP_CCF_ASSIST);
> -SPAPR_CAP_MIG_STATE(fwnmi, SPAPR_CAP_FWNMI_MCE);
> +SPAPR_CAP_MIG_STATE(fwnmi, SPAPR_CAP_FWNMI);
>  
>  void spapr_caps_init(SpaprMachineState *spapr)
>  {
> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> index 11303258d4..27ba8a2c19 100644
> --- a/hw/ppc/spapr_events.c
> +++ b/hw/ppc/spapr_events.c
> @@ -837,7 +837,7 @@ static void spapr_mce_dispatch_elog(PowerPCCPU *cpu, bool recovered)
>  
>      env->gpr[3] = rtas_addr + RTAS_ERROR_LOG_OFFSET;
>      env->msr = msr;
> -    env->nip = spapr->guest_machine_check_addr;
> +    env->nip = spapr->fwnmi_machine_check_addr;
>  
>      g_free(ext_elog);
>  }
> @@ -849,7 +849,7 @@ void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered)
>      int ret;
>      Error *local_err = NULL;
>  
> -    if (spapr->guest_machine_check_addr == -1) {
> +    if (spapr->fwnmi_machine_check_addr == -1) {
>          /*
>           * This implies that we have hit a machine check either when the
>           * guest has not registered FWNMI (i.e., "ibm,nmi-register" not
> @@ -861,19 +861,19 @@ void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered)
>          return;
>      }
>  
> -    while (spapr->mc_status != -1) {
> +    while (spapr->fwnmi_machine_check_interlock != -1) {
>          /*
>           * Check whether the same CPU got machine check error
>           * while still handling the mc error (i.e., before
>           * that CPU called "ibm,nmi-interlock")
>           */
> -        if (spapr->mc_status == cpu->vcpu_id) {
> +        if (spapr->fwnmi_machine_check_interlock == cpu->vcpu_id) {
>              qemu_system_guest_panicked(NULL);
>              return;
>          }
> -        qemu_cond_wait_iothread(&spapr->mc_delivery_cond);
> +        qemu_cond_wait_iothread(&spapr->fwnmi_machine_check_interlock_cond);
>          /* Meanwhile if the system is reset, then just return */
> -        if (spapr->guest_machine_check_addr == -1) {
> +        if (spapr->fwnmi_machine_check_addr == -1) {
>              return;
>          }
>      }
> @@ -889,7 +889,7 @@ void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered)
>          warn_report("Received a fwnmi while migration was in progress");
>      }
>  
> -    spapr->mc_status = cpu->vcpu_id;
> +    spapr->fwnmi_machine_check_interlock = cpu->vcpu_id;
>      spapr_mce_dispatch_elog(cpu, recovered);
>  }
>  
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index fe83b50c66..0b8c481593 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -415,7 +415,7 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
>  {
>      hwaddr rtas_addr;
>  
> -    if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == SPAPR_CAP_OFF) {
> +    if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI) == SPAPR_CAP_OFF) {
>          rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
>          return;
>      }
> @@ -426,7 +426,8 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
>          return;
>      }
>  
> -    spapr->guest_machine_check_addr = rtas_ld(args, 1);
> +    spapr->fwnmi_machine_check_addr = rtas_ld(args, 1);
> +
>      rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>  }
>  
> @@ -436,18 +437,18 @@ static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
>                                     target_ulong args,
>                                     uint32_t nret, target_ulong rets)
>  {
> -    if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == SPAPR_CAP_OFF) {
> +    if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI) == SPAPR_CAP_OFF) {
>          rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
>          return;
>      }
>  
> -    if (spapr->guest_machine_check_addr == -1) {
> +    if (spapr->fwnmi_machine_check_addr == -1) {
>          /* NMI register not called */
>          rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>          return;
>      }
>  
> -    if (spapr->mc_status != cpu->vcpu_id) {
> +    if (spapr->fwnmi_machine_check_interlock != cpu->vcpu_id) {
>          /* The vCPU that hit the NMI should invoke "ibm,nmi-interlock" */
>          rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>          return;
> @@ -455,10 +456,10 @@ static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
>  
>      /*
>       * vCPU issuing "ibm,nmi-interlock" is done with NMI handling,
> -     * hence unset mc_status.
> +     * hence unset fwnmi_machine_check_interlock.
>       */
> -    spapr->mc_status = -1;
> -    qemu_cond_signal(&spapr->mc_delivery_cond);
> +    spapr->fwnmi_machine_check_interlock = -1;
> +    qemu_cond_signal(&spapr->fwnmi_machine_check_interlock_cond);
>      rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>      migrate_del_blocker(spapr->fwnmi_migration_blocker);
>  }
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 35b489a549..64b83402cb 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -79,10 +79,10 @@ typedef enum {
>  #define SPAPR_CAP_LARGE_DECREMENTER     0x08
>  /* Count Cache Flush Assist HW Instruction */
>  #define SPAPR_CAP_CCF_ASSIST            0x09
> -/* FWNMI machine check handling */
> -#define SPAPR_CAP_FWNMI_MCE             0x0A
> +/* Implements PAPR FWNMI option */
> +#define SPAPR_CAP_FWNMI                 0x0A
>  /* Num Caps */
> -#define SPAPR_CAP_NUM                   (SPAPR_CAP_FWNMI_MCE + 1)
> +#define SPAPR_CAP_NUM                   (SPAPR_CAP_FWNMI + 1)
>  
>  /*
>   * Capability Values
> @@ -192,14 +192,21 @@ struct SpaprMachineState {
>       * occurs during the unplug process. */
>      QTAILQ_HEAD(, SpaprDimmState) pending_dimm_unplugs;
>  
> -    /* State related to "ibm,nmi-register" and "ibm,nmi-interlock" calls */
> -    target_ulong guest_machine_check_addr;
> -    /*
> -     * mc_status is set to -1 if mc is not in progress, else is set to the CPU
> -     * handling the mc.
> +    /* State related to FWNMI option */
> +
> +    /* Machine Check Notification Routine address
> +     * registered by "ibm,nmi-register" RTAS call.
> +     */
> +    target_ulong fwnmi_machine_check_addr;
> +
> +    /* Machine Check FWNMI synchronization, fwnmi_machine_check_interlock is
> +     * set to -1 if a FWNMI machine check is not in progress, else is set to
> +     * the CPU that was delivered the machine check, and is set back to -1
> +     * when that CPU makes an "ibm,nmi-interlock" RTAS call. The cond is used
> +     * to synchronize other CPUs.
>       */
> -    int mc_status;
> -    QemuCond mc_delivery_cond;
> +    int fwnmi_machine_check_interlock;
> +    QemuCond fwnmi_machine_check_interlock_cond;
>  
>      /*< public >*/
>      char *kvm_type;
> diff --git a/tests/qtest/libqos/libqos-spapr.h b/tests/qtest/libqos/libqos-spapr.h
> index d9c4c22343..16174dbada 100644
> --- a/tests/qtest/libqos/libqos-spapr.h
> +++ b/tests/qtest/libqos/libqos-spapr.h
> @@ -13,6 +13,6 @@ void qtest_spapr_shutdown(QOSState *qs);
>      "cap-sbbc=broken,"                           \
>      "cap-ibs=broken,"                            \
>      "cap-ccf-assist=off,"                        \
> -    "cap-fwnmi-mce=off"
> +    "cap-fwnmi=off"
>  
>  #endif
Mahesh J Salgaonkar March 16, 2020, 5:18 p.m. UTC | #2
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 <npiggin@gmail.com>
> ---
>  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.
Greg Kurz March 16, 2020, 5:25 p.m. UTC | #3
On Mon, 16 Mar 2020 22:48:45 +0530
Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com> 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 <npiggin@gmail.com>
> > ---
> >  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.
>
Cédric Le Goater March 16, 2020, 5:32 p.m. UTC | #4
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 <clg@kaod.org>


> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  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[SPAPR_CAP_FWNMI] = SPAPR_CAP_OFF;
>      smc->rma_limit = 16 * GiB;
>      mc->nvdimm_supported = false;
>  }
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index 8b27d3ac09..f626d769a0 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -509,7 +509,7 @@ static void cap_ccf_assist_apply(SpaprMachineState *spapr, uint8_t val,
>      }
>  }
>  
> -static void cap_fwnmi_mce_apply(SpaprMachineState *spapr, uint8_t val,
> +static void cap_fwnmi_apply(SpaprMachineState *spapr, uint8_t val,
>                                  Error **errp)
>  {
>      if (!val) {
> @@ -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",
> +        .description = "Implements PAPR FWNMI option",
> +        .index = SPAPR_CAP_FWNMI,
>          .get = spapr_cap_get_bool,
>          .set = spapr_cap_set_bool,
>          .type = "bool",
> -        .apply = cap_fwnmi_mce_apply,
> +        .apply = cap_fwnmi_apply,
>      },
>  };
>  
> @@ -774,7 +774,7 @@ SPAPR_CAP_MIG_STATE(hpt_maxpagesize, SPAPR_CAP_HPT_MAXPAGESIZE);
>  SPAPR_CAP_MIG_STATE(nested_kvm_hv, SPAPR_CAP_NESTED_KVM_HV);
>  SPAPR_CAP_MIG_STATE(large_decr, SPAPR_CAP_LARGE_DECREMENTER);
>  SPAPR_CAP_MIG_STATE(ccf_assist, SPAPR_CAP_CCF_ASSIST);
> -SPAPR_CAP_MIG_STATE(fwnmi, SPAPR_CAP_FWNMI_MCE);
> +SPAPR_CAP_MIG_STATE(fwnmi, SPAPR_CAP_FWNMI);
>  
>  void spapr_caps_init(SpaprMachineState *spapr)
>  {
> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> index 11303258d4..27ba8a2c19 100644
> --- a/hw/ppc/spapr_events.c
> +++ b/hw/ppc/spapr_events.c
> @@ -837,7 +837,7 @@ static void spapr_mce_dispatch_elog(PowerPCCPU *cpu, bool recovered)
>  
>      env->gpr[3] = rtas_addr + RTAS_ERROR_LOG_OFFSET;
>      env->msr = msr;
> -    env->nip = spapr->guest_machine_check_addr;
> +    env->nip = spapr->fwnmi_machine_check_addr;
>  
>      g_free(ext_elog);
>  }
> @@ -849,7 +849,7 @@ void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered)
>      int ret;
>      Error *local_err = NULL;
>  
> -    if (spapr->guest_machine_check_addr == -1) {
> +    if (spapr->fwnmi_machine_check_addr == -1) {
>          /*
>           * This implies that we have hit a machine check either when the
>           * guest has not registered FWNMI (i.e., "ibm,nmi-register" not
> @@ -861,19 +861,19 @@ void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered)
>          return;
>      }
>  
> -    while (spapr->mc_status != -1) {
> +    while (spapr->fwnmi_machine_check_interlock != -1) {
>          /*
>           * Check whether the same CPU got machine check error
>           * while still handling the mc error (i.e., before
>           * that CPU called "ibm,nmi-interlock")
>           */
> -        if (spapr->mc_status == cpu->vcpu_id) {
> +        if (spapr->fwnmi_machine_check_interlock == cpu->vcpu_id) {
>              qemu_system_guest_panicked(NULL);
>              return;
>          }
> -        qemu_cond_wait_iothread(&spapr->mc_delivery_cond);
> +        qemu_cond_wait_iothread(&spapr->fwnmi_machine_check_interlock_cond);
>          /* Meanwhile if the system is reset, then just return */
> -        if (spapr->guest_machine_check_addr == -1) {
> +        if (spapr->fwnmi_machine_check_addr == -1) {
>              return;
>          }
>      }
> @@ -889,7 +889,7 @@ void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered)
>          warn_report("Received a fwnmi while migration was in progress");
>      }
>  
> -    spapr->mc_status = cpu->vcpu_id;
> +    spapr->fwnmi_machine_check_interlock = cpu->vcpu_id;
>      spapr_mce_dispatch_elog(cpu, recovered);
>  }
>  
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index fe83b50c66..0b8c481593 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -415,7 +415,7 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
>  {
>      hwaddr rtas_addr;
>  
> -    if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == SPAPR_CAP_OFF) {
> +    if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI) == SPAPR_CAP_OFF) {
>          rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
>          return;
>      }
> @@ -426,7 +426,8 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
>          return;
>      }
>  
> -    spapr->guest_machine_check_addr = rtas_ld(args, 1);
> +    spapr->fwnmi_machine_check_addr = rtas_ld(args, 1);
> +
>      rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>  }
>  
> @@ -436,18 +437,18 @@ static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
>                                     target_ulong args,
>                                     uint32_t nret, target_ulong rets)
>  {
> -    if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == SPAPR_CAP_OFF) {
> +    if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI) == SPAPR_CAP_OFF) {
>          rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
>          return;
>      }
>  
> -    if (spapr->guest_machine_check_addr == -1) {
> +    if (spapr->fwnmi_machine_check_addr == -1) {
>          /* NMI register not called */
>          rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>          return;
>      }
>  
> -    if (spapr->mc_status != cpu->vcpu_id) {
> +    if (spapr->fwnmi_machine_check_interlock != cpu->vcpu_id) {
>          /* The vCPU that hit the NMI should invoke "ibm,nmi-interlock" */
>          rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>          return;
> @@ -455,10 +456,10 @@ static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
>  
>      /*
>       * vCPU issuing "ibm,nmi-interlock" is done with NMI handling,
> -     * hence unset mc_status.
> +     * hence unset fwnmi_machine_check_interlock.
>       */
> -    spapr->mc_status = -1;
> -    qemu_cond_signal(&spapr->mc_delivery_cond);
> +    spapr->fwnmi_machine_check_interlock = -1;
> +    qemu_cond_signal(&spapr->fwnmi_machine_check_interlock_cond);
>      rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>      migrate_del_blocker(spapr->fwnmi_migration_blocker);
>  }
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 35b489a549..64b83402cb 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -79,10 +79,10 @@ typedef enum {
>  #define SPAPR_CAP_LARGE_DECREMENTER     0x08
>  /* Count Cache Flush Assist HW Instruction */
>  #define SPAPR_CAP_CCF_ASSIST            0x09
> -/* FWNMI machine check handling */
> -#define SPAPR_CAP_FWNMI_MCE             0x0A
> +/* Implements PAPR FWNMI option */
> +#define SPAPR_CAP_FWNMI                 0x0A
>  /* Num Caps */
> -#define SPAPR_CAP_NUM                   (SPAPR_CAP_FWNMI_MCE + 1)
> +#define SPAPR_CAP_NUM                   (SPAPR_CAP_FWNMI + 1)
>  
>  /*
>   * Capability Values
> @@ -192,14 +192,21 @@ struct SpaprMachineState {
>       * occurs during the unplug process. */
>      QTAILQ_HEAD(, SpaprDimmState) pending_dimm_unplugs;
>  
> -    /* State related to "ibm,nmi-register" and "ibm,nmi-interlock" calls */
> -    target_ulong guest_machine_check_addr;
> -    /*
> -     * mc_status is set to -1 if mc is not in progress, else is set to the CPU
> -     * handling the mc.
> +    /* State related to FWNMI option */
> +
> +    /* Machine Check Notification Routine address
> +     * registered by "ibm,nmi-register" RTAS call.
> +     */
> +    target_ulong fwnmi_machine_check_addr;
> +
> +    /* Machine Check FWNMI synchronization, fwnmi_machine_check_interlock is
> +     * set to -1 if a FWNMI machine check is not in progress, else is set to
> +     * the CPU that was delivered the machine check, and is set back to -1
> +     * when that CPU makes an "ibm,nmi-interlock" RTAS call. The cond is used
> +     * to synchronize other CPUs.
>       */
> -    int mc_status;
> -    QemuCond mc_delivery_cond;
> +    int fwnmi_machine_check_interlock;
> +    QemuCond fwnmi_machine_check_interlock_cond;
>  
>      /*< public >*/
>      char *kvm_type;
> diff --git a/tests/qtest/libqos/libqos-spapr.h b/tests/qtest/libqos/libqos-spapr.h
> index d9c4c22343..16174dbada 100644
> --- a/tests/qtest/libqos/libqos-spapr.h
> +++ b/tests/qtest/libqos/libqos-spapr.h
> @@ -13,6 +13,6 @@ void qtest_spapr_shutdown(QOSState *qs);
>      "cap-sbbc=broken,"                           \
>      "cap-ibs=broken,"                            \
>      "cap-ccf-assist=off,"                        \
> -    "cap-fwnmi-mce=off"
> +    "cap-fwnmi=off"
>  
>  #endif
>
David Gibson March 16, 2020, 10:46 p.m. UTC | #5
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 <npiggin@gmail.com>

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;
> +    smc->default_caps.caps[SPAPR_CAP_FWNMI] = SPAPR_CAP_OFF;
>      smc->rma_limit = 16 * GiB;
>      mc->nvdimm_supported = false;
>  }
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index 8b27d3ac09..f626d769a0 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -509,7 +509,7 @@ static void cap_ccf_assist_apply(SpaprMachineState *spapr, uint8_t val,
>      }
>  }
>  
> -static void cap_fwnmi_mce_apply(SpaprMachineState *spapr, uint8_t val,
> +static void cap_fwnmi_apply(SpaprMachineState *spapr, uint8_t val,
>                                  Error **errp)
>  {
>      if (!val) {
> @@ -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",
> +        .description = "Implements PAPR FWNMI option",
> +        .index = SPAPR_CAP_FWNMI,
>          .get = spapr_cap_get_bool,
>          .set = spapr_cap_set_bool,
>          .type = "bool",
> -        .apply = cap_fwnmi_mce_apply,
> +        .apply = cap_fwnmi_apply,
>      },
>  };
>  
> @@ -774,7 +774,7 @@ SPAPR_CAP_MIG_STATE(hpt_maxpagesize, SPAPR_CAP_HPT_MAXPAGESIZE);
>  SPAPR_CAP_MIG_STATE(nested_kvm_hv, SPAPR_CAP_NESTED_KVM_HV);
>  SPAPR_CAP_MIG_STATE(large_decr, SPAPR_CAP_LARGE_DECREMENTER);
>  SPAPR_CAP_MIG_STATE(ccf_assist, SPAPR_CAP_CCF_ASSIST);
> -SPAPR_CAP_MIG_STATE(fwnmi, SPAPR_CAP_FWNMI_MCE);
> +SPAPR_CAP_MIG_STATE(fwnmi, SPAPR_CAP_FWNMI);
>  
>  void spapr_caps_init(SpaprMachineState *spapr)
>  {
> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> index 11303258d4..27ba8a2c19 100644
> --- a/hw/ppc/spapr_events.c
> +++ b/hw/ppc/spapr_events.c
> @@ -837,7 +837,7 @@ static void spapr_mce_dispatch_elog(PowerPCCPU *cpu, bool recovered)
>  
>      env->gpr[3] = rtas_addr + RTAS_ERROR_LOG_OFFSET;
>      env->msr = msr;
> -    env->nip = spapr->guest_machine_check_addr;
> +    env->nip = spapr->fwnmi_machine_check_addr;
>  
>      g_free(ext_elog);
>  }
> @@ -849,7 +849,7 @@ void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered)
>      int ret;
>      Error *local_err = NULL;
>  
> -    if (spapr->guest_machine_check_addr == -1) {
> +    if (spapr->fwnmi_machine_check_addr == -1) {
>          /*
>           * This implies that we have hit a machine check either when the
>           * guest has not registered FWNMI (i.e., "ibm,nmi-register" not
> @@ -861,19 +861,19 @@ void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered)
>          return;
>      }
>  
> -    while (spapr->mc_status != -1) {
> +    while (spapr->fwnmi_machine_check_interlock != -1) {
>          /*
>           * Check whether the same CPU got machine check error
>           * while still handling the mc error (i.e., before
>           * that CPU called "ibm,nmi-interlock")
>           */
> -        if (spapr->mc_status == cpu->vcpu_id) {
> +        if (spapr->fwnmi_machine_check_interlock == cpu->vcpu_id) {
>              qemu_system_guest_panicked(NULL);
>              return;
>          }
> -        qemu_cond_wait_iothread(&spapr->mc_delivery_cond);
> +        qemu_cond_wait_iothread(&spapr->fwnmi_machine_check_interlock_cond);
>          /* Meanwhile if the system is reset, then just return */
> -        if (spapr->guest_machine_check_addr == -1) {
> +        if (spapr->fwnmi_machine_check_addr == -1) {
>              return;
>          }
>      }
> @@ -889,7 +889,7 @@ void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered)
>          warn_report("Received a fwnmi while migration was in progress");
>      }
>  
> -    spapr->mc_status = cpu->vcpu_id;
> +    spapr->fwnmi_machine_check_interlock = cpu->vcpu_id;
>      spapr_mce_dispatch_elog(cpu, recovered);
>  }
>  
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index fe83b50c66..0b8c481593 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -415,7 +415,7 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
>  {
>      hwaddr rtas_addr;
>  
> -    if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == SPAPR_CAP_OFF) {
> +    if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI) == SPAPR_CAP_OFF) {
>          rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
>          return;
>      }
> @@ -426,7 +426,8 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
>          return;
>      }
>  
> -    spapr->guest_machine_check_addr = rtas_ld(args, 1);
> +    spapr->fwnmi_machine_check_addr = rtas_ld(args, 1);
> +
>      rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>  }
>  
> @@ -436,18 +437,18 @@ static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
>                                     target_ulong args,
>                                     uint32_t nret, target_ulong rets)
>  {
> -    if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == SPAPR_CAP_OFF) {
> +    if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI) == SPAPR_CAP_OFF) {
>          rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
>          return;
>      }
>  
> -    if (spapr->guest_machine_check_addr == -1) {
> +    if (spapr->fwnmi_machine_check_addr == -1) {
>          /* NMI register not called */
>          rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>          return;
>      }
>  
> -    if (spapr->mc_status != cpu->vcpu_id) {
> +    if (spapr->fwnmi_machine_check_interlock != cpu->vcpu_id) {
>          /* The vCPU that hit the NMI should invoke "ibm,nmi-interlock" */
>          rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>          return;
> @@ -455,10 +456,10 @@ static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
>  
>      /*
>       * vCPU issuing "ibm,nmi-interlock" is done with NMI handling,
> -     * hence unset mc_status.
> +     * hence unset fwnmi_machine_check_interlock.
>       */
> -    spapr->mc_status = -1;
> -    qemu_cond_signal(&spapr->mc_delivery_cond);
> +    spapr->fwnmi_machine_check_interlock = -1;
> +    qemu_cond_signal(&spapr->fwnmi_machine_check_interlock_cond);
>      rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>      migrate_del_blocker(spapr->fwnmi_migration_blocker);
>  }
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 35b489a549..64b83402cb 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -79,10 +79,10 @@ typedef enum {
>  #define SPAPR_CAP_LARGE_DECREMENTER     0x08
>  /* Count Cache Flush Assist HW Instruction */
>  #define SPAPR_CAP_CCF_ASSIST            0x09
> -/* FWNMI machine check handling */
> -#define SPAPR_CAP_FWNMI_MCE             0x0A
> +/* Implements PAPR FWNMI option */
> +#define SPAPR_CAP_FWNMI                 0x0A
>  /* Num Caps */
> -#define SPAPR_CAP_NUM                   (SPAPR_CAP_FWNMI_MCE + 1)
> +#define SPAPR_CAP_NUM                   (SPAPR_CAP_FWNMI + 1)
>  
>  /*
>   * Capability Values
> @@ -192,14 +192,21 @@ struct SpaprMachineState {
>       * occurs during the unplug process. */
>      QTAILQ_HEAD(, SpaprDimmState) pending_dimm_unplugs;
>  
> -    /* State related to "ibm,nmi-register" and "ibm,nmi-interlock" calls */
> -    target_ulong guest_machine_check_addr;
> -    /*
> -     * mc_status is set to -1 if mc is not in progress, else is set to the CPU
> -     * handling the mc.
> +    /* State related to FWNMI option */
> +
> +    /* Machine Check Notification Routine address
> +     * registered by "ibm,nmi-register" RTAS call.
> +     */
> +    target_ulong fwnmi_machine_check_addr;
> +
> +    /* Machine Check FWNMI synchronization, fwnmi_machine_check_interlock is
> +     * set to -1 if a FWNMI machine check is not in progress, else is set to
> +     * the CPU that was delivered the machine check, and is set back to -1
> +     * when that CPU makes an "ibm,nmi-interlock" RTAS call. The cond is used
> +     * to synchronize other CPUs.
>       */
> -    int mc_status;
> -    QemuCond mc_delivery_cond;
> +    int fwnmi_machine_check_interlock;
> +    QemuCond fwnmi_machine_check_interlock_cond;
>  
>      /*< public >*/
>      char *kvm_type;
> diff --git a/tests/qtest/libqos/libqos-spapr.h b/tests/qtest/libqos/libqos-spapr.h
> index d9c4c22343..16174dbada 100644
> --- a/tests/qtest/libqos/libqos-spapr.h
> +++ b/tests/qtest/libqos/libqos-spapr.h
> @@ -13,6 +13,6 @@ void qtest_spapr_shutdown(QOSState *qs);
>      "cap-sbbc=broken,"                           \
>      "cap-ibs=broken,"                            \
>      "cap-ccf-assist=off,"                        \
> -    "cap-fwnmi-mce=off"
> +    "cap-fwnmi=off"
>  
>  #endif
diff mbox series

Patch

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[SPAPR_CAP_FWNMI] = SPAPR_CAP_OFF;
     smc->rma_limit = 16 * GiB;
     mc->nvdimm_supported = false;
 }
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index 8b27d3ac09..f626d769a0 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -509,7 +509,7 @@  static void cap_ccf_assist_apply(SpaprMachineState *spapr, uint8_t val,
     }
 }
 
-static void cap_fwnmi_mce_apply(SpaprMachineState *spapr, uint8_t val,
+static void cap_fwnmi_apply(SpaprMachineState *spapr, uint8_t val,
                                 Error **errp)
 {
     if (!val) {
@@ -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",
+        .description = "Implements PAPR FWNMI option",
+        .index = SPAPR_CAP_FWNMI,
         .get = spapr_cap_get_bool,
         .set = spapr_cap_set_bool,
         .type = "bool",
-        .apply = cap_fwnmi_mce_apply,
+        .apply = cap_fwnmi_apply,
     },
 };
 
@@ -774,7 +774,7 @@  SPAPR_CAP_MIG_STATE(hpt_maxpagesize, SPAPR_CAP_HPT_MAXPAGESIZE);
 SPAPR_CAP_MIG_STATE(nested_kvm_hv, SPAPR_CAP_NESTED_KVM_HV);
 SPAPR_CAP_MIG_STATE(large_decr, SPAPR_CAP_LARGE_DECREMENTER);
 SPAPR_CAP_MIG_STATE(ccf_assist, SPAPR_CAP_CCF_ASSIST);
-SPAPR_CAP_MIG_STATE(fwnmi, SPAPR_CAP_FWNMI_MCE);
+SPAPR_CAP_MIG_STATE(fwnmi, SPAPR_CAP_FWNMI);
 
 void spapr_caps_init(SpaprMachineState *spapr)
 {
diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index 11303258d4..27ba8a2c19 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -837,7 +837,7 @@  static void spapr_mce_dispatch_elog(PowerPCCPU *cpu, bool recovered)
 
     env->gpr[3] = rtas_addr + RTAS_ERROR_LOG_OFFSET;
     env->msr = msr;
-    env->nip = spapr->guest_machine_check_addr;
+    env->nip = spapr->fwnmi_machine_check_addr;
 
     g_free(ext_elog);
 }
@@ -849,7 +849,7 @@  void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered)
     int ret;
     Error *local_err = NULL;
 
-    if (spapr->guest_machine_check_addr == -1) {
+    if (spapr->fwnmi_machine_check_addr == -1) {
         /*
          * This implies that we have hit a machine check either when the
          * guest has not registered FWNMI (i.e., "ibm,nmi-register" not
@@ -861,19 +861,19 @@  void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered)
         return;
     }
 
-    while (spapr->mc_status != -1) {
+    while (spapr->fwnmi_machine_check_interlock != -1) {
         /*
          * Check whether the same CPU got machine check error
          * while still handling the mc error (i.e., before
          * that CPU called "ibm,nmi-interlock")
          */
-        if (spapr->mc_status == cpu->vcpu_id) {
+        if (spapr->fwnmi_machine_check_interlock == cpu->vcpu_id) {
             qemu_system_guest_panicked(NULL);
             return;
         }
-        qemu_cond_wait_iothread(&spapr->mc_delivery_cond);
+        qemu_cond_wait_iothread(&spapr->fwnmi_machine_check_interlock_cond);
         /* Meanwhile if the system is reset, then just return */
-        if (spapr->guest_machine_check_addr == -1) {
+        if (spapr->fwnmi_machine_check_addr == -1) {
             return;
         }
     }
@@ -889,7 +889,7 @@  void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered)
         warn_report("Received a fwnmi while migration was in progress");
     }
 
-    spapr->mc_status = cpu->vcpu_id;
+    spapr->fwnmi_machine_check_interlock = cpu->vcpu_id;
     spapr_mce_dispatch_elog(cpu, recovered);
 }
 
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index fe83b50c66..0b8c481593 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -415,7 +415,7 @@  static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
 {
     hwaddr rtas_addr;
 
-    if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == SPAPR_CAP_OFF) {
+    if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI) == SPAPR_CAP_OFF) {
         rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
         return;
     }
@@ -426,7 +426,8 @@  static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
         return;
     }
 
-    spapr->guest_machine_check_addr = rtas_ld(args, 1);
+    spapr->fwnmi_machine_check_addr = rtas_ld(args, 1);
+
     rtas_st(rets, 0, RTAS_OUT_SUCCESS);
 }
 
@@ -436,18 +437,18 @@  static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
                                    target_ulong args,
                                    uint32_t nret, target_ulong rets)
 {
-    if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == SPAPR_CAP_OFF) {
+    if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI) == SPAPR_CAP_OFF) {
         rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
         return;
     }
 
-    if (spapr->guest_machine_check_addr == -1) {
+    if (spapr->fwnmi_machine_check_addr == -1) {
         /* NMI register not called */
         rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
         return;
     }
 
-    if (spapr->mc_status != cpu->vcpu_id) {
+    if (spapr->fwnmi_machine_check_interlock != cpu->vcpu_id) {
         /* The vCPU that hit the NMI should invoke "ibm,nmi-interlock" */
         rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
         return;
@@ -455,10 +456,10 @@  static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
 
     /*
      * vCPU issuing "ibm,nmi-interlock" is done with NMI handling,
-     * hence unset mc_status.
+     * hence unset fwnmi_machine_check_interlock.
      */
-    spapr->mc_status = -1;
-    qemu_cond_signal(&spapr->mc_delivery_cond);
+    spapr->fwnmi_machine_check_interlock = -1;
+    qemu_cond_signal(&spapr->fwnmi_machine_check_interlock_cond);
     rtas_st(rets, 0, RTAS_OUT_SUCCESS);
     migrate_del_blocker(spapr->fwnmi_migration_blocker);
 }
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 35b489a549..64b83402cb 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -79,10 +79,10 @@  typedef enum {
 #define SPAPR_CAP_LARGE_DECREMENTER     0x08
 /* Count Cache Flush Assist HW Instruction */
 #define SPAPR_CAP_CCF_ASSIST            0x09
-/* FWNMI machine check handling */
-#define SPAPR_CAP_FWNMI_MCE             0x0A
+/* Implements PAPR FWNMI option */
+#define SPAPR_CAP_FWNMI                 0x0A
 /* Num Caps */
-#define SPAPR_CAP_NUM                   (SPAPR_CAP_FWNMI_MCE + 1)
+#define SPAPR_CAP_NUM                   (SPAPR_CAP_FWNMI + 1)
 
 /*
  * Capability Values
@@ -192,14 +192,21 @@  struct SpaprMachineState {
      * occurs during the unplug process. */
     QTAILQ_HEAD(, SpaprDimmState) pending_dimm_unplugs;
 
-    /* State related to "ibm,nmi-register" and "ibm,nmi-interlock" calls */
-    target_ulong guest_machine_check_addr;
-    /*
-     * mc_status is set to -1 if mc is not in progress, else is set to the CPU
-     * handling the mc.
+    /* State related to FWNMI option */
+
+    /* Machine Check Notification Routine address
+     * registered by "ibm,nmi-register" RTAS call.
+     */
+    target_ulong fwnmi_machine_check_addr;
+
+    /* Machine Check FWNMI synchronization, fwnmi_machine_check_interlock is
+     * set to -1 if a FWNMI machine check is not in progress, else is set to
+     * the CPU that was delivered the machine check, and is set back to -1
+     * when that CPU makes an "ibm,nmi-interlock" RTAS call. The cond is used
+     * to synchronize other CPUs.
      */
-    int mc_status;
-    QemuCond mc_delivery_cond;
+    int fwnmi_machine_check_interlock;
+    QemuCond fwnmi_machine_check_interlock_cond;
 
     /*< public >*/
     char *kvm_type;
diff --git a/tests/qtest/libqos/libqos-spapr.h b/tests/qtest/libqos/libqos-spapr.h
index d9c4c22343..16174dbada 100644
--- a/tests/qtest/libqos/libqos-spapr.h
+++ b/tests/qtest/libqos/libqos-spapr.h
@@ -13,6 +13,6 @@  void qtest_spapr_shutdown(QOSState *qs);
     "cap-sbbc=broken,"                           \
     "cap-ibs=broken,"                            \
     "cap-ccf-assist=off,"                        \
-    "cap-fwnmi-mce=off"
+    "cap-fwnmi=off"
 
 #endif