Re: [tip:x86/urgent] x86/io: Mark target address as output in 'insb()' asm

From: Linus Torvalds
Date: Wed Jul 12 2017 - 18:44:06 EST


On Wed, Jul 12, 2017 at 2:47 PM, Arnd Bergmann <arnd@xxxxxxxx> wrote:
>>
>> The real fix is probably to just mark them as "clobbers memory" (ie
>> just add "memory" to the clobber list).
>>
>> If you want to be fancy, you can try to do what <asm/uaccess.h> does,
>> which is a disgusting hack, but has traditionally worked;
>>
>> struct __large_struct { unsigned long buf[100]; };
>> #define __m(x) (*(struct __large_struct __user *)(x))
>>
>> and then use your approach with "m" and "=m".
>
> Ok, I'll try both tomorrow and see where I end up.

Note that just adding the "memory" thing to the clobbers likely causes
slightly worse code generation (it basically says that the asm can
clobber anything at all, so cause re-loads etc that are entirely
unrelated to the asm).

But for something like "rep in/out", that really doesn't much matter.
PIO is very slow due to being fully serialized, and "rep ins/outs" is
just about the slowest thing you can do on a machine. So nobody really
does it, the main traditional user was the legacy PIO data transfer
for ST-506 disks. And, as you noticed a few *really* old network card
drivers.

So I suspect the big hammer memory clobber is the right thing to do.

> Would you expect that the missing clobber causes actual
> runtime bugs and the fix needs to be backported to stable
> kernels?

Probably not. "asm volatile" is already pretty serialized. It's not
entirely obvious what gcc will move around it, but almost certainly no
operations that matter for the network buffer.

And honestly, the wt3501 is basically an ISA card in PCMCIA format. I
don't think it's even cardbus (Cardbus aka Yenta is basically "hotplug
PCI"). So I don't think anybody even has that hardware.

It was a good card for its time. But its time was basically two decades ago.

Linus