Re: [PATCH v2 14/14] powerpc/pseries/iommu: Rename "direct window" to "dma window"

2021-04-13 Thread Leonardo Bras
On Wed, 2020-09-30 at 17:29 +1000, Alexey Kardashevskiy wrote:
> 
> On 30/09/2020 06:54, Leonardo Bras wrote:
> > On Tue, 2020-09-29 at 13:55 +1000, Alexey Kardashevskiy wrote:
> > > 
> > > On 12/09/2020 03:07, Leonardo Bras wrote:
> > > > Cc: linuxppc-dev@lists.ozlabs.org, linux-ker...@vger.kernel.org,
> > > > 
> > > > A previous change introduced the usage of DDW as a bigger indirect DMA
> > > > mapping when the DDW available size does not map the whole partition.
> > > > 
> > > > As most of the code that manipulates direct mappings was reused for
> > > > indirect mappings, it's necessary to rename all names and debug/info
> > > > messages to reflect that it can be used for both kinds of mapping.
> > > > 
> > > > Also, defines DEFAULT_DMA_WIN as "ibm,dma-window" to document that
> > > > it's the name of the default DMA window.
> > > 
> > > "ibm,dma-window" is so old so it does not need a macro (which btw would
> > > be DMA_WIN_PROPNAME to match the other names) :)
> > 
> > Thanks for bringing that to my attention!
> > In fact, DMA_WIN_PROPNAME makes more sense, but it's still generic and
> > doesn't look to point to a generic one.
> > 
> > Would that be ok to call it DEFAULT_WIN_PROPNAME ?
> 
> 
> I would not touch it at all, the property name is painfully known and 
> not going to change ever. Does anyone else define it as a macro? I do 
> not see any:

Ok then, reverting define :)

Thanks!

> 
> [fstn1-p1 kernel-dma-bypass]$ git grep "ibm,dma-window"  | wc -l
> 8
> [fstn1-p1 kernel-dma-bypass]$ git grep "define.*ibm,dma-window"  | wc -l
> 0
> 
> 
> 
> > 
> > 
> > > 
> > > 
> > > > Those changes are not supposed to change how the code works in any
> > > > way, just adjust naming.
> > > 
> > > I simply have this in my .vimrc for the cases like this one:
> > > 
> > > ===
> > > This should cause no behavioural change.
> > > ===
> > 
> > Great tip! I will make sure to have this saved here :)
> > 
> > Thank you!
> > 
> 




Re: [PATCH v2 14/14] powerpc/pseries/iommu: Rename "direct window" to "dma window"

2020-09-30 Thread Alexey Kardashevskiy




On 30/09/2020 06:54, Leonardo Bras wrote:

On Tue, 2020-09-29 at 13:55 +1000, Alexey Kardashevskiy wrote:


On 12/09/2020 03:07, Leonardo Bras wrote:

Cc: linuxppc-dev@lists.ozlabs.org, linux-ker...@vger.kernel.org,

A previous change introduced the usage of DDW as a bigger indirect DMA
mapping when the DDW available size does not map the whole partition.

As most of the code that manipulates direct mappings was reused for
indirect mappings, it's necessary to rename all names and debug/info
messages to reflect that it can be used for both kinds of mapping.

Also, defines DEFAULT_DMA_WIN as "ibm,dma-window" to document that
it's the name of the default DMA window.


"ibm,dma-window" is so old so it does not need a macro (which btw would
be DMA_WIN_PROPNAME to match the other names) :)


Thanks for bringing that to my attention!
In fact, DMA_WIN_PROPNAME makes more sense, but it's still generic and
doesn't look to point to a generic one.

Would that be ok to call it DEFAULT_WIN_PROPNAME ?



I would not touch it at all, the property name is painfully known and 
not going to change ever. Does anyone else define it as a macro? I do 
not see any:


[fstn1-p1 kernel-dma-bypass]$ git grep "ibm,dma-window"  | wc -l
8
[fstn1-p1 kernel-dma-bypass]$ git grep "define.*ibm,dma-window"  | wc -l
0










Those changes are not supposed to change how the code works in any
way, just adjust naming.


