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, 19 Jan 2022 11:04:18 -0800
From:   "Martinez, Ricardo" <>
To:     Ilpo Järvinen <>
Cc:     Netdev <>,,,,,,,,,,,,,
        Andy Shevchenko <>,,,,,,,
Subject: Re: [PATCH net-next v4 02/13] net: wwan: t7xx: Add control DMA

On 1/19/2022 1:52 AM, Ilpo Järvinen wrote:
> On Tue, 18 Jan 2022, Martinez, Ricardo wrote:
>> On 1/18/2022 6:13 AM, Ilpo Järvinen wrote:
>>> On Thu, 13 Jan 2022, Ricardo Martinez wrote:
>>> +static bool t7xx_cldma_qs_are_active(struct t7xx_cldma_hw *hw_info)
>>> +{
>>> +	unsigned int tx_active;
>>> +	unsigned int rx_active;
>>> +
>>> +	tx_active = t7xx_cldma_hw_queue_status(hw_info, CLDMA_ALL_Q, MTK_TX);
>>> +	rx_active = t7xx_cldma_hw_queue_status(hw_info, CLDMA_ALL_Q, MTK_RX);
>>> +	if (tx_active == CLDMA_INVALID_STATUS || rx_active ==
>>> These cannot ever be true because of mask in t7xx_cldma_hw_queue_status().
>> t7xx_cldma_hw_queue_status() shouldn't apply the mask for CLDMA_ALL_Q.
> I guess it shouldn't but it currently does apply 0xff (CLDMA_ALL_Q) as
> mask in that case. However, this now raises another question, if
> 0xffffffff (CLDMA_INVALID_STATUS) means status is invalid, should all
> callers both single Q and CLDMA_ALL_Q be returned/check/handle that value?
> Why would CLDMA_ALL_Q be special in this respect that the INVALID_STATUS
> means invalid only with it?

Reading 0xffffffff is used to detect if the PCI link was disconnected,
it is relevant in t7xx_cldma_qs_are_active() because it is a helper function
polled by t7xx_cldma_stop() to wait until the queues are not active anymore.

I think a cleaner implementation would be to use pci_device_is_present()
instead of the CLDMA_INVALID_STATUS check inside t7xx_cldma_qs_are_active()
and keep t7xx_cldma_hw_queue_status() free of that logic.


Powered by blists - more mailing lists