[PATCH v4 1/3] ppc64/book3s: fix branching to out of line handlers in relocation kernel

[PATCH v4 1/3] ppc64/book3s: fix branching to out of line handlers in relocation kernel

[Hari Bathini]

Some of the interrupt vectors on 64-bit POWER server processors  are
only 32 bytes long (8 instructions), which is not enough for the full
first-level interrupt handler. For these we need to branch to an out-
of-line (OOL) handler. But when we are running a relocatable kernel,
interrupt vectors till __end_interrupts marker are copied down to real
address 0x100. So, branching to labels (read OOL handlers) outside this
section should be handled differently (see LOAD_HANDLER()), considering
relocatable kernel, which would need atleast 4 instructions.

However, branching from interrupt vector means that we corrupt the CFAR
(come-from address register) on POWER7 and later processors as mentioned
in commit 1707dd16. So, EXCEPTION_PROLOG_0 (6 instructions) that contains
the part up to the point where the CFAR is saved in the PACA should be
part of the short interrupt vectors before we branch out to OOL handlers.

But as mentioned already, there are interrupt vectors on 64-bit POWER server
processors that are only 32 bytes long (like vectors 0x4f00, 0x4f20, etc.),
which cannot accomodate the above two cases at the same time owing to space
constraint. Currently, in these interrupt vectors, we simply branch out to
OOL handlers, without using LOAD_HANDLER(), which leaves us vulnerable when
running a relocatable kernel (eg. kdump case). While this has been the case
for sometime now and kdump is used widely, we were fortunate not to see any
problems so far, for three reasons:

    1. In almost all cases, production kernel (relocatable) is used for
       kdump as well, which would mean that crashed kernel's OOL handler
       would be at the same place where we endup branching to, from short
       interrupt vector of kdump kernel.
    2. Also, OOL handler was unlikely the reason for crash in almost all
       the kdump scenarios, which meant we had a sane OOL handler from
       crashed kernel that we branched to.
    3. On most 64-bit POWER server processors, page size is large enough
       that marking interrupt vector code as executable (see commit
       429d2e83) leads to marking OOL handler code from crashed kernel,
       that sits right below interrupt vector code from kdump kernel, as
       executable as well.

Let us fix this undependable code path by moving these OOL handlers below
__end_interrupts marker to make sure we also copy these handlers to real
address 0x100 when running a relocatable kernel. Because the interrupt
vectors branching to these OOL handlers are not long enough to use
LOAD_HANDLER() for branching as discussed above.

This fix has been tested successfully in kdump scenario, on a lpar with 4K page
size by using different default/production kernel and kdump kernel.

Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
---

Michael, I did test this patchset in different scenarios. But if you feel
the change is too radical, we could go with version2. But I thought this was
worth a shot.

changes from v3:
1. No changes in this patch except for a spellcheck
2. A new patch that tries to free up space below 0x7000 (2/3)
3. A new patch to remove __end_handlers marker (3/3)


 arch/powerpc/kernel/exceptions-64s.S |   29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 7716ceb..f76b2f3 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -953,6 +953,25 @@ hv_facility_unavailable_relon_trampoline:
 #endif
 	STD_RELON_EXCEPTION_PSERIES(0x5700, 0x1700, altivec_assist)
 
+	/*
+	 * Out-Of-Line handlers for relocation-on interrupt vectors
+	 *
+	 * We need these OOL handlers to be below __end_interrupts
+	 * marker to ensure we also copy these OOL handlers along
+	 * with the interrupt vectors to real address 0x100 when
+	 * running a relocatable kernel. Because the interrupt
+	 * vectors branching to these OOL handlers are not long
+	 * enough to use LOAD_HANDLER() for branching.
+	 */
+	STD_RELON_EXCEPTION_HV_OOL(0xe40, emulation_assist)
+	MASKABLE_RELON_EXCEPTION_HV_OOL(0xe80, h_doorbell)
+
+	STD_RELON_EXCEPTION_PSERIES_OOL(0xf00, performance_monitor)
+	STD_RELON_EXCEPTION_PSERIES_OOL(0xf20, altivec_unavailable)
+	STD_RELON_EXCEPTION_PSERIES_OOL(0xf40, vsx_unavailable)
+	STD_RELON_EXCEPTION_PSERIES_OOL(0xf60, facility_unavailable)
+	STD_RELON_EXCEPTION_HV_OOL(0xf80, hv_facility_unavailable)
+
 	/* Other future vectors */
 	.align	7
 	.globl	__end_interrupts
