From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=56824 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PzWhE-0004ae-Cj for qemu-devel@nongnu.org; Tue, 15 Mar 2011 12:02:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PzWhD-0003AD-82 for qemu-devel@nongnu.org; Tue, 15 Mar 2011 12:02:08 -0400 Received: from mail-fx0-f45.google.com ([209.85.161.45]:64140) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PzWhD-00039s-3h for qemu-devel@nongnu.org; Tue, 15 Mar 2011 12:02:07 -0400 Received: by fxm2 with SMTP id 2so773387fxm.4 for ; Tue, 15 Mar 2011 09:02:06 -0700 (PDT) Date: Tue, 15 Mar 2011 16:33:59 +0100 From: "Edgar E. Iglesias" Subject: Re: [Qemu-devel] [PATCH 2/3] xilinx: Add AXIENET & DMA models Message-ID: <20110315153359.GA13934@edde.se.axis.com> References: <1300110364-21373-1-git-send-email-edgar.iglesias@petalogix.com> <1300110364-21373-3-git-send-email-edgar.iglesias@petalogix.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: Michal Simek , qemu-devel@nongnu.org On Mon, Mar 14, 2011 at 09:56:23PM +0200, Blue Swirl wrote: > On Mon, Mar 14, 2011 at 3:46 PM, wrote: > > From: Edgar E. Iglesias > > > > Signed-off-by: Edgar E. Iglesias > > --- > >  Makefile.target     |    2 + > >  hw/xilinx_axidma.c  |  463 +++++++++++++++++++++++++++ > >  hw/xilinx_axidma.h  |   40 +++ > >  hw/xilinx_axienet.c |  857 +++++++++++++++++++++++++++++++++++++++++++++++++++ > >  4 files changed, 1362 insertions(+), 0 deletions(-) > >  create mode 100644 hw/xilinx_axidma.c > >  create mode 100644 hw/xilinx_axidma.h > >  create mode 100644 hw/xilinx_axienet.c I'll be posting a v2 shortly that addresses most of your comments. I'll answer the remaining here. > > +static void stream_process_mem2s(struct axi_stream *s, struct xlx_dma_ch *dmach) > > +{ > > +    uint32_t prev_d; > > +    unsigned char txbuf[16 * 1024]; > > +    unsigned int txlen; > > +    uint32_t app[6]; > > + > > +    if (!stream_running(s) || stream_idle(s)) { > > +        return; > > +    } > > + > > +    while (1) { > > +        stream_desc_load(s, s->regs[R_CURDESC]); > > + > > +        if (s->desc.status & (1 << 31)) { > > +            s->regs[R_DMASR] |= 2; > > +            break; > > +        } > > + > > +        if (stream_desc_sof(&s->desc)) { > > +            s->pos = 0; > > +            memcpy(app, s->desc.app, sizeof app); > > +        } > > + > > +        txlen = s->desc.control & ((1 << 23) - 1); > > +        if ((txlen + s->pos) > sizeof txbuf) { > > +            hw_error("%s: too small internal txbuf! %d\n", __func__, > > +                     txlen + s->pos); > > +        } > > + > > +        cpu_physical_memory_read(s->desc.buffer_address, > > +                                 txbuf + s->pos, txlen); > > +        s->pos += txlen; > > + > > +        if (stream_desc_eof(&s->desc)) { > > +            xlx_dma_push_to_client(dmach, txbuf, s->pos, app); > > Maybe the client could instead read the data directly from memory > (zero copy) without a bounce buffer. Then the DMA API could be useful. I'll need to investigate this more. Are there any examples in the tree of scatter-gathering zero-copy DMA to network? For the moment, I'll leave this part for future work. > > +static SysBusDeviceInfo xilinx_enet_info = { > > +    .init = xilinx_enet_init, > > +    .qdev.name  = "xilinx,axienet", > > +    .qdev.size  = sizeof(struct xlx_axienet), > > +    .qdev.props = (Property[]) { > > +        DEFINE_PROP_UINT32("phyaddr", struct xlx_axienet, c_phyaddr, 7), > > I'm not sure what c_phyaddr does, can you use DEFINE_PROP_MACADDR? It's a way for boards to choose the PHY's adress on the MDIO bus. It's a 5 bit nr on MII. It might grow slightly later if we add support for other PHY connections, not sure. uint32 should be more than enough I think, uint16 probably too. Doesnt matter so much though... Thanks again for good review. Cheers