netdev - Re: [PATCH v5 04/17] octeontx2-pf: Initialize and config queues

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: <CA+sq2CcQk47hZ9tue1-yjGmUFF7RONfG47c2T77SRU5t8ovpVg@mail.gmail.com>
Date:   Sun, 26 Jan 2020 23:30:04 +0530
From:   Sunil Kovvuri <sunil.kovvuri@...il.com>
To:     David Miller <davem@...emloft.net>
Cc:     Linux Netdev List <netdev@...r.kernel.org>,
        Jakub Kicinski <kubakici@...pl>,
        Michal Kubecek <mkubecek@...e.cz>,
        Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
        Sunil Goutham <sgoutham@...vell.com>,
        Geetha sowjanya <gakula@...vell.com>
Subject: Re: [PATCH v5 04/17] octeontx2-pf: Initialize and config queues

On Sun, Jan 26, 2020 at 4:31 PM David Miller <davem@...emloft.net> wrote:
>
> From: sunil.kovvuri@...il.com
> Date: Fri, 24 Jan 2020 23:15:42 +0530
>
> > @@ -184,6 +192,72 @@ static inline void otx2_mbox_unlock(struct mbox *mbox)
> >       mutex_unlock(&mbox->lock);
> >  }
> >
> > +/* With the absence of API for 128-bit IO memory access for arm64,
> > + * implement required operations at place.
> > + */
> > +#if defined(CONFIG_ARM64)
> > +static inline void otx2_write128(u64 lo, u64 hi, void __iomem *addr)
> > +{
> > +     __asm__ volatile("stp %x[x0], %x[x1], [%x[p1],#0]!"
> > +                      ::[x0]"r"(lo), [x1]"r"(hi), [p1]"r"(addr));
> > +}
> > +
> > +static inline u64 otx2_atomic64_add(u64 incr, u64 *ptr)
> > +{
> > +     u64 result;
> > +
> > +     __asm__ volatile(".cpu   generic+lse\n"
> > +                      "ldadd %x[i], %x[r], [%[b]]"
> > +                      : [r]"=r"(result), "+m"(*ptr)
> > +                      : [i]"r"(incr), [b]"r"(ptr)
> > +                      : "memory");
> > +     return result;
> > +}
> > +
> > +#else
> > +#define otx2_write128(lo, hi, addr)
> > +#define otx2_atomic64_add(incr, ptr)         ({ *ptr = incr; })
> > +#endif
>
> So what exactly is going on here?  Are these true 128-bit writes
> and atomic operations?  Why is it named atomic64 then?  Why can't
> the normal atomic64 kernel interfaces be used?

otx2_write128() is used to free receive buffer pointers into buffer pool.
It's a register write, which works like,
"A 128-bit write (STP) to NPA_LF_AURA_OP_FREE0 and
NPA_LF_AURA_OP_FREE1 frees a pointer into a given pool. All other
accesses to these registers (e.g. reads and 64-bit writes) are RAZ/WI."

Wrt otx2_atomic64_add(), registers for reading IRQ status, queue stats etc
works only with 64-bit atomic load-and-add instructions. The nornal
atomic64 kernel
interface for ARM64 which supports 'ldadd' instruction needs
CONFIG_ARM64_LSE_ATOMICS
to be enabled. LSE (Large system extensions) is a CPU feature which is supported
by silicons which implement ARMv8.1 and later version of instruction set.

To support kernel with and without LSE_ATOMICS config enabled, here we are
passing "cpu   generic+lse" to the compiler. This is also done to avoid making
ARM64 and ARM64_LSE_ATOMICS hard dependency for driver compilation.

>
> Finally why is the #else case doing an assignment to *ptr rather
> than an increment like "*ptr += incr;"?

This device is a on-chip network controller which is a ARM64 based.
Previously when i submitted driver with ARM64 dependency i was advised
to allow this driver to be built for other architectures as well for
static analysis
reports etc.
https://www.spinics.net/lists/linux-soc/msg05847.html

Hence added a dummy 'otx2_atomic64_add' just for compilation purposes.
Please ignore the definition.

Thanks,
Sunil.