diff mbox

KVM: PPC: bookehv: Allow duplicate calls of DO_KVM macro

Message ID 1347455894-30044-1-git-send-email-mihai.caraman@freescale.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mihai Caraman Sept. 12, 2012, 1:18 p.m. UTC
The current form of DO_KVM macro restricts its use to one call per input
parameter set. This is caused by kvmppc_resume_\intno\()_\srr1 symbol
definition.
Duplicate calls of DO_KVM are required by distinct implementations of
exeption handlers which are delegated at runtime. Use a rare label number
to avoid conflicts with the calling contexts.

Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
---
 arch/powerpc/include/asm/kvm_booke_hv_asm.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Comments

Alexander Graf Sept. 12, 2012, 6:56 p.m. UTC | #1
On 12.09.2012, at 15:18, Mihai Caraman <mihai.caraman@freescale.com> wrote:

> The current form of DO_KVM macro restricts its use to one call per input
> parameter set. This is caused by kvmppc_resume_\intno\()_\srr1 symbol
> definition.
> Duplicate calls of DO_KVM are required by distinct implementations of
> exeption handlers which are delegated at runtime.

Not sure I understand what you're trying to achieve here. Please elaborate ;)

Alex

> Use a rare label number
> to avoid conflicts with the calling contexts.
> 
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> ---
> arch/powerpc/include/asm/kvm_booke_hv_asm.h |    4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/kvm_booke_hv_asm.h b/arch/powerpc/include/asm/kvm_booke_hv_asm.h
> index 30a600f..a37a12a 100644
> --- a/arch/powerpc/include/asm/kvm_booke_hv_asm.h
> +++ b/arch/powerpc/include/asm/kvm_booke_hv_asm.h
> @@ -38,9 +38,9 @@
> #ifdef CONFIG_KVM_BOOKE_HV
> BEGIN_FTR_SECTION
>    mtocrf    0x80, r11    /* check MSR[GS] without clobbering reg */
> -    bf    3, kvmppc_resume_\intno\()_\srr1
> +    bf    3, 1975f
>    b    kvmppc_handler_\intno\()_\srr1
> -kvmppc_resume_\intno\()_\srr1:
> +1975:
> END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
> #endif
> .endm
> -- 
> 1.7.4.1
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood Sept. 12, 2012, 9:38 p.m. UTC | #2
On 09/12/2012 01:56 PM, Alexander Graf wrote:
> 
> 
> On 12.09.2012, at 15:18, Mihai Caraman <mihai.caraman@freescale.com> wrote:
> 
>> The current form of DO_KVM macro restricts its use to one call per input
>> parameter set. This is caused by kvmppc_resume_\intno\()_\srr1 symbol
>> definition.
>> Duplicate calls of DO_KVM are required by distinct implementations of
>> exeption handlers which are delegated at runtime.
> 
> Not sure I understand what you're trying to achieve here. Please elaborate ;)

On 64-bit book3e we compile multiple versions of the TLB miss handlers,
and choose from them at runtime.  Without this patch, we get duplicate
label errors if more than one variant of the same exception uses DO_KVM.

-Scott


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Graf Sept. 12, 2012, 9:45 p.m. UTC | #3
On 12.09.2012, at 23:38, Scott Wood <scottwood@freescale.com> wrote:

> On 09/12/2012 01:56 PM, Alexander Graf wrote:
>> 
>> 
>> On 12.09.2012, at 15:18, Mihai Caraman <mihai.caraman@freescale.com> wrote:
>> 
>>> The current form of DO_KVM macro restricts its use to one call per input
>>> parameter set. This is caused by kvmppc_resume_\intno\()_\srr1 symbol
>>> definition.
>>> Duplicate calls of DO_KVM are required by distinct implementations of
>>> exeption handlers which are delegated at runtime.
>> 
>> Not sure I understand what you're trying to achieve here. Please elaborate ;)
> 
> On 64-bit book3e we compile multiple versions of the TLB miss handlers,
> and choose from them at runtime.  

Why?

> Without this patch, we get duplicate
> label errors if more than one variant of the same exception uses DO_KVM.

Makes sense. The proposed solution also looks good. Just quickly walk me through the reasoning for the runtime check again please.


Alex

> 
> -Scott
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood Sept. 12, 2012, 9:54 p.m. UTC | #4
On 09/12/2012 04:45 PM, Alexander Graf wrote:
> 
> 
> On 12.09.2012, at 23:38, Scott Wood <scottwood@freescale.com> wrote:
> 
>> On 09/12/2012 01:56 PM, Alexander Graf wrote:
>>>
>>>
>>> On 12.09.2012, at 15:18, Mihai Caraman <mihai.caraman@freescale.com> wrote:
>>>
>>>> The current form of DO_KVM macro restricts its use to one call per input
>>>> parameter set. This is caused by kvmppc_resume_\intno\()_\srr1 symbol
>>>> definition.
>>>> Duplicate calls of DO_KVM are required by distinct implementations of
>>>> exeption handlers which are delegated at runtime.
>>>
>>> Not sure I understand what you're trying to achieve here. Please elaborate ;)
>>
>> On 64-bit book3e we compile multiple versions of the TLB miss handlers,
>> and choose from them at runtime.  
> 
> Why?

