netdev - Re: [PATCH v4 2/3] net: ethernet: renesas: Add Ethernet Switch driver

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: <CAMuHMdWbkro70fmyauUnEPyKZYytWD0o4a06=UzDTzCZ9-B6vw@mail.gmail.com>
Date:   Mon, 24 Oct 2022 22:35:39 +0200
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     kernel test robot <lkp@...el.com>,
        Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
        "David S . Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Rob Herring <robh+dt@...nel.org>,
        krzysztof.kozlowski+dt@...aro.org, kbuild-all@...ts.01.org,
        Netdev <netdev@...r.kernel.org>, devicetree@...r.kernel.org,
        Linux-Renesas <linux-renesas-soc@...r.kernel.org>
Subject: Re: [PATCH v4 2/3] net: ethernet: renesas: Add Ethernet Switch driver

Hi Arnd,

On Mon, Oct 24, 2022 at 9:55 PM Arnd Bergmann <arnd@...db.de> wrote:
> On Mon, Oct 24, 2022, at 17:27, Geert Uytterhoeven wrote:
> > On Wed, Oct 19, 2022 at 1:17 PM kernel test robot <lkp@...el.com> wrote:
> >>    drivers/net/ethernet/renesas/rswitch.c: In function 'rswitch_ext_desc_get_dptr':
> >> >> drivers/net/ethernet/renesas/rswitch.c:355:71: warning: left shift count >= width of type [-Wshift-count-overflow]
> >>      355 |         return __le32_to_cpu(desc->dptrl) | (dma_addr_t)(desc->dptrh) << 32;
> >>          |                                                                       ^~
> >>    drivers/net/ethernet/renesas/rswitch.c: In function 'rswitch_ext_ts_desc_get_dptr':
> >>    drivers/net/ethernet/renesas/rswitch.c:367:71: warning: left shift count >= width of type [-Wshift-count-overflow]
> >>      367 |         return __le32_to_cpu(desc->dptrl) | (dma_addr_t)(desc->dptrh) << 32;
> >>          |                                                                       ^~
> >>
> >>
> >> vim +355 drivers/net/ethernet/renesas/rswitch.c
> >>
> >>    352
> >>    353  static dma_addr_t rswitch_ext_desc_get_dptr(struct rswitch_ext_desc *desc)
> >>    354  {
> >>  > 355          return __le32_to_cpu(desc->dptrl) | (dma_addr_t)(desc->dptrh) << 32;
> >
> > A simple fix would be to replace the cast to "dma_addr_t" by a cast to "u64".
> > A more convoluted fix would be:
> >
> >     dma_addr_t dma;
> >
> >     dma = __le32_to_cpu(desc->dptrl);
> >     if (IS_ENABLED(CONFIG_ARCH_DMA_ADDR_T_64BIT))
> >             dma |= (u64)desc->dptrh << 32;
> >     return dma;
> >
> > Looking at the gcc compiler output, the both cases are optimized to the
> > exact same code, for both arm32 and arm64, so I'd go for the simple fix.
> >
> > BTW, if struct rswitch_ext_desc would just extend struct rswitch_desc,
> > you could use rswitch_ext_desc_get_dptr() for both.
> >
>
> Regardless of which way this is expressed, it looked like there is
> a missing __le32_to_cpu() around the high word.

I think it's OK, because desc->dptrh is u8:

    struct rswitch_desc {
            __le16 info_ds; /* Descriptor size */
            u8 die_dt;      /* Descriptor interrupt enable and type */
            __u8  dptrh;    /* Descriptor pointer MSB */
            __le32 dptrl;   /* Descriptor pointer LSW */
    } __packed;

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds