From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:37984)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <marc.mari.barcelo@gmail.com>) id 1XHgaK-0007W4-6a
	for qemu-devel@nongnu.org; Wed, 13 Aug 2014 18:00:05 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <marc.mari.barcelo@gmail.com>) id 1XHgaB-0001Nm-5K
	for qemu-devel@nongnu.org; Wed, 13 Aug 2014 17:59:56 -0400
Received: from mail-wi0-x22a.google.com ([2a00:1450:400c:c05::22a]:42141)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <marc.mari.barcelo@gmail.com>) id 1XHgaA-0001NZ-Vn
	for qemu-devel@nongnu.org; Wed, 13 Aug 2014 17:59:47 -0400
Received: by mail-wi0-f170.google.com with SMTP id f8so9002683wiw.1
	for <qemu-devel@nongnu.org>; Wed, 13 Aug 2014 14:59:45 -0700 (PDT)
Date: Wed, 13 Aug 2014 23:59:39 +0200
From: Marc =?UTF-8?B?TWFyw60=?= <marc.mari.barcelo@gmail.com>
Message-ID: <20140813235939.41a28781@crunchbang>
In-Reply-To: <20140813172740.GK27053@stefanha-thinkpad.redhat.com>
References: <1407843716-18301-1-git-send-email-marc.mari.barcelo@gmail.com>
	<1407843716-18301-8-git-send-email-marc.mari.barcelo@gmail.com>
	<20140813172740.GK27053@stefanha-thinkpad.redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [Qemu-devel] [PATCH v4 07/11] libqos: Added basic virtqueue
 support to virtio implementation
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: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>, qemu-devel@nongnu.org

El Wed, 13 Aug 2014 18:27:40 +0100
Stefan Hajnoczi <stefanha@redhat.com> escribi=C3=B3:
> > +void qvring_init(const QGuestAllocator *alloc, QVirtQueue *vq,
> > uint64_t addr) +{
> > +    int i;
> > +
> > +    vq->desc =3D addr;
> > +    vq->avail =3D vq->desc + vq->size*sizeof(QVRingDesc);
> > +    vq->used =3D (uint64_t)((vq->avail + sizeof(uint16_t) * (3 +
> > vq->size)
> > +        +vq->align-1) & ~(vq->align - 1));
> > +
> > +    for (i =3D 0; i < vq->size-1; i++) {
> > +        /* vq->desc[i].addr */
> > +        writew(vq->desc+(16*i), 0);
> > +        /* vq->desc[i].next */
> > +        writew(vq->desc+(16*i)+14, i+1);
> > +    }
>=20
> Please use space between operators:
> writew(vq->desc + (16 * i) + 14, i + 1)
>=20
> This applies to all patches.
>=20
> > +void qvirtqueue_kick(const QVirtioBus *bus, QVirtioDevice *d,
> > QVirtQueue *vq,
> > +
> > uint32_t free_head) +{
> > +    /* vq->avail->idx */
> > +    uint16_t idx =3D readl(vq->avail+2);
> > +
> > +    /* vq->avail->ring[idx % vq->size] */
> > +    writel(vq->avail+4+(2*(idx % vq->size)), free_head);
>=20
> If you want to use typed pointers you can.  It will make the code
> nicer to read, just be careful never to dereference them and only to
> access through writel()/readl():
>=20
> typedef struct {
> uint16_t foo; /* Forgot what these fields are called and didn't check
> */ uint16_t bar;
> uint16_t ring[];
> } VirtioAvail;
>=20
> writel(&vq->avail.ring[idx % vq->size], free_head);
>=20
> Now it looks more like normal C and the compiler is doing the address
> calculations for us.

If it is not dereferenced, it can work. But all references will have to
be casted from (void *) to (uint64_t) always which is what readl and
writel expect. I think this is better than calculating the addresses,
but is still a bit ugly.