Re: [Xen-devel] [PATCH v6 08/31] xen/arm: ITS: Introduce msi_desc for LPIs

[Ian Campbell]

On Mon, 2015-08-31 at 16:36 +0530, vijay.kil...@gmail.com wrote:
> From: Vijaya Kumar K 
> 
> Define msi_desc structure for arm and introduce
> helper functions to access msi_desc member variables.
> 
> Signed-off-by: Vijaya Kumar K 
> ---
>  xen/arch/arm/gic-v3-its.c |   28 
>  xen/arch/arm/irq.c|   12 
>  xen/include/asm-arm/gic-its.h |4 
>  xen/include/asm-arm/irq.h |9 +
>  4 files changed, 53 insertions(+)
> 
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index 88cc89d..e70c21a 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -109,6 +109,34 @@ static void dump_cmd(const its_cmd_block *cmd)
>  static void dump_cmd(const its_cmd_block *cmd) { }
>  #endif
>  
> +void irqdesc_set_lpi_event(struct irq_desc *desc, unsigned id)

LPIs are logically part of the GIC, not the ITS, so I think all four of
these belong at least in a gic (without -its) .c file if not in irq.c.

Or is there a reason for them to live here?

> +{
> +ASSERT(spin_is_locked(>lock));
> +
> +irq_get_msi_desc(desc)->eventID = id;
> +}
> +
> +unsigned int irqdesc_get_lpi_event(struct irq_desc *desc)
> +{
> +ASSERT(spin_is_locked(>lock));
> +
> +return irq_get_msi_desc(desc)->eventID;
> +}
> +
> +struct its_device *irqdesc_get_its_device(struct irq_desc *desc)
> +{
> +ASSERT(spin_is_locked(>lock));
> +
> +return irq_get_msi_desc(desc)->dev;
> +}
> +
> +void irqdesc_set_its_device(struct irq_desc *desc, struct its_device
> *dev)
> +{
> +ASSERT(spin_is_locked(>lock));
> +
> +irq_get_msi_desc(desc)->dev = dev;
> +}
> +
>  static struct its_collection *dev_event_to_col(struct its_device *dev,
> u32 event)
>  {
> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> index d8080c7..24c4f24 100644
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -143,6 +143,18 @@ static inline struct domain *irq_get_domain(struct
> irq_desc *desc)
>  return irq_get_guest_info(desc)->d;
>  }
>  
> +void irq_set_msi_desc(struct irq_desc *desc, struct msi_desc *msi)
> +{
> +desc->msi_desc = msi;
> +}
> +
> +struct msi_desc *irq_get_msi_desc(struct irq_desc *desc)
> +{
> +ASSERT(desc->msi_desc != NULL);
> +
> +return desc->msi_desc;
> +}
> +
>  void irq_set_affinity(struct irq_desc *desc, const cpumask_t *cpu_mask)
>  {
>  if ( desc != NULL )
> diff --git a/xen/include/asm-arm/gic-its.h b/xen/include/asm-arm/gic
> -its.h
> index 70e7f54..25c2176 100644
> --- a/xen/include/asm-arm/gic-its.h
> +++ b/xen/include/asm-arm/gic-its.h
> @@ -271,6 +271,10 @@ struct its_device {
>  struct rb_node  node;
>  };
>  
> +void irqdesc_set_lpi_event(struct irq_desc *desc, unsigned id);
> +unsigned int irqdesc_get_lpi_event(struct irq_desc *desc);
> +struct its_device *irqdesc_get_its_device(struct irq_desc *desc);
> +void irqdesc_set_its_device(struct irq_desc *desc, struct its_device
> *dev);
>  int its_init(struct rdist_prop *rdists);
>  int its_cpu_init(void);
>  
> diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h
> index cbdc1ab..bddd1ea 100644
> --- a/xen/include/asm-arm/irq.h
> +++ b/xen/include/asm-arm/irq.h
> @@ -18,6 +18,13 @@ struct arch_irq_desc {
>  unsigned int type;
>  };
>  
> +struct msi_desc {
> +#ifdef HAS_GICV3
> +unsigned int eventID;
> +struct its_device *dev;
> +#endif
> +};
> +
>  #define NR_LOCAL_IRQS32
>  /* Number of SGIs+PPIs+SPIs */
>  #define NR_LINE_IRQS 1024
> @@ -56,6 +63,8 @@ int irq_set_spi_type(unsigned int spi, unsigned int
> type);
>  int platform_get_irq(const struct dt_device_node *device, int index);
>  
>  void irq_set_affinity(struct irq_desc *desc, const cpumask_t *cpu_mask);
> +void irq_set_msi_desc(struct irq_desc *desc, struct msi_desc *msi);
> +struct msi_desc *irq_get_msi_desc(struct irq_desc *desc);
>  
>  #endif /* _ASM_HW_IRQ_H */
>  /*

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

Re: [Xen-devel] [PATCH v6 08/31] xen/arm: ITS: Introduce msi_desc for LPIs

[Julien Grall]

On 09/09/15 14:16, Ian Campbell wrote:
> On Mon, 2015-08-31 at 16:36 +0530, vijay.kil...@gmail.com wrote:
>> From: Vijaya Kumar K 
>>
>> Define msi_desc structure for arm and introduce
>> helper functions to access msi_desc member variables.
>>
>> Signed-off-by: Vijaya Kumar K 
>> ---
>>  xen/arch/arm/gic-v3-its.c |   28 
>>  xen/arch/arm/irq.c|   12 
>>  xen/include/asm-arm/gic-its.h |4 
>>  xen/include/asm-arm/irq.h |9 +
>>  4 files changed, 53 insertions(+)
>>
>> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
>> index 88cc89d..e70c21a 100644
>> --- a/xen/arch/arm/gic-v3-its.c
>> +++ b/xen/arch/arm/gic-v3-its.c
>> @@ -109,6 +109,34 @@ static void dump_cmd(const its_cmd_block *cmd)
>>  static void dump_cmd(const its_cmd_block *cmd) { }
>>  #endif
>>  
>> +void irqdesc_set_lpi_event(struct irq_desc *desc, unsigned id)
> 
> LPIs are logically part of the GIC, not the ITS, so I think all four of
> these belong at least in a gic (without -its) .c file if not in irq.c.
> 
> Or is there a reason for them to live here?

Putting those helpers in irq.c will require some #ifdery in order to not
compile when GICV3 is not built. I'd like to avoid any #ifdef in the
common code for ITS specific code (see irqdesc_set_its_device).

Although, as I said in a previous mail, those helpers are pointless as
they are only used a couple of time and always in a batch. So we should
result to fetch MSI everytime we'd like to access a field.

Regards,

-- 
Julien Grall

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

Re: [Xen-devel] [PATCH v6 08/31] xen/arm: ITS: Introduce msi_desc for LPIs

[Julien Grall]

Hi Vijay,

On 31/08/2015 12:06, vijay.kil...@gmail.com wrote:

From: Vijaya Kumar K 

Define msi_desc structure for arm and introduce
helper functions to access msi_desc member variables.


IHMO none of those helpers are useful in the code given you are only 
using in an handful number of places and they can't be optimized out.


Most of the time, you will have 2-3 times within the same function the 
msi_desc. So it means 6 functions call rather than directly access the 
msi_desc from the irq_desc.


Although, I guess we could rework later, so for now:

Reviewed-by: Julien Grall 

Regards,

--
Julien Grall

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

[Xen-devel] [PATCH v6 08/31] xen/arm: ITS: Introduce msi_desc for LPIs

[vijay . kilari]

From: Vijaya Kumar K 

Define msi_desc structure for arm and introduce
helper functions to access msi_desc member variables.

Signed-off-by: Vijaya Kumar K 
---
 xen/arch/arm/gic-v3-its.c |   28 
 xen/arch/arm/irq.c|   12 
 xen/include/asm-arm/gic-its.h |4 
 xen/include/asm-arm/irq.h |9 +
 4 files changed, 53 insertions(+)

diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index 88cc89d..e70c21a 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -109,6 +109,34 @@ static void dump_cmd(const its_cmd_block *cmd)
 static void dump_cmd(const its_cmd_block *cmd) { }
 #endif
 
+void irqdesc_set_lpi_event(struct irq_desc *desc, unsigned id)
+{
+ASSERT(spin_is_locked(>lock));
+
+irq_get_msi_desc(desc)->eventID = id;
+}
+
+unsigned int irqdesc_get_lpi_event(struct irq_desc *desc)
+{
+ASSERT(spin_is_locked(>lock));
+
+return irq_get_msi_desc(desc)->eventID;
+}
+
+struct its_device *irqdesc_get_its_device(struct irq_desc *desc)
+{
+ASSERT(spin_is_locked(>lock));
+
+return irq_get_msi_desc(desc)->dev;
+}
+
+void irqdesc_set_its_device(struct irq_desc *desc, struct its_device *dev)
+{
+ASSERT(spin_is_locked(>lock));
+
+irq_get_msi_desc(desc)->dev = dev;
+}
+
 static struct its_collection *dev_event_to_col(struct its_device *dev,
u32 event)
 {
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index d8080c7..24c4f24 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -143,6 +143,18 @@ static inline struct domain *irq_get_domain(struct 
irq_desc *desc)
 return irq_get_guest_info(desc)->d;
 }
 
+void irq_set_msi_desc(struct irq_desc *desc, struct msi_desc *msi)
+{
+desc->msi_desc = msi;
+}
+
+struct msi_desc *irq_get_msi_desc(struct irq_desc *desc)
+{
+ASSERT(desc->msi_desc != NULL);
+
+return desc->msi_desc;
+}
+
 void irq_set_affinity(struct irq_desc *desc, const cpumask_t *cpu_mask)
 {
 if ( desc != NULL )
diff --git a/xen/include/asm-arm/gic-its.h b/xen/include/asm-arm/gic-its.h
index 70e7f54..25c2176 100644
--- a/xen/include/asm-arm/gic-its.h
+++ b/xen/include/asm-arm/gic-its.h
@@ -271,6 +271,10 @@ struct its_device {
 struct rb_node  node;
 };
 
+void irqdesc_set_lpi_event(struct irq_desc *desc, unsigned id);
+unsigned int irqdesc_get_lpi_event(struct irq_desc *desc);
+struct its_device *irqdesc_get_its_device(struct irq_desc *desc);
+void irqdesc_set_its_device(struct irq_desc *desc, struct its_device *dev);
 int its_init(struct rdist_prop *rdists);
 int its_cpu_init(void);
 
diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h
index cbdc1ab..bddd1ea 100644
--- a/xen/include/asm-arm/irq.h
+++ b/xen/include/asm-arm/irq.h
@@ -18,6 +18,13 @@ struct arch_irq_desc {
 unsigned int type;
 };
 
+struct msi_desc {
+#ifdef HAS_GICV3
+unsigned int eventID;
+struct its_device *dev;
+#endif
+};
+
 #define NR_LOCAL_IRQS  32
 /* Number of SGIs+PPIs+SPIs */
 #define NR_LINE_IRQS   1024
@@ -56,6 +63,8 @@ int irq_set_spi_type(unsigned int spi, unsigned int type);
 int platform_get_irq(const struct dt_device_node *device, int index);
 
 void irq_set_affinity(struct irq_desc *desc, const cpumask_t *cpu_mask);
+void irq_set_msi_desc(struct irq_desc *desc, struct msi_desc *msi);
+struct msi_desc *irq_get_msi_desc(struct irq_desc *desc);
 
 #endif /* _ASM_HW_IRQ_H */
 /*
-- 
1.7.9.5


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