[SeaBIOS] Re: [PATCH v1 1/2] virtio-mmio: read/write the hi 32 features for mmio

[Xuan Zhuo]

On Mon, 14 Nov 2022 10:36:52 +0100, Gerd Hoffmann  wrote:
> On Mon, Nov 14, 2022 at 11:58:17AM +0800, Xuan Zhuo wrote:
> > Under mmio, when we read the feature from the device, we should read the
> > high 32-bit part. Similarly, when writing the feature back, we should
> > also write back the high 32-bit feature.
> > Signed-off-by: Xuan Zhuo
> >
> > src/hw/virtio-pci.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/hw/virtio-pci.c b/src/hw/virtio-pci.c
> > index 213c497..89a4f50 100644
> > --- a/src/hw/virtio-pci.c
> > +++ b/src/hw/virtio-pci.c
> > @@ -193,7 +193,8 @@ u64 vp_get_features(struct vp_device *vp)
> >      if (vp->use_mmio) {
> >          vp_write(&vp->common, virtio_mmio_cfg, device_feature_select, 0);
> >          f0 = vp_read(&vp->common, virtio_mmio_cfg, device_feature);
> >    f1 = 0;
> >    vp_write(&vp->common, virtio_mmio_cfg, device_feature_select, 1);
> >    f1 = vp_read(&vp->common, virtio_mmio_cfg, device_feature);
>
> You need to check the version here, legacy virtio has only 32 feature
> bits so you can't read/write f1 like this.
>    if (vp_read(&vp->common, virtio_mmio_cfg, version) == 2) {
>        /* virtio 1.0 */
>    } else {
>        /* legacy */
>    }

I refer to the implementation of the Linux kernel and confirmed that QEMU's MMIO
Device implementation. Legacy should also support this operation, although it
may return 0. 0.

Hi Michael, do I make mistakes?

Thanks.
> (see also vp_find_vq()).
>
> take care,
>   Gerd