Re: [PATCH v1 2/4] virtio: make seg_max virtqueue size dependent

[Denis Lunev <den@virtuozzo.com>]

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 <dplotnikov@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
- if queue length will be set to something < 128 - linux
  guest will crash

If we really need to preserve original __buggy__ behavior -
we can add boolean property, pls let us know.

Den

>
>> ---
>>  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