@@ -1234,16 +1253,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_VSX)
 	.globl	__end_handlers
 __end_handlers:
 
-	/* Equivalents to the above handlers for relocation-on interrupt vectors */
-	STD_RELON_EXCEPTION_HV_OOL(0xe40, emulation_assist)
-	MASKABLE_RELON_EXCEPTION_HV_OOL(0xe80, h_doorbell)
-
-	STD_RELON_EXCEPTION_PSERIES_OOL(0xf00, performance_monitor)
-	STD_RELON_EXCEPTION_PSERIES_OOL(0xf20, altivec_unavailable)
-	STD_RELON_EXCEPTION_PSERIES_OOL(0xf40, vsx_unavailable)
-	STD_RELON_EXCEPTION_PSERIES_OOL(0xf60, facility_unavailable)
-	STD_RELON_EXCEPTION_HV_OOL(0xf80, hv_facility_unavailable)
-
 #if defined(CONFIG_PPC_PSERIES) || defined(CONFIG_PPC_POWERNV)
 /*
  * Data area reserved for FWNMI option.

[PATCH v4 2/3] ppc64/book3s: make some room for common interrupt vector code

[Hari Bathini]

With the previous patch, we choke out whatever little space is left
below 0x7000 (FWNMI hard block) while there is a hole of ~1400 bytes
below __end_interrupts marker when CONFIG_CBE_RAS is disabled.
Considering CONFIG_CBE_RAS is not enabled by default for BOOK3S, this
is not a desirable scenario especially when we have to worry about
each additional instruction that goes below 0x7000.

Memory region from 0x1800 to 0x4000 is dedicated for common interrupt
vector code. Also, we never hit an interrupt below 0x300 when IR=DR=1
implying memory region between 0x4000 to 0x4300 can also be used for
common interrupt vector code. So, we can effectively use memory region
between 0x1800 to 0x4300 for common interrupt vector code.

This patch tries to free up some space below 0x7000 by rearranging the
common interrupt vector code. The approach here is to avoid large holes
below 0x4300 for any kernel configuration. For this, let us move common
interrupt vector code that only gets enabled with CONFIG_CBE_RAS above
0x8000, as it doesn't need to be too close to the call sites and can be
branched to with LOAD_HANDLER() as long as it is within the first 64KB
(0x10000) of the kernel image. Instead, lets move common interrupt vector
code marked h_instr_storage_common, facility_unavailable_common &
hv_facility_unavailable_common below 0x4300. This leaves ~250 bytes
free below 0x4300 and ~1150 bytes free below 0x7000 - enough space to
stop worrying about every additional instruction that goes below 0x7000.

This patch assumes at least commit 376af594, part of the patch series
that starts with commit 468a3302, is part of the code to avoid messy
compilation issues like:

    relocation truncated to fit: R_PPC64_REL14 against `.text'+1c90
    Makefile:864: recipe for target 'vmlinux' failed

I tested this patch successfully on ppc64, ppc64le lpars and baremetal
environments. Couldn't test it on IBM cell blade though but expecting no
problems with this patch in IBM cell blade environment as well. If
someone can test this patch in cell platform, it would be great.

Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/exceptions-64s.S |   20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index f76b2f3..c193ebd 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -786,6 +786,7 @@ kvmppc_skip_Hinterrupt:
 	STD_EXCEPTION_COMMON(0xb00, trap_0b, unknown_exception)
 	STD_EXCEPTION_COMMON(0xd00, single_step, single_step_exception)
 	STD_EXCEPTION_COMMON(0xe00, trap_0e, unknown_exception)
+	STD_EXCEPTION_COMMON(0xe20, h_instr_storage, unknown_exception)
 	STD_EXCEPTION_COMMON(0xe40, emulation_assist, emulation_assist_interrupt)
 	STD_EXCEPTION_COMMON_ASYNC(0xe60, hmi_exception, handle_hmi_exception)
 #ifdef CONFIG_PPC_DOORBELL
@@ -794,6 +795,9 @@ kvmppc_skip_Hinterrupt:
 	STD_EXCEPTION_COMMON_ASYNC(0xe80, h_doorbell, unknown_exception)
 #endif
 	STD_EXCEPTION_COMMON_ASYNC(0xf00, performance_monitor, performance_monitor_exception)
+	STD_EXCEPTION_COMMON(0xf60, facility_unavailable, facility_unavailable_exception)
+	STD_EXCEPTION_COMMON(0xf80, hv_facility_unavailable, facility_unavailable_exception)
+
 	STD_EXCEPTION_COMMON(0x1300, instruction_breakpoint, instruction_breakpoint_exception)
 	STD_EXCEPTION_COMMON(0x1502, denorm, unknown_exception)
 #ifdef CONFIG_ALTIVEC
@@ -801,11 +805,6 @@ kvmppc_skip_Hinterrupt:
 #else
 	STD_EXCEPTION_COMMON(0x1700, altivec_assist, unknown_exception)
 #endif
-#ifdef CONFIG_CBE_RAS
-	STD_EXCEPTION_COMMON(0x1200, cbe_system_error, cbe_system_error_exception)
-	STD_EXCEPTION_COMMON(0x1600, cbe_maintenance, cbe_maintenance_exception)
-	STD_EXCEPTION_COMMON(0x1800, cbe_thermal, cbe_thermal_exception)
-#endif /* CONFIG_CBE_RAS */
 
 	/*
 	 * Relocation-on interrupts: A subset of the interrupts can be delivered
@@ -1029,8 +1028,6 @@ instruction_access_common:
 	li	r5,0x400
 	b	do_hash_page		/* Try to handle as hpte fault */
 
