All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexander Graf <agraf@suse.de>
To: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: paulus@samba.org, linuxppc-dev@lists.ozlabs.org, kvm-ppc@vger.kernel.org
Subject: Re: [RFC PATCH 06/11] kvm: powerpc: book3s: Add is_hv_enabled to kvmppc_ops
Date: Mon, 30 Sep 2013 16:36:36 +0000	[thread overview]
Message-ID: <5249A894.80304@suse.de> (raw)
In-Reply-To: <878uyeb7s4.fsf@linux.vnet.ibm.com>

On 09/30/2013 06:20 PM, Aneesh Kumar K.V wrote:
> Alexander Graf<agraf@suse.de>  writes:
>
>> On 09/30/2013 02:56 PM, Aneesh Kumar K.V wrote:
>>> Alexander Graf<agraf@suse.de>   writes:
>>>
>>>> On 27.09.2013, at 15:03, Aneesh Kumar K.V wrote:
>>>>
>>>>> Alexander Graf<agraf@suse.de>   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.
>>
> Something like this ( btw non tested )

Yes :).


Alex


WARNING: multiple messages have this Message-ID (diff)
From: Alexander Graf <agraf@suse.de>
To: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: paulus@samba.org, linuxppc-dev@lists.ozlabs.org, kvm-ppc@vger.kernel.org
Subject: Re: [RFC PATCH 06/11] kvm: powerpc: book3s: Add is_hv_enabled to kvmppc_ops
Date: Mon, 30 Sep 2013 18:36:36 +0200	[thread overview]
Message-ID: <5249A894.80304@suse.de> (raw)
In-Reply-To: <878uyeb7s4.fsf@linux.vnet.ibm.com>

On 09/30/2013 06:20 PM, Aneesh Kumar K.V wrote:
> Alexander Graf<agraf@suse.de>  writes:
>
>> On 09/30/2013 02:56 PM, Aneesh Kumar K.V wrote:
>>> Alexander Graf<agraf@suse.de>   writes:
>>>
>>>> On 27.09.2013, at 15:03, Aneesh Kumar K.V wrote:
>>>>
>>>>> Alexander Graf<agraf@suse.de>   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.
>>
> Something like this ( btw non tested )

Yes :).


Alex

  reply	other threads:[~2013-09-30 16:36 UTC|newest]

