Re: Question Regarding ERMS memcpy

From: Borislav Petkov
Date: Sun Mar 05 2017 - 15:19:47 EST


On Sun, Mar 05, 2017 at 11:19:42AM -0800, Linus Torvalds wrote:
> Actually, the "fromio/toio" code should never use regular memcpy().
> There used to be devices that literally broke on 64-bit accesses due
> to broken PCI crud.
>
> We seem to have broken this *really* long ago, though.

I wonder why nothing blew up or failed strangely by now...

> On x86-64 we used to have a special __inline_memcpy() that copies our
> historical

It is still there in arch/x86/include/asm/string_64.h.

The comment "Only used for special circumstances." over it is grossly
understating it.

> 32-bit thing, and was used for memcpy_fromio() and memcpy_toio(). That
> was then undone by commit 6175ddf06b61 ("x86: Clean up mem*io
> functions")
>
> That commit says
>
> "Iomem has no special significance on x86"
>
> but that's not strictly true. iomem is in the same address space and
> uses the same access instructions as regular memory, but iomem _is_
> special.
>
> And I think it's a bug that we use "memcpy()" on it. Not because of
> any gcc issues, but simply because our own memcpy() optimizations are
> not appropriate for iomem.
>
> For example, "rep movsb" really is the right thing to use on normal
> memory on modern CPU's.

So Logan's box is a SNB and it doesn't have the ERMS optimizations. Are
you saying, regardless, we should let gcc put REP; MOVSB for smaller
sizes?

Because gcc does generate a REP; MOVSB there when it puts its own
memcpy, see mail upthread. (Even though that is wrong to do on iomem.)

> But it is *not* the right thing to use on IO memory, because the CPU
> only does the magic cacheline access optimizations on cacheable
> memory!

Ah, yes.

> So I think we should re-introduce that old "__inline_memcpy()" as that
> special "safe memcpy" thing. Not just for KMEMCHECK, and not just for
> 64-bit.

Logan, wanna give that a try, see if it takes care of your issue?

Oh, and along with the revert we would need a big fat warning explaining
why we need that special memcpy for IO memory.

--
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix ImendÃrffer, Jane Smithard, Graham Norton, HRB 21284 (AG NÃrnberg)
--