On 23.12.2019 17:31, Michael S. Tsirkin wrote:
> On Mon, Dec 23, 2019 at 02:37:58PM +0300, Denis Plotnikov wrote:
>> Currenly, the virtqueue size is saved to the proxy on pci writing and
>> is read from the device pci reading.
>> The virtqueue size is propagated later on form the proxy to the device
>> on virqueue enabling stage.
>>
>> This could be a problem, if a guest, on the virtqueue configuration, sets
>> the size and then re-read it immediatly before the queue enabling
>> in order to check if the desiged size has been set.
>>
>> This happens in seabios: (sebios snippet)
>>
>> vp_find_vq()
>> {
>> ...
>> /* check if the queue is available */
>> if (vp->use_modern) {
>> num = vp_read(&vp->common, virtio_pci_common_cfg, queue_size);
>> if (num > MAX_QUEUE_NUM) {
>> vp_write(&vp->common, virtio_pci_common_cfg, queue_size,
>> MAX_QUEUE_NUM);
>> num = vp_read(&vp->common, virtio_pci_common_cfg, queue_size);
>> }
>> } else {
>> num = vp_read(&vp->legacy, virtio_pci_legacy, queue_num);
>> }
>> if (!num) {
>> dprintf(1, "ERROR: queue size is 0\n");
>> goto fail;
>> }
>> if (num > MAX_QUEUE_NUM) {
>> dprintf(1, "ERROR: queue size %d > %d\n", num, MAX_QUEUE_NUM);
>> goto fail;
>> }
>> ...
>> }
>>
>> If the device queue num is greater then the max queue size supported by
>> seabios,
>> seabios tries to reduce the queue size, then re-read it again, I suppose to
>> check if the setting actually happens, and then checks the virtqueue size
>> again,
>> to deside whether it is satisfied with the vaule.
>> In this case, if device's virtqueue size is 512 and seabios max supported
>> queue
>> size is 256, seabios tries to set 256 but than read 512 again and can't
>> proceed
>> with that vaule, preventing the guest from successful booting.
>> The root case was investigated by Roman Kagan <rka...@virtuozzo.com>
>>
>> The patch fixes the problem, by propagating the queue size to the device
>> right
>> away, so the written value could be read on the next step, if the value was
>> ok for the device.
>>
>> Suggested-by: Roman Kagan <rka...@virtuozzo.com>
>> Suggested-by: Michael S. Tsirkin <m...@redhat.com>
>> Signed-off-by: Denis Plotnikov <dplotni...@virtuozzo.com>
> Thanks, I already have this queued as:
>
> commit 8aabbbd9d04f95d5581d2275362996ecb5516dd9
> Author: Michael S. Tsirkin <m...@redhat.com>
> Date: Fri Dec 13 09:22:48 2019 -0500
>
> virtio: update queue size on guest write
>
> Some guests read back queue size after writing it.
> Update the size immediatly upon write otherwise
> they get confused.
>
> Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
>
> I would appreciate checking other transports, they likely
> need the same fix.
ok, I'll send the patch shortly
>
>
>> ---
>> hw/virtio/virtio-pci.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
>> index c6b47a9c73..e5c759e19e 100644
>> --- a/hw/virtio/virtio-pci.c
>> +++ b/hw/virtio/virtio-pci.c
>> @@ -1256,6 +1256,8 @@ static void virtio_pci_common_write(void *opaque,
>> hwaddr addr,
>> break;
>> case VIRTIO_PCI_COMMON_Q_SIZE:
>> proxy->vqs[vdev->queue_sel].num = val;
>> + virtio_queue_set_num(vdev, vdev->queue_sel,
>> + proxy->vqs[vdev->queue_sel].num);
>> break;
>> case VIRTIO_PCI_COMMON_Q_MSIX:
>> msix_vector_unuse(&proxy->pci_dev,
>> --
>> 2.17.0