Re: [Xen-devel] [PATCH 1/5] ARM: ITS: Introduce common function add_to_host_its_list

[Julien Grall]

On 13/08/17 22:30, mja...@caviumnetworks.com wrote:

From: Manish Jaggi 

add_to_host_its_list will update the host_its_list. This common function to
be invoked from gicv3_its_dt_init and gic_v3_its_acpi_init.

Signed-off-by: Manish Jaggi 
---
 xen/arch/arm/gic-v3-its.c | 36 +++-
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index 2d36030..f844a0d 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -976,12 +976,31 @@ int gicv3_its_make_hwdom_dt_nodes(const struct domain *d,
 return res;
 }

+/* Common function for adding to host_its_list */
+static int add_to_host_its_list(u64 addr, u64 size, const void *node)


BTW this should be paddr_t and not u64 for both.


+{
+struct host_its *its_data;
+its_data = xzalloc(struct host_its);
+
+if ( !its_data )
+return -1;
+
+its_data->addr = addr;
+its_data->size = size;
+if ( node )
+its_data->dt_node = node;
+
+printk("GICv3: Found ITS @0x%lx\n", addr);
+
+list_add_tail(_data->entry, _its_list);
+
+return 0;
+}
+
 /* Scan the DT for any ITS nodes and create a list of host ITSes out of it. */
 void gicv3_its_dt_init(const struct dt_device_node *node)
 {
 const struct dt_device_node *its = NULL;
-struct host_its *its_data;
-
 /*
  * Check for ITS MSI subnodes. If any, add the ITS register
  * frames to the ITS list.
@@ -996,17 +1015,8 @@ void gicv3_its_dt_init(const struct dt_device_node *node)
 if ( dt_device_get_address(its, 0, , ) )
 panic("GICv3: Cannot find a valid ITS frame address");

-its_data = xzalloc(struct host_its);
-if ( !its_data )
-panic("GICv3: Cannot allocate memory for ITS frame");
-
-its_data->addr = addr;
-its_data->size = size;
-its_data->dt_node = its;
-
-printk("GICv3: Found ITS @0x%lx\n", addr);
-
-list_add_tail(_data->entry, _its_list);
+if ( add_to_host_its_list(addr, size, its) )
+panic("GICV3: Adding Host ITS failed ");
 }
 }




--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 1/5] ARM: ITS: Introduce common function add_to_host_its_list

[Julien Grall]

Hello,

On 13/08/17 22:30, mja...@caviumnetworks.com wrote:

From: Manish Jaggi 

add_to_host_its_list will update the host_its_list. This common function to
be invoked from gicv3_its_dt_init and gic_v3_its_acpi_init.

Signed-off-by: Manish Jaggi 
---
 xen/arch/arm/gic-v3-its.c | 36 +++-
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index 2d36030..f844a0d 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -976,12 +976,31 @@ int gicv3_its_make_hwdom_dt_nodes(const struct domain *d,
 return res;
 }

+/* Common function for adding to host_its_list */
+static int add_to_host_its_list(u64 addr, u64 size, const void *node)


Why void *? node will be always assigned to dt_node and we should keep 
some type safety.


Also this function only return -1 or 0. Please use boolean.


+{
+struct host_its *its_data;


Missing newline between the declaration and the code.


+its_data = xzalloc(struct host_its);
+
+if ( !its_data )
+return -1;
+
+its_data->addr = addr;
+its_data->size = size;
+if ( node )


This check is pointless. If it is NULL then dt_node will be NULL and 
this is what we want.



+its_data->dt_node = node;
+
+printk("GICv3: Found ITS @0x%lx\n", addr);
+
+list_add_tail(_data->entry, _its_list);
+
+return 0;
+}
+
 /* Scan the DT for any ITS nodes and create a list of host ITSes out of it. */
 void gicv3_its_dt_init(const struct dt_device_node *node)
 {
 const struct dt_device_node *its = NULL;
-struct host_its *its_data;
-


Why this newline is dropped?


 /*
  * Check for ITS MSI subnodes. If any, add the ITS register
  * frames to the ITS list.
@@ -996,17 +1015,8 @@ void gicv3_its_dt_init(const struct dt_device_node *node)
 if ( dt_device_get_address(its, 0, , ) )
 panic("GICv3: Cannot find a valid ITS frame address");

-its_data = xzalloc(struct host_its);
-if ( !its_data )
-panic("GICv3: Cannot allocate memory for ITS frame");
-
-its_data->addr = addr;
-its_data->size = size;
-its_data->dt_node = its;
-
-printk("GICv3: Found ITS @0x%lx\n", addr);
-
-list_add_tail(_data->entry, _its_list);
+if ( add_to_host_its_list(addr, size, its) )
+panic("GICV3: Adding Host ITS failed ");
 }
 }




Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 1/5] ARM: ITS: Introduce common function add_to_host_its_list

[mjaggi]

From: Manish Jaggi 

add_to_host_its_list will update the host_its_list. This common function to
be invoked from gicv3_its_dt_init and gic_v3_its_acpi_init.

Signed-off-by: Manish Jaggi 
---
 xen/arch/arm/gic-v3-its.c | 36 +++-
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index 2d36030..f844a0d 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -976,12 +976,31 @@ int gicv3_its_make_hwdom_dt_nodes(const struct domain *d,
 return res;
 }
 
+/* Common function for adding to host_its_list */
+static int add_to_host_its_list(u64 addr, u64 size, const void *node)
+{
+struct host_its *its_data;
+its_data = xzalloc(struct host_its);
+
+if ( !its_data )
+return -1;
+
+its_data->addr = addr;
+its_data->size = size;
+if ( node )
+its_data->dt_node = node;
+
+printk("GICv3: Found ITS @0x%lx\n", addr);
+
+list_add_tail(_data->entry, _its_list);
+
+return 0;
+}
+
 /* Scan the DT for any ITS nodes and create a list of host ITSes out of it. */
 void gicv3_its_dt_init(const struct dt_device_node *node)
 {
 const struct dt_device_node *its = NULL;
-struct host_its *its_data;
-
 /*
  * Check for ITS MSI subnodes. If any, add the ITS register
  * frames to the ITS list.
@@ -996,17 +1015,8 @@ void gicv3_its_dt_init(const struct dt_device_node *node)
 if ( dt_device_get_address(its, 0, , ) )
 panic("GICv3: Cannot find a valid ITS frame address");
 
-its_data = xzalloc(struct host_its);
-if ( !its_data )
-panic("GICv3: Cannot allocate memory for ITS frame");
-
-its_data->addr = addr;
-its_data->size = size;
-its_data->dt_node = its;
-
-printk("GICv3: Found ITS @0x%lx\n", addr);
-
-list_add_tail(_data->entry, _its_list);
+if ( add_to_host_its_list(addr, size, its) )
+panic("GICV3: Adding Host ITS failed ");
 }
 }
 
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 1/5] ARM: ITS: Introduce common function add_to_host_its_list

[mjaggi]

From: Manish Jaggi 

add_to_host_its_list will update the host_its_list. This common function to
be invoked from gicv3_its_dt_init and gic_v3_its_acpi_init.

Signed-off-by: Manish Jaggi 
---
 xen/arch/arm/gic-v3-its.c | 36 +++-
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index 2d36030..f844a0d 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -976,12 +976,31 @@ int gicv3_its_make_hwdom_dt_nodes(const struct domain *d,
 return res;
 }
 
+/* Common function for adding to host_its_list */
+static int add_to_host_its_list(u64 addr, u64 size, const void *node)
+{
+struct host_its *its_data;
+its_data = xzalloc(struct host_its);
+
+if ( !its_data )
+return -1;
+
+its_data->addr = addr;
+its_data->size = size;
+if ( node )
+its_data->dt_node = node;
+
+printk("GICv3: Found ITS @0x%lx\n", addr);
+
+list_add_tail(_data->entry, _its_list);
+
+return 0;
+}
+
 /* Scan the DT for any ITS nodes and create a list of host ITSes out of it. */
 void gicv3_its_dt_init(const struct dt_device_node *node)
 {
 const struct dt_device_node *its = NULL;
-struct host_its *its_data;
-
 /*
  * Check for ITS MSI subnodes. If any, add the ITS register
  * frames to the ITS list.
@@ -996,17 +1015,8 @@ void gicv3_its_dt_init(const struct dt_device_node *node)
 if ( dt_device_get_address(its, 0, , ) )
 panic("GICv3: Cannot find a valid ITS frame address");
 
-its_data = xzalloc(struct host_its);
-if ( !its_data )
-panic("GICv3: Cannot allocate memory for ITS frame");
-
-its_data->addr = addr;
-its_data->size = size;
-its_data->dt_node = its;
-
-printk("GICv3: Found ITS @0x%lx\n", addr);
-
-list_add_tail(_data->entry, _its_list);
+if ( add_to_host_its_list(addr, size, its) )
+panic("GICV3: Adding Host ITS failed ");
 }
 }
 
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel