All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cornelia Huck <cohuck@redhat.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Halil Pasic <pasic@linux.ibm.com>,
	Jason Wang <jasowang@redhat.com>,
	Xie Yongji <xieyongji@bytedance.com>,
	virtualization@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org, markver@us.ibm.com,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	linux-s390@vger.kernel.org, qemu-devel@nongnu.org
Subject: Re: [RFC PATCH 1/1] virtio: write back features before verify
Date: Wed, 06 Oct 2021 12:13:14 +0200	[thread overview]
Message-ID: <87sfxezcjp.fsf@redhat.com> (raw)
In-Reply-To: <20211004151408-mutt-send-email-mst@kernel.org>

On Mon, Oct 04 2021, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Oct 04, 2021 at 05:50:44PM +0200, Cornelia Huck wrote:
>> On Mon, Oct 04 2021, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> 
>> > On Mon, Oct 04, 2021 at 04:33:21PM +0200, Cornelia Huck wrote:
>> >> On Mon, Oct 04 2021, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> >> 
>> >> > On Mon, Oct 04, 2021 at 02:19:55PM +0200, Cornelia Huck wrote:
>> >> >> 
>> >> >> [cc:qemu-devel]
>> >> >> 
>> >> >> On Sat, Oct 02 2021, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> >> >> 
>> >> >> > ok so that's a QEMU bug. Any virtio 1.0 and up
>> >> >> > compatible device must use LE.
>> >> >> > It can also present a legacy config space where the
>> >> >> > endian depends on the guest.
>> >> >> 
>> >> >> So, how is the virtio core supposed to determine this? A
>> >> >> transport-specific callback?
>> >> >
>> >> > I'd say a field in VirtIODevice is easiest.
>> >> 
>> >> The transport needs to set this as soon as it has figured out whether
>> >> we're using legacy or not.
>> >
>> > Basically on each device config access?
>> 
>> Prior to the first one, I think. It should not change again, should it?
>
> Well yes but we never prohibited someone from poking at both ..
> Doing it on each access means we don't have state to migrate.

Yes; if it isn't too high overhead, that's probably the safest way to
handle it.

>
>> >
>> >> I guess we also need to fence off any
>> >> accesses respectively error out the device if the driver tries any
>> >> read/write operations that would depend on that knowledge?
>> >> 
>> >> And using a field in VirtIODevice would probably need some care when
>> >> migrating. Hm...
>> >
>> > It's just a shorthand to minimize changes. No need to migrate I think.
>> 
>> If we migrate in from an older QEMU, we don't know whether we are
>> dealing with legacy or not, until feature negotiation is already
>> done... don't we have to ask the transport?
>
> Right but the only thing that can happen is config access.

Checking on each config space access would be enough then.

> Well and for legacy a kick I guess.

I think any driver that does something that is not config space access,
status access, or feature bit handling without VERSION_1 being set is
neccessarily legacy? Does that really need special handling?


WARNING: multiple messages have this Message-ID (diff)
From: Cornelia Huck <cohuck@redhat.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: linux-s390@vger.kernel.org, markver@us.ibm.com,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	qemu-devel@nongnu.org, linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org,
	Halil Pasic <pasic@linux.ibm.com>,
	Xie Yongji <xieyongji@bytedance.com>
Subject: Re: [RFC PATCH 1/1] virtio: write back features before verify
Date: Wed, 06 Oct 2021 12:13:14 +0200	[thread overview]
Message-ID: <87sfxezcjp.fsf@redhat.com> (raw)
In-Reply-To: <20211004151408-mutt-send-email-mst@kernel.org>

On Mon, Oct 04 2021, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Oct 04, 2021 at 05:50:44PM +0200, Cornelia Huck wrote:
>> On Mon, Oct 04 2021, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> 
>> > On Mon, Oct 04, 2021 at 04:33:21PM +0200, Cornelia Huck wrote:
>> >> On Mon, Oct 04 2021, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> >> 
>> >> > On Mon, Oct 04, 2021 at 02:19:55PM +0200, Cornelia Huck wrote:
>> >> >> 
>> >> >> [cc:qemu-devel]
>> >> >> 
>> >> >> On Sat, Oct 02 2021, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> >> >> 
>> >> >> > ok so that's a QEMU bug. Any virtio 1.0 and up
>> >> >> > compatible device must use LE.
>> >> >> > It can also present a legacy config space where the
>> >> >> > endian depends on the guest.
>> >> >> 
>> >> >> So, how is the virtio core supposed to determine this? A
>> >> >> transport-specific callback?
>> >> >
>> >> > I'd say a field in VirtIODevice is easiest.
>> >> 
>> >> The transport needs to set this as soon as it has figured out whether
>> >> we're using legacy or not.
>> >
>> > Basically on each device config access?
>> 
>> Prior to the first one, I think. It should not change again, should it?
>
> Well yes but we never prohibited someone from poking at both ..
> Doing it on each access means we don't have state to migrate.

