linux-kernel - Re: [PATCH] x86: fix movdir64b() sparse warning

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: <CAPcyv4jYeJ48eY5-PxjkARizjQWqsSZuG=63+d526f7sjLK_tQ@mail.gmail.com>
Date:   Wed, 6 Jan 2021 12:59:35 -0800
From:   Dan Williams <dan.j.williams@...el.com>
To:     Dave Jiang <dave.jiang@...el.com>
Cc:     Borislav Petkov <bp@...en8.de>, X86 ML <x86@...nel.org>,
        kernel test robot <lkp@...el.com>,
        Ben Widawsky <ben.widawsky@...el.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] x86: fix movdir64b() sparse warning

On Wed, Jan 6, 2021 at 12:40 PM Dave Jiang <dave.jiang@...el.com> wrote:
>
> Add missing __iomem anotation to address sparse warning.

s/anotation/annotation/

>
> "sparse warnings: (new ones prefixed by >>)"
>    drivers/dma/idxd/submit.c: note: in included file (through include/linux/io.h, include/linux/pci.h):
> >> arch/x86/include/asm/io.h:422:27: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void *dst @@     got void [noderef] __iomem *dst @@
>    arch/x86/include/asm/io.h:422:27: sparse:     expected void *dst
>    arch/x86/include/asm/io.h:422:27: sparse:     got void [noderef] __iomem *dst

The sparse spew is somewhat interesting, but what would be more
helpful is explain the why. I.e. that existing and future users expect
to be passing an __iomem annotated pointer to this routine because...
<reasons go here>. Otherwise someone (reviewer / future git blame
user) might reasonably ask, "well, why is the driver passing an
__iomem annotated pointer in the first instance?".

To Ben's point you might also duplicate part of the comment from
movdir64b and say:

"Recall, from the comment in movdir64b @__dst  must be supplied as an
lvalue because this tells the compiler what the object is (its size)
the instruction accesses. I.e., not the pointers but what they point
to, thus the deref'ing '*'."

With clarified changelog for both you can add:

Reviewed-by: Dan Williams <dan.j.williams@...el.com>