Re: [PATCH v2 6/8] arm64: Import latest memcpy()/memmove() implementation

Hi,

On 08/06/2021 14:21, Marek Szyprowski wrote:

+ Kevin

On 08.06.2021 13:37, Robin Murphy wrote:

quoted

Hi Marek,

On 2021-06-08 12:15, Marek Szyprowski wrote:

quoted

Hi Robin,

On 27.05.2021 17:34, Robin Murphy wrote:

quoted

Import the latest implementation of memcpy(), based on the
upstream code of string/aarch64/memcpy.S at commit afd6244 from
https://protect2.fireeye.com/v1/url?k=0e25d630-51beef28-0e245d7f-0cc47a314e9a-b41fdb2d4d06ff75&q=1&e=fcfaf71d-f01a-4bc4-8e16-8ae86e0c0116&u=https%3A%2F%2Fgithub.com%2FARM-software%2Foptimized-routines, 
and subsuming
memmove() in the process.

Note that for simplicity Arm have chosen to contribute this code
to Linux under GPLv2 rather than the original MIT license.

Note also that the needs of the usercopy routines vs. regular memcpy()
have now diverged so far that we abandon the shared template idea
and the damage which that incurred to the tuning of LDP/STP loops.
We'll be back to tackle those routines separately in future.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>

This patch landed recently in linux-next as commit 285133040e6c ("arm64:
Import latest memcpy()/memmove() implementation"). Sadly it causes
serious issues on Khadas VIM3 board. Reverting it on top of linux
next-20210607 (together with 6b8f648959e5 and resolving the conflict in
the Makefile) fixes the issue. Here is the kernel log:

Unable to handle kernel paging request at virtual address 
ffff8000136bd204
Mem abort info:
    ESR = 0x96000061
    EC = 0x25: DABT (current EL), IL = 32 bits
    SET = 0, FnV = 0
    EA = 0, S1PTW = 0
Data abort info:
    ISV = 0, ISS = 0x00000061

That's an alignment fault, which implies we're accessing something 
which isn't normal memory.

quoted

    CM = 0, WnR = 1
swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000009da6000
[ffff8000136bd204] pgd=10000000f4806003, p4d=10000000f4806003,
pud=10000000f4805003, pmd=1000000000365003, pte=00680000ffe03713
Internal error: Oops: 96000061 [#1] PREEMPT SMP
Modules linked in: brcmfmac brcmutil cfg80211 dw_hdmi_i2s_audio
meson_gxl hci_uart btqca btbcm bluetooth panfrost ecdh_generic ecc
snd_soc_meson_axg_sound_card crct10dif_ce snd_soc_meson_card_utils
rfkill rtc_hym8563 gpu_sched dwmac_generic rc_khadas meson_gxbb_wdt
meson_ir pwm_meson snd_soc_meson_axg_tdmin snd_soc_meson_g12a_tohdmitx
rtc_meson_vrtc snd_soc_meson_axg_tdmout snd_soc_meson_axg_frddr
reset_meson_audio_arb snd_soc_meson_codec_glue axg_audio meson_rng
sclk_div dwmac_meson8b snd_soc_meson_axg_toddr mdio_mux_meson_g12a
clk_phase stmmac_platform rng_core snd_soc_meson_axg_fifo meson_dw_hdmi
stmmac meson_drm meson_canvas dw_hdmi pcs_xpcs display_connector
snd_soc_meson_axg_tdm_interface nvmem_meson_efuse adc_keys
snd_soc_meson_axg_tdm_formatter
CPU: 4 PID: 135 Comm: kworker/4:3 Not tainted 5.13.0-rc5-next-20210607
#10441
Hardware name: Khadas VIM3 (DT)
Workqueue: events request_firmware_work_func
pstate: 20000005 (nzCv daif -PAN -UAO -TCO BTYPE=--)
pc : __memcpy+0x2c/0x260
lr : sg_copy_buffer+0x90/0x118
...
Call trace:
   __memcpy+0x2c/0x260
   sg_copy_to_buffer+0x14/0x20
   meson_mmc_start_cmd+0xf4/0x2c8
   meson_mmc_request+0x4c/0xb8
   __mmc_start_request+0xa4/0x2a8
   mmc_start_request+0x80/0xa8
   mmc_wait_for_req+0x68/0xd8
   mmc_io_rw_extended+0x1d4/0x2e0
   sdio_io_rw_ext_helper+0xb0/0x1e8
   sdio_memcpy_toio+0x20/0x28
   brcmf_sdiod_skbuff_write.isra.18+0x2c/0x68 [brcmfmac]
   brcmf_sdiod_ramrw+0xe0/0x230 [brcmfmac]
   brcmf_sdio_firmware_callback+0xa8/0x7c8 [brcmfmac]
   brcmf_fw_request_done+0x7c/0x100 [brcmfmac]
   request_firmware_work_func+0x4c/0xd8
   process_one_work+0x2a8/0x718
   worker_thread+0x48/0x460
   kthread+0x12c/0x160
   ret_from_fork+0x10/0x18
Code: 540000c3 a9401c26 a97f348c a9001c06 (a93f34ac)
---[ end trace be83fa283dc82415 ]---

I hope that the above log helps fixing the issue. IIRC the SDHCI driver
on VIM3 board uses internal SRAM for transferring data (instead of DMA),
so the issue is somehow related to that.

Drivers shouldn't be using memcpy() on iomem mappings. Even if they 
happen to have got away with it sometimes ;)

Taking a quick look at that driver,

    host->bounce_buf = host->regs + SD_EMMC_SRAM_DATA_BUF_OFF;

is completely bogus, as Sparse will readily point out.

My bad, what's the correct way to copy data to an iomem mapping ?

Neil

quoted

Robin.

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic