| Message ID | 20220623071131.412457-2-thuth@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | pc-bios/s390-ccw: Two fixes for the virtio initialization | expand |
Am 23.06.22 um 09:11 schrieb Thomas Huth: > According chapter "3.1.1 Driver Requirements: Device Initialization" > of the Virtio specification (v1.1), a driver for a device has to set > the ACKNOWLEDGE and DRIVER bits in the status field after resetting > the device. The s390-ccw bios skipped these steps so far and seems > like QEMU never cared. Anyway, it's better to follow the spec, so > let's set these bits now in the right spots, too. I have not tested that but I agree with this Acked-by: Christian Borntraeger <borntraeger@linux.ibm.com> > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > pc-bios/s390-ccw/virtio.c | 18 ++++++++++++++---- > 1 file changed, 14 insertions(+), 4 deletions(-) > > diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c > index 5d2c6e3381..4e85a2eb82 100644 > --- a/pc-bios/s390-ccw/virtio.c > +++ b/pc-bios/s390-ccw/virtio.c > @@ -220,7 +220,7 @@ int virtio_run(VDev *vdev, int vqid, VirtioCmd *cmd) > void virtio_setup_ccw(VDev *vdev) > { > int i, rc, cfg_size = 0; > - unsigned char status = VIRTIO_CONFIG_S_DRIVER_OK; > + uint8_t status; > struct VirtioFeatureDesc { > uint32_t features; > uint8_t index; > @@ -234,6 +234,10 @@ void virtio_setup_ccw(VDev *vdev) > > run_ccw(vdev, CCW_CMD_VDEV_RESET, NULL, 0, false); > > + status = VIRTIO_CONFIG_S_ACKNOWLEDGE; > + rc = run_ccw(vdev, CCW_CMD_WRITE_STATUS, &status, sizeof(status), false); > + IPL_assert(rc == 0, "Could not write ACKNOWLEDGE status to host"); > + > switch (vdev->senseid.cu_model) { > case VIRTIO_ID_NET: > vdev->nr_vqs = 2; > @@ -253,6 +257,11 @@ void virtio_setup_ccw(VDev *vdev) > default: > panic("Unsupported virtio device\n"); > } > + > + status |= VIRTIO_CONFIG_S_DRIVER; > + rc = run_ccw(vdev, CCW_CMD_WRITE_STATUS, &status, sizeof(status), false); > + IPL_assert(rc == 0, "Could not write DRIVER status to host"); > + > IPL_assert( > run_ccw(vdev, CCW_CMD_READ_CONF, &vdev->config, cfg_size, false) == 0, > "Could not get block device configuration"); > @@ -291,9 +300,10 @@ void virtio_setup_ccw(VDev *vdev) > run_ccw(vdev, CCW_CMD_SET_VQ, &info, sizeof(info), false) == 0, > "Cannot set VQ info"); > } > - IPL_assert( > - run_ccw(vdev, CCW_CMD_WRITE_STATUS, &status, sizeof(status), false) == 0, > - "Could not write status to host"); > + > + status |= VIRTIO_CONFIG_S_DRIVER_OK; > + rc = run_ccw(vdev, CCW_CMD_WRITE_STATUS, &status, sizeof(status), false); > + IPL_assert(rc == 0, "Could not write DRIVER_OK status to host"); > } > > bool virtio_is_supported(SubChannelId schid)
On Thu, Jun 23 2022, Thomas Huth <thuth@redhat.com> wrote: > According chapter "3.1.1 Driver Requirements: Device Initialization" > of the Virtio specification (v1.1), a driver for a device has to set > the ACKNOWLEDGE and DRIVER bits in the status field after resetting > the device. The s390-ccw bios skipped these steps so far and seems > like QEMU never cared. Anyway, it's better to follow the spec, so > let's set these bits now in the right spots, too. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > pc-bios/s390-ccw/virtio.c | 18 ++++++++++++++---- > 1 file changed, 14 insertions(+), 4 deletions(-) The QEMU implementation seems to be pretty tolerant :) Reviewed-by: Cornelia Huck <cohuck@redhat.com>
On Thu, 2022-06-23 at 09:11 +0200, Thomas Huth wrote: > According chapter "3.1.1 Driver Requirements: Device Initialization" > of the Virtio specification (v1.1), a driver for a device has to set > the ACKNOWLEDGE and DRIVER bits in the status field after resetting > the device. The s390-ccw bios skipped these steps so far and seems > like QEMU never cared. Anyway, it's better to follow the spec, so > let's set these bits now in the right spots, too. > > Signed-off-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Eric Farman <farman@linux.ibm.com> > --- > pc-bios/s390-ccw/virtio.c | 18 ++++++++++++++---- > 1 file changed, 14 insertions(+), 4 deletions(-) > > diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c > index 5d2c6e3381..4e85a2eb82 100644 > --- a/pc-bios/s390-ccw/virtio.c > +++ b/pc-bios/s390-ccw/virtio.c > @@ -220,7 +220,7 @@ int virtio_run(VDev *vdev, int vqid, VirtioCmd > *cmd) > void virtio_setup_ccw(VDev *vdev) > { > int i, rc, cfg_size = 0; > - unsigned char status = VIRTIO_CONFIG_S_DRIVER_OK; > + uint8_t status; > struct VirtioFeatureDesc { > uint32_t features; > uint8_t index; > @@ -234,6 +234,10 @@ void virtio_setup_ccw(VDev *vdev) > > run_ccw(vdev, CCW_CMD_VDEV_RESET, NULL, 0, false); > > + status = VIRTIO_CONFIG_S_ACKNOWLEDGE; > + rc = run_ccw(vdev, CCW_CMD_WRITE_STATUS, &status, > sizeof(status), false); > + IPL_assert(rc == 0, "Could not write ACKNOWLEDGE status to > host"); > + > switch (vdev->senseid.cu_model) { > case VIRTIO_ID_NET: > vdev->nr_vqs = 2; > @@ -253,6 +257,11 @@ void virtio_setup_ccw(VDev *vdev) > default: > panic("Unsupported virtio device\n"); > } > + > + status |= VIRTIO_CONFIG_S_DRIVER; > + rc = run_ccw(vdev, CCW_CMD_WRITE_STATUS, &status, > sizeof(status), false); > + IPL_assert(rc == 0, "Could not write DRIVER status to host"); > + > IPL_assert( > run_ccw(vdev, CCW_CMD_READ_CONF, &vdev->config, cfg_size, > false) == 0, > "Could not get block device configuration"); > @@ -291,9 +300,10 @@ void virtio_setup_ccw(VDev *vdev) > run_ccw(vdev, CCW_CMD_SET_VQ, &info, sizeof(info), > false) == 0, > "Cannot set VQ info"); > } > - IPL_assert( > - run_ccw(vdev, CCW_CMD_WRITE_STATUS, &status, sizeof(status), > false) == 0, > - "Could not write status to host"); > + > + status |= VIRTIO_CONFIG_S_DRIVER_OK; > + rc = run_ccw(vdev, CCW_CMD_WRITE_STATUS, &status, > sizeof(status), false); > + IPL_assert(rc == 0, "Could not write DRIVER_OK status to host"); > } > > bool virtio_is_supported(SubChannelId schid)
diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c index 5d2c6e3381..4e85a2eb82 100644 --- a/pc-bios/s390-ccw/virtio.c +++ b/pc-bios/s390-ccw/virtio.c @@ -220,7 +220,7 @@ int virtio_run(VDev *vdev, int vqid, VirtioCmd *cmd) void virtio_setup_ccw(VDev *vdev) { int i, rc, cfg_size = 0; - unsigned char status = VIRTIO_CONFIG_S_DRIVER_OK; + uint8_t status; struct VirtioFeatureDesc { uint32_t features; uint8_t index; @@ -234,6 +234,10 @@ void virtio_setup_ccw(VDev *vdev) run_ccw(vdev, CCW_CMD_VDEV_RESET, NULL, 0, false); + status = VIRTIO_CONFIG_S_ACKNOWLEDGE; + rc = run_ccw(vdev, CCW_CMD_WRITE_STATUS, &status, sizeof(status), false); + IPL_assert(rc == 0, "Could not write ACKNOWLEDGE status to host"); + switch (vdev->senseid.cu_model) { case VIRTIO_ID_NET: vdev->nr_vqs = 2; @@ -253,6 +257,11 @@ void virtio_setup_ccw(VDev *vdev) default: panic("Unsupported virtio device\n"); } + + status |= VIRTIO_CONFIG_S_DRIVER; + rc = run_ccw(vdev, CCW_CMD_WRITE_STATUS, &status, sizeof(status), false); + IPL_assert(rc == 0, "Could not write DRIVER status to host"); + IPL_assert( run_ccw(vdev, CCW_CMD_READ_CONF, &vdev->config, cfg_size, false) == 0, "Could not get block device configuration"); @@ -291,9 +300,10 @@ void virtio_setup_ccw(VDev *vdev) run_ccw(vdev, CCW_CMD_SET_VQ, &info, sizeof(info), false) == 0, "Cannot set VQ info"); } - IPL_assert( - run_ccw(vdev, CCW_CMD_WRITE_STATUS, &status, sizeof(status), false) == 0, - "Could not write status to host"); + + status |= VIRTIO_CONFIG_S_DRIVER_OK; + rc = run_ccw(vdev, CCW_CMD_WRITE_STATUS, &status, sizeof(status), false); + IPL_assert(rc == 0, "Could not write DRIVER_OK status to host"); } bool virtio_is_supported(SubChannelId schid)
According chapter "3.1.1 Driver Requirements: Device Initialization" of the Virtio specification (v1.1), a driver for a device has to set the ACKNOWLEDGE and DRIVER bits in the status field after resetting the device. The s390-ccw bios skipped these steps so far and seems like QEMU never cared. Anyway, it's better to follow the spec, so let's set these bits now in the right spots, too. Signed-off-by: Thomas Huth <thuth@redhat.com> --- pc-bios/s390-ccw/virtio.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)