Re: [PATCH 1/2] KVM: PPC: Book3S HV: Simplify machine check handling
On 2019-02-20 12:05:50 Wed, Paul Mackerras wrote: > This makes the handling of machine check interrupts that occur inside > a guest simpler and more robust, with less done in assembler code and > in real mode. > > Now, when a machine check occurs inside a guest, we always get the > machine check event struct and put a copy in the vcpu struct for the > vcpu where the machine check occurred. We no longer call > machine_check_queue_event() from kvmppc_realmode_mc_power7(), because > on POWER8, when a vcpu is running on an offline secondary thread and > we call machine_check_queue_event(), that calls irq_work_queue(), > which doesn't work because the CPU is offline, but instead triggers > the WARN_ON(lazy_irq_pending()) in pnv_smp_cpu_kill_self() (which > fires again and again because nothing clears the condition). > > All that machine_check_queue_event() actually does is to cause the > event to be printed to the console. For a machine check occurring in > the guest, we now print the event in kvmppc_handle_exit_hv() > instead. > > The assembly code at label machine_check_realmode now just calls C > code and then continues exiting the guest. We no longer either > synthesize a machine check for the guest in assembly code or return > to the guest without a machine check. > > The code in kvmppc_handle_exit_hv() is extended to handle the case > where the guest is not FWNMI-capable. In that case we now always > synthesize a machine check interrupt for the guest. Previously, if > the host thinks it has recovered the machine check fully, it would > return to the guest without any notification that the machine check > had occurred. If the machine check was caused by some action of the > guest (such as creating duplicate SLB entries), it is much better to > tell the guest that it has caused a problem. Therefore we now always > generate a machine check interrupt for guests that are not > FWNMI-capable. Looks good to me. Reviewed-by: Mahesh Salgaonkar Thanks, -Mahesh.
Re: [PATCH 1/2] KVM: PPC: Book3S HV: Simplify machine check handling
On Wednesday 20 February 2019 06:35 AM, Paul Mackerras wrote:
> This makes the handling of machine check interrupts that occur inside
> a guest simpler and more robust, with less done in assembler code and
> in real mode.
>
> Now, when a machine check occurs inside a guest, we always get the
> machine check event struct and put a copy in the vcpu struct for the
> vcpu where the machine check occurred. We no longer call
> machine_check_queue_event() from kvmppc_realmode_mc_power7(), because
> on POWER8, when a vcpu is running on an offline secondary thread and
> we call machine_check_queue_event(), that calls irq_work_queue(),
> which doesn't work because the CPU is offline, but instead triggers
> the WARN_ON(lazy_irq_pending()) in pnv_smp_cpu_kill_self() (which
> fires again and again because nothing clears the condition).
>
> All that machine_check_queue_event() actually does is to cause the
> event to be printed to the console. For a machine check occurring in
> the guest, we now print the event in kvmppc_handle_exit_hv()
> instead.
>
> The assembly code at label machine_check_realmode now just calls C
> code and then continues exiting the guest. We no longer either
> synthesize a machine check for the guest in assembly code or return
> to the guest without a machine check.
>
> The code in kvmppc_handle_exit_hv() is extended to handle the case
> where the guest is not FWNMI-capable. In that case we now always
> synthesize a machine check interrupt for the guest. Previously, if
> the host thinks it has recovered the machine check fully, it would
> return to the guest without any notification that the machine check
> had occurred. If the machine check was caused by some action of the
> guest (such as creating duplicate SLB entries), it is much better to
> tell the guest that it has caused a problem. Therefore we now always
> generate a machine check interrupt for guests that are not
> FWNMI-capable.
>
> Signed-off-by: Paul Mackerras
> ---
> arch/powerpc/include/asm/kvm_ppc.h | 3 +-
> arch/powerpc/kvm/book3s.c | 7 +
> arch/powerpc/kvm/book3s_hv.c| 18 +--
> arch/powerpc/kvm/book3s_hv_ras.c| 56
> +
> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 40 ++-
> 5 files changed, 42 insertions(+), 82 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_ppc.h
> b/arch/powerpc/include/asm/kvm_ppc.h
> index b3bf4f6..d283d31 100644
> --- a/arch/powerpc/include/asm/kvm_ppc.h
> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> @@ -143,6 +143,7 @@ extern void kvmppc_core_vcpu_put(struct kvm_vcpu *vcpu);
>
> extern int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu);
> extern int kvmppc_core_pending_dec(struct kvm_vcpu *vcpu);
> +extern void kvmppc_core_queue_machine_check(struct kvm_vcpu *vcpu, ulong
> flags);
> extern void kvmppc_core_queue_program(struct kvm_vcpu *vcpu, ulong flags);
> extern void kvmppc_core_queue_fpunavail(struct kvm_vcpu *vcpu);
> extern void kvmppc_core_queue_vec_unavail(struct kvm_vcpu *vcpu);
> @@ -646,7 +647,7 @@ long int kvmppc_rm_h_confer(struct kvm_vcpu *vcpu, int
> target,
> unsigned int yield_count);
> long kvmppc_h_random(struct kvm_vcpu *vcpu);
> void kvmhv_commence_exit(int trap);
> -long kvmppc_realmode_machine_check(struct kvm_vcpu *vcpu);
> +void kvmppc_realmode_machine_check(struct kvm_vcpu *vcpu);
> void kvmppc_subcore_enter_guest(void);
> void kvmppc_subcore_exit_guest(void);
> long kvmppc_realmode_hmi_handler(void);
> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
> index 22a46c6..10c5579 100644
> --- a/arch/powerpc/kvm/book3s.c
> +++ b/arch/powerpc/kvm/book3s.c
> @@ -195,6 +195,13 @@ void kvmppc_book3s_queue_irqprio(struct kvm_vcpu *vcpu,
> unsigned int vec)
> }
> EXPORT_SYMBOL_GPL(kvmppc_book3s_queue_irqprio);
>
> +void kvmppc_core_queue_machine_check(struct kvm_vcpu *vcpu, ulong flags)
> +{
> + /* might as well deliver this straight away */
> + kvmppc_inject_interrupt(vcpu, BOOK3S_INTERRUPT_MACHINE_CHECK, flags);
> +}
> +EXPORT_SYMBOL_GPL(kvmppc_core_queue_machine_check);
> +
> void kvmppc_core_queue_program(struct kvm_vcpu *vcpu, ulong flags)
> {
> /* might as well deliver this straight away */
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 1860c0b..d8bf05a 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -1215,6 +1215,22 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run,
> struct kvm_vcpu *vcpu,
> r = RESUME_GUEST;
> break;
> case BOOK3S_INTERRUPT_MACHINE_CHECK:
> + /* Print the MCE event to host console. */
> + machine_check_print_event_info(>arch.mce_evt, false);
> +
> + /*
> + * If the guest can do FWNMI, exit to userspace so it can
> + * deliver a FWNMI to the guest.
> + * Otherwise
[PATCH 1/2] KVM: PPC: Book3S HV: Simplify machine check handling
This makes the handling of machine check interrupts that occur inside
a guest simpler and more robust, with less done in assembler code and
in real mode.
Now, when a machine check occurs inside a guest, we always get the
machine check event struct and put a copy in the vcpu struct for the
vcpu where the machine check occurred. We no longer call
machine_check_queue_event() from kvmppc_realmode_mc_power7(), because
on POWER8, when a vcpu is running on an offline secondary thread and
we call machine_check_queue_event(), that calls irq_work_queue(),
which doesn't work because the CPU is offline, but instead triggers
the WARN_ON(lazy_irq_pending()) in pnv_smp_cpu_kill_self() (which
fires again and again because nothing clears the condition).
All that machine_check_queue_event() actually does is to cause the
event to be printed to the console. For a machine check occurring in
the guest, we now print the event in kvmppc_handle_exit_hv()
instead.
The assembly code at label machine_check_realmode now just calls C
code and then continues exiting the guest. We no longer either
synthesize a machine check for the guest in assembly code or return
to the guest without a machine check.
The code in kvmppc_handle_exit_hv() is extended to handle the case
where the guest is not FWNMI-capable. In that case we now always
synthesize a machine check interrupt for the guest. Previously, if
the host thinks it has recovered the machine check fully, it would
return to the guest without any notification that the machine check
had occurred. If the machine check was caused by some action of the
guest (such as creating duplicate SLB entries), it is much better to
tell the guest that it has caused a problem. Therefore we now always
generate a machine check interrupt for guests that are not
FWNMI-capable.
Signed-off-by: Paul Mackerras
---
arch/powerpc/include/asm/kvm_ppc.h | 3 +-
arch/powerpc/kvm/book3s.c | 7 +
arch/powerpc/kvm/book3s_hv.c| 18 +--
arch/powerpc/kvm/book3s_hv_ras.c| 56 +
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 40 ++-
5 files changed, 42 insertions(+), 82 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_ppc.h
b/arch/powerpc/include/asm/kvm_ppc.h
index b3bf4f6..d283d31 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -143,6 +143,7 @@ extern void kvmppc_core_vcpu_put(struct kvm_vcpu *vcpu);
extern int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu);
extern int kvmppc_core_pending_dec(struct kvm_vcpu *vcpu);
+extern void kvmppc_core_queue_machine_check(struct kvm_vcpu *vcpu, ulong
flags);
extern void kvmppc_core_queue_program(struct kvm_vcpu *vcpu, ulong flags);
extern void kvmppc_core_queue_fpunavail(struct kvm_vcpu *vcpu);
extern void kvmppc_core_queue_vec_unavail(struct kvm_vcpu *vcpu);
@@ -646,7 +647,7 @@ long int kvmppc_rm_h_confer(struct kvm_vcpu *vcpu, int
target,
unsigned int yield_count);
long kvmppc_h_random(struct kvm_vcpu *vcpu);
void kvmhv_commence_exit(int trap);
-long kvmppc_realmode_machine_check(struct kvm_vcpu *vcpu);
+void kvmppc_realmode_machine_check(struct kvm_vcpu *vcpu);
void kvmppc_subcore_enter_guest(void);
void kvmppc_subcore_exit_guest(void);
long kvmppc_realmode_hmi_handler(void);
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index 22a46c6..10c5579 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -195,6 +195,13 @@ void kvmppc_book3s_queue_irqprio(struct kvm_vcpu *vcpu,
unsigned int vec)
}
EXPORT_SYMBOL_GPL(kvmppc_book3s_queue_irqprio);
+void kvmppc_core_queue_machine_check(struct kvm_vcpu *vcpu, ulong flags)
+{
+ /* might as well deliver this straight away */
+ kvmppc_inject_interrupt(vcpu, BOOK3S_INTERRUPT_MACHINE_CHECK, flags);
+}
+EXPORT_SYMBOL_GPL(kvmppc_core_queue_machine_check);
+
void kvmppc_core_queue_program(struct kvm_vcpu *vcpu, ulong flags)
{
/* might as well deliver this straight away */
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 1860c0b..d8bf05a 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1215,6 +1215,22 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run,
struct kvm_vcpu *vcpu,
r = RESUME_GUEST;
break;
case BOOK3S_INTERRUPT_MACHINE_CHECK:
+ /* Print the MCE event to host console. */
+ machine_check_print_event_info(>arch.mce_evt, false);
+
+ /*
+* If the guest can do FWNMI, exit to userspace so it can
+* deliver a FWNMI to the guest.
+* Otherwise we synthesize a machine check for the guest
+* so that it knows that the machine check occurred.
+*/
+ if (!vcpu->kvm->arch.fwnmi_enabled) {
+ ulong flags = vcpu->arch.shregs.msr
