From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:33462)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kraxel@redhat.com>) id 1Z9a8J-0004Ch-Ku
	for qemu-devel@nongnu.org; Mon, 29 Jun 2015 10:34:04 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <kraxel@redhat.com>) id 1Z9a8E-0005DW-43
	for qemu-devel@nongnu.org; Mon, 29 Jun 2015 10:34:03 -0400
Received: from mx1.redhat.com ([209.132.183.28]:58573)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kraxel@redhat.com>) id 1Z9a8D-0005DK-V9
	for qemu-devel@nongnu.org; Mon, 29 Jun 2015 10:33:58 -0400
Message-ID: <1435588434.30958.43.camel@redhat.com>
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Mon, 29 Jun 2015 16:33:54 +0200
In-Reply-To: <20150629140001.GC30040@morn.localdomain>
References: <1435568020-8669-1-git-send-email-kraxel@redhat.com>
	<1435568020-8669-11-git-send-email-kraxel@redhat.com>
	<20150629140001.GC30040@morn.localdomain>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [SeaBIOS] [PATCH 10/18] virtio: add version 1.0
 support to vp_get_isr
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Kevin O'Connor <kevin@koconnor.net>
Cc: seabios@seabios.org, qemu-devel@nongnu.org, "Michael S. Tsirkin" <mst@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