* [PATCH 0/6] powerpc/math-emu: e500 SPE float emulation fixes @ 2013-11-04 16:50 Joseph S. Myers 2013-11-04 16:52 ` [PATCH 1/6] powerpc: fix exception clearing in e500 SPE float emulation Joseph S. Myers ` (6 more replies) 0 siblings, 7 replies; 17+ messages in thread From: Joseph S. Myers @ 2013-11-04 16:50 UTC (permalink / raw) To: linuxppc-dev; +Cc: Liu Yu, linux-kernel, Shan Hai This patch series fixes various problems with the floating-point emulation code for powerpc e500 SPE (some being issues with the e500-specific emulation code, some with the generic math-emu headers). All six patches were sent individually last month as the issues were identified and fixed in the course of preparing the e500 glibc port, and received no comments. There are no substantive changes to the patches in this version, but I've retested the glibc port (which is now upstream, along with all the generic math-emu changes relevant to the glibc soft-fp code, and various fixes to soft-fp corresponding to fixes in the kernel code in the hope that at some point we can get the kernel using the current soft-fp code again) with current kernel sources with this patch series applied. The only dependencies between patches in this series should be that patch 5 (fix e500 SPE float to integer and fixed-point conversions) depends on patch 2 (fix e500 SPE float rounding inexactness detection). Other than that, I think any subset of the patches can be applied in any order, if some subset seems OK but there are concerns about other patches in the series. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/6] powerpc: fix exception clearing in e500 SPE float emulation 2013-11-04 16:50 [PATCH 0/6] powerpc/math-emu: e500 SPE float emulation fixes Joseph S. Myers @ 2013-11-04 16:52 ` Joseph S. Myers 2013-11-22 22:34 ` Scott Wood 2013-11-04 16:53 ` [PATCH 2/6] powerpc: fix e500 SPE float rounding inexactness detection Joseph S. Myers ` (5 subsequent siblings) 6 siblings, 1 reply; 17+ messages in thread From: Joseph S. Myers @ 2013-11-04 16:52 UTC (permalink / raw) To: linuxppc-dev; +Cc: Liu Yu, linux-kernel, Shan Hai From: Joseph Myers <joseph@codesourcery.com> The e500 SPE floating-point emulation code clears existing exceptions (__FPU_FPSCR &= ~FP_EX_MASK;) before ORing in the exceptions from the emulated operation. However, these exception bits are the "sticky", cumulative exception bits, and should only be cleared by the user program setting SPEFSCR, not implicitly by any floating-point instruction (whether executed purely by the hardware or emulated). The spurious clearing of these bits shows up as missing exceptions in glibc testing. Fixing this, however, is not as simple as just not clearing the bits, because while the bits may be from previous floating-point operations (in which case they should not be cleared), the processor can also set the sticky bits itself before the interrupt for an exception occurs, and this can happen in cases when IEEE 754 semantics are that the sticky bit should not be set. Specifically, the "invalid" sticky bit is set in various cases with non-finite operands, where IEEE 754 semantics do not involve raising such an exception, and the "underflow" sticky bit is set in cases of exact underflow, whereas IEEE 754 semantics are that this flag is set only for inexact underflow. Thus, for correct emulation the kernel needs to know the setting of these two sticky bits before the instruction being emulated. When a floating-point operation raises an exception, the kernel can note the state of the sticky bits immediately afterwards. Some <fenv.h> functions that affect the state of these bits, such as fesetenv and feholdexcept, need to use prctl with PR_GET_FPEXC and PR_SET_FPEXC anyway, and so it is natural to record the state of those bits during that call into the kernel and so avoid any need for a separate call into the kernel to inform it of a change to those bits. Thus, the interface I chose to use (in this patch and the glibc port) is that one of those prctl calls must be made after any userspace change to those sticky bits, other than through a floating-point operation that traps into the kernel anyway. feclearexcept and fesetexceptflag duly make those calls, which would not be required were it not for this issue. Signed-off-by: Joseph Myers <joseph@codesourcery.com> --- Previous submission: <http://lkml.org/lkml/2013/10/4/495>. diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h index ce4de5a..0b02e23 100644 --- a/arch/powerpc/include/asm/processor.h +++ b/arch/powerpc/include/asm/processor.h @@ -237,6 +237,8 @@ struct thread_struct { unsigned long evr[32]; /* upper 32-bits of SPE regs */ u64 acc; /* Accumulator */ unsigned long spefscr; /* SPE & eFP status */ + unsigned long spefscr_last; /* SPEFSCR value on last prctl + call or trap return */ int used_spe; /* set if process has used spe */ #endif /* CONFIG_SPE */ #ifdef CONFIG_PPC_TRANSACTIONAL_MEM @@ -303,7 +305,9 @@ struct thread_struct { (_ALIGN_UP(sizeof(init_thread_info), 16) + (unsigned long) &init_stack) #ifdef CONFIG_SPE -#define SPEFSCR_INIT .spefscr = SPEFSCR_FINVE | SPEFSCR_FDBZE | SPEFSCR_FUNFE | SPEFSCR_FOVFE, +#define SPEFSCR_INIT \ + .spefscr = SPEFSCR_FINVE | SPEFSCR_FDBZE | SPEFSCR_FUNFE | SPEFSCR_FOVFE, \ + .spefscr_last = SPEFSCR_FINVE | SPEFSCR_FDBZE | SPEFSCR_FUNFE | SPEFSCR_FOVFE, #else #define SPEFSCR_INIT #endif diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c index 96d2fdf..e3b91f1 100644 --- a/arch/powerpc/kernel/process.c +++ b/arch/powerpc/kernel/process.c @@ -1151,6 +1151,7 @@ int set_fpexc_mode(struct task_struct *tsk, unsigned int val) if (val & PR_FP_EXC_SW_ENABLE) { #ifdef CONFIG_SPE if (cpu_has_feature(CPU_FTR_SPE)) { + tsk->thread.spefscr_last = mfspr(SPRN_SPEFSCR); tsk->thread.fpexc_mode = val & (PR_FP_EXC_SW_ENABLE | PR_FP_ALL_EXCEPT); return 0; @@ -1182,9 +1183,10 @@ int get_fpexc_mode(struct task_struct *tsk, unsigned long adr) if (tsk->thread.fpexc_mode & PR_FP_EXC_SW_ENABLE) #ifdef CONFIG_SPE - if (cpu_has_feature(CPU_FTR_SPE)) + if (cpu_has_feature(CPU_FTR_SPE)) { + tsk->thread.spefscr_last = mfspr(SPRN_SPEFSCR); val = tsk->thread.fpexc_mode; - else + } else return -EINVAL; #else return -EINVAL; diff --git a/arch/powerpc/math-emu/math_efp.c b/arch/powerpc/math-emu/math_efp.c index a73f088..59835c6 100644 --- a/arch/powerpc/math-emu/math_efp.c +++ b/arch/powerpc/math-emu/math_efp.c @@ -630,9 +630,27 @@ update_ccr: regs->ccr |= (IR << ((7 - ((speinsn >> 23) & 0x7)) << 2)); update_regs: - __FPU_FPSCR &= ~FP_EX_MASK; + /* + * If the "invalid" exception sticky bit was set by the + * processor for non-finite input, but was not set before the + * instruction being emulated, clear it. Likewise for the + * "underflow" bit, which may have been set by the processor + * for exact underflow, not just inexact underflow when the + * flag should be set for IEEE 754 semantics. Other sticky + * exceptions will only be set by the processor when they are + * correct according to IEEE 754 semantics, and we must not + * clear sticky bits that were already set before the emulated + * instruction as they represent the user-visible sticky + * exception status. "inexact" traps to kernel are not + * required for IEEE semantics and are not enabled by default, + * so the "inexact" sticky bit may have been set by a previous + * instruction without the kernel being aware of it. + */ + __FPU_FPSCR + &= ~(FP_EX_INVALID | FP_EX_UNDERFLOW) | current->thread.spefscr_last; __FPU_FPSCR |= (FP_CUR_EXCEPTIONS & FP_EX_MASK); mtspr(SPRN_SPEFSCR, __FPU_FPSCR); + current->thread.spefscr_last = __FPU_FPSCR; current->thread.evr[fc] = vc.wp[0]; regs->gpr[fc] = vc.wp[1]; -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/6] powerpc: fix exception clearing in e500 SPE float emulation 2013-11-04 16:52 ` [PATCH 1/6] powerpc: fix exception clearing in e500 SPE float emulation Joseph S. Myers @ 2013-11-22 22:34 ` Scott Wood 2013-11-23 1:22 ` Joseph S. Myers 0 siblings, 1 reply; 17+ messages in thread From: Scott Wood @ 2013-11-22 22:34 UTC (permalink / raw) To: Joseph S. Myers; +Cc: Liu Yu, linuxppc-dev, linux-kernel, Shan Hai On Mon, 2013-11-04 at 16:52 +0000, Joseph S. Myers wrote: > From: Joseph Myers <joseph@codesourcery.com> > > The e500 SPE floating-point emulation code clears existing exceptions > (__FPU_FPSCR &= ~FP_EX_MASK;) before ORing in the exceptions from the > emulated operation. However, these exception bits are the "sticky", > cumulative exception bits, and should only be cleared by the user > program setting SPEFSCR, not implicitly by any floating-point > instruction (whether executed purely by the hardware or emulated). > The spurious clearing of these bits shows up as missing exceptions in > glibc testing. > > Fixing this, however, is not as simple as just not clearing the bits, > because while the bits may be from previous floating-point operations > (in which case they should not be cleared), the processor can also set > the sticky bits itself before the interrupt for an exception occurs, > and this can happen in cases when IEEE 754 semantics are that the > sticky bit should not be set. Specifically, the "invalid" sticky bit > is set in various cases with non-finite operands, where IEEE 754 > semantics do not involve raising such an exception, and the > "underflow" sticky bit is set in cases of exact underflow, whereas > IEEE 754 semantics are that this flag is set only for inexact > underflow. Thus, for correct emulation the kernel needs to know the > setting of these two sticky bits before the instruction being > emulated. > > When a floating-point operation raises an exception, the kernel can > note the state of the sticky bits immediately afterwards. Some > <fenv.h> functions that affect the state of these bits, such as > fesetenv and feholdexcept, need to use prctl with PR_GET_FPEXC and > PR_SET_FPEXC anyway, and so it is natural to record the state of those > bits during that call into the kernel and so avoid any need for a > separate call into the kernel to inform it of a change to those bits. > Thus, the interface I chose to use (in this patch and the glibc port) > is that one of those prctl calls must be made after any userspace > change to those sticky bits, other than through a floating-point > operation that traps into the kernel anyway. This sounds like an incompatible change to userspace API. What about older glibc? What about user code that directly manipulates these bits rather than going through libc, or uses a libc other than glibc? Where is this API requirement documented? I think the impact of this could be reduced by using this mechanism only to clear bits, rather than set them. That is, if the exception bit is unset, don't set it just because it's set in spefscr_last -- but if it's not set in spefscr_last, and the emulation code doesn't want to set it, then clear it. Are there any cases where the exception bit can be set without the kernel taking a trap, or is userspace manipulation limited to clearing the bits? -Scott ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/6] powerpc: fix exception clearing in e500 SPE float emulation 2013-11-22 22:34 ` Scott Wood @ 2013-11-23 1:22 ` Joseph S. Myers 2013-12-07 0:32 ` Scott Wood 0 siblings, 1 reply; 17+ messages in thread From: Joseph S. Myers @ 2013-11-23 1:22 UTC (permalink / raw) To: Scott Wood; +Cc: Liu Yu, linuxppc-dev, linux-kernel, Shan Hai On Fri, 22 Nov 2013, Scott Wood wrote: > This sounds like an incompatible change to userspace API. What about > older glibc? What about user code that directly manipulates these bits > rather than going through libc, or uses a libc other than glibc? Where > is this API requirement documented? The previous EGLIBC port, and the uClibc code copied from it, is fundamentally broken as regards any use of prctl for floating-point exceptions because it didn't use the PR_FP_EXC_SW_ENABLE bit in its prctl calls (and did various worse things, such as passing a pointer when prctl expected an integer). If you avoid anything where prctl is used, the clearing of sticky bits still means it will never give anything approximating correct exception semantics with existing kernels. I don't believe the patch makes things any worse for existing code that doesn't try to inform the kernel of changes to sticky bits - such code may get incorrect exceptions in some cases, but it would have done so anyway in other cases. This is the best API I could come up with to fix the fundamentally broken nature of what came before, taking into account that in many cases a prctl call is already needed along with userspace manipulation of exception bits. I'm not aware of any kernel documentation where this sort of subarchitecture-specific API detail is documented. (The API also includes such things as needing to leave the spefscr trap-enable bits set and use prctl to control whether SIGFPE results from exceptions.) > I think the impact of this could be reduced by using this mechanism only > to clear bits, rather than set them. That is, if the exception bit is > unset, don't set it just because it's set in spefscr_last -- but if it's > not set in spefscr_last, and the emulation code doesn't want to set it, > then clear it. It should already be the case in this patch that if a bit is clear in spefscr, and set in spefscr_last (i.e. userspace did not inform the kernel of clearing the bit, and no traps since then have resulted in the kernel noticing it was cleared), it won't get set unless the emulation code wants to set it. The sole place spefscr_last is read is in the statement "__FPU_FPSCR &= ~(FP_EX_INVALID | FP_EX_UNDERFLOW) | current->thread.spefscr_last;" - if the bit is already clear in spefscr, this statement has no effect on it. > Are there any cases where the exception bit can be set without the > kernel taking a trap, or is userspace manipulation limited to clearing > the bits? Userspace can both set and clear the bits without a trap. For example, fesetenv restores a saved value of spefscr which may both set and clear bits (and then it calls prctl because it needs to do so anyway to restore the saved state for which exceptions were enabled). fesetexceptflag restores saved state of particular exceptions without a trap (so needs to call prctl specially to inform the kernel of a change). -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/6] powerpc: fix exception clearing in e500 SPE float emulation 2013-11-23 1:22 ` Joseph S. Myers @ 2013-12-07 0:32 ` Scott Wood 2013-12-07 0:48 ` questions: second of the 2 pcie controllers does not scan the bus Ruchika 2013-12-10 23:07 ` [PATCHv2 1/6] powerpc: fix exception clearing in e500 SPE float emulation Joseph S. Myers 0 siblings, 2 replies; 17+ messages in thread From: Scott Wood @ 2013-12-07 0:32 UTC (permalink / raw) To: Joseph S. Myers; +Cc: Liu Yu, linuxppc-dev, linux-kernel, Shan Hai On Sat, 2013-11-23 at 01:22 +0000, Joseph S. Myers wrote: > On Fri, 22 Nov 2013, Scott Wood wrote: > > > This sounds like an incompatible change to userspace API. What about > > older glibc? What about user code that directly manipulates these bits > > rather than going through libc, or uses a libc other than glibc? Where > > is this API requirement documented? > > The previous EGLIBC port, and the uClibc code copied from it, is > fundamentally broken as regards any use of prctl for floating-point > exceptions because it didn't use the PR_FP_EXC_SW_ENABLE bit in its prctl > calls (and did various worse things, such as passing a pointer when prctl > expected an integer). If you avoid anything where prctl is used, the > clearing of sticky bits still means it will never give anything > approximating correct exception semantics with existing kernels. I don't > believe the patch makes things any worse for existing code that doesn't > try to inform the kernel of changes to sticky bits - such code may get > incorrect exceptions in some cases, but it would have done so anyway in > other cases. OK -- please mention this in the changelog. > This is the best API I could come up with to fix the fundamentally broken > nature of what came before, taking into account that in many cases a prctl > call is already needed along with userspace manipulation of exception > bits. I'm not aware of any kernel documentation where this sort of > subarchitecture-specific API detail is documented. (The API also includes > such things as needing to leave the spefscr trap-enable bits set and use > prctl to control whether SIGFPE results from exceptions.) I don't know of a formal place for it, but there should at least be a code comment somewhere. > > I think the impact of this could be reduced by using this mechanism only > > to clear bits, rather than set them. That is, if the exception bit is > > unset, don't set it just because it's set in spefscr_last -- but if it's > > not set in spefscr_last, and the emulation code doesn't want to set it, > > then clear it. > > It should already be the case in this patch that if a bit is clear in > spefscr, and set in spefscr_last (i.e. userspace did not inform the kernel > of clearing the bit, and no traps since then have resulted in the kernel > noticing it was cleared), it won't get set unless the emulation code wants > to set it. The sole place spefscr_last is read is in the statement > "__FPU_FPSCR &= ~(FP_EX_INVALID | FP_EX_UNDERFLOW) | > current->thread.spefscr_last;" - if the bit is already clear in spefscr, > this statement has no effect on it. OK -- I must have misread it before. -Scott ^ permalink raw reply [flat|nested] 17+ messages in thread
* questions: second of the 2 pcie controllers does not scan the bus. 2013-12-07 0:32 ` Scott Wood @ 2013-12-07 0:48 ` Ruchika 2013-12-07 13:26 ` Ruchika 2013-12-09 22:50 ` Scott Wood 2013-12-10 23:07 ` [PATCHv2 1/6] powerpc: fix exception clearing in e500 SPE float emulation Joseph S. Myers 1 sibling, 2 replies; 17+ messages in thread From: Ruchika @ 2013-12-07 0:48 UTC (permalink / raw) Cc: linuxppc-dev Hi, I am working with an p4080 based board. I am trying to get 2 PCIE controllers probed properly. In uboot I have no problems scanning and discovering what is connected to both controllers/PCI bridges. For both PCIE1/2 uboot sets up the Primary, secondary and Subordinate bus numbers to 0,1,1 respectively. When linux boots up and probes the controllers, PCIE1 is probed and the bridge scanned properly but PCIE2 is probed at the bridge but not attempted a scan. I see this message "pci 0001:02:00.0: bridge configuration invalid ([bus 01-01]), reconfiguring " I updated uboot to set the secondary and subordinate numbers to 2 (left the primary number to 0) and a subsequent kernel boot scanned the bus for PCIE2 successfully. I found these numbers to be very critical since the device tree blob (bus-range) for pci is also based off these. I'd like to get a good fix rather than the uboot hack and get better understanding of the problem. If there are any pointers someone could provide it would be awesome. Thank you Regards Ruchika ^ permalink raw reply [flat|nested] 17+ messages in thread
* questions: second of the 2 pcie controllers does not scan the bus. 2013-12-07 0:48 ` questions: second of the 2 pcie controllers does not scan the bus Ruchika @ 2013-12-07 13:26 ` Ruchika 2013-12-09 22:50 ` Scott Wood 1 sibling, 0 replies; 17+ messages in thread From: Ruchika @ 2013-12-07 13:26 UTC (permalink / raw) Cc: linuxppc-dev Any ideas/pointers anyone ? Thank you On 12/6/2013 6:48 PM, Ruchika wrote: > > Hi, > I am working with an p4080 based board. I am trying to get 2 PCIE > controllers probed properly. > > In uboot I have no problems scanning and discovering what is connected > to both controllers/PCI bridges. > > For both PCIE1/2 uboot sets up the Primary, secondary and Subordinate > bus numbers to 0,1,1 respectively. > > When linux boots up and probes the controllers, PCIE1 is probed and the > bridge scanned properly but PCIE2 is probed at the bridge but not > attempted a scan. > I see this message > "pci 0001:02:00.0: bridge configuration invalid ([bus 01-01]), > reconfiguring > " > > I updated uboot to set the secondary and subordinate numbers to 2 (left > the primary number to 0) and a subsequent kernel boot scanned the bus > for PCIE2 successfully. > I found these numbers to be very critical since the device tree blob > (bus-range) for pci is also based off these. > > I'd like to get a good fix rather than the uboot hack and get better > understanding of the problem. If there are any pointers someone could > provide it would be awesome. > > Thank you > Regards > Ruchika > > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: questions: second of the 2 pcie controllers does not scan the bus. 2013-12-07 0:48 ` questions: second of the 2 pcie controllers does not scan the bus Ruchika 2013-12-07 13:26 ` Ruchika @ 2013-12-09 22:50 ` Scott Wood 1 sibling, 0 replies; 17+ messages in thread From: Scott Wood @ 2013-12-09 22:50 UTC (permalink / raw) To: Ruchika; +Cc: linuxppc-dev On Fri, 2013-12-06 at 18:48 -0600, Ruchika wrote: > Hi, > I am working with an p4080 based board. I am trying to get 2 PCIE > controllers probed properly. > > In uboot I have no problems scanning and discovering what is connected > to both controllers/PCI bridges. > > For both PCIE1/2 uboot sets up the Primary, secondary and Subordinate > bus numbers to 0,1,1 respectively. > > When linux boots up and probes the controllers, PCIE1 is probed and the > bridge scanned properly but PCIE2 is probed at the bridge but not > attempted a scan. > I see this message > "pci 0001:02:00.0: bridge configuration invalid ([bus 01-01]), reconfiguring > " > > I updated uboot to set the secondary and subordinate numbers to 2 (left > the primary number to 0) and a subsequent kernel boot scanned the bus > for PCIE2 successfully. > I found these numbers to be very critical since the device tree blob > (bus-range) for pci is also based off these. > > I'd like to get a good fix rather than the uboot hack and get better > understanding of the problem. If there are any pointers someone could > provide it would be awesome. This is the code that prints that: /* Check if setup is sensible at all */ if (!pass && (primary != bus->number || secondary <= bus->number || secondary > subordinate)) { dev_info(&dev->dev, "bridge configuration invalid ([bus %02x-%0 secondary, subordinate); broken = 1; } Start by printing out more information to determine which of those checks is failing (e.g. what is primary and bus->number). If it turns out that U-Boot is configuring the PCI bus incorrectly, send e-mail to the U-Boot list. Be sure to mention what version of Linux and U-Boot you're using. -Scott ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCHv2 1/6] powerpc: fix exception clearing in e500 SPE float emulation 2013-12-07 0:32 ` Scott Wood 2013-12-07 0:48 ` questions: second of the 2 pcie controllers does not scan the bus Ruchika @ 2013-12-10 23:07 ` Joseph S. Myers 1 sibling, 0 replies; 17+ messages in thread From: Joseph S. Myers @ 2013-12-10 23:07 UTC (permalink / raw) To: Scott Wood; +Cc: Liu Yu, linuxppc-dev, linux-kernel, Shan Hai From: Joseph Myers <joseph@codesourcery.com> The e500 SPE floating-point emulation code clears existing exceptions (__FPU_FPSCR &= ~FP_EX_MASK;) before ORing in the exceptions from the emulated operation. However, these exception bits are the "sticky", cumulative exception bits, and should only be cleared by the user program setting SPEFSCR, not implicitly by any floating-point instruction (whether executed purely by the hardware or emulated). The spurious clearing of these bits shows up as missing exceptions in glibc testing. Fixing this, however, is not as simple as just not clearing the bits, because while the bits may be from previous floating-point operations (in which case they should not be cleared), the processor can also set the sticky bits itself before the interrupt for an exception occurs, and this can happen in cases when IEEE 754 semantics are that the sticky bit should not be set. Specifically, the "invalid" sticky bit is set in various cases with non-finite operands, where IEEE 754 semantics do not involve raising such an exception, and the "underflow" sticky bit is set in cases of exact underflow, whereas IEEE 754 semantics are that this flag is set only for inexact underflow. Thus, for correct emulation the kernel needs to know the setting of these two sticky bits before the instruction being emulated. When a floating-point operation raises an exception, the kernel can note the state of the sticky bits immediately afterwards. Some <fenv.h> functions that affect the state of these bits, such as fesetenv and feholdexcept, need to use prctl with PR_GET_FPEXC and PR_SET_FPEXC anyway, and so it is natural to record the state of those bits during that call into the kernel and so avoid any need for a separate call into the kernel to inform it of a change to those bits. Thus, the interface I chose to use (in this patch and the glibc port) is that one of those prctl calls must be made after any userspace change to those sticky bits, other than through a floating-point operation that traps into the kernel anyway. feclearexcept and fesetexceptflag duly make those calls, which would not be required were it not for this issue. The previous EGLIBC port, and the uClibc code copied from it, is fundamentally broken as regards any use of prctl for floating-point exceptions because it didn't use the PR_FP_EXC_SW_ENABLE bit in its prctl calls (and did various worse things, such as passing a pointer when prctl expected an integer). If you avoid anything where prctl is used, the clearing of sticky bits still means it will never give anything approximating correct exception semantics with existing kernels. I don't believe the patch makes things any worse for existing code that doesn't try to inform the kernel of changes to sticky bits - such code may get incorrect exceptions in some cases, but it would have done so anyway in other cases. Signed-off-by: Joseph Myers <joseph@codesourcery.com> --- > OK -- please mention this in the changelog. The description of brokenness of existing code attempting to use floating-point exceptions on e500 is now included above. > I don't know of a formal place for it, but there should at least be a > code comment somewhere. This version now adds a comment alongside the relevant settings of spefscr_last. The other patches in this series (2-6) remain independent of this one (and as previously noted, with no dependencies except that patch 5 depends on patch 2) and so are not resent. diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h index fc14a38..91441d9 100644 --- a/arch/powerpc/include/asm/processor.h +++ b/arch/powerpc/include/asm/processor.h @@ -256,6 +256,8 @@ struct thread_struct { unsigned long evr[32]; /* upper 32-bits of SPE regs */ u64 acc; /* Accumulator */ unsigned long spefscr; /* SPE & eFP status */ + unsigned long spefscr_last; /* SPEFSCR value on last prctl + call or trap return */ int used_spe; /* set if process has used spe */ #endif /* CONFIG_SPE */ #ifdef CONFIG_PPC_TRANSACTIONAL_MEM @@ -317,7 +319,9 @@ struct thread_struct { (_ALIGN_UP(sizeof(init_thread_info), 16) + (unsigned long) &init_stack) #ifdef CONFIG_SPE -#define SPEFSCR_INIT .spefscr = SPEFSCR_FINVE | SPEFSCR_FDBZE | SPEFSCR_FUNFE | SPEFSCR_FOVFE, +#define SPEFSCR_INIT \ + .spefscr = SPEFSCR_FINVE | SPEFSCR_FDBZE | SPEFSCR_FUNFE | SPEFSCR_FOVFE, \ + .spefscr_last = SPEFSCR_FINVE | SPEFSCR_FDBZE | SPEFSCR_FUNFE | SPEFSCR_FOVFE, #else #define SPEFSCR_INIT #endif diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c index 3386d8a..b08c0d0 100644 --- a/arch/powerpc/kernel/process.c +++ b/arch/powerpc/kernel/process.c @@ -1175,6 +1175,19 @@ int set_fpexc_mode(struct task_struct *tsk, unsigned int val) if (val & PR_FP_EXC_SW_ENABLE) { #ifdef CONFIG_SPE if (cpu_has_feature(CPU_FTR_SPE)) { + /* + * When the sticky exception bits are set + * directly by userspace, it must call prctl + * with PR_GET_FPEXC (with PR_FP_EXC_SW_ENABLE + * in the existing prctl settings) or + * PR_SET_FPEXC (with PR_FP_EXC_SW_ENABLE in + * the bits being set). <fenv.h> functions + * saving and restoring the whole + * floating-point environment need to do so + * anyway to restore the prctl settings from + * the saved environment. + */ + tsk->thread.spefscr_last = mfspr(SPRN_SPEFSCR); tsk->thread.fpexc_mode = val & (PR_FP_EXC_SW_ENABLE | PR_FP_ALL_EXCEPT); return 0; @@ -1206,9 +1219,22 @@ int get_fpexc_mode(struct task_struct *tsk, unsigned long adr) if (tsk->thread.fpexc_mode & PR_FP_EXC_SW_ENABLE) #ifdef CONFIG_SPE - if (cpu_has_feature(CPU_FTR_SPE)) + if (cpu_has_feature(CPU_FTR_SPE)) { + /* + * When the sticky exception bits are set + * directly by userspace, it must call prctl + * with PR_GET_FPEXC (with PR_FP_EXC_SW_ENABLE + * in the existing prctl settings) or + * PR_SET_FPEXC (with PR_FP_EXC_SW_ENABLE in + * the bits being set). <fenv.h> functions + * saving and restoring the whole + * floating-point environment need to do so + * anyway to restore the prctl settings from + * the saved environment. + */ + tsk->thread.spefscr_last = mfspr(SPRN_SPEFSCR); val = tsk->thread.fpexc_mode; - else + } else return -EINVAL; #else return -EINVAL; diff --git a/arch/powerpc/math-emu/math_efp.c b/arch/powerpc/math-emu/math_efp.c index a73f088..59835c6 100644 --- a/arch/powerpc/math-emu/math_efp.c +++ b/arch/powerpc/math-emu/math_efp.c @@ -630,9 +630,27 @@ update_ccr: regs->ccr |= (IR << ((7 - ((speinsn >> 23) & 0x7)) << 2)); update_regs: - __FPU_FPSCR &= ~FP_EX_MASK; + /* + * If the "invalid" exception sticky bit was set by the + * processor for non-finite input, but was not set before the + * instruction being emulated, clear it. Likewise for the + * "underflow" bit, which may have been set by the processor + * for exact underflow, not just inexact underflow when the + * flag should be set for IEEE 754 semantics. Other sticky + * exceptions will only be set by the processor when they are + * correct according to IEEE 754 semantics, and we must not + * clear sticky bits that were already set before the emulated + * instruction as they represent the user-visible sticky + * exception status. "inexact" traps to kernel are not + * required for IEEE semantics and are not enabled by default, + * so the "inexact" sticky bit may have been set by a previous + * instruction without the kernel being aware of it. + */ + __FPU_FPSCR + &= ~(FP_EX_INVALID | FP_EX_UNDERFLOW) | current->thread.spefscr_last; __FPU_FPSCR |= (FP_CUR_EXCEPTIONS & FP_EX_MASK); mtspr(SPRN_SPEFSCR, __FPU_FPSCR); + current->thread.spefscr_last = __FPU_FPSCR; current->thread.evr[fc] = vc.wp[0]; regs->gpr[fc] = vc.wp[1]; -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/6] powerpc: fix e500 SPE float rounding inexactness detection 2013-11-04 16:50 [PATCH 0/6] powerpc/math-emu: e500 SPE float emulation fixes Joseph S. Myers 2013-11-04 16:52 ` [PATCH 1/6] powerpc: fix exception clearing in e500 SPE float emulation Joseph S. Myers @ 2013-11-04 16:53 ` Joseph S. Myers 2013-11-04 16:53 ` [PATCH 3/6] math-emu: fix floating-point to integer unsigned saturation Joseph S. Myers ` (4 subsequent siblings) 6 siblings, 0 replies; 17+ messages in thread From: Joseph S. Myers @ 2013-11-04 16:53 UTC (permalink / raw) To: linuxppc-dev; +Cc: Liu Yu, linux-kernel, Shan Hai From: Joseph Myers <joseph@codesourcery.com> The e500 SPE floating-point emulation code for the rounding modes rounding to positive or negative infinity (which may not be implemented in hardware) tries to avoid emulating rounding if the result was inexact. However, it tests inexactness using the sticky bit with the cumulative result of previous operations, rather than with the non-sticky bits relating to the operation that generated the interrupt. Furthermore, when a vector operation generates the interrupt, it's possible that only one of the low and high parts is inexact, and so only that part should have rounding emulated. This results in incorrect rounding of exact results in these modes when the sticky bit is set from a previous operation. (I'm not sure why the rounding interrupts are generated at all when the result is exact, but empirically the hardware does generate them.) This patch checks for inexactness using the correct bits of SPEFSCR, and ensures that rounding only occurs when the relevant part of the result was actually inexact. Signed-off-by: Joseph Myers <joseph@codesourcery.com> --- Previous submission: <http://lkml.org/lkml/2013/10/4/497>. diff --git a/arch/powerpc/math-emu/math_efp.c b/arch/powerpc/math-emu/math_efp.c index 59835c6..ecdf35d 100644 --- a/arch/powerpc/math-emu/math_efp.c +++ b/arch/powerpc/math-emu/math_efp.c @@ -680,7 +680,8 @@ int speround_handler(struct pt_regs *regs) { union dw_union fgpr; int s_lo, s_hi; - unsigned long speinsn, type, fc; + int lo_inexact, hi_inexact; + unsigned long speinsn, type, fc, fptype; if (get_user(speinsn, (unsigned int __user *) regs->nip)) return -EFAULT; @@ -693,8 +694,12 @@ int speround_handler(struct pt_regs *regs) __FPU_FPSCR = mfspr(SPRN_SPEFSCR); pr_debug("speinsn:%08lx spefscr:%08lx\n", speinsn, __FPU_FPSCR); + fptype = (speinsn >> 5) & 0x7; + /* No need to round if the result is exact */ - if (!(__FPU_FPSCR & FP_EX_INEXACT)) + lo_inexact = __FPU_FPSCR & (SPEFSCR_FG | SPEFSCR_FX); + hi_inexact = __FPU_FPSCR & (SPEFSCR_FGH | SPEFSCR_FXH); + if (!(lo_inexact || (hi_inexact && fptype == VCT))) return 0; fc = (speinsn >> 21) & 0x1f; @@ -705,7 +710,7 @@ int speround_handler(struct pt_regs *regs) pr_debug("round fgpr: %08x %08x\n", fgpr.wp[0], fgpr.wp[1]); - switch ((speinsn >> 5) & 0x7) { + switch (fptype) { /* Since SPE instructions on E500 core can handle round to nearest * and round toward zero with IEEE-754 complied, we just need * to handle round toward +Inf and round toward -Inf by software. @@ -728,11 +733,15 @@ int speround_handler(struct pt_regs *regs) case VCT: if (FP_ROUNDMODE == FP_RND_PINF) { - if (!s_lo) fgpr.wp[1]++; /* Z_low > 0, choose Z1 */ - if (!s_hi) fgpr.wp[0]++; /* Z_high word > 0, choose Z1 */ + if (lo_inexact && !s_lo) + fgpr.wp[1]++; /* Z_low > 0, choose Z1 */ + if (hi_inexact && !s_hi) + fgpr.wp[0]++; /* Z_high word > 0, choose Z1 */ } else { /* round to -Inf */ - if (s_lo) fgpr.wp[1]++; /* Z_low < 0, choose Z2 */ - if (s_hi) fgpr.wp[0]++; /* Z_high < 0, choose Z2 */ + if (lo_inexact && s_lo) + fgpr.wp[1]++; /* Z_low < 0, choose Z2 */ + if (hi_inexact && s_hi) + fgpr.wp[0]++; /* Z_high < 0, choose Z2 */ } break; -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/6] math-emu: fix floating-point to integer unsigned saturation 2013-11-04 16:50 [PATCH 0/6] powerpc/math-emu: e500 SPE float emulation fixes Joseph S. Myers 2013-11-04 16:52 ` [PATCH 1/6] powerpc: fix exception clearing in e500 SPE float emulation Joseph S. Myers 2013-11-04 16:53 ` [PATCH 2/6] powerpc: fix e500 SPE float rounding inexactness detection Joseph S. Myers @ 2013-11-04 16:53 ` Joseph S. Myers 2013-11-04 16:54 ` [PATCH 4/6] math-emu: fix floating-point to integer overflow detection Joseph S. Myers ` (3 subsequent siblings) 6 siblings, 0 replies; 17+ messages in thread From: Joseph S. Myers @ 2013-11-04 16:53 UTC (permalink / raw) To: linuxppc-dev; +Cc: Liu Yu, linux-kernel, Shan Hai From: Joseph Myers <joseph@codesourcery.com> The math-emu macros _FP_TO_INT and _FP_TO_INT_ROUND are supposed to saturate their results for out-of-range arguments, except in the case rsigned == 2 (when instead the low bits of the result are taken). However, in the case rsigned == 0 (converting to unsigned integers), they mistakenly produce 0 for positive results and the maximum unsigned integer for negative results, the opposite of correct unsigned saturation. This patch fixes the logic. Signed-off-by: Joseph Myers <joseph@codesourcery.com> --- Previous submission: <http://lkml.org/lkml/2013/10/8/694>. I have made the corresponding changes to the glibc/libgcc copy of this code, given that it would be desirable to resync the Linux and glibc/libgcc copies (the latter has had many enhancements and bug fixes since it was copied into Linux), although strictly this incorrect saturation is only a bug when trying to emulate particular instruction semantics, not when used in userspace to implement C operations where the results of out-of-range conversions are unspecified or undefined. diff --git a/include/math-emu/op-common.h b/include/math-emu/op-common.h index 9696a5e..70fe5e9 100644 --- a/include/math-emu/op-common.h +++ b/include/math-emu/op-common.h @@ -685,7 +685,7 @@ do { \ else \ { \ r = 0; \ - if (X##_s) \ + if (!X##_s) \ r = ~r; \ } \ FP_SET_EXCEPTION(FP_EX_INVALID); \ @@ -762,7 +762,7 @@ do { \ if (!rsigned) \ { \ r = 0; \ - if (X##_s) \ + if (!X##_s) \ r = ~r; \ } \ else if (rsigned != 2) \ -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/6] math-emu: fix floating-point to integer overflow detection 2013-11-04 16:50 [PATCH 0/6] powerpc/math-emu: e500 SPE float emulation fixes Joseph S. Myers ` (2 preceding siblings ...) 2013-11-04 16:53 ` [PATCH 3/6] math-emu: fix floating-point to integer unsigned saturation Joseph S. Myers @ 2013-11-04 16:54 ` Joseph S. Myers 2013-11-04 16:54 ` [PATCH 5/6] powerpc: fix e500 SPE float to integer and fixed-point conversions Joseph S. Myers ` (2 subsequent siblings) 6 siblings, 0 replies; 17+ messages in thread From: Joseph S. Myers @ 2013-11-04 16:54 UTC (permalink / raw) To: linuxppc-dev; +Cc: Liu Yu, linux-kernel, Shan Hai From: Joseph Myers <joseph@codesourcery.com> On overflow, the math-emu macro _FP_TO_INT_ROUND tries to saturate its result (subject to the value of rsigned specifying the desired overflow semantics). However, if the rounding step has the effect of increasing the exponent so as to cause overflow (if the rounded result is 1 larger than the largest positive value with the given number of bits, allowing for signedness), the overflow does not get detected, meaning that for unsigned results 0 is produced instead of the maximum unsigned integer with the give number of bits, without an exception being raised for overflow, and that for signed results the minimum (negative) value is produced instead of the maximum (positive) value, again without an exception. This patch makes the code check for rounding increasing the exponent and adjusts the exponent value as needed for the overflow check. Signed-off-by: Joseph Myers <joseph@codesourcery.com> --- Previous submission: <http://lkml.org/lkml/2013/10/8/700>. This macro is not present in the glibc/libgcc version of the code. It remains the case both before and after this patch that the conversions wrongly treat a signed result of the most negative integer as an overflow, when actually only that integer minus 1 or smaller should be an overflow, although this only means an incorrect exception rather than affecting the value returned; that was one of the bugs I fixed in the glibc/libgcc version of this code in 2006 (as part of a major overhaul of the code including various interface changes, so not trivially backportable to the kernel version). diff --git a/include/math-emu/op-common.h b/include/math-emu/op-common.h index 70fe5e9..6bdf8c6 100644 --- a/include/math-emu/op-common.h +++ b/include/math-emu/op-common.h @@ -743,12 +743,17 @@ do { \ } \ else \ { \ + int _lz0, _lz1; \ if (X##_e <= -_FP_WORKBITS - 1) \ _FP_FRAC_SET_##wc(X, _FP_MINFRAC_##wc); \ else \ _FP_FRAC_SRS_##wc(X, _FP_FRACBITS_##fs - 1 - X##_e, \ _FP_WFRACBITS_##fs); \ + _FP_FRAC_CLZ_##wc(_lz0, X); \ _FP_ROUND(wc, X); \ + _FP_FRAC_CLZ_##wc(_lz1, X); \ + if (_lz1 < _lz0) \ + X##_e++; /* For overflow detection. */ \ _FP_FRAC_SRL_##wc(X, _FP_WORKBITS); \ _FP_FRAC_ASSEMBLE_##wc(r, X, rsize); \ } \ -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 5/6] powerpc: fix e500 SPE float to integer and fixed-point conversions 2013-11-04 16:50 [PATCH 0/6] powerpc/math-emu: e500 SPE float emulation fixes Joseph S. Myers ` (3 preceding siblings ...) 2013-11-04 16:54 ` [PATCH 4/6] math-emu: fix floating-point to integer overflow detection Joseph S. Myers @ 2013-11-04 16:54 ` Joseph S. Myers 2013-11-04 16:55 ` [PATCH 6/6] powerpc: fix e500 SPE float SIGFPE generation Joseph S. Myers 2013-11-11 14:29 ` Ping Re: [PATCH 0/6] powerpc/math-emu: e500 SPE float emulation fixes Joseph S. Myers 6 siblings, 0 replies; 17+ messages in thread From: Joseph S. Myers @ 2013-11-04 16:54 UTC (permalink / raw) To: linuxppc-dev; +Cc: Liu Yu, linux-kernel, Shan Hai From: Joseph Myers <joseph@codesourcery.com> The e500 SPE floating-point emulation code has several problems in how it handles conversions to integer and fixed-point fractional types. There are the following 20 relevant instructions. These can convert to signed or unsigned 32-bit integers, either rounding towards zero (as correct for C casts from floating-point to integer) or according to the current rounding mode, or to signed or unsigned 32-bit fixed-point values (values in the range [-1, 1) or [0, 1)). For conversion from double precision there are also instructions to convert to 64-bit integers, rounding towards zero, although as far as I know those instructions are completely theoretical (they are only defined for implementations that support both SPE and classic 64-bit, and I'm not aware of any such hardware even though the architecture definition permits that combination). #define EFSCTUI 0x2d4 #define EFSCTSI 0x2d5 #define EFSCTUF 0x2d6 #define EFSCTSF 0x2d7 #define EFSCTUIZ 0x2d8 #define EFSCTSIZ 0x2da #define EVFSCTUI 0x294 #define EVFSCTSI 0x295 #define EVFSCTUF 0x296 #define EVFSCTSF 0x297 #define EVFSCTUIZ 0x298 #define EVFSCTSIZ 0x29a #define EFDCTUIDZ 0x2ea #define EFDCTSIDZ 0x2eb #define EFDCTUI 0x2f4 #define EFDCTSI 0x2f5 #define EFDCTUF 0x2f6 #define EFDCTSF 0x2f7 #define EFDCTUIZ 0x2f8 #define EFDCTSIZ 0x2fa The emulation code, for the instructions that come in variants rounding either towards zero or according to the current rounding direction, uses "if (func & 0x4)" as a condition for using _FP_ROUND (otherwise _FP_ROUND_ZERO is used). The condition is correct, but the code it controls isn't. Whether _FP_ROUND or _FP_ROUND_ZERO is used makes no difference, as the effect of those soft-fp macros is to round an intermediate floating-point result using the low three bits (the last one sticky) of the working format. As these operations are dealing with a freshly unpacked floating-point input, those low bits are zero and no rounding occurs. The emulation code then uses the FP_TO_INT_* macros for the actual integer conversion, with the effect of always rounding towards zero; for rounding according to the current rounding direction, it should be using FP_TO_INT_ROUND_*. The instructions in question have semantics defined (in the Power ISA documents) for out-of-range values and NaNs: out-of-range values saturate and NaNs are converted to zero. The emulation does nothing to follow those semantics for NaNs (the soft-fp handling is to treat them as infinities), and messes up the saturation semantics. For single-precision conversion to integers, (((func & 0x3) != 0) || SB_s) is the condition used for doing a signed conversion. The first part is correct, but the second isn't: negative numbers should result in saturation to 0 when converted to unsigned. Double-precision conversion to 64-bit integers correctly uses ((func & 0x1) == 0). Double-precision conversion to 32-bit integers uses (((func & 0x3) != 0) || DB_s), with correct first part and incorrect second part. And vector float conversion to integers uses (((func & 0x3) != 0) || SB0_s) (and similar for the other vector element), where the sign bit check is again wrong. The incorrect handling of negative numbers converted to unsigned was introduced in commit afc0a07d4a283599ac3a6a31d7454e9baaeccca0. The rationale given there was a C testcase with cast from float to unsigned int. Conversion of out-of-range floating-point numbers to integer types in C is undefined behavior in the base standard, defined in Annex F to produce an unspecified value. That is, the C testcase used to justify that patch is incorrect - there is no ISO C requirement for a particular value resulting from this conversion - and in any case, the correct semantics for such emulation are the semantics for the instruction (unsigned saturation, which is what it does in hardware when the emulation is disabled). The conversion to fixed-point values has its own problems. That code doesn't try to do a full emulation; it relies on the trap handler only being called for arguments that are infinities, NaNs, subnormal or out of range. That's fine, but the logic ((vb.wp[1] >> 23) == 0xff && ((vb.wp[1] & 0x7fffff) > 0)) for NaN detection won't detect negative NaNs as being NaNs (the same applies for the double-precision case), and subnormals are mapped to 0 rather than respecting the rounding mode; the code should also explicitly raise the "invalid" exception. The code for vectors works by executing the scalar float instruction with the trapping disabled, meaning at least subnormals won't be handled correctly. As well as all those problems in the main emulation code, the rounding handler - used to emulate rounding upward and downward when not supported in hardware and when no higher priority exception occurred - has its own problems. * It gets called in some cases even for the instructions rounding to zero, and then acts according to the current rounding mode when it should just leave alone the truncated result provided by hardware. * It presumes that the result is a single-precision, double-precision or single-precision vector as appropriate for the instruction type, determines the sign of the result accordingly, and then adjusts the result based on that sign and the rounding mode. - In the single-precision cases at least the sign determination for an integer result is the same as for a floating-point result; in the double-precision case, converted to 32-bit integer or fixed point, the sign of a double-precision value is in the high part of the register but it's the low part of the register that has the result of the conversion. - If the result is unsigned fixed-point, its sign may be wrongly determined as negative (does not actually cause problems, because inexact unsigned fixed-point results with the high bit set can only appear when converting from double, in which case the sign determination is instead wrongly using the high part of the register). - If the sign of the result is correctly determined as negative, any adjustment required to change the truncated result to one correct for the rounding mode should be in the opposite direction for two's-complement integers as for sign-magnitude floating-point values. - And if the integer result is zero, the correct sign can only be determined by examining the original operand, and not at all (as far as I can tell) if the operand and result are the same register. This patch fixes all these problems (as far as possible, given the inability to determine the correct sign in the rounding handler when the truncated result is 0, the conversion is to a signed type and the truncated result has overwritten the original operand). Conversion to fixed-point now uses full emulation, and does not use "asm" in the vector case; the semantics are exactly those of converting to integer according to the current rounding direction, once the exponent has been adjusted, so the code makes such an adjustment then uses the FP_TO_INT_ROUND macros. The testcase I used for verifying that the instructions (other than the theoretical conversions to 64-bit integers) produce the correct results is at <http://lkml.org/lkml/2013/10/8/708>. Signed-off-by: Joseph Myers <joseph@codesourcery.com> --- Previous submission: <http://lkml.org/lkml/2013/10/8/705>. diff --git a/arch/powerpc/math-emu/math_efp.c b/arch/powerpc/math-emu/math_efp.c index ecdf35d..01a0abb 100644 --- a/arch/powerpc/math-emu/math_efp.c +++ b/arch/powerpc/math-emu/math_efp.c @@ -275,21 +275,13 @@ int do_spe_mathemu(struct pt_regs *regs) case EFSCTSF: case EFSCTUF: - if (!((vb.wp[1] >> 23) == 0xff && ((vb.wp[1] & 0x7fffff) > 0))) { - /* NaN */ - if (((vb.wp[1] >> 23) & 0xff) == 0) { - /* denorm */ - vc.wp[1] = 0x0; - } else if ((vb.wp[1] >> 31) == 0) { - /* positive normal */ - vc.wp[1] = (func == EFSCTSF) ? - 0x7fffffff : 0xffffffff; - } else { /* negative normal */ - vc.wp[1] = (func == EFSCTSF) ? - 0x80000000 : 0x0; - } - } else { /* rB is NaN */ - vc.wp[1] = 0x0; + if (SB_c == FP_CLS_NAN) { + vc.wp[1] = 0; + FP_SET_EXCEPTION(FP_EX_INVALID); + } else { + SB_e += (func == EFSCTSF ? 31 : 32); + FP_TO_INT_ROUND_S(vc.wp[1], SB, 32, + (func == EFSCTSF)); } goto update_regs; @@ -306,16 +298,25 @@ int do_spe_mathemu(struct pt_regs *regs) } case EFSCTSI: - case EFSCTSIZ: case EFSCTUI: + if (SB_c == FP_CLS_NAN) { + vc.wp[1] = 0; + FP_SET_EXCEPTION(FP_EX_INVALID); + } else { + FP_TO_INT_ROUND_S(vc.wp[1], SB, 32, + ((func & 0x3) != 0)); + } + goto update_regs; + + case EFSCTSIZ: case EFSCTUIZ: - if (func & 0x4) { - _FP_ROUND(1, SB); + if (SB_c == FP_CLS_NAN) { + vc.wp[1] = 0; + FP_SET_EXCEPTION(FP_EX_INVALID); } else { - _FP_ROUND_ZERO(1, SB); + FP_TO_INT_S(vc.wp[1], SB, 32, + ((func & 0x3) != 0)); } - FP_TO_INT_S(vc.wp[1], SB, 32, - (((func & 0x3) != 0) || SB_s)); goto update_regs; default: @@ -404,22 +405,13 @@ cmp_s: case EFDCTSF: case EFDCTUF: - if (!((vb.wp[0] >> 20) == 0x7ff && - ((vb.wp[0] & 0xfffff) > 0 || (vb.wp[1] > 0)))) { - /* not a NaN */ - if (((vb.wp[0] >> 20) & 0x7ff) == 0) { - /* denorm */ - vc.wp[1] = 0x0; - } else if ((vb.wp[0] >> 31) == 0) { - /* positive normal */ - vc.wp[1] = (func == EFDCTSF) ? - 0x7fffffff : 0xffffffff; - } else { /* negative normal */ - vc.wp[1] = (func == EFDCTSF) ? - 0x80000000 : 0x0; - } - } else { /* NaN */ - vc.wp[1] = 0x0; + if (DB_c == FP_CLS_NAN) { + vc.wp[1] = 0; + FP_SET_EXCEPTION(FP_EX_INVALID); + } else { + DB_e += (func == EFDCTSF ? 31 : 32); + FP_TO_INT_ROUND_D(vc.wp[1], DB, 32, + (func == EFDCTSF)); } goto update_regs; @@ -437,21 +429,35 @@ cmp_s: case EFDCTUIDZ: case EFDCTSIDZ: - _FP_ROUND_ZERO(2, DB); - FP_TO_INT_D(vc.dp[0], DB, 64, ((func & 0x1) == 0)); + if (DB_c == FP_CLS_NAN) { + vc.dp[0] = 0; + FP_SET_EXCEPTION(FP_EX_INVALID); + } else { + FP_TO_INT_D(vc.dp[0], DB, 64, + ((func & 0x1) == 0)); + } goto update_regs; case EFDCTUI: case EFDCTSI: + if (DB_c == FP_CLS_NAN) { + vc.wp[1] = 0; + FP_SET_EXCEPTION(FP_EX_INVALID); + } else { + FP_TO_INT_ROUND_D(vc.wp[1], DB, 32, + ((func & 0x3) != 0)); + } + goto update_regs; + case EFDCTUIZ: case EFDCTSIZ: - if (func & 0x4) { - _FP_ROUND(2, DB); + if (DB_c == FP_CLS_NAN) { + vc.wp[1] = 0; + FP_SET_EXCEPTION(FP_EX_INVALID); } else { - _FP_ROUND_ZERO(2, DB); + FP_TO_INT_D(vc.wp[1], DB, 32, + ((func & 0x3) != 0)); } - FP_TO_INT_D(vc.wp[1], DB, 32, - (((func & 0x3) != 0) || DB_s)); goto update_regs; default: @@ -556,37 +562,60 @@ cmp_d: cmp = -1; goto cmp_vs; - case EVFSCTSF: - __asm__ __volatile__ ("mtspr 512, %4\n" - "efsctsf %0, %2\n" - "efsctsf %1, %3\n" - : "=r" (vc.wp[0]), "=r" (vc.wp[1]) - : "r" (vb.wp[0]), "r" (vb.wp[1]), "r" (0)); - goto update_regs; - case EVFSCTUF: - __asm__ __volatile__ ("mtspr 512, %4\n" - "efsctuf %0, %2\n" - "efsctuf %1, %3\n" - : "=r" (vc.wp[0]), "=r" (vc.wp[1]) - : "r" (vb.wp[0]), "r" (vb.wp[1]), "r" (0)); + case EVFSCTSF: + if (SB0_c == FP_CLS_NAN) { + vc.wp[0] = 0; + FP_SET_EXCEPTION(FP_EX_INVALID); + } else { + SB0_e += (func == EVFSCTSF ? 31 : 32); + FP_TO_INT_ROUND_S(vc.wp[0], SB0, 32, + (func == EVFSCTSF)); + } + if (SB1_c == FP_CLS_NAN) { + vc.wp[1] = 0; + FP_SET_EXCEPTION(FP_EX_INVALID); + } else { + SB1_e += (func == EVFSCTSF ? 31 : 32); + FP_TO_INT_ROUND_S(vc.wp[1], SB1, 32, + (func == EVFSCTSF)); + } goto update_regs; case EVFSCTUI: case EVFSCTSI: + if (SB0_c == FP_CLS_NAN) { + vc.wp[0] = 0; + FP_SET_EXCEPTION(FP_EX_INVALID); + } else { + FP_TO_INT_ROUND_S(vc.wp[0], SB0, 32, + ((func & 0x3) != 0)); + } + if (SB1_c == FP_CLS_NAN) { + vc.wp[1] = 0; + FP_SET_EXCEPTION(FP_EX_INVALID); + } else { + FP_TO_INT_ROUND_S(vc.wp[1], SB1, 32, + ((func & 0x3) != 0)); + } + goto update_regs; + case EVFSCTUIZ: case EVFSCTSIZ: - if (func & 0x4) { - _FP_ROUND(1, SB0); - _FP_ROUND(1, SB1); + if (SB0_c == FP_CLS_NAN) { + vc.wp[0] = 0; + FP_SET_EXCEPTION(FP_EX_INVALID); } else { - _FP_ROUND_ZERO(1, SB0); - _FP_ROUND_ZERO(1, SB1); + FP_TO_INT_S(vc.wp[0], SB0, 32, + ((func & 0x3) != 0)); + } + if (SB1_c == FP_CLS_NAN) { + vc.wp[1] = 0; + FP_SET_EXCEPTION(FP_EX_INVALID); + } else { + FP_TO_INT_S(vc.wp[1], SB1, 32, + ((func & 0x3) != 0)); } - FP_TO_INT_S(vc.wp[0], SB0, 32, - (((func & 0x3) != 0) || SB0_s)); - FP_TO_INT_S(vc.wp[1], SB1, 32, - (((func & 0x3) != 0) || SB1_s)); goto update_regs; default: @@ -681,14 +710,16 @@ int speround_handler(struct pt_regs *regs) union dw_union fgpr; int s_lo, s_hi; int lo_inexact, hi_inexact; - unsigned long speinsn, type, fc, fptype; + int fp_result; + unsigned long speinsn, type, fb, fc, fptype, func; if (get_user(speinsn, (unsigned int __user *) regs->nip)) return -EFAULT; if ((speinsn >> 26) != 4) return -EINVAL; /* not an spe instruction */ - type = insn_type(speinsn & 0x7ff); + func = speinsn & 0x7ff; + type = insn_type(func); if (type == XCR) return -ENOSYS; __FPU_FPSCR = mfspr(SPRN_SPEFSCR); @@ -708,6 +739,65 @@ int speround_handler(struct pt_regs *regs) fgpr.wp[0] = current->thread.evr[fc]; fgpr.wp[1] = regs->gpr[fc]; + fb = (speinsn >> 11) & 0x1f; + switch (func) { + case EFSCTUIZ: + case EFSCTSIZ: + case EVFSCTUIZ: + case EVFSCTSIZ: + case EFDCTUIDZ: + case EFDCTSIDZ: + case EFDCTUIZ: + case EFDCTSIZ: + /* + * These instructions always round to zero, + * independent of the rounding mode. + */ + return 0; + + case EFSCTUI: + case EFSCTUF: + case EVFSCTUI: + case EVFSCTUF: + case EFDCTUI: + case EFDCTUF: + fp_result = 0; + s_lo = 0; + s_hi = 0; + break; + + case EFSCTSI: + case EFSCTSF: + fp_result = 0; + /* Recover the sign of a zero result if possible. */ + if (fgpr.wp[1] == 0) + s_lo = regs->gpr[fb] & SIGN_BIT_S; + break; + + case EVFSCTSI: + case EVFSCTSF: + fp_result = 0; + /* Recover the sign of a zero result if possible. */ + if (fgpr.wp[1] == 0) + s_lo = regs->gpr[fb] & SIGN_BIT_S; + if (fgpr.wp[0] == 0) + s_hi = current->thread.evr[fb] & SIGN_BIT_S; + break; + + case EFDCTSI: + case EFDCTSF: + fp_result = 0; + s_hi = s_lo; + /* Recover the sign of a zero result if possible. */ + if (fgpr.wp[1] == 0) + s_hi = current->thread.evr[fb] & SIGN_BIT_S; + break; + + default: + fp_result = 1; + break; + } + pr_debug("round fgpr: %08x %08x\n", fgpr.wp[0], fgpr.wp[1]); switch (fptype) { @@ -719,15 +809,30 @@ int speround_handler(struct pt_regs *regs) if ((FP_ROUNDMODE) == FP_RND_PINF) { if (!s_lo) fgpr.wp[1]++; /* Z > 0, choose Z1 */ } else { /* round to -Inf */ - if (s_lo) fgpr.wp[1]++; /* Z < 0, choose Z2 */ + if (s_lo) { + if (fp_result) + fgpr.wp[1]++; /* Z < 0, choose Z2 */ + else + fgpr.wp[1]--; /* Z < 0, choose Z2 */ + } } break; case DPFP: if (FP_ROUNDMODE == FP_RND_PINF) { - if (!s_hi) fgpr.dp[0]++; /* Z > 0, choose Z1 */ + if (!s_hi) { + if (fp_result) + fgpr.dp[0]++; /* Z > 0, choose Z1 */ + else + fgpr.wp[1]++; /* Z > 0, choose Z1 */ + } } else { /* round to -Inf */ - if (s_hi) fgpr.dp[0]++; /* Z < 0, choose Z2 */ + if (s_hi) { + if (fp_result) + fgpr.dp[0]++; /* Z < 0, choose Z2 */ + else + fgpr.wp[1]--; /* Z < 0, choose Z2 */ + } } break; @@ -738,10 +843,18 @@ int speround_handler(struct pt_regs *regs) if (hi_inexact && !s_hi) fgpr.wp[0]++; /* Z_high word > 0, choose Z1 */ } else { /* round to -Inf */ - if (lo_inexact && s_lo) - fgpr.wp[1]++; /* Z_low < 0, choose Z2 */ - if (hi_inexact && s_hi) - fgpr.wp[0]++; /* Z_high < 0, choose Z2 */ + if (lo_inexact && s_lo) { + if (fp_result) + fgpr.wp[1]++; /* Z_low < 0, choose Z2 */ + else + fgpr.wp[1]--; /* Z_low < 0, choose Z2 */ + } + if (hi_inexact && s_hi) { + if (fp_result) + fgpr.wp[0]++; /* Z_high < 0, choose Z2 */ + else + fgpr.wp[0]--; /* Z_high < 0, choose Z2 */ + } } break; -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 6/6] powerpc: fix e500 SPE float SIGFPE generation 2013-11-04 16:50 [PATCH 0/6] powerpc/math-emu: e500 SPE float emulation fixes Joseph S. Myers ` (4 preceding siblings ...) 2013-11-04 16:54 ` [PATCH 5/6] powerpc: fix e500 SPE float to integer and fixed-point conversions Joseph S. Myers @ 2013-11-04 16:55 ` Joseph S. Myers 2013-11-11 14:29 ` Ping Re: [PATCH 0/6] powerpc/math-emu: e500 SPE float emulation fixes Joseph S. Myers 6 siblings, 0 replies; 17+ messages in thread From: Joseph S. Myers @ 2013-11-04 16:55 UTC (permalink / raw) To: linuxppc-dev; +Cc: Liu Yu, linux-kernel, Shan Hai From: Joseph Myers <joseph@codesourcery.com> The e500 SPE floating-point emulation code is called from SPEFloatingPointException and SPEFloatingPointRoundException in arch/powerpc/kernel/traps.c. Those functions have support for generating SIGFPE, but do_spe_mathemu and speround_handler don't generate a return value to indicate that this should be done. Such a return value should depend on whether an exception is raised that has been set via prctl to generate SIGFPE. This patch adds the relevant logic in these functions so that SIGFPE is generated as expected by the glibc testsuite. Signed-off-by: Joseph Myers <joseph@codesourcery.com> --- Previous submission: <http://lkml.org/lkml/2013/10/10/626>. diff --git a/arch/powerpc/math-emu/math_efp.c b/arch/powerpc/math-emu/math_efp.c index 01a0abb..28337c9 100644 --- a/arch/powerpc/math-emu/math_efp.c +++ b/arch/powerpc/math-emu/math_efp.c @@ -20,6 +20,7 @@ */ #include <linux/types.h> +#include <linux/prctl.h> #include <asm/uaccess.h> #include <asm/reg.h> @@ -691,6 +692,23 @@ update_regs: pr_debug("va: %08x %08x\n", va.wp[0], va.wp[1]); pr_debug("vb: %08x %08x\n", vb.wp[0], vb.wp[1]); + if (current->thread.fpexc_mode & PR_FP_EXC_SW_ENABLE) { + if ((FP_CUR_EXCEPTIONS & FP_EX_DIVZERO) + && (current->thread.fpexc_mode & PR_FP_EXC_DIV)) + return 1; + if ((FP_CUR_EXCEPTIONS & FP_EX_OVERFLOW) + && (current->thread.fpexc_mode & PR_FP_EXC_OVF)) + return 1; + if ((FP_CUR_EXCEPTIONS & FP_EX_UNDERFLOW) + && (current->thread.fpexc_mode & PR_FP_EXC_UND)) + return 1; + if ((FP_CUR_EXCEPTIONS & FP_EX_INEXACT) + && (current->thread.fpexc_mode & PR_FP_EXC_RES)) + return 1; + if ((FP_CUR_EXCEPTIONS & FP_EX_INVALID) + && (current->thread.fpexc_mode & PR_FP_EXC_INV)) + return 1; + } return 0; illegal: @@ -867,6 +885,8 @@ int speround_handler(struct pt_regs *regs) pr_debug(" to fgpr: %08x %08x\n", fgpr.wp[0], fgpr.wp[1]); + if (current->thread.fpexc_mode & PR_FP_EXC_SW_ENABLE) + return (current->thread.fpexc_mode & PR_FP_EXC_RES) ? 1 : 0; return 0; } -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Ping Re: [PATCH 0/6] powerpc/math-emu: e500 SPE float emulation fixes 2013-11-04 16:50 [PATCH 0/6] powerpc/math-emu: e500 SPE float emulation fixes Joseph S. Myers ` (5 preceding siblings ...) 2013-11-04 16:55 ` [PATCH 6/6] powerpc: fix e500 SPE float SIGFPE generation Joseph S. Myers @ 2013-11-11 14:29 ` Joseph S. Myers 2013-11-18 14:54 ` Ping^2 " Joseph S. Myers 6 siblings, 1 reply; 17+ messages in thread From: Joseph S. Myers @ 2013-11-11 14:29 UTC (permalink / raw) To: linuxppc-dev; +Cc: Liu Yu, linux-kernel, Shan Hai Ping. I haven't seen any comments on any of these patches. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* Ping^2 Re: [PATCH 0/6] powerpc/math-emu: e500 SPE float emulation fixes 2013-11-11 14:29 ` Ping Re: [PATCH 0/6] powerpc/math-emu: e500 SPE float emulation fixes Joseph S. Myers @ 2013-11-18 14:54 ` Joseph S. Myers 2013-11-18 19:07 ` Scott Wood 0 siblings, 1 reply; 17+ messages in thread From: Joseph S. Myers @ 2013-11-18 14:54 UTC (permalink / raw) To: linuxppc-dev; +Cc: Liu Yu, linux-kernel, Shan Hai Ping^2. I still haven't seen any comments on any of these patches. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Ping^2 Re: [PATCH 0/6] powerpc/math-emu: e500 SPE float emulation fixes 2013-11-18 14:54 ` Ping^2 " Joseph S. Myers @ 2013-11-18 19:07 ` Scott Wood 0 siblings, 0 replies; 17+ messages in thread From: Scott Wood @ 2013-11-18 19:07 UTC (permalink / raw) To: Joseph S. Myers; +Cc: Liu Yu, linuxppc-dev, linux-kernel, Shan Hai On Mon, 2013-11-18 at 14:54 +0000, Joseph S. Myers wrote: > Ping^2. I still haven't seen any comments on any of these patches. > Sorry for the delay -- I plan to take a look at them, but they came in too late for the last pull request, especially since I'm not that familiar with SPE and thus it'll take some time (and I've got other time-consuming patches yet to review that came in even earlier). As long as the patches are still "action required" in patchwork, there's no need to ping unless it's been a few months. -Scott ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2013-12-10 23:07 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-11-04 16:50 [PATCH 0/6] powerpc/math-emu: e500 SPE float emulation fixes Joseph S. Myers 2013-11-04 16:52 ` [PATCH 1/6] powerpc: fix exception clearing in e500 SPE float emulation Joseph S. Myers 2013-11-22 22:34 ` Scott Wood 2013-11-23 1:22 ` Joseph S. Myers 2013-12-07 0:32 ` Scott Wood 2013-12-07 0:48 ` questions: second of the 2 pcie controllers does not scan the bus Ruchika 2013-12-07 13:26 ` Ruchika 2013-12-09 22:50 ` Scott Wood 2013-12-10 23:07 ` [PATCHv2 1/6] powerpc: fix exception clearing in e500 SPE float emulation Joseph S. Myers 2013-11-04 16:53 ` [PATCH 2/6] powerpc: fix e500 SPE float rounding inexactness detection Joseph S. Myers 2013-11-04 16:53 ` [PATCH 3/6] math-emu: fix floating-point to integer unsigned saturation Joseph S. Myers 2013-11-04 16:54 ` [PATCH 4/6] math-emu: fix floating-point to integer overflow detection Joseph S. Myers 2013-11-04 16:54 ` [PATCH 5/6] powerpc: fix e500 SPE float to integer and fixed-point conversions Joseph S. Myers 2013-11-04 16:55 ` [PATCH 6/6] powerpc: fix e500 SPE float SIGFPE generation Joseph S. Myers 2013-11-11 14:29 ` Ping Re: [PATCH 0/6] powerpc/math-emu: e500 SPE float emulation fixes Joseph S. Myers 2013-11-18 14:54 ` Ping^2 " Joseph S. Myers 2013-11-18 19:07 ` Scott Wood
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).