Re: [patch 134/149] x86, paravirt: Add a global synchronization point for pvclock
From: Zachary Amsden
Date: Wed Jul 14 2010 - 16:55:08 EST
On 07/14/2010 10:45 AM, Zachary Amsden wrote:
On 07/14/2010 10:40 AM, Jeremy Fitzhardinge wrote:
On 07/14/2010 01:16 PM, Avi Kivity wrote:
On 07/14/2010 08:57 PM, Jeremy Fitzhardinge wrote:
Anything else?
1. set up a mapping
2. invlpg or set cr3
3. use the mapping
Moving the invlpg will break your code.
invlpg uses memory clobbers. All the crX ops seem to use a
__force_order variable to sequence them - but it looks like it's done
precisely backwards and it's barking mad to do allow write_crX to be
reordered with respect to memory ops.
Hm, looks like glommer added it surreptitiously while unifying
system_32.h and system_64.h (system_32.h relied on asm volatile not
being reordered; system_64.h used memory clobbers).
J
clts() has no memory clobber; it is used to serialize execution of
code within kernel_fpu_begin() / kernel_fpu_end() blocks.
If the code within is reordered before the clts(), we've corrupted
guest FPU state.
That's the kind of bug I think Linus is talking about. We've been
expecting volatile to work that way for over a decade, by my
recollection, and if it doesn't, there is going to be a lot of broken
code.
Shouldn't we at least get a compiler switch to force the volatile
behavior? I'd suggest it default to conservative.
Hmm, well, despite that not being quite correct (if guest has used FPU,
we save it, which has a memory clobber), it seems to be the case that a
reordering of the clts() among the other volatile asm statements would
be a very bad thing - you'd get kernel FPU exceptions.
And among asm volatiles, clts() is fairly unique in not having any
clobbers or dependencies at all, so it could happen.
Zach
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/