On Wed, Nov 06, 2019 at 10:07:02AM +0000, Denis Lunev wrote: > On 11/5/19 9:51 PM, Michael S. Tsirkin wrote: > > On Tue, Nov 05, 2019 at 07:11:03PM +0300, Denis Plotnikov wrote: > >> seg_max has a restriction to be less or equal to virtqueue size > >> according to Virtio 1.0 specification > >> > >> Although seg_max can't be set directly, it's worth to express this > >> dependancy directly in the code for sanity purpose. > >> > >> Signed-off-by: Denis Plotnikov <dplotni...@virtuozzo.com> > > This is guest visible so needs to be machine type dependent, right? > > we have discussed this verbally with Stefan and think that > there is no need to add that to the machine type as: > > - original default was 126, which matches 128 as queue > length in old machine types > - queue length > 128 is not observed in the field as > SeaBios has quirk that asserts
Well that's just the SeaBios virtio driver. Not everyone's using that to drive their devices. > - if queue length will be set to something < 128 - linux > guest will crash Again that's just one guest driver. Not everyone is using that either. > If we really need to preserve original __buggy__ behavior - > we can add boolean property, pls let us know. > > Den Looks like some drivers are buggy but I'm not sure it's the same as saying the behavior is buggy. So yes, I'd say it's preferable to be compatible. > >> --- > >> hw/block/virtio-blk.c | 2 +- > >> hw/scsi/virtio-scsi.c | 2 +- > >> 2 files changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > >> index 06e57a4d39..21530304cf 100644 > >> --- a/hw/block/virtio-blk.c > >> +++ b/hw/block/virtio-blk.c > >> @@ -903,7 +903,7 @@ static void virtio_blk_update_config(VirtIODevice > >> *vdev, uint8_t *config) > >> blk_get_geometry(s->blk, &capacity); > >> memset(&blkcfg, 0, sizeof(blkcfg)); > >> virtio_stq_p(vdev, &blkcfg.capacity, capacity); > >> - virtio_stl_p(vdev, &blkcfg.seg_max, 128 - 2); > >> + virtio_stl_p(vdev, &blkcfg.seg_max, s->conf.queue_size - 2); > >> virtio_stw_p(vdev, &blkcfg.geometry.cylinders, conf->cyls); > >> virtio_stl_p(vdev, &blkcfg.blk_size, blk_size); > >> virtio_stw_p(vdev, &blkcfg.min_io_size, conf->min_io_size / blk_size); > >> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c > >> index 839f120256..f7e5533cd5 100644 > >> --- a/hw/scsi/virtio-scsi.c > >> +++ b/hw/scsi/virtio-scsi.c > >> @@ -650,7 +650,7 @@ static void virtio_scsi_get_config(VirtIODevice *vdev, > >> VirtIOSCSICommon *s = VIRTIO_SCSI_COMMON(vdev); > >> > >> virtio_stl_p(vdev, &scsiconf->num_queues, s->conf.num_queues); > >> - virtio_stl_p(vdev, &scsiconf->seg_max, 128 - 2); > >> + virtio_stl_p(vdev, &scsiconf->seg_max, s->conf.virtqueue_size - 2); > >> virtio_stl_p(vdev, &scsiconf->max_sectors, s->conf.max_sectors); > >> virtio_stl_p(vdev, &scsiconf->cmd_per_lun, s->conf.cmd_per_lun); > >> virtio_stl_p(vdev, &scsiconf->event_info_size, > >> sizeof(VirtIOSCSIEvent)); > >> -- > >> 2.17.0 >