Yes; if it isn't too high overhead, that's probably the safest way to
handle it.

>
>> >
>> >> I guess we also need to fence off any
>> >> accesses respectively error out the device if the driver tries any
>> >> read/write operations that would depend on that knowledge?
>> >> 
>> >> And using a field in VirtIODevice would probably need some care when
>> >> migrating. Hm...
>> >
>> > It's just a shorthand to minimize changes. No need to migrate I think.
>> 
>> If we migrate in from an older QEMU, we don't know whether we are
>> dealing with legacy or not, until feature negotiation is already
>> done... don't we have to ask the transport?
>
> Right but the only thing that can happen is config access.

Checking on each config space access would be enough then.

> Well and for legacy a kick I guess.

I think any driver that does something that is not config space access,
status access, or feature bit handling without VERSION_1 being set is
neccessarily legacy? Does that really need special handling?

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

WARNING: multiple messages have this Message-ID (diff)
From: Cornelia Huck <cohuck@redhat.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: linux-s390@vger.kernel.org, markver@us.ibm.com,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	qemu-devel@nongnu.org, Jason Wang <jasowang@redhat.com>,
	linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org,
	Halil Pasic <pasic@linux.ibm.com>,
	Xie Yongji <xieyongji@bytedance.com>
Subject: Re: [RFC PATCH 1/1] virtio: write back features before verify
Date: Wed, 06 Oct 2021 12:13:14 +0200	[thread overview]
Message-ID: <87sfxezcjp.fsf@redhat.com> (raw)
In-Reply-To: <20211004151408-mutt-send-email-mst@kernel.org>

On Mon, Oct 04 2021, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Oct 04, 2021 at 05:50:44PM +0200, Cornelia Huck wrote:
>> On Mon, Oct 04 2021, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> 
>> > On Mon, Oct 04, 2021 at 04:33:21PM +0200, Cornelia Huck wrote:
>> >> On Mon, Oct 04 2021, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> >> 
>> >> > On Mon, Oct 04, 2021 at 02:19:55PM +0200, Cornelia Huck wrote:
>> >> >> 
>> >> >> [cc:qemu-devel]
>> >> >> 
>> >> >> On Sat, Oct 02 2021, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> >> >> 
>> >> >> > ok so that's a QEMU bug. Any virtio 1.0 and up
>> >> >> > compatible device must use LE.
>> >> >> > It can also present a legacy config space where the
>> >> >> > endian depends on the guest.
>> >> >> 
>> >> >> So, how is the virtio core supposed to determine this? A
>> >> >> transport-specific callback?
>> >> >
>> >> > I'd say a field in VirtIODevice is easiest.
>> >> 
>> >> The transport needs to set this as soon as it has figured out whether
>> >> we're using legacy or not.
>> >
>> > Basically on each device config access?
>> 
>> Prior to the first one, I think. It should not change again, should it?
>
> Well yes but we never prohibited someone from poking at both ..
> Doing it on each access means we don't have state to migrate.

Yes; if it isn't too high overhead, that's probably the safest way to
handle it.

>
>> >
>> >> I guess we also need to fence off any
>> >> accesses respectively error out the device if the driver tries any
>> >> read/write operations that would depend on that knowledge?
>> >> 
>> >> And using a field in VirtIODevice would probably need some care when
>> >> migrating. Hm...
>> >
>> > It's just a shorthand to minimize changes. No need to migrate I think.
>> 
>> If we migrate in from an older QEMU, we don't know whether we are
>> dealing with legacy or not, until feature negotiation is already
>> done... don't we have to ask the transport?
>
> Right but the only thing that can happen is config access.