I simply have this in my .vimrc for the cases like this one:

===
This should cause no behavioural change.
===


Great tip! I will make sure to have this saved here :)

Thank you!



--
Alexey


Re: [PATCH v2 14/14] powerpc/pseries/iommu: Rename "direct window" to "dma window"

2020-09-29 Thread Leonardo Bras
On Tue, 2020-09-29 at 13:55 +1000, Alexey Kardashevskiy wrote:
> 
> On 12/09/2020 03:07, Leonardo Bras wrote:
> > Cc: linuxppc-dev@lists.ozlabs.org, linux-ker...@vger.kernel.org,
> > 
> > A previous change introduced the usage of DDW as a bigger indirect DMA
> > mapping when the DDW available size does not map the whole partition.
> > 
> > As most of the code that manipulates direct mappings was reused for
> > indirect mappings, it's necessary to rename all names and debug/info
> > messages to reflect that it can be used for both kinds of mapping.
> > 
> > Also, defines DEFAULT_DMA_WIN as "ibm,dma-window" to document that
> > it's the name of the default DMA window.
> 
> "ibm,dma-window" is so old so it does not need a macro (which btw would 
> be DMA_WIN_PROPNAME to match the other names) :)

Thanks for bringing that to my attention!
In fact, DMA_WIN_PROPNAME makes more sense, but it's still generic and
doesn't look to point to a generic one.

Would that be ok to call it DEFAULT_WIN_PROPNAME ?


> 
> 
> > Those changes are not supposed to change how the code works in any
> > way, just adjust naming.
> 
> I simply have this in my .vimrc for the cases like this one:
> 
> ===
> This should cause no behavioural change.
> ===

Great tip! I will make sure to have this saved here :)

Thank you!



Re: [PATCH v2 14/14] powerpc/pseries/iommu: Rename "direct window" to "dma window"

2020-09-28 Thread Alexey Kardashevskiy




On 12/09/2020 03:07, Leonardo Bras wrote:

Cc: linuxppc-dev@lists.ozlabs.org, linux-ker...@vger.kernel.org,

A previous change introduced the usage of DDW as a bigger indirect DMA
mapping when the DDW available size does not map the whole partition.

As most of the code that manipulates direct mappings was reused for
indirect mappings, it's necessary to rename all names and debug/info
messages to reflect that it can be used for both kinds of mapping.

Also, defines DEFAULT_DMA_WIN as "ibm,dma-window" to document that
it's the name of the default DMA window.


"ibm,dma-window" is so old so it does not need a macro (which btw would 
be DMA_WIN_PROPNAME to match the other names) :)




Those changes are not supposed to change how the code works in any
way, just adjust naming.


I simply have this in my .vimrc for the cases like this one:

===
This should cause no behavioural change.
===




Signed-off-by: Leonardo Bras 
---
  arch/powerpc/platforms/pseries/iommu.c | 102 +
  1 file changed, 53 insertions(+), 49 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/iommu.c 
b/arch/powerpc/platforms/pseries/iommu.c
index c4de23080d1b..56638b7f07fc 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -349,7 +349,7 @@ struct dynamic_dma_window_prop {
__be32  window_shift;   /* ilog2(tce_window_size) */
  };
  
