Re: [Qemu-devel] [SeaBIOS] [PATCH 10/18] virtio: add version 1.0 support to vp_get_isr

[Gerd Hoffmann <kraxel@redhat.com>]

  Hi,

> > +u8 vp_get_isr(struct vp_device *vp)
> > +{
> > +    u8 isr;
> > +
> > +    if (vp->use_modern) {
> > +        vp_modern_read(vp->isr, virtio_pci_isr, isr, isr);
> > +    } else {
> > +        isr = inb(vp->ioaddr + VIRTIO_PCI_ISR);
> > +    }
> > +    return isr;
> > +}
> 
> How about renaming "use_modern" to something more descriptive - like
> "use_abi1"?  Also, couldn't vp_modern_read just be renamed to vp_read.

In both qemu and linux kernel "legacy" and "modern" are used for the two
interface revisions (0.9.5 and 1.0), and I'd prefer to stay consistent
with that.

> BTW, out of curiosity, did you consider retroactively making ABIv0
> structs and using vp_read() for both the new and old cases?  I'm not
> sure it would save any code, but mixing the struct/offset/sizeof
> method with the inb/define method seems a little awkward.

Makes sense to do that in indeed (and of course naming it vp_modern_read
looks odd then ;)

I'll look into that for v2.

cheers,
  Gerd