From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33546) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XHcL0-0005qB-OX for qemu-devel@nongnu.org; Wed, 13 Aug 2014 13:27:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XHcKu-0002F1-Ij for qemu-devel@nongnu.org; Wed, 13 Aug 2014 13:27:50 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48037) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XHcKu-0002D8-90 for qemu-devel@nongnu.org; Wed, 13 Aug 2014 13:27:44 -0400 Date: Wed, 13 Aug 2014 18:27:40 +0100 From: Stefan Hajnoczi Message-ID: <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> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="5dNcufZ4prhark0F" Content-Disposition: inline In-Reply-To: <1407843716-18301-8-git-send-email-marc.mari.barcelo@gmail.com> Subject: Re: [Qemu-devel] [PATCH v4 07/11] libqos: Added basic virtqueue support to virtio implementation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marc =?iso-8859-1?Q?Mar=ED?= Cc: Paolo Bonzini , qemu-devel@nongnu.org --5dNcufZ4prhark0F Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Aug 12, 2014 at 01:41:52PM +0200, Marc Mar=ED wrote: > + /* Check power of 2 */ > + aux =3D vq->size; > + while ((aux & 1) !=3D 0) { > + aux =3D aux >> 1; > + } > + g_assert_cmpint(aux, !=3D, 1); Power of 2 can be tested like this without a while loop: g_assert_cmpint(vq->size & (vq->size - 1), =3D=3D, 0) > +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->siz= e) > + +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); > + } Please use space between operators: writew(vq->desc + (16 * i) + 14, i + 1) This applies to all patches. > +void qvirtqueue_kick(const QVirtioBus *bus, QVirtioDevice *d, QVirtQueue= *vq, > + uint32_t fre= e_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); 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(): typedef struct { uint16_t foo; /* Forgot what these fields are called and didn't check */ uint16_t bar; uint16_t ring[]; } VirtioAvail; writel(&vq->avail.ring[idx % vq->size], free_head); Now it looks more like normal C and the compiler is doing the address calculations for us. --5dNcufZ4prhark0F Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJT66AMAAoJEJykq7OBq3PIdwcH/iVqhlBheDyMyo4aHhfUUPxO d7Pt3ZmqW/3ZIbXLX9dCKnDJ0eph6iZJs5hFP6VBjSa40vSQC03qN55yKmD9NN/S 7O+MjRZgs0+ScXzYTLePml1U3bGlLbgHi2MmB9R17sOtnnddt2PAQusGHDo0HyYU JTF2YKOCCANExGAZG+B3vUZ2u4VGpBrU2HuZN9Td7IZauaxTTJ3Ie1THTgHfqeGV mHySQASFWrGr+EX5NpsvpbLMT60BaRGlEC+8qPoWpaVUf1ql5/TTLzJzBllPM1fX 4J/MGhb14KZEu4I5aFq8aZVFnm1p+M/aUYdzafD1lFehY/03m+TOsJwUQPgWQSg= =r8mE -----END PGP SIGNATURE----- --5dNcufZ4prhark0F--