On Wed, Dec 23, 2009 at 05:04:19PM +1030, Rusty Russell wrote: > On Wed, 23 Dec 2009 01:21:43 am Anthony Liguori wrote: > > On 12/22/2009 05:26 AM, Michael S. Tsirkin wrote: > > > On Tue, Dec 08, 2009 at 06:18:18PM +0200, Michael S. Tsirkin wrote: > > > > > >> The following fixes a class of long-standing bugs in qemu: > > >> when kvm is enabled, guest might access device structures > > >> in memory while they are updated by qemu on another CPU. > > >> In this scenario, memory barriers are necessary to prevent > > >> host CPU from reordering memory accesses, which might confuse > > >> the guest. > > >> > > >> This patch only fixes virtio, but other emulated devices > > >> might have a similar bug. They'll need to be discovered > > >> and addressed case by case. > > >> > > >> This is still under test ... meanwhile: any early feedback/flames? > > >> > > >> > > > Any comments on this one? > > > The patch works fine in my testing, and even though > > > it did not fix a crash that I hoped it will fix, > > > it seems required for correctness... Right? > > > > > > > It's definitely better than what we have. Rusty mentioned something to > > me a bit ago about the barriers for virtio in the kernel needing some > > work. I've been meaning to ask him about it in the context of this patch. > > > > Rusty, am I remembering correctly or have I been sipping too much > > eggnog? :-) > > It's possible, but I don't know of any missing cases. Certainly *lguest* i > is missing barriers, since it's UP, but the core virtio should have them. > > Cheers, > Rusty.
Something that Paul Brook pointed out, is that using a 16 bit value in C like we do in guest, e.g. with ring->avail.idx might in theory result in two single byte reads. If this happens, guest will see a wrong index value. I think this does not happen in practice, because gcc is not that silly, and it can even optimize out memcpy operations for 2 bytes. But maybe this means Linux and qemu need an API to do atomic loads/stores for fixed width integers, just to make the asumption explicit. Thoughts? -- MST
