[Xen-devel] [PATCH v5 29/30] ARM: vITS: create ITS subnodes for Dom0 DT

2017-04-05 Thread Andre Przywara
Dom0 expects all ITSes in the system to be propagated to be able to
use MSIs.
Create Dom0 DT nodes for each hardware ITS, keeping the register frame
address the same, as the doorbell address that the Dom0 drivers program
into the BARs has to match the hardware.

Signed-off-by: Andre Przywara 
---
 xen/arch/arm/gic-v3-its.c| 78 
 xen/arch/arm/gic-v3.c|  4 ++-
 xen/include/asm-arm/gic_v3_its.h | 13 +++
 3 files changed, 94 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index a57e63a..a167471 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -20,6 +20,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -875,6 +876,83 @@ int gicv3_lpi_change_vcpu(struct domain *d, paddr_t 
vdoorbell,
 return 0;
 }
 
+/*
+ * Create the respective guest DT nodes for a list of host ITSes.
+ * This copies the reg property, so the guest sees the ITS at the same address
+ * as the host.
+ * Giving NULL for the its_list will make it use the list of host ITSes.
+ */
+int gicv3_its_make_dt_nodes(struct list_head *its_list,
+const struct domain *d,
+const struct dt_device_node *gic,
+void *fdt)
+{
+uint32_t len;
+int res;
+const void *prop = NULL;
+const struct dt_device_node *its = NULL;
+const struct host_its *its_data;
+
+if ( !its_list )
+its_list = &host_its_list;
+
+if ( list_empty(its_list) )
+return 0;
+
+/* The sub-nodes require the ranges property */
+prop = dt_get_property(gic, "ranges", &len);
+if ( !prop )
+{
+printk(XENLOG_ERR "Can't find ranges property for the gic node\n");
+return -FDT_ERR_XEN(ENOENT);
+}
+
+res = fdt_property(fdt, "ranges", prop, len);
+if ( res )
+return res;
+
+list_for_each_entry(its_data, its_list, entry)
+{
+its = its_data->dt_node;
+
+res = fdt_begin_node(fdt, its->name);
+if ( res )
+return res;
+
+res = fdt_property_string(fdt, "compatible", "arm,gic-v3-its");
+if ( res )
+return res;
+
+res = fdt_property(fdt, "msi-controller", NULL, 0);
+if ( res )
+return res;
+
+if ( its->phandle )
+{
+res = fdt_property_cell(fdt, "phandle", its->phandle);
+if ( res )
+return res;
+}
+
+/* Use the same reg regions as the ITS node in host DTB. */
+prop = dt_get_property(its, "reg", &len);
+if ( !prop )
+{
+printk(XENLOG_ERR "GICv3: Can't find ITS reg property.\n");
+res = -FDT_ERR_XEN(ENOENT);
+return res;
+}
+
+res = fdt_property(fdt, "reg", prop, len);
+if ( res )
+return res;
+
+fdt_end_node(fdt);
+}
+
+return res;
+}
+
 /* 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)
 {
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 54fbb19..2fbcf52 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1172,8 +1172,10 @@ static int gicv3_make_hwdom_dt_node(const struct domain 
*d,
 
 res = fdt_property(fdt, "reg", new_cells, len);
 xfree(new_cells);
+if ( res )
+return res;
 
-return res;
+return gicv3_its_make_dt_nodes(NULL, d, gic, fdt);
 }
 
 static const hw_irq_controller gicv3_host_irq_type = {
diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h
index 1b8e47c..6538916 100644
--- a/xen/include/asm-arm/gic_v3_its.h
+++ b/xen/include/asm-arm/gic_v3_its.h
@@ -165,6 +165,12 @@ void vgic_v3_its_free_domain(struct domain *d);
 int vgic_v3_its_init_virtual(struct domain *d, paddr_t guest_addr,
 unsigned int devid_bits, unsigned int intid_bits);
 
+/* Given a list of ITSes, create the appropriate DT nodes for a domain. */
+int gicv3_its_make_dt_nodes(struct list_head *its_list,
+const struct domain *d,
+const struct dt_device_node *gic,
+void *fdt);
+
 /*
  * Map a device on the host by allocating an ITT on the host (ITS).
  * "nr_event" specifies how many events (interrupts) this device will need.
@@ -248,6 +254,13 @@ static inline int vgic_v3_its_init_virtual(struct domain 
*d,
 {
 return 0;
 }
+static inline int gicv3_its_make_dt_nodes(struct list_head *its_list,
+   const struct domain *d,
+   const struct dt_device_node *gic,
+   void *fdt)
+{
+return 0;
+}
 
 #endif /* CONFIG_HAS_ITS */
 
