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

[Kevin O'Connor]

On Mon, Jun 29, 2015 at 10:53:32AM +0200, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <address@hidden>
> ---
>  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