[SeaBIOS] Re: [PATCH v1 0/2] virtio: finalize features before using modern device

[Michael S. Tsirkin]

On Mon, Nov 14, 2022 at 11:58:16AM +0800, Xuan Zhuo wrote:
> https://www.mail-archive.com/[e-mail]/msg920776.html
> Then:
> $ build/qemu-system-x86_64 -M accel=kvm -m 1G -cpu host -blockdev
>   file,node-name=drive0,filename=test.img -device
>   virtio-blk-pci,drive=drive0
>   qemu: queue_enable is only suppported in devices of virtio 1.0 or later.
>
> In this PATCH, it was added to check the FEATURE VIRTIO_1 when enabling queue.
> This code should not be executed, but it is reported here. Because Seabios
> use the legacy sequence to initialize the modern device.

Acked-by: Michael S. Tsirkin

Gerd, is there a chance to have the fix included in this QEMU release?
It would be preferable since then we can keep the warning in qemu
(probably with some changes such as machine type and LOG_GUEST_ERROR).
> The driver MUST follow this sequence to initialize a device:
>     1. Reset the device.
>     2. Set the ACKNOWLEDGE status bit: the guest OS has noticed the device.
>     3. Set the DRIVER status bit: the guest OS knows how to drive the device.
> > Read device feature bits, and write the subset of feature bits understood by the OS and driver to the
>    device. During this step the driver MAY read (but MUST NOT write) the device-specific configuration
>    fields to check that it can support the device before accepting it.
> > Set the FEATURES_OK status bit. The driver MUST NOT accept new feature bits after this step.
> > Re-read device status to ensure the FEATURES_OK bit is still set: otherwise, the device does not
>    support our subset of features and the device is unusable.
> 7. Perform device-specific setup, including discovery of virtqueues for the device, optional per-bus setup,
>    reading and possibly writing the device’s virtio configuration space, and population of virtqueues.
> 8. Set the DRIVER_OK status bit. At this point the device is “live”.
>
> Test with:
> qemu-system-x86_64 \
>    -M microvm,x-option-roms=off,pit=off,pic=off,isa-serial=off,rtc=off \
>    -global virtio-mmio.force-legacy={false,true} \
>    -enable-kvm -cpu host -m 512m -smp 2 \
>    -nodefaults -no-user-config -nographic \
>    -drive id=test,file=./test.img,format=raw,if=none \
>    -device virtio-blk-device,drive=test \
>    -bios ../../seabios/out/bios.bin \
>    -chardev stdio,id=seabios -device isa-debugcon,iobase=0x402,chardev=seabios
>
> qemu-system-x86_64 \
>    -M microvm,x-option-roms=off,pit=off,pic=off,isa-serial=off,rtc=off \
>    -global virtio-mmio.force-legacy={false,true} \
>    -enable-kvm -cpu host -m 512m -smp 2 \
>    -nodefaults -no-user-config -nographic \
>    -drive id=test,file=./test.img,format=raw,if=none \
>    -device virtio-scsi-device \
>    -bios ../../seabios/out/bios.bin \
>    -chardev stdio,id=seabios -device isa-debugcon,iobase=0x402,chardev=seabios
>
> qemu-system-x86_64 \
>    -enable-kvm -cpu host -m 512m -smp 2 \
>    -nodefaults -no-user-config -nographic \
>    -drive id=test,file=./test.img,format=raw,if=none \
>    -device virtio-scsi,disable-legacy={off,on},disable-modern={on,off} \
>    -bios ../../seabios/out/bios.bin \
>    -chardev stdio,id=seabios -device isa-debugcon,iobase=0x402,chardev=seabios
>
> qemu-system-x86_64 \
>    -enable-kvm -cpu host -m 512m -smp 2 \
>    -nodefaults -no-user-config -nographic \
>    -drive id=test,file=./test.img,format=raw,if=none \
>    -device virtio-blk-pci,drive=test,disable-legacy={off,on},disable-modern={on,off} \
>    -bios ../../seabios/out/bios.bin \
>    -chardev stdio,id=seabios -device isa-debugcon,iobase=0x402,chardev=seabios
>
> Xuan Zhuo (2):
>   virtio-mmio: read/write the hi 32 features for mmio
>   virtio: finalize features before using device
>
> src/hw/virtio-blk.c  | 22 +++++++++++++---------
>  src/hw/virtio-pci.c  |  7 +++++--
>  src/hw/virtio-scsi.c | 13 +++++++++++++
>  3 files changed, 31 insertions(+), 11 deletions(-)