netdev - Re: [PATCH v5 3/9] net: stmmac: Add Loongson DWGMAC definitions

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: <4f9a4d75-7052-4314-bbd1-838a642b80ab@loongson.cn>
Date: Sun, 26 Nov 2023 20:25:13 +0800
From: Yanteng Si <siyanteng@...ngson.cn>
To: Serge Semin <fancer.lancer@...il.com>, Andrew Lunn <andrew@...n.ch>
Cc: hkallweit1@...il.com, peppe.cavallaro@...com,
 alexandre.torgue@...s.st.com, joabreu@...opsys.com, Jose.Abreu@...opsys.com,
 chenhuacai@...ngson.cn, linux@...linux.org.uk, dongbiao@...ngson.cn,
 guyinggang@...ngson.cn, netdev@...r.kernel.org, loongarch@...ts.linux.dev,
 chris.chenfeiyang@...il.com
Subject: Re: [PATCH v5 3/9] net: stmmac: Add Loongson DWGMAC definitions


在 2023/11/25 00:44, Serge Semin 写道:
> On Fri, Nov 24, 2023 at 03:51:08PM +0100, Andrew Lunn wrote:
>>> In general, we split one into two.
>>>
>>> the details are as follows：
>>>
>>> DMA_INTR_ENA_NIE = DMA_INTR_ENA_NIE_LOONGSON= DMA_INTR_ENA_TX_NIE +
>>> DMA_INTR_ENA_RX_NIE
>> What does the documentation from Synopsys say about the bit you have
>> used for DMA_INTR_ENA_NIE_LOONGSON? Is it marked as being usable by IP
>> integrators for whatever they want, or is it marked as reserved?
>>
>> I'm just wondering if we are heading towards a problem when the next
>> version of this IP assigns the bit to mean something else.
> That's what I started to figure out in my initial message:
> Link: https://lore.kernel.org/netdev/gxods3yclaqkrud6jxhvcjm67vfp5zmuoxjlr6llcddny7zwsr@473g74uk36xn/
> but Yanteng for some reason ignored all my comments.

Sorry, I always keep your review comments in mind, and even this version 
of the patch is

largely based on your previous comments. Please give me some more time 
and I promise

to answer your comments before the next patch is made.

>
> Anyway AFAICS this Loongson GMAC has NIE and AIE flags defined differently:
> DW GMAC: NIE - BIT(16) - all non-fatal Tx and Rx errors,
>           AIE - BIT(15) - all fatal Tx, Rx and Bus errors.
> Loongson GMAC: NIE - BIT(18) | BIT(17) - one flag for Tx and another for Rx errors.
>                 AIE - BIT(16) | BIT(15) - Tx, Rx and don't know about the Bus errors.
> So the Loongson GMAC has not only NIE/AIE flags re-defined, but
> also get to occupy two reserved in the generic DW GMAC bits: BIT(18) and BIT(17).
>
> Moreover Yanteng in his patch defines DMA_INTR_NORMAL as a combination
> of both _generic_ DW and Loongson-specific NIE flags and
> DMA_INTR_ABNORMAL as a combination of both _generic_ DW and
> Loongson-specific AIE flags. At the very least it doesn't look
> correct, since _generic_ DW GMAC NIE flag BIT(16) is defined as a part
> of the Loongson GMAC AIE flags set.

We will consider this seriously, please give us some more time, and of 
course, we

are looking forward to your opinions on this problem.


I hope you can accept my apologies, Please allow me to say sorry again.


Thanks for your review!


Thanks,

Yanteng

>
> -Serge(y)
>
>> 	Andrew