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

[Gerd Hoffmann]

On Mon, Nov 14, 2022 at 05:51:28AM -0500, Michael S. Tsirkin wrote:
> On Mon, Nov 14, 2022 at 10:36:52AM +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 */
> >    }
> >
> > (see also vp_find_vq()).
> >
> > take care,
> >   Gerd
>
> Hmm we won't want to re-read it many times I think.

It's not in any hot path.  It's needed only at initialization time (for
features and queue setup) because the differences between legacy and
modern virtio-mmio are quite small.  So IMHO not required.

take care,
  Gerd