On Wed, Apr 29, 2015 at 04:43:19PM +0200, Christian Borntraeger wrote: > Am 29.04.2015 um 12:32 schrieb Michael S. Tsirkin: > > On Wed, Apr 29, 2015 at 10:52:15AM +0200, Cornelia Huck wrote: > >> On Wed, 29 Apr 2015 10:17:55 +0200 > >> Christian Borntraeger <borntrae...@de.ibm.com> wrote: > >> > >>> Am 28.04.2015 um 20:32 schrieb Michael S. Tsirkin: > >>>> On Tue, Apr 28, 2015 at 08:14:44PM +0200, Michael S. Tsirkin wrote: > >>>>> On Tue, Apr 28, 2015 at 04:35:16PM +0200, Michael S. Tsirkin wrote: > >>>>>> On Tue, Apr 28, 2015 at 03:24:19PM +0200, Cornelia Huck wrote: > >>>>>>> On Tue, 28 Apr 2015 14:16:40 +0100 > >>>>>>> Peter Maydell <peter.mayd...@linaro.org> wrote: > >>>>>>> > >>>>>>>> On 28 April 2015 at 14:13, Michael S. Tsirkin <m...@redhat.com> > >>>>>>>> wrote: > >>>>>>>>> The patches look correct to me too, but I want s390 > >>>>>>>>> cleaned up so it does not include COMMON_FEATURES > >>>>>>>>> in 100 places, and I prefer merging it all together. > >>>>>>>> > >>>>>>>> It seems a bit harsh to ask Shannon to do s390 cleanup when > >>>>>>>> he doesn't have any access to s390 guests or test cases... > >>>>>>>> Making S390 put COMMON_FEATURES in the right places seems > >>>>>>>> to me like a separate bit of s390-specific cleanup. > >>>>>>> > >>>>>>> Yep, see my other reply... I'm not quite sure what's wrong with > >>>>>>> event_idx on virtio-blk for s390-virtio, or I would gladly make this > >>>>>>> consistent with the other transports. Any hints appreciated :) > >>>>>> > >>>>>> Is this still happening? > >>>>>> > >>>>>> It is possible that what was missing was > >>>>>> 92045d80badc43c9f95897aad675dc7ef17a3b3f > >>>>>> and/or > >>>>>> a281ebc11a6917fbc27e1a93bb5772cd14e241fc > >>>>>> > >>>>> > >>>>> Found this: > >>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/280334/focus=280357 > >>>>> so it's unlikely: these commits are from 2012, you saw > >>>>> issues in 2014. > >>>>> > >>>>> We really need to fix it. virtio 1 work will be much easier if > >>>>> we can just move features into virtio dev. > >>> > >>> Yes, we have to understand why event_idx breaks for the s390-virtio > >>> transport. > >>>> > >>>> I'm beginning to suspect this is a wrong implementation of barriers. > >>>> Questions: > >>>> - which compiler to you use? > >>>> - can you pls disassemble code for smp_wmb smp_rmb and smp_mb? > >>>> They all must do br %r14 I think, and this is what > >>>> s390x-linux-gnu-gcc generated for me: > >>>> s390x-linux-gnu-gcc (GCC) 4.9.1 > >>> > >>> s390 has strong memory ordering. Reads are in order, writes are in order. > >>> bcr 14,0 or bcr 15,0 then only serialize the reads against the writes. > >>> So smp_rmb and smp_wmb can be implemented as no-ops like QEMU. > >>> If your change "fixes" the issue then we have a problem somewhere else > >> > >> And (surprise, surprise) virtio-blk now works - but it also works when > >> I back out the atomic.h change again. No barrier problems :) > >> > >> Good news is that we can change s390-virtio to be just like the other > >> transports. Although I'd like to understand why it was broken before. > >> Maybe a guest change? > > > > Or a compiler change? Try compiling some old release, see what happens. > > Anyway, let's move DEFINE_VIRTIO_COMMON_FEATURES into the base class > > now. Can you send a patch pls? > > 3.17 as guest fails, 3.18 as guest works. Not sure yet why. >
Fascinating. block core changes? bisect will tell. -- MST
