Re: [Qemu-devel] [Qemu-ppc] [PATCH v7 3/6] target/ppc: Handle NMI guest exit

[Greg Kurz]

On Fri, 5 Apr 2019 16:04:27 +1100
Alexey Kardashevskiy <address@hidden> wrote:

> On 05/04/2019 10:17, David Gibson wrote:
> > On Thu, Apr 04, 2019 at 02:40:45PM +0530, Aravinda Prasad wrote:  
> >>
> >>
> >> On Monday 25 March 2019 11:52 AM, David Gibson wrote:  
> >>> On Fri, Mar 22, 2019 at 12:03:58PM +0530, Aravinda Prasad 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 <address@hidden>
> >>>> ---
> >>>>  hw/ppc/spapr_events.c  |   22 ++++++++++++++++++++++
> >>>>  include/hw/ppc/spapr.h |    1 +
> >>>>  target/ppc/kvm.c       |   16 ++++++++++++++++
> >>>>  target/ppc/kvm_ppc.h   |    2 ++
> >>>>  4 files changed, 41 insertions(+)
> >>>>  
> >>
> >> [...]
> >>  
> >>>> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> >>>> index 2427c8e..a593448 100644
> >>>> --- a/target/ppc/kvm.c
> >>>> +++ b/target/ppc/kvm.c
> >>>> @@ -1738,6 +1738,11 @@ int kvm_arch_handle_exit(CPUState *cs, struct 
> >>>> kvm_run *run)
> >>>>          ret = 0;
> >>>>          break;
> >>>>  
> >>>> +    case KVM_EXIT_NMI:
> >>>> +        DPRINTF("handle NMI exception\n");  
> >>>
> >>> tracepoints are generally preferred to new DPRINTFs.  
> >>
> >> I see DPRINTFs used in all other exit reasons in this function. Do you
> >> want me to change this particular exit case to tracepoints? I think it
> >> is better to keep this DPRINTF as of now and change all the DPRINTFs to
> >> tracepoints in a separate patch set.  
> > 
> > Ah, good point.   
> 
> imho not. The kvm.c already knows about traces (there are two) and even
> if every other trace in kvm_arch_handle_exit() is DPRINTF (enabled all
> at once), having at least one which can be enabled without QEMU
> recompile and separately from the others is a small but nice bonus
> before someone gets rid of DPRINTF.
> 

Done.

https://patchwork.ozlabs.org/project/qemu-devel/list/?series=101141

> 
> > Tracepoints are generally preferred, but since
> > DPRINTFs are in use here, stick with that (at some point it would be
> > good to change the whole file, but that's out of scope here).  
> 
>