On Mon, Oct 08, 2018 at 01:24:51PM +0100, Peter Maydell wrote:
> On 3 October 2018 at 16:07, Edgar E. Iglesias <edgar.igles...@gmail.com> 
> wrote:
> > From: "Edgar E. Iglesias" <edgar.igles...@xilinx.com>
> >
> > Add support for selecting the Memory Region that the GEM
> > will do DMA to.
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
> > ---
> 
> 
> > @@ -1500,6 +1506,13 @@ static void gem_realize(DeviceState *dev, Error 
> > **errp)
> >      CadenceGEMState *s = CADENCE_GEM(dev);
> >      int i;
> >
> > +    if (s->dma_mr) {
> > +        s->dma_as = g_malloc0(sizeof(AddressSpace));
> > +        address_space_init(s->dma_as, s->dma_mr, NULL);
> 
> Why not just have the CadenceGEMState embed the AddressSpace
> 
>     AddressSpace dma_as;
> 
> rather than doing a separate memory allocation here?

No reason not to, I copied this from a pattern in our code and didn't reflect 
too much about the allocation.
I'll change it for next version.

Cheers,
Edgar


> 
> > +    } else {
> > +        s->dma_as = &address_space_memory;
> > +    }
> 
> thanks
> -- PMM

Reply via email to