Thread overview: 114+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-27 10:03 [RFC PATCH 00/11 Allow PR and HV KVM to coexist in one kernel Aneesh Kumar K.V
2013-09-27 10:15 ` Aneesh Kumar K.V
2013-09-27 10:03 ` [RFC PATCH 01/11] kvm: powerpc: book3s hv: Fix vcore leak Aneesh Kumar K.V
2013-09-27 10:15   ` Aneesh Kumar K.V
2013-09-27 11:39   ` Alexander Graf
2013-09-27 11:39     ` Alexander Graf
2013-09-27 10:03 ` [RFC PATCH 02/11] kvm: powerpc: book3s: remove kvmppc_handler_highmem label Aneesh Kumar K.V
2013-09-27 10:15   ` Aneesh Kumar K.V
2013-09-27 10:03 ` [RFC PATCH 03/11] kvm: powerpc: book3s: move book3s_64_vio_hv.c into the main kernel binary Aneesh Kumar K.V
2013-09-27 10:15   ` Aneesh Kumar K.V
2013-09-27 10:03 ` [RFC PATCH 04/11] kvm: powerpc: book3s: Add a new config variable CONFIG_KVM_BOOK3S_HV Aneesh Kumar K.V
2013-09-27 10:15   ` Aneesh Kumar K.V
2013-09-27 11:43   ` Alexander Graf
2013-09-27 11:43     ` Alexander Graf
2013-09-27 12:45     ` Aneesh Kumar K.V
2013-09-27 12:57       ` Aneesh Kumar K.V
2013-09-27 10:03 ` [RFC PATCH 05/11] kvm: powerpc: book3s: Add kvmppc_ops callback for HV and PR specific operations Aneesh Kumar K.V
2013-09-27 10:15   ` Aneesh Kumar K.V
2013-09-27 12:04   ` [RFC PATCH 05/11] kvm: powerpc: book3s: Add kvmppc_ops callback for HV and PR specific operation Alexander Graf
2013-09-27 12:04     ` [RFC PATCH 05/11] kvm: powerpc: book3s: Add kvmppc_ops callback for HV and PR specific operations Alexander Graf
2013-09-27 12:52     ` Aneesh Kumar K.V
2013-09-27 12:52       ` [RFC PATCH 05/11] kvm: powerpc: book3s: Add kvmppc_ops callback for HV and PR specific operation Aneesh Kumar K.V
2013-09-27 10:03 ` [RFC PATCH 06/11] kvm: powerpc: book3s: Add is_hv_enabled to kvmppc_ops Aneesh Kumar K.V
2013-09-27 10:15   ` Aneesh Kumar K.V
2013-09-27 12:18   ` Alexander Graf
2013-09-27 12:18     ` Alexander Graf
2013-09-27 13:03     ` Aneesh Kumar K.V
2013-09-27 13:15       ` Aneesh Kumar K.V
2013-09-30 10:09       ` Alexander Graf
2013-09-30 10:09         ` Alexander Graf
2013-09-30 12:56         ` Aneesh Kumar K.V
2013-09-30 12:56           ` Aneesh Kumar K.V
2013-09-30 14:51           ` Alexander Graf
2013-09-30 14:51             ` Alexander Graf
2013-09-30 16:20             ` Aneesh Kumar K.V
2013-09-30 16:32               ` Aneesh Kumar K.V
2013-09-30 16:36               ` Alexander Graf [this message]
2013-09-30 16:36                 ` Alexander Graf
2013-09-27 10:03 ` [RFC PATCH 07/11] kvm: powerpc: book3s: pr: move PR related tracepoints to a separate header Aneesh Kumar K.V
2013-09-27 10:15   ` Aneesh Kumar K.V
2013-09-27 12:22   ` Alexander Graf
2013-09-27 12:22     ` Alexander Graf
2013-09-27 13:06     ` Aneesh Kumar K.V
2013-09-27 13:18       ` Aneesh Kumar K.V
2013-09-30 10:02       ` Alexander Graf
2013-09-30 10:02         ` Alexander Graf
2013-09-30 12:57         ` Aneesh Kumar K.V
2013-09-30 12:57           ` Aneesh Kumar K.V
2013-09-30 14:51           ` Alexander Graf
2013-09-30 14:51             ` Alexander Graf
2013-09-30 15:53             ` Aneesh Kumar K.V
2013-09-30 15:53               ` Aneesh Kumar K.V
2013-09-30 15:55               ` Alexander Graf
2013-09-30 15:55                 ` Alexander Graf
2013-09-27 10:03 ` [RFC PATCH 08/11] kvm: powerpc: book3s: Support building HV and PR KVM as module Aneesh Kumar K.V
2013-09-27 10:15   ` Aneesh Kumar K.V
2013-09-27 12:25   ` Alexander Graf
2013-09-27 12:25     ` Alexander Graf
2013-09-27 13:08     ` Aneesh Kumar K.V
2013-09-27 13:20       ` Aneesh Kumar K.V
2013-09-30 10:04       ` Alexander Graf
2013-09-30 10:04         ` Alexander Graf
2013-09-30 12:57         ` Aneesh Kumar K.V
2013-09-30 12:57           ` Aneesh Kumar K.V
2013-09-27 10:03 ` [RFC PATCH 09/11] kvm: simplify processor compat check Aneesh Kumar K.V
2013-09-27 10:15   ` Aneesh Kumar K.V
2013-09-27 12:31   ` Alexander Graf
2013-09-27 12:31     ` Alexander Graf
2013-09-27 12:31     ` Alexander Graf
2013-09-27 13:13     ` Aneesh Kumar K.V
2013-09-27 13:25       ` Aneesh Kumar K.V
2013-09-27 13:13       ` Aneesh Kumar K.V
2013-09-27 15:14       ` Paolo Bonzini
2013-09-27 15:14         ` Paolo Bonzini
2013-09-27 15:14         ` Paolo Bonzini
2013-09-28 15:36         ` Aneesh Kumar K.V
2013-09-28 15:48           ` Aneesh Kumar K.V
2013-09-28 15:36           ` Aneesh Kumar K.V
2013-09-29  8:58           ` Gleb Natapov
2013-09-29  8:58             ` Gleb Natapov
2013-09-29  8:58             ` Gleb Natapov
2013-09-29 15:05             ` Aneesh Kumar K.V
2013-09-29 15:17               ` Aneesh Kumar K.V
2013-09-29 15:05               ` Aneesh Kumar K.V
2013-09-29 15:11               ` Gleb Natapov
2013-09-29 15:11                 ` Gleb Natapov
2013-09-29 15:11                 ` Gleb Natapov
2013-09-27 10:03 ` [RFC PATCH 10/11] kvm: powerpc: book3s: Allow the HV and PR selection per virtual machine Aneesh Kumar K.V
2013-09-27 10:15   ` Aneesh Kumar K.V
2013-09-27 10:03 ` [RFC PATCH 11/11] kvm: powerpc: book3s: Fix module ownership Aneesh Kumar K.V
2013-09-27 10:15   ` Aneesh Kumar K.V
2013-09-27 10:52 ` [RFC PATCH 00/11 Allow PR and HV KVM to coexist in one kernel Aneesh Kumar K.V
2013-09-27 10:52   ` Aneesh Kumar K.V
2013-09-30 10:16   ` Alexander Graf
2013-09-30 10:16     ` Alexander Graf
2013-09-30 10:16     ` Alexander Graf
2013-09-30 13:09     ` Aneesh Kumar K.V
2013-09-30 13:21       ` Aneesh Kumar K.V
2013-09-30 13:09       ` Aneesh Kumar K.V
2013-09-30 14:54       ` Alexander Graf
2013-09-30 14:54         ` Alexander Graf
2013-09-30 14:54         ` Alexander Graf
2013-10-01 11:26         ` Aneesh Kumar K.V
2013-10-01 11:38           ` Aneesh Kumar K.V
2013-10-01 11:26           ` Aneesh Kumar K.V
2013-10-01 11:36           ` Alexander Graf
2013-10-01 11:36             ` Alexander Graf
2013-10-01 11:36             ` Alexander Graf
2013-10-01 11:41             ` Paolo Bonzini
2013-10-01 11:41               ` Paolo Bonzini
2013-10-01 11:41               ` Paolo Bonzini
2013-10-01 11:43             ` Alexander Graf
2013-10-01 11:43               ` Alexander Graf
2013-10-01 11:43               ` Alexander Graf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5249A894.80304@suse.de \
    --to=agraf@suse.de \
    --cc=aneesh.kumar@linux.vnet.ibm.com \
    --cc=kvm-ppc@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=paulus@samba.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.