netdev - Re: [PATCH net-next V1 09/10] net/mlx5: Fix global UAR mapping

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20160228.232804.1263072658789063316.davem@davemloft.net>
Date:	Sun, 28 Feb 2016 23:28:04 -0500 (EST)
From:	David Miller <davem@...emloft.net>
To:	saeedm@...lanox.com
Cc:	netdev@...r.kernel.org, ogerlitz@...lanox.com, eranbe@...lanox.com,
	talal@...lanox.com, majd@...lanox.com, moshel@...lanox.com
Subject: Re: [PATCH net-next V1 09/10] net/mlx5: Fix global UAR mapping

From: Saeed Mahameed <saeedm@...lanox.com>
Date: Sun, 28 Feb 2016 17:09:10 +0200

> We use ARCH_HAS_IOREMAP_WC to know if the current arch supports WC
> (Write combining) IO memory mapping, if it is not supported
> "uar->bf_map" will be NULL, thus we will use NC (Non Cached) mapping
> "uar->map".

This description sucks.

You're just saying what will happen if the CPP is defined or not
(uar->bf_map ends up being NULL).

Well anyone can see that from the code.

You have to explain why.

And BTW, ARCH_HAS_IOREMAP_WC doesn't even tell you if the platform
will actually give you a write-combining mapping.

So if it's the driver operates properly if a non-WC mapping is used
for uar->bf_map, then get rid of this CPP test altogether PLEASE!

Otherwise your driver is buggy, because ARCH_HAS_IOREMAP_WC only says
whether the default implementation of ioremap_wc() needs to be
provided by include/asm-generic/iomap.h It does not guarantee that a
write-combining mapping will be provided.

I really can't think of any reason why you absolutely require a
WC mapping, and the CPP test just makes your driver look more
ugly than it needs to me.

So can you please explain what the hell is happening here and why you
are doing things this way rather than just reading the code to me?

Thanks.