On 6 October 2018 at 00:14, Philippe Mathieu-Daudé <phi...@redhat.com> wrote: > Hi Edgar, > > On 03/10/2018 17:07, Edgar E. Iglesias wrote: >> From: "Edgar E. Iglesias" <edgar.igles...@xilinx.com> >> >> Add support for selecting the Memory Region that the GEM >> will do DMA to.
>> @@ -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); >> + } else { >> + s->dma_as = &address_space_memory; >> + } > > This is not the first time I see this if() block. > > Should we consider refactor it? Given that there are only three users of the device in the tree, I think the nicest cleanup would be to make those callers pass in a suitable MemoryRegion (which could just be the system memory MR), and make the "dma" property of the device mandatory to set. We can do that as followup, though. thanks -- PMM