-struct direct_window {

+struct dma_win {
struct device_node *device;
const struct dynamic_dma_window_prop *prop;
struct list_head list;
@@ -369,13 +369,14 @@ struct ddw_create_response {
u32 addr_lo;
  };
  
-static LIST_HEAD(direct_window_list);

+static LIST_HEAD(dma_win_list);
  /* prevents races between memory on/offline and window creation */
-static DEFINE_SPINLOCK(direct_window_list_lock);
+static DEFINE_SPINLOCK(dma_win_list_lock);
  /* protects initializing window twice for same device */
-static DEFINE_MUTEX(direct_window_init_mutex);
+static DEFINE_MUTEX(dma_win_init_mutex);
  #define DIRECT64_PROPNAME "linux,direct64-ddr-window-info"
  #define DMA64_PROPNAME "linux,dma64-ddr-window-info"
+#define DEFAULT_DMA_WIN "ibm,dma-window"
  
  static int tce_clearrange_multi_pSeriesLP(unsigned long start_pfn,

unsigned long num_pfn, const void *arg)
@@ -706,15 +707,18 @@ static void pci_dma_bus_setup_pSeriesLP(struct pci_bus 
*bus)
pr_debug("pci_dma_bus_setup_pSeriesLP: setting up bus %pOF\n",
 dn);
  
-	/* Find nearest ibm,dma-window, walking up the device tree */

+   /*
+* Find nearest ibm,dma-window (default DMA window), walking up the
+* device tree
+*/
for (pdn = dn; pdn != NULL; pdn = pdn->parent) {
-   dma_window = of_get_property(pdn, "ibm,dma-window", NULL);
+   dma_window = of_get_property(pdn, DEFAULT_DMA_WIN, NULL);
if (dma_window != NULL)
break;
}
  
  	if (dma_window == NULL) {

-   pr_debug("  no ibm,dma-window property !\n");
+   pr_debug("  no %s property !\n", DEFAULT_DMA_WIN);
return;
}
  
@@ -810,11 +814,11 @@ static void remove_dma_window(struct device_node *np, u32 *ddw_avail,
  
  	ret = rtas_call(ddw_avail[DDW_REMOVE_PE_DMA_WIN], 1, 1, NULL, liobn);

if (ret)
-   pr_warn("%pOF: failed to remove direct window: rtas returned "
+   pr_warn("%pOF: failed to remove dma window: rtas returned "
"%d to ibm,remove-pe-dma-window(%x) %llx\n",
np, ret, ddw_avail[DDW_REMOVE_PE_DMA_WIN], liobn);
else
-   pr_debug("%pOF: successfully removed direct window: rtas returned 
"
+   pr_debug("%pOF: successfully removed dma window: rtas returned "



s/dma/DMA/ in all user visible strings.




"%d to ibm,remove-pe-dma-window(%x) %llx\n",
np, ret, ddw_avail[DDW_REMOVE_PE_DMA_WIN], liobn);
  }
@@ -842,7 +846,7 @@ static int remove_ddw(struct device_node *np, bool 
remove_prop, const char *win_
  
  	ret = of_remove_property(np, win);

if (ret)
-   pr_warn("%pOF: failed to remove direct window property: %d\n",
+   pr_warn("%pOF: failed to remove dma window property: %d\n",
np, ret);
return 0;
  }
@@ -886,34 +890,34 @@ static phys_addr_t ddw_memory_hotplug_max(void)
  
  static bool find_existing_ddw(struct device_node *pdn, u64 *dma_addr, bool *direct_mapping)

  {
-   struct direct_window *window;
-   const struct dynamic_dma_window_prop *direct64;
+   struct dma_win *window;
+   const struct dynamic_dma_window_prop *dma64;
unsigned long window_size;
bool found = false;
  
-	spin_lock(_window_list_lock);

+   spin_lock(_win_list_lock);
/* check if we already created a window and dupe that config if so */
-  

[PATCH v2 14/14] powerpc/pseries/iommu: Rename "direct window" to "dma window"

2020-09-11 Thread Leonardo Bras
Cc: linuxppc-dev@lists.ozlabs.org, linux-ker...@vger.kernel.org, 

A previous change introduced the usage of DDW as a bigger indirect DMA
mapping when the DDW available size does not map the whole partition.

As most of the code that manipulates direct mappings was reused for
indirect mappings, it's necessary to rename all names and debug/info
messages to reflect that it can be used for both kinds of mapping.

Also, defines DEFAULT_DMA_WIN as "ibm,dma-window" to document that
it's the name of the default DMA window.

Those changes are not supposed to change how the code works in any
way, just adjust naming.

Signed-off-by: Leonardo Bras 
---
 arch/powerpc/platforms/pseries/iommu.c | 102 +
 1 file changed, 53 insertions(+), 49 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/iommu.c 
b/arch/powerpc/platforms/pseries/iommu.c
index c4de23080d1b..56638b7f07fc 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -349,7 +349,7 @@ struct dynamic_dma_window_prop {
__be32  window_shift;   /* ilog2(tce_window_size) */
 };
 
-struct direct_window {
+struct dma_win {
struct device_node *device;
const struct dynamic_dma_window_prop *prop;
struct list_head list;
@@ -369,13 +369,14 @@ struct ddw_create_response {
u32 addr_lo;
 };
 
-static LIST_HEAD(direct_window_list);
+static LIST_HEAD(dma_win_list);
 /* prevents races between memory on/offline and window creation */
-static DEFINE_SPINLOCK(direct_window_list_lock);
+static DEFINE_SPINLOCK(dma_win_list_lock);
 /* protects initializing window twice for same device */
-static DEFINE_MUTEX(direct_window_init_mutex);
+static DEFINE_MUTEX(dma_win_init_mutex);
 #define DIRECT64_PROPNAME "linux,direct64-ddr-window-info"
 #define DMA64_PROPNAME "linux,dma64-ddr-window-info"
+#define DEFAULT_DMA_WIN "ibm,dma-window"
 
 static int tce_clearrange_multi_pSeriesLP(unsigned long start_pfn,
unsigned long num_pfn, const void *arg)
@@ -706,15 +707,18 @@ static void pci_dma_bus_setup_pSeriesLP(struct pci_bus 
*bus)
pr_debug("pci_dma_bus_setup_pSeriesLP: setting up bus %pOF\n",
 dn);
 
-   /* Find nearest ibm,dma-window, walking up the device tree */
+   /*
+* Find nearest ibm,dma-window (default DMA window), walking up the
+* device tree
+*/
for (pdn = dn; pdn != NULL; pdn = pdn->parent) {
-   dma_window = of_get_property(pdn, "ibm,dma-window", NULL);
+   dma_window = of_get_property(pdn, DEFAULT_DMA_WIN, NULL);
if (dma_window != NULL)
break;
}
 
if (dma_window == NULL) {
-   pr_debug("  no ibm,dma-window property !\n");
+   pr_debug("  no %s property !\n", DEFAULT_DMA_WIN);
return;
}
 
@@ -810,11 +814,11 @@ static void remove_dma_window(struct device_node *np, u32 
*ddw_avail,
 
ret = rtas_call(ddw_avail[DDW_REMOVE_PE_DMA_WIN], 1, 1, NULL, liobn);
if (ret)
-   pr_warn("%pOF: failed to remove direct window: rtas returned "
+   pr_warn("%pOF: failed to remove dma window: rtas returned "
"%d to ibm,remove-pe-dma-window(%x) %llx\n",
np, ret, ddw_avail[DDW_REMOVE_PE_DMA_WIN], liobn);
else
-   pr_debug("%pOF: successfully removed direct window: rtas 
returned "
+   pr_debug("%pOF: successfully removed dma window: rtas returned "
"%d to ibm,remove-pe-dma-window(%x) %llx\n",
np, ret, ddw_avail[DDW_REMOVE_PE_DMA_WIN], liobn);
 }
@@ -842,7 +846,7 @@ static int remove_ddw(struct device_node *np, bool 
remove_prop, const char *win_
 
ret = of_remove_property(np, win);
if (ret)
-   pr_warn("%pOF: failed to remove direct window property: %d\n",
+   pr_warn("%pOF: failed to remove dma window property: %d\n",
np, ret);
return 0;
 }
@@ -886,34 +890,34 @@ static phys_addr_t ddw_memory_hotplug_max(void)
 
 static bool find_existing_ddw(struct device_node *pdn, u64 *dma_addr, bool 
*direct_mapping)
 {
-   struct direct_window *window;
-   const struct dynamic_dma_window_prop *direct64;
+   struct dma_win *window;
+   const struct dynamic_dma_window_prop *dma64;
unsigned long window_size;
bool found = false;
 
-   spin_lock(_window_list_lock);
+   spin_lock(_win_list_lock);
/* check if we already created a window and dupe that config if so */
-   list_for_each_entry(window, _window_list, list) {
+   list_for_each_entry(window, _win_list, list) {
if (window->device == pdn) {
-   direct64 = window->prop;
-   *dma_addr = be64_to_cpu(direct64->dma_base);
+   dma64 = window->prop;
+