Because one size does not fit all, and we try to not force a separate
kernel build based on what sort of TLB miss handler a piece of hardware
wants.  Some of the differences are too large to be sanely handled by
feature fixups.

>> Without this patch, we get duplicate
>> label errors if more than one variant of the same exception uses DO_KVM.
> 
> Makes sense. The proposed solution also looks good. Just quickly walk me through the reasoning for the runtime check again please.

To start with, you have a TLB miss handler for when partial hardware
tablewalk is used (only the final page table level is looked up in
hardware, so we still need a TLB miss handler to load indirect entries),
and one where that feature is not available.  Then you have the "bolted"
variant used by e5500, which is faster than the generic version because
it doesn't have to deal with recursive faults.  So far the bolted
version is the only one with DO_KVM.

I posted a patch to add another variant, for e6500-style hardware
tablewalk, which shares the bolted prolog/epilog (besides prolog/epilog
performance, e6500 is incompatible with the IBM tablewalk code for
various reasons).  That caused us to have two DO_KVMs for the same
exception type.

-Scott


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Caraman Mihai Claudiu-B02008 Sept. 13, 2012, 12:50 p.m. UTC | #5
> -----Original Message-----

> From: Wood Scott-B07421

> Sent: Thursday, September 13, 2012 12:54 AM

> To: Alexander Graf

> Cc: Caraman Mihai Claudiu-B02008; <kvm-ppc@vger.kernel.org>; <linuxppc-

> dev@lists.ozlabs.org>; <kvm@vger.kernel.org>

> Subject: Re: [PATCH] KVM: PPC: bookehv: Allow duplicate calls of DO_KVM

> macro

> 

> On 09/12/2012 04:45 PM, Alexander Graf wrote:

> >

> >

> > On 12.09.2012, at 23:38, Scott Wood <scottwood@freescale.com> wrote:

> >

> >> On 09/12/2012 01:56 PM, Alexander Graf wrote:

> >>>

> >>>

> >>> On 12.09.2012, at 15:18, Mihai Caraman <mihai.caraman@freescale.com>

> wrote:

> >>>

> >>>> The current form of DO_KVM macro restricts its use to one call per

> input

> >>>> parameter set. This is caused by kvmppc_resume_\intno\()_\srr1

> symbol

> >>>> definition.

> >>>> Duplicate calls of DO_KVM are required by distinct implementations

> of

> >>>> exeption handlers which are delegated at runtime.

> >>>

> >>> Not sure I understand what you're trying to achieve here. Please

> elaborate ;)

> >>

> >> On 64-bit book3e we compile multiple versions of the TLB miss

> handlers,

> >> and choose from them at runtime.


The exception handler patching is active in __early_init_mmu() function
powerpc/mm/tlb_nohash.c for quite a few years. For tlb miss exceptions
there are three handler versions: standard, HW tablewalk and bolted.

> I posted a patch to add another variant, for e6500-style hardware

> tablewalk, which shares the bolted prolog/epilog (besides prolog/epilog

> performance, e6500 is incompatible with the IBM tablewalk code for

> various reasons).  That caused us to have two DO_KVMs for the same

> exception type.


Sorry, I missed to cc kvm-ppc mailist when I replayed to that discussion
thread.

-Mike
Alexander Graf Sept. 13, 2012, 3:02 p.m. UTC | #6
On 09/12/2012 03:18 PM, Mihai Caraman wrote:
> The current form of DO_KVM macro restricts its use to one call per input
> parameter set. This is caused by kvmppc_resume_\intno\()_\srr1 symbol
> definition.
> Duplicate calls of DO_KVM are required by distinct implementations of
> exeption handlers which are delegated at runtime. Use a rare label number
> to avoid conflicts with the calling contexts.
>
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>

Thanks, applied to kvm-ppc-next.


Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/powerpc/include/asm/kvm_booke_hv_asm.h b/arch/powerpc/include/asm/kvm_booke_hv_asm.h
index 30a600f..a37a12a 100644
--- a/arch/powerpc/include/asm/kvm_booke_hv_asm.h
+++ b/arch/powerpc/include/asm/kvm_booke_hv_asm.h
@@ -38,9 +38,9 @@ 
 #ifdef CONFIG_KVM_BOOKE_HV
 BEGIN_FTR_SECTION
 	mtocrf	0x80, r11	/* check MSR[GS] without clobbering reg */
-	bf	3, kvmppc_resume_\intno\()_\srr1
+	bf	3, 1975f
 	b	kvmppc_handler_\intno\()_\srr1
-kvmppc_resume_\intno\()_\srr1:
+1975:
 END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
 #endif
 .endm