* [PATCH for-3.18 0/5] Stable commits picked up from lede project
@ 2017-07-04 11:44 Amit Pundir
2017-07-04 11:44 ` [PATCH for-3.18 1/5] bgmac: fix device initialization on Northstar SoCs (condition typo) Amit Pundir
` (5 more replies)
0 siblings, 6 replies; 10+ messages in thread
From: Amit Pundir @ 2017-07-04 11:44 UTC (permalink / raw)
To: Greg KH; +Cc: Stable
Hi Greg,
For your consideration, stable commits taken from lede
source tree https://git.lede-project.org/?p=source.git
for 3.18.y.
Cherry-picked and build tested on Linux v3.18.59 for
ARCH=arm/arm64 + allmodconfig.
Regards,
Amit Pundir
Felix Fietkau (1):
bgmac: add check for oversized packets
Jonas Gorski (1):
usb: ehci-orion: fix probe for !GENERIC_PHY
Rafał Miłecki (2):
bgmac: fix device initialization on Northstar SoCs (condition typo)
bgmac: reset & enable Ethernet core before using it
Yousong Zhou (1):
MIPS: UAPI: Ignore __arch_swab{16,32,64} when using MIPS16
arch/mips/include/uapi/asm/swab.h | 7 ++++---
drivers/net/ethernet/broadcom/bgmac.c | 17 +++++++++++++++--
drivers/usb/host/ehci-orion.c | 3 ++-
3 files changed, 21 insertions(+), 6 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH for-3.18 1/5] bgmac: fix device initialization on Northstar SoCs (condition typo)
2017-07-04 11:44 [PATCH for-3.18 0/5] Stable commits picked up from lede project Amit Pundir
@ 2017-07-04 11:44 ` Amit Pundir
2017-07-04 11:44 ` [PATCH for-3.18 2/5] bgmac: add check for oversized packets Amit Pundir
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Amit Pundir @ 2017-07-04 11:44 UTC (permalink / raw)
To: Greg KH; +Cc: Stable, Rafał Miłecki, David S . Miller
From: Rafał Miłecki <zajec5@gmail.com>
commit 21697336d46b71dd031f29e426dda0b1e7f06cc0 upstream.
On Northstar (Broadcom's ARM architecture) we need to manually enable
all cores. Code for that is already in place, but the condition for it
was wrong.
Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
---
drivers/net/ethernet/broadcom/bgmac.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 05c6af6c418f..7f34d91acdb9 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -1412,6 +1412,7 @@ static void bgmac_mii_unregister(struct bgmac *bgmac)
/* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipattach */
static int bgmac_probe(struct bcma_device *core)
{
+ struct bcma_chipinfo *ci = &core->bus->chipinfo;
struct net_device *net_dev;
struct bgmac *bgmac;
struct ssb_sprom *sprom = &core->bus->sprom;
@@ -1474,8 +1475,8 @@ static int bgmac_probe(struct bcma_device *core)
bgmac_chip_reset(bgmac);
/* For Northstar, we have to take all GMAC core out of reset */
- if (core->id.id == BCMA_CHIP_ID_BCM4707 ||
- core->id.id == BCMA_CHIP_ID_BCM53018) {
+ if (ci->id == BCMA_CHIP_ID_BCM4707 ||
+ ci->id == BCMA_CHIP_ID_BCM53018) {
struct bcma_device *ns_core;
int ns_gmac;
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH for-3.18 2/5] bgmac: add check for oversized packets
2017-07-04 11:44 [PATCH for-3.18 0/5] Stable commits picked up from lede project Amit Pundir
2017-07-04 11:44 ` [PATCH for-3.18 1/5] bgmac: fix device initialization on Northstar SoCs (condition typo) Amit Pundir
@ 2017-07-04 11:44 ` Amit Pundir
2017-07-04 11:44 ` [PATCH for-3.18 3/5] bgmac: reset & enable Ethernet core before using it Amit Pundir
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Amit Pundir @ 2017-07-04 11:44 UTC (permalink / raw)
To: Greg KH; +Cc: Stable, Felix Fietkau, David S . Miller
From: Felix Fietkau <nbd@openwrt.org>
commit 6a6c708469c9e10fd87adcc3abff164270538d62 upstream.
In very rare cases, the MAC can catch an internal buffer that is bigger
than it's supposed to be. Instead of crashing the kernel, simply pass
the buffer back to the hardware
Signed-off-by: Felix Fietkau <nbd@openwrt.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
---
drivers/net/ethernet/broadcom/bgmac.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 7f34d91acdb9..54a7da860a2d 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -373,6 +373,13 @@ static int bgmac_dma_rx_read(struct bgmac *bgmac, struct bgmac_dma_ring *ring,
break;
}
+ if (len > BGMAC_RX_ALLOC_SIZE) {
+ bgmac_err(bgmac, "Found oversized packet at slot %d, DMA issue!\n",
+ ring->start);
+ put_page(virt_to_head_page(buf));
+ break;
+ }
+
/* Omit CRC. */
len -= ETH_FCS_LEN;
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH for-3.18 3/5] bgmac: reset & enable Ethernet core before using it
2017-07-04 11:44 [PATCH for-3.18 0/5] Stable commits picked up from lede project Amit Pundir
2017-07-04 11:44 ` [PATCH for-3.18 1/5] bgmac: fix device initialization on Northstar SoCs (condition typo) Amit Pundir
2017-07-04 11:44 ` [PATCH for-3.18 2/5] bgmac: add check for oversized packets Amit Pundir
@ 2017-07-04 11:44 ` Amit Pundir
2017-07-04 11:44 ` [PATCH for-3.18 4/5] MIPS: UAPI: Ignore __arch_swab{16,32,64} when using MIPS16 Amit Pundir
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Amit Pundir @ 2017-07-04 11:44 UTC (permalink / raw)
To: Greg KH; +Cc: Stable, Rafał Miłecki, Felix Fietkau, David S . Miller
From: Rafał Miłecki <zajec5@gmail.com>
commit b4dfd8e92956b396d3438212bc9a0be6267b8b34 upstream.
This fixes Ethernet on D-Link DIR-885L with BCM47094 SoC. Felix reported
similar fix was needed for his BCM4709 device (Buffalo WXR-1900DHP?).
I tested this for regressions on BCM4706, BCM4708A0 and BCM47081A0.
Cc: Felix Fietkau <nbd@openwrt.org>
Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
---
To be cherry-picked on linux-4.4.y as well.
drivers/net/ethernet/broadcom/bgmac.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 54a7da860a2d..2b58df0c4f48 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -1438,6 +1438,11 @@ static int bgmac_probe(struct bcma_device *core)
dev_warn(&core->dev, "Using random MAC: %pM\n", mac);
}
+ /* This (reset &) enable is not preset in specs or reference driver but
+ * Broadcom does it in arch PCI code when enabling fake PCI device.
+ */
+ bcma_core_enable(core, 0);
+
/* Allocation and references */
net_dev = alloc_etherdev(sizeof(*bgmac));
if (!net_dev)
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH for-3.18 4/5] MIPS: UAPI: Ignore __arch_swab{16,32,64} when using MIPS16
2017-07-04 11:44 [PATCH for-3.18 0/5] Stable commits picked up from lede project Amit Pundir
` (2 preceding siblings ...)
2017-07-04 11:44 ` [PATCH for-3.18 3/5] bgmac: reset & enable Ethernet core before using it Amit Pundir
@ 2017-07-04 11:44 ` Amit Pundir
2017-07-04 15:32 ` Maciej W. Rozycki
2017-07-04 11:44 ` [PATCH for-3.18 5/5] usb: ehci-orion: fix probe for !GENERIC_PHY Amit Pundir
2017-07-07 9:05 ` [PATCH for-3.18 0/5] Stable commits picked up from lede project Greg KH
5 siblings, 1 reply; 10+ messages in thread
From: Amit Pundir @ 2017-07-04 11:44 UTC (permalink / raw)
To: Greg KH
Cc: Stable, Yousong Zhou, Maciej W . Rozycki, linux-mips, Ralf Baechle
From: Yousong Zhou <yszhou4tech@gmail.com>
commit 71a0a72456b48de972d7ed613b06a22a3aa9057f upstream.
Some GCC versions (e.g. 4.8.3) can incorrectly inline a function with
MIPS32 instructions into another function with MIPS16 code [1], causing
the assembler to genereate incorrect binary code or fail right away
complaining about unrecognized opcode.
In the case of __arch_swab{16,32}, when inlined by the compiler with
flags `-mips32r2 -mips16 -Os', the assembler can fail with the following
error.
{standard input}:79: Error: unrecognized opcode `wsbh $2,$2'
For performance concerns and to workaround the issue already existing in
older compilers, just ignore these 2 functions when compiling with
mips16 enabled.
[1] Inlining nomips16 function into mips16 function can result in
undefined builtins, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55777
Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com>
Cc: Maciej W. Rozycki <macro@linux-mips.org>
Cc: linux-mips@linux-mips.org
Patchwork: https://patchwork.linux-mips.org/patch/11241/
Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
---
arch/mips/include/uapi/asm/swab.h | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/arch/mips/include/uapi/asm/swab.h b/arch/mips/include/uapi/asm/swab.h
index 8f2d184dbe9f..23cd9b118c9e 100644
--- a/arch/mips/include/uapi/asm/swab.h
+++ b/arch/mips/include/uapi/asm/swab.h
@@ -13,8 +13,9 @@
#define __SWAB_64_THRU_32__
-#if (defined(__mips_isa_rev) && (__mips_isa_rev >= 2)) || \
- defined(_MIPS_ARCH_LOONGSON3A)
+#if !defined(__mips16) && \
+ ((defined(__mips_isa_rev) && (__mips_isa_rev >= 2)) || \
+ defined(_MIPS_ARCH_LOONGSON3A))
static inline __attribute_const__ __u16 __arch_swab16(__u16 x)
{
@@ -65,5 +66,5 @@ static inline __attribute_const__ __u64 __arch_swab64(__u64 x)
}
#define __arch_swab64 __arch_swab64
#endif /* __mips64 */
-#endif /* MIPS R2 or newer or Loongson 3A */
+#endif /* (not __mips16) and (MIPS R2 or newer or Loongson 3A) */
#endif /* _ASM_SWAB_H */
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH for-3.18 5/5] usb: ehci-orion: fix probe for !GENERIC_PHY
2017-07-04 11:44 [PATCH for-3.18 0/5] Stable commits picked up from lede project Amit Pundir
` (3 preceding siblings ...)
2017-07-04 11:44 ` [PATCH for-3.18 4/5] MIPS: UAPI: Ignore __arch_swab{16,32,64} when using MIPS16 Amit Pundir
@ 2017-07-04 11:44 ` Amit Pundir
2017-07-07 9:05 ` [PATCH for-3.18 0/5] Stable commits picked up from lede project Greg KH
5 siblings, 0 replies; 10+ messages in thread
From: Amit Pundir @ 2017-07-04 11:44 UTC (permalink / raw)
To: Greg KH; +Cc: Stable, Jonas Gorski
From: Jonas Gorski <jogo@openwrt.org>
commit db1319e166c5e872c4be54eac4e47454133708cf upstream.
Commit d445913ce0ab7f ("usb: ehci-orion: add optional PHY support")
added support for optional phys, but devm_phy_optional_get returns
-ENOSYS if GENERIC_PHY is not enabled.
This causes probe failures, even when there are no phys specified:
[ 1.443365] orion-ehci f1058000.usb: init f1058000.usb fail, -38
[ 1.449403] orion-ehci: probe of f1058000.usb failed with error -38
Similar to dwc3, treat -ENOSYS as no phy.
Fixes: d445913ce0ab7f ("usb: ehci-orion: add optional PHY support")
Signed-off-by: Jonas Gorski <jogo@openwrt.org>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
---
drivers/usb/host/ehci-orion.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c
index 22e15cab8ea5..8de069abd15e 100644
--- a/drivers/usb/host/ehci-orion.c
+++ b/drivers/usb/host/ehci-orion.c
@@ -226,7 +226,8 @@ static int ehci_orion_drv_probe(struct platform_device *pdev)
priv->phy = devm_phy_optional_get(&pdev->dev, "usb");
if (IS_ERR(priv->phy)) {
err = PTR_ERR(priv->phy);
- goto err_phy_get;
+ if (err != -ENOSYS)
+ goto err_phy_get;
} else {
err = phy_init(priv->phy);
if (err)
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH for-3.18 4/5] MIPS: UAPI: Ignore __arch_swab{16,32,64} when using MIPS16
@ 2017-07-04 15:32 ` Maciej W. Rozycki
0 siblings, 0 replies; 10+ messages in thread
From: Maciej W. Rozycki @ 2017-07-04 15:32 UTC (permalink / raw)
To: Amit Pundir; +Cc: Greg KH, Stable, Yousong Zhou, linux-mips, Ralf Baechle
On Tue, 4 Jul 2017, Amit Pundir wrote:
> From: Yousong Zhou <yszhou4tech@gmail.com>
>
> commit 71a0a72456b48de972d7ed613b06a22a3aa9057f upstream.
>
> Some GCC versions (e.g. 4.8.3) can incorrectly inline a function with
> MIPS32 instructions into another function with MIPS16 code [1], causing
> the assembler to genereate incorrect binary code or fail right away
> complaining about unrecognized opcode.
>
> In the case of __arch_swab{16,32}, when inlined by the compiler with
> flags `-mips32r2 -mips16 -Os', the assembler can fail with the following
> error.
>
> {standard input}:79: Error: unrecognized opcode `wsbh $2,$2'
>
> For performance concerns and to workaround the issue already existing in
> older compilers, just ignore these 2 functions when compiling with
> mips16 enabled.
>
> [1] Inlining nomips16 function into mips16 function can result in
> undefined builtins, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55777
The patch is correct, however the description does not match reality.
There is no GCC bug involved here as: "Some GCC versions (e.g. 4.8.3) can
incorrectly inline a function [...]" would suggest, and GCC PR
target/55777 has nothing to do with it.
Here you have inline functions including an inline asm each with assembly
instructions that do not have a MIPS16 representation. Unlike with GCC PR
target/55777 these functions are *not* marked with
`__attribute__((nomips16))', so the compiler is free to inline them into
any code, including MIPS16 code in particular, not being aware that the
inline asm is incompatible with MIPS16 assembly. It can't be aware as the
compiler is not an assembler and it does not interpret the string
representing assembly code to be produced from an inline asm (beyond
counting assembly instruction separators).
Marking these functions with `__attribute__((nomips16))' would instruct
the compiler to compile these functions as well as any function they get
inlined into as regular MIPS code, which may or may not be desirable (plus
*then* you might hit GCC PR target/55777, and IIRC some other bugs in
older compilers). So excluding them from MIPS16 code instead seems like a
reasonable choice.
OTOH, generic MIPS16 byte-swap code produced is awful, especially
`swab32' (and `swab64' does not apply as we don't support 64-bit MIPS16
code under Linux), so perhaps for MIPS16 code these really ought to be
`__attribute__((nomips16, noinline))' instead, avoiding turning the caller
into regular MIPS code as well as any old `nomips16' function inlining
bugs; although the benefit might outweigh the cost if one of these
functions is called from an otherwise leaf function and spilling registers
to the stack turns out necessary where it otherwise would not be.
Finally, for regular MIPS compilations contemporary versions of GCC
already produce the same assembly as our <uapi/asm/swab.h> provides, e.g.
from:
$ cat swap.c
unsigned short int swap16(unsigned short int i)
{
return i << 8 | i >> 8;
}
unsigned int swap32(unsigned int i)
{
return i << 24 | (i & 0xff00) << 8 | (i & 0xff0000) >> 8 | i >> 24;
}
unsigned long long int swap64(unsigned long long int i)
{
return (i << 56 | (i & 0xff00LL) << 40 |
(i & 0xff0000LL) << 24 | (i & 0xff000000LL) << 8 |
(i & 0xff00000000LL) >> 8 | (i & 0xff0000000000LL) >> 24 |
(i & 0xff000000000000LL) >> 40 | i >> 56);
}
$
you get:
$ gcc -O2 -mabi=64 -march=mips64r2 -c swap.c
$ objdump -d swap.o
swap.o: file format elf64-tradbigmips
Disassembly of section .text:
0000000000000000 <swap16>:
0: 7c0410a0 wsbh v0,a0
4: 03e00008 jr ra
8: 3042ffff andi v0,v0,0xffff
c: 00000000 nop
0000000000000010 <swap32>:
10: 7c0410a0 wsbh v0,a0
14: 03e00008 jr ra
18: 00221402 ror v0,v0,0x10
1c: 00000000 nop
0000000000000020 <swap64>:
20: 7c0410a4 dsbh v0,a0
24: 03e00008 jr ra
28: 7c021164 dshd v0,v0
2c: 00000000 nop
$
so I think we ought to make all this conditional on GCC being old enough,
as the compiler is always better suited to make code scheduling decisions
when there's no inline asm involved.
FWIW,
Maciej
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH for-3.18 4/5] MIPS: UAPI: Ignore __arch_swab{16,32,64} when using MIPS16
@ 2017-07-04 15:32 ` Maciej W. Rozycki
0 siblings, 0 replies; 10+ messages in thread
From: Maciej W. Rozycki @ 2017-07-04 15:32 UTC (permalink / raw)
To: Amit Pundir; +Cc: Greg KH, Stable, Yousong Zhou, linux-mips, Ralf Baechle
On Tue, 4 Jul 2017, Amit Pundir wrote:
> From: Yousong Zhou <yszhou4tech@gmail.com>
>
> commit 71a0a72456b48de972d7ed613b06a22a3aa9057f upstream.
>
> Some GCC versions (e.g. 4.8.3) can incorrectly inline a function with
> MIPS32 instructions into another function with MIPS16 code [1], causing
> the assembler to genereate incorrect binary code or fail right away
> complaining about unrecognized opcode.
>
> In the case of __arch_swab{16,32}, when inlined by the compiler with
> flags `-mips32r2 -mips16 -Os', the assembler can fail with the following
> error.
>
> {standard input}:79: Error: unrecognized opcode `wsbh $2,$2'
>
> For performance concerns and to workaround the issue already existing in
> older compilers, just ignore these 2 functions when compiling with
> mips16 enabled.
>
> [1] Inlining nomips16 function into mips16 function can result in
> undefined builtins, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55777
The patch is correct, however the description does not match reality.
There is no GCC bug involved here as: "Some GCC versions (e.g. 4.8.3) can
incorrectly inline a function [...]" would suggest, and GCC PR
target/55777 has nothing to do with it.
Here you have inline functions including an inline asm each with assembly
instructions that do not have a MIPS16 representation. Unlike with GCC PR
target/55777 these functions are *not* marked with
`__attribute__((nomips16))', so the compiler is free to inline them into
any code, including MIPS16 code in particular, not being aware that the
inline asm is incompatible with MIPS16 assembly. It can't be aware as the
compiler is not an assembler and it does not interpret the string
representing assembly code to be produced from an inline asm (beyond
counting assembly instruction separators).
Marking these functions with `__attribute__((nomips16))' would instruct
the compiler to compile these functions as well as any function they get
inlined into as regular MIPS code, which may or may not be desirable (plus
*then* you might hit GCC PR target/55777, and IIRC some other bugs in
older compilers). So excluding them from MIPS16 code instead seems like a
reasonable choice.
OTOH, generic MIPS16 byte-swap code produced is awful, especially
`swab32' (and `swab64' does not apply as we don't support 64-bit MIPS16
code under Linux), so perhaps for MIPS16 code these really ought to be
`__attribute__((nomips16, noinline))' instead, avoiding turning the caller
into regular MIPS code as well as any old `nomips16' function inlining
bugs; although the benefit might outweigh the cost if one of these
functions is called from an otherwise leaf function and spilling registers
to the stack turns out necessary where it otherwise would not be.
Finally, for regular MIPS compilations contemporary versions of GCC
already produce the same assembly as our <uapi/asm/swab.h> provides, e.g.
from:
$ cat swap.c
unsigned short int swap16(unsigned short int i)
{
return i << 8 | i >> 8;
}
unsigned int swap32(unsigned int i)
{
return i << 24 | (i & 0xff00) << 8 | (i & 0xff0000) >> 8 | i >> 24;
}
unsigned long long int swap64(unsigned long long int i)
{
return (i << 56 | (i & 0xff00LL) << 40 |
(i & 0xff0000LL) << 24 | (i & 0xff000000LL) << 8 |
(i & 0xff00000000LL) >> 8 | (i & 0xff0000000000LL) >> 24 |
(i & 0xff000000000000LL) >> 40 | i >> 56);
}
$
you get:
$ gcc -O2 -mabi=64 -march=mips64r2 -c swap.c
$ objdump -d swap.o
swap.o: file format elf64-tradbigmips
Disassembly of section .text:
0000000000000000 <swap16>:
0: 7c0410a0 wsbh v0,a0
4: 03e00008 jr ra
8: 3042ffff andi v0,v0,0xffff
c: 00000000 nop
0000000000000010 <swap32>:
10: 7c0410a0 wsbh v0,a0
14: 03e00008 jr ra
18: 00221402 ror v0,v0,0x10
1c: 00000000 nop
0000000000000020 <swap64>:
20: 7c0410a4 dsbh v0,a0
24: 03e00008 jr ra
28: 7c021164 dshd v0,v0
2c: 00000000 nop
$
so I think we ought to make all this conditional on GCC being old enough,
as the compiler is always better suited to make code scheduling decisions
when there's no inline asm involved.
FWIW,
Maciej
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH for-3.18 4/5] MIPS: UAPI: Ignore __arch_swab{16,32,64} when using MIPS16
2017-07-04 15:32 ` Maciej W. Rozycki
(?)
@ 2017-07-07 9:03 ` Greg KH
-1 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2017-07-07 9:03 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: Amit Pundir, Stable, Yousong Zhou, linux-mips, Ralf Baechle
On Tue, Jul 04, 2017 at 04:32:56PM +0100, Maciej W. Rozycki wrote:
> On Tue, 4 Jul 2017, Amit Pundir wrote:
>
> > From: Yousong Zhou <yszhou4tech@gmail.com>
> >
> > commit 71a0a72456b48de972d7ed613b06a22a3aa9057f upstream.
> >
> > Some GCC versions (e.g. 4.8.3) can incorrectly inline a function with
> > MIPS32 instructions into another function with MIPS16 code [1], causing
> > the assembler to genereate incorrect binary code or fail right away
> > complaining about unrecognized opcode.
> >
> > In the case of __arch_swab{16,32}, when inlined by the compiler with
> > flags `-mips32r2 -mips16 -Os', the assembler can fail with the following
> > error.
> >
> > {standard input}:79: Error: unrecognized opcode `wsbh $2,$2'
> >
> > For performance concerns and to workaround the issue already existing in
> > older compilers, just ignore these 2 functions when compiling with
> > mips16 enabled.
> >
> > [1] Inlining nomips16 function into mips16 function can result in
> > undefined builtins, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55777
>
> The patch is correct, however the description does not match reality.
> There is no GCC bug involved here as: "Some GCC versions (e.g. 4.8.3) can
> incorrectly inline a function [...]" would suggest, and GCC PR
> target/55777 has nothing to do with it.
>
> Here you have inline functions including an inline asm each with assembly
> instructions that do not have a MIPS16 representation. Unlike with GCC PR
> target/55777 these functions are *not* marked with
> `__attribute__((nomips16))', so the compiler is free to inline them into
> any code, including MIPS16 code in particular, not being aware that the
> inline asm is incompatible with MIPS16 assembly. It can't be aware as the
> compiler is not an assembler and it does not interpret the string
> representing assembly code to be produced from an inline asm (beyond
> counting assembly instruction separators).
>
> Marking these functions with `__attribute__((nomips16))' would instruct
> the compiler to compile these functions as well as any function they get
> inlined into as regular MIPS code, which may or may not be desirable (plus
> *then* you might hit GCC PR target/55777, and IIRC some other bugs in
> older compilers). So excluding them from MIPS16 code instead seems like a
> reasonable choice.
>
> OTOH, generic MIPS16 byte-swap code produced is awful, especially
> `swab32' (and `swab64' does not apply as we don't support 64-bit MIPS16
> code under Linux), so perhaps for MIPS16 code these really ought to be
> `__attribute__((nomips16, noinline))' instead, avoiding turning the caller
> into regular MIPS code as well as any old `nomips16' function inlining
> bugs; although the benefit might outweigh the cost if one of these
> functions is called from an otherwise leaf function and spilling registers
> to the stack turns out necessary where it otherwise would not be.
>
> Finally, for regular MIPS compilations contemporary versions of GCC
> already produce the same assembly as our <uapi/asm/swab.h> provides, e.g.
> from:
>
> $ cat swap.c
> unsigned short int swap16(unsigned short int i)
> {
> return i << 8 | i >> 8;
> }
>
> unsigned int swap32(unsigned int i)
> {
> return i << 24 | (i & 0xff00) << 8 | (i & 0xff0000) >> 8 | i >> 24;
> }
>
> unsigned long long int swap64(unsigned long long int i)
> {
> return (i << 56 | (i & 0xff00LL) << 40 |
> (i & 0xff0000LL) << 24 | (i & 0xff000000LL) << 8 |
> (i & 0xff00000000LL) >> 8 | (i & 0xff0000000000LL) >> 24 |
> (i & 0xff000000000000LL) >> 40 | i >> 56);
> }
> $
>
> you get:
>
> $ gcc -O2 -mabi=64 -march=mips64r2 -c swap.c
> $ objdump -d swap.o
>
> swap.o: file format elf64-tradbigmips
>
>
> Disassembly of section .text:
>
> 0000000000000000 <swap16>:
> 0: 7c0410a0 wsbh v0,a0
> 4: 03e00008 jr ra
> 8: 3042ffff andi v0,v0,0xffff
> c: 00000000 nop
>
> 0000000000000010 <swap32>:
> 10: 7c0410a0 wsbh v0,a0
> 14: 03e00008 jr ra
> 18: 00221402 ror v0,v0,0x10
> 1c: 00000000 nop
>
> 0000000000000020 <swap64>:
> 20: 7c0410a4 dsbh v0,a0
> 24: 03e00008 jr ra
> 28: 7c021164 dshd v0,v0
> 2c: 00000000 nop
> $
>
> so I think we ought to make all this conditional on GCC being old enough,
> as the compiler is always better suited to make code scheduling decisions
> when there's no inline asm involved.
Ok, but I'm not changing the changelog comments from the original patch
:)
If you think a follow-on patch is needed in the tree, please submit it
and mark it for stable inclusion.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH for-3.18 0/5] Stable commits picked up from lede project
2017-07-04 11:44 [PATCH for-3.18 0/5] Stable commits picked up from lede project Amit Pundir
` (4 preceding siblings ...)
2017-07-04 11:44 ` [PATCH for-3.18 5/5] usb: ehci-orion: fix probe for !GENERIC_PHY Amit Pundir
@ 2017-07-07 9:05 ` Greg KH
5 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2017-07-07 9:05 UTC (permalink / raw)
To: Amit Pundir; +Cc: Stable
On Tue, Jul 04, 2017 at 05:14:19PM +0530, Amit Pundir wrote:
> Hi Greg,
>
> For your consideration, stable commits taken from lede
> source tree https://git.lede-project.org/?p=source.git
> for 3.18.y.
>
> Cherry-picked and build tested on Linux v3.18.59 for
> ARCH=arm/arm64 + allmodconfig.
All now queued up, thanks.
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-07-07 9:05 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-04 11:44 [PATCH for-3.18 0/5] Stable commits picked up from lede project Amit Pundir
2017-07-04 11:44 ` [PATCH for-3.18 1/5] bgmac: fix device initialization on Northstar SoCs (condition typo) Amit Pundir
2017-07-04 11:44 ` [PATCH for-3.18 2/5] bgmac: add check for oversized packets Amit Pundir
2017-07-04 11:44 ` [PATCH for-3.18 3/5] bgmac: reset & enable Ethernet core before using it Amit Pundir
2017-07-04 11:44 ` [PATCH for-3.18 4/5] MIPS: UAPI: Ignore __arch_swab{16,32,64} when using MIPS16 Amit Pundir
2017-07-04 15:32 ` Maciej W. Rozycki
2017-07-04 15:32 ` Maciej W. Rozycki
2017-07-07 9:03 ` Greg KH
2017-07-04 11:44 ` [PATCH for-3.18 5/5] usb: ehci-orion: fix probe for !GENERIC_PHY Amit Pundir
2017-07-07 9:05 ` [PATCH for-3.18 0/5] Stable commits picked up from lede project Greg KH
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.