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

[Ben Widawsky]

On 21-01-06 12:59:35, Dan Williams wrote:
> On Wed, Jan 6, 2021 at 12:40 PM Dave Jiang <dave.jiang@xxxxxxxxx> 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@xxxxxxxxx>