-- 
2.8.2


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

Re: [Xen-devel] [PATCH v5 29/30] ARM: vITS: create ITS subnodes for Dom0 DT

2017-04-07 Thread Julien Grall

Hi Andre,

On 06/04/17 00:19, Andre Przywara wrote:

Dom0 expects all ITSes in the system to be propagated to be able to
use MSIs.
Create Dom0 DT nodes for each hardware ITS, keeping the register frame
address the same, as the doorbell address that the Dom0 drivers program
into the BARs has to match the hardware.

Signed-off-by: Andre Przywara 
---
 xen/arch/arm/gic-v3-its.c| 78 
 xen/arch/arm/gic-v3.c|  4 ++-
 xen/include/asm-arm/gic_v3_its.h | 13 +++
 3 files changed, 94 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index a57e63a..a167471 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -20,6 +20,7 @@

 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -875,6 +876,83 @@ int gicv3_lpi_change_vcpu(struct domain *d, paddr_t 
vdoorbell,
 return 0;
 }

+/*
+ * Create the respective guest DT nodes for a list of host ITSes.
+ * This copies the reg property, so the guest sees the ITS at the same address
+ * as the host.
+ * Giving NULL for the its_list will make it use the list of host ITSes.
+ */
+int gicv3_its_make_dt_nodes(struct list_head *its_list,


Why do you have this parameter its_list that you always set to NULL?


+const struct domain *d,
+const struct dt_device_node *gic,
+void *fdt)
+{
+uint32_t len;
+int res;
+const void *prop = NULL;
+const struct dt_device_node *its = NULL;
+const struct host_its *its_data;
+
+if ( !its_list )
+its_list = &host_its_list;
+
+if ( list_empty(its_list) )
+return 0;
+
+/* The sub-nodes require the ranges property */
+prop = dt_get_property(gic, "ranges", &len);
+if ( !prop )
+{
+printk(XENLOG_ERR "Can't find ranges property for the gic node\n");
+return -FDT_ERR_XEN(ENOENT);
+}
+
+res = fdt_property(fdt, "ranges", prop, len);
+if ( res )
+return res;
+
+list_for_each_entry(its_data, its_list, entry)
+{
+its = its_data->dt_node;
+
+res = fdt_begin_node(fdt, its->name);
+if ( res )
+return res;
+
+res = fdt_property_string(fdt, "compatible", "arm,gic-v3-its");
+if ( res )
+return res;
+
+res = fdt_property(fdt, "msi-controller", NULL, 0);
+if ( res )
+return res;
+
+if ( its->phandle )
+{
+res = fdt_property_cell(fdt, "phandle", its->phandle);
+if ( res )
+return res;
+}
+
+/* Use the same reg regions as the ITS node in host DTB. */
+prop = dt_get_property(its, "reg", &len);
+if ( !prop )
+{
+printk(XENLOG_ERR "GICv3: Can't find ITS reg property.\n");
+res = -FDT_ERR_XEN(ENOENT);
+return res;
+}
+
+res = fdt_property(fdt, "reg", prop, len);
+if ( res )
+return res;
+
+fdt_end_node(fdt);
+}
+
+return res;
+}
+
 /* 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)
 {
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 54fbb19..2fbcf52 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1172,8 +1172,10 @@ static int gicv3_make_hwdom_dt_node(const struct domain 
*d,

 res = fdt_property(fdt, "reg", new_cells, len);
 xfree(new_cells);
+if ( res )
+return res;

-return res;
+return gicv3_its_make_dt_nodes(NULL, d, gic, fdt);
 }

 static const hw_irq_controller gicv3_host_irq_type = {
diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h
index 1b8e47c..6538916 100644
--- a/xen/include/asm-arm/gic_v3_its.h
+++ b/xen/include/asm-arm/gic_v3_its.h
@@ -165,6 +165,12 @@ void vgic_v3_its_free_domain(struct domain *d);
 int vgic_v3_its_init_virtual(struct domain *d, paddr_t guest_addr,
 unsigned int devid_bits, unsigned int intid_bits);

+/* Given a list of ITSes, create the appropriate DT nodes for a domain. */
+int gicv3_its_make_dt_nodes(struct list_head *its_list,
+const struct domain *d,
+const struct dt_device_node *gic,
+void *fdt);
+
 /*
  * Map a device on the host by allocating an ITT on the host (ITS).
  * "nr_event" specifies how many events (interrupts) this device will need.
@@ -248,6 +254,13 @@ static inline int vgic_v3_its_init_virtual(struct domain 
*d,
 {
 return 0;
 }
+static inline int gicv3_its_make_dt_nodes(struct list_head *its_list,
+   const struct domain *d,
+   const struct dt_device_node *gic,
+   void *fdt)
+{
+return 0;
+}

 #endif /* CONFIG_HAS_ITS *

Re: [Xen-devel] [PATCH v5 29/30] ARM: vITS: create ITS subnodes for Dom0 DT

2017-04-07 Thread Andre Przywara
Hi,

On 07/04/17 14:41, Julien Grall wrote:
> Hi Andre,
> 
> On 06/04/17 00:19, Andre Przywara wrote:
>> Dom0 expects all ITSes in the system to be propagated to be able to
>> use MSIs.
>> Create Dom0 DT nodes for each hardware ITS, keeping the register frame
>> address the same, as the doorbell address that the Dom0 drivers program
>> into the BARs has to match the hardware.
>>
>> Signed-off-by: Andre Przywara 
>> ---
>>  xen/arch/arm/gic-v3-its.c| 78
>> 
>>  xen/arch/arm/gic-v3.c|  4 ++-
>>  xen/include/asm-arm/gic_v3_its.h | 13 +++
>>  3 files changed, 94 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
>> index a57e63a..a167471 100644
>> --- a/xen/arch/arm/gic-v3-its.c
>> +++ b/xen/arch/arm/gic-v3-its.c
>> @@ -20,6 +20,7 @@
>>
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -875,6 +876,83 @@ int gicv3_lpi_change_vcpu(struct domain *d,
>> paddr_t vdoorbell,
>>  return 0;
>>  }
>>
>> +/*
>> + * Create the respective guest DT nodes for a list of host ITSes.
>> + * This copies the reg property, so the guest sees the ITS at the
>> same address
>> + * as the host.
>> + * Giving NULL for the its_list will make it use the list of host ITSes.
>> + */
>> +int gicv3_its_make_dt_nodes(struct list_head *its_list,
> 
> Why do you have this parameter its_list that you always set to NULL?

The idea is to later allow only a subset of ITSes to be mapped into a
guest. In this case this function would be called with a specific
version of the list.
So this feature is basically just in for future DomU support.

Cheers,
Andre.

> 
>> +const struct domain *d,
>> +const struct dt_device_node *gic,
>> +void *fdt)
>> +{
>> +uint32_t len;
>> +int res;
>> +const void *prop = NULL;
>> +const struct dt_device_node *its = NULL;
>> +const struct host_its *its_data;
>> +
>> +if ( !its_list )
>> +its_list = &host_its_list;
>> +
>> +if ( list_empty(its_list) )
>> +return 0;
>> +
>> +/* The sub-nodes require the ranges property */
>> +prop = dt_get_property(gic, "ranges", &len);
>> +if ( !prop )
>> +{
>> +printk(XENLOG_ERR "Can't find ranges property for the gic
>> node\n");
>> +return -FDT_ERR_XEN(ENOENT);
>> +}
>> +
>> +res = fdt_property(fdt, "ranges", prop, len);
>> +if ( res )
>> +return res;
>> +
>> +list_for_each_entry(its_data, its_list, entry)
>> +{
>> +its = its_data->dt_node;
>> +
>> +res = fdt_begin_node(fdt, its->name);
>> +if ( res )
>> +return res;
>> +
>> +res = fdt_property_string(fdt, "compatible", "arm,gic-v3-its");
>> +if ( res )
>> +return res;
>> +
>> +res = fdt_property(fdt, "msi-controller", NULL, 0);
>> +if ( res )
>> +return res;
>> +
>> +if ( its->phandle )
>> +{
>> +res = fdt_property_cell(fdt, "phandle", its->phandle);
>> +if ( res )
>> +return res;
>> +}
>> +
>> +/* Use the same reg regions as the ITS node in host DTB. */
>> +prop = dt_get_property(its, "reg", &len);
>> +if ( !prop )
>> +{
>> +printk(XENLOG_ERR "GICv3: Can't find ITS reg property.\n");
>> +res = -FDT_ERR_XEN(ENOENT);
>> +return res;
>> +}
>> +
>> +res = fdt_property(fdt, "reg", prop, len);
>> +if ( res )
>> +return res;
>> +
>> +fdt_end_node(fdt);
>> +}
>> +
>> +return res;
>> +}
>> +
>>  /* 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)
>>  {
>> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
>> index 54fbb19..2fbcf52 100644
>> --- a/xen/arch/arm/gic-v3.c
>> +++ b/xen/arch/arm/gic-v3.c
>> @@ -1172,8 +1172,10 @@ static int gicv3_make_hwdom_dt_node(const
>> struct domain *d,
>>
>>  res = fdt_property(fdt, "reg", new_cells, len);
>>  xfree(new_cells);
>> +if ( res )
>> +return res;
>>
>> -return res;
>> +return gicv3_its_make_dt_nodes(NULL, d, gic, fdt);
>>  }
>>
>>  static const hw_irq_controller gicv3_host_irq_type = {
>> diff --git a/xen/include/asm-arm/gic_v3_its.h
>> b/xen/include/asm-arm/gic_v3_its.h
>> index 1b8e47c..6538916 100644
>> --- a/xen/include/asm-arm/gic_v3_its.h
>> +++ b/xen/include/asm-arm/gic_v3_its.h
>> @@ -165,6 +165,12 @@ void vgic_v3_its_free_domain(struct domain *d);
>>  int vgic_v3_its_init_virtual(struct domain *d, paddr_t guest_addr,
>>   unsigned int devid_bits, unsigned int intid_bits);
>>
>> +/* Given a list of ITSes, create the appropriate DT nodes for a
>> domain. */
>> +int gicv3_its_make_dt_nodes(struct list_head *its_list,
>> + 

Re: [Xen-devel] [PATCH v5 29/30] ARM: vITS: create ITS subnodes for Dom0 DT

2017-04-07 Thread Julien Grall



On 07/04/17 14:46, Andre Przywara wrote:

Hi,

On 07/04/17 14:41, Julien Grall wrote:

Hi Andre,

On 06/04/17 00:19, Andre Przywara wrote:

Dom0 expects all ITSes in the system to be propagated to be able to
use MSIs.
Create Dom0 DT nodes for each hardware ITS, keeping the register frame
address the same, as the doorbell address that the Dom0 drivers program
into the BARs has to match the hardware.

Signed-off-by: Andre Przywara 
---
 xen/arch/arm/gic-v3-its.c| 78

 xen/arch/arm/gic-v3.c|  4 ++-
 xen/include/asm-arm/gic_v3_its.h | 13 +++
 3 files changed, 94 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index a57e63a..a167471 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -20,6 +20,7 @@

 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -875,6 +876,83 @@ int gicv3_lpi_change_vcpu(struct domain *d,
paddr_t vdoorbell,
 return 0;
 }

+/*
+ * Create the respective guest DT nodes for a list of host ITSes.
+ * This copies the reg property, so the guest sees the ITS at the
same address
+ * as the host.
+ * Giving NULL for the its_list will make it use the list of host ITSes.
+ */
+int gicv3_its_make_dt_nodes(struct list_head *its_list,


Why do you have this parameter its_list that you always set to NULL?


The idea is to later allow only a subset of ITSes to be mapped into a
guest. In this case this function would be called with a specific
version of the list.
So this feature is basically just in for future DomU support.


DomU DTS are created by the toolstack and not Xen. So please drop this 
pointless variable.


Cheers,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v5 29/30] ARM: vITS: create ITS subnodes for Dom0 DT

2017-04-07 Thread Julien Grall



On 07/04/17 14:45, Julien Grall wrote:



On 07/04/17 14:46, Andre Przywara wrote:

Hi,

On 07/04/17 14:41, Julien Grall wrote:

Hi Andre,

On 06/04/17 00:19, Andre Przywara wrote:

Dom0 expects all ITSes in the system to be propagated to be able to
use MSIs.
Create Dom0 DT nodes for each hardware ITS, keeping the register frame
address the same, as the doorbell address that the Dom0 drivers program
into the BARs has to match the hardware.

Signed-off-by: Andre Przywara 
---
 xen/arch/arm/gic-v3-its.c| 78

 xen/arch/arm/gic-v3.c|  4 ++-
 xen/include/asm-arm/gic_v3_its.h | 13 +++
 3 files changed, 94 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index a57e63a..a167471 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -20,6 +20,7 @@

 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -875,6 +876,83 @@ int gicv3_lpi_change_vcpu(struct domain *d,
paddr_t vdoorbell,
 return 0;
 }

+/*
+ * Create the respective guest DT nodes for a list of host ITSes.
+ * This copies the reg property, so the guest sees the ITS at the
same address
+ * as the host.
+ * Giving NULL for the its_list will make it use the list of host
ITSes.
+ */
+int gicv3_its_make_dt_nodes(struct list_head *its_list,


Why do you have this parameter its_list that you always set to NULL?


The idea is to later allow only a subset of ITSes to be mapped into a
guest. In this case this function would be called with a specific
version of the list.
So this feature is basically just in for future DomU support.


DomU DTS are created by the toolstack and not Xen. So please drop this
pointless variable.


To complete what I just said, DOM0 will always have a virtual ITS per 
host ITS not matter whether they will be used. So there is really no 
point of that.


Cheers,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v5 29/30] ARM: vITS: create ITS subnodes for Dom0 DT

2017-04-07 Thread Andre Przywara
Hi,

On 07/04/17 14:47, Julien Grall wrote:
> 
> 
> On 07/04/17 14:45, Julien Grall wrote:
>>
>>
>> On 07/04/17 14:46, Andre Przywara wrote:
>>> Hi,
>>>
>>> On 07/04/17 14:41, Julien Grall wrote:
 Hi Andre,

 On 06/04/17 00:19, Andre Przywara wrote:
> Dom0 expects all ITSes in the system to be propagated to be able to
> use MSIs.
> Create Dom0 DT nodes for each hardware ITS, keeping the register frame
> address the same, as the doorbell address that the Dom0 drivers
> program
> into the BARs has to match the hardware.
>
> Signed-off-by: Andre Przywara 
> ---
>  xen/arch/arm/gic-v3-its.c| 78
> 
>  xen/arch/arm/gic-v3.c|  4 ++-
>  xen/include/asm-arm/gic_v3_its.h | 13 +++
>  3 files changed, 94 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index a57e63a..a167471 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -20,6 +20,7 @@
>
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -875,6 +876,83 @@ int gicv3_lpi_change_vcpu(struct domain *d,
> paddr_t vdoorbell,
>  return 0;
>  }
>
> +/*
> + * Create the respective guest DT nodes for a list of host ITSes.
> + * This copies the reg property, so the guest sees the ITS at the
> same address
> + * as the host.
> + * Giving NULL for the its_list will make it use the list of host
> ITSes.
> + */
> +int gicv3_its_make_dt_nodes(struct list_head *its_list,

 Why do you have this parameter its_list that you always set to NULL?
>>>
>>> The idea is to later allow only a subset of ITSes to be mapped into a
>>> guest. In this case this function would be called with a specific
>>> version of the list.
>>> So this feature is basically just in for future DomU support.
>>
>> DomU DTS are created by the toolstack and not Xen. So please drop this
>> pointless variable.
> 
> To complete what I just said, DOM0 will always have a virtual ITS per
> host ITS not matter whether they will be used. So there is really no
> point of that.

OK, fair enough.

Thanks!
Andre.

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