On Tue, Aug 12, 2014 at 01:41:52PM +0200, Marc Marí wrote: > + /* Check power of 2 */ > + aux = vq->size; > + while ((aux & 1) != 0) { > + aux = aux >> 1; > + } > + g_assert_cmpint(aux, !=, 1);
Power of 2 can be tested like this without a while loop: g_assert_cmpint(vq->size & (vq->size - 1), ==, 0) > +void qvring_init(const QGuestAllocator *alloc, QVirtQueue *vq, uint64_t addr) > +{ > + int i; > + > + vq->desc = addr; > + vq->avail = vq->desc + vq->size*sizeof(QVRingDesc); > + vq->used = (uint64_t)((vq->avail + sizeof(uint16_t) * (3 + vq->size) > + +vq->align-1) & ~(vq->align - 1)); > + > + for (i = 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 > free_head) > +{ > + /* vq->avail->idx */ > + uint16_t idx = 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.
pgpmP7XXqhrFi.pgp
Description: PGP signature