-	STD_EXCEPTION_COMMON(0xe20, h_instr_storage, unknown_exception)
-
 /*
  * Here is the common SLB miss user that is used when going to virtual
  * mode for SLB misses, that is currently not used
@@ -1246,9 +1243,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_VSX)
 	bl	vsx_unavailable_exception
 	b	ret_from_except
 
-	STD_EXCEPTION_COMMON(0xf60, facility_unavailable, facility_unavailable_exception)
-	STD_EXCEPTION_COMMON(0xf80, hv_facility_unavailable, facility_unavailable_exception)
-
 	.align	7
 	.globl	__end_handlers
 __end_handlers:
@@ -1268,6 +1262,12 @@ fwnmi_data_area:
 	. = 0x8000
 #endif /* defined(CONFIG_PPC_PSERIES) || defined(CONFIG_PPC_POWERNV) */
 
+#ifdef CONFIG_CBE_RAS
+	STD_EXCEPTION_COMMON(0x1200, cbe_system_error, cbe_system_error_exception)
+	STD_EXCEPTION_COMMON(0x1600, cbe_maintenance, cbe_maintenance_exception)
+	STD_EXCEPTION_COMMON(0x1800, cbe_thermal, cbe_thermal_exception)
+#endif /* CONFIG_CBE_RAS */
+
 	.globl hmi_exception_early
 hmi_exception_early:
 	EXCEPTION_PROLOG_1(PACA_EXGEN, NOTEST, 0xe60)

[PATCH v4 3/3] ppc64/book3s: remove __end_handlers marker

[Hari Bathini]

__end_handlers marker was intended to mark down upto code that gets
called from exception prologs. But that hasn't kept pace with code
changes. Case in point, slb_miss_realmode being called from exception
prolog code but isn't below __end_handlers marker. So, __end_handlers
marker is as good as a comment but could be misleading at times if
it isn't in sync with the code, as is the case now. So, let us avoid
this confusion by having a better comment and removing __end_handlers
marker altogether.

Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/exceptions-64s.S |   13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index c193ebd..80f9fc4 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -764,11 +764,10 @@ kvmppc_skip_Hinterrupt:
 #endif
 
 /*
- * Code from here down to __end_handlers is invoked from the
- * exception prologs above.  Because the prologs assemble the
- * addresses of these handlers using the LOAD_HANDLER macro,
- * which uses an ori instruction, these handlers must be in
- * the first 64k of the kernel image.
+ * Ensure that any handlers that get invoked from the exception prologs
+ * above are below the first 64KB (0x10000) of the kernel image because
+ * the prologs assemble the addresses of these handlers using the
+ * LOAD_HANDLER macro, which uses an ori instruction.
  */
 
 /*** Common interrupt handlers ***/
@@ -1243,10 +1242,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_VSX)
 	bl	vsx_unavailable_exception
 	b	ret_from_except
 
