* [PATCH] ppc/iommu: prevent false TCE leak message
@ 2012-12-27 16:28 Thadeu Lima de Souza Cascardo
2012-12-28 5:21 ` Gavin Shan
0 siblings, 1 reply; 5+ messages in thread
From: Thadeu Lima de Souza Cascardo @ 2012-12-27 16:28 UTC (permalink / raw)
To: linuxppc-dev; +Cc: paulus, anton, Thadeu Lima de Souza Cascardo
When a device DMA window includes the address 0, it's reserved in the
TCE bitmap to avoid returning that address to drivers.
When the device is removed, the bitmap is checked for any mappings not
removed by the driver, indicating a possible DMA mapping leak. Since the
reserved address is not cleared, a message is printed, warning of such a
leak.
Check for the reservation, and clear it before checking for any other
standing mappings.
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
---
arch/powerpc/kernel/iommu.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index 8226c6c..226e9e5 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -717,6 +717,11 @@ void iommu_free_table(struct iommu_table *tbl, const char *node_name)
return;
}
+ /* In case we have reserved the first bit, we should not emit
+ * the warning below. */
+ if (tbl->it_offset == 0)
+ clear_bit(0, tbl->it_map);
+
/* verify that table contains no entries */
/* it_size is in entries, and we're examining 64 at a time */
for (i = 0; i < (tbl->it_size/64); i++) {
--
1.7.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ppc/iommu: prevent false TCE leak message
2012-12-27 16:28 [PATCH] ppc/iommu: prevent false TCE leak message Thadeu Lima de Souza Cascardo
@ 2012-12-28 5:21 ` Gavin Shan
2012-12-28 16:55 ` Thadeu Lima de Souza Cascardo
0 siblings, 1 reply; 5+ messages in thread
From: Gavin Shan @ 2012-12-28 5:21 UTC (permalink / raw)
To: Thadeu Lima de Souza Cascardo; +Cc: paulus, linuxppc-dev, anton
On Thu, Dec 27, 2012 at 02:28:06PM -0200, Thadeu Lima de Souza Cascardo wrote:
>When a device DMA window includes the address 0, it's reserved in the
>TCE bitmap to avoid returning that address to drivers.
>
>When the device is removed, the bitmap is checked for any mappings not
>removed by the driver, indicating a possible DMA mapping leak. Since the
>reserved address is not cleared, a message is printed, warning of such a
>leak.
>
>Check for the reservation, and clear it before checking for any other
>standing mappings.
>
>Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
>---
> arch/powerpc/kernel/iommu.c | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
>diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
>index 8226c6c..226e9e5 100644
>--- a/arch/powerpc/kernel/iommu.c
>+++ b/arch/powerpc/kernel/iommu.c
>@@ -717,6 +717,11 @@ void iommu_free_table(struct iommu_table *tbl, const char *node_name)
> return;
> }
>
>+ /* In case we have reserved the first bit, we should not emit
>+ * the warning below. */
At least, the comment would look like:
/*
* xxxxxxx
*/
>+ if (tbl->it_offset == 0)
>+ clear_bit(0, tbl->it_map);
>+
> /* verify that table contains no entries */
> /* it_size is in entries, and we're examining 64 at a time */
The comment would be merged as well? :-)
> for (i = 0; i < (tbl->it_size/64); i++) {
Thanks,
Gavin
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ppc/iommu: prevent false TCE leak message
2012-12-28 5:21 ` Gavin Shan
@ 2012-12-28 16:55 ` Thadeu Lima de Souza Cascardo
2012-12-28 19:08 ` [PATCH 1/2] " Thadeu Lima de Souza Cascardo
2012-12-28 19:08 ` [PATCH 2/2] ppc/iommu: use find_first_bit to look up entries in the iommu table Thadeu Lima de Souza Cascardo
0 siblings, 2 replies; 5+ messages in thread
From: Thadeu Lima de Souza Cascardo @ 2012-12-28 16:55 UTC (permalink / raw)
To: Gavin Shan; +Cc: paulus, linuxppc-dev, anton
On Fri, Dec 28, 2012 at 01:21:35PM +0800, Gavin Shan wrote:
> On Thu, Dec 27, 2012 at 02:28:06PM -0200, Thadeu Lima de Souza Cascardo wrote:
> >When a device DMA window includes the address 0, it's reserved in the
> >TCE bitmap to avoid returning that address to drivers.
> >
> >When the device is removed, the bitmap is checked for any mappings not
> >removed by the driver, indicating a possible DMA mapping leak. Since the
> >reserved address is not cleared, a message is printed, warning of such a
> >leak.
> >
> >Check for the reservation, and clear it before checking for any other
> >standing mappings.
> >
> >Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
> >---
> > arch/powerpc/kernel/iommu.c | 5 +++++
> > 1 files changed, 5 insertions(+), 0 deletions(-)
> >
> >diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
> >index 8226c6c..226e9e5 100644
> >--- a/arch/powerpc/kernel/iommu.c
> >+++ b/arch/powerpc/kernel/iommu.c
> >@@ -717,6 +717,11 @@ void iommu_free_table(struct iommu_table *tbl, const char *node_name)
> > return;
> > }
> >
> >+ /* In case we have reserved the first bit, we should not emit
> >+ * the warning below. */
>
> At least, the comment would look like:
>
> /*
> * xxxxxxx
> */
>
Sure. New code should always follow coding style. :-)
How do you suggest merging with the comment below? I think it's closer
to the code it comments about, so I'd rather keep it where it is.
> >+ if (tbl->it_offset == 0)
> >+ clear_bit(0, tbl->it_map);
> >+
> > /* verify that table contains no entries */
> > /* it_size is in entries, and we're examining 64 at a time */
>
> The comment would be merged as well? :-)
>
I also considered replacing this code by this:
/* verify that table contains no entries */
- /* it_size is in entries, and we're examining 64 at a time */
- for (i = 0; i < (tbl->it_size/64); i++) {
- if (tbl->it_map[i] != 0) {
+ if (find_first_bit(tbl->it_map, tbl->it_size) < tbl->it_size)
printk(KERN_WARNING "%s: Unexpected TCEs for
%s\n",
__func__, node_name);
- break;
- }
- }
I'll resend the "unreserving" patch with the fixed comment styling, but
without merging comments. And I will send this other patch for comments.
Regards.
Thadeu Cascardo.
> > for (i = 0; i < (tbl->it_size/64); i++) {
>
> Thanks,
> Gavin
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] ppc/iommu: prevent false TCE leak message
2012-12-28 16:55 ` Thadeu Lima de Souza Cascardo
@ 2012-12-28 19:08 ` Thadeu Lima de Souza Cascardo
2012-12-28 19:08 ` [PATCH 2/2] ppc/iommu: use find_first_bit to look up entries in the iommu table Thadeu Lima de Souza Cascardo
1 sibling, 0 replies; 5+ messages in thread
From: Thadeu Lima de Souza Cascardo @ 2012-12-28 19:08 UTC (permalink / raw)
To: linuxppc-dev; +Cc: paulus, shangw, Thadeu Lima de Souza Cascardo, anton
When a device DMA window includes the address 0, it's reserved in the
TCE bitmap to avoid returning that address to drivers.
When the device is removed, the bitmap is checked for any mappings not
removed by the driver, indicating a possible DMA mapping leak. Since the
reserved address is not cleared, a message is printed, warning of such a
leak.
Check for the reservation, and clear it before checking for any other
standing mappings.
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
---
arch/powerpc/kernel/iommu.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index 8226c6c..6d48ff8 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -717,6 +717,13 @@ void iommu_free_table(struct iommu_table *tbl, const char *node_name)
return;
}
+ /*
+ * In case we have reserved the first bit, we should not emit
+ * the warning below.
+ */
+ if (tbl->it_offset == 0)
+ clear_bit(0, tbl->it_map);
+
/* verify that table contains no entries */
/* it_size is in entries, and we're examining 64 at a time */
for (i = 0; i < (tbl->it_size/64); i++) {
--
1.7.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] ppc/iommu: use find_first_bit to look up entries in the iommu table
2012-12-28 16:55 ` Thadeu Lima de Souza Cascardo
2012-12-28 19:08 ` [PATCH 1/2] " Thadeu Lima de Souza Cascardo
@ 2012-12-28 19:08 ` Thadeu Lima de Souza Cascardo
1 sibling, 0 replies; 5+ messages in thread
From: Thadeu Lima de Souza Cascardo @ 2012-12-28 19:08 UTC (permalink / raw)
To: linuxppc-dev; +Cc: paulus, shangw, Thadeu Lima de Souza Cascardo, anton
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
---
arch/powerpc/kernel/iommu.c | 7 +------
1 files changed, 1 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index 6d48ff8..91e2b99 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -725,14 +725,9 @@ void iommu_free_table(struct iommu_table *tbl, const char *node_name)
clear_bit(0, tbl->it_map);
/* verify that table contains no entries */
- /* it_size is in entries, and we're examining 64 at a time */
- for (i = 0; i < (tbl->it_size/64); i++) {
- if (tbl->it_map[i] != 0) {
+ if (find_first_bit(tbl->it_map, tbl->it_size) < tbl->it_size)
printk(KERN_WARNING "%s: Unexpected TCEs for %s\n",
__func__, node_name);
- break;
- }
- }
/* calculate bitmap size in bytes */
bitmap_sz = (tbl->it_size + 7) / 8;
--
1.7.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-12-28 19:09 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-27 16:28 [PATCH] ppc/iommu: prevent false TCE leak message Thadeu Lima de Souza Cascardo
2012-12-28 5:21 ` Gavin Shan
2012-12-28 16:55 ` Thadeu Lima de Souza Cascardo
2012-12-28 19:08 ` [PATCH 1/2] " Thadeu Lima de Souza Cascardo
2012-12-28 19:08 ` [PATCH 2/2] ppc/iommu: use find_first_bit to look up entries in the iommu table Thadeu Lima de Souza Cascardo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).