From: Sasha Levin <sashal@kernel.org> To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Linus Torvalds <torvalds@linux-foundation.org>, Guenter Roeck <linux@roeck-us.net>, Sasha Levin <sashal@kernel.org>, linuxppc-dev@lists.ozlabs.org, dmaengine@vger.kernel.org Subject: [PATCH AUTOSEL 5.8 42/42] fsldma: fix very broken 32-bit ppc ioread64 functionality Date: Mon, 31 Aug 2020 11:29:34 -0400 [thread overview] Message-ID: <20200831152934.1023912-42-sashal@kernel.org> (raw) In-Reply-To: <20200831152934.1023912-1-sashal@kernel.org> From: Linus Torvalds <torvalds@linux-foundation.org> [ Upstream commit 0a4c56c80f90797e9b9f8426c6aae4c0cf1c9785 ] Commit ef91bb196b0d ("kernel.h: Silence sparse warning in lower_32_bits") caused new warnings to show in the fsldma driver, but that commit was not to blame: it only exposed some very incorrect code that tried to take the low 32 bits of an address. That made no sense for multiple reasons, the most notable one being that that code was intentionally limited to only 32-bit ppc builds, so "only low 32 bits of an address" was completely nonsensical. There were no high bits to mask off to begin with. But even more importantly fropm a correctness standpoint, turning the address into an integer then caused the subsequent address arithmetic to be completely wrong too, and the "+1" actually incremented the address by one, rather than by four. Which again was incorrect, since the code was reading two 32-bit values and trying to make a 64-bit end result of it all. Surprisingly, the iowrite64() did not suffer from the same odd and incorrect model. This code has never worked, but it's questionable whether anybody cared: of the two users that actually read the 64-bit value (by way of some C preprocessor hackery and eventually the 'get_cdar()' inline function), one of them explicitly ignored the value, and the other one might just happen to work despite the incorrect value being read. This patch at least makes it not fail the build any more, and makes the logic superficially sane. Whether it makes any difference to the code _working_ or not shall remain a mystery. Compile-tested-by: Guenter Roeck <linux@roeck-us.net> Reviewed-by: Guenter Roeck <linux@roeck-us.net> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Sasha Levin <sashal@kernel.org> --- drivers/dma/fsldma.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h index 56f18ae992332..308bed0a560ac 100644 --- a/drivers/dma/fsldma.h +++ b/drivers/dma/fsldma.h @@ -205,10 +205,10 @@ struct fsldma_chan { #else static u64 fsl_ioread64(const u64 __iomem *addr) { - u32 fsl_addr = lower_32_bits(addr); - u64 fsl_addr_hi = (u64)in_le32((u32 *)(fsl_addr + 1)) << 32; + u32 val_lo = in_le32((u32 __iomem *)addr); + u32 val_hi = in_le32((u32 __iomem *)addr + 1); - return fsl_addr_hi | in_le32((u32 *)fsl_addr); + return ((u64)val_hi << 32) + val_lo; } static void fsl_iowrite64(u64 val, u64 __iomem *addr) @@ -219,10 +219,10 @@ static void fsl_iowrite64(u64 val, u64 __iomem *addr) static u64 fsl_ioread64be(const u64 __iomem *addr) { - u32 fsl_addr = lower_32_bits(addr); - u64 fsl_addr_hi = (u64)in_be32((u32 *)fsl_addr) << 32; + u32 val_hi = in_be32((u32 __iomem *)addr); + u32 val_lo = in_be32((u32 __iomem *)addr + 1); - return fsl_addr_hi | in_be32((u32 *)(fsl_addr + 1)); + return ((u64)val_hi << 32) + val_lo; } static void fsl_iowrite64be(u64 val, u64 __iomem *addr) -- 2.25.1
WARNING: multiple messages have this Message-ID (diff)
From: Sasha Levin <sashal@kernel.org> To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Sasha Levin <sashal@kernel.org>, linuxppc-dev@lists.ozlabs.org, Linus Torvalds <torvalds@linux-foundation.org>, dmaengine@vger.kernel.org, Guenter Roeck <linux@roeck-us.net> Subject: [PATCH AUTOSEL 5.8 42/42] fsldma: fix very broken 32-bit ppc ioread64 functionality Date: Mon, 31 Aug 2020 11:29:34 -0400 [thread overview] Message-ID: <20200831152934.1023912-42-sashal@kernel.org> (raw) In-Reply-To: <20200831152934.1023912-1-sashal@kernel.org> From: Linus Torvalds <torvalds@linux-foundation.org> [ Upstream commit 0a4c56c80f90797e9b9f8426c6aae4c0cf1c9785 ] Commit ef91bb196b0d ("kernel.h: Silence sparse warning in lower_32_bits") caused new warnings to show in the fsldma driver, but that commit was not to blame: it only exposed some very incorrect code that tried to take the low 32 bits of an address. That made no sense for multiple reasons, the most notable one being that that code was intentionally limited to only 32-bit ppc builds, so "only low 32 bits of an address" was completely nonsensical. There were no high bits to mask off to begin with. But even more importantly fropm a correctness standpoint, turning the address into an integer then caused the subsequent address arithmetic to be completely wrong too, and the "+1" actually incremented the address by one, rather than by four. Which again was incorrect, since the code was reading two 32-bit values and trying to make a 64-bit end result of it all. Surprisingly, the iowrite64() did not suffer from the same odd and incorrect model. This code has never worked, but it's questionable whether anybody cared: of the two users that actually read the 64-bit value (by way of some C preprocessor hackery and eventually the 'get_cdar()' inline function), one of them explicitly ignored the value, and the other one might just happen to work despite the incorrect value being read. This patch at least makes it not fail the build any more, and makes the logic superficially sane. Whether it makes any difference to the code _working_ or not shall remain a mystery. Compile-tested-by: Guenter Roeck <linux@roeck-us.net> Reviewed-by: Guenter Roeck <linux@roeck-us.net> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Sasha Levin <sashal@kernel.org> --- drivers/dma/fsldma.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h index 56f18ae992332..308bed0a560ac 100644 --- a/drivers/dma/fsldma.h +++ b/drivers/dma/fsldma.h @@ -205,10 +205,10 @@ struct fsldma_chan { #else static u64 fsl_ioread64(const u64 __iomem *addr) { - u32 fsl_addr = lower_32_bits(addr); - u64 fsl_addr_hi = (u64)in_le32((u32 *)(fsl_addr + 1)) << 32; + u32 val_lo = in_le32((u32 __iomem *)addr); + u32 val_hi = in_le32((u32 __iomem *)addr + 1); - return fsl_addr_hi | in_le32((u32 *)fsl_addr); + return ((u64)val_hi << 32) + val_lo; } static void fsl_iowrite64(u64 val, u64 __iomem *addr) @@ -219,10 +219,10 @@ static void fsl_iowrite64(u64 val, u64 __iomem *addr) static u64 fsl_ioread64be(const u64 __iomem *addr) { - u32 fsl_addr = lower_32_bits(addr); - u64 fsl_addr_hi = (u64)in_be32((u32 *)fsl_addr) << 32; + u32 val_hi = in_be32((u32 __iomem *)addr); + u32 val_lo = in_be32((u32 __iomem *)addr + 1); - return fsl_addr_hi | in_be32((u32 *)(fsl_addr + 1)); + return ((u64)val_hi << 32) + val_lo; } static void fsl_iowrite64be(u64 val, u64 __iomem *addr) -- 2.25.1
next prev parent reply other threads:[~2020-08-31 15:36 UTC|newest] Thread overview: 81+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-08-31 15:28 [PATCH AUTOSEL 5.8 01/42] hwmon: (pmbus/isl68137) remove READ_TEMPERATURE_1 telemetry for RAA228228 Sasha Levin 2020-08-31 15:28 ` [PATCH AUTOSEL 5.8 02/42] HID: quirks: Always poll three more Lenovo PixArt mice Sasha Levin 2020-08-31 15:28 ` [PATCH AUTOSEL 5.8 03/42] HID: hiddev: Fix slab-out-of-bounds write in hiddev_ioctl_usage() Sasha Levin 2020-08-31 15:28 ` [PATCH AUTOSEL 5.8 04/42] drm/msm/dpu: Fix reservation failures in modeset Sasha Levin 2020-08-31 15:28 ` Sasha Levin 2020-08-31 15:28 ` [PATCH AUTOSEL 5.8 05/42] drm/msm/dpu: Fix scale params in plane validation Sasha Levin 2020-08-31 15:28 ` Sasha Levin 2020-08-31 15:28 ` [PATCH AUTOSEL 5.8 06/42] drm/msm/dpu: fix unitialized variable error Sasha Levin 2020-08-31 15:28 ` Sasha Levin 2020-08-31 15:28 ` [PATCH AUTOSEL 5.8 07/42] speakup: Fix wait_for_xmitr for ttyio case Sasha Levin 2020-08-31 15:28 ` Sasha Levin 2020-08-31 15:33 ` Greg Kroah-Hartman 2020-08-31 15:33 ` Greg Kroah-Hartman 2020-09-05 12:02 ` Sasha Levin 2020-09-05 12:02 ` Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 08/42] tty: serial: qcom_geni_serial: Drop __init from qcom_geni_console_setup Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 09/42] drm/msm: add shutdown support for display platform_driver Sasha Levin 2020-08-31 15:29 ` Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 10/42] hwmon: (applesmc) check status earlier Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 11/42] nvme: skip noiob for zoned devices Sasha Levin 2020-08-31 15:29 ` Sasha Levin 2020-08-31 15:38 ` Keith Busch 2020-08-31 15:38 ` Keith Busch 2020-09-05 12:03 ` Sasha Levin 2020-09-05 12:03 ` Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 12/42] nvmet: Disable keep-alive timer when kato is cleared to 0h Sasha Levin 2020-08-31 15:29 ` Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 13/42] drm/msm: enable vblank during atomic commits Sasha Levin 2020-08-31 15:29 ` Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 14/42] habanalabs: unmap PCI bars upon iATU failure Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 15/42] habanalabs: validate packet id during CB parse Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 16/42] habanalabs: set clock gating according to mask Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 17/42] habanalabs: proper handling of alloc size in coresight Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 18/42] habanalabs: set max power according to card type Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 19/42] habanalabs: validate FW file size Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 20/42] habanalabs: check correct vmalloc return code Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 21/42] drm/msm/a6xx: fix gmu start on newer firmware Sasha Levin 2020-08-31 15:29 ` Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 22/42] gfs2: add some much needed cleanup for log flushes that fail Sasha Levin 2020-08-31 15:29 ` [Cluster-devel] " Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 23/42] hv_utils: return error if host timesysnc update is stale Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 24/42] hv_utils: drain the timesync packets on onchannelcallback Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 25/42] ceph: fix inode number handling on arches with 32-bit ino_t Sasha Levin 2020-08-31 16:08 ` Ilya Dryomov 2020-09-05 12:04 ` Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 26/42] ceph: don't allow setlease on cephfs Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 27/42] i2c: iproc: Fix shifting 31 bits Sasha Levin 2020-08-31 15:29 ` Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 28/42] drm/omap: fix incorrect lock state Sasha Levin 2020-08-31 15:29 ` Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 29/42] irqchip/ingenic: Leave parent IRQ unmasked on suspend Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 30/42] cpuidle: Fixup IRQ state Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 31/42] nbd: restore default timeout when setting it to zero Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 32/42] s390: don't trace preemption in percpu macros Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 33/42] drm/amd/display: should check error using DC_OK Sasha Levin 2020-08-31 15:29 ` Sasha Levin 2020-08-31 15:29 ` Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 34/42] drm/amd/display: Reject overlay plane configurations in multi-display scenarios Sasha Levin 2020-08-31 15:29 ` Sasha Levin 2020-08-31 15:29 ` Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 35/42] drivers: gpu: amd: Initialize amdgpu_dm_backlight_caps object to 0 in amdgpu_dm_update_backlight_caps Sasha Levin 2020-08-31 15:29 ` Sasha Levin 2020-08-31 15:29 ` Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 36/42] drm/amd/display: Revert HDCP disable sequence change Sasha Levin 2020-08-31 15:29 ` Sasha Levin 2020-08-31 15:29 ` Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 37/42] drm/amd/display: Fix passive dongle mistaken as active dongle in EDID emulation Sasha Levin 2020-08-31 15:29 ` Sasha Levin 2020-08-31 15:29 ` Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 38/42] drm/amd/display: Keep current gain when ABM disable immediately Sasha Levin 2020-08-31 15:29 ` Sasha Levin 2020-08-31 15:29 ` Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 39/42] drm/amd/display: Retry AUX write when fail occurs Sasha Levin 2020-08-31 15:29 ` Sasha Levin 2020-08-31 15:29 ` Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 40/42] drm/amd/display: Fix memleak in amdgpu_dm_mode_config_init Sasha Levin 2020-08-31 15:29 ` Sasha Levin 2020-08-31 15:29 ` Sasha Levin 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 41/42] xen/xenbus: Fix granting of vmalloc'd memory Sasha Levin 2020-08-31 15:29 ` Sasha Levin [this message] 2020-08-31 15:29 ` [PATCH AUTOSEL 5.8 42/42] fsldma: fix very broken 32-bit ppc ioread64 functionality Sasha Levin
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200831152934.1023912-42-sashal@kernel.org \ --to=sashal@kernel.org \ --cc=dmaengine@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux@roeck-us.net \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=stable@vger.kernel.org \ --cc=torvalds@linux-foundation.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.