From c5ac3240a5ff68b531c91f36106e0e91086a0c4d Mon Sep 17 00:00:00 2001 From: SecureCRT Date: Tue, 19 Jun 2012 23:30:34 +0800 Subject: [PATCH] msm: kgsl: improve postmortem and cff bounds checking Some hangs are fooling the postmortem dump code into running off the end of a buffer. Fix this by making its bounds check logic work better by reusing the logic from kgsl_find_region(). --- drivers/gpu/msm/adreno.c | 45 ++++++++++++++++------------- drivers/gpu/msm/adreno.h | 9 ++++-- drivers/gpu/msm/adreno_postmortem.c | 23 ++++++++------- drivers/gpu/msm/kgsl.c | 23 +-------------- drivers/gpu/msm/kgsl.h | 18 ++++++++---- drivers/gpu/msm/kgsl_cffdump.c | 20 ++++--------- 6 files changed, 64 insertions(+), 74 deletions(-) mode change 100644 => 100755 drivers/gpu/msm/adreno.c mode change 100644 => 100755 drivers/gpu/msm/adreno_postmortem.c mode change 100644 => 100755 drivers/gpu/msm/kgsl.c mode change 100644 => 100755 drivers/gpu/msm/kgsl.h mode change 100644 => 100755 drivers/gpu/msm/kgsl_cffdump.c diff --git a/drivers/gpu/msm/adreno.c b/drivers/gpu/msm/adreno.c old mode 100644 new mode 100755 index 61f14a4a..39f3004d --- a/drivers/gpu/msm/adreno.c +++ b/drivers/gpu/msm/adreno.c @@ -918,29 +918,25 @@ static int adreno_suspend_context(struct kgsl_device *device) return status; } -uint8_t *kgsl_sharedmem_convertaddr(struct kgsl_device *device, - unsigned int pt_base, unsigned int gpuaddr, unsigned int *size) +const struct kgsl_memdesc *adreno_find_region(struct kgsl_device *device, + unsigned int pt_base, + unsigned int gpuaddr, + unsigned int size) { - uint8_t *result = NULL; + struct kgsl_memdesc *result = NULL; struct kgsl_mem_entry *entry; struct kgsl_process_private *priv; struct adreno_device *adreno_dev = ADRENO_DEVICE(device); struct adreno_ringbuffer *ringbuffer = &adreno_dev->ringbuffer; - if (kgsl_gpuaddr_in_memdesc(&ringbuffer->buffer_desc, gpuaddr)) { - return kgsl_gpuaddr_to_vaddr(&ringbuffer->buffer_desc, - gpuaddr, size); - } + if (kgsl_gpuaddr_in_memdesc(&ringbuffer->buffer_desc, gpuaddr, size)) + return &ringbuffer->buffer_desc; - if (kgsl_gpuaddr_in_memdesc(&ringbuffer->memptrs_desc, gpuaddr)) { - return kgsl_gpuaddr_to_vaddr(&ringbuffer->memptrs_desc, - gpuaddr, size); - } + if (kgsl_gpuaddr_in_memdesc(&ringbuffer->memptrs_desc, gpuaddr, size)) + return &ringbuffer->memptrs_desc; - if (kgsl_gpuaddr_in_memdesc(&device->memstore, gpuaddr)) { - return kgsl_gpuaddr_to_vaddr(&device->memstore, - gpuaddr, size); - } + if (kgsl_gpuaddr_in_memdesc(&device->memstore, gpuaddr, size)) + return &device->memstore; mutex_lock(&kgsl_driver.process_mutex); list_for_each_entry(priv, &kgsl_driver.process_list, list) { @@ -950,8 +946,7 @@ uint8_t *kgsl_sharedmem_convertaddr(struct kgsl_device *device, entry = kgsl_sharedmem_find_region(priv, gpuaddr, sizeof(unsigned int)); if (entry) { - result = kgsl_gpuaddr_to_vaddr(&entry->memdesc, - gpuaddr, size); + result = &entry->memdesc; spin_unlock(&priv->mem_lock); mutex_unlock(&kgsl_driver.process_mutex); return result; @@ -962,14 +957,24 @@ uint8_t *kgsl_sharedmem_convertaddr(struct kgsl_device *device, BUG_ON(!mutex_is_locked(&device->mutex)); list_for_each_entry(entry, &device->memqueue, list) { - if (kgsl_gpuaddr_in_memdesc(&entry->memdesc, gpuaddr)) { - result = kgsl_gpuaddr_to_vaddr(&entry->memdesc, - gpuaddr, size); + if (kgsl_gpuaddr_in_memdesc(&entry->memdesc, gpuaddr, size)) { + result = &entry->memdesc; break; } } return result; + +} + +uint8_t *adreno_convertaddr(struct kgsl_device *device, unsigned int pt_base, + unsigned int gpuaddr, unsigned int size) +{ + const struct kgsl_memdesc *memdesc; + + memdesc = adreno_find_region(device, pt_base, gpuaddr, size); + + return memdesc ? kgsl_gpuaddr_to_vaddr(memdesc, gpuaddr) : NULL; } void adreno_regread(struct kgsl_device *device, unsigned int offsetwords, diff --git a/drivers/gpu/msm/adreno.h b/drivers/gpu/msm/adreno.h index e0857e05..40238313 100755 --- a/drivers/gpu/msm/adreno.h +++ b/drivers/gpu/msm/adreno.h @@ -83,8 +83,13 @@ void adreno_regread(struct kgsl_device *device, unsigned int offsetwords, void adreno_regwrite(struct kgsl_device *device, unsigned int offsetwords, unsigned int value); -uint8_t *kgsl_sharedmem_convertaddr(struct kgsl_device *device, - unsigned int pt_base, unsigned int gpuaddr, unsigned int *size); +const struct kgsl_memdesc *adreno_find_region(struct kgsl_device *device, + unsigned int pt_base, + unsigned int gpuaddr, + unsigned int size); + +uint8_t *adreno_convertaddr(struct kgsl_device *device, + unsigned int pt_base, unsigned int gpuaddr, unsigned int size); static inline int adreno_is_a200(struct adreno_device *adreno_dev) { diff --git a/drivers/gpu/msm/adreno_postmortem.c b/drivers/gpu/msm/adreno_postmortem.c old mode 100644 new mode 100755 index 3d957f69..b9b97377 --- a/drivers/gpu/msm/adreno_postmortem.c +++ b/drivers/gpu/msm/adreno_postmortem.c @@ -247,9 +247,8 @@ static void adreno_dump_regs(struct kgsl_device *device, static void dump_ib(struct kgsl_device *device, char* buffId, uint32_t pt_base, uint32_t base_offset, uint32_t ib_base, uint32_t ib_size, bool dump) { - unsigned int memsize; - uint8_t *base_addr = kgsl_sharedmem_convertaddr(device, pt_base, - ib_base, &memsize); + uint8_t *base_addr = adreno_convertaddr(device, pt_base, + ib_base, ib_size*sizeof(uint32_t)); if (base_addr && dump) print_hex_dump(KERN_ERR, buffId, DUMP_PREFIX_OFFSET, @@ -277,14 +276,13 @@ static void dump_ib1(struct kgsl_device *device, uint32_t pt_base, int i, j; uint32_t value; uint32_t *ib1_addr; - unsigned int memsize; dump_ib(device, "IB1:", pt_base, base_offset, ib1_base, ib1_size, dump); /* fetch virtual address for given IB base */ - ib1_addr = (uint32_t *)kgsl_sharedmem_convertaddr(device, pt_base, - ib1_base, &memsize); + ib1_addr = (uint32_t *)adreno_convertaddr(device, pt_base, + ib1_base, ib1_size*sizeof(uint32_t)); if (!ib1_addr) return; @@ -466,7 +464,7 @@ static int adreno_dump(struct kgsl_device *device) const uint32_t *rb_vaddr; int num_item = 0; int read_idx, write_idx; - unsigned int ts_processed, rb_memsize; + unsigned int ts_processed; static struct ib_list ib_list; @@ -681,11 +679,16 @@ static int adreno_dump(struct kgsl_device *device) KGSL_LOG_DUMP(device, "RB: rd_addr:%8.8x rb_size:%d num_item:%d\n", cp_rb_base, rb_count<<2, num_item); - rb_vaddr = (const uint32_t *)kgsl_sharedmem_convertaddr(device, - cur_pt_base, cp_rb_base, &rb_memsize); + + if (adreno_dev->ringbuffer.buffer_desc.gpuaddr != cp_rb_base) + KGSL_LOG_POSTMORTEM_WRITE(device, + "rb address mismatch, should be 0x%08x\n", + adreno_dev->ringbuffer.buffer_desc.gpuaddr); + + rb_vaddr = adreno_dev->ringbuffer.buffer_desc.hostptr; if (!rb_vaddr) { KGSL_LOG_POSTMORTEM_WRITE(device, - "Can't fetch vaddr for CP_RB_BASE\n"); + "rb has no kernel mapping!\n"); goto error_vfree; } diff --git a/drivers/gpu/msm/kgsl.c b/drivers/gpu/msm/kgsl.c old mode 100644 new mode 100755 index e21ca09c..02fbbd94 --- a/drivers/gpu/msm/kgsl.c +++ b/drivers/gpu/msm/kgsl.c @@ -758,9 +758,7 @@ kgsl_sharedmem_find_region(struct kgsl_process_private *private, BUG_ON(private == NULL); list_for_each_entry(entry, &private->mem_list, list) { - if (gpuaddr >= entry->memdesc.gpuaddr && - ((gpuaddr + size) <= - (entry->memdesc.gpuaddr + entry->memdesc.size))) { + if (kgsl_gpuaddr_in_memdesc(&entry->memdesc, gpuaddr, size)) { result = entry; break; } @@ -770,20 +768,6 @@ kgsl_sharedmem_find_region(struct kgsl_process_private *private, } EXPORT_SYMBOL(kgsl_sharedmem_find_region); -uint8_t *kgsl_gpuaddr_to_vaddr(const struct kgsl_memdesc *memdesc, - unsigned int gpuaddr, unsigned int *size) -{ - BUG_ON(memdesc->hostptr == NULL); - - if (memdesc->gpuaddr == 0 || (gpuaddr < memdesc->gpuaddr || - gpuaddr >= memdesc->gpuaddr + memdesc->size)) - return NULL; - - *size = memdesc->size - (gpuaddr - memdesc->gpuaddr); - return memdesc->hostptr + (gpuaddr - memdesc->gpuaddr); -} -EXPORT_SYMBOL(kgsl_gpuaddr_to_vaddr); - /*call all ioctl sub functions with driver locked*/ static long kgsl_ioctl_device_getproperty(struct kgsl_device_private *dev_priv, unsigned int cmd, void *data) @@ -1608,11 +1592,6 @@ kgsl_ioctl_sharedmem_flush_cache(struct kgsl_device_private *dev_priv, result = -EINVAL; goto done; } - if (!entry->memdesc.hostptr) - entry->memdesc.hostptr = - kgsl_gpuaddr_to_vaddr(&entry->memdesc, - param->gpuaddr, &entry->memdesc.size); - if (!entry->memdesc.hostptr) { KGSL_CORE_ERR("invalid hostptr with gpuaddr %08x\n", param->gpuaddr); diff --git a/drivers/gpu/msm/kgsl.h b/drivers/gpu/msm/kgsl.h old mode 100644 new mode 100755 index e26cdc9e..968f2b11 --- a/drivers/gpu/msm/kgsl.h +++ b/drivers/gpu/msm/kgsl.h @@ -139,8 +139,6 @@ struct kgsl_mem_entry { #endif void kgsl_mem_entry_destroy(struct kref *kref); -uint8_t *kgsl_gpuaddr_to_vaddr(const struct kgsl_memdesc *memdesc, - unsigned int gpuaddr, unsigned int *size); struct kgsl_mem_entry *kgsl_sharedmem_find_region( struct kgsl_process_private *private, unsigned int gpuaddr, size_t size); @@ -169,14 +167,24 @@ static inline void kgsl_drm_exit(void) #endif static inline int kgsl_gpuaddr_in_memdesc(const struct kgsl_memdesc *memdesc, - unsigned int gpuaddr) + unsigned int gpuaddr, unsigned int size) { - if (gpuaddr >= memdesc->gpuaddr && (gpuaddr + sizeof(unsigned int)) <= - (memdesc->gpuaddr + memdesc->size)) { + if (gpuaddr >= memdesc->gpuaddr && + ((gpuaddr + size) <= (memdesc->gpuaddr + memdesc->size))) { return 1; } return 0; } +static inline uint8_t *kgsl_gpuaddr_to_vaddr(const struct kgsl_memdesc *memdesc, + unsigned int gpuaddr) +{ + if (memdesc->hostptr == NULL || memdesc->gpuaddr == 0 || + (gpuaddr < memdesc->gpuaddr || + gpuaddr >= memdesc->gpuaddr + memdesc->size)) + return NULL; + + return memdesc->hostptr + (gpuaddr - memdesc->gpuaddr); +} static inline int timestamp_cmp(unsigned int new, unsigned int old) { diff --git a/drivers/gpu/msm/kgsl_cffdump.c b/drivers/gpu/msm/kgsl_cffdump.c old mode 100644 new mode 100755 index aa33152c..4d5de540 --- a/drivers/gpu/msm/kgsl_cffdump.c +++ b/drivers/gpu/msm/kgsl_cffdump.c @@ -401,8 +401,6 @@ void kgsl_cffdump_syncmem(struct kgsl_device_private *dev_priv, bool clean_cache) { const void *src; - uint host_size; - uint physaddr; if (!kgsl_cff_dump_enable) return; @@ -422,13 +420,9 @@ void kgsl_cffdump_syncmem(struct kgsl_device_private *dev_priv, } memdesc = &entry->memdesc; } - BUG_ON(memdesc->gpuaddr == 0); - BUG_ON(gpuaddr == 0); - physaddr = kgsl_get_realaddr(memdesc) + (gpuaddr - memdesc->gpuaddr); - - src = kgsl_gpuaddr_to_vaddr(memdesc, gpuaddr, &host_size); - if (src == NULL || host_size < sizebytes) { - KGSL_CORE_ERR("did not find mapping for " + src = (uint *)kgsl_gpuaddr_to_vaddr(memdesc, gpuaddr); + if (memdesc->hostptr == NULL) { + KGSL_CORE_ERR("no kernel mapping for " "gpuaddr: 0x%08x, m->host: 0x%p, phys: 0x%08x\n", gpuaddr, memdesc->hostptr, memdesc->physaddr); return; @@ -444,7 +438,6 @@ void kgsl_cffdump_syncmem(struct kgsl_device_private *dev_priv, KGSL_CACHE_OP_INV); } - BUG_ON(physaddr > 0x66000000 && physaddr < 0x66ffffff); while (sizebytes > 3) { cffdump_printline(-1, CFF_OP_WRITE_MEM, gpuaddr, *(uint *)src, 0, 0, 0); @@ -462,7 +455,6 @@ void kgsl_cffdump_setmem(uint addr, uint value, uint sizebytes) if (!kgsl_cff_dump_enable) return; - BUG_ON(addr > 0x66000000 && addr < 0x66ffffff); while (sizebytes > 3) { /* Use 32bit memory writes as long as there's at least * 4 bytes left */ @@ -575,7 +567,6 @@ bool kgsl_cffdump_parse_ibs(struct kgsl_device_private *dev_priv, { static uint level; /* recursion level */ bool ret = true; - uint host_size; uint *hostaddr, *hoststart; int dwords_left = sizedwords; /* dwords left in the current command buffer */ @@ -596,10 +587,9 @@ bool kgsl_cffdump_parse_ibs(struct kgsl_device_private *dev_priv, } memdesc = &entry->memdesc; } - - hostaddr = (uint *)kgsl_gpuaddr_to_vaddr(memdesc, gpuaddr, &host_size); + hostaddr = (uint *)kgsl_gpuaddr_to_vaddr(memdesc, gpuaddr); if (hostaddr == NULL) { - KGSL_CORE_ERR("did not find mapping for " + KGSL_CORE_ERR("no kernel mapping for " "gpuaddr: 0x%08x\n", gpuaddr); return true; }