netdev - Re: [PATCH 2/2] net: Add driver for LiteX's LiteETH network interface

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>] [day] [month] [year] [list]

Message-ID: <CACPK8Xds9RysihvNSJfxcL4n1-eum3v1gauh406jEfdu=S7pcw@mail.gmail.com>
Date:   Fri, 20 Aug 2021 07:46:27 +0000
From:   Joel Stanley <joel@....id.au>
To:     Florent Kermarrec <florent@...oy-digital.fr>
Cc:     "Gabriel L. Somlo" <gsomlo@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Stafford Horne <shorne@...il.com>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Anton Blanchard <anton@...abs.org>, David Shah <dave@....me>,
        Karol Gugala <kgugala@...micro.com>,
        Mateusz Holenko <mholenko@...micro.com>,
        devicetree <devicetree@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] net: Add driver for LiteX's LiteETH network interface

Hi Florent and Gabriel,

On Fri, 6 Aug 2021 at 12:27, Florent Kermarrec <florent@...oy-digital.fr> wrote:
>
> Hi Gabriel, Joel,
>
> for 1), the polling mode has been useful recently with Linux-On-LiteX-Vexriscv to bring up the SMP version before enabling the IRQs and can probably be useful in the future for similar purposes. So if it's not too complicated to maintain, this could be useful to keep it.

I would prefer to get a basic working driver for the straight forward
Linux-on-FPGA use case merged first. We can then work out what should
be done next. I would like to see hardware improvements to make it go
faster, and we can look at other features like polling if you still
need them.

>
> Le ven. 6 août 2021 à 13:48, Gabriel L. Somlo <gsomlo@...il.com> a écrit :
>>
>> Hi Joel,
>>
>> Thanks for pushing this upstream (and for writing it to begin with)!
>>
>> Would you mind diff-ing your version of litex_liteeth.c against what
>> is currently in
>> https://github.com/litex-hub/linux/blob/litex-rebase/drivers/net/ethernet/litex/litex_liteeth.c ?
>>
>> Two main differences we should discuss:
>>
>>         1. there's a polling mode (added by Antony Pavlov), and if we
>>            decide *not* to keep it around, I want to ensure we do that
>>            deliberately, with an explanation as to why;

I assumed this was only for broken systems, or systems that were still
being brought up. Haven't the risc-v socs improved to the point where
they have working interrupts now?

>>
>>         2. LiteX CSRs are accessed using `litex_[read|write][8|16|32]()`
>>            as opposed to simply `[read|write][b|w|l]()`. The former set
>>            are defined in `include/linux/litex.h` and are needed to
>>            ensure correct accesses regardless of endianness, since by
>>            default LiteX registers' endianness mirrors that of the
>>            configured CPU.

I don't like that they get the parameters in the wrong order compared
to the rest of the kernel, and that they force 32-bit access no matter
the width of the access being performed. They also mean we can't use
_releaxed variants of accessors to eg. read a set of registers without
having a barrier in between.

That said, I have used the csr accessors so we can support the mor1k,
which afaict is the only litex CPU that defaults to big endian.

If you could help review the patch so we can get it merged this cycle,
then we can work on future enhancements.

Cheers,

Joel