Re: [PATCH] arm: dma-mapping: Fix mapping size value
Hello, On 2014-04-22 10:53, Will Deacon wrote: On Mon, Apr 21, 2014 at 07:47:27AM +0100, Ritesh Harjani wrote: 68efd7d2fb(arm: dma-mapping: remove order parameter from arm_iommu_create_mapping()) is causing kernel panic because it wrongly sets the value of mapping-size: Unable to handle kernel NULL pointer dereference at virtual address 00a0 pgd = e7a84000 [00a0] *pgd= ... PC is at bitmap_clear+0x48/0xd0 LR is at __iommu_remove_mapping+0x130/0x164 Fix it by correcting mapping-size value. Signed-off-by: Ritesh Harjani ritesh.harj...@gmail.com Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- arch/arm/mm/dma-mapping.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index f62aa06..6b00be1 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -1963,8 +1963,8 @@ arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t size) mapping-nr_bitmaps = 1; mapping-extensions = extensions; mapping-base = base; - mapping-size = bitmap_size PAGE_SHIFT; mapping-bits = BITS_PER_BYTE * bitmap_size; + mapping-size = mapping-bits PAGE_SHIFT; Ok, but given that mapping-size is derived from mapping-bits, do we really need both of these fields in struct dma_iommu_mapping? You are right. I didn't notice this while I was refactoring the code. Ritesh, could you update your patch and simply replace all references of mapping-size with (mapping-bits PAGE_SHIFT), probably with some temporary variable to make the code easier to understand? I've didn't apply your patch yet. Best regards -- Marek Szyprowski, PhD Samsung RD Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] arm: dma-mapping: Fix mapping size value
Hi Marek/Will On Wed, Apr 23, 2014 at 3:00 PM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: Hi Marek, On Wednesday 23 April 2014 10:53:24 Marek Szyprowski wrote: On 2014-04-22 10:53, Will Deacon wrote: On Mon, Apr 21, 2014 at 07:47:27AM +0100, Ritesh Harjani wrote: 68efd7d2fb(arm: dma-mapping: remove order parameter from arm_iommu_create_mapping()) is causing kernel panic because it wrongly sets the value of mapping-size: Unable to handle kernel NULL pointer dereference at virtual address 00a0 pgd = e7a84000 [00a0] *pgd= ... PC is at bitmap_clear+0x48/0xd0 LR is at __iommu_remove_mapping+0x130/0x164 Fix it by correcting mapping-size value. Signed-off-by: Ritesh Harjani ritesh.harj...@gmail.com Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- arch/arm/mm/dma-mapping.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index f62aa06..6b00be1 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -1963,8 +1963,8 @@ arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t size) mapping-nr_bitmaps = 1; mapping-extensions = extensions; mapping-base = base; - mapping-size = bitmap_size PAGE_SHIFT; mapping-bits = BITS_PER_BYTE * bitmap_size; + mapping-size = mapping-bits PAGE_SHIFT; Ok, but given that mapping-size is derived from mapping-bits, do we really need both of these fields in struct dma_iommu_mapping? You are right. I didn't notice this while I was refactoring the code. Ritesh, could you update your patch and simply replace all references of mapping-size with (mapping-bits PAGE_SHIFT), probably with some temporary variable to make the code easier to understand? I've didn't apply your patch yet. As this patch fixes a v3.15 regression, shouldn't it be applied as-is and ASAP, with the cleanup that removes mapping-size coming in a later, less urgent patch ? I agree with Laurent. Anyway this cleanup can be taken care when we will be doing refactoring of common code to lib/iommu-helper.c. Anyways, if you still insist I can prepare and submit the patch. Let me know again on this. -- Regards, Laurent Pinchart Thanks Ritesh ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] arm: dma-mapping: Fix mapping size value
Hello, On 2014-04-23 12:04, Ritesh Harjani wrote: Hi Marek/Will On Wed, Apr 23, 2014 at 3:00 PM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: Hi Marek, On Wednesday 23 April 2014 10:53:24 Marek Szyprowski wrote: On 2014-04-22 10:53, Will Deacon wrote: On Mon, Apr 21, 2014 at 07:47:27AM +0100, Ritesh Harjani wrote: 68efd7d2fb(arm: dma-mapping: remove order parameter from arm_iommu_create_mapping()) is causing kernel panic because it wrongly sets the value of mapping-size: Unable to handle kernel NULL pointer dereference at virtual address 00a0 pgd = e7a84000 [00a0] *pgd= ... PC is at bitmap_clear+0x48/0xd0 LR is at __iommu_remove_mapping+0x130/0x164 Fix it by correcting mapping-size value. Signed-off-by: Ritesh Harjani ritesh.harj...@gmail.com Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- arch/arm/mm/dma-mapping.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index f62aa06..6b00be1 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -1963,8 +1963,8 @@ arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t size) mapping-nr_bitmaps = 1; mapping-extensions = extensions; mapping-base = base; - mapping-size = bitmap_size PAGE_SHIFT; mapping-bits = BITS_PER_BYTE * bitmap_size; + mapping-size = mapping-bits PAGE_SHIFT; Ok, but given that mapping-size is derived from mapping-bits, do we really need both of these fields in struct dma_iommu_mapping? You are right. I didn't notice this while I was refactoring the code. Ritesh, could you update your patch and simply replace all references of mapping-size with (mapping-bits PAGE_SHIFT), probably with some temporary variable to make the code easier to understand? I've didn't apply your patch yet. As this patch fixes a v3.15 regression, shouldn't it be applied as-is and ASAP, with the cleanup that removes mapping-size coming in a later, less urgent patch ? I agree with Laurent. Anyway this cleanup can be taken care when we will be doing refactoring of common code to lib/iommu-helper.c. Anyways, if you still insist I can prepare and submit the patch. Let me know again on this. Ok, I've merged the patch as is and I will send pull request soon. Please include the above discussed cleanup while refactoring common code to lib. Best regards -- Marek Szyprowski, PhD Samsung RD Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] arm: dma-mapping: Fix mapping size value
Ok thanks Marek, I was about to send a new patch (as I had now got hold of my system). Anyways, I will add this discussion of cleaning up this variable in my to-do list. Thanks Ritesh On Wed, Apr 23, 2014 at 6:47 PM, Marek Szyprowski m.szyprow...@samsung.com wrote: Hello, On 2014-04-23 12:04, Ritesh Harjani wrote: Hi Marek/Will On Wed, Apr 23, 2014 at 3:00 PM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: Hi Marek, On Wednesday 23 April 2014 10:53:24 Marek Szyprowski wrote: On 2014-04-22 10:53, Will Deacon wrote: On Mon, Apr 21, 2014 at 07:47:27AM +0100, Ritesh Harjani wrote: 68efd7d2fb(arm: dma-mapping: remove order parameter from arm_iommu_create_mapping()) is causing kernel panic because it wrongly sets the value of mapping-size: Unable to handle kernel NULL pointer dereference at virtual address 00a0 pgd = e7a84000 [00a0] *pgd= ... PC is at bitmap_clear+0x48/0xd0 LR is at __iommu_remove_mapping+0x130/0x164 Fix it by correcting mapping-size value. Signed-off-by: Ritesh Harjani ritesh.harj...@gmail.com Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- arch/arm/mm/dma-mapping.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index f62aa06..6b00be1 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -1963,8 +1963,8 @@ arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t size) mapping-nr_bitmaps = 1; mapping-extensions = extensions; mapping-base = base; - mapping-size = bitmap_size PAGE_SHIFT; mapping-bits = BITS_PER_BYTE * bitmap_size; + mapping-size = mapping-bits PAGE_SHIFT; Ok, but given that mapping-size is derived from mapping-bits, do we really need both of these fields in struct dma_iommu_mapping? You are right. I didn't notice this while I was refactoring the code. Ritesh, could you update your patch and simply replace all references of mapping-size with (mapping-bits PAGE_SHIFT), probably with some temporary variable to make the code easier to understand? I've didn't apply your patch yet. As this patch fixes a v3.15 regression, shouldn't it be applied as-is and ASAP, with the cleanup that removes mapping-size coming in a later, less urgent patch ? I agree with Laurent. Anyway this cleanup can be taken care when we will be doing refactoring of common code to lib/iommu-helper.c. Anyways, if you still insist I can prepare and submit the patch. Let me know again on this. Ok, I've merged the patch as is and I will send pull request soon. Please include the above discussed cleanup while refactoring common code to lib. Best regards -- Marek Szyprowski, PhD Samsung RD Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
