Enable the KVM capability KVM_CAP_PPC_FWNMI so that
the KVM causes guest exit with NMI as exit reason
when it encounters a machine check exception on the
address belonging to a guest. Without this capability
enabled, KVM redirects machine check exceptions to
guest's 0x200 vector.
This patch also deals with the case when a guest with
the KVM_CAP_PPC_FWNMI capability enabled is attempted
to migrate to a host that does not support this
capability.
Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com>
---
hw/ppc/spapr.c | 1 +
hw/ppc/spapr_caps.c | 24 ++++++++++++++++++++++++
hw/ppc/spapr_rtas.c | 18 ++++++++++++++++++
include/hw/ppc/spapr.h | 4 +++-
target/ppc/kvm.c | 19 +++++++++++++++++++
target/ppc/kvm_ppc.h | 12 ++++++++++++
6 files changed, 77 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index c97f6a6..e8a77636 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4364,6 +4364,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_OFF;
+ smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_ON;
spapr_caps_add_properties(smc, &error_abort);
smc->irq = &spapr_irq_dual;
smc->dr_phb_enabled = true;
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index 31b4661..ef9e612 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -479,6 +479,20 @@ static void cap_ccf_assist_apply(SpaprMachineState *spapr, uint8_t val,
}
}
+static void cap_fwnmi_mce_apply(SpaprMachineState *spapr, uint8_t val,
+ Error **errp)
+{
+ if (!val) {
+ return; /* Disabled by default */
+ }
+
+ if (tcg_enabled()) {
+ error_setg(errp, "No fwnmi support in TCG, try cap-fwnmi-mce=off");
+ } else if (kvm_enabled() && !kvmppc_has_cap_ppc_fwnmi()) {
+ error_setg(errp, "Requested fwnmi capability not support by KVM");
+ }
+}
+
SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
[SPAPR_CAP_HTM] = {
.name = "htm",
@@ -578,6 +592,15 @@ 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,
+ .get = spapr_cap_get_bool,
+ .set = spapr_cap_set_bool,
+ .type = "bool",
+ .apply = cap_fwnmi_mce_apply,
+ },
};
static SpaprCapabilities default_caps_with_cpu(SpaprMachineState *spapr,
@@ -717,6 +740,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);
void spapr_caps_init(SpaprMachineState *spapr)
{
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index e0bdfc8..91a7ab9 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -49,6 +49,7 @@
#include "hw/ppc/fdt.h"
#include "target/ppc/mmu-hash64.h"
#include "target/ppc/mmu-book3s-v3.h"
+#include "kvm_ppc.h"
static void rtas_display_character(PowerPCCPU *cpu, SpaprMachineState *spapr,
uint32_t token, uint32_t nargs,
@@ -358,6 +359,7 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
target_ulong args,
uint32_t nret, target_ulong rets)
{
+ int ret;
hwaddr rtas_addr = spapr_get_rtas_addr();
if (!rtas_addr) {
@@ -365,6 +367,22 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
return;
}
+ if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == 0) {
+ rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
+ return;
+ }
+
+ ret = kvmppc_fwnmi_enable(cpu);
+ if (ret == 1) {
+ rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
+ return;
+ }
+
+ if (ret < 0) {
+ rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
+ return;
+ }
+
spapr->guest_machine_check_addr = rtas_ld(args, 1);
rtas_st(rets, 0, RTAS_OUT_SUCCESS);
}
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index c717ab2..bd75d4b 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -78,8 +78,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
/* Num Caps */
-#define SPAPR_CAP_NUM (SPAPR_CAP_CCF_ASSIST + 1)
+#define SPAPR_CAP_NUM (SPAPR_CAP_FWNMI_MCE + 1)
/*
* Capability Values
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 39f1a73..368ec6e 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -84,6 +84,7 @@ static int cap_ppc_safe_indirect_branch;
static int cap_ppc_count_cache_flush_assist;
static int cap_ppc_nested_kvm_hv;
static int cap_large_decr;
+static int cap_ppc_fwnmi;
static uint32_t debug_inst_opcode;
@@ -152,6 +153,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
kvmppc_get_cpu_characteristics(s);
cap_ppc_nested_kvm_hv = kvm_vm_check_extension(s, KVM_CAP_PPC_NESTED_HV);
cap_large_decr = kvmppc_get_dec_bits();
+ cap_ppc_fwnmi = kvm_check_extension(s, KVM_CAP_PPC_FWNMI);
/*
* Note: setting it to false because there is not such capability
* in KVM at this moment.
@@ -2119,6 +2121,18 @@ void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
}
}
+int kvmppc_fwnmi_enable(PowerPCCPU *cpu)
+{
+ CPUState *cs = CPU(cpu);
+
+ if (!cap_ppc_fwnmi) {
+ return 1;
+ }
+
+ return kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0);
+}
+
+
int kvmppc_smt_threads(void)
{
return cap_ppc_smt ? cap_ppc_smt : 1;
@@ -2419,6 +2433,11 @@ bool kvmppc_has_cap_mmu_hash_v3(void)
return cap_mmu_hash_v3;
}
+bool kvmppc_has_cap_ppc_fwnmi(void)
+{
+ return cap_ppc_fwnmi;
+}
+
static bool kvmppc_power8_host(void)
{
bool ret = false;
diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
index 18693f1..3d9f0b4 100644
--- a/target/ppc/kvm_ppc.h
+++ b/target/ppc/kvm_ppc.h
@@ -27,6 +27,8 @@ void kvmppc_enable_h_page_init(void);
void kvmppc_set_papr(PowerPCCPU *cpu);
int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr);
void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
+int kvmppc_fwnmi_enable(PowerPCCPU *cpu);
+bool kvmppc_has_cap_ppc_fwnmi(void);
int kvmppc_smt_threads(void);
void kvmppc_hint_smt_possible(Error **errp);
int kvmppc_set_smt_threads(int smt);
@@ -160,6 +162,16 @@ static inline void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
{
}
+static inline int kvmppc_fwnmi_enable(PowerPCCPU *cpu)
+{
+ return 1;
+}
+
+static inline bool kvmppc_has_cap_ppc_fwnmi(void)
+{
+ return false;
+}
+
static inline int kvmppc_smt_threads(void)
{
return 1;
On Wed, 29 May 2019 11:10:49 +0530
Aravinda Prasad <aravinda@linux.vnet.ibm.com> wrote:
> Enable the KVM capability KVM_CAP_PPC_FWNMI so that
> the KVM causes guest exit with NMI as exit reason
> when it encounters a machine check exception on the
> address belonging to a guest. Without this capability
> enabled, KVM redirects machine check exceptions to
> guest's 0x200 vector.
>
> This patch also deals with the case when a guest with
> the KVM_CAP_PPC_FWNMI capability enabled is attempted
> to migrate to a host that does not support this
> capability.
>
> Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com>
> ---
As suggested in another mail, it may be worth introducing the sPAPR cap
in its own patch, earlier in the series.
Anyway, I have some comments below.
> hw/ppc/spapr.c | 1 +
> hw/ppc/spapr_caps.c | 24 ++++++++++++++++++++++++
> hw/ppc/spapr_rtas.c | 18 ++++++++++++++++++
> include/hw/ppc/spapr.h | 4 +++-
> target/ppc/kvm.c | 19 +++++++++++++++++++
> target/ppc/kvm_ppc.h | 12 ++++++++++++
> 6 files changed, 77 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index c97f6a6..e8a77636 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -4364,6 +4364,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_OFF;
> + smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_ON;
> spapr_caps_add_properties(smc, &error_abort);
> smc->irq = &spapr_irq_dual;
> smc->dr_phb_enabled = true;
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index 31b4661..ef9e612 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -479,6 +479,20 @@ static void cap_ccf_assist_apply(SpaprMachineState *spapr, uint8_t val,
> }
> }
>
> +static void cap_fwnmi_mce_apply(SpaprMachineState *spapr, uint8_t val,
> + Error **errp)
> +{
> + if (!val) {
> + return; /* Disabled by default */
> + }
> +
> + if (tcg_enabled()) {
> + error_setg(errp, "No fwnmi support in TCG, try cap-fwnmi-mce=off");
Maybe expand "fwnmi" to "Firmware Assisted Non-Maskable Interrupts" ?
> + } else if (kvm_enabled() && !kvmppc_has_cap_ppc_fwnmi()) {
> + error_setg(errp, "Requested fwnmi capability not support by KVM");
Maybe reword and add a hint:
"KVM implementation does not support Firmware Assisted Non-Maskable Interrupts, try cap-fwnmi-mce=off"
> + }
> +}
> +
> SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> [SPAPR_CAP_HTM] = {
> .name = "htm",
> @@ -578,6 +592,15 @@ 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,
> + .get = spapr_cap_get_bool,
> + .set = spapr_cap_set_bool,
> + .type = "bool",
> + .apply = cap_fwnmi_mce_apply,
> + },
> };
>
> static SpaprCapabilities default_caps_with_cpu(SpaprMachineState *spapr,
> @@ -717,6 +740,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);
>
> void spapr_caps_init(SpaprMachineState *spapr)
> {
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index e0bdfc8..91a7ab9 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -49,6 +49,7 @@
> #include "hw/ppc/fdt.h"
> #include "target/ppc/mmu-hash64.h"
> #include "target/ppc/mmu-book3s-v3.h"
> +#include "kvm_ppc.h"
>
> static void rtas_display_character(PowerPCCPU *cpu, SpaprMachineState *spapr,
> uint32_t token, uint32_t nargs,
> @@ -358,6 +359,7 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
> target_ulong args,
> uint32_t nret, target_ulong rets)
> {
> + int ret;
> hwaddr rtas_addr = spapr_get_rtas_addr();
>
> if (!rtas_addr) {
> @@ -365,6 +367,22 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
> return;
> }
>
> + if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == 0) {
> + rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
> + return;
> + }
> +
> + ret = kvmppc_fwnmi_enable(cpu);
> + if (ret == 1) {
I have the impression that this should really not happen,
otherwise something has gone terribly wrong in QEMU or
in KVM... this maybe deserves an error message as well ?
No big deal.
> + rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
> + return;
> + }
> +
> + if (ret < 0) {
> + rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
> + return;
> + }
> +
> spapr->guest_machine_check_addr = rtas_ld(args, 1);
> rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> }
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index c717ab2..bd75d4b 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -78,8 +78,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
> /* Num Caps */
> -#define SPAPR_CAP_NUM (SPAPR_CAP_CCF_ASSIST + 1)
> +#define SPAPR_CAP_NUM (SPAPR_CAP_FWNMI_MCE + 1)
>
> /*
> * Capability Values
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 39f1a73..368ec6e 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -84,6 +84,7 @@ static int cap_ppc_safe_indirect_branch;
> static int cap_ppc_count_cache_flush_assist;
> static int cap_ppc_nested_kvm_hv;
> static int cap_large_decr;
> +static int cap_ppc_fwnmi;
>
> static uint32_t debug_inst_opcode;
>
> @@ -152,6 +153,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
> kvmppc_get_cpu_characteristics(s);
> cap_ppc_nested_kvm_hv = kvm_vm_check_extension(s, KVM_CAP_PPC_NESTED_HV);
> cap_large_decr = kvmppc_get_dec_bits();
> + cap_ppc_fwnmi = kvm_check_extension(s, KVM_CAP_PPC_FWNMI);
> /*
> * Note: setting it to false because there is not such capability
> * in KVM at this moment.
> @@ -2119,6 +2121,18 @@ void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
> }
> }
>
> +int kvmppc_fwnmi_enable(PowerPCCPU *cpu)
> +{
> + CPUState *cs = CPU(cpu);
> +
> + if (!cap_ppc_fwnmi) {
> + return 1;
> + }
> +
> + return kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0);
> +}
> +
> +
> int kvmppc_smt_threads(void)
> {
> return cap_ppc_smt ? cap_ppc_smt : 1;
> @@ -2419,6 +2433,11 @@ bool kvmppc_has_cap_mmu_hash_v3(void)
> return cap_mmu_hash_v3;
> }
>
> +bool kvmppc_has_cap_ppc_fwnmi(void)
> +{
> + return cap_ppc_fwnmi;
> +}
> +
> static bool kvmppc_power8_host(void)
> {
> bool ret = false;
> diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
> index 18693f1..3d9f0b4 100644
> --- a/target/ppc/kvm_ppc.h
> +++ b/target/ppc/kvm_ppc.h
> @@ -27,6 +27,8 @@ void kvmppc_enable_h_page_init(void);
> void kvmppc_set_papr(PowerPCCPU *cpu);
> int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr);
> void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
> +int kvmppc_fwnmi_enable(PowerPCCPU *cpu);
> +bool kvmppc_has_cap_ppc_fwnmi(void);
> int kvmppc_smt_threads(void);
> void kvmppc_hint_smt_possible(Error **errp);
> int kvmppc_set_smt_threads(int smt);
> @@ -160,6 +162,16 @@ static inline void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
> {
> }
>
> +static inline int kvmppc_fwnmi_enable(PowerPCCPU *cpu)
> +{
> + return 1;
> +}
> +
> +static inline bool kvmppc_has_cap_ppc_fwnmi(void)
> +{
> + return false;
> +}
> +
> static inline int kvmppc_smt_threads(void)
> {
> return 1;
>
>
On Monday 03 June 2019 08:55 PM, Greg Kurz wrote:
> On Wed, 29 May 2019 11:10:49 +0530
> Aravinda Prasad <aravinda@linux.vnet.ibm.com> wrote:
>
>> Enable the KVM capability KVM_CAP_PPC_FWNMI so that
>> the KVM causes guest exit with NMI as exit reason
>> when it encounters a machine check exception on the
>> address belonging to a guest. Without this capability
>> enabled, KVM redirects machine check exceptions to
>> guest's 0x200 vector.
>>
>> This patch also deals with the case when a guest with
>> the KVM_CAP_PPC_FWNMI capability enabled is attempted
>> to migrate to a host that does not support this
>> capability.
>>
>> Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com>
>> ---
>
> As suggested in another mail, it may be worth introducing the sPAPR cap
> in its own patch, earlier in the series.
Sure, also as a workaround mentioned in the reply to that mail, I am
thinking of returning RTAS_OUT_NOT_SUPPORTED to rtas nmi register call
until the entire functionality is implemented. This will help solve
spapr cap issue as well.
>
> Anyway, I have some comments below.
>
>> hw/ppc/spapr.c | 1 +
>> hw/ppc/spapr_caps.c | 24 ++++++++++++++++++++++++
>> hw/ppc/spapr_rtas.c | 18 ++++++++++++++++++
>> include/hw/ppc/spapr.h | 4 +++-
>> target/ppc/kvm.c | 19 +++++++++++++++++++
>> target/ppc/kvm_ppc.h | 12 ++++++++++++
>> 6 files changed, 77 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index c97f6a6..e8a77636 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -4364,6 +4364,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_OFF;
>> + smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_ON;
>> spapr_caps_add_properties(smc, &error_abort);
>> smc->irq = &spapr_irq_dual;
>> smc->dr_phb_enabled = true;
>> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
>> index 31b4661..ef9e612 100644
>> --- a/hw/ppc/spapr_caps.c
>> +++ b/hw/ppc/spapr_caps.c
>> @@ -479,6 +479,20 @@ static void cap_ccf_assist_apply(SpaprMachineState *spapr, uint8_t val,
>> }
>> }
>>
>> +static void cap_fwnmi_mce_apply(SpaprMachineState *spapr, uint8_t val,
>> + Error **errp)
>> +{
>> + if (!val) {
>> + return; /* Disabled by default */
>> + }
>> +
>> + if (tcg_enabled()) {
>> + error_setg(errp, "No fwnmi support in TCG, try cap-fwnmi-mce=off");
>
> Maybe expand "fwnmi" to "Firmware Assisted Non-Maskable Interrupts" ?
sure..
>
>> + } else if (kvm_enabled() && !kvmppc_has_cap_ppc_fwnmi()) {
>> + error_setg(errp, "Requested fwnmi capability not support by KVM");
>
> Maybe reword and add a hint:
>
> "KVM implementation does not support Firmware Assisted Non-Maskable Interrupts, try cap-fwnmi-mce=off"
sure..
>
>
>> + }
>> +}
>> +
>> SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
>> [SPAPR_CAP_HTM] = {
>> .name = "htm",
>> @@ -578,6 +592,15 @@ 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,
>> + .get = spapr_cap_get_bool,
>> + .set = spapr_cap_set_bool,
>> + .type = "bool",
>> + .apply = cap_fwnmi_mce_apply,
>> + },
>> };
>>
>> static SpaprCapabilities default_caps_with_cpu(SpaprMachineState *spapr,
>> @@ -717,6 +740,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);
>>
>> void spapr_caps_init(SpaprMachineState *spapr)
>> {
>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>> index e0bdfc8..91a7ab9 100644
>> --- a/hw/ppc/spapr_rtas.c
>> +++ b/hw/ppc/spapr_rtas.c
>> @@ -49,6 +49,7 @@
>> #include "hw/ppc/fdt.h"
>> #include "target/ppc/mmu-hash64.h"
>> #include "target/ppc/mmu-book3s-v3.h"
>> +#include "kvm_ppc.h"
>>
>> static void rtas_display_character(PowerPCCPU *cpu, SpaprMachineState *spapr,
>> uint32_t token, uint32_t nargs,
>> @@ -358,6 +359,7 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
>> target_ulong args,
>> uint32_t nret, target_ulong rets)
>> {
>> + int ret;
>> hwaddr rtas_addr = spapr_get_rtas_addr();
>>
>> if (!rtas_addr) {
>> @@ -365,6 +367,22 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
>> return;
>> }
>>
>> + if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == 0) {
>> + rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
>> + return;
>> + }
>> +
>> + ret = kvmppc_fwnmi_enable(cpu);
>> + if (ret == 1) {
>
> I have the impression that this should really not happen,
> otherwise something has gone terribly wrong in QEMU or
> in KVM... this maybe deserves an error message as well ?
> No big deal.
I think so.. will add an error message.
Also I should check for non zero return value, not just ret == 1, as
kvmppc_fwnmi_enable() returns the error value from ioctl().
>
>> + rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
>> + return;
>> + }
>> +
>> + if (ret < 0) {
>> + rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
>> + return;
>> + }
>> +
>> spapr->guest_machine_check_addr = rtas_ld(args, 1);
>> rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>> }
>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>> index c717ab2..bd75d4b 100644
>> --- a/include/hw/ppc/spapr.h
>> +++ b/include/hw/ppc/spapr.h
>> @@ -78,8 +78,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
>> /* Num Caps */
>> -#define SPAPR_CAP_NUM (SPAPR_CAP_CCF_ASSIST + 1)
>> +#define SPAPR_CAP_NUM (SPAPR_CAP_FWNMI_MCE + 1)
>>
>> /*
>> * Capability Values
>> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
>> index 39f1a73..368ec6e 100644
>> --- a/target/ppc/kvm.c
>> +++ b/target/ppc/kvm.c
>> @@ -84,6 +84,7 @@ static int cap_ppc_safe_indirect_branch;
>> static int cap_ppc_count_cache_flush_assist;
>> static int cap_ppc_nested_kvm_hv;
>> static int cap_large_decr;
>> +static int cap_ppc_fwnmi;
>>
>> static uint32_t debug_inst_opcode;
>>
>> @@ -152,6 +153,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>> kvmppc_get_cpu_characteristics(s);
>> cap_ppc_nested_kvm_hv = kvm_vm_check_extension(s, KVM_CAP_PPC_NESTED_HV);
>> cap_large_decr = kvmppc_get_dec_bits();
>> + cap_ppc_fwnmi = kvm_check_extension(s, KVM_CAP_PPC_FWNMI);
>> /*
>> * Note: setting it to false because there is not such capability
>> * in KVM at this moment.
>> @@ -2119,6 +2121,18 @@ void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
>> }
>> }
>>
>> +int kvmppc_fwnmi_enable(PowerPCCPU *cpu)
>> +{
>> + CPUState *cs = CPU(cpu);
>> +
>> + if (!cap_ppc_fwnmi) {
>> + return 1;
>> + }
>> +
>> + return kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0);
>> +}
>> +
>> +
>> int kvmppc_smt_threads(void)
>> {
>> return cap_ppc_smt ? cap_ppc_smt : 1;
>> @@ -2419,6 +2433,11 @@ bool kvmppc_has_cap_mmu_hash_v3(void)
>> return cap_mmu_hash_v3;
>> }
>>
>> +bool kvmppc_has_cap_ppc_fwnmi(void)
>> +{
>> + return cap_ppc_fwnmi;
>> +}
>> +
>> static bool kvmppc_power8_host(void)
>> {
>> bool ret = false;
>> diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
>> index 18693f1..3d9f0b4 100644
>> --- a/target/ppc/kvm_ppc.h
>> +++ b/target/ppc/kvm_ppc.h
>> @@ -27,6 +27,8 @@ void kvmppc_enable_h_page_init(void);
>> void kvmppc_set_papr(PowerPCCPU *cpu);
>> int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr);
>> void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
>> +int kvmppc_fwnmi_enable(PowerPCCPU *cpu);
>> +bool kvmppc_has_cap_ppc_fwnmi(void);
>> int kvmppc_smt_threads(void);
>> void kvmppc_hint_smt_possible(Error **errp);
>> int kvmppc_set_smt_threads(int smt);
>> @@ -160,6 +162,16 @@ static inline void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
>> {
>> }
>>
>> +static inline int kvmppc_fwnmi_enable(PowerPCCPU *cpu)
>> +{
>> + return 1;
>> +}
>> +
>> +static inline bool kvmppc_has_cap_ppc_fwnmi(void)
>> +{
>> + return false;
>> +}
>> +
>> static inline int kvmppc_smt_threads(void)
>> {
>> return 1;
>>
>>
>
>
--
Regards,
Aravinda
On Tue, Jun 04, 2019 at 12:15:26PM +0530, Aravinda Prasad wrote:
>
>
> On Monday 03 June 2019 08:55 PM, Greg Kurz wrote:
> > On Wed, 29 May 2019 11:10:49 +0530
> > Aravinda Prasad <aravinda@linux.vnet.ibm.com> wrote:
> >
> >> Enable the KVM capability KVM_CAP_PPC_FWNMI so that
> >> the KVM causes guest exit with NMI as exit reason
> >> when it encounters a machine check exception on the
> >> address belonging to a guest. Without this capability
> >> enabled, KVM redirects machine check exceptions to
> >> guest's 0x200 vector.
> >>
> >> This patch also deals with the case when a guest with
> >> the KVM_CAP_PPC_FWNMI capability enabled is attempted
> >> to migrate to a host that does not support this
> >> capability.
> >>
> >> Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com>
> >> ---
> >
> > As suggested in another mail, it may be worth introducing the sPAPR cap
> > in its own patch, earlier in the series.
>
> Sure, also as a workaround mentioned in the reply to that mail, I am
> thinking of returning RTAS_OUT_NOT_SUPPORTED to rtas nmi register call
> until the entire functionality is implemented. This will help solve
> spapr cap issue as well.
Not registering the RTAS call at all is the correct way to handle that
case.
>
> >
> > Anyway, I have some comments below.
> >
> >> hw/ppc/spapr.c | 1 +
> >> hw/ppc/spapr_caps.c | 24 ++++++++++++++++++++++++
> >> hw/ppc/spapr_rtas.c | 18 ++++++++++++++++++
> >> include/hw/ppc/spapr.h | 4 +++-
> >> target/ppc/kvm.c | 19 +++++++++++++++++++
> >> target/ppc/kvm_ppc.h | 12 ++++++++++++
> >> 6 files changed, 77 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >> index c97f6a6..e8a77636 100644
> >> --- a/hw/ppc/spapr.c
> >> +++ b/hw/ppc/spapr.c
> >> @@ -4364,6 +4364,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_OFF;
> >> + smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_ON;
> >> spapr_caps_add_properties(smc, &error_abort);
> >> smc->irq = &spapr_irq_dual;
> >> smc->dr_phb_enabled = true;
> >> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> >> index 31b4661..ef9e612 100644
> >> --- a/hw/ppc/spapr_caps.c
> >> +++ b/hw/ppc/spapr_caps.c
> >> @@ -479,6 +479,20 @@ static void cap_ccf_assist_apply(SpaprMachineState *spapr, uint8_t val,
> >> }
> >> }
> >>
> >> +static void cap_fwnmi_mce_apply(SpaprMachineState *spapr, uint8_t val,
> >> + Error **errp)
> >> +{
> >> + if (!val) {
> >> + return; /* Disabled by default */
> >> + }
> >> +
> >> + if (tcg_enabled()) {
> >> + error_setg(errp, "No fwnmi support in TCG, try cap-fwnmi-mce=off");
> >
> > Maybe expand "fwnmi" to "Firmware Assisted Non-Maskable Interrupts" ?
>
> sure..
>
> >
> >> + } else if (kvm_enabled() && !kvmppc_has_cap_ppc_fwnmi()) {
> >> + error_setg(errp, "Requested fwnmi capability not support by KVM");
> >
> > Maybe reword and add a hint:
> >
> > "KVM implementation does not support Firmware Assisted Non-Maskable Interrupts, try cap-fwnmi-mce=off"
>
> sure..
>
> >
> >
> >> + }
> >> +}
> >> +
> >> SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> >> [SPAPR_CAP_HTM] = {
> >> .name = "htm",
> >> @@ -578,6 +592,15 @@ 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,
> >> + .get = spapr_cap_get_bool,
> >> + .set = spapr_cap_set_bool,
> >> + .type = "bool",
> >> + .apply = cap_fwnmi_mce_apply,
> >> + },
> >> };
> >>
> >> static SpaprCapabilities default_caps_with_cpu(SpaprMachineState *spapr,
> >> @@ -717,6 +740,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);
> >>
> >> void spapr_caps_init(SpaprMachineState *spapr)
> >> {
> >> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> >> index e0bdfc8..91a7ab9 100644
> >> --- a/hw/ppc/spapr_rtas.c
> >> +++ b/hw/ppc/spapr_rtas.c
> >> @@ -49,6 +49,7 @@
> >> #include "hw/ppc/fdt.h"
> >> #include "target/ppc/mmu-hash64.h"
> >> #include "target/ppc/mmu-book3s-v3.h"
> >> +#include "kvm_ppc.h"
> >>
> >> static void rtas_display_character(PowerPCCPU *cpu, SpaprMachineState *spapr,
> >> uint32_t token, uint32_t nargs,
> >> @@ -358,6 +359,7 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
> >> target_ulong args,
> >> uint32_t nret, target_ulong rets)
> >> {
> >> + int ret;
> >> hwaddr rtas_addr = spapr_get_rtas_addr();
> >>
> >> if (!rtas_addr) {
> >> @@ -365,6 +367,22 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
> >> return;
> >> }
> >>
> >> + if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == 0) {
> >> + rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
> >> + return;
> >> + }
> >> +
> >> + ret = kvmppc_fwnmi_enable(cpu);
> >> + if (ret == 1) {
> >
> > I have the impression that this should really not happen,
> > otherwise something has gone terribly wrong in QEMU or
> > in KVM... this maybe deserves an error message as well ?
> > No big deal.
>
> I think so.. will add an error message.
Right, and because this is essentially a host side error,
RTAS_OUT_HW_ERROR would be more appropriate.
>
> Also I should check for non zero return value, not just ret == 1, as
> kvmppc_fwnmi_enable() returns the error value from ioctl().
>
>
> >
> >> + rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
> >> + return;
> >> + }
> >> +
> >> + if (ret < 0) {
> >> + rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
> >> + return;
> >> + }
> >> +
> >> spapr->guest_machine_check_addr = rtas_ld(args, 1);
> >> rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> >> }
> >> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> >> index c717ab2..bd75d4b 100644
> >> --- a/include/hw/ppc/spapr.h
> >> +++ b/include/hw/ppc/spapr.h
> >> @@ -78,8 +78,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
> >> /* Num Caps */
> >> -#define SPAPR_CAP_NUM (SPAPR_CAP_CCF_ASSIST + 1)
> >> +#define SPAPR_CAP_NUM (SPAPR_CAP_FWNMI_MCE + 1)
> >>
> >> /*
> >> * Capability Values
> >> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> >> index 39f1a73..368ec6e 100644
> >> --- a/target/ppc/kvm.c
> >> +++ b/target/ppc/kvm.c
> >> @@ -84,6 +84,7 @@ static int cap_ppc_safe_indirect_branch;
> >> static int cap_ppc_count_cache_flush_assist;
> >> static int cap_ppc_nested_kvm_hv;
> >> static int cap_large_decr;
> >> +static int cap_ppc_fwnmi;
> >>
> >> static uint32_t debug_inst_opcode;
> >>
> >> @@ -152,6 +153,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
> >> kvmppc_get_cpu_characteristics(s);
> >> cap_ppc_nested_kvm_hv = kvm_vm_check_extension(s, KVM_CAP_PPC_NESTED_HV);
> >> cap_large_decr = kvmppc_get_dec_bits();
> >> + cap_ppc_fwnmi = kvm_check_extension(s, KVM_CAP_PPC_FWNMI);
> >> /*
> >> * Note: setting it to false because there is not such capability
> >> * in KVM at this moment.
> >> @@ -2119,6 +2121,18 @@ void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
> >> }
> >> }
> >>
> >> +int kvmppc_fwnmi_enable(PowerPCCPU *cpu)
> >> +{
> >> + CPUState *cs = CPU(cpu);
> >> +
> >> + if (!cap_ppc_fwnmi) {
> >> + return 1;
> >> + }
> >> +
> >> + return kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0);
> >> +}
> >> +
> >> +
> >> int kvmppc_smt_threads(void)
> >> {
> >> return cap_ppc_smt ? cap_ppc_smt : 1;
> >> @@ -2419,6 +2433,11 @@ bool kvmppc_has_cap_mmu_hash_v3(void)
> >> return cap_mmu_hash_v3;
> >> }
> >>
> >> +bool kvmppc_has_cap_ppc_fwnmi(void)
> >> +{
> >> + return cap_ppc_fwnmi;
> >> +}
> >> +
> >> static bool kvmppc_power8_host(void)
> >> {
> >> bool ret = false;
> >> diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
> >> index 18693f1..3d9f0b4 100644
> >> --- a/target/ppc/kvm_ppc.h
> >> +++ b/target/ppc/kvm_ppc.h
> >> @@ -27,6 +27,8 @@ void kvmppc_enable_h_page_init(void);
> >> void kvmppc_set_papr(PowerPCCPU *cpu);
> >> int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr);
> >> void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
> >> +int kvmppc_fwnmi_enable(PowerPCCPU *cpu);
> >> +bool kvmppc_has_cap_ppc_fwnmi(void);
> >> int kvmppc_smt_threads(void);
> >> void kvmppc_hint_smt_possible(Error **errp);
> >> int kvmppc_set_smt_threads(int smt);
> >> @@ -160,6 +162,16 @@ static inline void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
> >> {
> >> }
> >>
> >> +static inline int kvmppc_fwnmi_enable(PowerPCCPU *cpu)
> >> +{
> >> + return 1;
> >> +}
> >> +
> >> +static inline bool kvmppc_has_cap_ppc_fwnmi(void)
> >> +{
> >> + return false;
> >> +}
> >> +
> >> static inline int kvmppc_smt_threads(void)
> >> {
> >> return 1;
> >>
> >>
> >
> >
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
On Thursday 06 June 2019 08:32 AM, David Gibson wrote:
> On Tue, Jun 04, 2019 at 12:15:26PM +0530, Aravinda Prasad wrote:
>>
>>
>> On Monday 03 June 2019 08:55 PM, Greg Kurz wrote:
>>> On Wed, 29 May 2019 11:10:49 +0530
>>> Aravinda Prasad <aravinda@linux.vnet.ibm.com> wrote:
>>>
>>>> Enable the KVM capability KVM_CAP_PPC_FWNMI so that
>>>> the KVM causes guest exit with NMI as exit reason
>>>> when it encounters a machine check exception on the
>>>> address belonging to a guest. Without this capability
>>>> enabled, KVM redirects machine check exceptions to
>>>> guest's 0x200 vector.
>>>>
>>>> This patch also deals with the case when a guest with
>>>> the KVM_CAP_PPC_FWNMI capability enabled is attempted
>>>> to migrate to a host that does not support this
>>>> capability.
>>>>
>>>> Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com>
>>>> ---
>>>
>>> As suggested in another mail, it may be worth introducing the sPAPR cap
>>> in its own patch, earlier in the series.
>>
>> Sure, also as a workaround mentioned in the reply to that mail, I am
>> thinking of returning RTAS_OUT_NOT_SUPPORTED to rtas nmi register call
>> until the entire functionality is implemented. This will help solve
>> spapr cap issue as well.
>
> Not registering the RTAS call at all is the correct way to handle that
> case.
ok.
>
>>
>>>
>>> Anyway, I have some comments below.
>>>
>>>> hw/ppc/spapr.c | 1 +
>>>> hw/ppc/spapr_caps.c | 24 ++++++++++++++++++++++++
>>>> hw/ppc/spapr_rtas.c | 18 ++++++++++++++++++
>>>> include/hw/ppc/spapr.h | 4 +++-
>>>> target/ppc/kvm.c | 19 +++++++++++++++++++
>>>> target/ppc/kvm_ppc.h | 12 ++++++++++++
>>>> 6 files changed, 77 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>>> index c97f6a6..e8a77636 100644
>>>> --- a/hw/ppc/spapr.c
>>>> +++ b/hw/ppc/spapr.c
>>>> @@ -4364,6 +4364,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_OFF;
>>>> + smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_ON;
>>>> spapr_caps_add_properties(smc, &error_abort);
>>>> smc->irq = &spapr_irq_dual;
>>>> smc->dr_phb_enabled = true;
>>>> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
>>>> index 31b4661..ef9e612 100644
>>>> --- a/hw/ppc/spapr_caps.c
>>>> +++ b/hw/ppc/spapr_caps.c
>>>> @@ -479,6 +479,20 @@ static void cap_ccf_assist_apply(SpaprMachineState *spapr, uint8_t val,
>>>> }
>>>> }
>>>>
>>>> +static void cap_fwnmi_mce_apply(SpaprMachineState *spapr, uint8_t val,
>>>> + Error **errp)
>>>> +{
>>>> + if (!val) {
>>>> + return; /* Disabled by default */
>>>> + }
>>>> +
>>>> + if (tcg_enabled()) {
>>>> + error_setg(errp, "No fwnmi support in TCG, try cap-fwnmi-mce=off");
>>>
>>> Maybe expand "fwnmi" to "Firmware Assisted Non-Maskable Interrupts" ?
>>
>> sure..
>>
>>>
>>>> + } else if (kvm_enabled() && !kvmppc_has_cap_ppc_fwnmi()) {
>>>> + error_setg(errp, "Requested fwnmi capability not support by KVM");
>>>
>>> Maybe reword and add a hint:
>>>
>>> "KVM implementation does not support Firmware Assisted Non-Maskable Interrupts, try cap-fwnmi-mce=off"
>>
>> sure..
>>
>>>
>>>
>>>> + }
>>>> +}
>>>> +
>>>> SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
>>>> [SPAPR_CAP_HTM] = {
>>>> .name = "htm",
>>>> @@ -578,6 +592,15 @@ 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,
>>>> + .get = spapr_cap_get_bool,
>>>> + .set = spapr_cap_set_bool,
>>>> + .type = "bool",
>>>> + .apply = cap_fwnmi_mce_apply,
>>>> + },
>>>> };
>>>>
>>>> static SpaprCapabilities default_caps_with_cpu(SpaprMachineState *spapr,
>>>> @@ -717,6 +740,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);
>>>>
>>>> void spapr_caps_init(SpaprMachineState *spapr)
>>>> {
>>>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>>>> index e0bdfc8..91a7ab9 100644
>>>> --- a/hw/ppc/spapr_rtas.c
>>>> +++ b/hw/ppc/spapr_rtas.c
>>>> @@ -49,6 +49,7 @@
>>>> #include "hw/ppc/fdt.h"
>>>> #include "target/ppc/mmu-hash64.h"
>>>> #include "target/ppc/mmu-book3s-v3.h"
>>>> +#include "kvm_ppc.h"
>>>>
>>>> static void rtas_display_character(PowerPCCPU *cpu, SpaprMachineState *spapr,
>>>> uint32_t token, uint32_t nargs,
>>>> @@ -358,6 +359,7 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
>>>> target_ulong args,
>>>> uint32_t nret, target_ulong rets)
>>>> {
>>>> + int ret;
>>>> hwaddr rtas_addr = spapr_get_rtas_addr();
>>>>
>>>> if (!rtas_addr) {
>>>> @@ -365,6 +367,22 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
>>>> return;
>>>> }
>>>>
>>>> + if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == 0) {
>>>> + rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
>>>> + return;
>>>> + }
>>>> +
>>>> + ret = kvmppc_fwnmi_enable(cpu);
>>>> + if (ret == 1) {
>>>
>>> I have the impression that this should really not happen,
>>> otherwise something has gone terribly wrong in QEMU or
>>> in KVM... this maybe deserves an error message as well ?
>>> No big deal.
>>
>> I think so.. will add an error message.
>
> Right, and because this is essentially a host side error,
> RTAS_OUT_HW_ERROR would be more appropriate.
sure
>
>>
>> Also I should check for non zero return value, not just ret == 1, as
>> kvmppc_fwnmi_enable() returns the error value from ioctl().
>>
>>
>>>
>>>> + rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
>>>> + return;
>>>> + }
>>>> +
>>>> + if (ret < 0) {
>>>> + rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
>>>> + return;
>>>> + }
>>>> +
>>>> spapr->guest_machine_check_addr = rtas_ld(args, 1);
>>>> rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>>>> }
>>>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>>>> index c717ab2..bd75d4b 100644
>>>> --- a/include/hw/ppc/spapr.h
>>>> +++ b/include/hw/ppc/spapr.h
>>>> @@ -78,8 +78,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
>>>> /* Num Caps */
>>>> -#define SPAPR_CAP_NUM (SPAPR_CAP_CCF_ASSIST + 1)
>>>> +#define SPAPR_CAP_NUM (SPAPR_CAP_FWNMI_MCE + 1)
>>>>
>>>> /*
>>>> * Capability Values
>>>> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
>>>> index 39f1a73..368ec6e 100644
>>>> --- a/target/ppc/kvm.c
>>>> +++ b/target/ppc/kvm.c
>>>> @@ -84,6 +84,7 @@ static int cap_ppc_safe_indirect_branch;
>>>> static int cap_ppc_count_cache_flush_assist;
>>>> static int cap_ppc_nested_kvm_hv;
>>>> static int cap_large_decr;
>>>> +static int cap_ppc_fwnmi;
>>>>
>>>> static uint32_t debug_inst_opcode;
>>>>
>>>> @@ -152,6 +153,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>>>> kvmppc_get_cpu_characteristics(s);
>>>> cap_ppc_nested_kvm_hv = kvm_vm_check_extension(s, KVM_CAP_PPC_NESTED_HV);
>>>> cap_large_decr = kvmppc_get_dec_bits();
>>>> + cap_ppc_fwnmi = kvm_check_extension(s, KVM_CAP_PPC_FWNMI);
>>>> /*
>>>> * Note: setting it to false because there is not such capability
>>>> * in KVM at this moment.
>>>> @@ -2119,6 +2121,18 @@ void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
>>>> }
>>>> }
>>>>
>>>> +int kvmppc_fwnmi_enable(PowerPCCPU *cpu)
>>>> +{
>>>> + CPUState *cs = CPU(cpu);
>>>> +
>>>> + if (!cap_ppc_fwnmi) {
>>>> + return 1;
>>>> + }
>>>> +
>>>> + return kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0);
>>>> +}
>>>> +
>>>> +
>>>> int kvmppc_smt_threads(void)
>>>> {
>>>> return cap_ppc_smt ? cap_ppc_smt : 1;
>>>> @@ -2419,6 +2433,11 @@ bool kvmppc_has_cap_mmu_hash_v3(void)
>>>> return cap_mmu_hash_v3;
>>>> }
>>>>
>>>> +bool kvmppc_has_cap_ppc_fwnmi(void)
>>>> +{
>>>> + return cap_ppc_fwnmi;
>>>> +}
>>>> +
>>>> static bool kvmppc_power8_host(void)
>>>> {
>>>> bool ret = false;
>>>> diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
>>>> index 18693f1..3d9f0b4 100644
>>>> --- a/target/ppc/kvm_ppc.h
>>>> +++ b/target/ppc/kvm_ppc.h
>>>> @@ -27,6 +27,8 @@ void kvmppc_enable_h_page_init(void);
>>>> void kvmppc_set_papr(PowerPCCPU *cpu);
>>>> int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr);
>>>> void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
>>>> +int kvmppc_fwnmi_enable(PowerPCCPU *cpu);
>>>> +bool kvmppc_has_cap_ppc_fwnmi(void);
>>>> int kvmppc_smt_threads(void);
>>>> void kvmppc_hint_smt_possible(Error **errp);
>>>> int kvmppc_set_smt_threads(int smt);
>>>> @@ -160,6 +162,16 @@ static inline void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
>>>> {
>>>> }
>>>>
>>>> +static inline int kvmppc_fwnmi_enable(PowerPCCPU *cpu)
>>>> +{
>>>> + return 1;
>>>> +}
>>>> +
>>>> +static inline bool kvmppc_has_cap_ppc_fwnmi(void)
>>>> +{
>>>> + return false;
>>>> +}
>>>> +
>>>> static inline int kvmppc_smt_threads(void)
>>>> {
>>>> return 1;
>>>>
>>>>
>>>
>>>
>>
>
--
Regards,
Aravinda
On Thursday 06 June 2019 08:32 AM, David Gibson wrote:
> On Tue, Jun 04, 2019 at 12:15:26PM +0530, Aravinda Prasad wrote:
>>
>>
>> On Monday 03 June 2019 08:55 PM, Greg Kurz wrote:
>>> On Wed, 29 May 2019 11:10:49 +0530
>>> Aravinda Prasad <aravinda@linux.vnet.ibm.com> wrote:
>>>
>>>> Enable the KVM capability KVM_CAP_PPC_FWNMI so that
>>>> the KVM causes guest exit with NMI as exit reason
>>>> when it encounters a machine check exception on the
>>>> address belonging to a guest. Without this capability
>>>> enabled, KVM redirects machine check exceptions to
>>>> guest's 0x200 vector.
>>>>
>>>> This patch also deals with the case when a guest with
>>>> the KVM_CAP_PPC_FWNMI capability enabled is attempted
>>>> to migrate to a host that does not support this
>>>> capability.
>>>>
>>>> Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com>
>>>> ---
>>>
>>> As suggested in another mail, it may be worth introducing the sPAPR cap
>>> in its own patch, earlier in the series.
>>
>> Sure, also as a workaround mentioned in the reply to that mail, I am
>> thinking of returning RTAS_OUT_NOT_SUPPORTED to rtas nmi register call
>> until the entire functionality is implemented. This will help solve
>> spapr cap issue as well.
>
> Not registering the RTAS call at all is the correct way to handle that
> case.
>
>>
>>>
>>> Anyway, I have some comments below.
>>>
>>>> hw/ppc/spapr.c | 1 +
>>>> hw/ppc/spapr_caps.c | 24 ++++++++++++++++++++++++
>>>> hw/ppc/spapr_rtas.c | 18 ++++++++++++++++++
>>>> include/hw/ppc/spapr.h | 4 +++-
>>>> target/ppc/kvm.c | 19 +++++++++++++++++++
>>>> target/ppc/kvm_ppc.h | 12 ++++++++++++
>>>> 6 files changed, 77 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>>> index c97f6a6..e8a77636 100644
>>>> --- a/hw/ppc/spapr.c
>>>> +++ b/hw/ppc/spapr.c
>>>> @@ -4364,6 +4364,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_OFF;
>>>> + smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_ON;
>>>> spapr_caps_add_properties(smc, &error_abort);
>>>> smc->irq = &spapr_irq_dual;
>>>> smc->dr_phb_enabled = true;
>>>> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
>>>> index 31b4661..ef9e612 100644
>>>> --- a/hw/ppc/spapr_caps.c
>>>> +++ b/hw/ppc/spapr_caps.c
>>>> @@ -479,6 +479,20 @@ static void cap_ccf_assist_apply(SpaprMachineState *spapr, uint8_t val,
>>>> }
>>>> }
>>>>
>>>> +static void cap_fwnmi_mce_apply(SpaprMachineState *spapr, uint8_t val,
>>>> + Error **errp)
>>>> +{
>>>> + if (!val) {
>>>> + return; /* Disabled by default */
>>>> + }
>>>> +
>>>> + if (tcg_enabled()) {
>>>> + error_setg(errp, "No fwnmi support in TCG, try cap-fwnmi-mce=off");
>>>
>>> Maybe expand "fwnmi" to "Firmware Assisted Non-Maskable Interrupts" ?
>>
>> sure..
>>
>>>
>>>> + } else if (kvm_enabled() && !kvmppc_has_cap_ppc_fwnmi()) {
>>>> + error_setg(errp, "Requested fwnmi capability not support by KVM");
>>>
>>> Maybe reword and add a hint:
>>>
>>> "KVM implementation does not support Firmware Assisted Non-Maskable Interrupts, try cap-fwnmi-mce=off"
>>
>> sure..
>>
>>>
>>>
>>>> + }
>>>> +}
>>>> +
>>>> SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
>>>> [SPAPR_CAP_HTM] = {
>>>> .name = "htm",
>>>> @@ -578,6 +592,15 @@ 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,
>>>> + .get = spapr_cap_get_bool,
>>>> + .set = spapr_cap_set_bool,
>>>> + .type = "bool",
>>>> + .apply = cap_fwnmi_mce_apply,
>>>> + },
>>>> };
>>>>
>>>> static SpaprCapabilities default_caps_with_cpu(SpaprMachineState *spapr,
>>>> @@ -717,6 +740,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);
>>>>
>>>> void spapr_caps_init(SpaprMachineState *spapr)
>>>> {
>>>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>>>> index e0bdfc8..91a7ab9 100644
>>>> --- a/hw/ppc/spapr_rtas.c
>>>> +++ b/hw/ppc/spapr_rtas.c
>>>> @@ -49,6 +49,7 @@
>>>> #include "hw/ppc/fdt.h"
>>>> #include "target/ppc/mmu-hash64.h"
>>>> #include "target/ppc/mmu-book3s-v3.h"
>>>> +#include "kvm_ppc.h"
>>>>
>>>> static void rtas_display_character(PowerPCCPU *cpu, SpaprMachineState *spapr,
>>>> uint32_t token, uint32_t nargs,
>>>> @@ -358,6 +359,7 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
>>>> target_ulong args,
>>>> uint32_t nret, target_ulong rets)
>>>> {
>>>> + int ret;
>>>> hwaddr rtas_addr = spapr_get_rtas_addr();
>>>>
>>>> if (!rtas_addr) {
>>>> @@ -365,6 +367,22 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
>>>> return;
>>>> }
>>>>
>>>> + if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == 0) {
>>>> + rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
>>>> + return;
>>>> + }
>>>> +
>>>> + ret = kvmppc_fwnmi_enable(cpu);
>>>> + if (ret == 1) {
>>>
>>> I have the impression that this should really not happen,
>>> otherwise something has gone terribly wrong in QEMU or
>>> in KVM... this maybe deserves an error message as well ?
>>> No big deal.
>>
>> I think so.. will add an error message.
>
> Right, and because this is essentially a host side error,
> RTAS_OUT_HW_ERROR would be more appropriate.
In fact I am already doing that. If ret == 1 I set it to
RTAS_OUT_NOT_SUPPORTE else if ret < 0 than I set the error to
RTAS_OUT_HW_ERROR.
>
>>
>> Also I should check for non zero return value, not just ret == 1, as
>> kvmppc_fwnmi_enable() returns the error value from ioctl().
>>
>>
>>>
>>>> + rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
>>>> + return;
>>>> + }
>>>> +
>>>> + if (ret < 0) {
>>>> + rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
>>>> + return;
>>>> + }
>>>> +
>>>> spapr->guest_machine_check_addr = rtas_ld(args, 1);
>>>> rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>>>> }
>>>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>>>> index c717ab2..bd75d4b 100644
>>>> --- a/include/hw/ppc/spapr.h
>>>> +++ b/include/hw/ppc/spapr.h
>>>> @@ -78,8 +78,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
>>>> /* Num Caps */
>>>> -#define SPAPR_CAP_NUM (SPAPR_CAP_CCF_ASSIST + 1)
>>>> +#define SPAPR_CAP_NUM (SPAPR_CAP_FWNMI_MCE + 1)
>>>>
>>>> /*
>>>> * Capability Values
>>>> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
>>>> index 39f1a73..368ec6e 100644
>>>> --- a/target/ppc/kvm.c
>>>> +++ b/target/ppc/kvm.c
>>>> @@ -84,6 +84,7 @@ static int cap_ppc_safe_indirect_branch;
>>>> static int cap_ppc_count_cache_flush_assist;
>>>> static int cap_ppc_nested_kvm_hv;
>>>> static int cap_large_decr;
>>>> +static int cap_ppc_fwnmi;
>>>>
>>>> static uint32_t debug_inst_opcode;
>>>>
>>>> @@ -152,6 +153,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>>>> kvmppc_get_cpu_characteristics(s);
>>>> cap_ppc_nested_kvm_hv = kvm_vm_check_extension(s, KVM_CAP_PPC_NESTED_HV);
>>>> cap_large_decr = kvmppc_get_dec_bits();
>>>> + cap_ppc_fwnmi = kvm_check_extension(s, KVM_CAP_PPC_FWNMI);
>>>> /*
>>>> * Note: setting it to false because there is not such capability
>>>> * in KVM at this moment.
>>>> @@ -2119,6 +2121,18 @@ void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
>>>> }
>>>> }
>>>>
>>>> +int kvmppc_fwnmi_enable(PowerPCCPU *cpu)
>>>> +{
>>>> + CPUState *cs = CPU(cpu);
>>>> +
>>>> + if (!cap_ppc_fwnmi) {
>>>> + return 1;
>>>> + }
>>>> +
>>>> + return kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0);
>>>> +}
>>>> +
>>>> +
>>>> int kvmppc_smt_threads(void)
>>>> {
>>>> return cap_ppc_smt ? cap_ppc_smt : 1;
>>>> @@ -2419,6 +2433,11 @@ bool kvmppc_has_cap_mmu_hash_v3(void)
>>>> return cap_mmu_hash_v3;
>>>> }
>>>>
>>>> +bool kvmppc_has_cap_ppc_fwnmi(void)
>>>> +{
>>>> + return cap_ppc_fwnmi;
>>>> +}
>>>> +
>>>> static bool kvmppc_power8_host(void)
>>>> {
>>>> bool ret = false;
>>>> diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
>>>> index 18693f1..3d9f0b4 100644
>>>> --- a/target/ppc/kvm_ppc.h
>>>> +++ b/target/ppc/kvm_ppc.h
>>>> @@ -27,6 +27,8 @@ void kvmppc_enable_h_page_init(void);
>>>> void kvmppc_set_papr(PowerPCCPU *cpu);
>>>> int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr);
>>>> void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
>>>> +int kvmppc_fwnmi_enable(PowerPCCPU *cpu);
>>>> +bool kvmppc_has_cap_ppc_fwnmi(void);
>>>> int kvmppc_smt_threads(void);
>>>> void kvmppc_hint_smt_possible(Error **errp);
>>>> int kvmppc_set_smt_threads(int smt);
>>>> @@ -160,6 +162,16 @@ static inline void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
>>>> {
>>>> }
>>>>
>>>> +static inline int kvmppc_fwnmi_enable(PowerPCCPU *cpu)
>>>> +{
>>>> + return 1;
>>>> +}
>>>> +
>>>> +static inline bool kvmppc_has_cap_ppc_fwnmi(void)
>>>> +{
>>>> + return false;
>>>> +}
>>>> +
>>>> static inline int kvmppc_smt_threads(void)
>>>> {
>>>> return 1;
>>>>
>>>>
>>>
>>>
>>
>
--
Regards,
Aravinda
On Wed, May 29, 2019 at 11:10:49AM +0530, Aravinda Prasad wrote:
> Enable the KVM capability KVM_CAP_PPC_FWNMI so that
> the KVM causes guest exit with NMI as exit reason
> when it encounters a machine check exception on the
> address belonging to a guest. Without this capability
> enabled, KVM redirects machine check exceptions to
> guest's 0x200 vector.
>
> This patch also deals with the case when a guest with
> the KVM_CAP_PPC_FWNMI capability enabled is attempted
> to migrate to a host that does not support this
> capability.
>
> Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com>
> ---
> hw/ppc/spapr.c | 1 +
> hw/ppc/spapr_caps.c | 24 ++++++++++++++++++++++++
> hw/ppc/spapr_rtas.c | 18 ++++++++++++++++++
> include/hw/ppc/spapr.h | 4 +++-
> target/ppc/kvm.c | 19 +++++++++++++++++++
> target/ppc/kvm_ppc.h | 12 ++++++++++++
> 6 files changed, 77 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index c97f6a6..e8a77636 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -4364,6 +4364,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_OFF;
> + smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_ON;
You need to turn this back off by default for the older machine types.
> spapr_caps_add_properties(smc, &error_abort);
> smc->irq = &spapr_irq_dual;
> smc->dr_phb_enabled = true;
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index 31b4661..ef9e612 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -479,6 +479,20 @@ static void cap_ccf_assist_apply(SpaprMachineState *spapr, uint8_t val,
> }
> }
>
> +static void cap_fwnmi_mce_apply(SpaprMachineState *spapr, uint8_t val,
> + Error **errp)
> +{
> + if (!val) {
> + return; /* Disabled by default */
> + }
> +
> + if (tcg_enabled()) {
> + error_setg(errp, "No fwnmi support in TCG, try cap-fwnmi-mce=off");
> + } else if (kvm_enabled() && !kvmppc_has_cap_ppc_fwnmi()) {
> + error_setg(errp, "Requested fwnmi capability not support by KVM");
> + }
> +}
> +
> SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> [SPAPR_CAP_HTM] = {
> .name = "htm",
> @@ -578,6 +592,15 @@ 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,
> + .get = spapr_cap_get_bool,
> + .set = spapr_cap_set_bool,
> + .type = "bool",
> + .apply = cap_fwnmi_mce_apply,
> + },
> };
>
> static SpaprCapabilities default_caps_with_cpu(SpaprMachineState *spapr,
> @@ -717,6 +740,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);
>
> void spapr_caps_init(SpaprMachineState *spapr)
> {
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index e0bdfc8..91a7ab9 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -49,6 +49,7 @@
> #include "hw/ppc/fdt.h"
> #include "target/ppc/mmu-hash64.h"
> #include "target/ppc/mmu-book3s-v3.h"
> +#include "kvm_ppc.h"
>
> static void rtas_display_character(PowerPCCPU *cpu, SpaprMachineState *spapr,
> uint32_t token, uint32_t nargs,
> @@ -358,6 +359,7 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
> target_ulong args,
> uint32_t nret, target_ulong rets)
> {
> + int ret;
> hwaddr rtas_addr = spapr_get_rtas_addr();
>
> if (!rtas_addr) {
> @@ -365,6 +367,22 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
> return;
> }
>
> + if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == 0) {
> + rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
> + return;
> + }
> +
> + ret = kvmppc_fwnmi_enable(cpu);
> + if (ret == 1) {
> + rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
> + return;
> + }
> +
> + if (ret < 0) {
> + rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
> + return;
> + }
> +
> spapr->guest_machine_check_addr = rtas_ld(args, 1);
> rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> }
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index c717ab2..bd75d4b 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -78,8 +78,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
> /* Num Caps */
> -#define SPAPR_CAP_NUM (SPAPR_CAP_CCF_ASSIST + 1)
> +#define SPAPR_CAP_NUM (SPAPR_CAP_FWNMI_MCE + 1)
>
> /*
> * Capability Values
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 39f1a73..368ec6e 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -84,6 +84,7 @@ static int cap_ppc_safe_indirect_branch;
> static int cap_ppc_count_cache_flush_assist;
> static int cap_ppc_nested_kvm_hv;
> static int cap_large_decr;
> +static int cap_ppc_fwnmi;
>
> static uint32_t debug_inst_opcode;
>
> @@ -152,6 +153,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
> kvmppc_get_cpu_characteristics(s);
> cap_ppc_nested_kvm_hv = kvm_vm_check_extension(s, KVM_CAP_PPC_NESTED_HV);
> cap_large_decr = kvmppc_get_dec_bits();
> + cap_ppc_fwnmi = kvm_check_extension(s, KVM_CAP_PPC_FWNMI);
> /*
> * Note: setting it to false because there is not such capability
> * in KVM at this moment.
> @@ -2119,6 +2121,18 @@ void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
> }
> }
>
> +int kvmppc_fwnmi_enable(PowerPCCPU *cpu)
> +{
> + CPUState *cs = CPU(cpu);
> +
> + if (!cap_ppc_fwnmi) {
> + return 1;
> + }
> +
> + return kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0);
> +}
> +
> +
> int kvmppc_smt_threads(void)
> {
> return cap_ppc_smt ? cap_ppc_smt : 1;
> @@ -2419,6 +2433,11 @@ bool kvmppc_has_cap_mmu_hash_v3(void)
> return cap_mmu_hash_v3;
> }
>
> +bool kvmppc_has_cap_ppc_fwnmi(void)
> +{
> + return cap_ppc_fwnmi;
> +}
> +
> static bool kvmppc_power8_host(void)
> {
> bool ret = false;
> diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
> index 18693f1..3d9f0b4 100644
> --- a/target/ppc/kvm_ppc.h
> +++ b/target/ppc/kvm_ppc.h
> @@ -27,6 +27,8 @@ void kvmppc_enable_h_page_init(void);
> void kvmppc_set_papr(PowerPCCPU *cpu);
> int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr);
> void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
> +int kvmppc_fwnmi_enable(PowerPCCPU *cpu);
> +bool kvmppc_has_cap_ppc_fwnmi(void);
> int kvmppc_smt_threads(void);
> void kvmppc_hint_smt_possible(Error **errp);
> int kvmppc_set_smt_threads(int smt);
> @@ -160,6 +162,16 @@ static inline void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
> {
> }
>
> +static inline int kvmppc_fwnmi_enable(PowerPCCPU *cpu)
> +{
> + return 1;
> +}
> +
> +static inline bool kvmppc_has_cap_ppc_fwnmi(void)
> +{
> + return false;
> +}
> +
> static inline int kvmppc_smt_threads(void)
> {
> return 1;
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
© 2016 - 2025 Red Hat, Inc.