[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v1 2/4] virtio: make seg_max virtqueue size dependent
From: |
Michael S. Tsirkin |
Subject: |
Re: [PATCH v1 2/4] virtio: make seg_max virtqueue size dependent |
Date: |
Wed, 6 Nov 2019 06:54:12 -0500 |
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 <address@hidden>
> > 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
>
- Re: [PATCH v1 1/4] virtio: protect non-modern devices from too big virtqueue size setting, (continued)
[PATCH v1 2/4] virtio: make seg_max virtqueue size dependent, Denis Plotnikov, 2019/11/05
[PATCH v1 3/4] virtio: increase virtuqueue sizes in new machine types, Denis Plotnikov, 2019/11/05
[PATCH v1 4/4] iotests: add test for virtio-scsi and virtio-blk machine type settings, Denis Plotnikov, 2019/11/05