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 <aravi...@linux.vnet.ibm.com>
>>>> ---
>>>> 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.
> 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).
--
Alexey Kardashevskiy
IBM OzLabs, LTC Team
e-mail: a...@linux.ibm.com
