Subject: Re: [RFC PATCH 06/11] kvm: powerpc: book3s: Add is_hv_enabled to kvmppc_ops

From: Alexander Graf <[email-masked]>
Date: Mon, 30 Sep 2013 16:51:05 +0200

On 09/30/2013 02:56 PM, Aneesh Kumar K.V wrote:
> Alexander Graf<[email-masked]>  writes:
>
>> On 27.09.2013, at 15:03, Aneesh Kumar K.V wrote:
>>
>>> Alexander Graf<[email-masked]>  writes:
>>>
>>>
>>>>> diff --git a/arch/powerpc/kvm/book3s_segment.S b/arch/powerpc/kvm/book3s_segment.S
>>>>> index 1abe478..e0229dd 100644
>>>>> --- a/arch/powerpc/kvm/book3s_segment.S
>>>>> +++ b/arch/powerpc/kvm/book3s_segment.S
>>>>> @@ -161,9 +161,14 @@ kvmppc_handler_trampoline_enter_end:
>>>>> .global kvmppc_handler_trampoline_exit
>>>>> kvmppc_handler_trampoline_exit:
>>>>>
>>>>> +#if defined(CONFIG_KVM_BOOK3S_HV)
>>>>> +.global kvmppc_interrupt_pr
>>>>> +kvmppc_interrupt_pr:
>>>>> +	ld	r9, HSTATE_SCRATCH2(r13)
>>>>> +#else
>>>>> .global kvmppc_interrupt
>>>>> kvmppc_interrupt:
>>>> Just always call it kvmppc_interrupt_pr and thus share at least that
>>>> part of the code :).
>>> But if i don't have HV enabled, we don't compile book3s_hv_rmhandlers.S
>>> Hence don't have the kvmppc_interrupt symbol defined.
>> Ah, because we're always jumping to kvmppc_interrupt. Can we make this
>> slightly less magical? How about we always call kvmppc_interrupt_hv
>> when CONFIG_KVM_BOOK3S_HV_POSSIBLE and always kvmppc_interrupt_pr when
>> CONFIG_KVM_BOOK3S_PR_POSSIBLE and then branch to kvmppc_interrupt_pr
>> from kvmppc_interrupt_hv?
>>
>> IMHO that would make the code flow more obvious.
>
> To make sure I understand you correctly, what you are suggesting is
> to update __KVM_HANDLER to call kvmppc_interupt_pr when HV is not
> enabled ?

Yes, I think that makes the code flow more obvious. Every function 
always has the same name regardless of config options then.

Alex