Re: [PATCH 2/2] powerpc/64s: Better printing of machine check info for guest MCEs

2019-02-20 Thread Mahesh J Salgaonkar
On 2019-02-20 12:06:23 Wed, Paul Mackerras wrote:
> This adds an "in_guest" parameter to machine_check_print_event_info()
> so that we can avoid trying to translate guest NIP values into
> symbolic form using the host kernel's symbol table.
> 
> Signed-off-by: Paul Mackerras 
> ---
>  arch/powerpc/include/asm/mce.h| 2 +-
>  arch/powerpc/kernel/mce.c | 8 +---
>  arch/powerpc/kvm/book3s_hv.c  | 4 ++--
>  arch/powerpc/platforms/powernv/opal.c | 2 +-
>  4 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
> index a8b8903..17996bc 100644
> --- a/arch/powerpc/include/asm/mce.h
> +++ b/arch/powerpc/include/asm/mce.h
> @@ -209,7 +209,7 @@ extern int get_mce_event(struct machine_check_event *mce, 
> bool release);
>  extern void release_mce_event(void);
>  extern void machine_check_queue_event(void);
>  extern void machine_check_print_event_info(struct machine_check_event *evt,
> -bool user_mode);
> +bool user_mode, bool in_guest);
>  #ifdef CONFIG_PPC_BOOK3S_64
>  void flush_and_reload_slb(void);
>  #endif /* CONFIG_PPC_BOOK3S_64 */
> diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
> index bd933a7..d01b690 100644
> --- a/arch/powerpc/kernel/mce.c
> +++ b/arch/powerpc/kernel/mce.c
> @@ -301,13 +301,13 @@ static void machine_check_process_queued_event(struct 
> irq_work *work)
>   while (__this_cpu_read(mce_queue_count) > 0) {
>   index = __this_cpu_read(mce_queue_count) - 1;
>   evt = this_cpu_ptr(_event_queue[index]);
> - machine_check_print_event_info(evt, false);
> + machine_check_print_event_info(evt, false, false);
>   __this_cpu_dec(mce_queue_count);
>   }
>  }
> 
>  void machine_check_print_event_info(struct machine_check_event *evt,
> - bool user_mode)
> + bool user_mode, bool in_guest)
>  {
>   const char *level, *sevstr, *subtype;
>   static const char *mc_ue_types[] = {
> @@ -387,7 +387,9 @@ void machine_check_print_event_info(struct 
> machine_check_event *evt,
>  evt->disposition == MCE_DISPOSITION_RECOVERED ?
>  "Recovered" : "Not recovered");
> 
> - if (user_mode) {
> + if (in_guest) {
> + printk("%s  Guest NIP: %016llx\n", evt->srr0);

Missing 'level' argument. This should be:
printk("%s  Guest NIP: %016llx\n", level, evt->srr0);

Rest looks fine to me.

Reviewed-by: Mahesh Salgaonkar 

Thanks,
-Mahesh.



Re: [PATCH 2/2] powerpc/64s: Better printing of machine check info for guest MCEs

2019-02-20 Thread Aravinda Prasad



On Wednesday 20 February 2019 06:36 AM, Paul Mackerras wrote:
> This adds an "in_guest" parameter to machine_check_print_event_info()
> so that we can avoid trying to translate guest NIP values into
> symbolic form using the host kernel's symbol table.

Reviewed-by: Aravinda Prasad 

Regards,
Aravinda

> 
> Signed-off-by: Paul Mackerras 
> ---
>  arch/powerpc/include/asm/mce.h| 2 +-
>  arch/powerpc/kernel/mce.c | 8 +---
>  arch/powerpc/kvm/book3s_hv.c  | 4 ++--
>  arch/powerpc/platforms/powernv/opal.c | 2 +-
>  4 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
> index a8b8903..17996bc 100644
> --- a/arch/powerpc/include/asm/mce.h
> +++ b/arch/powerpc/include/asm/mce.h
> @@ -209,7 +209,7 @@ extern int get_mce_event(struct machine_check_event *mce, 
> bool release);
>  extern void release_mce_event(void);
>  extern void machine_check_queue_event(void);
>  extern void machine_check_print_event_info(struct machine_check_event *evt,
> -bool user_mode);
> +bool user_mode, bool in_guest);
>  #ifdef CONFIG_PPC_BOOK3S_64
>  void flush_and_reload_slb(void);
>  #endif /* CONFIG_PPC_BOOK3S_64 */
> diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
> index bd933a7..d01b690 100644
> --- a/arch/powerpc/kernel/mce.c
> +++ b/arch/powerpc/kernel/mce.c
> @@ -301,13 +301,13 @@ static void machine_check_process_queued_event(struct 
> irq_work *work)
>   while (__this_cpu_read(mce_queue_count) > 0) {
>   index = __this_cpu_read(mce_queue_count) - 1;
>   evt = this_cpu_ptr(_event_queue[index]);
> - machine_check_print_event_info(evt, false);
> + machine_check_print_event_info(evt, false, false);
>   __this_cpu_dec(mce_queue_count);
>   }
>  }
> 
>  void machine_check_print_event_info(struct machine_check_event *evt,
> - bool user_mode)
> + bool user_mode, bool in_guest)
>  {
>   const char *level, *sevstr, *subtype;
>   static const char *mc_ue_types[] = {
> @@ -387,7 +387,9 @@ void machine_check_print_event_info(struct 
> machine_check_event *evt,
>  evt->disposition == MCE_DISPOSITION_RECOVERED ?
>  "Recovered" : "Not recovered");
> 
> - if (user_mode) {
> + if (in_guest) {
> + printk("%s  Guest NIP: %016llx\n", evt->srr0);
> + } else if (user_mode) {
>   printk("%s  NIP: [%016llx] PID: %d Comm: %s\n", level,
>   evt->srr0, current->pid, current->comm);
>   } else {
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index d8bf05a..81cba4b 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -1216,7 +1216,7 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, 
> struct kvm_vcpu *vcpu,
>   break;
>   case BOOK3S_INTERRUPT_MACHINE_CHECK:
>   /* Print the MCE event to host console. */
> - machine_check_print_event_info(>arch.mce_evt, false);
> + machine_check_print_event_info(>arch.mce_evt, false, 
> true);
> 
>   /*
>* If the guest can do FWNMI, exit to userspace so it can
> @@ -1406,7 +1406,7 @@ static int kvmppc_handle_nested_exit(struct kvm_run 
> *run, struct kvm_vcpu *vcpu)
>   /* Pass the machine check to the L1 guest */
>   r = RESUME_HOST;
>   /* Print the MCE event to host console. */
> - machine_check_print_event_info(>arch.mce_evt, false);
> + machine_check_print_event_info(>arch.mce_evt, false, 
> true);
>   break;
>   /*
>* We get these next two if the guest accesses a page which it thinks
> diff --git a/arch/powerpc/platforms/powernv/opal.c 
> b/arch/powerpc/platforms/powernv/opal.c
> index 79586f1..05c85be 100644
> --- a/arch/powerpc/platforms/powernv/opal.c
> +++ b/arch/powerpc/platforms/powernv/opal.c
> @@ -587,7 +587,7 @@ int opal_machine_check(struct pt_regs *regs)
>  evt.version);
>   return 0;
>   }
> - machine_check_print_event_info(, user_mode(regs));
> + machine_check_print_event_info(, user_mode(regs), false);
> 
>   if (opal_recover_mce(regs, ))
>   return 1;
> 

-- 
Regards,
Aravinda



[PATCH 2/2] powerpc/64s: Better printing of machine check info for guest MCEs

2019-02-19 Thread Paul Mackerras
This adds an "in_guest" parameter to machine_check_print_event_info()
so that we can avoid trying to translate guest NIP values into
symbolic form using the host kernel's symbol table.

Signed-off-by: Paul Mackerras 
---
 arch/powerpc/include/asm/mce.h| 2 +-
 arch/powerpc/kernel/mce.c | 8 +---
 arch/powerpc/kvm/book3s_hv.c  | 4 ++--
 arch/powerpc/platforms/powernv/opal.c | 2 +-
 4 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
index a8b8903..17996bc 100644
--- a/arch/powerpc/include/asm/mce.h
+++ b/arch/powerpc/include/asm/mce.h
@@ -209,7 +209,7 @@ extern int get_mce_event(struct machine_check_event *mce, 
bool release);
 extern void release_mce_event(void);
 extern void machine_check_queue_event(void);
 extern void machine_check_print_event_info(struct machine_check_event *evt,
-  bool user_mode);
+  bool user_mode, bool in_guest);
 #ifdef CONFIG_PPC_BOOK3S_64
 void flush_and_reload_slb(void);
 #endif /* CONFIG_PPC_BOOK3S_64 */
diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
index bd933a7..d01b690 100644
--- a/arch/powerpc/kernel/mce.c
+++ b/arch/powerpc/kernel/mce.c
@@ -301,13 +301,13 @@ static void machine_check_process_queued_event(struct 
irq_work *work)
while (__this_cpu_read(mce_queue_count) > 0) {
index = __this_cpu_read(mce_queue_count) - 1;
evt = this_cpu_ptr(_event_queue[index]);
-   machine_check_print_event_info(evt, false);
+   machine_check_print_event_info(evt, false, false);
__this_cpu_dec(mce_queue_count);
}
 }
 
 void machine_check_print_event_info(struct machine_check_event *evt,
-   bool user_mode)
+   bool user_mode, bool in_guest)
 {
const char *level, *sevstr, *subtype;
static const char *mc_ue_types[] = {
@@ -387,7 +387,9 @@ void machine_check_print_event_info(struct 
machine_check_event *evt,
   evt->disposition == MCE_DISPOSITION_RECOVERED ?
   "Recovered" : "Not recovered");
 
-   if (user_mode) {
+   if (in_guest) {
+   printk("%s  Guest NIP: %016llx\n", evt->srr0);
+   } else if (user_mode) {
printk("%s  NIP: [%016llx] PID: %d Comm: %s\n", level,
evt->srr0, current->pid, current->comm);
} else {
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index d8bf05a..81cba4b 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1216,7 +1216,7 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, 
struct kvm_vcpu *vcpu,
break;
case BOOK3S_INTERRUPT_MACHINE_CHECK:
/* Print the MCE event to host console. */
-   machine_check_print_event_info(>arch.mce_evt, false);
+   machine_check_print_event_info(>arch.mce_evt, false, 
true);
 
/*
 * If the guest can do FWNMI, exit to userspace so it can
@@ -1406,7 +1406,7 @@ static int kvmppc_handle_nested_exit(struct kvm_run *run, 
struct kvm_vcpu *vcpu)
/* Pass the machine check to the L1 guest */
r = RESUME_HOST;
/* Print the MCE event to host console. */
-   machine_check_print_event_info(>arch.mce_evt, false);
+   machine_check_print_event_info(>arch.mce_evt, false, 
true);
break;
/*
 * We get these next two if the guest accesses a page which it thinks
diff --git a/arch/powerpc/platforms/powernv/opal.c 
b/arch/powerpc/platforms/powernv/opal.c
index 79586f1..05c85be 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -587,7 +587,7 @@ int opal_machine_check(struct pt_regs *regs)
   evt.version);
return 0;
}
-   machine_check_print_event_info(, user_mode(regs));
+   machine_check_print_event_info(, user_mode(regs), false);
 
if (opal_recover_mce(regs, ))
return 1;
-- 
2.7.4