netdev - Re: [PATCH mlx5-next 4/4] net/mlx5: Remove spinlock support from mlx5

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]

Date:   Mon, 21 Jan 2019 18:22:08 +0000
From:   Jason Gunthorpe <jgg@...lanox.com>
To:     Saeed Mahameed <saeedm@....mellanox.co.il>
CC:     Leon Romanovsky <leonro@...lanox.com>,
        Saeed Mahameed <saeedm@...lanox.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        Maxim Mikityanskiy <maximmi@...lanox.com>,
        Eran Ben Elisha <eranbe@...lanox.com>
Subject: Re: [PATCH mlx5-next 4/4] net/mlx5: Remove spinlock support from
 mlx5_write64

On Mon, Jan 21, 2019 at 10:12:58AM -0800, Saeed Mahameed wrote:
> On Mon, Jan 21, 2019 at 8:46 AM Jason Gunthorpe <jgg@...lanox.com> wrote:
> >
> > On Sat, Jan 19, 2019 at 12:43:14AM -0700, Leon Romanovsky wrote:
> > > You need to do one of two things:
> > > 1. Require CONFIG_64BIT and delete this 32bit code.
> > > 2. Declare global mlx5 DB spinlock and use on 32bit systems, something
> > > like this:
> > > #if BITS_PER_LONG == 64
> > >  __raw_writeq(*(u64 *)val, dest);
> > > #else
> > >   spin_lock_irqsave(doorbell_lock, flags);
> > >   __raw_writel((__force u32) val[0], dest);
> > >   __raw_writel((__force u32) val[1], dest + 4);
> > >    spin_unlock_irqrestore(doorbell_lock, flags);
> > > #endif
> >
> > And why is this code using the __raw_ versions? Seems wrong too...
> >
> 
> for 64 and 32 as well?

yes

> what is wrong with the raw version ?

It should only be used by arch code (or in drivers linked to a
specific arch). The actual properties of the 'raw' version are arch
specific and make it hard to know if the driver will work on different
archs. ie some arches may not byte swap their raw accessors, or may
omit barriers.

Most likely this just wants to be writeq for 64 bit and
writel_relaxed() & writel() for 32 bit - unless there was some reason
to have used __raw versions in the first place (in which case a
comment is missing).

Jason