Halil Pasic
2021-Oct-05 07:25 UTC
[RFC PATCH 1/1] virtio: write back features before verify
On Mon, 4 Oct 2021 09:11:04 -0400 "Michael S. Tsirkin" <mst at redhat.com> wrote:> > >> static inline bool virtio_access_is_big_endian(VirtIODevice *vdev) > > >> { > > >> #if defined(LEGACY_VIRTIO_IS_BIENDIAN) > > >> return virtio_is_big_endian(vdev); > > >> #elif defined(TARGET_WORDS_BIGENDIAN) > > >> if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) { > > >> /* Devices conforming to VIRTIO 1.0 or later are always LE. */ > > >> return false; > > >> } > > >> return true; > > >> #else > > >> return false; > > >> #endif > > >> } > > >> > > > > > > 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.Wouldn't a call from transport code into virtio core be more handy? What I have in mind is stuff like vhost-user and vdpa. My understanding is, that for vhost setups where the config is outside qemu, we probably need a new command that tells the vhost backend what endiannes to use for config. I don't think we can use VHOST_USER_SET_VRING_ENDIAN because that one is on a virtqueue basis according to the doc. So for vhost-user and similar we would fire that command and probably also set the filed, while for devices for which control plane is handled by QEMU we would just set the field. Does that sound about right?
Michael S. Tsirkin
2021-Oct-05 07:53 UTC
[RFC PATCH 1/1] virtio: write back features before verify
On Tue, Oct 05, 2021 at 09:25:39AM +0200, Halil Pasic wrote:> On Mon, 4 Oct 2021 09:11:04 -0400 > "Michael S. Tsirkin" <mst at redhat.com> wrote: > > > > >> static inline bool virtio_access_is_big_endian(VirtIODevice *vdev) > > > >> { > > > >> #if defined(LEGACY_VIRTIO_IS_BIENDIAN) > > > >> return virtio_is_big_endian(vdev); > > > >> #elif defined(TARGET_WORDS_BIGENDIAN) > > > >> if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) { > > > >> /* Devices conforming to VIRTIO 1.0 or later are always LE. */ > > > >> return false; > > > >> } > > > >> return true; > > > >> #else > > > >> return false; > > > >> #endif > > > >> } > > > >> > > > > > > > > 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. > > Wouldn't a call from transport code into virtio core > be more handy? What I have in mind is stuff like vhost-user and vdpa. My > understanding is, that for vhost setups where the config is outside qemu, > we probably need a new command that tells the vhost backend what > endiannes to use for config. I don't think we can use > VHOST_USER_SET_VRING_ENDIAN because that one is on a virtqueue basis > according to the doc. So for vhost-user and similar we would fire that > command and probably also set the filed, while for devices for which > control plane is handled by QEMU we would just set the field. > > Does that sound about right?I'm fine either way, but when would you invoke this? With my idea backends can check the field when get_config is invoked. As for using this in VHOST, can we maybe re-use SET_FEATURES? Kind of hacky but nice in that it will actually make existing backends work... -- MST