-	.align	7
-	.globl	__end_handlers
-__end_handlers:
-
 #if defined(CONFIG_PPC_PSERIES) || defined(CONFIG_PPC_POWERNV)
 /*
  * Data area reserved for FWNMI option.

Re: [v4, 2/3] ppc64/book3s: make some room for common interrupt vector code

[Michael Ellerman]

Hi Hari,

Thanks for persisting with this.

On Thu, 2016-07-04 at 21:58:50 UTC, Hari Bathini wrote:
> With the previous patch, we choke out whatever little space is left
> below 0x7000 (FWNMI hard block) while there is a hole of ~1400 bytes
> below __end_interrupts marker when CONFIG_CBE_RAS is disabled.
> Considering CONFIG_CBE_RAS is not enabled by default for BOOK3S, this
> is not a desirable scenario especially when we have to worry about
> each additional instruction that goes below 0x7000.
> 
> Memory region from 0x1800 to 0x4000 is dedicated for common interrupt
> vector code. Also, we never hit an interrupt below 0x300 when IR=DR=1
> implying memory region between 0x4000 to 0x4300 can also be used for
> common interrupt vector code. So, we can effectively use memory region
> between 0x1800 to 0x4300 for common interrupt vector code.

On Power9 the system-call-vectored instruction will use the region at 0x3000, so
moving code into that space is not a good long term plan.

I'll take your v2 and put it in next next week.

cheers

Re: [v4, 2/3] ppc64/book3s: make some room for common interrupt vector code

[Michael Ellerman]

On Fri, 2016-04-15 at 21:06 +1000, Michael Ellerman wrote:
> Hi Hari,
>
> Thanks for persisting with this.
>
> On Thu, 2016-07-04 at 21:58:50 UTC, Hari Bathini wrote:

> > With the previous patch, we choke out whatever little space is left
> > below 0x7000 (FWNMI hard block) while there is a hole of ~1400 bytes
> > below __end_interrupts marker when CONFIG_CBE_RAS is disabled.
> > Considering CONFIG_CBE_RAS is not enabled by default for BOOK3S, this
> > is not a desirable scenario especially when we have to worry about
> > each additional instruction that goes below 0x7000.
> >
> > Memory region from 0x1800 to 0x4000 is dedicated for common interrupt
> > vector code. Also, we never hit an interrupt below 0x300 when IR=DR=1
> > implying memory region between 0x4000 to 0x4300 can also be used for
> > common interrupt vector code. So, we can effectively use memory region
> > between 0x1800 to 0x4300 for common interrupt vector code.
>
> On Power9 the system-call-vectored instruction will use the region at 0x3000, so
> moving code into that space is not a good long term plan.
>
> I'll take your v2 and put it in next next week.

I'll add this fixes line, which I think is correct:

Fixes: c1fb6816fb1b ("powerpc: Add relocation on exception vector handlers")

cheers

Re: [v4, 2/3] ppc64/book3s: make some room for common interrupt vector code

[Hari Bathini]

On 04/15/2016 06:29 PM, Michael Ellerman wrote:
> On Fri, 2016-04-15 at 21:06 +1000, Michael Ellerman wrote:
>> Hi Hari,
>>
>> Thanks for persisting with this.
>>
>> On Thu, 2016-07-04 at 21:58:50 UTC, Hari Bathini wrote:
>>> With the previous patch, we choke out whatever little space is left
>>> below 0x7000 (FWNMI hard block) while there is a hole of ~1400 bytes
>>> below __end_interrupts marker when CONFIG_CBE_RAS is disabled.
>>> Considering CONFIG_CBE_RAS is not enabled by default for BOOK3S, this
>>> is not a desirable scenario especially when we have to worry about
>>> each additional instruction that goes below 0x7000.
>>>
>>> Memory region from 0x1800 to 0x4000 is dedicated for common interrupt
>>> vector code. Also, we never hit an interrupt below 0x300 when IR=DR=1
>>> implying memory region between 0x4000 to 0x4300 can also be used for
>>> common interrupt vector code. So, we can effectively use memory region
>>> between 0x1800 to 0x4300 for common interrupt vector code.
>> On Power9 the system-call-vectored instruction will use the region at 0x3000, so
>> moving code into that space is not a good long term plan.
>>
>> I'll take your v2 and put it in next next week.
> I'll add this fixes line, which I think is correct:
>
> Fixes: c1fb6816fb1b ("powerpc: Add relocation on exception vector handlers")

Yeah. Thanks!

> cheers
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [v4,3/3] ppc64/book3s: remove __end_handlers marker

[Michael Ellerman]

On Thu, 2016-07-04 at 22:00:34 UTC, Hari Bathini wrote:
> __end_handlers marker was intended to mark down upto code that gets
> called from exception prologs. But that hasn't kept pace with code
> changes. Case in point, slb_miss_realmode being called from exception
> prolog code but isn't below __end_handlers marker. So, __end_handlers
> marker is as good as a comment but could be misleading at times if
> it isn't in sync with the code, as is the case now. So, let us avoid
> this confusion by having a better comment and removing __end_handlers
> marker altogether.
> 
> Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/057b6d7e62ea4e6b1809e29469

cheers