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] [day] [month] [year] [list]

Date:   Wed, 6 Jan 2021 13:04:06 -0800
From:   Ben Widawsky <ben.widawsky@...el.com>
To:     Dan Williams <dan.j.williams@...el.com>
Cc:     Dave Jiang <dave.jiang@...el.com>, Borislav Petkov <bp@...en8.de>,
        X86 ML <x86@...nel.org>, kernel test robot <lkp@...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 21-01-06 12:59:35, Dan Williams wrote:
> 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 '*'."

Thanks for pasting this, I missed that. It still doesn't make sense to me why
the compiler needs to know this. I guess it makes sense to the rest of you.

> 
> With clarified changelog for both you can add:
> 
> Reviewed-by: Dan Williams <dan.j.williams@...el.com>