linux-kernel - Re: [PATCH v2 1/2] mips: add <asm-generic/io.h> including

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: <5edd5304-ef11-4607-9189-a07613ecfee2@app.fastmail.com>
Date:   Wed, 01 Mar 2023 15:06:57 +0100
From:   "Arnd Bergmann" <arnd@...db.de>
To:     "Baoquan He" <bhe@...hat.com>, linux-kernel@...r.kernel.org
Cc:     Linux-Arch <linux-arch@...r.kernel.org>, linux-mm@...ck.org,
        "Geert Uytterhoeven" <geert@...ux-m68k.org>,
        "Christoph Hellwig" <hch@...radead.org>,
        "Luis Chamberlain" <mcgrof@...nel.org>,
        "Thomas Bogendoerfer" <tsbogend@...ha.franken.de>,
        "Huacai Chen" <chenhuacai@...nel.org>,
        "Jiaxun Yang" <jiaxun.yang@...goat.com>, linux-mips@...r.kernel.org
Subject: Re: [PATCH v2 1/2] mips: add <asm-generic/io.h> including

On Wed, Mar 1, 2023, at 11:22, Baoquan He wrote:
> With the adding, some default ioremap_xx methods defined in
> asm-generic/io.h can be used. E.g the default ioremap_uc() returning
> NULL.
>
> Here, remove the <asm/io.h> including in asm/mmiowb.h, otherwise nested
> including will cause compiling error.
>
> Signed-off-by: Baoquan He <bhe@...hat.com>
> Cc: Thomas Bogendoerfer <tsbogend@...ha.franken.de>
> Cc: Huacai Chen <chenhuacai@...nel.org>
> Cc: Jiaxun Yang <jiaxun.yang@...goat.com>
> Cc: linux-mips@...r.kernel.org

This looks good to me,

Reviewed-by: Arnd Bergmann <arnd@...db.de>

but it obviously needs to be properly reviewed by the MIPS
maintainers as well. I think others have tried to do this
in the past but did not make it in.

> @@ -548,6 +552,46 @@ extern void (*_dma_cache_inv)(unsigned long start, 
> unsigned long size);
>  #define csr_out32(v, a) (*(volatile u32 *)((unsigned long)(a) + 
> __CSR_32_ADJUST) = (v))
>  #define csr_in32(a)    (*(volatile u32 *)((unsigned long)(a) + 
> __CSR_32_ADJUST))
> 
> +
> +#define inb_p inb_p
> +#define inw_p inw_p
> +#define inl_p inl_p
> +#define insb insb
> +#define insw insw
> +#define insl insl

I would prefer to put the #defines next to the function declarations,
even when they come from macros.

> 
> -#include <asm/io.h>
> -
>  #define mmiowb()	iobarrier_w()
> 

I think this only works as long as asm/spinlock.h also includes
asm/io.h, otherwise linux/spinlock.h will be missing the
iobarrier_w definition.

Most likely this is implicitly included from somewhere else
below linux/spinlock.h, but it would be better not to rely
on that, and instead define mmiowb() to wmb() directly.

     Arnd