* [Qemu-devel] [PATCH v12 0/6] target-ppc/spapr: Add FWNMI support in QEMU for PowerKVM guests
@ 2019-08-30 9:13 Aravinda Prasad
2019-08-30 9:13 ` [Qemu-devel] [PATCH v12 1/6] Wrapper function to wait on condition for the main loop mutex Aravinda Prasad
` (5 more replies)
0 siblings, 6 replies; 16+ messages in thread
From: Aravinda Prasad @ 2019-08-30 9:13 UTC (permalink / raw)
To: aik, qemu-ppc, qemu-devel, david; +Cc: paulus, aravinda, groug
This patch set adds support for FWNMI in PowerKVM guests.
System errors such as SLB multihit and memory errors
that cannot be corrected by hardware is passed on to
the kernel for handling by raising machine check
exception (an NMI). Upon such machine check exceptions,
if the address in error belongs to guest then KVM
invokes guests' 0x200 interrupt vector if the guest
is not FWNMI capable. For FWNMI capable guest
KVM passes the control to QEMU by exiting the guest.
This patch series adds functionality to QEMU to pass
on such machine check exceptions to the FWNMI capable
guest kernel by building an error log and invoking
the guest registered machine check handling routine.
The KVM changes are now part of the upstream kernel
(commit e20bbd3d). This series contain QEMU changes.
Change Log v12:
- Rebased to latest ppc-for-4.2 (SHA b1e8156743)
Change Log v11:
- Moved FWNMI SPAPR cap defaults to 4.2 class option
- Fixed issues with handling fwnmi KVM capability
Change Log v10:
- Reshuffled the patch sequence + minor fixes
Change Log v9:
- Fixed kvm cap and spapr cap issues
Change Log v8:
- Added functionality to check FWNMI capability during
VM migration
---
Aravinda Prasad (6):
Wrapper function to wait on condition for the main loop mutex
ppc: spapr: Introduce FWNMI capability
target/ppc: Handle NMI guest exit
target/ppc: Build rtas error log upon an MCE
ppc: spapr: Handle "ibm,nmi-register" and "ibm,nmi-interlock" RTAS calls
migration: Include migration support for machine check handling
cpus.c | 5 +
hw/ppc/spapr.c | 76 +++++++++++++
hw/ppc/spapr_caps.c | 29 +++++
hw/ppc/spapr_events.c | 269 ++++++++++++++++++++++++++++++++++++++++++++++
hw/ppc/spapr_rtas.c | 78 +++++++++++++
include/hw/ppc/spapr.h | 25 ++++
include/qemu/main-loop.h | 8 +
target/ppc/kvm.c | 38 ++++++
target/ppc/kvm_ppc.h | 13 ++
target/ppc/trace-events | 1
10 files changed, 540 insertions(+), 2 deletions(-)
--
Aravinda Prasad
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH v12 1/6] Wrapper function to wait on condition for the main loop mutex
2019-08-30 9:13 [Qemu-devel] [PATCH v12 0/6] target-ppc/spapr: Add FWNMI support in QEMU for PowerKVM guests Aravinda Prasad
@ 2019-08-30 9:13 ` Aravinda Prasad
2019-08-30 9:13 ` [Qemu-devel] [PATCH v12 2/6] ppc: spapr: Introduce FWNMI capability Aravinda Prasad
` (4 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: Aravinda Prasad @ 2019-08-30 9:13 UTC (permalink / raw)
To: aik, qemu-ppc, qemu-devel, david; +Cc: paulus, aravinda, groug
Introduce a wrapper function to wait on condition for
the main loop mutex. This function atomically releases
the main loop mutex and causes the calling thread to
block on the condition. This wrapper is required because
qemu_global_mutex is a static variable.
Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
cpus.c | 5 +++++
include/qemu/main-loop.h | 8 ++++++++
2 files changed, 13 insertions(+)
diff --git a/cpus.c b/cpus.c
index 85cd451..d229830 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1877,6 +1877,11 @@ void qemu_mutex_unlock_iothread(void)
qemu_mutex_unlock(&qemu_global_mutex);
}
+void qemu_cond_wait_iothread(QemuCond *cond)
+{
+ qemu_cond_wait(cond, &qemu_global_mutex);
+}
+
static bool all_vcpus_paused(void)
{
CPUState *cpu;
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index f6ba78e..a6d20b0 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -295,6 +295,14 @@ void qemu_mutex_lock_iothread_impl(const char *file, int line);
*/
void qemu_mutex_unlock_iothread(void);
+/*
+ * qemu_cond_wait_iothread: Wait on condition for the main loop mutex
+ *
+ * This function atomically releases the main loop mutex and causes
+ * the calling thread to block on the condition.
+ */
+void qemu_cond_wait_iothread(QemuCond *cond);
+
/* internal interfaces */
void qemu_fd_register(int fd);
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH v12 2/6] ppc: spapr: Introduce FWNMI capability
2019-08-30 9:13 [Qemu-devel] [PATCH v12 0/6] target-ppc/spapr: Add FWNMI support in QEMU for PowerKVM guests Aravinda Prasad
2019-08-30 9:13 ` [Qemu-devel] [PATCH v12 1/6] Wrapper function to wait on condition for the main loop mutex Aravinda Prasad
@ 2019-08-30 9:13 ` Aravinda Prasad
2019-08-30 13:58 ` Greg Kurz
2019-08-30 9:13 ` [Qemu-devel] [PATCH v12 3/6] target/ppc: Handle NMI guest exit Aravinda Prasad
` (3 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Aravinda Prasad @ 2019-08-30 9:13 UTC (permalink / raw)
To: aik, qemu-ppc, qemu-devel, david; +Cc: paulus, aravinda, groug
Introduce 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 introduces fwnmi-mce capability to
deal 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 | 29 +++++++++++++++++++++++++++++
include/hw/ppc/spapr.h | 4 +++-
target/ppc/kvm.c | 22 ++++++++++++++++++++++
target/ppc/kvm_ppc.h | 11 +++++++++++
5 files changed, 66 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index ea56499..8288e8b 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4487,6 +4487,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_OFF;
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 481dfd2..c11ff87 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -496,6 +496,25 @@ 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()) {
+ /*
+ * TCG support may not be correct in some conditions (e.g., in case
+ * of software injected faults like duplicate SLBs).
+ */
+ warn_report("Firmware Assisted Non-Maskable Interrupts not supported in TCG");
+ } else if (kvm_enabled() && !kvmppc_has_cap_ppc_fwnmi()) {
+ error_setg(errp,
+"Firmware Assisted Non-Maskable Interrupts not supported by KVM, try cap-fwnmi-mce=off");
+ }
+}
+
SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
[SPAPR_CAP_HTM] = {
.name = "htm",
@@ -595,6 +614,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,
@@ -734,6 +762,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/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 03111fd..66049ac 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -79,8 +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
/* 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 8c5b1f2..c055fc1 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -85,6 +85,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;
@@ -2055,6 +2056,22 @@ void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
}
}
+int kvmppc_set_fwnmi(void)
+{
+ PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
+ CPUState *cs = CPU(cpu);
+ int ret;
+
+ ret = kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0);
+ if (ret) {
+ error_report("This KVM version does not support FWNMI");
+ return ret;
+ }
+
+ cap_ppc_fwnmi = 1;
+ return ret;
+}
+
int kvmppc_smt_threads(void)
{
return cap_ppc_smt ? cap_ppc_smt : 1;
@@ -2355,6 +2372,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 98bd7d5..2990898 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_set_fwnmi(void);
+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);
@@ -159,6 +161,15 @@ static inline void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
{
}
+static inline int kvmppc_set_fwnmi(void)
+{
+}
+
+static inline bool kvmppc_has_cap_ppc_fwnmi(void)
+{
+ return false;
+}
+
static inline int kvmppc_smt_threads(void)
{
return 1;
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH v12 3/6] target/ppc: Handle NMI guest exit
2019-08-30 9:13 [Qemu-devel] [PATCH v12 0/6] target-ppc/spapr: Add FWNMI support in QEMU for PowerKVM guests Aravinda Prasad
2019-08-30 9:13 ` [Qemu-devel] [PATCH v12 1/6] Wrapper function to wait on condition for the main loop mutex Aravinda Prasad
2019-08-30 9:13 ` [Qemu-devel] [PATCH v12 2/6] ppc: spapr: Introduce FWNMI capability Aravinda Prasad
@ 2019-08-30 9:13 ` Aravinda Prasad
2019-09-03 8:02 ` Greg Kurz
2019-08-30 9:13 ` [Qemu-devel] [PATCH v12 4/6] target/ppc: Build rtas error log upon an MCE Aravinda Prasad
` (2 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Aravinda Prasad @ 2019-08-30 9:13 UTC (permalink / raw)
To: aik, qemu-ppc, qemu-devel, david; +Cc: paulus, aravinda, groug
Memory error such as bit flips that cannot be corrected
by hardware are passed on to the kernel for handling.
If the memory address in error belongs to guest then
the guest kernel is responsible for taking suitable action.
Patch [1] enhances KVM to exit guest with exit reason
set to KVM_EXIT_NMI in such cases. This patch handles
KVM_EXIT_NMI exit.
[1] https://www.spinics.net/lists/kvm-ppc/msg12637.html
(e20bbd3d and related commits)
Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ppc/spapr.c | 8 ++++++++
hw/ppc/spapr_events.c | 24 ++++++++++++++++++++++++
include/hw/ppc/spapr.h | 10 ++++++++++
target/ppc/kvm.c | 14 ++++++++++++++
target/ppc/kvm_ppc.h | 2 ++
target/ppc/trace-events | 1 +
6 files changed, 59 insertions(+)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 8288e8b..76ed988 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1823,6 +1823,12 @@ static void spapr_machine_reset(MachineState *machine)
first_ppc_cpu->env.gpr[5] = 0;
spapr->cas_reboot = false;
+
+ spapr->mc_status = -1;
+ spapr->guest_machine_check_addr = -1;
+
+ /* Signal all vCPUs waiting on this condition */
+ qemu_cond_broadcast(&spapr->mc_delivery_cond);
}
static void spapr_create_nvram(SpaprMachineState *spapr)
@@ -3099,6 +3105,8 @@ static void spapr_machine_init(MachineState *machine)
kvmppc_spapr_enable_inkernel_multitce();
}
+
+ qemu_cond_init(&spapr->mc_delivery_cond);
}
static int spapr_kvm_type(MachineState *machine, const char *vm_type)
diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index 0e4c195..e76c1a7 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -40,6 +40,7 @@
#include "hw/ppc/spapr_drc.h"
#include "qemu/help_option.h"
#include "qemu/bcd.h"
+#include "qemu/main-loop.h"
#include "hw/ppc/spapr_ovec.h"
#include <libfdt.h>
@@ -621,6 +622,29 @@ void spapr_hotplug_req_remove_by_count_indexed(SpaprDrcType drc_type,
RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, &drc_id);
}
+void spapr_mce_req_event(PowerPCCPU *cpu)
+{
+ SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
+
+ while (spapr->mc_status != -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) {
+ qemu_system_guest_panicked(NULL);
+ return;
+ }
+ qemu_cond_wait_iothread(&spapr->mc_delivery_cond);
+ /* Meanwhile if the system is reset, then just return */
+ if (spapr->guest_machine_check_addr == -1) {
+ return;
+ }
+ }
+ spapr->mc_status = cpu->vcpu_id;
+}
+
static void check_exception(PowerPCCPU *cpu, SpaprMachineState *spapr,
uint32_t token, uint32_t nargs,
target_ulong args,
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 66049ac..99a2966 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -192,6 +192,15 @@ 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.
+ */
+ int mc_status;
+ QemuCond mc_delivery_cond;
+
/*< public >*/
char *kvm_type;
char *host_model;
@@ -805,6 +814,7 @@ void spapr_clear_pending_events(SpaprMachineState *spapr);
int spapr_max_server_number(SpaprMachineState *spapr);
void spapr_store_hpte(PowerPCCPU *cpu, hwaddr ptex,
uint64_t pte0, uint64_t pte1);
+void spapr_mce_req_event(PowerPCCPU *cpu);
/* DRC callbacks. */
void spapr_core_release(DeviceState *dev);
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index c055fc1..4e282f6 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -1704,6 +1704,11 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
ret = 0;
break;
+ case KVM_EXIT_NMI:
+ trace_kvm_handle_nmi_exception();
+ ret = kvm_handle_nmi(cpu, run);
+ break;
+
default:
fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason);
ret = -1;
@@ -2807,6 +2812,15 @@ int kvm_arch_msi_data_to_gsi(uint32_t data)
return data & 0xffff;
}
+int kvm_handle_nmi(PowerPCCPU *cpu, struct kvm_run *run)
+{
+ cpu_synchronize_state(CPU(cpu));
+
+ spapr_mce_req_event(cpu);
+
+ return 0;
+}
+
int kvmppc_enable_hwrng(void)
{
if (!kvm_enabled() || !kvm_check_extension(kvm_state, KVM_CAP_PPC_HWRNG)) {
diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
index 2990898..173c000 100644
--- a/target/ppc/kvm_ppc.h
+++ b/target/ppc/kvm_ppc.h
@@ -84,6 +84,8 @@ void kvm_check_mmu(PowerPCCPU *cpu, Error **errp);
void kvmppc_set_reg_ppc_online(PowerPCCPU *cpu, unsigned int online);
void kvmppc_set_reg_tb_offset(PowerPCCPU *cpu, int64_t tb_offset);
+int kvm_handle_nmi(PowerPCCPU *cpu, struct kvm_run *run);
+
#else
static inline uint32_t kvmppc_get_tbfreq(void)
diff --git a/target/ppc/trace-events b/target/ppc/trace-events
index 3dc6740..6d15aa9 100644
--- a/target/ppc/trace-events
+++ b/target/ppc/trace-events
@@ -28,3 +28,4 @@ kvm_handle_papr_hcall(void) "handle PAPR hypercall"
kvm_handle_epr(void) "handle epr"
kvm_handle_watchdog_expiry(void) "handle watchdog expiry"
kvm_handle_debug_exception(void) "handle debug exception"
+kvm_handle_nmi_exception(void) "handle NMI exception"
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH v12 4/6] target/ppc: Build rtas error log upon an MCE
2019-08-30 9:13 [Qemu-devel] [PATCH v12 0/6] target-ppc/spapr: Add FWNMI support in QEMU for PowerKVM guests Aravinda Prasad
` (2 preceding siblings ...)
2019-08-30 9:13 ` [Qemu-devel] [PATCH v12 3/6] target/ppc: Handle NMI guest exit Aravinda Prasad
@ 2019-08-30 9:13 ` Aravinda Prasad
2019-09-03 10:06 ` Greg Kurz
2019-08-30 9:14 ` [Qemu-devel] [PATCH v12 5/6] ppc: spapr: Handle "ibm, nmi-register" and "ibm, nmi-interlock" RTAS calls Aravinda Prasad
2019-08-30 9:14 ` [Qemu-devel] [PATCH v12 6/6] migration: Include migration support for machine check handling Aravinda Prasad
5 siblings, 1 reply; 16+ messages in thread
From: Aravinda Prasad @ 2019-08-30 9:13 UTC (permalink / raw)
To: aik, qemu-ppc, qemu-devel, david; +Cc: paulus, aravinda, groug
Upon a machine check exception (MCE) in a guest address space,
KVM causes a guest exit to enable QEMU to build and pass the
error to the guest in the PAPR defined rtas error log format.
This patch builds the rtas error log, copies it to the rtas_addr
and then invokes the guest registered machine check handler. The
handler in the guest takes suitable action(s) depending on the type
and criticality of the error. For example, if an error is
unrecoverable memory corruption in an application inside the
guest, then the guest kernel sends a SIGBUS to the application.
For recoverable errors, the guest performs recovery actions and
logs the error.
Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com>
---
hw/ppc/spapr.c | 13 +++
hw/ppc/spapr_events.c | 233 ++++++++++++++++++++++++++++++++++++++++++++++++
hw/ppc/spapr_rtas.c | 26 +++++
include/hw/ppc/spapr.h | 6 +
target/ppc/kvm.c | 4 +
5 files changed, 279 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 76ed988..9f2e5d2 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2930,6 +2930,19 @@ static void spapr_machine_init(MachineState *machine)
error_report("Could not get size of LPAR rtas '%s'", filename);
exit(1);
}
+
+ if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == SPAPR_CAP_ON) {
+ /*
+ * Ensure that the rtas image size is less than RTAS_ERROR_LOG_OFFSET
+ * or else the rtas image will be overwritten with the rtas error log
+ * when a machine check exception is encountered.
+ */
+ g_assert(spapr->rtas_size < RTAS_ERROR_LOG_OFFSET);
+
+ /* Resize rtas blob to accommodate error log */
+ spapr->rtas_size = RTAS_ERROR_LOG_MAX;
+ }
+
spapr->rtas_blob = g_malloc(spapr->rtas_size);
if (load_image_size(filename, spapr->rtas_blob, spapr->rtas_size) < 0) {
error_report("Could not load LPAR rtas '%s'", filename);
diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index e76c1a7..8ebb85e 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -214,6 +214,106 @@ struct hp_extended_log {
struct rtas_event_log_v6_hp hp;
} QEMU_PACKED;
+struct rtas_event_log_v6_mc {
+#define RTAS_LOG_V6_SECTION_ID_MC 0x4D43 /* MC */
+ struct rtas_event_log_v6_section_header hdr;
+ uint32_t fru_id;
+ uint32_t proc_id;
+ uint8_t error_type;
+#define RTAS_LOG_V6_MC_TYPE_UE 0
+#define RTAS_LOG_V6_MC_TYPE_SLB 1
+#define RTAS_LOG_V6_MC_TYPE_ERAT 2
+#define RTAS_LOG_V6_MC_TYPE_TLB 4
+#define RTAS_LOG_V6_MC_TYPE_D_CACHE 5
+#define RTAS_LOG_V6_MC_TYPE_I_CACHE 7
+ uint8_t sub_err_type;
+#define RTAS_LOG_V6_MC_UE_INDETERMINATE 0
+#define RTAS_LOG_V6_MC_UE_IFETCH 1
+#define RTAS_LOG_V6_MC_UE_PAGE_TABLE_WALK_IFETCH 2
+#define RTAS_LOG_V6_MC_UE_LOAD_STORE 3
+#define RTAS_LOG_V6_MC_UE_PAGE_TABLE_WALK_LOAD_STORE 4
+#define RTAS_LOG_V6_MC_SLB_PARITY 0
+#define RTAS_LOG_V6_MC_SLB_MULTIHIT 1
+#define RTAS_LOG_V6_MC_SLB_INDETERMINATE 2
+#define RTAS_LOG_V6_MC_ERAT_PARITY 1
+#define RTAS_LOG_V6_MC_ERAT_MULTIHIT 2
+#define RTAS_LOG_V6_MC_ERAT_INDETERMINATE 3
+#define RTAS_LOG_V6_MC_TLB_PARITY 1
+#define RTAS_LOG_V6_MC_TLB_MULTIHIT 2
+#define RTAS_LOG_V6_MC_TLB_INDETERMINATE 3
+ uint8_t reserved_1[6];
+ uint64_t effective_address;
+ uint64_t logical_address;
+} QEMU_PACKED;
+
+struct mc_extended_log {
+ struct rtas_event_log_v6 v6hdr;
+ struct rtas_event_log_v6_mc mc;
+} QEMU_PACKED;
+
+struct MC_ierror_table {
+ unsigned long srr1_mask;
+ unsigned long srr1_value;
+ bool nip_valid; /* nip is a valid indicator of faulting address */
+ uint8_t error_type;
+ uint8_t error_subtype;
+ unsigned int initiator;
+ unsigned int severity;
+};
+
+static const struct MC_ierror_table mc_ierror_table[] = {
+{ 0x00000000081c0000, 0x0000000000040000, true,
+ RTAS_LOG_V6_MC_TYPE_UE, RTAS_LOG_V6_MC_UE_IFETCH,
+ RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
+{ 0x00000000081c0000, 0x0000000000080000, true,
+ RTAS_LOG_V6_MC_TYPE_SLB, RTAS_LOG_V6_MC_SLB_PARITY,
+ RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
+{ 0x00000000081c0000, 0x00000000000c0000, true,
+ RTAS_LOG_V6_MC_TYPE_SLB, RTAS_LOG_V6_MC_SLB_MULTIHIT,
+ RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
+{ 0x00000000081c0000, 0x0000000000100000, true,
+ RTAS_LOG_V6_MC_TYPE_ERAT, RTAS_LOG_V6_MC_ERAT_MULTIHIT,
+ RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
+{ 0x00000000081c0000, 0x0000000000140000, true,
+ RTAS_LOG_V6_MC_TYPE_TLB, RTAS_LOG_V6_MC_TLB_MULTIHIT,
+ RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
+{ 0x00000000081c0000, 0x0000000000180000, true,
+ RTAS_LOG_V6_MC_TYPE_UE, RTAS_LOG_V6_MC_UE_PAGE_TABLE_WALK_IFETCH,
+ RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
+{ 0, 0, 0, 0, 0, 0 } };
+
+struct MC_derror_table {
+ unsigned long dsisr_value;
+ bool dar_valid; /* dar is a valid indicator of faulting address */
+ uint8_t error_type;
+ uint8_t error_subtype;
+ unsigned int initiator;
+ unsigned int severity;
+};
+
+static const struct MC_derror_table mc_derror_table[] = {
+{ 0x00008000, false,
+ RTAS_LOG_V6_MC_TYPE_UE, RTAS_LOG_V6_MC_UE_LOAD_STORE,
+ RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
+{ 0x00004000, true,
+ RTAS_LOG_V6_MC_TYPE_UE, RTAS_LOG_V6_MC_UE_PAGE_TABLE_WALK_LOAD_STORE,
+ RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
+{ 0x00000800, true,
+ RTAS_LOG_V6_MC_TYPE_ERAT, RTAS_LOG_V6_MC_ERAT_MULTIHIT,
+ RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
+{ 0x00000400, true,
+ RTAS_LOG_V6_MC_TYPE_TLB, RTAS_LOG_V6_MC_TLB_MULTIHIT,
+ RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
+{ 0x00000080, true,
+ RTAS_LOG_V6_MC_TYPE_SLB, RTAS_LOG_V6_MC_SLB_MULTIHIT, /* Before PARITY */
+ RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
+{ 0x00000100, true,
+ RTAS_LOG_V6_MC_TYPE_SLB, RTAS_LOG_V6_MC_SLB_PARITY,
+ RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
+{ 0, false, 0, 0, 0, 0 } };
+
+#define SRR1_MC_LOADSTORE(srr1) ((srr1) & PPC_BIT(42))
+
typedef enum EventClass {
EVENT_CLASS_INTERNAL_ERRORS = 0,
EVENT_CLASS_EPOW = 1,
@@ -622,7 +722,136 @@ void spapr_hotplug_req_remove_by_count_indexed(SpaprDrcType drc_type,
RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, &drc_id);
}
-void spapr_mce_req_event(PowerPCCPU *cpu)
+static uint32_t spapr_mce_get_elog_type(PowerPCCPU *cpu, bool recovered,
+ struct mc_extended_log *ext_elog)
+{
+ int i;
+ CPUPPCState *env = &cpu->env;
+ uint32_t summary;
+ uint64_t dsisr = env->spr[SPR_DSISR];
+
+ summary = RTAS_LOG_VERSION_6 | RTAS_LOG_OPTIONAL_PART_PRESENT;
+ if (recovered) {
+ summary |= RTAS_LOG_DISPOSITION_FULLY_RECOVERED;
+ } else {
+ summary |= RTAS_LOG_DISPOSITION_NOT_RECOVERED;
+ }
+
+ if (SRR1_MC_LOADSTORE(env->spr[SPR_SRR1])) {
+ for (i = 0; mc_derror_table[i].dsisr_value; i++) {
+ if (!(dsisr & mc_derror_table[i].dsisr_value)) {
+ continue;
+ }
+
+ ext_elog->mc.error_type = mc_derror_table[i].error_type;
+ ext_elog->mc.sub_err_type = mc_derror_table[i].error_subtype;
+ if (mc_derror_table[i].dar_valid) {
+ ext_elog->mc.effective_address = cpu_to_be64(env->spr[SPR_DAR]);
+ }
+
+ summary |= mc_derror_table[i].initiator
+ | mc_derror_table[i].severity;
+
+ return summary;
+ }
+ } else {
+ for (i = 0; mc_ierror_table[i].srr1_mask; i++) {
+ if ((env->spr[SPR_SRR1] & mc_ierror_table[i].srr1_mask) !=
+ mc_ierror_table[i].srr1_value) {
+ continue;
+ }
+
+ ext_elog->mc.error_type = mc_ierror_table[i].error_type;
+ ext_elog->mc.sub_err_type = mc_ierror_table[i].error_subtype;
+ if (mc_ierror_table[i].nip_valid) {
+ ext_elog->mc.effective_address = cpu_to_be64(env->nip);
+ }
+
+ summary |= mc_ierror_table[i].initiator
+ | mc_ierror_table[i].severity;
+
+ return summary;
+ }
+ }
+
+ summary |= RTAS_LOG_INITIATOR_CPU;
+ return summary;
+}
+
+static void spapr_mce_dispatch_elog(PowerPCCPU *cpu, bool recovered)
+{
+ SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
+ CPUState *cs = CPU(cpu);
+ uint64_t rtas_addr;
+ CPUPPCState *env = &cpu->env;
+ PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+ target_ulong msr = 0;
+ struct rtas_error_log log;
+ struct mc_extended_log *ext_elog;
+ uint32_t summary;
+
+ /*
+ * Properly set bits in MSR before we invoke the handler.
+ * SRR0/1, DAR and DSISR are properly set by KVM
+ */
+ if (!(*pcc->interrupts_big_endian)(cpu)) {
+ msr |= (1ULL << MSR_LE);
+ }
+
+ if (env->msr & (1ULL << MSR_SF)) {
+ msr |= (1ULL << MSR_SF);
+ }
+
+ msr |= (1ULL << MSR_ME);
+
+ if (spapr->guest_machine_check_addr == -1) {
+ /*
+ * This implies that we have hit a machine check between system
+ * reset and "ibm,nmi-register". Fall back to the old machine
+ * check behavior in such cases.
+ */
+ cs->exception_index = POWERPC_EXCP_MCHECK;
+ ppc_cpu_do_interrupt(cs);
+ return;
+ }
+
+ ext_elog = g_malloc0(sizeof(*ext_elog));
+ summary = spapr_mce_get_elog_type(cpu, recovered, ext_elog);
+
+ log.summary = cpu_to_be32(summary);
+ log.extended_length = cpu_to_be32(sizeof(*ext_elog));
+
+ spapr_init_v6hdr(&ext_elog->v6hdr);
+ ext_elog->mc.hdr.section_id = cpu_to_be16(RTAS_LOG_V6_SECTION_ID_MC);
+ ext_elog->mc.hdr.section_length =
+ cpu_to_be16(sizeof(struct rtas_event_log_v6_mc));
+ ext_elog->mc.hdr.section_version = 1;
+
+ /* get rtas addr from fdt */
+ rtas_addr = spapr_get_rtas_addr();
+ if (!rtas_addr) {
+ /* Unable to fetch rtas_addr. Hence reset the guest */
+ ppc_cpu_do_system_reset(cs);
+ g_free(ext_elog);
+ return;
+ }
+
+ stq_be_phys(&address_space_memory, rtas_addr + RTAS_ERROR_LOG_OFFSET,
+ env->gpr[3]);
+ cpu_physical_memory_write(rtas_addr + RTAS_ERROR_LOG_OFFSET +
+ sizeof(env->gpr[3]), &log, sizeof(log));
+ cpu_physical_memory_write(rtas_addr + RTAS_ERROR_LOG_OFFSET +
+ sizeof(env->gpr[3]) + sizeof(log), ext_elog,
+ sizeof(*ext_elog));
+
+ env->gpr[3] = rtas_addr + RTAS_ERROR_LOG_OFFSET;
+ env->msr = msr;
+ env->nip = spapr->guest_machine_check_addr;
+
+ g_free(ext_elog);
+}
+
+void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered)
{
SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
@@ -643,6 +872,8 @@ void spapr_mce_req_event(PowerPCCPU *cpu)
}
}
spapr->mc_status = cpu->vcpu_id;
+
+ spapr_mce_dispatch_elog(cpu, recovered);
}
static void check_exception(PowerPCCPU *cpu, SpaprMachineState *spapr,
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index bee3835..d8fb8a8 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -518,6 +518,32 @@ void spapr_load_rtas(SpaprMachineState *spapr, void *fdt, hwaddr addr)
}
}
+hwaddr spapr_get_rtas_addr(void)
+{
+ SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
+ int rtas_node;
+ const fdt32_t *rtas_data;
+ void *fdt = spapr->fdt_blob;
+
+ /* fetch rtas addr from fdt */
+ rtas_node = fdt_path_offset(fdt, "/rtas");
+ if (rtas_node < 0) {
+ return 0;
+ }
+
+ rtas_data = fdt_getprop(fdt, rtas_node, "linux,rtas-base", NULL);
+ if (!rtas_data) {
+ return 0;
+ }
+
+ /*
+ * We assume that the OS called RTAS instantiate-rtas, but some other
+ * OS might call RTAS instantiate-rtas-64 instead. This fine as of now
+ * as SLOF only supports 32-bit variant.
+ */
+ return (hwaddr)fdt32_to_cpu(*rtas_data);
+}
+
static void core_rtas_register_types(void)
{
spapr_rtas_register(RTAS_DISPLAY_CHARACTER, "display-character",
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 99a2966..ffefde7 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -726,6 +726,9 @@ void spapr_load_rtas(SpaprMachineState *spapr, void *fdt, hwaddr addr);
#define RTAS_ERROR_LOG_MAX 2048
+/* Offset from rtas-base where error log is placed */
+#define RTAS_ERROR_LOG_OFFSET 0x30
+
#define RTAS_EVENT_SCAN_RATE 1
/* This helper should be used to encode interrupt specifiers when the related
@@ -814,7 +817,7 @@ void spapr_clear_pending_events(SpaprMachineState *spapr);
int spapr_max_server_number(SpaprMachineState *spapr);
void spapr_store_hpte(PowerPCCPU *cpu, hwaddr ptex,
uint64_t pte0, uint64_t pte1);
-void spapr_mce_req_event(PowerPCCPU *cpu);
+void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered);
/* DRC callbacks. */
void spapr_core_release(DeviceState *dev);
@@ -904,4 +907,5 @@ void spapr_check_pagesize(SpaprMachineState *spapr, hwaddr pagesize,
#define SPAPR_OV5_XIVE_BOTH 0x80 /* Only to advertise on the platform */
void spapr_set_all_lpcrs(target_ulong value, target_ulong mask);
+hwaddr spapr_get_rtas_addr(void);
#endif /* HW_SPAPR_H */
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 4e282f6..68080b9 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -2814,9 +2814,11 @@ int kvm_arch_msi_data_to_gsi(uint32_t data)
int kvm_handle_nmi(PowerPCCPU *cpu, struct kvm_run *run)
{
+ bool recovered = run->flags & KVM_RUN_PPC_NMI_DISP_FULLY_RECOV;
+
cpu_synchronize_state(CPU(cpu));
- spapr_mce_req_event(cpu);
+ spapr_mce_req_event(cpu, recovered);
return 0;
}
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH v12 5/6] ppc: spapr: Handle "ibm, nmi-register" and "ibm, nmi-interlock" RTAS calls
2019-08-30 9:13 [Qemu-devel] [PATCH v12 0/6] target-ppc/spapr: Add FWNMI support in QEMU for PowerKVM guests Aravinda Prasad
` (3 preceding siblings ...)
2019-08-30 9:13 ` [Qemu-devel] [PATCH v12 4/6] target/ppc: Build rtas error log upon an MCE Aravinda Prasad
@ 2019-08-30 9:14 ` Aravinda Prasad
2019-08-30 17:08 ` Greg Kurz
2019-08-30 9:14 ` [Qemu-devel] [PATCH v12 6/6] migration: Include migration support for machine check handling Aravinda Prasad
5 siblings, 1 reply; 16+ messages in thread
From: Aravinda Prasad @ 2019-08-30 9:14 UTC (permalink / raw)
To: aik, qemu-ppc, qemu-devel, david; +Cc: paulus, aravinda, groug
This patch adds support in QEMU to handle "ibm,nmi-register"
and "ibm,nmi-interlock" RTAS calls and sets the default
value of SPAPR_CAP_FWNMI_MCE to SPAPR_CAP_ON for machine
type 4.2.
The machine check notification address is saved when the
OS issues "ibm,nmi-register" RTAS call.
This patch also handles the case when multiple processors
experience machine check at or about the same time by
handling "ibm,nmi-interlock" call. In such cases, as per
PAPR, subsequent processors serialize waiting for the first
processor to issue the "ibm,nmi-interlock" call. The second
processor that also received a machine check error waits
till the first processor is done reading the error log.
The first processor issues "ibm,nmi-interlock" call
when the error log is consumed.
Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com>
---
hw/ppc/spapr.c | 12 +++++++++++-
hw/ppc/spapr_rtas.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
include/hw/ppc/spapr.h | 5 ++++-
3 files changed, 65 insertions(+), 2 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 9f2e5d2..1c0908e 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2941,6 +2941,15 @@ static void spapr_machine_init(MachineState *machine)
/* Resize rtas blob to accommodate error log */
spapr->rtas_size = RTAS_ERROR_LOG_MAX;
+
+ /* Set fwnmi capability in KVM */
+ if (kvmppc_set_fwnmi() < 0) {
+ error_report("Could not enable FWNMI capability");
+ exit(1);
+ }
+
+ /* Register ibm,nmi-register and ibm,nmi-interlock RTAS calls */
+ spapr_fwnmi_register();
}
spapr->rtas_blob = g_malloc(spapr->rtas_size);
@@ -4508,7 +4517,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_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;
@@ -4582,6 +4591,7 @@ static void spapr_machine_4_1_class_options(MachineClass *mc)
smc->linux_pci_probe = false;
compat_props_add(mc->compat_props, hw_compat_4_1, hw_compat_4_1_len);
compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat));
+ smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_OFF;
}
DEFINE_SPAPR_MACHINE(4_1, "4.1", false);
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index d8fb8a8..d892583 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -400,6 +400,48 @@ static void rtas_get_power_level(PowerPCCPU *cpu, SpaprMachineState *spapr,
rtas_st(rets, 1, 100);
}
+static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
+ SpaprMachineState *spapr,
+ uint32_t token, uint32_t nargs,
+ target_ulong args,
+ uint32_t nret, target_ulong rets)
+{
+ hwaddr rtas_addr = spapr_get_rtas_addr();
+
+ if (!rtas_addr) {
+ rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
+ return;
+ }
+
+ if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == SPAPR_CAP_OFF) {
+ rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
+ return;
+ }
+
+ spapr->guest_machine_check_addr = rtas_ld(args, 1);
+ rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+}
+
+static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
+ SpaprMachineState *spapr,
+ uint32_t token, uint32_t nargs,
+ target_ulong args,
+ uint32_t nret, target_ulong rets)
+{
+ if (spapr->guest_machine_check_addr == -1) {
+ /* NMI register not called */
+ rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+ } else {
+ /*
+ * vCPU issuing "ibm,nmi-interlock" is done with NMI handling,
+ * hence unset mc_status.
+ */
+ spapr->mc_status = -1;
+ qemu_cond_signal(&spapr->mc_delivery_cond);
+ rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+ }
+}
+
static struct rtas_call {
const char *name;
spapr_rtas_fn fn;
@@ -544,6 +586,14 @@ hwaddr spapr_get_rtas_addr(void)
return (hwaddr)fdt32_to_cpu(*rtas_data);
}
+void spapr_fwnmi_register(void)
+{
+ spapr_rtas_register(RTAS_IBM_NMI_REGISTER, "ibm,nmi-register",
+ rtas_ibm_nmi_register);
+ spapr_rtas_register(RTAS_IBM_NMI_INTERLOCK, "ibm,nmi-interlock",
+ rtas_ibm_nmi_interlock);
+}
+
static void core_rtas_register_types(void)
{
spapr_rtas_register(RTAS_DISPLAY_CHARACTER, "display-character",
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index ffefde7..dada821 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -655,8 +655,10 @@ target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong opcode,
#define RTAS_IBM_REMOVE_PE_DMA_WINDOW (RTAS_TOKEN_BASE + 0x28)
#define RTAS_IBM_RESET_PE_DMA_WINDOW (RTAS_TOKEN_BASE + 0x29)
#define RTAS_IBM_SUSPEND_ME (RTAS_TOKEN_BASE + 0x2A)
+#define RTAS_IBM_NMI_REGISTER (RTAS_TOKEN_BASE + 0x2B)
+#define RTAS_IBM_NMI_INTERLOCK (RTAS_TOKEN_BASE + 0x2C)
-#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x2B)
+#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x2D)
/* RTAS ibm,get-system-parameter token values */
#define RTAS_SYSPARM_SPLPAR_CHARACTERISTICS 20
@@ -908,4 +910,5 @@ void spapr_check_pagesize(SpaprMachineState *spapr, hwaddr pagesize,
void spapr_set_all_lpcrs(target_ulong value, target_ulong mask);
hwaddr spapr_get_rtas_addr(void);
+void spapr_fwnmi_register(void);
#endif /* HW_SPAPR_H */
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH v12 6/6] migration: Include migration support for machine check handling
2019-08-30 9:13 [Qemu-devel] [PATCH v12 0/6] target-ppc/spapr: Add FWNMI support in QEMU for PowerKVM guests Aravinda Prasad
` (4 preceding siblings ...)
2019-08-30 9:14 ` [Qemu-devel] [PATCH v12 5/6] ppc: spapr: Handle "ibm, nmi-register" and "ibm, nmi-interlock" RTAS calls Aravinda Prasad
@ 2019-08-30 9:14 ` Aravinda Prasad
5 siblings, 0 replies; 16+ messages in thread
From: Aravinda Prasad @ 2019-08-30 9:14 UTC (permalink / raw)
To: aik, qemu-ppc, qemu-devel, david; +Cc: paulus, aravinda, groug
This patch includes migration support for machine check
handling. Especially this patch blocks VM migration
requests until the machine check error handling is
complete as (i) these errors are specific to the source
hardware and is irrelevant on the target hardware,
(ii) these errors cause data corruption and should
be handled before migration.
Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com>
---
hw/ppc/spapr.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
hw/ppc/spapr_events.c | 14 ++++++++++++++
hw/ppc/spapr_rtas.c | 2 ++
include/hw/ppc/spapr.h | 2 ++
4 files changed, 62 insertions(+)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 1c0908e..f6262f0 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -46,6 +46,7 @@
#include "migration/qemu-file-types.h"
#include "migration/global_state.h"
#include "migration/register.h"
+#include "migration/blocker.h"
#include "mmu-hash64.h"
#include "mmu-book3s-v3.h"
#include "cpu-models.h"
@@ -1829,6 +1830,8 @@ static void spapr_machine_reset(MachineState *machine)
/* Signal all vCPUs waiting on this condition */
qemu_cond_broadcast(&spapr->mc_delivery_cond);
+
+ migrate_del_blocker(spapr->fwnmi_migration_blocker);
}
static void spapr_create_nvram(SpaprMachineState *spapr)
@@ -2119,6 +2122,42 @@ static const VMStateDescription vmstate_spapr_dtb = {
},
};
+static bool spapr_fwnmi_needed(void *opaque)
+{
+ SpaprMachineState *spapr = (SpaprMachineState *)opaque;
+
+ return spapr->guest_machine_check_addr != -1;
+}
+
+static int spapr_fwnmi_post_load(void *opaque, int version_id)
+{
+ SpaprMachineState *spapr = (SpaprMachineState *)opaque;
+
+ if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == SPAPR_CAP_ON) {
+
+ if (kvmppc_has_cap_ppc_fwnmi()) {
+ return 0;
+ }
+
+ return kvmppc_set_fwnmi();
+ }
+
+ return 0;
+}
+
+static const VMStateDescription vmstate_spapr_machine_check = {
+ .name = "spapr_machine_check",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = spapr_fwnmi_needed,
+ .post_load = spapr_fwnmi_post_load,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT64(guest_machine_check_addr, SpaprMachineState),
+ VMSTATE_INT32(mc_status, SpaprMachineState),
+ VMSTATE_END_OF_LIST()
+ },
+};
+
static const VMStateDescription vmstate_spapr = {
.name = "spapr",
.version_id = 3,
@@ -2152,6 +2191,7 @@ static const VMStateDescription vmstate_spapr = {
&vmstate_spapr_dtb,
&vmstate_spapr_cap_large_decr,
&vmstate_spapr_cap_ccf_assist,
+ &vmstate_spapr_machine_check,
NULL
}
};
@@ -2948,6 +2988,10 @@ static void spapr_machine_init(MachineState *machine)
exit(1);
}
+ /* Create the error string for live migration blocker */
+ error_setg(&spapr->fwnmi_migration_blocker,
+ "Live migration not supported during machine check handling");
+
/* Register ibm,nmi-register and ibm,nmi-interlock RTAS calls */
spapr_fwnmi_register();
}
diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index 8ebb85e..a8ce9bd 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -43,6 +43,7 @@
#include "qemu/main-loop.h"
#include "hw/ppc/spapr_ovec.h"
#include <libfdt.h>
+#include "migration/blocker.h"
#define RTAS_LOG_VERSION_MASK 0xff000000
#define RTAS_LOG_VERSION_6 0x06000000
@@ -854,6 +855,19 @@ static void spapr_mce_dispatch_elog(PowerPCCPU *cpu, bool recovered)
void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered)
{
SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
+ int ret;
+ Error *local_err = NULL;
+
+ ret = migrate_add_blocker(spapr->fwnmi_migration_blocker, &local_err);
+ if (ret < 0) {
+ /*
+ * We don't want to abort and let the migration to continue. In a
+ * rare case, the machine check handler will run on the target
+ * hardware. Though this is not preferable, it is better than aborting
+ * the migration or killing the VM.
+ */
+ warn_report_err(local_err);
+ }
while (spapr->mc_status != -1) {
/*
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index d892583..b682cc2 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -50,6 +50,7 @@
#include "hw/ppc/fdt.h"
#include "target/ppc/mmu-hash64.h"
#include "target/ppc/mmu-book3s-v3.h"
+#include "migration/blocker.h"
static void rtas_display_character(PowerPCCPU *cpu, SpaprMachineState *spapr,
uint32_t token, uint32_t nargs,
@@ -438,6 +439,7 @@ static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
*/
spapr->mc_status = -1;
qemu_cond_signal(&spapr->mc_delivery_cond);
+ migrate_del_blocker(spapr->fwnmi_migration_blocker);
rtas_st(rets, 0, RTAS_OUT_SUCCESS);
}
}
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index dada821..ea7625e 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -217,6 +217,8 @@ struct SpaprMachineState {
unsigned gpu_numa_id;
SpaprTpmProxy *tpm_proxy;
+
+ Error *fwnmi_migration_blocker;
};
#define H_SUCCESS 0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v12 2/6] ppc: spapr: Introduce FWNMI capability
2019-08-30 9:13 ` [Qemu-devel] [PATCH v12 2/6] ppc: spapr: Introduce FWNMI capability Aravinda Prasad
@ 2019-08-30 13:58 ` Greg Kurz
2019-09-03 7:22 ` Aravinda Prasad
0 siblings, 1 reply; 16+ messages in thread
From: Greg Kurz @ 2019-08-30 13:58 UTC (permalink / raw)
To: Aravinda Prasad; +Cc: aik, qemu-devel, paulus, qemu-ppc, david
On Fri, 30 Aug 2019 14:43:40 +0530
Aravinda Prasad <aravinda@linux.vnet.ibm.com> wrote:
> Introduce 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 introduces fwnmi-mce capability to
> deal 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 | 29 +++++++++++++++++++++++++++++
> include/hw/ppc/spapr.h | 4 +++-
> target/ppc/kvm.c | 22 ++++++++++++++++++++++
> target/ppc/kvm_ppc.h | 11 +++++++++++
> 5 files changed, 66 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index ea56499..8288e8b 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -4487,6 +4487,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_OFF;
> 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 481dfd2..c11ff87 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -496,6 +496,25 @@ 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()) {
> + /*
> + * TCG support may not be correct in some conditions (e.g., in case
> + * of software injected faults like duplicate SLBs).
> + */
> + warn_report("Firmware Assisted Non-Maskable Interrupts not supported in TCG");
> + } else if (kvm_enabled() && !kvmppc_has_cap_ppc_fwnmi()) {
> + error_setg(errp,
> +"Firmware Assisted Non-Maskable Interrupts not supported by KVM, try cap-fwnmi-mce=off");
> + }
> +}
> +
> SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> [SPAPR_CAP_HTM] = {
> .name = "htm",
> @@ -595,6 +614,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,
> @@ -734,6 +762,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/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 03111fd..66049ac 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -79,8 +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
> /* 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 8c5b1f2..c055fc1 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -85,6 +85,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;
>
> @@ -2055,6 +2056,22 @@ void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
> }
> }
>
> +int kvmppc_set_fwnmi(void)
> +{
> + PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
> + CPUState *cs = CPU(cpu);
> + int ret;
> +
> + ret = kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0);
> + if (ret) {
> + error_report("This KVM version does not support FWNMI");
> + return ret;
> + }
> +
> + cap_ppc_fwnmi = 1;
Hmm... AFAICT the meaning of cap_ppc_fwnmi should just be "KVM knows about
FWNMI", not "FWNMI was successfully enabled in KVM". Your v10 used to set
cap_ppc_fwnmi in kvm_arch_init() just like the other guys... why this change ?
> + return ret;
> +}
> +
> int kvmppc_smt_threads(void)
> {
> return cap_ppc_smt ? cap_ppc_smt : 1;
> @@ -2355,6 +2372,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 98bd7d5..2990898 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_set_fwnmi(void);
> +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);
> @@ -159,6 +161,15 @@ static inline void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
> {
> }
>
> +static inline int kvmppc_set_fwnmi(void)
> +{
Missing return -1;
> +}
> +
> +static inline bool kvmppc_has_cap_ppc_fwnmi(void)
> +{
> + return false;
> +}
> +
> static inline int kvmppc_smt_threads(void)
> {
> return 1;
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v12 5/6] ppc: spapr: Handle "ibm, nmi-register" and "ibm, nmi-interlock" RTAS calls
2019-08-30 9:14 ` [Qemu-devel] [PATCH v12 5/6] ppc: spapr: Handle "ibm, nmi-register" and "ibm, nmi-interlock" RTAS calls Aravinda Prasad
@ 2019-08-30 17:08 ` Greg Kurz
2019-09-03 7:38 ` Aravinda Prasad
0 siblings, 1 reply; 16+ messages in thread
From: Greg Kurz @ 2019-08-30 17:08 UTC (permalink / raw)
To: Aravinda Prasad; +Cc: aik, qemu-devel, paulus, qemu-ppc, david
On Fri, 30 Aug 2019 14:44:07 +0530
Aravinda Prasad <aravinda@linux.vnet.ibm.com> wrote:
> This patch adds support in QEMU to handle "ibm,nmi-register"
> and "ibm,nmi-interlock" RTAS calls and sets the default
> value of SPAPR_CAP_FWNMI_MCE to SPAPR_CAP_ON for machine
> type 4.2.
>
> The machine check notification address is saved when the
> OS issues "ibm,nmi-register" RTAS call.
>
> This patch also handles the case when multiple processors
> experience machine check at or about the same time by
> handling "ibm,nmi-interlock" call. In such cases, as per
> PAPR, subsequent processors serialize waiting for the first
> processor to issue the "ibm,nmi-interlock" call. The second
> processor that also received a machine check error waits
> till the first processor is done reading the error log.
> The first processor issues "ibm,nmi-interlock" call
> when the error log is consumed.
>
> Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com>
> ---
> hw/ppc/spapr.c | 12 +++++++++++-
> hw/ppc/spapr_rtas.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
> include/hw/ppc/spapr.h | 5 ++++-
> 3 files changed, 65 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 9f2e5d2..1c0908e 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2941,6 +2941,15 @@ static void spapr_machine_init(MachineState *machine)
>
> /* Resize rtas blob to accommodate error log */
> spapr->rtas_size = RTAS_ERROR_LOG_MAX;
> +
> + /* Set fwnmi capability in KVM */
> + if (kvmppc_set_fwnmi() < 0) {
> + error_report("Could not enable FWNMI capability");
> + exit(1);
> + }
Hmm... shouldn't this be performed only when the guest
calls "ibm,nmi-register" ?
> +
> + /* Register ibm,nmi-register and ibm,nmi-interlock RTAS calls */
> + spapr_fwnmi_register();
> }
>
> spapr->rtas_blob = g_malloc(spapr->rtas_size);
> @@ -4508,7 +4517,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_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;
> @@ -4582,6 +4591,7 @@ static void spapr_machine_4_1_class_options(MachineClass *mc)
> smc->linux_pci_probe = false;
> compat_props_add(mc->compat_props, hw_compat_4_1, hw_compat_4_1_len);
> compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat));
> + smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_OFF;
> }
>
> DEFINE_SPAPR_MACHINE(4_1, "4.1", false);
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index d8fb8a8..d892583 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -400,6 +400,48 @@ static void rtas_get_power_level(PowerPCCPU *cpu, SpaprMachineState *spapr,
> rtas_st(rets, 1, 100);
> }
>
> +static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
> + SpaprMachineState *spapr,
> + uint32_t token, uint32_t nargs,
> + target_ulong args,
> + uint32_t nret, target_ulong rets)
> +{
> + hwaddr rtas_addr = spapr_get_rtas_addr();
> +
> + if (!rtas_addr) {
> + rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
> + return;
> + }
> +
> + if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == SPAPR_CAP_OFF) {
> + rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
> + return;
> + }
> +
> + spapr->guest_machine_check_addr = rtas_ld(args, 1);
Doesn't this need some sanity checks ? At least error out on -1
which has a special meaning in the code and cannot really be used
as a valid instruction address.
Also PAPR+ says:
R1–7.3.14–6. For the FWNMI option: The Real/Logical address of the
registered OS Machine Check and System Reset routines must be in the
first 32 MB of the OS’s memory address space.
And only at this point you may enable the cap in KVM since the
guest has decided to use FWNMI.
My concern is: what happens when the guest reboots ? We set
guest_machine_check_addr back to -1 during machine reset but
KVM still assumes the guest has enabled FWNMI... I see that
enabling FWNMI boils down to setting a kvm->arch.fwnmi_enabled
to true... what about providing a way to disable FWNMI ?
> + rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> +}
> +
> +static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
> + SpaprMachineState *spapr,
> + uint32_t token, uint32_t nargs,
> + target_ulong args,
> + uint32_t nret, target_ulong rets)
> +{
> + if (spapr->guest_machine_check_addr == -1) {
> + /* NMI register not called */
> + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> + } else {
> + /*
> + * vCPU issuing "ibm,nmi-interlock" is done with NMI handling,
> + * hence unset mc_status.
> + */
> + spapr->mc_status = -1;
> + qemu_cond_signal(&spapr->mc_delivery_cond);
> + rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> + }
> +}
> +
> static struct rtas_call {
> const char *name;
> spapr_rtas_fn fn;
> @@ -544,6 +586,14 @@ hwaddr spapr_get_rtas_addr(void)
> return (hwaddr)fdt32_to_cpu(*rtas_data);
> }
>
> +void spapr_fwnmi_register(void)
> +{
> + spapr_rtas_register(RTAS_IBM_NMI_REGISTER, "ibm,nmi-register",
> + rtas_ibm_nmi_register);
> + spapr_rtas_register(RTAS_IBM_NMI_INTERLOCK, "ibm,nmi-interlock",
> + rtas_ibm_nmi_interlock);
> +}
> +
> static void core_rtas_register_types(void)
> {
> spapr_rtas_register(RTAS_DISPLAY_CHARACTER, "display-character",
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index ffefde7..dada821 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -655,8 +655,10 @@ target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong opcode,
> #define RTAS_IBM_REMOVE_PE_DMA_WINDOW (RTAS_TOKEN_BASE + 0x28)
> #define RTAS_IBM_RESET_PE_DMA_WINDOW (RTAS_TOKEN_BASE + 0x29)
> #define RTAS_IBM_SUSPEND_ME (RTAS_TOKEN_BASE + 0x2A)
> +#define RTAS_IBM_NMI_REGISTER (RTAS_TOKEN_BASE + 0x2B)
> +#define RTAS_IBM_NMI_INTERLOCK (RTAS_TOKEN_BASE + 0x2C)
>
> -#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x2B)
> +#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x2D)
>
> /* RTAS ibm,get-system-parameter token values */
> #define RTAS_SYSPARM_SPLPAR_CHARACTERISTICS 20
> @@ -908,4 +910,5 @@ void spapr_check_pagesize(SpaprMachineState *spapr, hwaddr pagesize,
>
> void spapr_set_all_lpcrs(target_ulong value, target_ulong mask);
> hwaddr spapr_get_rtas_addr(void);
> +void spapr_fwnmi_register(void);
> #endif /* HW_SPAPR_H */
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v12 2/6] ppc: spapr: Introduce FWNMI capability
2019-08-30 13:58 ` Greg Kurz
@ 2019-09-03 7:22 ` Aravinda Prasad
2019-09-03 7:56 ` Greg Kurz
0 siblings, 1 reply; 16+ messages in thread
From: Aravinda Prasad @ 2019-09-03 7:22 UTC (permalink / raw)
To: Greg Kurz; +Cc: aik, qemu-devel, paulus, qemu-ppc, david
On Friday 30 August 2019 07:28 PM, Greg Kurz wrote:
> On Fri, 30 Aug 2019 14:43:40 +0530
> Aravinda Prasad <aravinda@linux.vnet.ibm.com> wrote:
>
>> Introduce 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 introduces fwnmi-mce capability to
>> deal 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 | 29 +++++++++++++++++++++++++++++
>> include/hw/ppc/spapr.h | 4 +++-
>> target/ppc/kvm.c | 22 ++++++++++++++++++++++
>> target/ppc/kvm_ppc.h | 11 +++++++++++
>> 5 files changed, 66 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index ea56499..8288e8b 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -4487,6 +4487,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_OFF;
>> 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 481dfd2..c11ff87 100644
>> --- a/hw/ppc/spapr_caps.c
>> +++ b/hw/ppc/spapr_caps.c
>> @@ -496,6 +496,25 @@ 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()) {
>> + /*
>> + * TCG support may not be correct in some conditions (e.g., in case
>> + * of software injected faults like duplicate SLBs).
>> + */
>> + warn_report("Firmware Assisted Non-Maskable Interrupts not supported in TCG");
>> + } else if (kvm_enabled() && !kvmppc_has_cap_ppc_fwnmi()) {
>> + error_setg(errp,
>> +"Firmware Assisted Non-Maskable Interrupts not supported by KVM, try cap-fwnmi-mce=off");
>> + }
>> +}
>> +
>> SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
>> [SPAPR_CAP_HTM] = {
>> .name = "htm",
>> @@ -595,6 +614,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,
>> @@ -734,6 +762,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/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>> index 03111fd..66049ac 100644
>> --- a/include/hw/ppc/spapr.h
>> +++ b/include/hw/ppc/spapr.h
>> @@ -79,8 +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
>> /* 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 8c5b1f2..c055fc1 100644
>> --- a/target/ppc/kvm.c
>> +++ b/target/ppc/kvm.c
>> @@ -85,6 +85,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;
>>
>> @@ -2055,6 +2056,22 @@ void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
>> }
>> }
>>
>> +int kvmppc_set_fwnmi(void)
>> +{
>> + PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
>> + CPUState *cs = CPU(cpu);
>> + int ret;
>> +
>> + ret = kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0);
>> + if (ret) {
>> + error_report("This KVM version does not support FWNMI");
>> + return ret;
>> + }
>> +
>> + cap_ppc_fwnmi = 1;
>
> Hmm... AFAICT the meaning of cap_ppc_fwnmi should just be "KVM knows about
> FWNMI", not "FWNMI was successfully enabled in KVM". Your v10 used to set
> cap_ppc_fwnmi in kvm_arch_init() just like the other guys... why this change ?
Even I thought that cap_* is for that, but cap_papr uses it the other
way. So I decided to use a similar convention for cap_ppc_fwnmi.
In v10, cap_ppc_fwnmi is set in kvm_arch_init() if FWNMI is available in
KVM and we try to enable FWNMI later in the boot phase when
rtas_ibm_nmi_register() is called. It's possible that
SPAPR_CAP_FWNMI_MCE is set and we may fail to enable FWNMI due to errors
in KVM or QEMU.
To avoid this, in v12, FWNMI is enabled if SPAPR_CAP_FWNMI_MCE is set
irrespective of rtas_ibm_nmi_register() is called or not. This way we
fail early if we are not able to enable FWNMI.
So, when SPAPR_CAP_FWNMI_MCE is set with FWNMI enabled in KVM, a machine
check error will always cause a guest exit. If the guest has registered
a handler by calling rtas_ibm_nmi_register(), then we will invoke the
guest registered handler else we will jump to 0x200 interrupt vector in
the guest.
With this change we don't need "KVM knows about FWNMI" information. So I
removed that part and used cap_ppc_fwnmi to track if FWNMI is enabled or
disabled in KVM. This also simplifies spapr_fwnmi_post_load() to avoid
enabling FWNMI multiple times (see patch 6).
>
>> + return ret;
>> +}
>> +
>> int kvmppc_smt_threads(void)
>> {
>> return cap_ppc_smt ? cap_ppc_smt : 1;
>> @@ -2355,6 +2372,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 98bd7d5..2990898 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_set_fwnmi(void);
>> +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);
>> @@ -159,6 +161,15 @@ static inline void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
>> {
>> }
>>
>> +static inline int kvmppc_set_fwnmi(void)
>> +{
>
> Missing return -1;
Hmm.. missed it.
Regards,
Aravinda
>
>> +}
>> +
>> +static inline bool kvmppc_has_cap_ppc_fwnmi(void)
>> +{
>> + return false;
>> +}
>> +
>> static inline int kvmppc_smt_threads(void)
>> {
>> return 1;
>>
>
--
Regards,
Aravinda
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v12 5/6] ppc: spapr: Handle "ibm, nmi-register" and "ibm, nmi-interlock" RTAS calls
2019-08-30 17:08 ` Greg Kurz
@ 2019-09-03 7:38 ` Aravinda Prasad
0 siblings, 0 replies; 16+ messages in thread
From: Aravinda Prasad @ 2019-09-03 7:38 UTC (permalink / raw)
To: Greg Kurz; +Cc: aik, qemu-devel, paulus, qemu-ppc, david
On Friday 30 August 2019 10:38 PM, Greg Kurz wrote:
> On Fri, 30 Aug 2019 14:44:07 +0530
> Aravinda Prasad <aravinda@linux.vnet.ibm.com> wrote:
>
>> This patch adds support in QEMU to handle "ibm,nmi-register"
>> and "ibm,nmi-interlock" RTAS calls and sets the default
>> value of SPAPR_CAP_FWNMI_MCE to SPAPR_CAP_ON for machine
>> type 4.2.
>>
>> The machine check notification address is saved when the
>> OS issues "ibm,nmi-register" RTAS call.
>>
>> This patch also handles the case when multiple processors
>> experience machine check at or about the same time by
>> handling "ibm,nmi-interlock" call. In such cases, as per
>> PAPR, subsequent processors serialize waiting for the first
>> processor to issue the "ibm,nmi-interlock" call. The second
>> processor that also received a machine check error waits
>> till the first processor is done reading the error log.
>> The first processor issues "ibm,nmi-interlock" call
>> when the error log is consumed.
>>
>> Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com>
>> ---
>> hw/ppc/spapr.c | 12 +++++++++++-
>> hw/ppc/spapr_rtas.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
>> include/hw/ppc/spapr.h | 5 ++++-
>> 3 files changed, 65 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 9f2e5d2..1c0908e 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -2941,6 +2941,15 @@ static void spapr_machine_init(MachineState *machine)
>>
>> /* Resize rtas blob to accommodate error log */
>> spapr->rtas_size = RTAS_ERROR_LOG_MAX;
>> +
>> + /* Set fwnmi capability in KVM */
>> + if (kvmppc_set_fwnmi() < 0) {
>> + error_report("Could not enable FWNMI capability");
>> + exit(1);
>> + }
>
> Hmm... shouldn't this be performed only when the guest
> calls "ibm,nmi-register" ?
The above code is called only when SPAPR_CAP_FWNMI_MCE is set. Please
see my reply to your comments on patch 2 where I have explained why we
don't call it during "ibm,nmi-register".
>
>> +
>> + /* Register ibm,nmi-register and ibm,nmi-interlock RTAS calls */
>> + spapr_fwnmi_register();
>> }
>>
>> spapr->rtas_blob = g_malloc(spapr->rtas_size);
>> @@ -4508,7 +4517,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_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;
>> @@ -4582,6 +4591,7 @@ static void spapr_machine_4_1_class_options(MachineClass *mc)
>> smc->linux_pci_probe = false;
>> compat_props_add(mc->compat_props, hw_compat_4_1, hw_compat_4_1_len);
>> compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat));
>> + smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_OFF;
>> }
>>
>> DEFINE_SPAPR_MACHINE(4_1, "4.1", false);
>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>> index d8fb8a8..d892583 100644
>> --- a/hw/ppc/spapr_rtas.c
>> +++ b/hw/ppc/spapr_rtas.c
>> @@ -400,6 +400,48 @@ static void rtas_get_power_level(PowerPCCPU *cpu, SpaprMachineState *spapr,
>> rtas_st(rets, 1, 100);
>> }
>>
>> +static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
>> + SpaprMachineState *spapr,
>> + uint32_t token, uint32_t nargs,
>> + target_ulong args,
>> + uint32_t nret, target_ulong rets)
>> +{
>> + hwaddr rtas_addr = spapr_get_rtas_addr();
>> +
>> + if (!rtas_addr) {
>> + rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
>> + return;
>> + }
>> +
>> + if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == SPAPR_CAP_OFF) {
>> + rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
>> + return;
>> + }
>> +
>> + spapr->guest_machine_check_addr = rtas_ld(args, 1);
>
> Doesn't this need some sanity checks ? At least error out on -1
> which has a special meaning in the code and cannot really be used
> as a valid instruction address.
>
> Also PAPR+ says:
>
> R1–7.3.14–6. For the FWNMI option: The Real/Logical address of the
> registered OS Machine Check and System Reset routines must be in the
> first 32 MB of the OS’s memory address space.
Yes, we can add a sanity check, but I feel it is not required, as a
guest can still provide a wrong address in the first 32MB address space.
>
>
> And only at this point you may enable the cap in KVM since the
> guest has decided to use FWNMI.
>
> My concern is: what happens when the guest reboots ? We set
> guest_machine_check_addr back to -1 during machine reset but
> KVM still assumes the guest has enabled FWNMI... I see that
> enabling FWNMI boils down to setting a kvm->arch.fwnmi_enabled
> to true... what about providing a way to disable FWNMI ?
When the guest reboots, as per my understanding, FWNMI is still set in
KVM. So a machine check will still cause a guest exit. Upon reboot if
the guest does not call "ibm,nmi-register" then we invoke 0x200
interrupt vector of the guest upon a machine check (because
guest_machine_check_addr is set to -1). If the guest calls
"ibm,nmi-register", then the registered handler is invoked.
And, unfortunately disabling a capability is not supported in KVM.
Regards,
Aravinda
>
>> + rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>> +}
>> +
>> +static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
>> + SpaprMachineState *spapr,
>> + uint32_t token, uint32_t nargs,
>> + target_ulong args,
>> + uint32_t nret, target_ulong rets)
>> +{
>> + if (spapr->guest_machine_check_addr == -1) {
>> + /* NMI register not called */
>> + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>> + } else {
>> + /*
>> + * vCPU issuing "ibm,nmi-interlock" is done with NMI handling,
>> + * hence unset mc_status.
>> + */
>> + spapr->mc_status = -1;
>> + qemu_cond_signal(&spapr->mc_delivery_cond);
>> + rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>> + }
>> +}
>> +
>> static struct rtas_call {
>> const char *name;
>> spapr_rtas_fn fn;
>> @@ -544,6 +586,14 @@ hwaddr spapr_get_rtas_addr(void)
>> return (hwaddr)fdt32_to_cpu(*rtas_data);
>> }
>>
>> +void spapr_fwnmi_register(void)
>> +{
>> + spapr_rtas_register(RTAS_IBM_NMI_REGISTER, "ibm,nmi-register",
>> + rtas_ibm_nmi_register);
>> + spapr_rtas_register(RTAS_IBM_NMI_INTERLOCK, "ibm,nmi-interlock",
>> + rtas_ibm_nmi_interlock);
>> +}
>> +
>> static void core_rtas_register_types(void)
>> {
>> spapr_rtas_register(RTAS_DISPLAY_CHARACTER, "display-character",
>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>> index ffefde7..dada821 100644
>> --- a/include/hw/ppc/spapr.h
>> +++ b/include/hw/ppc/spapr.h
>> @@ -655,8 +655,10 @@ target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong opcode,
>> #define RTAS_IBM_REMOVE_PE_DMA_WINDOW (RTAS_TOKEN_BASE + 0x28)
>> #define RTAS_IBM_RESET_PE_DMA_WINDOW (RTAS_TOKEN_BASE + 0x29)
>> #define RTAS_IBM_SUSPEND_ME (RTAS_TOKEN_BASE + 0x2A)
>> +#define RTAS_IBM_NMI_REGISTER (RTAS_TOKEN_BASE + 0x2B)
>> +#define RTAS_IBM_NMI_INTERLOCK (RTAS_TOKEN_BASE + 0x2C)
>>
>> -#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x2B)
>> +#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x2D)
>>
>> /* RTAS ibm,get-system-parameter token values */
>> #define RTAS_SYSPARM_SPLPAR_CHARACTERISTICS 20
>> @@ -908,4 +910,5 @@ void spapr_check_pagesize(SpaprMachineState *spapr, hwaddr pagesize,
>>
>> void spapr_set_all_lpcrs(target_ulong value, target_ulong mask);
>> hwaddr spapr_get_rtas_addr(void);
>> +void spapr_fwnmi_register(void);
>> #endif /* HW_SPAPR_H */
>>
>
--
Regards,
Aravinda
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v12 2/6] ppc: spapr: Introduce FWNMI capability
2019-09-03 7:22 ` Aravinda Prasad
@ 2019-09-03 7:56 ` Greg Kurz
2019-09-03 8:27 ` Aravinda Prasad
0 siblings, 1 reply; 16+ messages in thread
From: Greg Kurz @ 2019-09-03 7:56 UTC (permalink / raw)
To: Aravinda Prasad; +Cc: aik, qemu-devel, paulus, qemu-ppc, david
On Tue, 3 Sep 2019 12:52:39 +0530
Aravinda Prasad <aravinda@linux.vnet.ibm.com> wrote:
>
>
> On Friday 30 August 2019 07:28 PM, Greg Kurz wrote:
> > On Fri, 30 Aug 2019 14:43:40 +0530
> > Aravinda Prasad <aravinda@linux.vnet.ibm.com> wrote:
> >
> >> Introduce 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 introduces fwnmi-mce capability to
> >> deal 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 | 29 +++++++++++++++++++++++++++++
> >> include/hw/ppc/spapr.h | 4 +++-
> >> target/ppc/kvm.c | 22 ++++++++++++++++++++++
> >> target/ppc/kvm_ppc.h | 11 +++++++++++
> >> 5 files changed, 66 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >> index ea56499..8288e8b 100644
> >> --- a/hw/ppc/spapr.c
> >> +++ b/hw/ppc/spapr.c
> >> @@ -4487,6 +4487,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_OFF;
> >> 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 481dfd2..c11ff87 100644
> >> --- a/hw/ppc/spapr_caps.c
> >> +++ b/hw/ppc/spapr_caps.c
> >> @@ -496,6 +496,25 @@ 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()) {
> >> + /*
> >> + * TCG support may not be correct in some conditions (e.g., in case
> >> + * of software injected faults like duplicate SLBs).
> >> + */
> >> + warn_report("Firmware Assisted Non-Maskable Interrupts not supported in TCG");
> >> + } else if (kvm_enabled() && !kvmppc_has_cap_ppc_fwnmi()) {
> >> + error_setg(errp,
> >> +"Firmware Assisted Non-Maskable Interrupts not supported by KVM, try cap-fwnmi-mce=off");
> >> + }
> >> +}
> >> +
> >> SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> >> [SPAPR_CAP_HTM] = {
> >> .name = "htm",
> >> @@ -595,6 +614,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,
> >> @@ -734,6 +762,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/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> >> index 03111fd..66049ac 100644
> >> --- a/include/hw/ppc/spapr.h
> >> +++ b/include/hw/ppc/spapr.h
> >> @@ -79,8 +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
> >> /* 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 8c5b1f2..c055fc1 100644
> >> --- a/target/ppc/kvm.c
> >> +++ b/target/ppc/kvm.c
> >> @@ -85,6 +85,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;
> >>
> >> @@ -2055,6 +2056,22 @@ void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
> >> }
> >> }
> >>
> >> +int kvmppc_set_fwnmi(void)
> >> +{
> >> + PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
> >> + CPUState *cs = CPU(cpu);
> >> + int ret;
> >> +
> >> + ret = kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0);
> >> + if (ret) {
> >> + error_report("This KVM version does not support FWNMI");
> >> + return ret;
> >> + }
> >> +
> >> + cap_ppc_fwnmi = 1;
> >
> > Hmm... AFAICT the meaning of cap_ppc_fwnmi should just be "KVM knows about
> > FWNMI", not "FWNMI was successfully enabled in KVM". Your v10 used to set
> > cap_ppc_fwnmi in kvm_arch_init() just like the other guys... why this change ?
>
> Even I thought that cap_* is for that, but cap_papr uses it the other
> way. So I decided to use a similar convention for cap_ppc_fwnmi.
>
> In v10, cap_ppc_fwnmi is set in kvm_arch_init() if FWNMI is available in
> KVM and we try to enable FWNMI later in the boot phase when
> rtas_ibm_nmi_register() is called. It's possible that
> SPAPR_CAP_FWNMI_MCE is set and we may fail to enable FWNMI due to errors
> in KVM or QEMU.
>
> To avoid this, in v12, FWNMI is enabled if SPAPR_CAP_FWNMI_MCE is set
> irrespective of rtas_ibm_nmi_register() is called or not. This way we
> fail early if we are not able to enable FWNMI.
>
Ok. Then maybe you should add a comment in kvm_arch_init(), similar to the
cap_papr one ?
> So, when SPAPR_CAP_FWNMI_MCE is set with FWNMI enabled in KVM, a machine
> check error will always cause a guest exit. If the guest has registered
> a handler by calling rtas_ibm_nmi_register(), then we will invoke the
> guest registered handler else we will jump to 0x200 interrupt vector in
> the guest.
>
Yeah, this seems to be handled in patch 4... which I haven't read in detail
yet.
+ if (spapr->guest_machine_check_addr == -1) {
+ /*
+ * This implies that we have hit a machine check between system
+ * reset and "ibm,nmi-register". Fall back to the old machine
+ * check behavior in such cases.
+ */
+ cs->exception_index = POWERPC_EXCP_MCHECK;
+ ppc_cpu_do_interrupt(cs);
+ return;
+ }
Thanks for the clarification.
> With this change we don't need "KVM knows about FWNMI" information. So I
> removed that part and used cap_ppc_fwnmi to track if FWNMI is enabled or
> disabled in KVM. This also simplifies spapr_fwnmi_post_load() to avoid
> enabling FWNMI multiple times (see patch 6).
>
> >
> >> + return ret;
> >> +}
> >> +
> >> int kvmppc_smt_threads(void)
> >> {
> >> return cap_ppc_smt ? cap_ppc_smt : 1;
> >> @@ -2355,6 +2372,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 98bd7d5..2990898 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_set_fwnmi(void);
> >> +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);
> >> @@ -159,6 +161,15 @@ static inline void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
> >> {
> >> }
> >>
> >> +static inline int kvmppc_set_fwnmi(void)
> >> +{
> >
> > Missing return -1;
>
> Hmm.. missed it.
>
> Regards,
> Aravinda
>
> >
> >> +}
> >> +
> >> +static inline bool kvmppc_has_cap_ppc_fwnmi(void)
> >> +{
> >> + return false;
> >> +}
> >> +
> >> static inline int kvmppc_smt_threads(void)
> >> {
> >> return 1;
> >>
> >
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v12 3/6] target/ppc: Handle NMI guest exit
2019-08-30 9:13 ` [Qemu-devel] [PATCH v12 3/6] target/ppc: Handle NMI guest exit Aravinda Prasad
@ 2019-09-03 8:02 ` Greg Kurz
0 siblings, 0 replies; 16+ messages in thread
From: Greg Kurz @ 2019-09-03 8:02 UTC (permalink / raw)
To: Aravinda Prasad; +Cc: aik, qemu-devel, paulus, qemu-ppc, david
On Fri, 30 Aug 2019 14:43:50 +0530
Aravinda Prasad <aravinda@linux.vnet.ibm.com> wrote:
> Memory error such as bit flips that cannot be corrected
> by hardware are passed on to the kernel for handling.
> If the memory address in error belongs to guest then
> the guest kernel is responsible for taking suitable action.
> Patch [1] enhances KVM to exit guest with exit reason
> set to KVM_EXIT_NMI in such cases. This patch handles
> KVM_EXIT_NMI exit.
>
> [1] https://www.spinics.net/lists/kvm-ppc/msg12637.html
> (e20bbd3d and related commits)
>
> Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
Reviewed-by: Greg Kurz <groug@kaod.org>
> hw/ppc/spapr.c | 8 ++++++++
> hw/ppc/spapr_events.c | 24 ++++++++++++++++++++++++
> include/hw/ppc/spapr.h | 10 ++++++++++
> target/ppc/kvm.c | 14 ++++++++++++++
> target/ppc/kvm_ppc.h | 2 ++
> target/ppc/trace-events | 1 +
> 6 files changed, 59 insertions(+)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 8288e8b..76ed988 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1823,6 +1823,12 @@ static void spapr_machine_reset(MachineState *machine)
> first_ppc_cpu->env.gpr[5] = 0;
>
> spapr->cas_reboot = false;
> +
> + spapr->mc_status = -1;
> + spapr->guest_machine_check_addr = -1;
> +
> + /* Signal all vCPUs waiting on this condition */
> + qemu_cond_broadcast(&spapr->mc_delivery_cond);
> }
>
> static void spapr_create_nvram(SpaprMachineState *spapr)
> @@ -3099,6 +3105,8 @@ static void spapr_machine_init(MachineState *machine)
>
> kvmppc_spapr_enable_inkernel_multitce();
> }
> +
> + qemu_cond_init(&spapr->mc_delivery_cond);
> }
>
> static int spapr_kvm_type(MachineState *machine, const char *vm_type)
> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> index 0e4c195..e76c1a7 100644
> --- a/hw/ppc/spapr_events.c
> +++ b/hw/ppc/spapr_events.c
> @@ -40,6 +40,7 @@
> #include "hw/ppc/spapr_drc.h"
> #include "qemu/help_option.h"
> #include "qemu/bcd.h"
> +#include "qemu/main-loop.h"
> #include "hw/ppc/spapr_ovec.h"
> #include <libfdt.h>
>
> @@ -621,6 +622,29 @@ void spapr_hotplug_req_remove_by_count_indexed(SpaprDrcType drc_type,
> RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, &drc_id);
> }
>
> +void spapr_mce_req_event(PowerPCCPU *cpu)
> +{
> + SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> +
> + while (spapr->mc_status != -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) {
> + qemu_system_guest_panicked(NULL);
> + return;
> + }
> + qemu_cond_wait_iothread(&spapr->mc_delivery_cond);
> + /* Meanwhile if the system is reset, then just return */
> + if (spapr->guest_machine_check_addr == -1) {
> + return;
> + }
> + }
> + spapr->mc_status = cpu->vcpu_id;
> +}
> +
> static void check_exception(PowerPCCPU *cpu, SpaprMachineState *spapr,
> uint32_t token, uint32_t nargs,
> target_ulong args,
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 66049ac..99a2966 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -192,6 +192,15 @@ 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.
> + */
> + int mc_status;
> + QemuCond mc_delivery_cond;
> +
> /*< public >*/
> char *kvm_type;
> char *host_model;
> @@ -805,6 +814,7 @@ void spapr_clear_pending_events(SpaprMachineState *spapr);
> int spapr_max_server_number(SpaprMachineState *spapr);
> void spapr_store_hpte(PowerPCCPU *cpu, hwaddr ptex,
> uint64_t pte0, uint64_t pte1);
> +void spapr_mce_req_event(PowerPCCPU *cpu);
>
> /* DRC callbacks. */
> void spapr_core_release(DeviceState *dev);
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index c055fc1..4e282f6 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -1704,6 +1704,11 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
> ret = 0;
> break;
>
> + case KVM_EXIT_NMI:
> + trace_kvm_handle_nmi_exception();
> + ret = kvm_handle_nmi(cpu, run);
> + break;
> +
> default:
> fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason);
> ret = -1;
> @@ -2807,6 +2812,15 @@ int kvm_arch_msi_data_to_gsi(uint32_t data)
> return data & 0xffff;
> }
>
> +int kvm_handle_nmi(PowerPCCPU *cpu, struct kvm_run *run)
> +{
> + cpu_synchronize_state(CPU(cpu));
> +
> + spapr_mce_req_event(cpu);
> +
> + return 0;
> +}
> +
> int kvmppc_enable_hwrng(void)
> {
> if (!kvm_enabled() || !kvm_check_extension(kvm_state, KVM_CAP_PPC_HWRNG)) {
> diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
> index 2990898..173c000 100644
> --- a/target/ppc/kvm_ppc.h
> +++ b/target/ppc/kvm_ppc.h
> @@ -84,6 +84,8 @@ void kvm_check_mmu(PowerPCCPU *cpu, Error **errp);
> void kvmppc_set_reg_ppc_online(PowerPCCPU *cpu, unsigned int online);
> void kvmppc_set_reg_tb_offset(PowerPCCPU *cpu, int64_t tb_offset);
>
> +int kvm_handle_nmi(PowerPCCPU *cpu, struct kvm_run *run);
> +
> #else
>
> static inline uint32_t kvmppc_get_tbfreq(void)
> diff --git a/target/ppc/trace-events b/target/ppc/trace-events
> index 3dc6740..6d15aa9 100644
> --- a/target/ppc/trace-events
> +++ b/target/ppc/trace-events
> @@ -28,3 +28,4 @@ kvm_handle_papr_hcall(void) "handle PAPR hypercall"
> kvm_handle_epr(void) "handle epr"
> kvm_handle_watchdog_expiry(void) "handle watchdog expiry"
> kvm_handle_debug_exception(void) "handle debug exception"
> +kvm_handle_nmi_exception(void) "handle NMI exception"
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v12 2/6] ppc: spapr: Introduce FWNMI capability
2019-09-03 7:56 ` Greg Kurz
@ 2019-09-03 8:27 ` Aravinda Prasad
0 siblings, 0 replies; 16+ messages in thread
From: Aravinda Prasad @ 2019-09-03 8:27 UTC (permalink / raw)
To: Greg Kurz; +Cc: aik, qemu-devel, paulus, qemu-ppc, david
On Tuesday 03 September 2019 01:26 PM, Greg Kurz wrote:
> On Tue, 3 Sep 2019 12:52:39 +0530
> Aravinda Prasad <aravinda@linux.vnet.ibm.com> wrote:
>
>>
>>
>> On Friday 30 August 2019 07:28 PM, Greg Kurz wrote:
>>> On Fri, 30 Aug 2019 14:43:40 +0530
>>> Aravinda Prasad <aravinda@linux.vnet.ibm.com> wrote:
>>>
>>>> Introduce 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 introduces fwnmi-mce capability to
>>>> deal 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 | 29 +++++++++++++++++++++++++++++
>>>> include/hw/ppc/spapr.h | 4 +++-
>>>> target/ppc/kvm.c | 22 ++++++++++++++++++++++
>>>> target/ppc/kvm_ppc.h | 11 +++++++++++
>>>> 5 files changed, 66 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>>> index ea56499..8288e8b 100644
>>>> --- a/hw/ppc/spapr.c
>>>> +++ b/hw/ppc/spapr.c
>>>> @@ -4487,6 +4487,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_OFF;
>>>> 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 481dfd2..c11ff87 100644
>>>> --- a/hw/ppc/spapr_caps.c
>>>> +++ b/hw/ppc/spapr_caps.c
>>>> @@ -496,6 +496,25 @@ 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()) {
>>>> + /*
>>>> + * TCG support may not be correct in some conditions (e.g., in case
>>>> + * of software injected faults like duplicate SLBs).
>>>> + */
>>>> + warn_report("Firmware Assisted Non-Maskable Interrupts not supported in TCG");
>>>> + } else if (kvm_enabled() && !kvmppc_has_cap_ppc_fwnmi()) {
>>>> + error_setg(errp,
>>>> +"Firmware Assisted Non-Maskable Interrupts not supported by KVM, try cap-fwnmi-mce=off");
>>>> + }
>>>> +}
>>>> +
>>>> SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
>>>> [SPAPR_CAP_HTM] = {
>>>> .name = "htm",
>>>> @@ -595,6 +614,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,
>>>> @@ -734,6 +762,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/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>>>> index 03111fd..66049ac 100644
>>>> --- a/include/hw/ppc/spapr.h
>>>> +++ b/include/hw/ppc/spapr.h
>>>> @@ -79,8 +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
>>>> /* 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 8c5b1f2..c055fc1 100644
>>>> --- a/target/ppc/kvm.c
>>>> +++ b/target/ppc/kvm.c
>>>> @@ -85,6 +85,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;
>>>>
>>>> @@ -2055,6 +2056,22 @@ void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
>>>> }
>>>> }
>>>>
>>>> +int kvmppc_set_fwnmi(void)
>>>> +{
>>>> + PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
>>>> + CPUState *cs = CPU(cpu);
>>>> + int ret;
>>>> +
>>>> + ret = kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0);
>>>> + if (ret) {
>>>> + error_report("This KVM version does not support FWNMI");
>>>> + return ret;
>>>> + }
>>>> +
>>>> + cap_ppc_fwnmi = 1;
>>>
>>> Hmm... AFAICT the meaning of cap_ppc_fwnmi should just be "KVM knows about
>>> FWNMI", not "FWNMI was successfully enabled in KVM". Your v10 used to set
>>> cap_ppc_fwnmi in kvm_arch_init() just like the other guys... why this change ?
>>
>> Even I thought that cap_* is for that, but cap_papr uses it the other
>> way. So I decided to use a similar convention for cap_ppc_fwnmi.
>>
>> In v10, cap_ppc_fwnmi is set in kvm_arch_init() if FWNMI is available in
>> KVM and we try to enable FWNMI later in the boot phase when
>> rtas_ibm_nmi_register() is called. It's possible that
>> SPAPR_CAP_FWNMI_MCE is set and we may fail to enable FWNMI due to errors
>> in KVM or QEMU.
>>
>> To avoid this, in v12, FWNMI is enabled if SPAPR_CAP_FWNMI_MCE is set
>> irrespective of rtas_ibm_nmi_register() is called or not. This way we
>> fail early if we are not able to enable FWNMI.
>>
>
> Ok. Then maybe you should add a comment in kvm_arch_init(), similar to the
> cap_papr one ?
Sure..
Regards,
Aravinda
>
>> So, when SPAPR_CAP_FWNMI_MCE is set with FWNMI enabled in KVM, a machine
>> check error will always cause a guest exit. If the guest has registered
>> a handler by calling rtas_ibm_nmi_register(), then we will invoke the
>> guest registered handler else we will jump to 0x200 interrupt vector in
>> the guest.
>>
>
> Yeah, this seems to be handled in patch 4... which I haven't read in detail
> yet.
>
> + if (spapr->guest_machine_check_addr == -1) {
> + /*
> + * This implies that we have hit a machine check between system
> + * reset and "ibm,nmi-register". Fall back to the old machine
> + * check behavior in such cases.
> + */
> + cs->exception_index = POWERPC_EXCP_MCHECK;
> + ppc_cpu_do_interrupt(cs);
> + return;
> + }
>
> Thanks for the clarification.
>
>> With this change we don't need "KVM knows about FWNMI" information. So I
>> removed that part and used cap_ppc_fwnmi to track if FWNMI is enabled or
>> disabled in KVM. This also simplifies spapr_fwnmi_post_load() to avoid
>> enabling FWNMI multiple times (see patch 6).
>>
>>>
>>>> + return ret;
>>>> +}
>>>> +
>>>> int kvmppc_smt_threads(void)
>>>> {
>>>> return cap_ppc_smt ? cap_ppc_smt : 1;
>>>> @@ -2355,6 +2372,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 98bd7d5..2990898 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_set_fwnmi(void);
>>>> +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);
>>>> @@ -159,6 +161,15 @@ static inline void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
>>>> {
>>>> }
>>>>
>>>> +static inline int kvmppc_set_fwnmi(void)
>>>> +{
>>>
>>> Missing return -1;
>>
>> Hmm.. missed it.
>>
>> Regards,
>> Aravinda
>>
>>>
>>>> +}
>>>> +
>>>> +static inline bool kvmppc_has_cap_ppc_fwnmi(void)
>>>> +{
>>>> + return false;
>>>> +}
>>>> +
>>>> static inline int kvmppc_smt_threads(void)
>>>> {
>>>> return 1;
>>>>
>>>
>>
>
--
Regards,
Aravinda
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v12 4/6] target/ppc: Build rtas error log upon an MCE
2019-08-30 9:13 ` [Qemu-devel] [PATCH v12 4/6] target/ppc: Build rtas error log upon an MCE Aravinda Prasad
@ 2019-09-03 10:06 ` Greg Kurz
2019-09-03 10:22 ` Aravinda Prasad
0 siblings, 1 reply; 16+ messages in thread
From: Greg Kurz @ 2019-09-03 10:06 UTC (permalink / raw)
To: Aravinda Prasad; +Cc: aik, qemu-devel, paulus, qemu-ppc, david
On Fri, 30 Aug 2019 14:43:58 +0530
Aravinda Prasad <aravinda@linux.vnet.ibm.com> wrote:
> Upon a machine check exception (MCE) in a guest address space,
> KVM causes a guest exit to enable QEMU to build and pass the
> error to the guest in the PAPR defined rtas error log format.
>
> This patch builds the rtas error log, copies it to the rtas_addr
> and then invokes the guest registered machine check handler. The
> handler in the guest takes suitable action(s) depending on the type
> and criticality of the error. For example, if an error is
> unrecoverable memory corruption in an application inside the
> guest, then the guest kernel sends a SIGBUS to the application.
> For recoverable errors, the guest performs recovery actions and
> logs the error.
>
> Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com>
> ---
> hw/ppc/spapr.c | 13 +++
> hw/ppc/spapr_events.c | 233 ++++++++++++++++++++++++++++++++++++++++++++++++
> hw/ppc/spapr_rtas.c | 26 +++++
> include/hw/ppc/spapr.h | 6 +
> target/ppc/kvm.c | 4 +
> 5 files changed, 279 insertions(+), 3 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 76ed988..9f2e5d2 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2930,6 +2930,19 @@ static void spapr_machine_init(MachineState *machine)
> error_report("Could not get size of LPAR rtas '%s'", filename);
> exit(1);
> }
> +
> + if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == SPAPR_CAP_ON) {
> + /*
> + * Ensure that the rtas image size is less than RTAS_ERROR_LOG_OFFSET
> + * or else the rtas image will be overwritten with the rtas error log
> + * when a machine check exception is encountered.
> + */
> + g_assert(spapr->rtas_size < RTAS_ERROR_LOG_OFFSET);
> +
> + /* Resize rtas blob to accommodate error log */
> + spapr->rtas_size = RTAS_ERROR_LOG_MAX;
> + }
> +
> spapr->rtas_blob = g_malloc(spapr->rtas_size);
> if (load_image_size(filename, spapr->rtas_blob, spapr->rtas_size) < 0) {
> error_report("Could not load LPAR rtas '%s'", filename);
> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> index e76c1a7..8ebb85e 100644
> --- a/hw/ppc/spapr_events.c
> +++ b/hw/ppc/spapr_events.c
> @@ -214,6 +214,106 @@ struct hp_extended_log {
> struct rtas_event_log_v6_hp hp;
> } QEMU_PACKED;
>
> +struct rtas_event_log_v6_mc {
> +#define RTAS_LOG_V6_SECTION_ID_MC 0x4D43 /* MC */
> + struct rtas_event_log_v6_section_header hdr;
> + uint32_t fru_id;
> + uint32_t proc_id;
> + uint8_t error_type;
> +#define RTAS_LOG_V6_MC_TYPE_UE 0
> +#define RTAS_LOG_V6_MC_TYPE_SLB 1
> +#define RTAS_LOG_V6_MC_TYPE_ERAT 2
> +#define RTAS_LOG_V6_MC_TYPE_TLB 4
> +#define RTAS_LOG_V6_MC_TYPE_D_CACHE 5
> +#define RTAS_LOG_V6_MC_TYPE_I_CACHE 7
> + uint8_t sub_err_type;
> +#define RTAS_LOG_V6_MC_UE_INDETERMINATE 0
> +#define RTAS_LOG_V6_MC_UE_IFETCH 1
> +#define RTAS_LOG_V6_MC_UE_PAGE_TABLE_WALK_IFETCH 2
> +#define RTAS_LOG_V6_MC_UE_LOAD_STORE 3
> +#define RTAS_LOG_V6_MC_UE_PAGE_TABLE_WALK_LOAD_STORE 4
> +#define RTAS_LOG_V6_MC_SLB_PARITY 0
> +#define RTAS_LOG_V6_MC_SLB_MULTIHIT 1
> +#define RTAS_LOG_V6_MC_SLB_INDETERMINATE 2
> +#define RTAS_LOG_V6_MC_ERAT_PARITY 1
> +#define RTAS_LOG_V6_MC_ERAT_MULTIHIT 2
> +#define RTAS_LOG_V6_MC_ERAT_INDETERMINATE 3
> +#define RTAS_LOG_V6_MC_TLB_PARITY 1
> +#define RTAS_LOG_V6_MC_TLB_MULTIHIT 2
> +#define RTAS_LOG_V6_MC_TLB_INDETERMINATE 3
> + uint8_t reserved_1[6];
> + uint64_t effective_address;
> + uint64_t logical_address;
> +} QEMU_PACKED;
> +
> +struct mc_extended_log {
> + struct rtas_event_log_v6 v6hdr;
> + struct rtas_event_log_v6_mc mc;
> +} QEMU_PACKED;
> +
> +struct MC_ierror_table {
> + unsigned long srr1_mask;
> + unsigned long srr1_value;
> + bool nip_valid; /* nip is a valid indicator of faulting address */
> + uint8_t error_type;
> + uint8_t error_subtype;
> + unsigned int initiator;
> + unsigned int severity;
> +};
> +
> +static const struct MC_ierror_table mc_ierror_table[] = {
> +{ 0x00000000081c0000, 0x0000000000040000, true,
> + RTAS_LOG_V6_MC_TYPE_UE, RTAS_LOG_V6_MC_UE_IFETCH,
> + RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
> +{ 0x00000000081c0000, 0x0000000000080000, true,
> + RTAS_LOG_V6_MC_TYPE_SLB, RTAS_LOG_V6_MC_SLB_PARITY,
> + RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
> +{ 0x00000000081c0000, 0x00000000000c0000, true,
> + RTAS_LOG_V6_MC_TYPE_SLB, RTAS_LOG_V6_MC_SLB_MULTIHIT,
> + RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
> +{ 0x00000000081c0000, 0x0000000000100000, true,
> + RTAS_LOG_V6_MC_TYPE_ERAT, RTAS_LOG_V6_MC_ERAT_MULTIHIT,
> + RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
> +{ 0x00000000081c0000, 0x0000000000140000, true,
> + RTAS_LOG_V6_MC_TYPE_TLB, RTAS_LOG_V6_MC_TLB_MULTIHIT,
> + RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
> +{ 0x00000000081c0000, 0x0000000000180000, true,
> + RTAS_LOG_V6_MC_TYPE_UE, RTAS_LOG_V6_MC_UE_PAGE_TABLE_WALK_IFETCH,
> + RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
> +{ 0, 0, 0, 0, 0, 0 } };
> +
> +struct MC_derror_table {
> + unsigned long dsisr_value;
> + bool dar_valid; /* dar is a valid indicator of faulting address */
> + uint8_t error_type;
> + uint8_t error_subtype;
> + unsigned int initiator;
> + unsigned int severity;
> +};
> +
> +static const struct MC_derror_table mc_derror_table[] = {
> +{ 0x00008000, false,
> + RTAS_LOG_V6_MC_TYPE_UE, RTAS_LOG_V6_MC_UE_LOAD_STORE,
> + RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
> +{ 0x00004000, true,
> + RTAS_LOG_V6_MC_TYPE_UE, RTAS_LOG_V6_MC_UE_PAGE_TABLE_WALK_LOAD_STORE,
> + RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
> +{ 0x00000800, true,
> + RTAS_LOG_V6_MC_TYPE_ERAT, RTAS_LOG_V6_MC_ERAT_MULTIHIT,
> + RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
> +{ 0x00000400, true,
> + RTAS_LOG_V6_MC_TYPE_TLB, RTAS_LOG_V6_MC_TLB_MULTIHIT,
> + RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
> +{ 0x00000080, true,
> + RTAS_LOG_V6_MC_TYPE_SLB, RTAS_LOG_V6_MC_SLB_MULTIHIT, /* Before PARITY */
> + RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
> +{ 0x00000100, true,
> + RTAS_LOG_V6_MC_TYPE_SLB, RTAS_LOG_V6_MC_SLB_PARITY,
> + RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
> +{ 0, false, 0, 0, 0, 0 } };
> +
> +#define SRR1_MC_LOADSTORE(srr1) ((srr1) & PPC_BIT(42))
> +
> typedef enum EventClass {
> EVENT_CLASS_INTERNAL_ERRORS = 0,
> EVENT_CLASS_EPOW = 1,
> @@ -622,7 +722,136 @@ void spapr_hotplug_req_remove_by_count_indexed(SpaprDrcType drc_type,
> RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, &drc_id);
> }
>
> -void spapr_mce_req_event(PowerPCCPU *cpu)
> +static uint32_t spapr_mce_get_elog_type(PowerPCCPU *cpu, bool recovered,
> + struct mc_extended_log *ext_elog)
> +{
> + int i;
> + CPUPPCState *env = &cpu->env;
> + uint32_t summary;
> + uint64_t dsisr = env->spr[SPR_DSISR];
> +
> + summary = RTAS_LOG_VERSION_6 | RTAS_LOG_OPTIONAL_PART_PRESENT;
> + if (recovered) {
> + summary |= RTAS_LOG_DISPOSITION_FULLY_RECOVERED;
> + } else {
> + summary |= RTAS_LOG_DISPOSITION_NOT_RECOVERED;
> + }
> +
> + if (SRR1_MC_LOADSTORE(env->spr[SPR_SRR1])) {
> + for (i = 0; mc_derror_table[i].dsisr_value; i++) {
> + if (!(dsisr & mc_derror_table[i].dsisr_value)) {
> + continue;
> + }
> +
> + ext_elog->mc.error_type = mc_derror_table[i].error_type;
> + ext_elog->mc.sub_err_type = mc_derror_table[i].error_subtype;
> + if (mc_derror_table[i].dar_valid) {
> + ext_elog->mc.effective_address = cpu_to_be64(env->spr[SPR_DAR]);
> + }
> +
> + summary |= mc_derror_table[i].initiator
> + | mc_derror_table[i].severity;
> +
> + return summary;
> + }
> + } else {
> + for (i = 0; mc_ierror_table[i].srr1_mask; i++) {
> + if ((env->spr[SPR_SRR1] & mc_ierror_table[i].srr1_mask) !=
> + mc_ierror_table[i].srr1_value) {
> + continue;
> + }
> +
> + ext_elog->mc.error_type = mc_ierror_table[i].error_type;
> + ext_elog->mc.sub_err_type = mc_ierror_table[i].error_subtype;
> + if (mc_ierror_table[i].nip_valid) {
> + ext_elog->mc.effective_address = cpu_to_be64(env->nip);
> + }
> +
> + summary |= mc_ierror_table[i].initiator
> + | mc_ierror_table[i].severity;
> +
> + return summary;
> + }
> + }
> +
> + summary |= RTAS_LOG_INITIATOR_CPU;
> + return summary;
> +}
> +
> +static void spapr_mce_dispatch_elog(PowerPCCPU *cpu, bool recovered)
> +{
> + SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> + CPUState *cs = CPU(cpu);
> + uint64_t rtas_addr;
> + CPUPPCState *env = &cpu->env;
> + PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> + target_ulong msr = 0;
> + struct rtas_error_log log;
> + struct mc_extended_log *ext_elog;
> + uint32_t summary;
> +
> + /*
> + * Properly set bits in MSR before we invoke the handler.
> + * SRR0/1, DAR and DSISR are properly set by KVM
> + */
> + if (!(*pcc->interrupts_big_endian)(cpu)) {
> + msr |= (1ULL << MSR_LE);
> + }
> +
> + if (env->msr & (1ULL << MSR_SF)) {
> + msr |= (1ULL << MSR_SF);
> + }
> +
> + msr |= (1ULL << MSR_ME);
> +
> + if (spapr->guest_machine_check_addr == -1) {
> + /*
> + * This implies that we have hit a machine check between system
> + * reset and "ibm,nmi-register". Fall back to the old machine
> + * check behavior in such cases.
> + */
> + cs->exception_index = POWERPC_EXCP_MCHECK;
> + ppc_cpu_do_interrupt(cs);
> + return;
> + }
Does this _really_ belong to this function ? It doesn't need the updated
msr obviously, and this is the case when we don't dispatch the error log...
Shouldn't it even be the very first thing to done by spapr_mce_req_event(),
so that we completely skip all the FWNMI logic if the guest hasn't
registered anything ? If yes, I guess this would better appear in patch 3.
> +
> + ext_elog = g_malloc0(sizeof(*ext_elog));
> + summary = spapr_mce_get_elog_type(cpu, recovered, ext_elog);
> +
> + log.summary = cpu_to_be32(summary);
> + log.extended_length = cpu_to_be32(sizeof(*ext_elog));
> +
> + spapr_init_v6hdr(&ext_elog->v6hdr);
> + ext_elog->mc.hdr.section_id = cpu_to_be16(RTAS_LOG_V6_SECTION_ID_MC);
> + ext_elog->mc.hdr.section_length =
> + cpu_to_be16(sizeof(struct rtas_event_log_v6_mc));
> + ext_elog->mc.hdr.section_version = 1;
> +
> + /* get rtas addr from fdt */
> + rtas_addr = spapr_get_rtas_addr();
> + if (!rtas_addr) {
> + /* Unable to fetch rtas_addr. Hence reset the guest */
> + ppc_cpu_do_system_reset(cs);
> + g_free(ext_elog);
> + return;
> + }
> +
> + stq_be_phys(&address_space_memory, rtas_addr + RTAS_ERROR_LOG_OFFSET,
> + env->gpr[3]);
> + cpu_physical_memory_write(rtas_addr + RTAS_ERROR_LOG_OFFSET +
> + sizeof(env->gpr[3]), &log, sizeof(log));
> + cpu_physical_memory_write(rtas_addr + RTAS_ERROR_LOG_OFFSET +
> + sizeof(env->gpr[3]) + sizeof(log), ext_elog,
> + sizeof(*ext_elog));
> +
> + env->gpr[3] = rtas_addr + RTAS_ERROR_LOG_OFFSET;
> + env->msr = msr;
> + env->nip = spapr->guest_machine_check_addr;
> +
> + g_free(ext_elog);
> +}
> +
> +void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered)
> {
> SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>
> @@ -643,6 +872,8 @@ void spapr_mce_req_event(PowerPCCPU *cpu)
> }
> }
> spapr->mc_status = cpu->vcpu_id;
> +
> + spapr_mce_dispatch_elog(cpu, recovered);
> }
>
> static void check_exception(PowerPCCPU *cpu, SpaprMachineState *spapr,
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index bee3835..d8fb8a8 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -518,6 +518,32 @@ void spapr_load_rtas(SpaprMachineState *spapr, void *fdt, hwaddr addr)
> }
> }
>
> +hwaddr spapr_get_rtas_addr(void)
> +{
> + SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> + int rtas_node;
> + const fdt32_t *rtas_data;
> + void *fdt = spapr->fdt_blob;
> +
> + /* fetch rtas addr from fdt */
> + rtas_node = fdt_path_offset(fdt, "/rtas");
> + if (rtas_node < 0) {
> + return 0;
> + }
> +
> + rtas_data = fdt_getprop(fdt, rtas_node, "linux,rtas-base", NULL);
> + if (!rtas_data) {
> + return 0;
> + }
> +
> + /*
> + * We assume that the OS called RTAS instantiate-rtas, but some other
> + * OS might call RTAS instantiate-rtas-64 instead. This fine as of now
> + * as SLOF only supports 32-bit variant.
> + */
> + return (hwaddr)fdt32_to_cpu(*rtas_data);
> +}
> +
> static void core_rtas_register_types(void)
> {
> spapr_rtas_register(RTAS_DISPLAY_CHARACTER, "display-character",
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 99a2966..ffefde7 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -726,6 +726,9 @@ void spapr_load_rtas(SpaprMachineState *spapr, void *fdt, hwaddr addr);
>
> #define RTAS_ERROR_LOG_MAX 2048
>
> +/* Offset from rtas-base where error log is placed */
> +#define RTAS_ERROR_LOG_OFFSET 0x30
> +
> #define RTAS_EVENT_SCAN_RATE 1
>
> /* This helper should be used to encode interrupt specifiers when the related
> @@ -814,7 +817,7 @@ void spapr_clear_pending_events(SpaprMachineState *spapr);
> int spapr_max_server_number(SpaprMachineState *spapr);
> void spapr_store_hpte(PowerPCCPU *cpu, hwaddr ptex,
> uint64_t pte0, uint64_t pte1);
> -void spapr_mce_req_event(PowerPCCPU *cpu);
> +void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered);
>
> /* DRC callbacks. */
> void spapr_core_release(DeviceState *dev);
> @@ -904,4 +907,5 @@ void spapr_check_pagesize(SpaprMachineState *spapr, hwaddr pagesize,
> #define SPAPR_OV5_XIVE_BOTH 0x80 /* Only to advertise on the platform */
>
> void spapr_set_all_lpcrs(target_ulong value, target_ulong mask);
> +hwaddr spapr_get_rtas_addr(void);
> #endif /* HW_SPAPR_H */
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 4e282f6..68080b9 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -2814,9 +2814,11 @@ int kvm_arch_msi_data_to_gsi(uint32_t data)
>
> int kvm_handle_nmi(PowerPCCPU *cpu, struct kvm_run *run)
> {
> + bool recovered = run->flags & KVM_RUN_PPC_NMI_DISP_FULLY_RECOV;
> +
> cpu_synchronize_state(CPU(cpu));
>
> - spapr_mce_req_event(cpu);
> + spapr_mce_req_event(cpu, recovered);
>
> return 0;
> }
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v12 4/6] target/ppc: Build rtas error log upon an MCE
2019-09-03 10:06 ` Greg Kurz
@ 2019-09-03 10:22 ` Aravinda Prasad
0 siblings, 0 replies; 16+ messages in thread
From: Aravinda Prasad @ 2019-09-03 10:22 UTC (permalink / raw)
To: Greg Kurz; +Cc: aik, qemu-devel, paulus, qemu-ppc, david
On Tuesday 03 September 2019 03:36 PM, Greg Kurz wrote:
> On Fri, 30 Aug 2019 14:43:58 +0530
> Aravinda Prasad <aravinda@linux.vnet.ibm.com> wrote:
>
>> Upon a machine check exception (MCE) in a guest address space,
>> KVM causes a guest exit to enable QEMU to build and pass the
>> error to the guest in the PAPR defined rtas error log format.
>>
>> This patch builds the rtas error log, copies it to the rtas_addr
>> and then invokes the guest registered machine check handler. The
>> handler in the guest takes suitable action(s) depending on the type
>> and criticality of the error. For example, if an error is
>> unrecoverable memory corruption in an application inside the
>> guest, then the guest kernel sends a SIGBUS to the application.
>> For recoverable errors, the guest performs recovery actions and
>> logs the error.
>>
>> Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com>
>> ---
>> hw/ppc/spapr.c | 13 +++
>> hw/ppc/spapr_events.c | 233 ++++++++++++++++++++++++++++++++++++++++++++++++
>> hw/ppc/spapr_rtas.c | 26 +++++
>> include/hw/ppc/spapr.h | 6 +
>> target/ppc/kvm.c | 4 +
>> 5 files changed, 279 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 76ed988..9f2e5d2 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -2930,6 +2930,19 @@ static void spapr_machine_init(MachineState *machine)
>> error_report("Could not get size of LPAR rtas '%s'", filename);
>> exit(1);
>> }
>> +
>> + if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == SPAPR_CAP_ON) {
>> + /*
>> + * Ensure that the rtas image size is less than RTAS_ERROR_LOG_OFFSET
>> + * or else the rtas image will be overwritten with the rtas error log
>> + * when a machine check exception is encountered.
>> + */
>> + g_assert(spapr->rtas_size < RTAS_ERROR_LOG_OFFSET);
>> +
>> + /* Resize rtas blob to accommodate error log */
>> + spapr->rtas_size = RTAS_ERROR_LOG_MAX;
>> + }
>> +
>> spapr->rtas_blob = g_malloc(spapr->rtas_size);
>> if (load_image_size(filename, spapr->rtas_blob, spapr->rtas_size) < 0) {
>> error_report("Could not load LPAR rtas '%s'", filename);
>> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
>> index e76c1a7..8ebb85e 100644
>> --- a/hw/ppc/spapr_events.c
>> +++ b/hw/ppc/spapr_events.c
>> @@ -214,6 +214,106 @@ struct hp_extended_log {
>> struct rtas_event_log_v6_hp hp;
>> } QEMU_PACKED;
>>
>> +struct rtas_event_log_v6_mc {
>> +#define RTAS_LOG_V6_SECTION_ID_MC 0x4D43 /* MC */
>> + struct rtas_event_log_v6_section_header hdr;
>> + uint32_t fru_id;
>> + uint32_t proc_id;
>> + uint8_t error_type;
>> +#define RTAS_LOG_V6_MC_TYPE_UE 0
>> +#define RTAS_LOG_V6_MC_TYPE_SLB 1
>> +#define RTAS_LOG_V6_MC_TYPE_ERAT 2
>> +#define RTAS_LOG_V6_MC_TYPE_TLB 4
>> +#define RTAS_LOG_V6_MC_TYPE_D_CACHE 5
>> +#define RTAS_LOG_V6_MC_TYPE_I_CACHE 7
>> + uint8_t sub_err_type;
>> +#define RTAS_LOG_V6_MC_UE_INDETERMINATE 0
>> +#define RTAS_LOG_V6_MC_UE_IFETCH 1
>> +#define RTAS_LOG_V6_MC_UE_PAGE_TABLE_WALK_IFETCH 2
>> +#define RTAS_LOG_V6_MC_UE_LOAD_STORE 3
>> +#define RTAS_LOG_V6_MC_UE_PAGE_TABLE_WALK_LOAD_STORE 4
>> +#define RTAS_LOG_V6_MC_SLB_PARITY 0
>> +#define RTAS_LOG_V6_MC_SLB_MULTIHIT 1
>> +#define RTAS_LOG_V6_MC_SLB_INDETERMINATE 2
>> +#define RTAS_LOG_V6_MC_ERAT_PARITY 1
>> +#define RTAS_LOG_V6_MC_ERAT_MULTIHIT 2
>> +#define RTAS_LOG_V6_MC_ERAT_INDETERMINATE 3
>> +#define RTAS_LOG_V6_MC_TLB_PARITY 1
>> +#define RTAS_LOG_V6_MC_TLB_MULTIHIT 2
>> +#define RTAS_LOG_V6_MC_TLB_INDETERMINATE 3
>> + uint8_t reserved_1[6];
>> + uint64_t effective_address;
>> + uint64_t logical_address;
>> +} QEMU_PACKED;
>> +
>> +struct mc_extended_log {
>> + struct rtas_event_log_v6 v6hdr;
>> + struct rtas_event_log_v6_mc mc;
>> +} QEMU_PACKED;
>> +
>> +struct MC_ierror_table {
>> + unsigned long srr1_mask;
>> + unsigned long srr1_value;
>> + bool nip_valid; /* nip is a valid indicator of faulting address */
>> + uint8_t error_type;
>> + uint8_t error_subtype;
>> + unsigned int initiator;
>> + unsigned int severity;
>> +};
>> +
>> +static const struct MC_ierror_table mc_ierror_table[] = {
>> +{ 0x00000000081c0000, 0x0000000000040000, true,
>> + RTAS_LOG_V6_MC_TYPE_UE, RTAS_LOG_V6_MC_UE_IFETCH,
>> + RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
>> +{ 0x00000000081c0000, 0x0000000000080000, true,
>> + RTAS_LOG_V6_MC_TYPE_SLB, RTAS_LOG_V6_MC_SLB_PARITY,
>> + RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
>> +{ 0x00000000081c0000, 0x00000000000c0000, true,
>> + RTAS_LOG_V6_MC_TYPE_SLB, RTAS_LOG_V6_MC_SLB_MULTIHIT,
>> + RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
>> +{ 0x00000000081c0000, 0x0000000000100000, true,
>> + RTAS_LOG_V6_MC_TYPE_ERAT, RTAS_LOG_V6_MC_ERAT_MULTIHIT,
>> + RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
>> +{ 0x00000000081c0000, 0x0000000000140000, true,
>> + RTAS_LOG_V6_MC_TYPE_TLB, RTAS_LOG_V6_MC_TLB_MULTIHIT,
>> + RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
>> +{ 0x00000000081c0000, 0x0000000000180000, true,
>> + RTAS_LOG_V6_MC_TYPE_UE, RTAS_LOG_V6_MC_UE_PAGE_TABLE_WALK_IFETCH,
>> + RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
>> +{ 0, 0, 0, 0, 0, 0 } };
>> +
>> +struct MC_derror_table {
>> + unsigned long dsisr_value;
>> + bool dar_valid; /* dar is a valid indicator of faulting address */
>> + uint8_t error_type;
>> + uint8_t error_subtype;
>> + unsigned int initiator;
>> + unsigned int severity;
>> +};
>> +
>> +static const struct MC_derror_table mc_derror_table[] = {
>> +{ 0x00008000, false,
>> + RTAS_LOG_V6_MC_TYPE_UE, RTAS_LOG_V6_MC_UE_LOAD_STORE,
>> + RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
>> +{ 0x00004000, true,
>> + RTAS_LOG_V6_MC_TYPE_UE, RTAS_LOG_V6_MC_UE_PAGE_TABLE_WALK_LOAD_STORE,
>> + RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
>> +{ 0x00000800, true,
>> + RTAS_LOG_V6_MC_TYPE_ERAT, RTAS_LOG_V6_MC_ERAT_MULTIHIT,
>> + RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
>> +{ 0x00000400, true,
>> + RTAS_LOG_V6_MC_TYPE_TLB, RTAS_LOG_V6_MC_TLB_MULTIHIT,
>> + RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
>> +{ 0x00000080, true,
>> + RTAS_LOG_V6_MC_TYPE_SLB, RTAS_LOG_V6_MC_SLB_MULTIHIT, /* Before PARITY */
>> + RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
>> +{ 0x00000100, true,
>> + RTAS_LOG_V6_MC_TYPE_SLB, RTAS_LOG_V6_MC_SLB_PARITY,
>> + RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
>> +{ 0, false, 0, 0, 0, 0 } };
>> +
>> +#define SRR1_MC_LOADSTORE(srr1) ((srr1) & PPC_BIT(42))
>> +
>> typedef enum EventClass {
>> EVENT_CLASS_INTERNAL_ERRORS = 0,
>> EVENT_CLASS_EPOW = 1,
>> @@ -622,7 +722,136 @@ void spapr_hotplug_req_remove_by_count_indexed(SpaprDrcType drc_type,
>> RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, &drc_id);
>> }
>>
>> -void spapr_mce_req_event(PowerPCCPU *cpu)
>> +static uint32_t spapr_mce_get_elog_type(PowerPCCPU *cpu, bool recovered,
>> + struct mc_extended_log *ext_elog)
>> +{
>> + int i;
>> + CPUPPCState *env = &cpu->env;
>> + uint32_t summary;
>> + uint64_t dsisr = env->spr[SPR_DSISR];
>> +
>> + summary = RTAS_LOG_VERSION_6 | RTAS_LOG_OPTIONAL_PART_PRESENT;
>> + if (recovered) {
>> + summary |= RTAS_LOG_DISPOSITION_FULLY_RECOVERED;
>> + } else {
>> + summary |= RTAS_LOG_DISPOSITION_NOT_RECOVERED;
>> + }
>> +
>> + if (SRR1_MC_LOADSTORE(env->spr[SPR_SRR1])) {
>> + for (i = 0; mc_derror_table[i].dsisr_value; i++) {
>> + if (!(dsisr & mc_derror_table[i].dsisr_value)) {
>> + continue;
>> + }
>> +
>> + ext_elog->mc.error_type = mc_derror_table[i].error_type;
>> + ext_elog->mc.sub_err_type = mc_derror_table[i].error_subtype;
>> + if (mc_derror_table[i].dar_valid) {
>> + ext_elog->mc.effective_address = cpu_to_be64(env->spr[SPR_DAR]);
>> + }
>> +
>> + summary |= mc_derror_table[i].initiator
>> + | mc_derror_table[i].severity;
>> +
>> + return summary;
>> + }
>> + } else {
>> + for (i = 0; mc_ierror_table[i].srr1_mask; i++) {
>> + if ((env->spr[SPR_SRR1] & mc_ierror_table[i].srr1_mask) !=
>> + mc_ierror_table[i].srr1_value) {
>> + continue;
>> + }
>> +
>> + ext_elog->mc.error_type = mc_ierror_table[i].error_type;
>> + ext_elog->mc.sub_err_type = mc_ierror_table[i].error_subtype;
>> + if (mc_ierror_table[i].nip_valid) {
>> + ext_elog->mc.effective_address = cpu_to_be64(env->nip);
>> + }
>> +
>> + summary |= mc_ierror_table[i].initiator
>> + | mc_ierror_table[i].severity;
>> +
>> + return summary;
>> + }
>> + }
>> +
>> + summary |= RTAS_LOG_INITIATOR_CPU;
>> + return summary;
>> +}
>> +
>> +static void spapr_mce_dispatch_elog(PowerPCCPU *cpu, bool recovered)
>> +{
>> + SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>> + CPUState *cs = CPU(cpu);
>> + uint64_t rtas_addr;
>> + CPUPPCState *env = &cpu->env;
>> + PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
>> + target_ulong msr = 0;
>> + struct rtas_error_log log;
>> + struct mc_extended_log *ext_elog;
>> + uint32_t summary;
>> +
>> + /*
>> + * Properly set bits in MSR before we invoke the handler.
>> + * SRR0/1, DAR and DSISR are properly set by KVM
>> + */
>> + if (!(*pcc->interrupts_big_endian)(cpu)) {
>> + msr |= (1ULL << MSR_LE);
>> + }
>> +
>> + if (env->msr & (1ULL << MSR_SF)) {
>> + msr |= (1ULL << MSR_SF);
>> + }
>> +
>> + msr |= (1ULL << MSR_ME);
>> +
>> + if (spapr->guest_machine_check_addr == -1) {
>> + /*
>> + * This implies that we have hit a machine check between system
>> + * reset and "ibm,nmi-register". Fall back to the old machine
>> + * check behavior in such cases.
>> + */
>> + cs->exception_index = POWERPC_EXCP_MCHECK;
>> + ppc_cpu_do_interrupt(cs);
>> + return;
>> + }
>
> Does this _really_ belong to this function ? It doesn't need the updated
> msr obviously, and this is the case when we don't dispatch the error log...
> Shouldn't it even be the very first thing to done by spapr_mce_req_event(),
> so that we completely skip all the FWNMI logic if the guest hasn't
> registered anything ? If yes, I guess this would better appear in patch 3.
Yes, true. This can be moved to spapr_mce_req_event() in patch 3.
Regards,
Aravinda
>
>> +
>> + ext_elog = g_malloc0(sizeof(*ext_elog));
>> + summary = spapr_mce_get_elog_type(cpu, recovered, ext_elog);
>> +
>> + log.summary = cpu_to_be32(summary);
>> + log.extended_length = cpu_to_be32(sizeof(*ext_elog));
>> +
>> + spapr_init_v6hdr(&ext_elog->v6hdr);
>> + ext_elog->mc.hdr.section_id = cpu_to_be16(RTAS_LOG_V6_SECTION_ID_MC);
>> + ext_elog->mc.hdr.section_length =
>> + cpu_to_be16(sizeof(struct rtas_event_log_v6_mc));
>> + ext_elog->mc.hdr.section_version = 1;
>> +
>> + /* get rtas addr from fdt */
>> + rtas_addr = spapr_get_rtas_addr();
>> + if (!rtas_addr) {
>> + /* Unable to fetch rtas_addr. Hence reset the guest */
>> + ppc_cpu_do_system_reset(cs);
>> + g_free(ext_elog);
>> + return;
>> + }
>> +
>> + stq_be_phys(&address_space_memory, rtas_addr + RTAS_ERROR_LOG_OFFSET,
>> + env->gpr[3]);
>> + cpu_physical_memory_write(rtas_addr + RTAS_ERROR_LOG_OFFSET +
>> + sizeof(env->gpr[3]), &log, sizeof(log));
>> + cpu_physical_memory_write(rtas_addr + RTAS_ERROR_LOG_OFFSET +
>> + sizeof(env->gpr[3]) + sizeof(log), ext_elog,
>> + sizeof(*ext_elog));
>> +
>> + env->gpr[3] = rtas_addr + RTAS_ERROR_LOG_OFFSET;
>> + env->msr = msr;
>> + env->nip = spapr->guest_machine_check_addr;
>> +
>> + g_free(ext_elog);
>> +}
>> +
>> +void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered)
>> {
>> SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>>
>> @@ -643,6 +872,8 @@ void spapr_mce_req_event(PowerPCCPU *cpu)
>> }
>> }
>> spapr->mc_status = cpu->vcpu_id;
>> +
>> + spapr_mce_dispatch_elog(cpu, recovered);
>> }
>>
>> static void check_exception(PowerPCCPU *cpu, SpaprMachineState *spapr,
>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>> index bee3835..d8fb8a8 100644
>> --- a/hw/ppc/spapr_rtas.c
>> +++ b/hw/ppc/spapr_rtas.c
>> @@ -518,6 +518,32 @@ void spapr_load_rtas(SpaprMachineState *spapr, void *fdt, hwaddr addr)
>> }
>> }
>>
>> +hwaddr spapr_get_rtas_addr(void)
>> +{
>> + SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>> + int rtas_node;
>> + const fdt32_t *rtas_data;
>> + void *fdt = spapr->fdt_blob;
>> +
>> + /* fetch rtas addr from fdt */
>> + rtas_node = fdt_path_offset(fdt, "/rtas");
>> + if (rtas_node < 0) {
>> + return 0;
>> + }
>> +
>> + rtas_data = fdt_getprop(fdt, rtas_node, "linux,rtas-base", NULL);
>> + if (!rtas_data) {
>> + return 0;
>> + }
>> +
>> + /*
>> + * We assume that the OS called RTAS instantiate-rtas, but some other
>> + * OS might call RTAS instantiate-rtas-64 instead. This fine as of now
>> + * as SLOF only supports 32-bit variant.
>> + */
>> + return (hwaddr)fdt32_to_cpu(*rtas_data);
>> +}
>> +
>> static void core_rtas_register_types(void)
>> {
>> spapr_rtas_register(RTAS_DISPLAY_CHARACTER, "display-character",
>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>> index 99a2966..ffefde7 100644
>> --- a/include/hw/ppc/spapr.h
>> +++ b/include/hw/ppc/spapr.h
>> @@ -726,6 +726,9 @@ void spapr_load_rtas(SpaprMachineState *spapr, void *fdt, hwaddr addr);
>>
>> #define RTAS_ERROR_LOG_MAX 2048
>>
>> +/* Offset from rtas-base where error log is placed */
>> +#define RTAS_ERROR_LOG_OFFSET 0x30
>> +
>> #define RTAS_EVENT_SCAN_RATE 1
>>
>> /* This helper should be used to encode interrupt specifiers when the related
>> @@ -814,7 +817,7 @@ void spapr_clear_pending_events(SpaprMachineState *spapr);
>> int spapr_max_server_number(SpaprMachineState *spapr);
>> void spapr_store_hpte(PowerPCCPU *cpu, hwaddr ptex,
>> uint64_t pte0, uint64_t pte1);
>> -void spapr_mce_req_event(PowerPCCPU *cpu);
>> +void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered);
>>
>> /* DRC callbacks. */
>> void spapr_core_release(DeviceState *dev);
>> @@ -904,4 +907,5 @@ void spapr_check_pagesize(SpaprMachineState *spapr, hwaddr pagesize,
>> #define SPAPR_OV5_XIVE_BOTH 0x80 /* Only to advertise on the platform */
>>
>> void spapr_set_all_lpcrs(target_ulong value, target_ulong mask);
>> +hwaddr spapr_get_rtas_addr(void);
>> #endif /* HW_SPAPR_H */
>> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
>> index 4e282f6..68080b9 100644
>> --- a/target/ppc/kvm.c
>> +++ b/target/ppc/kvm.c
>> @@ -2814,9 +2814,11 @@ int kvm_arch_msi_data_to_gsi(uint32_t data)
>>
>> int kvm_handle_nmi(PowerPCCPU *cpu, struct kvm_run *run)
>> {
>> + bool recovered = run->flags & KVM_RUN_PPC_NMI_DISP_FULLY_RECOV;
>> +
>> cpu_synchronize_state(CPU(cpu));
>>
>> - spapr_mce_req_event(cpu);
>> + spapr_mce_req_event(cpu, recovered);
>>
>> return 0;
>> }
>>
>
--
Regards,
Aravinda
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2019-09-03 10:27 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-30 9:13 [Qemu-devel] [PATCH v12 0/6] target-ppc/spapr: Add FWNMI support in QEMU for PowerKVM guests Aravinda Prasad
2019-08-30 9:13 ` [Qemu-devel] [PATCH v12 1/6] Wrapper function to wait on condition for the main loop mutex Aravinda Prasad
2019-08-30 9:13 ` [Qemu-devel] [PATCH v12 2/6] ppc: spapr: Introduce FWNMI capability Aravinda Prasad
2019-08-30 13:58 ` Greg Kurz
2019-09-03 7:22 ` Aravinda Prasad
2019-09-03 7:56 ` Greg Kurz
2019-09-03 8:27 ` Aravinda Prasad
2019-08-30 9:13 ` [Qemu-devel] [PATCH v12 3/6] target/ppc: Handle NMI guest exit Aravinda Prasad
2019-09-03 8:02 ` Greg Kurz
2019-08-30 9:13 ` [Qemu-devel] [PATCH v12 4/6] target/ppc: Build rtas error log upon an MCE Aravinda Prasad
2019-09-03 10:06 ` Greg Kurz
2019-09-03 10:22 ` Aravinda Prasad
2019-08-30 9:14 ` [Qemu-devel] [PATCH v12 5/6] ppc: spapr: Handle "ibm, nmi-register" and "ibm, nmi-interlock" RTAS calls Aravinda Prasad
2019-08-30 17:08 ` Greg Kurz
2019-09-03 7:38 ` Aravinda Prasad
2019-08-30 9:14 ` [Qemu-devel] [PATCH v12 6/6] migration: Include migration support for machine check handling Aravinda Prasad
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).