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

["Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>]

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 )

diff --git a/arch/powerpc/include/asm/exception-64s.h b/arch/powerpc/include/asm/exception-64s.h
index cca12f0..0b798d4 100644
--- a/arch/powerpc/include/asm/exception-64s.h
+++ b/arch/powerpc/include/asm/exception-64s.h
@@ -198,6 +198,17 @@ END_FTR_SECTION_NESTED(ftr,ftr,943)
 	cmpwi	r10,0;							\
 	bne	do_kvm_##n
 
+#ifdef CONFIG_KVM_BOOK3S_HV
+/*
+ * If hv is possible, interrupts come into to the hv version
+ * of the kvmppc_interrupt code, which then jumps to the PR handler,
+ * kvmppc_interrupt_pr, if the guest is a PR guest.
+ */
+#define kvmppc_interrupt kvmppc_interrupt_hv
+#else
+#define kvmppc_interrupt kvmppc_interrupt_pr
+#endif
+
 #define __KVM_HANDLER(area, h, n)					\
 do_kvm_##n:								\
 	BEGIN_FTR_SECTION_NESTED(947)					\
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index 5ede7fc..2eb6622 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -563,8 +563,8 @@ END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
 /*
  * We come here from the first-level interrupt handlers.
  */
-	.globl	kvmppc_interrupt
-kvmppc_interrupt:
+	.globl	kvmppc_interrupt_hv
+kvmppc_interrupt_hv:
 	/*
 	 * Register contents:
 	 * R12		= interrupt vector
@@ -577,6 +577,11 @@ kvmppc_interrupt:
 	lbz	r9, HSTATE_IN_GUEST(r13)
 	cmpwi	r9, KVM_GUEST_MODE_HOST_HV
 	beq	kvmppc_bad_host_intr
+#ifdef CONFIG_KVM_BOOK3S_PR
+	cmpwi	r9, KVM_GUEST_MODE_GUEST
+	ld	r9, HSTATE_SCRATCH2(r13)
+	beq	kvmppc_interrupt_pr
+#endif
 	/* We're now back in the host but in guest MMU context */
 	li	r9, KVM_GUEST_MODE_HOST_HV
 	stb	r9, HSTATE_IN_GUEST(r13)
diff --git a/arch/powerpc/kvm/book3s_segment.S b/arch/powerpc/kvm/book3s_segment.S
index 1abe478..bc50c97 100644
--- a/arch/powerpc/kvm/book3s_segment.S
+++ b/arch/powerpc/kvm/book3s_segment.S
@@ -161,8 +161,8 @@ kvmppc_handler_trampoline_enter_end:
 .global kvmppc_handler_trampoline_exit
 kvmppc_handler_trampoline_exit:
 
-.global kvmppc_interrupt
-kvmppc_interrupt:
+.global kvmppc_interrupt_pr
+kvmppc_interrupt_pr:
 
 	/* Register usage at this point:
 	 *