* [PATCH v2 0/2] Migo-R: reserve memory block with memblock APIs @ 2017-11-14 17:08 ` Jacopo Mondi 0 siblings, 0 replies; 18+ messages in thread From: Jacopo Mondi @ 2017-11-14 17:08 UTC (permalink / raw) To: ysato, dalias, laurent.pinchart, geert Cc: Jacopo Mondi, linux-sh, linux-renesas-soc, iommu, linux-kernel Re-spinn of "[PATCH] sh: migor: Reserve memory block for CEU" with additional patch on top to post-pone page_to_pfn() mapping during mmap() on DMA memory. Laurent is probably working on a proper fix, so I'm just copying the iommu list here to make them aware of the problem on SH4 architecture, but please do not consider that patch for inclusion for now. Thanks j Jacopo Mondi (2): sh: migor: Reserve memory block for CEU base: dma-mapping: Postpone page_to_pfn() on mmap() arch/sh/boards/mach-migor/setup.c | 29 +++++++++++++++++++++++++++-- drivers/base/dma-mapping.c | 3 ++- 2 files changed, 29 insertions(+), 3 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 0/2] Migo-R: reserve memory block with memblock APIs @ 2017-11-14 17:08 ` Jacopo Mondi 0 siblings, 0 replies; 18+ messages in thread From: Jacopo Mondi @ 2017-11-14 17:08 UTC (permalink / raw) To: ysato, dalias, laurent.pinchart, geert Cc: Jacopo Mondi, linux-sh, linux-renesas-soc, iommu, linux-kernel Re-spinn of "[PATCH] sh: migor: Reserve memory block for CEU" with additional patch on top to post-pone page_to_pfn() mapping during mmap() on DMA memory. Laurent is probably working on a proper fix, so I'm just copying the iommu list here to make them aware of the problem on SH4 architecture, but please do not consider that patch for inclusion for now. Thanks j Jacopo Mondi (2): sh: migor: Reserve memory block for CEU base: dma-mapping: Postpone page_to_pfn() on mmap() arch/sh/boards/mach-migor/setup.c | 29 +++++++++++++++++++++++++++-- drivers/base/dma-mapping.c | 3 ++- 2 files changed, 29 insertions(+), 3 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 1/2] sh: migor: Reserve memory block for CEU 2017-11-14 17:08 ` Jacopo Mondi @ 2017-11-14 17:08 ` Jacopo Mondi -1 siblings, 0 replies; 18+ messages in thread From: Jacopo Mondi @ 2017-11-14 17:08 UTC (permalink / raw) To: ysato, dalias, laurent.pinchart, geert Cc: Jacopo Mondi, linux-sh, linux-renesas-soc, iommu, linux-kernel A memory region for CEU (Capture Engine Unit) video buffers has to be reserved during initialization. Originally, it was allocated through DMA API helpers and stored in the second IORESOURCE_MEM entry, to be later remapped by the CEU driver with a call to 'dma_declare_coherent_memory()' As Linux does not allow anymore to remap system RAM regions with 'memremap' function, sh_mobile_ceu driver fails when trying to remap the memory area: kernel/memremap.c: WARN_ONCE(1, "memremap attempted on ram %pa size: %#lx\n", &offset, (unsigned long) size) As we need to reserve a block of memory using memblock APIs early enough to have it removed from the available system RAM and thus being later re-mappable, use '.mv_mem_reserve' member of SH's 'struct sh_machine_vector' which is called during system initialization. This is similar to what happens on ARM architecture with 'arm_memblock_steal()' function. Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> --- arch/sh/boards/mach-migor/setup.c | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/arch/sh/boards/mach-migor/setup.c b/arch/sh/boards/mach-migor/setup.c index 0bcbe58..98aa094 100644 --- a/arch/sh/boards/mach-migor/setup.c +++ b/arch/sh/boards/mach-migor/setup.c @@ -12,6 +12,7 @@ #include <linux/interrupt.h> #include <linux/input.h> #include <linux/input/sh_keysc.h> +#include <linux/memblock.h> #include <linux/mmc/host.h> #include <linux/mtd/physmap.h> #include <linux/mfd/tmio.h> @@ -45,6 +46,9 @@ * 0x18000000 8GB 8 NAND Flash (K9K8G08U0A) */ +#define CEU_BUFFER_MEMORY_SIZE (4 << 20) +static phys_addr_t ceu_dma_membase; + static struct smc91x_platdata smc91x_info = { .flags = SMC91X_USE_16BIT | SMC91X_NOWAIT, }; @@ -501,6 +505,8 @@ extern char migor_sdram_leave_end; static int __init migor_devices_setup(void) { + struct resource *r; + /* register board specific self-refresh code */ sh_mobile_register_self_refresh(SUSP_SH_STANDBY | SUSP_SH_SF, &migor_sdram_enter_start, @@ -632,8 +638,6 @@ static int __init migor_devices_setup(void) #endif __raw_writew(__raw_readw(PORT_MSELCRB) | 0x2000, PORT_MSELCRB); /* D15->D8 */ - platform_resource_setup_memory(&migor_ceu_device, "ceu", 4 << 20); - /* SIU: Port B */ gpio_request(GPIO_FN_SIUBOLR, NULL); gpio_request(GPIO_FN_SIUBOBT, NULL); @@ -647,6 +651,13 @@ static int __init migor_devices_setup(void) */ __raw_writew(__raw_readw(PORT_MSELCRA) | 1, PORT_MSELCRA); + /* Setup additional memory resource for CEU video buffers */ + r = &migor_ceu_device.resource[2]; + r->flags = IORESOURCE_MEM; + r->start = ceu_dma_membase; + r->end = r->start + CEU_BUFFER_MEMORY_SIZE - 1; + r->name = "ceu"; + i2c_register_board_info(0, migor_i2c_devices, ARRAY_SIZE(migor_i2c_devices)); @@ -665,10 +676,24 @@ static int migor_mode_pins(void) return MODE_PIN0 | MODE_PIN1 | MODE_PIN5; } +/* Reserve a portion of memory for CEU buffers */ +static void __init migor_mv_mem_reserve(void) +{ + phys_addr_t phys; + phys_addr_t size = CEU_BUFFER_MEMORY_SIZE; + + phys = memblock_alloc_base(size, PAGE_SIZE, MEMBLOCK_ALLOC_ANYWHERE); + memblock_free(phys, size); + memblock_remove(phys, size); + + ceu_dma_membase = phys; +} + /* * The Machine Vector */ static struct sh_machine_vector mv_migor __initmv = { .mv_name = "Migo-R", .mv_mode_pins = migor_mode_pins, + .mv_mem_reserve = migor_mv_mem_reserve, }; -- 2.7.4 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 1/2] sh: migor: Reserve memory block for CEU @ 2017-11-14 17:08 ` Jacopo Mondi 0 siblings, 0 replies; 18+ messages in thread From: Jacopo Mondi @ 2017-11-14 17:08 UTC (permalink / raw) To: ysato, dalias, laurent.pinchart, geert Cc: Jacopo Mondi, linux-sh, linux-renesas-soc, iommu, linux-kernel A memory region for CEU (Capture Engine Unit) video buffers has to be reserved during initialization. Originally, it was allocated through DMA API helpers and stored in the second IORESOURCE_MEM entry, to be later remapped by the CEU driver with a call to 'dma_declare_coherent_memory()' As Linux does not allow anymore to remap system RAM regions with 'memremap' function, sh_mobile_ceu driver fails when trying to remap the memory area: kernel/memremap.c: WARN_ONCE(1, "memremap attempted on ram %pa size: %#lx\n", &offset, (unsigned long) size) As we need to reserve a block of memory using memblock APIs early enough to have it removed from the available system RAM and thus being later re-mappable, use '.mv_mem_reserve' member of SH's 'struct sh_machine_vector' which is called during system initialization. This is similar to what happens on ARM architecture with 'arm_memblock_steal()' function. Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> --- arch/sh/boards/mach-migor/setup.c | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/arch/sh/boards/mach-migor/setup.c b/arch/sh/boards/mach-migor/setup.c index 0bcbe58..98aa094 100644 --- a/arch/sh/boards/mach-migor/setup.c +++ b/arch/sh/boards/mach-migor/setup.c @@ -12,6 +12,7 @@ #include <linux/interrupt.h> #include <linux/input.h> #include <linux/input/sh_keysc.h> +#include <linux/memblock.h> #include <linux/mmc/host.h> #include <linux/mtd/physmap.h> #include <linux/mfd/tmio.h> @@ -45,6 +46,9 @@ * 0x18000000 8GB 8 NAND Flash (K9K8G08U0A) */ +#define CEU_BUFFER_MEMORY_SIZE (4 << 20) +static phys_addr_t ceu_dma_membase; + static struct smc91x_platdata smc91x_info = { .flags = SMC91X_USE_16BIT | SMC91X_NOWAIT, }; @@ -501,6 +505,8 @@ extern char migor_sdram_leave_end; static int __init migor_devices_setup(void) { + struct resource *r; + /* register board specific self-refresh code */ sh_mobile_register_self_refresh(SUSP_SH_STANDBY | SUSP_SH_SF, &migor_sdram_enter_start, @@ -632,8 +638,6 @@ static int __init migor_devices_setup(void) #endif __raw_writew(__raw_readw(PORT_MSELCRB) | 0x2000, PORT_MSELCRB); /* D15->D8 */ - platform_resource_setup_memory(&migor_ceu_device, "ceu", 4 << 20); - /* SIU: Port B */ gpio_request(GPIO_FN_SIUBOLR, NULL); gpio_request(GPIO_FN_SIUBOBT, NULL); @@ -647,6 +651,13 @@ static int __init migor_devices_setup(void) */ __raw_writew(__raw_readw(PORT_MSELCRA) | 1, PORT_MSELCRA); + /* Setup additional memory resource for CEU video buffers */ + r = &migor_ceu_device.resource[2]; + r->flags = IORESOURCE_MEM; + r->start = ceu_dma_membase; + r->end = r->start + CEU_BUFFER_MEMORY_SIZE - 1; + r->name = "ceu"; + i2c_register_board_info(0, migor_i2c_devices, ARRAY_SIZE(migor_i2c_devices)); @@ -665,10 +676,24 @@ static int migor_mode_pins(void) return MODE_PIN0 | MODE_PIN1 | MODE_PIN5; } +/* Reserve a portion of memory for CEU buffers */ +static void __init migor_mv_mem_reserve(void) +{ + phys_addr_t phys; + phys_addr_t size = CEU_BUFFER_MEMORY_SIZE; + + phys = memblock_alloc_base(size, PAGE_SIZE, MEMBLOCK_ALLOC_ANYWHERE); + memblock_free(phys, size); + memblock_remove(phys, size); + + ceu_dma_membase = phys; +} + /* * The Machine Vector */ static struct sh_machine_vector mv_migor __initmv = { .mv_name = "Migo-R", .mv_mode_pins = migor_mode_pins, + .mv_mem_reserve = migor_mv_mem_reserve, }; -- 2.7.4 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [RFC v2 2/2] base: dma-mapping: Postpone page_to_pfn() on mmap() 2017-11-14 17:08 ` Jacopo Mondi @ 2017-11-14 17:08 ` Jacopo Mondi -1 siblings, 0 replies; 18+ messages in thread From: Jacopo Mondi @ 2017-11-14 17:08 UTC (permalink / raw) To: ysato, dalias, laurent.pinchart, geert Cc: Jacopo Mondi, linux-sh, linux-renesas-soc, iommu, linux-kernel On SH4 architecture, with SPARSEMEM memory model, translating page to pfn hangs the CPU. Post-pone translation to pfn after dma_mmap_from_dev_coherent() function call as it succeeds and make page translation not necessary. This patch was suggested by Laurent Pinchart and he's working to submit a proper fix mainline. Not sending for inclusion at the moment. Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> --- drivers/base/dma-mapping.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c index e584edd..73d64d3 100644 --- a/drivers/base/dma-mapping.c +++ b/drivers/base/dma-mapping.c @@ -227,8 +227,8 @@ int dma_common_mmap(struct device *dev, struct vm_area_struct *vma, #ifndef CONFIG_ARCH_NO_COHERENT_DMA_MMAP unsigned long user_count = vma_pages(vma); unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT; - unsigned long pfn = page_to_pfn(virt_to_page(cpu_addr)); unsigned long off = vma->vm_pgoff; + unsigned long pfn; vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); @@ -236,6 +236,7 @@ int dma_common_mmap(struct device *dev, struct vm_area_struct *vma, return ret; if (off < count && user_count <= (count - off)) { + pfn = page_to_pfn(virt_to_page(cpu_addr)); ret = remap_pfn_range(vma, vma->vm_start, pfn + off, user_count << PAGE_SHIFT, -- 2.7.4 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [RFC v2 2/2] base: dma-mapping: Postpone page_to_pfn() on mmap() @ 2017-11-14 17:08 ` Jacopo Mondi 0 siblings, 0 replies; 18+ messages in thread From: Jacopo Mondi @ 2017-11-14 17:08 UTC (permalink / raw) To: ysato, dalias, laurent.pinchart, geert Cc: Jacopo Mondi, linux-sh, linux-renesas-soc, iommu, linux-kernel On SH4 architecture, with SPARSEMEM memory model, translating page to pfn hangs the CPU. Post-pone translation to pfn after dma_mmap_from_dev_coherent() function call as it succeeds and make page translation not necessary. This patch was suggested by Laurent Pinchart and he's working to submit a proper fix mainline. Not sending for inclusion at the moment. Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> --- drivers/base/dma-mapping.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c index e584edd..73d64d3 100644 --- a/drivers/base/dma-mapping.c +++ b/drivers/base/dma-mapping.c @@ -227,8 +227,8 @@ int dma_common_mmap(struct device *dev, struct vm_area_struct *vma, #ifndef CONFIG_ARCH_NO_COHERENT_DMA_MMAP unsigned long user_count = vma_pages(vma); unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT; - unsigned long pfn = page_to_pfn(virt_to_page(cpu_addr)); unsigned long off = vma->vm_pgoff; + unsigned long pfn; vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); @@ -236,6 +236,7 @@ int dma_common_mmap(struct device *dev, struct vm_area_struct *vma, return ret; if (off < count && user_count <= (count - off)) { + pfn = page_to_pfn(virt_to_page(cpu_addr)); ret = remap_pfn_range(vma, vma->vm_start, pfn + off, user_count << PAGE_SHIFT, -- 2.7.4 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [RFC v2 2/2] base: dma-mapping: Postpone page_to_pfn() on mmap() 2017-11-14 17:08 ` Jacopo Mondi (?) @ 2017-11-15 13:38 ` Robin Murphy 2018-04-09 7:25 ` jacopo mondi -1 siblings, 1 reply; 18+ messages in thread From: Robin Murphy @ 2017-11-15 13:38 UTC (permalink / raw) To: Jacopo Mondi, ysato, dalias, laurent.pinchart, geert Cc: linux-renesas-soc, iommu, linux-kernel, linux-sh On 14/11/17 17:08, Jacopo Mondi wrote: > On SH4 architecture, with SPARSEMEM memory model, translating page to > pfn hangs the CPU. Post-pone translation to pfn after > dma_mmap_from_dev_coherent() function call as it succeeds and make page > translation not necessary. > > This patch was suggested by Laurent Pinchart and he's working to submit > a proper fix mainline. Not sending for inclusion at the moment. Y'know, I think this patch does have some merit by itself - until we know that cpu_addr *doesn't* represent some device-private memory which is not guaranteed to be backed by a struct page, calling virt_to_page() on it is arguably semantically incorrect, even if it might happen to be benign in most cases. Robin. > Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > --- > drivers/base/dma-mapping.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c > index e584edd..73d64d3 100644 > --- a/drivers/base/dma-mapping.c > +++ b/drivers/base/dma-mapping.c > @@ -227,8 +227,8 @@ int dma_common_mmap(struct device *dev, struct vm_area_struct *vma, > #ifndef CONFIG_ARCH_NO_COHERENT_DMA_MMAP > unsigned long user_count = vma_pages(vma); > unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT; > - unsigned long pfn = page_to_pfn(virt_to_page(cpu_addr)); > unsigned long off = vma->vm_pgoff; > + unsigned long pfn; > > vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); > > @@ -236,6 +236,7 @@ int dma_common_mmap(struct device *dev, struct vm_area_struct *vma, > return ret; > > if (off < count && user_count <= (count - off)) { > + pfn = page_to_pfn(virt_to_page(cpu_addr)); > ret = remap_pfn_range(vma, vma->vm_start, > pfn + off, > user_count << PAGE_SHIFT, > -- > 2.7.4 > > _______________________________________________ > iommu mailing list > iommu@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC v2 2/2] base: dma-mapping: Postpone page_to_pfn() on mmap() 2017-11-15 13:38 ` Robin Murphy @ 2018-04-09 7:25 ` jacopo mondi 0 siblings, 0 replies; 18+ messages in thread From: jacopo mondi @ 2018-04-09 7:25 UTC (permalink / raw) To: Robin Murphy Cc: Jacopo Mondi, ysato, dalias, laurent.pinchart, geert, linux-renesas-soc, iommu, linux-kernel, linux-sh [-- Attachment #1: Type: text/plain, Size: 2565 bytes --] Hi Robin, Laurent, a long time passed, sorry about this. On Wed, Nov 15, 2017 at 01:38:23PM +0000, Robin Murphy wrote: > On 14/11/17 17:08, Jacopo Mondi wrote: > >On SH4 architecture, with SPARSEMEM memory model, translating page to > >pfn hangs the CPU. Post-pone translation to pfn after > >dma_mmap_from_dev_coherent() function call as it succeeds and make page > >translation not necessary. > > > >This patch was suggested by Laurent Pinchart and he's working to submit > >a proper fix mainline. Not sending for inclusion at the moment. > > Y'know, I think this patch does have some merit by itself - until we know > that cpu_addr *doesn't* represent some device-private memory which is not > guaranteed to be backed by a struct page, calling virt_to_page() on it is > arguably semantically incorrect, even if it might happen to be benign in > most cases. I still need to carry this patch in my trees to have a working dma memory mapping on SH4 platforms. My understanding from your comment is that there may be a way forward for this patch, do you still think the same? Have you got any suggestion on how to improve this eventually for inclusion? Thanks j > > Robin. > > >Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > >--- > > drivers/base/dma-mapping.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > >diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c > >index e584edd..73d64d3 100644 > >--- a/drivers/base/dma-mapping.c > >+++ b/drivers/base/dma-mapping.c > >@@ -227,8 +227,8 @@ int dma_common_mmap(struct device *dev, struct vm_area_struct *vma, > > #ifndef CONFIG_ARCH_NO_COHERENT_DMA_MMAP > > unsigned long user_count = vma_pages(vma); > > unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT; > >- unsigned long pfn = page_to_pfn(virt_to_page(cpu_addr)); > > unsigned long off = vma->vm_pgoff; > >+ unsigned long pfn; > > > > vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); > > > >@@ -236,6 +236,7 @@ int dma_common_mmap(struct device *dev, struct vm_area_struct *vma, > > return ret; > > > > if (off < count && user_count <= (count - off)) { > >+ pfn = page_to_pfn(virt_to_page(cpu_addr)); > > ret = remap_pfn_range(vma, vma->vm_start, > > pfn + off, > > user_count << PAGE_SHIFT, > >-- > >2.7.4 > > > >_______________________________________________ > >iommu mailing list > >iommu@lists.linux-foundation.org > >https://lists.linuxfoundation.org/mailman/listinfo/iommu > > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC v2 2/2] base: dma-mapping: Postpone page_to_pfn() on mmap() @ 2018-04-09 7:25 ` jacopo mondi 0 siblings, 0 replies; 18+ messages in thread From: jacopo mondi @ 2018-04-09 7:25 UTC (permalink / raw) To: Robin Murphy Cc: Jacopo Mondi, ysato, dalias, laurent.pinchart, geert, linux-renesas-soc, iommu, linux-kernel, linux-sh [-- Attachment #1: Type: text/plain, Size: 2565 bytes --] Hi Robin, Laurent, a long time passed, sorry about this. On Wed, Nov 15, 2017 at 01:38:23PM +0000, Robin Murphy wrote: > On 14/11/17 17:08, Jacopo Mondi wrote: > >On SH4 architecture, with SPARSEMEM memory model, translating page to > >pfn hangs the CPU. Post-pone translation to pfn after > >dma_mmap_from_dev_coherent() function call as it succeeds and make page > >translation not necessary. > > > >This patch was suggested by Laurent Pinchart and he's working to submit > >a proper fix mainline. Not sending for inclusion at the moment. > > Y'know, I think this patch does have some merit by itself - until we know > that cpu_addr *doesn't* represent some device-private memory which is not > guaranteed to be backed by a struct page, calling virt_to_page() on it is > arguably semantically incorrect, even if it might happen to be benign in > most cases. I still need to carry this patch in my trees to have a working dma memory mapping on SH4 platforms. My understanding from your comment is that there may be a way forward for this patch, do you still think the same? Have you got any suggestion on how to improve this eventually for inclusion? Thanks j > > Robin. > > >Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > >--- > > drivers/base/dma-mapping.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > >diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c > >index e584edd..73d64d3 100644 > >--- a/drivers/base/dma-mapping.c > >+++ b/drivers/base/dma-mapping.c > >@@ -227,8 +227,8 @@ int dma_common_mmap(struct device *dev, struct vm_area_struct *vma, > > #ifndef CONFIG_ARCH_NO_COHERENT_DMA_MMAP > > unsigned long user_count = vma_pages(vma); > > unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT; > >- unsigned long pfn = page_to_pfn(virt_to_page(cpu_addr)); > > unsigned long off = vma->vm_pgoff; > >+ unsigned long pfn; > > > > vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); > > > >@@ -236,6 +236,7 @@ int dma_common_mmap(struct device *dev, struct vm_area_struct *vma, > > return ret; > > > > if (off < count && user_count <= (count - off)) { > >+ pfn = page_to_pfn(virt_to_page(cpu_addr)); > > ret = remap_pfn_range(vma, vma->vm_start, > > pfn + off, > > user_count << PAGE_SHIFT, > >-- > >2.7.4 > > > >_______________________________________________ > >iommu mailing list > >iommu@lists.linux-foundation.org > >https://lists.linuxfoundation.org/mailman/listinfo/iommu > > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC v2 2/2] base: dma-mapping: Postpone page_to_pfn() on mmap() 2018-04-09 7:25 ` jacopo mondi (?) @ 2018-04-09 11:11 ` Robin Murphy -1 siblings, 0 replies; 18+ messages in thread From: Robin Murphy @ 2018-04-09 11:11 UTC (permalink / raw) To: jacopo mondi Cc: dalias-8zAoT0mYgF4, ysato-Rn4VEauK+AKRv+LV9MX5uooqe+aC9MnS, linux-sh-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, geert-Td1EMuHUCqxL1ZNQvxDV9g, laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw, Jacopo Mondi Hi Jacopo, On 09/04/18 08:25, jacopo mondi wrote: > Hi Robin, Laurent, > a long time passed, sorry about this. > > On Wed, Nov 15, 2017 at 01:38:23PM +0000, Robin Murphy wrote: >> On 14/11/17 17:08, Jacopo Mondi wrote: >>> On SH4 architecture, with SPARSEMEM memory model, translating page to >>> pfn hangs the CPU. Post-pone translation to pfn after >>> dma_mmap_from_dev_coherent() function call as it succeeds and make page >>> translation not necessary. >>> >>> This patch was suggested by Laurent Pinchart and he's working to submit >>> a proper fix mainline. Not sending for inclusion at the moment. >> >> Y'know, I think this patch does have some merit by itself - until we know >> that cpu_addr *doesn't* represent some device-private memory which is not >> guaranteed to be backed by a struct page, calling virt_to_page() on it is >> arguably semantically incorrect, even if it might happen to be benign in >> most cases. > > I still need to carry this patch in my trees to have a working dma memory mapping > on SH4 platforms. My understanding from your comment is that there may > be a way forward for this patch, do you still think the same? Have you > got any suggestion on how to improve this eventually for inclusion? As before, the change itself does seem reasonable; it might be worth rewording the commit message in more general terms rather than making it sound like an SH-specific workaround (which I really don't think it is), but otherwise I'd say just repost it as a non-RFC patch. Robin. > > Thanks > j > >> >> Robin. >> >>> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >>> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> >>> --- >>> drivers/base/dma-mapping.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c >>> index e584edd..73d64d3 100644 >>> --- a/drivers/base/dma-mapping.c >>> +++ b/drivers/base/dma-mapping.c >>> @@ -227,8 +227,8 @@ int dma_common_mmap(struct device *dev, struct vm_area_struct *vma, >>> #ifndef CONFIG_ARCH_NO_COHERENT_DMA_MMAP >>> unsigned long user_count = vma_pages(vma); >>> unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT; >>> - unsigned long pfn = page_to_pfn(virt_to_page(cpu_addr)); >>> unsigned long off = vma->vm_pgoff; >>> + unsigned long pfn; >>> >>> vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); >>> >>> @@ -236,6 +236,7 @@ int dma_common_mmap(struct device *dev, struct vm_area_struct *vma, >>> return ret; >>> >>> if (off < count && user_count <= (count - off)) { >>> + pfn = page_to_pfn(virt_to_page(cpu_addr)); >>> ret = remap_pfn_range(vma, vma->vm_start, >>> pfn + off, >>> user_count << PAGE_SHIFT, >>> -- >>> 2.7.4 >>> >>> _______________________________________________ >>> iommu mailing list >>> iommu@lists.linux-foundation.org >>> https://lists.linuxfoundation.org/mailman/listinfo/iommu >>> ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC v2 2/2] base: dma-mapping: Postpone page_to_pfn() on mmap() @ 2018-04-09 11:11 ` Robin Murphy 0 siblings, 0 replies; 18+ messages in thread From: Robin Murphy @ 2018-04-09 11:11 UTC (permalink / raw) To: jacopo mondi Cc: dalias-8zAoT0mYgF4, ysato-Rn4VEauK+AKRv+LV9MX5uooqe+aC9MnS, linux-sh-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, geert-Td1EMuHUCqxL1ZNQvxDV9g, laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw, Jacopo Mondi Hi Jacopo, On 09/04/18 08:25, jacopo mondi wrote: > Hi Robin, Laurent, > a long time passed, sorry about this. > > On Wed, Nov 15, 2017 at 01:38:23PM +0000, Robin Murphy wrote: >> On 14/11/17 17:08, Jacopo Mondi wrote: >>> On SH4 architecture, with SPARSEMEM memory model, translating page to >>> pfn hangs the CPU. Post-pone translation to pfn after >>> dma_mmap_from_dev_coherent() function call as it succeeds and make page >>> translation not necessary. >>> >>> This patch was suggested by Laurent Pinchart and he's working to submit >>> a proper fix mainline. Not sending for inclusion at the moment. >> >> Y'know, I think this patch does have some merit by itself - until we know >> that cpu_addr *doesn't* represent some device-private memory which is not >> guaranteed to be backed by a struct page, calling virt_to_page() on it is >> arguably semantically incorrect, even if it might happen to be benign in >> most cases. > > I still need to carry this patch in my trees to have a working dma memory mapping > on SH4 platforms. My understanding from your comment is that there may > be a way forward for this patch, do you still think the same? Have you > got any suggestion on how to improve this eventually for inclusion? As before, the change itself does seem reasonable; it might be worth rewording the commit message in more general terms rather than making it sound like an SH-specific workaround (which I really don't think it is), but otherwise I'd say just repost it as a non-RFC patch. Robin. > > Thanks > j > >> >> Robin. >> >>> Suggested-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> >>> Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org> >>> --- >>> drivers/base/dma-mapping.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c >>> index e584edd..73d64d3 100644 >>> --- a/drivers/base/dma-mapping.c >>> +++ b/drivers/base/dma-mapping.c >>> @@ -227,8 +227,8 @@ int dma_common_mmap(struct device *dev, struct vm_area_struct *vma, >>> #ifndef CONFIG_ARCH_NO_COHERENT_DMA_MMAP >>> unsigned long user_count = vma_pages(vma); >>> unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT; >>> - unsigned long pfn = page_to_pfn(virt_to_page(cpu_addr)); >>> unsigned long off = vma->vm_pgoff; >>> + unsigned long pfn; >>> >>> vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); >>> >>> @@ -236,6 +236,7 @@ int dma_common_mmap(struct device *dev, struct vm_area_struct *vma, >>> return ret; >>> >>> if (off < count && user_count <= (count - off)) { >>> + pfn = page_to_pfn(virt_to_page(cpu_addr)); >>> ret = remap_pfn_range(vma, vma->vm_start, >>> pfn + off, >>> user_count << PAGE_SHIFT, >>> -- >>> 2.7.4 >>> >>> _______________________________________________ >>> iommu mailing list >>> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org >>> https://lists.linuxfoundation.org/mailman/listinfo/iommu >>> ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC v2 2/2] base: dma-mapping: Postpone page_to_pfn() on mmap() @ 2018-04-09 11:11 ` Robin Murphy 0 siblings, 0 replies; 18+ messages in thread From: Robin Murphy @ 2018-04-09 11:11 UTC (permalink / raw) To: jacopo mondi Cc: Jacopo Mondi, ysato, dalias, laurent.pinchart, geert, linux-renesas-soc, iommu, linux-kernel, linux-sh Hi Jacopo, On 09/04/18 08:25, jacopo mondi wrote: > Hi Robin, Laurent, > a long time passed, sorry about this. > > On Wed, Nov 15, 2017 at 01:38:23PM +0000, Robin Murphy wrote: >> On 14/11/17 17:08, Jacopo Mondi wrote: >>> On SH4 architecture, with SPARSEMEM memory model, translating page to >>> pfn hangs the CPU. Post-pone translation to pfn after >>> dma_mmap_from_dev_coherent() function call as it succeeds and make page >>> translation not necessary. >>> >>> This patch was suggested by Laurent Pinchart and he's working to submit >>> a proper fix mainline. Not sending for inclusion at the moment. >> >> Y'know, I think this patch does have some merit by itself - until we know >> that cpu_addr *doesn't* represent some device-private memory which is not >> guaranteed to be backed by a struct page, calling virt_to_page() on it is >> arguably semantically incorrect, even if it might happen to be benign in >> most cases. > > I still need to carry this patch in my trees to have a working dma memory mapping > on SH4 platforms. My understanding from your comment is that there may > be a way forward for this patch, do you still think the same? Have you > got any suggestion on how to improve this eventually for inclusion? As before, the change itself does seem reasonable; it might be worth rewording the commit message in more general terms rather than making it sound like an SH-specific workaround (which I really don't think it is), but otherwise I'd say just repost it as a non-RFC patch. Robin. > > Thanks > j > >> >> Robin. >> >>> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >>> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> >>> --- >>> drivers/base/dma-mapping.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c >>> index e584edd..73d64d3 100644 >>> --- a/drivers/base/dma-mapping.c >>> +++ b/drivers/base/dma-mapping.c >>> @@ -227,8 +227,8 @@ int dma_common_mmap(struct device *dev, struct vm_area_struct *vma, >>> #ifndef CONFIG_ARCH_NO_COHERENT_DMA_MMAP >>> unsigned long user_count = vma_pages(vma); >>> unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT; >>> - unsigned long pfn = page_to_pfn(virt_to_page(cpu_addr)); >>> unsigned long off = vma->vm_pgoff; >>> + unsigned long pfn; >>> >>> vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); >>> >>> @@ -236,6 +236,7 @@ int dma_common_mmap(struct device *dev, struct vm_area_struct *vma, >>> return ret; >>> >>> if (off < count && user_count <= (count - off)) { >>> + pfn = page_to_pfn(virt_to_page(cpu_addr)); >>> ret = remap_pfn_range(vma, vma->vm_start, >>> pfn + off, >>> user_count << PAGE_SHIFT, >>> -- >>> 2.7.4 >>> >>> _______________________________________________ >>> iommu mailing list >>> iommu@lists.linux-foundation.org >>> https://lists.linuxfoundation.org/mailman/listinfo/iommu >>> ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC v2 2/2] base: dma-mapping: Postpone page_to_pfn() on mmap() 2018-04-09 11:11 ` Robin Murphy @ 2018-04-09 13:06 ` Laurent Pinchart -1 siblings, 0 replies; 18+ messages in thread From: Laurent Pinchart @ 2018-04-09 13:06 UTC (permalink / raw) To: Robin Murphy Cc: jacopo mondi, Jacopo Mondi, ysato, dalias, geert, linux-renesas-soc, iommu, linux-kernel, linux-sh Hello, On Monday, 9 April 2018 14:11:22 EEST Robin Murphy wrote: > On 09/04/18 08:25, jacopo mondi wrote: > > Hi Robin, Laurent, > > > > a long time passed, sorry about this. > > > > On Wed, Nov 15, 2017 at 01:38:23PM +0000, Robin Murphy wrote: > >> On 14/11/17 17:08, Jacopo Mondi wrote: > >>> On SH4 architecture, with SPARSEMEM memory model, translating page to > >>> pfn hangs the CPU. Post-pone translation to pfn after > >>> dma_mmap_from_dev_coherent() function call as it succeeds and make page > >>> translation not necessary. > >>> > >>> This patch was suggested by Laurent Pinchart and he's working to submit > >>> a proper fix mainline. Not sending for inclusion at the moment. > >> > >> Y'know, I think this patch does have some merit by itself - until we know > >> that cpu_addr *doesn't* represent some device-private memory which is not > >> guaranteed to be backed by a struct page, calling virt_to_page() on it is > >> arguably semantically incorrect, even if it might happen to be benign in > >> most cases. > > > > I still need to carry this patch in my trees to have a working dma memory > > mapping on SH4 platforms. My understanding from your comment is that > > there may be a way forward for this patch, do you still think the same? > > Have you got any suggestion on how to improve this eventually for > > inclusion? > > As before, the change itself does seem reasonable; it might be worth > rewording the commit message in more general terms rather than making it > sound like an SH-specific workaround (which I really don't think it is), > but otherwise I'd say just repost it as a non-RFC patch. I actually can't remember any better fix I would have in mind, so this looks good to me :-) I agree with Robin, the commit message should be reworded. Robin's explanation of why virt_to_page() should be postponed until we know that cpu_addr represents memory that is guaranteed to be backed by a struct page is a good starting point. You can mention SH4 as an example of an architecture that will crash when calling virt_to_page() in such a case, but the fix isn't specific to SH4. > >>> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >>> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > >>> --- > >>> > >>> drivers/base/dma-mapping.c | 3 ++- > >>> 1 file changed, 2 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c > >>> index e584edd..73d64d3 100644 > >>> --- a/drivers/base/dma-mapping.c > >>> +++ b/drivers/base/dma-mapping.c > >>> @@ -227,8 +227,8 @@ int dma_common_mmap(struct device *dev, struct > >>> vm_area_struct *vma, > >>> #ifndef CONFIG_ARCH_NO_COHERENT_DMA_MMAP > >>> unsigned long user_count = vma_pages(vma); > >>> unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT; > >>> - unsigned long pfn = page_to_pfn(virt_to_page(cpu_addr)); > >>> unsigned long off = vma->vm_pgoff; > >>> + unsigned long pfn; > >>> > >>> vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); > >>> > >>> @@ -236,6 +236,7 @@ int dma_common_mmap(struct device *dev, struct > >>> vm_area_struct *vma, > >>> return ret; > >>> > >>> if (off < count && user_count <= (count - off)) { > >>> + pfn = page_to_pfn(virt_to_page(cpu_addr)); > >>> ret = remap_pfn_range(vma, vma->vm_start, > >>> pfn + off, > >>> user_count << PAGE_SHIFT, -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC v2 2/2] base: dma-mapping: Postpone page_to_pfn() on mmap() @ 2018-04-09 13:06 ` Laurent Pinchart 0 siblings, 0 replies; 18+ messages in thread From: Laurent Pinchart @ 2018-04-09 13:06 UTC (permalink / raw) To: Robin Murphy Cc: jacopo mondi, Jacopo Mondi, ysato, dalias, geert, linux-renesas-soc, iommu, linux-kernel, linux-sh Hello, On Monday, 9 April 2018 14:11:22 EEST Robin Murphy wrote: > On 09/04/18 08:25, jacopo mondi wrote: > > Hi Robin, Laurent, > > > > a long time passed, sorry about this. > > > > On Wed, Nov 15, 2017 at 01:38:23PM +0000, Robin Murphy wrote: > >> On 14/11/17 17:08, Jacopo Mondi wrote: > >>> On SH4 architecture, with SPARSEMEM memory model, translating page to > >>> pfn hangs the CPU. Post-pone translation to pfn after > >>> dma_mmap_from_dev_coherent() function call as it succeeds and make page > >>> translation not necessary. > >>> > >>> This patch was suggested by Laurent Pinchart and he's working to submit > >>> a proper fix mainline. Not sending for inclusion at the moment. > >> > >> Y'know, I think this patch does have some merit by itself - until we know > >> that cpu_addr *doesn't* represent some device-private memory which is not > >> guaranteed to be backed by a struct page, calling virt_to_page() on it is > >> arguably semantically incorrect, even if it might happen to be benign in > >> most cases. > > > > I still need to carry this patch in my trees to have a working dma memory > > mapping on SH4 platforms. My understanding from your comment is that > > there may be a way forward for this patch, do you still think the same? > > Have you got any suggestion on how to improve this eventually for > > inclusion? > > As before, the change itself does seem reasonable; it might be worth > rewording the commit message in more general terms rather than making it > sound like an SH-specific workaround (which I really don't think it is), > but otherwise I'd say just repost it as a non-RFC patch. I actually can't remember any better fix I would have in mind, so this looks good to me :-) I agree with Robin, the commit message should be reworded. Robin's explanation of why virt_to_page() should be postponed until we know that cpu_addr represents memory that is guaranteed to be backed by a struct page is a good starting point. You can mention SH4 as an example of an architecture that will crash when calling virt_to_page() in such a case, but the fix isn't specific to SH4. > >>> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >>> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > >>> --- > >>> > >>> drivers/base/dma-mapping.c | 3 ++- > >>> 1 file changed, 2 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c > >>> index e584edd..73d64d3 100644 > >>> --- a/drivers/base/dma-mapping.c > >>> +++ b/drivers/base/dma-mapping.c > >>> @@ -227,8 +227,8 @@ int dma_common_mmap(struct device *dev, struct > >>> vm_area_struct *vma, > >>> #ifndef CONFIG_ARCH_NO_COHERENT_DMA_MMAP > >>> unsigned long user_count = vma_pages(vma); > >>> unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT; > >>> - unsigned long pfn = page_to_pfn(virt_to_page(cpu_addr)); > >>> unsigned long off = vma->vm_pgoff; > >>> + unsigned long pfn; > >>> > >>> vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); > >>> > >>> @@ -236,6 +236,7 @@ int dma_common_mmap(struct device *dev, struct > >>> vm_area_struct *vma, > >>> return ret; > >>> > >>> if (off < count && user_count <= (count - off)) { > >>> + pfn = page_to_pfn(virt_to_page(cpu_addr)); > >>> ret = remap_pfn_range(vma, vma->vm_start, > >>> pfn + off, > >>> user_count << PAGE_SHIFT, -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC v2 2/2] base: dma-mapping: Postpone page_to_pfn() on mmap() 2018-04-09 13:06 ` Laurent Pinchart @ 2018-04-09 15:11 ` Rich Felker -1 siblings, 0 replies; 18+ messages in thread From: Rich Felker @ 2018-04-09 15:11 UTC (permalink / raw) To: Laurent Pinchart Cc: Robin Murphy, jacopo mondi, Jacopo Mondi, ysato, geert, linux-renesas-soc, iommu, linux-kernel, linux-sh On Mon, Apr 09, 2018 at 04:06:15PM +0300, Laurent Pinchart wrote: > Hello, > > On Monday, 9 April 2018 14:11:22 EEST Robin Murphy wrote: > > On 09/04/18 08:25, jacopo mondi wrote: > > > Hi Robin, Laurent, > > > > > > a long time passed, sorry about this. > > > > > > On Wed, Nov 15, 2017 at 01:38:23PM +0000, Robin Murphy wrote: > > >> On 14/11/17 17:08, Jacopo Mondi wrote: > > >>> On SH4 architecture, with SPARSEMEM memory model, translating page to > > >>> pfn hangs the CPU. Post-pone translation to pfn after > > >>> dma_mmap_from_dev_coherent() function call as it succeeds and make page > > >>> translation not necessary. > > >>> > > >>> This patch was suggested by Laurent Pinchart and he's working to submit > > >>> a proper fix mainline. Not sending for inclusion at the moment. > > >> > > >> Y'know, I think this patch does have some merit by itself - until we know > > >> that cpu_addr *doesn't* represent some device-private memory which is not > > >> guaranteed to be backed by a struct page, calling virt_to_page() on it is > > >> arguably semantically incorrect, even if it might happen to be benign in > > >> most cases. > > > > > > I still need to carry this patch in my trees to have a working dma memory > > > mapping on SH4 platforms. My understanding from your comment is that > > > there may be a way forward for this patch, do you still think the same? > > > Have you got any suggestion on how to improve this eventually for > > > inclusion? > > > > As before, the change itself does seem reasonable; it might be worth > > rewording the commit message in more general terms rather than making it > > sound like an SH-specific workaround (which I really don't think it is), > > but otherwise I'd say just repost it as a non-RFC patch. > > I actually can't remember any better fix I would have in mind, so this looks > good to me :-) I agree with Robin, the commit message should be reworded. > Robin's explanation of why virt_to_page() should be postponed until we know > that cpu_addr represents memory that is guaranteed to be backed by a struct > page is a good starting point. You can mention SH4 as an example of an > architecture that will crash when calling virt_to_page() in such a case, but > the fix isn't specific to SH4. Just checking since I joined late -- is the consensus that this does not require any action/review specific to SH (in my role as maintainer way behind on maintenance)? Rich ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC v2 2/2] base: dma-mapping: Postpone page_to_pfn() on mmap() @ 2018-04-09 15:11 ` Rich Felker 0 siblings, 0 replies; 18+ messages in thread From: Rich Felker @ 2018-04-09 15:11 UTC (permalink / raw) To: Laurent Pinchart Cc: Robin Murphy, jacopo mondi, Jacopo Mondi, ysato, geert, linux-renesas-soc, iommu, linux-kernel, linux-sh On Mon, Apr 09, 2018 at 04:06:15PM +0300, Laurent Pinchart wrote: > Hello, > > On Monday, 9 April 2018 14:11:22 EEST Robin Murphy wrote: > > On 09/04/18 08:25, jacopo mondi wrote: > > > Hi Robin, Laurent, > > > > > > a long time passed, sorry about this. > > > > > > On Wed, Nov 15, 2017 at 01:38:23PM +0000, Robin Murphy wrote: > > >> On 14/11/17 17:08, Jacopo Mondi wrote: > > >>> On SH4 architecture, with SPARSEMEM memory model, translating page to > > >>> pfn hangs the CPU. Post-pone translation to pfn after > > >>> dma_mmap_from_dev_coherent() function call as it succeeds and make page > > >>> translation not necessary. > > >>> > > >>> This patch was suggested by Laurent Pinchart and he's working to submit > > >>> a proper fix mainline. Not sending for inclusion at the moment. > > >> > > >> Y'know, I think this patch does have some merit by itself - until we know > > >> that cpu_addr *doesn't* represent some device-private memory which is not > > >> guaranteed to be backed by a struct page, calling virt_to_page() on it is > > >> arguably semantically incorrect, even if it might happen to be benign in > > >> most cases. > > > > > > I still need to carry this patch in my trees to have a working dma memory > > > mapping on SH4 platforms. My understanding from your comment is that > > > there may be a way forward for this patch, do you still think the same? > > > Have you got any suggestion on how to improve this eventually for > > > inclusion? > > > > As before, the change itself does seem reasonable; it might be worth > > rewording the commit message in more general terms rather than making it > > sound like an SH-specific workaround (which I really don't think it is), > > but otherwise I'd say just repost it as a non-RFC patch. > > I actually can't remember any better fix I would have in mind, so this looks > good to me :-) I agree with Robin, the commit message should be reworded. > Robin's explanation of why virt_to_page() should be postponed until we know > that cpu_addr represents memory that is guaranteed to be backed by a struct > page is a good starting point. You can mention SH4 as an example of an > architecture that will crash when calling virt_to_page() in such a case, but > the fix isn't specific to SH4. Just checking since I joined late -- is the consensus that this does not require any action/review specific to SH (in my role as maintainer way behind on maintenance)? Rich ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC v2 2/2] base: dma-mapping: Postpone page_to_pfn() on mmap() 2018-04-09 15:11 ` Rich Felker @ 2018-04-09 15:18 ` Laurent Pinchart -1 siblings, 0 replies; 18+ messages in thread From: Laurent Pinchart @ 2018-04-09 15:18 UTC (permalink / raw) To: Rich Felker Cc: Robin Murphy, jacopo mondi, Jacopo Mondi, ysato, geert, linux-renesas-soc, iommu, linux-kernel, linux-sh Hi Rich, On Monday, 9 April 2018 18:11:13 EEST Rich Felker wrote: > On Mon, Apr 09, 2018 at 04:06:15PM +0300, Laurent Pinchart wrote: > > On Monday, 9 April 2018 14:11:22 EEST Robin Murphy wrote: > >> On 09/04/18 08:25, jacopo mondi wrote: > >>> Hi Robin, Laurent, > >>> > >>> a long time passed, sorry about this. > >>> > >>> On Wed, Nov 15, 2017 at 01:38:23PM +0000, Robin Murphy wrote: > >>>> On 14/11/17 17:08, Jacopo Mondi wrote: > >>>>> On SH4 architecture, with SPARSEMEM memory model, translating page > >>>>> to pfn hangs the CPU. Post-pone translation to pfn after > >>>>> dma_mmap_from_dev_coherent() function call as it succeeds and make > >>>>> page translation not necessary. > >>>>> > >>>>> This patch was suggested by Laurent Pinchart and he's working to > >>>>> submit a proper fix mainline. Not sending for inclusion at the > >>>>> moment. > >>>> > >>>> Y'know, I think this patch does have some merit by itself - until we > >>>> know that cpu_addr *doesn't* represent some device-private memory > >>>> which is not guaranteed to be backed by a struct page, calling > >>>> virt_to_page() on it is arguably semantically incorrect, even if it > >>>> might happen to be benign in most cases. > >>> > >>> I still need to carry this patch in my trees to have a working dma > >>> memory mapping on SH4 platforms. My understanding from your comment is > >>> that there may be a way forward for this patch, do you still think the > >>> same? Have you got any suggestion on how to improve this eventually > >>> for inclusion? > >> > >> As before, the change itself does seem reasonable; it might be worth > >> rewording the commit message in more general terms rather than making it > >> sound like an SH-specific workaround (which I really don't think it is), > >> but otherwise I'd say just repost it as a non-RFC patch. > > > > I actually can't remember any better fix I would have in mind, so this > > looks good to me :-) I agree with Robin, the commit message should be > > reworded. Robin's explanation of why virt_to_page() should be postponed > > until we know that cpu_addr represents memory that is guaranteed to be > > backed by a struct page is a good starting point. You can mention SH4 as > > an example of an architecture that will crash when calling virt_to_page() > > in such a case, but the fix isn't specific to SH4. > > Just checking since I joined late -- is the consensus that this does > not require any action/review specific to SH (in my role as maintainer > way behind on maintenance)? I don't think it requires any action specific to SH. If you want to review the patch in the context of SH though, please feel free to do so :-) -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC v2 2/2] base: dma-mapping: Postpone page_to_pfn() on mmap() @ 2018-04-09 15:18 ` Laurent Pinchart 0 siblings, 0 replies; 18+ messages in thread From: Laurent Pinchart @ 2018-04-09 15:18 UTC (permalink / raw) To: Rich Felker Cc: Robin Murphy, jacopo mondi, Jacopo Mondi, ysato, geert, linux-renesas-soc, iommu, linux-kernel, linux-sh Hi Rich, On Monday, 9 April 2018 18:11:13 EEST Rich Felker wrote: > On Mon, Apr 09, 2018 at 04:06:15PM +0300, Laurent Pinchart wrote: > > On Monday, 9 April 2018 14:11:22 EEST Robin Murphy wrote: > >> On 09/04/18 08:25, jacopo mondi wrote: > >>> Hi Robin, Laurent, > >>> > >>> a long time passed, sorry about this. > >>> > >>> On Wed, Nov 15, 2017 at 01:38:23PM +0000, Robin Murphy wrote: > >>>> On 14/11/17 17:08, Jacopo Mondi wrote: > >>>>> On SH4 architecture, with SPARSEMEM memory model, translating page > >>>>> to pfn hangs the CPU. Post-pone translation to pfn after > >>>>> dma_mmap_from_dev_coherent() function call as it succeeds and make > >>>>> page translation not necessary. > >>>>> > >>>>> This patch was suggested by Laurent Pinchart and he's working to > >>>>> submit a proper fix mainline. Not sending for inclusion at the > >>>>> moment. > >>>> > >>>> Y'know, I think this patch does have some merit by itself - until we > >>>> know that cpu_addr *doesn't* represent some device-private memory > >>>> which is not guaranteed to be backed by a struct page, calling > >>>> virt_to_page() on it is arguably semantically incorrect, even if it > >>>> might happen to be benign in most cases. > >>> > >>> I still need to carry this patch in my trees to have a working dma > >>> memory mapping on SH4 platforms. My understanding from your comment is > >>> that there may be a way forward for this patch, do you still think the > >>> same? Have you got any suggestion on how to improve this eventually > >>> for inclusion? > >> > >> As before, the change itself does seem reasonable; it might be worth > >> rewording the commit message in more general terms rather than making it > >> sound like an SH-specific workaround (which I really don't think it is), > >> but otherwise I'd say just repost it as a non-RFC patch. > > > > I actually can't remember any better fix I would have in mind, so this > > looks good to me :-) I agree with Robin, the commit message should be > > reworded. Robin's explanation of why virt_to_page() should be postponed > > until we know that cpu_addr represents memory that is guaranteed to be > > backed by a struct page is a good starting point. You can mention SH4 as > > an example of an architecture that will crash when calling virt_to_page() > > in such a case, but the fix isn't specific to SH4. > > Just checking since I joined late -- is the consensus that this does > not require any action/review specific to SH (in my role as maintainer > way behind on maintenance)? I don't think it requires any action specific to SH. If you want to review the patch in the context of SH though, please feel free to do so :-) -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2018-04-09 15:18 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-11-14 17:08 [PATCH v2 0/2] Migo-R: reserve memory block with memblock APIs Jacopo Mondi 2017-11-14 17:08 ` Jacopo Mondi 2017-11-14 17:08 ` [PATCH v2 1/2] sh: migor: Reserve memory block for CEU Jacopo Mondi 2017-11-14 17:08 ` Jacopo Mondi 2017-11-14 17:08 ` [RFC v2 2/2] base: dma-mapping: Postpone page_to_pfn() on mmap() Jacopo Mondi 2017-11-14 17:08 ` Jacopo Mondi 2017-11-15 13:38 ` Robin Murphy 2018-04-09 7:25 ` jacopo mondi 2018-04-09 7:25 ` jacopo mondi 2018-04-09 11:11 ` Robin Murphy 2018-04-09 11:11 ` Robin Murphy 2018-04-09 11:11 ` Robin Murphy 2018-04-09 13:06 ` Laurent Pinchart 2018-04-09 13:06 ` Laurent Pinchart 2018-04-09 15:11 ` Rich Felker 2018-04-09 15:11 ` Rich Felker 2018-04-09 15:18 ` Laurent Pinchart 2018-04-09 15:18 ` Laurent Pinchart
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.