| Message ID | 20130925172816.GB6995@phenom.dumpdata.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Sep 25, 2013 at 1:28 PM, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: > On Wed, Sep 25, 2013 at 08:29:07PM +0400, Alex Ivanov wrote: >> 24.09.2013, 00:11, "Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com>: >> > On Sat, Sep 21, 2013 at 07:39:10AM +0400, Alex Ivanov wrote: >> > >> >> 21.09.2013, ? 1:27, Alex Deucher <alexdeucher@gmail.com> ???????(?): >> >>> The register writes seems to be going through the register backbone correctly: >> >>> >> >>> [0x00B] 0x15E0=0x00000000 >> >>> [0x00C] 0x15E4=0xCAFEDEAD >> >>> [0x00D] 0x4274=0x0000000F >> >>> [0x00E] 0x42C8=0x00000007 >> >>> [0x00F] 0x4018=0x0000001D >> >>> [0x010] 0x170C=0x80000000 >> >>> [0x011] 0x3428=0x00020100 >> >>> [0x012] 0x15E4=0xCAFEDEAD >> >>> >> >>> You can see the 0xCAFEDEAD written to the scratch register via MMIO >> >>> from the ring_test(). The CP fifo however seems to be full of garbage. >> >>> The CP is busy though, so it seems to be functional. I guess it's >> >>> just fetching garbage rather than commands. >> > >> > If it is fetching garbage, that would imply the DMA (or bus addresses) >> > that are programmed in the GART are bogus. If you dump them and try >> > to figure out if bus adress -> physical address -> virtual address == >> > virtual address -> bus address that could help. And perhaps seeing what >> > the virtual address has - and or poisoning it with known data? >> > >> > Or perhaps the the card has picked up an incorrect page table? Meaning >> > the (bus) address given to it is not the correct one? >> > >> >> Konrad, >> >> Let's see. Please notice that i'm not PA-RISC or general linux kernel >> developer, just the user, so i may do things completely wrong. >> I was hoping that PA-RISC smarties will join me here, but they seem >> to be busy with other duties. Even port's mail list activity is low >> during last weeks. > > I took a look at the arch/parisc/kernel/pci-dma.c and I see that > is mostly a flat platform. That is bus addresses == physical addresses. > Unless it is an pclx or pclx2 CPU type (huh?) - if its it that > then any calls to dma_alloc_coherent will map memory out of a pool. > In essence it will look like a SWIOTLB bounce buffer. > > But interestingly enough there is a lot of 'flush_kernel_dcache_range' > call for every DMA operation. And I think the you need to do > dma_sync_for_cpu call in the radeon_test_writeback for it to > use the flush_kernel_dcache_range. I don't know what the > flush_kernel_dcache_range does thought so I could be wrong. > > That means you can ignore the little code below I wrote and > see about doing something like this: > > > diff --git a/drivers/gpu/drm/radeon/radeon_cp.c b/drivers/gpu/drm/radeon/radeon_cp.c > index 3cae2bb..9e5923d 100644 > --- a/drivers/gpu/drm/radeon/radeon_cp.c > +++ b/drivers/gpu/drm/radeon/radeon_cp.c > @@ -876,6 +876,7 @@ static void radeon_test_writeback(drm_radeon_private_t * dev_priv) > > RADEON_WRITE(RADEON_SCRATCH_REG1, 0xdeadbeef); > > + flush_kernel_dcache_range(dev_priv->ring_rptr, PAGE_SIZE); > for (tmp = 0; tmp < dev_priv->usec_timeout; tmp++) { > u32 val; > You'd want to add the add the flush to r100_ring_test() in r100.c. radeon_cp.c is for the old UMS support. Alex > But that is probably a shot in the dark. I have no clue what the flush_.. > is doing. > > [edit: And then I noticed sba_iommu.c, which is a complete IOMMU driver > where bus and physical addresses are different. sigh. What type of machine > is this? Does it have the IOMMU in it?] >> >> > If you dump them and try >> > to figure out if bus adress -> physical address -> virtual address == >> > virtual address -> bus address that could help >> >> With following >> >> radeon/radeon_ttm.c: >> >> radeon_ttm_tt_populate(): >> ... >> for (i = 0; i < ttm->num_pages; i++) { >> gtt->ttm.dma_address[i] = pci_map_page(rdev->pdev, ttm->pages[i], >> 0, PAGE_SIZE, >> PCI_DMA_BIDIRECTIONAL); >> >> void *va = bus_to_virt(gtt->ttm.dma_address[i]); >> if ((phys_addr_t) va != virt_to_bus(va)) { > > You are missing a translation here (you were comparing the virtual address > to the bus address). I was thinking something along this: > > unsigned int pfn = page_to_pfn(ttm->pages[i]); > dma_addr_t bus = gtt->ttm.dma_address[i]; > void *va_bus, *va, *va_pfn; > > if ((pfn << PAGE_SHIFT) != bus) > printk("Bus 0x%lx != PFN 0x%lx, bus, pfn << PAGE_SHIFT); /* OK, that means > bus addresses are different */ > > va_bus = bus_to_virt(gtt->ttm.dma_address[i]); > va_pfn = __va(pfn << PAGE_SHIFT); > > if (!virt_addr_valid(va_bus)) > printk("va_bus (0x%lx) not good!\n", va_bus); > if (!virt_addr_valid(va_pfn)) > printk("va_pfn (0x%lx) not good!\n", va_pfn); > > /* We got VA for both bus -> va, and pfn -> va. Should be the > same if bus and physical addresses are on the same namespace. */ > if (va_bus != va_pfn) > printk("va bus:%lx != va pfn: %lx\n", va_bus, va_pfn); > > /* Now that we have bus -> pa -> va (va_bus) try to go va_bus -> bus address. > The bus address should be the same */ > if (gtt->tmm.dma_address[i] != virt_to_bus(va_bus)) > printk("bus->pa->va:%lx != bus->pa->va->ba: %lx\n", gtt->tmm.dma_address[i],virt_to_bus(va_bus)); > >> DRM_INFO("MISMATCH: %p != %p\n", va, (void *) virt_to_bus(va)); >> /*DRM_INFO("CONTENTS: %x\n", *((uint32_t *)va));*/ // Leads to a Kernel Fault > > That is odd. I would have thought it would be usuable. > >> ... >> } >> >> I'm getting the output: >> >> [drm] MISMATCH: 0000000080280000 != 0000000040280000 > > In theory that means the bus address that is programmed in (gtt->dma_address[i]) > is 0000000040280000 (which is what virt_to_bus(va) should have resolved itself to). > > > Tha you can't get access to 'va' (0000000080280000) is odd. One way to try to > access it is to do: > > va = __va(page_to_pfn(ttm->pages[i]) << PAGE_SHIFT); > DRM_INFO("CONTENTS: %x\n", *((uint32_t)va)); > > As that would get it via the page -> va. > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
Alex, > You'd want to add the add the flush to r100_ring_test() in r100.c. > radeon_cp.c is for the old UMS support. Right! Konrad, Thanks for the code! I'll try asap. 25.09.2013, 21:28, "Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com>: > I took a look at the arch/parisc/kernel/pci-dma.c and I see that > is mostly a flat platform. That is bus addresses == physical addresses. > Unless it is an pclx or pclx2 CPU type (huh?) - if its it that > then any calls to dma_alloc_coherent will map memory out of a pool. > In essence it will look like a SWIOTLB bounce buffer. arch/parisc/kernel/pci-dma.c: ** PARISC 1.1 Dynamic DMA mapping support. ** This implementation is for PA-RISC platforms that do not support ** I/O TLBs (aka DMA address translation hardware). That's very old. PA-RISC 2.0 came into the game circa 1996. PA-RISC 1.1 is 32-bit only and i even not sure whether these machines had PCI bus. Only old boxes (PA7200 CPU and lower) cannot use dma_alloc_coherent() (and forced to do syncs iirc). That's not our case. And PA-RISC configs have 'Discontiguous Memory' choosen. > > But interestingly enough there is a lot of 'flush_kernel_dcache_range' > call for every DMA operation. And I think the you need to do > dma_sync_for_cpu call in the radeon_test_writeback for it to > use the flush_kernel_dcache_range. I don't know what the > flush_kernel_dcache_range does thought so I could be wrong. D-cache is a CPU cache (if they meant it). Seems to be L1-level. There is an I-cache at same level. > > That means you can ignore the little code below I wrote and > see about doing something like this: > > diff --git a/drivers/gpu/drm/radeon/radeon_cp.c b/drivers/gpu/drm/radeon/radeon_cp.c > index 3cae2bb..9e5923d 100644 > --- a/drivers/gpu/drm/radeon/radeon_cp.c > +++ b/drivers/gpu/drm/radeon/radeon_cp.c > @@ -876,6 +876,7 @@ static void radeon_test_writeback(drm_radeon_private_t * dev_priv) > > RADEON_WRITE(RADEON_SCRATCH_REG1, 0xdeadbeef); > > + flush_kernel_dcache_range(dev_priv->ring_rptr, PAGE_SIZE); > for (tmp = 0; tmp < dev_priv->usec_timeout; tmp++) { > u32 val; > > But that is probably a shot in the dark. I have no clue what the flush_.. > is doing. > > [edit: And then I noticed sba_iommu.c, which is a complete IOMMU driver > where bus and physical addresses are different. sigh. What type of machine > is this? Does it have the IOMMU in it?] That's our case. Yes, recent IA64 and PA-RISC machines have SBA IOMMU device. PCI I/O seem to go through it. There is a note for my chipset in sba_iommu.c: /* We are just "encouraging" 32-bit DMA masks here since we can * never allow IOMMU bypass unless we add special support for ZX1. */ And it indeed right. When i've tried to bypass hw IOMMU like in ia64 code it lead to the faults from drivers which do the DMA (like Fusion MPT SCSI driver). >> void *va = bus_to_virt(gtt->ttm.dma_address[i]); >> if ((phys_addr_t) va != virt_to_bus(va)) { > > You are missing a translation here (you were comparing the virtual address > to the bus address). I was thinking something along this: Yes, this confused me. I've translated your suggestion literally :\ > > unsigned int pfn = page_to_pfn(ttm->pages[i]); > dma_addr_t bus = gtt->ttm.dma_address[i]; > void *va_bus, *va, *va_pfn; > > if ((pfn << PAGE_SHIFT) != bus) > printk("Bus 0x%lx != PFN 0x%lx, bus, pfn << PAGE_SHIFT); /* OK, that means > bus addresses are different */ > > va_bus = bus_to_virt(gtt->ttm.dma_address[i]); > va_pfn = __va(pfn << PAGE_SHIFT); > > if (!virt_addr_valid(va_bus)) > printk("va_bus (0x%lx) not good!\n", va_bus); > if (!virt_addr_valid(va_pfn)) > printk("va_pfn (0x%lx) not good!\n", va_pfn); > > /* We got VA for both bus -> va, and pfn -> va. Should be the > same if bus and physical addresses are on the same namespace. */ > if (va_bus != va_pfn) > printk("va bus:%lx != va pfn: %lx\n", va_bus, va_pfn); > > /* Now that we have bus -> pa -> va (va_bus) try to go va_bus -> bus address. > The bus address should be the same */ > if (gtt->tmm.dma_address[i] != virt_to_bus(va_bus)) > printk("bus->pa->va:%lx != bus->pa->va->ba: %lx\n", gtt->tmm.dma_address[i],virt_to_bus(va_bus)); > >> DRM_INFO("MISMATCH: %p != %p\n", va, (void *) virt_to_bus(va)); >> /*DRM_INFO("CONTENTS: %x\n", *((uint32_t *)va));*/ // Leads to a Kernel Fault > > That is odd. I would have thought it would be usuable. > >> ... >> } >> >> I'm getting the output: >> >> [drm] MISMATCH: 0000000080280000 != 0000000040280000 > > In theory that means the bus address that is programmed in (gtt->dma_address[i]) > is 0000000040280000 (which is what virt_to_bus(va) should have resolved itself to). > Should resolved properly. I had a sane check of virt_to_bus(va) == gtt->ttm.dma_address[i] > Tha you can't get access to 'va' (0000000080280000) is odd. One way to try to > access it is to do: > > va = __va(page_to_pfn(ttm->pages[i]) << PAGE_SHIFT); > DRM_INFO("CONTENTS: %x\n", *((uint32_t)va)); > > As that would get it via the page -> va.
Let's go futher. 25.09.2013, 22:58, "Alex Ivanov" <gnidorah@p0n4ik.tk>: > 25.09.2013, 21:28, "Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com>: >> I took a look at the arch/parisc/kernel/pci-dma.c and I see that >> is mostly a flat platform. That is bus addresses == physical addresses. >> Unless it is an pclx or pclx2 CPU type (huh?) - if its it that >> then any calls to dma_alloc_coherent will map memory out of a pool. >> In essence it will look like a SWIOTLB bounce buffer. > arch/parisc/kernel/pci-dma.c: > ** PARISC 1.1 Dynamic DMA mapping support. > ** This implementation is for PA-RISC platforms that do not support > ** I/O TLBs (aka DMA address translation hardware). > > That's very old. PA-RISC 2.0 came into the game circa 1996. > PA-RISC 1.1 is 32-bit only and i even not sure whether these machines > had PCI bus. > > Only old boxes (PA7200 CPU and lower) cannot use dma_alloc_coherent() > (and forced to do syncs iirc). That's not our case. > And PA-RISC configs have 'Discontiguous Memory' choosen. >> But interestingly enough there is a lot of 'flush_kernel_dcache_range' >> call for every DMA operation. >> And I think the you need to do >> dma_sync_for_cpu call in the radeon_test_writeback for it to >> use the flush_kernel_dcache_range. I was correct regarding syncs. In our case (SBA IOMMU) dma_sync* calls are no-ops: sba_iommu.c: static struct hppa_dma_ops sba_ops = { ... .dma_sync_single_for_cpu = NULL, .dma_sync_single_for_device = NULL, .dma_sync_sg_for_cpu = NULL, .dma_sync_sg_for_device = NULL, } dma-mapping.h: dma_cache_sync(struct device *dev, void *vaddr, size_t size, enum dma_data_direction direction) { if(hppa_dma_ops->dma_sync_single_for_cpu) flush_kernel_dcache_range((unsigned long)vaddr, size); } So i'll skip doing the flush_kernel_dcache_range(). >> I don't know what the >> flush_kernel_dcache_range does thought so I could be wrong. > D-cache is a CPU cache (if they meant it). > Seems to be L1-level. There is an I-cache at same level. >> You are missing a translation here (you were comparing the virtual address >> to the bus address). I was thinking something along this: > Yes, this confused me. I've translated your suggestion literally :\ >> unsigned int pfn = page_to_pfn(ttm->pages[i]); >> dma_addr_t bus = gtt->ttm.dma_address[i]; >> void *va_bus, *va, *va_pfn; >> >> if ((pfn << PAGE_SHIFT) != bus) >> printk("Bus 0x%lx != PFN 0x%lx, bus, pfn << PAGE_SHIFT); /* OK, that means >> bus addresses are different */ >> >> va_bus = bus_to_virt(gtt->ttm.dma_address[i]); >> va_pfn = __va(pfn << PAGE_SHIFT); >> >> if (!virt_addr_valid(va_bus)) >> printk("va_bus (0x%lx) not good!\n", va_bus); >> if (!virt_addr_valid(va_pfn)) >> printk("va_pfn (0x%lx) not good!\n", va_pfn); >> >> /* We got VA for both bus -> va, and pfn -> va. Should be the >> same if bus and physical addresses are on the same namespace. */ >> if (va_bus != va_pfn) >> printk("va bus:%lx != va pfn: %lx\n", va_bus, va_pfn); >> >> /* Now that we have bus -> pa -> va (va_bus) try to go va_bus -> bus address. >> The bus address should be the same */ >> if (gtt->tmm.dma_address[i] != virt_to_bus(va_bus)) >> printk("bus->pa->va:%lx != bus->pa->va->ba: %lx\n", gtt->tmm.dma_address[i],virt_to_bus(va_bus)); Ok, slightly modified: struct page *page = ttm->pages[i]; unsigned long pfn = page_to_pfn(page); dma_addr_t bus = gtt->ttm.dma_address[i]; void *va_bus, *va, *va_pfn; BUG_ON(!pfn_valid(pfn)); //BUG_ON(!page_mapping(page)); // Leads to a kernel BUG /* Avoid floodage */ if (i % 100 == 0) { if ((pfn << PAGE_SHIFT) != bus) printk("Bus 0x%lx != PFN 0x%lx\n", bus, pfn << PAGE_SHIFT); /* OK, that means bus addresses are different */ va_bus = bus_to_virt(bus); va_pfn = __va(pfn << PAGE_SHIFT); if (!virt_addr_valid(va_bus)) printk("va_bus (0x%lx) not good!\n", va_bus); if (!virt_addr_valid(va_pfn)) printk("va_pfn (0x%lx) not good!\n", va_pfn); /* We got VA for both bus -> va, and pfn -> va. Should be the same if bus and physical addresses are on the same namespace. */ if (va_bus != va_pfn) printk("va bus: %lx != va pfn: %lx\n", va_bus, va_pfn); /* Now that we have bus -> pa -> va (va_bus) try to go va_bus -> bus address. The bus address should be the same */ if (bus != virt_to_bus(va_bus)) printk("bus->pa->va: %lx != bus->pa->va->ba: %lx\n", bus,virt_to_bus(va_bus)); } Output: Bus 0x40280000 != PFN 0x3e92d000 va_bus (0x80280000) not good! va bus: 80280000 != va pfn: 7e92d000 Bus 0x40281000 != PFN 0x3e930000 va_bus (0x80281000) not good! va bus: 80281000 != va pfn: 7e930000 ... va_bus is invalid. That's the reason i was getting a KF trying to read data behind it. >>> DRM_INFO("MISMATCH: %p != %p\n", va, (void *) virt_to_bus(va)); >>> /*DRM_INFO("CONTENTS: %x\n", *((uint32_t *)va));*/ // Leads to a Kernel Fault >> That is odd. I would have thought it would be usuable. >>> ... >>> } >>> >>> I'm getting the output: >>> >>> [drm] MISMATCH: 0000000080280000 != 0000000040280000 >> In theory that means the bus address that is programmed in (gtt->dma_address[i]) >> is 0000000040280000 (which is what virt_to_bus(va) should have resolved itself to). > Should resolved properly. I had a sane check of virt_to_bus(va) == gtt->ttm.dma_address[i] >> Tha you can't get access to 'va' (0000000080280000) is odd. One way to try to >> access it is to do: >> >> va = __va(page_to_pfn(ttm->pages[i]) << PAGE_SHIFT); >> DRM_INFO("CONTENTS: %x\n", *((uint32_t)va)); >> >> As that would get it via the page -> va. This way i get CONTENTS: 0 > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/radeon/radeon_cp.c b/drivers/gpu/drm/radeon/radeon_cp.c index 3cae2bb..9e5923d 100644 --- a/drivers/gpu/drm/radeon/radeon_cp.c +++ b/drivers/gpu/drm/radeon/radeon_cp.c @@ -876,6 +876,7 @@ static void radeon_test_writeback(drm_radeon_private_t * dev_priv) RADEON_WRITE(RADEON_SCRATCH_REG1, 0xdeadbeef); + flush_kernel_dcache_range(dev_priv->ring_rptr, PAGE_SIZE); for (tmp = 0; tmp < dev_priv->usec_timeout; tmp++) { u32 val;