qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Christian Borntraeger <borntraeger@de.ibm.com>
To: "Michael S. Tsirkin" <mst@redhat.com>,
	Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	hangaohuai@huawei.com,
	"Huangpeng (Peter)" <peter.huangpeng@huawei.com>,
	QEMU Developers <qemu-devel@nongnu.org>,
	Shannon Zhao <shannon.zhao@linaro.org>,
	Shannon Zhao <zhaoshenglong@huawei.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Christoffer Dall <christoffer.dall@linaro.org>
Subject: Re: [Qemu-devel] [PATCH v4 0/2] virtio: Move host features to backends
Date: Wed, 29 Apr 2015 10:17:55 +0200	[thread overview]
Message-ID: <554093B3.2080205@de.ibm.com> (raw)
In-Reply-To: <20150428203016-mutt-send-email-mst@redhat.com>

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.maydell@linaro.org> wrote:
>>>>
>>>>> On 28 April 2015 at 14:13, Michael S. Tsirkin <mst@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

  parent reply	other threads:[~2015-04-29  8:18 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-28 11:51 [Qemu-devel] [PATCH v4 0/2] virtio: Move host features to backends shannon.zhao
2015-04-28 11:51 ` [Qemu-devel] [PATCH v4 1/2] virtio-net: Move DEFINE_VIRTIO_NET_FEATURES to virtio-net shannon.zhao
2015-04-28 11:51 ` [Qemu-devel] [PATCH v4 2/2] virtio-scsi: Move DEFINE_VIRTIO_SCSI_FEATURES to virtio-scsi shannon.zhao
2015-04-28 12:48 ` [Qemu-devel] [PATCH v4 0/2] virtio: Move host features to backends Michael S. Tsirkin
2015-04-28 12:52   ` Peter Maydell
2015-04-28 13:05     ` Michael S. Tsirkin
2015-04-28 13:20       ` Cornelia Huck
2015-04-28 14:03         ` Michael S. Tsirkin
2015-04-28 13:06 ` Peter Maydell
2015-04-28 13:13   ` Michael S. Tsirkin
2015-04-28 13:16     ` Peter Maydell
2015-04-28 13:24       ` Cornelia Huck
2015-04-28 14:35         ` Michael S. Tsirkin
2015-04-28 18:14           ` Michael S. Tsirkin
2015-04-28 18:32             ` Michael S. Tsirkin
2015-04-28 18:35               ` Michael S. Tsirkin
2015-04-29  8:17               ` Christian Borntraeger [this message]
2015-04-29  8:52                 ` Cornelia Huck
2015-04-29 10:32                   ` Michael S. Tsirkin
2015-04-29 14:33                     ` Cornelia Huck
2015-04-29 18:32                       ` Michael S. Tsirkin
2015-04-29 14:43                     ` Christian Borntraeger
2015-04-29 18:35                       ` Michael S. Tsirkin
2015-04-29 19:49                         ` Christian Borntraeger
2015-04-29 20:19                           ` Michael S. Tsirkin
2015-04-30 15:50                             ` Christian Borntraeger
2015-04-28 18:34             ` Peter Maydell
2015-04-28 18:38               ` Michael S. Tsirkin
2015-04-29  9:55       ` Shannon Zhao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=554093B3.2080205@de.ibm.com \
    --to=borntraeger@de.ibm.com \
    --cc=christoffer.dall@linaro.org \
    --cc=cornelia.huck@de.ibm.com \
    --cc=hangaohuai@huawei.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.huangpeng@huawei.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=shannon.zhao@linaro.org \
    --cc=zhaoshenglong@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).