From: "Winkler, Tomas" <tomas.winkler@intel.com> To: Marc Gonzalez <marc.w.gonzalez@free.fr>, Vinayak Holikatti <vinholikatti@gmail.com>, Joao Pinto <jpinto@synopsys.com>, Avri Altman <avri.altman@wdc.com>, "Martin K. Petersen" <martin.petersen@oracle.com>, Robin Murphy <robin.murphy@arm.com> Cc: MSM <linux-arm-msm@vger.kernel.org>, SCSI <linux-scsi@vger.kernel.org>, Linux ARM <linux-arm-kernel@lists.infradead.org> Subject: RE: [PATCH v1] scsi: ufs: Use explicit access size in ufshcd_dump_regs Date: Thu, 13 Dec 2018 22:34:52 +0000 [thread overview] Message-ID: <5B8DA87D05A7694D9FA63FD143655C1B9DA52659@hasmsx108.ger.corp.intel.com> (raw) In-Reply-To: <3bf7ea8e-4cf2-1f9b-7f46-15609dfcad19@free.fr> > memcpy_fromio() doesn't provide any control over access size. > For example, on arm64, it is implemented using readb and readq. > This may trigger a synchronous external abort: > > [ 3.729943] Internal error: synchronous external abort: 96000210 [#1] > PREEMPT SMP > [ 3.737000] Modules linked in: > [ 3.744371] CPU: 2 PID: 1 Comm: swapper/0 Tainted: G S 4.20.0-rc4 > #16 > [ 3.747413] Hardware name: Qualcomm Technologies, Inc. MSM8998 v1 > MTP (DT) > [ 3.755295] pstate: 00000005 (nzcv daif -PAN -UAO) > [ 3.761978] pc : __memcpy_fromio+0x68/0x80 > [ 3.766718] lr : ufshcd_dump_regs+0x50/0xb0 > [ 3.770767] sp : ffff00000807ba00 > [ 3.774830] x29: ffff00000807ba00 x28: 00000000fffffffb > [ 3.778344] x27: ffff0000089db068 x26: ffff8000f6e58000 > [ 3.783728] x25: 000000000000000e x24: 0000000000000800 > [ 3.789023] x23: ffff8000f6e587c8 x22: 0000000000000800 > [ 3.794319] x21: ffff000008908368 x20: ffff8000f6e1ab80 > [ 3.799615] x19: 000000000000006c x18: ffffffffffffffff > [ 3.804910] x17: 0000000000000000 x16: 0000000000000000 > [ 3.810206] x15: ffff000009199648 x14: ffff000089244187 > [ 3.815502] x13: ffff000009244195 x12: ffff0000091ab000 > [ 3.820797] x11: 0000000005f5e0ff x10: ffff0000091998a0 > [ 3.826093] x9 : 0000000000000000 x8 : ffff8000f6e1ac00 > [ 3.831389] x7 : 0000000000000000 x6 : 0000000000000068 > [ 3.836676] x5 : ffff8000f6e1abe8 x4 : 0000000000000000 > [ 3.841971] x3 : ffff00000928c868 x2 : ffff8000f6e1abec > [ 3.847267] x1 : ffff00000928c868 x0 : ffff8000f6e1abe8 > [ 3.852567] Process swapper/0 (pid: 1, stack limit = 0x(____ptrval____)) > [ 3.857900] Call trace: > [ 3.864473] __memcpy_fromio+0x68/0x80 > [ 3.866683] ufs_qcom_dump_dbg_regs+0x1c0/0x370 > [ 3.870522] ufshcd_print_host_regs+0x168/0x190 > [ 3.874946] ufshcd_init+0xd4c/0xde0 > [ 3.879459] ufshcd_pltfrm_init+0x3c8/0x550 > [ 3.883264] ufs_qcom_probe+0x24/0x60 > [ 3.887188] platform_drv_probe+0x50/0xa0 > > Assuming aligned 32-bit registers, let's use readl, after making sure that 'offset' > and 'len' are indeed multiples of 4. > > Fixes: ba80917d9932d ("scsi: ufs: ufshcd_dump_regs to use memcpy_fromio") > Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr> LGTM Thanks Tomas > --- > drivers/scsi/ufs/ufshcd.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index > 535180c01ce8..320bbd9849bc 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -112,13 +112,19 @@ > int ufshcd_dump_regs(struct ufs_hba *hba, size_t offset, size_t len, > const char *prefix) > { > - u8 *regs; > + u32 *regs; > + size_t pos; > + > + if (offset % 4 != 0 || len % 4 != 0) /* keep readl happy */ > + return -EINVAL; > > regs = kzalloc(len, GFP_KERNEL); > if (!regs) > return -ENOMEM; > > - memcpy_fromio(regs, hba->mmio_base + offset, len); > + for (pos = 0; pos < len; pos += 4) > + regs[pos / 4] = ufshcd_readl(hba, offset + pos); > + > ufshcd_hex_dump(prefix, regs, len); > kfree(regs); > > -- > 2.17.1
WARNING: multiple messages have this Message-ID (diff)
From: "Winkler, Tomas" <tomas.winkler@intel.com> To: Marc Gonzalez <marc.w.gonzalez@free.fr>, Vinayak Holikatti <vinholikatti@gmail.com>, Joao Pinto <jpinto@synopsys.com>, Avri Altman <avri.altman@wdc.com>, "Martin K. Petersen" <martin.petersen@oracle.com>, Robin Murphy <robin.murphy@arm.com> Cc: MSM <linux-arm-msm@vger.kernel.org>, SCSI <linux-scsi@vger.kernel.org>, Linux ARM <linux-arm-kernel@lists.infradead.org> Subject: RE: [PATCH v1] scsi: ufs: Use explicit access size in ufshcd_dump_regs Date: Thu, 13 Dec 2018 22:34:52 +0000 [thread overview] Message-ID: <5B8DA87D05A7694D9FA63FD143655C1B9DA52659@hasmsx108.ger.corp.intel.com> (raw) In-Reply-To: <3bf7ea8e-4cf2-1f9b-7f46-15609dfcad19@free.fr> > memcpy_fromio() doesn't provide any control over access size. > For example, on arm64, it is implemented using readb and readq. > This may trigger a synchronous external abort: > > [ 3.729943] Internal error: synchronous external abort: 96000210 [#1] > PREEMPT SMP > [ 3.737000] Modules linked in: > [ 3.744371] CPU: 2 PID: 1 Comm: swapper/0 Tainted: G S 4.20.0-rc4 > #16 > [ 3.747413] Hardware name: Qualcomm Technologies, Inc. MSM8998 v1 > MTP (DT) > [ 3.755295] pstate: 00000005 (nzcv daif -PAN -UAO) > [ 3.761978] pc : __memcpy_fromio+0x68/0x80 > [ 3.766718] lr : ufshcd_dump_regs+0x50/0xb0 > [ 3.770767] sp : ffff00000807ba00 > [ 3.774830] x29: ffff00000807ba00 x28: 00000000fffffffb > [ 3.778344] x27: ffff0000089db068 x26: ffff8000f6e58000 > [ 3.783728] x25: 000000000000000e x24: 0000000000000800 > [ 3.789023] x23: ffff8000f6e587c8 x22: 0000000000000800 > [ 3.794319] x21: ffff000008908368 x20: ffff8000f6e1ab80 > [ 3.799615] x19: 000000000000006c x18: ffffffffffffffff > [ 3.804910] x17: 0000000000000000 x16: 0000000000000000 > [ 3.810206] x15: ffff000009199648 x14: ffff000089244187 > [ 3.815502] x13: ffff000009244195 x12: ffff0000091ab000 > [ 3.820797] x11: 0000000005f5e0ff x10: ffff0000091998a0 > [ 3.826093] x9 : 0000000000000000 x8 : ffff8000f6e1ac00 > [ 3.831389] x7 : 0000000000000000 x6 : 0000000000000068 > [ 3.836676] x5 : ffff8000f6e1abe8 x4 : 0000000000000000 > [ 3.841971] x3 : ffff00000928c868 x2 : ffff8000f6e1abec > [ 3.847267] x1 : ffff00000928c868 x0 : ffff8000f6e1abe8 > [ 3.852567] Process swapper/0 (pid: 1, stack limit = 0x(____ptrval____)) > [ 3.857900] Call trace: > [ 3.864473] __memcpy_fromio+0x68/0x80 > [ 3.866683] ufs_qcom_dump_dbg_regs+0x1c0/0x370 > [ 3.870522] ufshcd_print_host_regs+0x168/0x190 > [ 3.874946] ufshcd_init+0xd4c/0xde0 > [ 3.879459] ufshcd_pltfrm_init+0x3c8/0x550 > [ 3.883264] ufs_qcom_probe+0x24/0x60 > [ 3.887188] platform_drv_probe+0x50/0xa0 > > Assuming aligned 32-bit registers, let's use readl, after making sure that 'offset' > and 'len' are indeed multiples of 4. > > Fixes: ba80917d9932d ("scsi: ufs: ufshcd_dump_regs to use memcpy_fromio") > Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr> LGTM Thanks Tomas > --- > drivers/scsi/ufs/ufshcd.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index > 535180c01ce8..320bbd9849bc 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -112,13 +112,19 @@ > int ufshcd_dump_regs(struct ufs_hba *hba, size_t offset, size_t len, > const char *prefix) > { > - u8 *regs; > + u32 *regs; > + size_t pos; > + > + if (offset % 4 != 0 || len % 4 != 0) /* keep readl happy */ > + return -EINVAL; > > regs = kzalloc(len, GFP_KERNEL); > if (!regs) > return -ENOMEM; > > - memcpy_fromio(regs, hba->mmio_base + offset, len); > + for (pos = 0; pos < len; pos += 4) > + regs[pos / 4] = ufshcd_readl(hba, offset + pos); > + > ufshcd_hex_dump(prefix, regs, len); > kfree(regs); > > -- > 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2018-12-13 22:34 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-12-11 14:18 [PATCH v1] scsi: ufs: Use explicit access size in ufshcd_dump_regs Marc Gonzalez 2018-12-11 14:18 ` Marc Gonzalez 2018-12-13 22:34 ` Winkler, Tomas [this message] 2018-12-13 22:34 ` Winkler, Tomas 2018-12-27 15:54 ` Marc Gonzalez 2018-12-27 15:54 ` Marc Gonzalez 2019-01-09 12:42 ` Marc Gonzalez 2019-01-09 12:42 ` Marc Gonzalez 2019-01-09 15:38 ` Jeffrey Hugo 2019-01-09 15:38 ` Jeffrey Hugo 2019-01-16 9:36 ` Marc Gonzalez 2019-01-16 9:36 ` Marc Gonzalez 2019-01-16 9:43 ` Winkler, Tomas 2019-01-16 9:43 ` Winkler, Tomas 2019-01-23 2:13 ` Martin K. Petersen 2019-01-23 2:13 ` Martin K. Petersen
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=5B8DA87D05A7694D9FA63FD143655C1B9DA52659@hasmsx108.ger.corp.intel.com \ --to=tomas.winkler@intel.com \ --cc=avri.altman@wdc.com \ --cc=jpinto@synopsys.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-scsi@vger.kernel.org \ --cc=marc.w.gonzalez@free.fr \ --cc=martin.petersen@oracle.com \ --cc=robin.murphy@arm.com \ --cc=vinholikatti@gmail.com \ /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.