* [PATCH v4 1/3] ppc64/book3s: fix branching to out of line handlers in relocation kernel @ 2016-04-07 21:57 Hari Bathini 2016-04-07 21:58 ` [PATCH v4 2/3] ppc64/book3s: make some room for common interrupt vector code Hari Bathini 2016-04-07 22:00 ` [PATCH v4 3/3] ppc64/book3s: remove __end_handlers marker Hari Bathini 0 siblings, 2 replies; 7+ messages in thread From: Hari Bathini @ 2016-04-07 21:57 UTC (permalink / raw) To: linuxppc-dev Cc: Michael Neuling, Ananth N Mavinakayanahalli, Mahesh J Salgaonkar, Paul Mackerras, Michael Ellerman, Benjamin Herrenschmidt 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. ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v4 2/3] ppc64/book3s: make some room for common interrupt vector code 2016-04-07 21:57 [PATCH v4 1/3] ppc64/book3s: fix branching to out of line handlers in relocation kernel Hari Bathini @ 2016-04-07 21:58 ` Hari Bathini 2016-04-15 11:06 ` [v4, " Michael Ellerman 2016-04-07 22:00 ` [PATCH v4 3/3] ppc64/book3s: remove __end_handlers marker Hari Bathini 1 sibling, 1 reply; 7+ messages in thread From: Hari Bathini @ 2016-04-07 21:58 UTC (permalink / raw) To: linuxppc-dev Cc: Michael Neuling, Benjamin Herrenschmidt, Ananth N Mavinakayanahalli, Mahesh J Salgaonkar, Paul Mackerras, Michael Ellerman 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) ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [v4, 2/3] ppc64/book3s: make some room for common interrupt vector code 2016-04-07 21:58 ` [PATCH v4 2/3] ppc64/book3s: make some room for common interrupt vector code Hari Bathini @ 2016-04-15 11:06 ` Michael Ellerman 2016-04-15 12:59 ` Michael Ellerman 0 siblings, 1 reply; 7+ messages in thread From: Michael Ellerman @ 2016-04-15 11:06 UTC (permalink / raw) To: Hari Bathini, linuxppc-dev Cc: Michael Neuling, Mahesh J Salgaonkar, Paul Mackerras 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [v4, 2/3] ppc64/book3s: make some room for common interrupt vector code 2016-04-15 11:06 ` [v4, " Michael Ellerman @ 2016-04-15 12:59 ` Michael Ellerman 2016-04-18 3:44 ` Hari Bathini 0 siblings, 1 reply; 7+ messages in thread From: Michael Ellerman @ 2016-04-15 12:59 UTC (permalink / raw) To: Hari Bathini, linuxppc-dev Cc: Mahesh J Salgaonkar, Michael Neuling, Paul Mackerras 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [v4, 2/3] ppc64/book3s: make some room for common interrupt vector code 2016-04-15 12:59 ` Michael Ellerman @ 2016-04-18 3:44 ` Hari Bathini 0 siblings, 0 replies; 7+ messages in thread From: Hari Bathini @ 2016-04-18 3:44 UTC (permalink / raw) To: Michael Ellerman, linuxppc-dev Cc: Mahesh J Salgaonkar, Michael Neuling, Paul Mackerras 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4 3/3] ppc64/book3s: remove __end_handlers marker 2016-04-07 21:57 [PATCH v4 1/3] ppc64/book3s: fix branching to out of line handlers in relocation kernel Hari Bathini 2016-04-07 21:58 ` [PATCH v4 2/3] ppc64/book3s: make some room for common interrupt vector code Hari Bathini @ 2016-04-07 22:00 ` Hari Bathini 2016-04-21 13:39 ` [v4,3/3] " Michael Ellerman 1 sibling, 1 reply; 7+ messages in thread From: Hari Bathini @ 2016-04-07 22:00 UTC (permalink / raw) To: linuxppc-dev Cc: Michael Neuling, Benjamin Herrenschmidt, Ananth N Mavinakayanahalli, Mahesh J Salgaonkar, Paul Mackerras, Michael Ellerman __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. ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [v4,3/3] ppc64/book3s: remove __end_handlers marker 2016-04-07 22:00 ` [PATCH v4 3/3] ppc64/book3s: remove __end_handlers marker Hari Bathini @ 2016-04-21 13:39 ` Michael Ellerman 0 siblings, 0 replies; 7+ messages in thread From: Michael Ellerman @ 2016-04-21 13:39 UTC (permalink / raw) To: Hari Bathini, linuxppc-dev Cc: Michael Neuling, Mahesh J Salgaonkar, Paul Mackerras 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-04-21 13:39 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-04-07 21:57 [PATCH v4 1/3] ppc64/book3s: fix branching to out of line handlers in relocation kernel Hari Bathini 2016-04-07 21:58 ` [PATCH v4 2/3] ppc64/book3s: make some room for common interrupt vector code Hari Bathini 2016-04-15 11:06 ` [v4, " Michael Ellerman 2016-04-15 12:59 ` Michael Ellerman 2016-04-18 3:44 ` Hari Bathini 2016-04-07 22:00 ` [PATCH v4 3/3] ppc64/book3s: remove __end_handlers marker Hari Bathini 2016-04-21 13:39 ` [v4,3/3] " Michael Ellerman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).