Checking on each config space access would be enough then.

> Well and for legacy a kick I guess.

I think any driver that does something that is not config space access,
status access, or feature bit handling without VERSION_1 being set is
neccessarily legacy? Does that really need special handling?



  reply	other threads:[~2021-10-06 10:13 UTC|newest]

Thread overview: 131+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-30  1:20 [RFC PATCH 1/1] virtio: write back features before verify Halil Pasic
2021-09-30  1:20 ` Halil Pasic
2021-09-30  8:04 ` Christian Borntraeger
2021-09-30  8:04   ` Christian Borntraeger
2021-09-30  9:28 ` Cornelia Huck
2021-09-30  9:28   ` Cornelia Huck
2021-09-30 11:03   ` Halil Pasic
2021-09-30 11:03     ` Halil Pasic
2021-09-30 11:31     ` Cornelia Huck
2021-09-30 11:31       ` Cornelia Huck
2021-10-01 14:22       ` Halil Pasic
2021-10-01 14:22         ` Halil Pasic
2021-10-01 15:18         ` Cornelia Huck
2021-10-01 15:18           ` Cornelia Huck
2021-10-02 18:13           ` Michael S. Tsirkin
2021-10-02 18:13             ` Michael S. Tsirkin
2021-10-04  2:23             ` Halil Pasic
2021-10-04  2:23               ` Halil Pasic
2021-10-04  9:07               ` Michael S. Tsirkin
2021-10-04  9:07                 ` Michael S. Tsirkin
2021-10-04  9:07                 ` Michael S. Tsirkin
2021-10-05 10:06                 ` Cornelia Huck
2021-10-05 10:06                   ` Cornelia Huck
2021-10-05 10:06                   ` Cornelia Huck
2021-10-05 10:43                 ` Halil Pasic
2021-10-05 10:43                   ` Halil Pasic
2021-10-05 10:43                   ` Halil Pasic
2021-10-05 11:11                   ` Michael S. Tsirkin
2021-10-05 11:11                     ` Michael S. Tsirkin
2021-10-05 11:11                     ` Michael S. Tsirkin
2021-10-05 11:13                   ` Cornelia Huck
2021-10-05 11:13                     ` Cornelia Huck
2021-10-05 11:13                     ` Cornelia Huck
2021-10-05 11:20                     ` Michael S. Tsirkin
2021-10-05 11:20                       ` Michael S. Tsirkin
2021-10-05 11:20                       ` Michael S. Tsirkin
2021-10-05 11:59                     ` Halil Pasic
2021-10-05 11:59                       ` Halil Pasic
2021-10-05 11:59                       ` Halil Pasic
2021-10-05 15:25                       ` Cornelia Huck
2021-10-05 15:25                         ` Cornelia Huck
2021-10-05 15:25                         ` Cornelia Huck
2021-10-04  7:01             ` Cornelia Huck
2021-10-04  7:01               ` Cornelia Huck
2021-10-04  9:25               ` Halil Pasic
2021-10-04  9:25                 ` Halil Pasic
2021-10-04  9:51                 ` Cornelia Huck
2021-10-04  9:51                   ` Cornelia Huck
2021-10-02 12:09       ` Michael S. Tsirkin
2021-10-02 12:09         ` Michael S. Tsirkin
2021-09-30 11:12 ` Michael S. Tsirkin
2021-09-30 11:12   ` Michael S. Tsirkin
2021-09-30 11:36   ` Cornelia Huck
2021-09-30 11:36     ` Cornelia Huck
2021-10-02 18:20     ` Michael S. Tsirkin
2021-10-02 18:20       ` Michael S. Tsirkin
2021-10-03  5:00       ` Halil Pasic
2021-10-03  5:00         ` Halil Pasic
2021-10-03  6:42         ` Michael S. Tsirkin
2021-10-03  6:42           ` Michael S. Tsirkin
2021-10-03  7:26           ` Michael S. Tsirkin
2021-10-03  7:26             ` Michael S. Tsirkin
2021-10-04 12:01             ` Cornelia Huck
2021-10-04 12:01               ` Cornelia Huck
2021-10-04 12:01               ` [virtio-dev] " Cornelia Huck
2021-10-04 12:54               ` Michael S. Tsirkin
2021-10-04 12:54                 ` Michael S. Tsirkin
2021-10-04 14:27                 ` Cornelia Huck
2021-10-04 14:27                   ` Cornelia Huck
2021-10-04 14:27                   ` [virtio-dev] " Cornelia Huck
2021-10-04 15:05                   ` Michael S. Tsirkin
2021-10-04 15:05                     ` Michael S. Tsirkin
2021-10-04 15:05                     ` [virtio-dev] " Michael S. Tsirkin
2021-10-04 15:45                     ` Cornelia Huck
2021-10-04 15:45                       ` Cornelia Huck
2021-10-04 15:45                       ` Cornelia Huck
2021-10-04 20:01                       ` Michael S. Tsirkin
2021-10-04 20:01                         ` Michael S. Tsirkin
2021-10-05  7:38                         ` Cornelia Huck
2021-10-05  7:38                           ` Cornelia Huck
2021-10-05  7:38                           ` Cornelia Huck
2021-10-05 11:17                         ` Halil Pasic
2021-10-05 11:17                           ` Halil Pasic
2021-10-05 11:22                           ` Michael S. Tsirkin
2021-10-05 11:22                             ` Michael S. Tsirkin
2021-10-05 15:20                             ` Cornelia Huck
2021-10-05 15:20                               ` Cornelia Huck
2021-10-05 15:20                               ` Cornelia Huck
2021-10-05 15:20                               ` Cornelia Huck
2021-10-01  7:21   ` Halil Pasic
2021-10-01  7:21     ` Halil Pasic
2021-10-02 10:21     ` Michael S. Tsirkin
2021-10-02 10:21       ` Michael S. Tsirkin
2021-10-04 12:19       ` Cornelia Huck
2021-10-04 12:19         ` Cornelia Huck
2021-10-04 12:19         ` Cornelia Huck
2021-10-04 13:11         ` Michael S. Tsirkin
2021-10-04 13:11           ` Michael S. Tsirkin
2021-10-04 13:11           ` Michael S. Tsirkin
2021-10-04 14:33           ` Cornelia Huck
2021-10-04 14:33             ` Cornelia Huck
2021-10-04 14:33             ` Cornelia Huck
2021-10-04 15:07             ` Michael S. Tsirkin
2021-10-04 15:07               ` Michael S. Tsirkin
2021-10-04 15:07               ` Michael S. Tsirkin
2021-10-04 15:50               ` Cornelia Huck
2021-10-04 15:50                 ` Cornelia Huck
2021-10-04 15:50                 ` Cornelia Huck
2021-10-04 19:17                 ` Michael S. Tsirkin
2021-10-04 19:17                   ` Michael S. Tsirkin
2021-10-04 19:17                   ` Michael S. Tsirkin
2021-10-06 10:13                   ` Cornelia Huck [this message]
2021-10-06 10:13                     ` Cornelia Huck
2021-10-06 10:13                     ` Cornelia Huck
2021-10-06 12:15                     ` Michael S. Tsirkin
2021-10-06 12:15                       ` Michael S. Tsirkin
2021-10-06 12:15                       ` Michael S. Tsirkin
2021-10-05  7:25           ` Halil Pasic
2021-10-05  7:25             ` Halil Pasic
2021-10-05  7:25             ` Halil Pasic
2021-10-05  7:53             ` Michael S. Tsirkin
2021-10-05  7:53               ` Michael S. Tsirkin
2021-10-05  7:53               ` Michael S. Tsirkin
2021-10-05 10:46               ` Halil Pasic
2021-10-05 10:46                 ` Halil Pasic
2021-10-05 10:46                 ` Halil Pasic
2021-10-05 11:11                 ` Michael S. Tsirkin
2021-10-05 11:11                   ` Michael S. Tsirkin
2021-10-05 11:11                   ` Michael S. Tsirkin
2021-10-01 14:34 ` Christian Borntraeger
2021-10-01 14:34   ` Christian Borntraeger

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=87sfxezcjp.fsf@redhat.com \
    --to=cohuck@redhat.com \
    --cc=borntraeger@de.ibm.com \
    --cc=jasowang@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=markver@us.ibm.com \
    --cc=mst@redhat.com \
    --cc=pasic@linux.ibm.com \
    --cc=qemu-devel@nongnu.org \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=xieyongji@bytedance.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.