Subject: Re: [RFC PATCH 07/11] kvm: powerpc: book3s: pr: move PR related tracepoints to a separate header

From: "Aneesh Kumar K.V" <[email-masked]>
Date: Mon, 30 Sep 2013 21:23:11 +0530

Alexander Graf <[email-masked]> writes:

> On 09/30/2013 02:57 PM, Aneesh Kumar K.V wrote:
>> Alexander Graf<[email-masked]>  writes:
>>>>>> diff --git a/arch/powerpc/kvm/trace.h b/arch/powerpc/kvm/trace.h
>>>>>> index a088e9a..7d5a136 100644
>>>>>> --- a/arch/powerpc/kvm/trace.h
>>>>>> +++ b/arch/powerpc/kvm/trace.h
>>>>>> @@ -85,6 +85,12 @@ TRACE_EVENT(kvm_ppc_instr,
>>>>>> 	{41, "HV_PRIV"}
>>>>>> #endif
>>>>>>
>>>>>> +#ifndef CONFIG_KVM_BOOK3S_PR
>>>>>> +/*
>>>>>> + * For pr we define this in trace_pr.h since it pr can be built as
>>>>>> + * a module
>>>>> Not sure I understand the need. If the config option is available, so
>>>>> should the struct field. Worst case that happens with HV is that we
>>>>> get empty shadow_srr1 values in our trace, no?
>>>> That is not the real reason. trace.h get built as part of kvm.ko or as
>>>> part of kernel. These trace functions actually get called from
>>>> kvm-pr.ko. To make they build i would either need EXPORT_SYMBOL or move
>>>> the definition of them to kvm-pr.ko. I did the later and moved only pr
>>>> related traces to kvm-pr.ko
>>> I fail to see why we wouldn't have a trace_hv.h file then, as that can
>>> also be built as a module, no? And at that point I don't see why we
>>> would need any conditionals at all in trace.h anymore, as it would
>>> only cover generic code.
>> Currently HV module is not using any tracepoints. Once it start using
>> tracepoints we would have trace_hv.h
>
> So why would there be an #ifndef in trace.h?
>

to handle things like trace_kvm_exit in booke.c. For that we still don't
have kernel module, and booke.c include trace.h.

-aneesh