linux-kernel - RE: [PATCH] ASoC: Intel: sst: ipc command timeout

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 6 May 2020 01:38:07 +0000
From:   "Lu, Brent" <brent.lu@...el.com>
To:     Amadeusz Sławiński 
        <amadeuszx.slawinski@...ux.intel.com>,
        "Rojewski, Cezary" <cezary.rojewski@...el.com>,
        Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
        "alsa-devel@...a-project.org" <alsa-devel@...a-project.org>
CC:     Kate Stewart <kstewart@...uxfoundation.org>,
        Richard Fontana <rfontana@...hat.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Jie Yang" <yang.jie@...ux.intel.com>,
        Takashi Iwai <tiwai@...e.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Liam Girdwood <liam.r.girdwood@...ux.intel.com>,
        "clang-built-linux@...glegroups.com" 
        <clang-built-linux@...glegroups.com>,
        Mark Brown <broonie@...nel.org>,
        "Thomas Gleixner" <tglx@...utronix.de>,
        Allison Randal <allison@...utok.net>
Subject: RE: [PATCH] ASoC: Intel: sst: ipc command timeout

> 
> Hi,
> 
> That's why I would suggest trying with readq, it should also generate one
> instruction read x86_64 platforms, I looked a bit more and there is fallback to
> generate two 32 bit reads on 32bit platforms, so my previous concern about
> having to write separate handling for those platforms was unneeded. So I
> would recommend checking using it.
Hi,

The readq/writeq works. Code is optimized unlike memcpy_fromio/memcpy_toio
and the defect is not able to reproduce.

(gdb) disas sst_shim32_read64
Dump of assembler code for function sst_shim32_read64:
   0x000000000000096c <+0>:     call   0x971 <sst_shim32_read64+5>
   0x0000000000000971 <+5>:     push   rbp
   0x0000000000000972 <+6>:     mov    rbp,rsp
   0x0000000000000975 <+9>:     mov    eax,esi
   0x0000000000000977 <+11>:    mov    rax,QWORD PTR [rdi+rax*1]
   0x000000000000097b <+15>:    pop    rbp
   0x000000000000097c <+16>:    ret
End of assembler dump.
(gdb) disas sst_shim32_write64
Dump of assembler code for function sst_shim32_write64:
   0x000000000000095b <+0>:     call   0x960 <sst_shim32_write64+5>
   0x0000000000000960 <+5>:     push   rbp
   0x0000000000000961 <+6>:     mov    rbp,rsp
   0x0000000000000964 <+9>:     mov    eax,esi
   0x0000000000000966 <+11>:    mov    QWORD PTR [rdi+rax*1],rdx
   0x000000000000096a <+15>:    pop    rbp
   0x000000000000096b <+16>:    ret
End of assembler dump.


Regards,
Brent

> 
> diff --git a/sound/soc/intel/common/sst-dsp.c
> b/sound/soc/intel/common/sst-dsp.c
> index ec66be269b69..e96f636387d9 100644
> --- a/sound/soc/intel/common/sst-dsp.c
> +++ b/sound/soc/intel/common/sst-dsp.c
> @@ -34,16 +34,13 @@ EXPORT_SYMBOL_GPL(sst_shim32_read);
> 
>   void sst_shim32_write64(void __iomem *addr, u32 offset, u64 value)
>   {
> -       memcpy_toio(addr + offset, &value, sizeof(value));
> +       writeq(value, addr + offset);
>   }
>   EXPORT_SYMBOL_GPL(sst_shim32_write64);
> 
>   u64 sst_shim32_read64(void __iomem *addr, u32 offset)
>   {
> -       u64 val;
> -
> -       memcpy_fromio(&val, addr + offset, sizeof(val));
> -       return val;
> +       return readq(addr + offset);
>   }
>   EXPORT_SYMBOL_GPL(sst_shim32_read64);
> 
> 
> Thanks,
> Amadeusz