On Wed, Aug 08, 2018 at 11:11:11PM +0100, Lionel Landwerlin wrote: > On 08/08/18 20:07, Rafael Antognolli wrote: > > On Tue, Aug 07, 2018 at 06:35:18PM +0100, Lionel Landwerlin wrote: > > > When we map a PPGTT buffer into a continous address space of aubinator > > > to be able to inspect it, we currently add it to the list of BOs to > > > unmap once we're finished. An optimization we can apply it to look up > > > that list before trying to remap PPGTT buffers again (we already do > > > this for GGTT buffers). > > > > > > We need to take some care before doing this because the list also > > > contains GGTT BOs. As GGTT & PPGTT are 2 different address spaces, we > > > can have matching addresses in both that point to different physical > > > locations. > > So, before this change, we could have the same address for PPGTT and > > GGTT on the map list, but they never clashed because we only added the > > PPGTT ones at the end, and then unmapped them? Or was there something > > else preventing them from conflicting? > > Before this change we could get clashes when asking for a GGTT address and > get a PPGTT one. > I think we got lucky so far because we use a very small amount of GGTT and > that didn't happen.
You explained it, I understood and moved on, and forgot about it :-/ But this is: Reviewed-by: Rafael Antognolli <rafael.antogno...@intel.com> > > > > This changes adds a flag on the elements of the list of mapped BOs to > > > differenciate between GGTT & PPGTT, which allows use to reuse that > > > list when looking up both address spaces. > > > > > > Signed-off-by: Lionel Landwerlin <lionel.g.landwer...@intel.com> > > > --- > > > src/intel/tools/aub_mem.c | 16 +++++++++++----- > > > 1 file changed, 11 insertions(+), 5 deletions(-) > > > > > > diff --git a/src/intel/tools/aub_mem.c b/src/intel/tools/aub_mem.c > > > index 2d29386e57c..3d4dc8061bd 100644 > > > --- a/src/intel/tools/aub_mem.c > > > +++ b/src/intel/tools/aub_mem.c > > > @@ -42,6 +42,7 @@ struct bo_map { > > > struct list_head link; > > > struct gen_batch_decode_bo bo; > > > bool unmap_after_use; > > > + bool ppgtt; > > > }; > > > struct ggtt_entry { > > > @@ -59,10 +60,11 @@ struct phys_mem { > > > }; > > > static void > > > -add_gtt_bo_map(struct aub_mem *mem, struct gen_batch_decode_bo bo, bool > > > unmap_after_use) > > > +add_gtt_bo_map(struct aub_mem *mem, struct gen_batch_decode_bo bo, bool > > > ppgtt, bool unmap_after_use) > > > { > > > struct bo_map *m = calloc(1, sizeof(*m)); > > > + m->ppgtt = ppgtt; > > > m->bo = bo; > > > m->unmap_after_use = unmap_after_use; > > > list_add(&m->link, &mem->maps); > > > @@ -190,7 +192,7 @@ aub_mem_local_write(void *_mem, uint64_t address, > > > .addr = address, > > > .size = size, > > > }; > > > - add_gtt_bo_map(mem, bo, false); > > > + add_gtt_bo_map(mem, bo, false, false); > > > } > > > void > > > @@ -253,7 +255,7 @@ aub_mem_get_ggtt_bo(void *_mem, uint64_t address) > > > struct gen_batch_decode_bo bo = {0}; > > > list_for_each_entry(struct bo_map, i, &mem->maps, link) > > > - if (i->bo.addr <= address && i->bo.addr + i->bo.size > address) > > > + if (!i->ppgtt && i->bo.addr <= address && i->bo.addr + i->bo.size > > > > address) > > > return i->bo; > > > address &= ~0xfff; > > > @@ -292,7 +294,7 @@ aub_mem_get_ggtt_bo(void *_mem, uint64_t address) > > > assert(res != MAP_FAILED); > > > } > > > - add_gtt_bo_map(mem, bo, true); > > > + add_gtt_bo_map(mem, bo, false, true); > > > return bo; > > > } > > > @@ -328,6 +330,10 @@ aub_mem_get_ppgtt_bo(void *_mem, uint64_t address) > > > struct aub_mem *mem = _mem; > > > struct gen_batch_decode_bo bo = {0}; > > > + list_for_each_entry(struct bo_map, i, &mem->maps, link) > > > + if (i->ppgtt && i->bo.addr <= address && i->bo.addr + i->bo.size > > > > address) > > > + return i->bo; > > > + > > > address &= ~0xfff; > > > if (!ppgtt_mapped(mem, mem->pml4, address)) > > > @@ -353,7 +359,7 @@ aub_mem_get_ppgtt_bo(void *_mem, uint64_t address) > > > assert(res != MAP_FAILED); > > > } > > > - add_gtt_bo_map(mem, bo, true); > > > + add_gtt_bo_map(mem, bo, true, true); > > > return bo; > > > } > > > -- > > > 2.18.0 > > > > > > _______________________________________________ > > > mesa-dev mailing list > > > mesa-dev@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev