linux-kernel - Re: [PATCH 0/2] soundwire: Remove redundant zeroing of page registers

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 linux-cve-announce 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]

Message-ID: <266bf397-4395-873b-c933-73a9e28f463c@opensource.cirrus.com>
Date:   Fri, 2 Dec 2022 11:26:21 +0000
From:   Richard Fitzgerald <rf@...nsource.cirrus.com>
To:     Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
        <vkoul@...nel.org>
CC:     <alsa-devel@...a-project.org>, <patches@...nsource.cirrus.com>,
        <linux-kernel@...r.kernel.org>, <sanyog.r.kale@...el.com>,
        <yung-chuan.liao@...ux.intel.com>
Subject: Re: [PATCH 0/2] soundwire: Remove redundant zeroing of page registers

On 01/12/2022 18:31, Pierre-Louis Bossart wrote:
> 
> 
> On 12/1/22 08:08, Richard Fitzgerald wrote:
>> Writing zero to the page registers after each message transaction can add
>> up to a lot of overhead for codecs that need to transfer large amount of
>> data - for example a firmware download.
>>
>> There's no spec reason I can see for this zeroing. The page registers are
>> only used for a paged address. The bus code uses a non-paged address for
>> registers in page 0. It always writes the page registers at the start of
>> a paged transaction.
>>
>> If this zeroing was a workaround for anything, let me know and I will
>> re-implement the zeroing as a quirk that can be enabled only when it is
>> necessary.
> 
> It's a feature, not a bug :-)
> 
> The page registers have to be zeroed out so that any bus-management
> command hits the page0 instead of using a value that was set by codec
> driver for vendor-specific configurations.
> 

Why would these bus management commands set bit 15 to indicate a paged
access? If they don't set bit 15 the page registers are not used and
bits 15..31 of the register address must be 0. Table 78 in the Soundwire
1.2 spec. Table 71 in the 1.0 spec. Table 43 in the 0.6 draft spec.


> The implementation is far from optimal though, and indeed if we have
> long transactions that are not interrupted by anything else we could
> avoid resetting the page registers.
> 
> I tried to implement a 'lazy approach' some time back, but at the time I
> didn't see any benefits due to the limited number of configurations.
> 
> I can't remember where the code is, but the initial enhancement was
> listed here: https://github.com/thesofproject/linux/issues/2881
> 
>>
>> Richard Fitzgerald (2):
>>    soundwire: bus: Don't zero page registers after every transaction
>>    soundwire: bus: Remove unused reset_page_addr() callback
>>
>>   drivers/soundwire/bus.c             | 23 -----------------------
>>   drivers/soundwire/cadence_master.c  | 14 --------------
>>   drivers/soundwire/cadence_master.h  |  3 ---
>>   drivers/soundwire/intel_auxdevice.c |  1 -
>>   include/linux/soundwire/sdw.h       |  3 ---
>>   5 files changed, 44 deletions(-)
>>