On Mon, Jun 29, 2015 at 10:53:32AM +0200, Gerd Hoffmann wrote: > Signed-off-by: Gerd Hoffmann <[email protected]> > --- > src/hw/virtio-pci.c | 12 ++++++++++++ > src/hw/virtio-pci.h | 6 +----- > 2 files changed, 13 insertions(+), 5 deletions(-) > > diff --git a/src/hw/virtio-pci.c b/src/hw/virtio-pci.c > index b414c71..481b365 100644 > --- a/src/hw/virtio-pci.c > +++ b/src/hw/virtio-pci.c > @@ -90,6 +90,18 @@ void vp_set_status(struct vp_device *vp, u8 status) > } > } > > +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. 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. -Kevin
